Constrain data encryption. #1139
Labels
A-security
Area: Relates to security. Something is insecure.
C-aztec.nr
Component: Aztec smart contract framework
Comments
iAmMichaelConnor
added
C-aztec.nr
This was referenced May 15, 2024
MirandaWood
added a commit
that referenced
this issue
May 16, 2024
Closes #1641 ## This PR: ### Logs as `SideEffect`s -> `LogHash` or `NoteLogHash` Adds new structs to track logs in the kernel circuits, so we can track individual log lengths (and assign gas accordingly) and separate logs linked to notes from generic logs. The length changes contribute to #4712 but this PR does not close it - we need some more work in ts to ensure logs from the non-revertible side actually persist to the tx. ### Adds `note_encrypted_logs` To aid the kernels/pxe/note dbs in managing note encrypted logs, most of the changed files are adding a new array of note logs to anything already containing `.encrypted_logs_<hashes/hash>`. Since all our current contracts/tests only emit encrypted logs for notes, the values expected in `.encrypted_logs` have basically moved over to `.note_encrypted_logs`. A later PR will add code for generic encrypted logs (see below). Includes adding logs to the note cache in `client_execution_context` and removing `LogsCache`, as the note cache handles note specific logs. ### Squashes transient note logs Removes any logs linked to note hashes nullified in the same tx. This involves adding note logs to transient hints for the private kernel tail and chopping them inside the circuit. ### TODOs/Notes - This PR ended up pretty large, so while we can silo log hashes now, it would be cleaner to do in a follow up PR. - There are some small changes to public/AVM contexts which are mostly to replace `SideEffect` with `LogHash` and a couple of things addressed in comments below. - Currently, the only encrypted logs that are emitted are linked to notes. To allow for generic values emitted as encrypted logs, I'll need to add a new oracle call and methods, which makes sense to do once we encrypt inside the circuit (#1139). - Total logs length (e.g. `encrypted_log_preimages_length` is no longer required now we track indvidual lengths (it's also not trusted since it comes from contexts), so it can be removed from the kernels. - We originally tried to track log lengths the same way as `ts`, which adds 4 bytes to each nested 'group' of logs: `L2BlockL2Logs.txLogs = TxL2Logs -> .functionLogs = FunctionL2Logs -> .logs = <Un>EncryptedL2Log[]` but this never actually matched up because we accumulate and sort logs regardless of what call they came from. E.g. 3 groups of `FunctionL2Logs` would each have +4 length when added to a tx, but we take the contents, sort them, then add to a single `FunctionL2Logs.logs` array before assigning to a tx, so end up with a single +4 length. This length value is only used on L1 to destructure the blob of bytes representing logs. Any incorrect lengths would lead to a logs hash mismatch, so we don't need to match them in the circuit anyway. For this reason, I've removed a few unnecessary +4s from the circuits.
LHerskind
added a commit
that referenced
this issue
May 18, 2024
Fixes #5901. Uses the new format to match the keys and logs spec. The current implementation here is still using a typescript implementation to encrypt the data, mainly using the new flow. The intention is that #6408 and #1139 can be addresses separately and than we can just replace the import to use the constrained version instead of the oracle at that point. Note, that the outgoing logs are currently less than meaningful, as some of the infrastructure is not yet in place to handle those nicely, see #6410 for more on that. What was called the encrypted_log_payload in #6348 have been moved into the l1_payload, to better integrate with the rest of the setup.
signorecello
pushed a commit
that referenced
this issue
May 20, 2024
Fixes #5901. Uses the new format to match the keys and logs spec. The current implementation here is still using a typescript implementation to encrypt the data, mainly using the new flow. The intention is that #6408 and #1139 can be addresses separately and than we can just replace the import to use the constrained version instead of the oracle at that point. Note, that the outgoing logs are currently less than meaningful, as some of the infrastructure is not yet in place to handle those nicely, see What was called the encrypted_log_payload in #6348 have been moved into the l1_payload, to better integrate with the rest of the setup.
|
This was done for notes in #6408 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we are encrypting data in the simulator (private_execution.ts). But we should do that in the circuit to ensure the data is encrypted with the correct key and algorithm.
The text was updated successfully, but these errors were encountered: