feat!: ciphertext padding - #630
Conversation
Ciphers submitted currently have no data
BREAKING! Before: The ciphertext key got generated via using the commmitment and its index as the key. After: The ciphertext key replaces the commitment dependence by a nullifier dependence, still making the key unique by global state. The nullifier is assumed to be the nullifier of the pre-state of the account. Mitigation: Use the new decryption algorithm.
BREAKING! Before: Commitments, nullifier, and ciphertexts of a transaction were position-aligned. The position revealed which nullifier was hiding a pre-state to a commitment to which post-state. Similarly with ciphertexts. After: The nullifier-cipher connection remains, but now they (and commitments independently) are shuffled by the byte-order of the hashes. Mitigation: Expect the circuit outputs to be jumbled.
BREAKING! Before: The message format for private transactions included separate fields for private transaction state identifiers such as commitments and nullifiers. After: The commitments/nullifiers/ciphertexts are bound in an Action struct, likewise for public state-bearing messages. Mitigation: assume incoming messages are in a newly-specified format.
Benchmark for d88ed21Click to view benchmark
|
fdc06fe to
da15030
Compare
Benchmark for 8ea7f3dClick to view benchmark
|
da15030 to
9a5ee3c
Compare
Benchmark for 4a923ecClick to view benchmark
|
BREAKING! Before: Ciphertexts get generated without padding. After: The circuit expects maximum padding and pads the plaintext of the ChaCha cipher accordingly. Mitigation: Expect possible panics on undervalued padding lengths and expect larger possible load sized.
9a5ee3c to
dfaf151
Compare
Benchmark for c40bd1aClick to view benchmark
|
| buffer.extend_from_slice(&account.to_bytes()); | ||
| if let Some(pad_to_len) = pad_to_len { | ||
| let pad_to_len = usize::try_from(pad_to_len).expect("pad length fits in usize"); | ||
| assert!( |
There was a problem hiding this comment.
Padding closes the length leak nicely. One robustness note: the pad_to_len >= buffer.len() assert runs in-guest, so any account whose plaintext exceeds CIPHERTEXT_PAD_SIZE panics proving and the wallet can't send any private tx for it. At pad 512, HEADER_LEN (81) + 68 overhead leaves ~363 bytes for data, vs DATA_MAX_LENGTH (100 KiB) allowed, so it's latent but could bite on larger payloads. Could we add a host-side check before env write returning a typed error naming the account and overflow, instead of an opaque in-guest panic?
| NullifierPublicKey, NullifierSecretKey, SharedSecretKey, | ||
| account::{AccountWithMetadata, Nonce}, | ||
| compute_digest_for_path, | ||
| encryption::{ |
There was a problem hiding this comment.
Padding is wired always-on here (Some(CIPHERTEXT_PAD_SIZE) at the one call site), so the issue's opt-out for users who'd rather not pay the storage cost isn't reachable from the wallet, every private tx now writes 7×512 bytes. Reasonable as a default, could you add a line noting the always-on is deliberate, or leave a knob for later?
There was a problem hiding this comment.
Padding closes the #562 length leak cleanly, but i think that the added tests only assert padded ciphertext length. A padded encrypt then decrypt roundtrip would be worth adding, since the trailing-zero tolerance in Account::from_cursor is what keeps padded notes decryptable and nothing currently pins it. Encrypt with Some(512), decrypt, assert the original kind and account come back.
9a2c9a2 to
846e194
Compare
2 participants
🎯 Purpose
This issue tackles #562. Currently the data size of the encrypted post-state is visible via the ChaCha encryption as it is variable size and unpadded, This PR introduces an opt-in padding mechanism.
The user now supplies an
Option<u32>to the PPC specifying to what size does the user want all of the ciphertexts padded. The idea is that if they do not want to pay for storage, they do not need to. If they want to pad, then it is best all notes are padded to the same size so as to not leak in-transaction data. However the circuit itself does not contain exactly how much we pad this is user-supplied.The wallet behavior is changed to pad everything up to 512 bytes, which is ~200 bytes above the largest AMM program data there is, giving ample room to other applications. The idea is that once an application using a larger data field arrives, the users will simply change the hardcoded size to increase the privacy pool if needed.
⚙️ Approach
🧪 How to Test
cargo test -p lee_core --features host encrypt_p🔗 Dependencies
#604
🔜 Future Work
Consider whether individual padding is needed or whether a smaller pad ammount is better.
📋 PR Completion Checklist