fix(aztec-nr): account for AES PKCS#7 padding in message plaintext length (#20840)

Author: nchamo

noir-projects/aztec-nr/aztec/src/messages/encoding.nr

@@ -9,20 +9,24 @@ use crate::utils::array;
 // fields, so MESSAGE_CIPHERTEXT_LEN is the size of the message in fields.
 pub global MESSAGE_CIPHERTEXT_LEN: u32 = PRIVATE_LOG_CIPHERTEXT_LEN;
 
-// TODO(#12750): The global variables below should not be here as they are AES128 specific. ciphertext_length (2) + 14
-// bytes pkcs#7 AES padding.
+// TODO(#12750): The global variables below should not be here as they are AES128 specific.
+// The header plaintext is 2 bytes (ciphertext length), padded to the 16-byte AES block size by PKCS#7.
 pub(crate) global HEADER_CIPHERTEXT_SIZE_IN_BYTES: u32 = 16;
+// AES PKCS#7 always adds at least one byte of padding. Since each plaintext field is 32 bytes (a multiple of the
+// 16-byte AES block size), a full 16-byte padding block is always appended.
+pub(crate) global AES128_PKCS7_EXPANSION_IN_BYTES: u32 = 16;
 
 pub global EPH_PK_X_SIZE_IN_FIELDS: u32 = 1;
 pub global EPH_PK_SIGN_BYTE_SIZE_IN_BYTES: u32 = 1;
 
-// (17 - 1) * 31 - 16 - 1 = 479 Note: We multiply by 31 because ciphertext bytes are stored in fields using
+// (15 - 1) * 31 - 16 - 1 - 16 = 401. Note: We multiply by 31 because ciphertext bytes are stored in fields using
 // bytes_to_fields, which packs 31 bytes per field (since a Field is ~254 bits and can safely store 31 whole bytes).
-global MESSAGE_PLAINTEXT_SIZE_IN_BYTES: u32 = (MESSAGE_CIPHERTEXT_LEN - EPH_PK_X_SIZE_IN_FIELDS) * 31
+pub(crate) global MESSAGE_PLAINTEXT_SIZE_IN_BYTES: u32 = (MESSAGE_CIPHERTEXT_LEN - EPH_PK_X_SIZE_IN_FIELDS) * 31
     - HEADER_CIPHERTEXT_SIZE_IN_BYTES
-    - EPH_PK_SIGN_BYTE_SIZE_IN_BYTES;
+    - EPH_PK_SIGN_BYTE_SIZE_IN_BYTES
+    - AES128_PKCS7_EXPANSION_IN_BYTES;
 // The plaintext bytes represent Field values that were originally serialized using fields_to_bytes, which converts
-// each Field to 32 bytes. To convert the plaintext bytes back to fields, we divide by 32. 479 / 32 = 14
+// each Field to 32 bytes. To convert the plaintext bytes back to fields, we divide by 32. 401 / 32 = 12
 pub global MESSAGE_PLAINTEXT_LEN: u32 = MESSAGE_PLAINTEXT_SIZE_IN_BYTES / 32;
 
 pub global MESSAGE_EXPANDED_METADATA_LEN: u32 = 1;
@@ -244,4 +248,27 @@ mod tests {
         assert_eq(original_msg_type, unpacked_msg_type);
         assert_eq(original_msg_metadata, unpacked_msg_metadata);
     }
+
+    #[test]
+    unconstrained fn encode_decode_max_size_message() {
+        let msg_type_id: u64 = 42;
+        let msg_metadata: u64 = 99;
+        let mut msg_content = [0; MAX_MESSAGE_CONTENT_LEN];
+        for i in 0..MAX_MESSAGE_CONTENT_LEN {
+            msg_content[i] = i as Field;
+        }
+
+        let encoded = encode_message(msg_type_id, msg_metadata, msg_content);
+        let (decoded_type_id, decoded_metadata, decoded_content) = decode_message(BoundedVec::from_array(encoded));
+
+        assert_eq(decoded_type_id, msg_type_id);
+        assert_eq(decoded_metadata, msg_metadata);
+        assert_eq(decoded_content, BoundedVec::from_array(msg_content));
+    }
+
+    #[test(should_fail_with = "Invalid message content: it must have a length of at most MAX_MESSAGE_CONTENT_LEN")]
+    fn encode_oversized_message_fails() {
+        let msg_content = [0; MAX_MESSAGE_CONTENT_LEN + 1];
+        let _ = encode_message(0, 0, msg_content);
+    }
 }

noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr

@@ -11,7 +11,7 @@ use crate::{
     messages::{
         encoding::{
             EPH_PK_SIGN_BYTE_SIZE_IN_BYTES, EPH_PK_X_SIZE_IN_FIELDS, HEADER_CIPHERTEXT_SIZE_IN_BYTES,
-            MESSAGE_CIPHERTEXT_LEN, MESSAGE_PLAINTEXT_LEN,
+            MESSAGE_CIPHERTEXT_LEN, MESSAGE_PLAINTEXT_LEN, MESSAGE_PLAINTEXT_SIZE_IN_BYTES,
         },
         encryption::message_encryption::MessageEncryption,
         logs::arithmetic_generics_utils::{
@@ -150,17 +150,101 @@ pub fn derive_aes_symmetric_key_and_iv_from_ecdh_shared_secret_using_poseidon2_u
 pub struct AES128 {}
 
 impl MessageEncryption for AES128 {
+
+    /// AES128-CBC encryption for Aztec protocol messages.
+    ///
+    /// ## Overview
+    ///
+    /// The plaintext is an array of up to `MESSAGE_PLAINTEXT_LEN` (12) fields. The output is always exactly
+    /// `MESSAGE_CIPHERTEXT_LEN` (15) fields, regardless of plaintext size. Unused trailing fields are filled with
+    /// random data so that all encrypted messages are indistinguishable by size.
+    ///
+    /// ## PKCS#7 Padding
+    ///
+    /// AES operates on 16-byte blocks, so the plaintext must be padded to a multiple of 16. PKCS#7 padding always
+    /// adds at least 1 byte (so the receiver can always detect and strip it), which means:
+    /// - 1 B plaintext  -> 15 B padding -> 16 B total
+    /// - 15 B plaintext ->  1 B padding -> 16 B total
+    /// - 16 B plaintext -> 16 B padding -> 32 B total (full extra block)
+    ///
+    /// In general: if the plaintext is already a multiple of 16, a full 16-byte padding block is appended.
+    ///
+    /// ## Encryption Steps
+    ///
+    /// **1. Body encryption.** The plaintext fields are serialized to bytes (32 bytes per field) and AES-128-CBC
+    /// encrypted. Since 32 is a multiple of 16, PKCS#7 always adds a full 16-byte padding block (see above):
+    ///
+    /// ```text
+    /// +---------------------------------------------+
+    /// |                   body ct                    |
+    /// |            PlaintextLen*32 + 16 B            |
+    /// +-------------------------------+--------------+
+    /// | encrypted plaintext fields    | PKCS#7 (16B) |
+    /// | (serialized at 32 B each)     |              |
+    /// +-------------------------------+--------------+
+    /// ```
+    ///
+    /// **2. Header encryption.** The byte length of `body_ct` is stored as a 2-byte big-endian integer. This 2-byte
+    /// header plaintext is then AES-encrypted; PKCS#7 pads the remaining 14 bytes to fill one 16-byte AES block,
+    /// producing a 16-byte header ciphertext:
+    ///
+    /// ```text
+    /// +---------------------------+
+    /// |         header ct         |
+    /// |            16 B           |
+    /// +--------+------------------+
+    /// | body ct| PKCS#7 (14B)     |
+    /// | length |                  |
+    /// | (2 B)  |                  |
+    /// +--------+------------------+
+    /// ```
+    ///
+    /// ## Wire Format
+    ///
+    /// Messages are transmitted as fields, not bytes. A field is ~254 bits and can safely store 31 whole bytes, so
+    /// we need to pack our byte data into 31-byte chunks. This packing drives the wire format.
+    ///
+    /// **Step 1 -- Assemble bytes.** The ciphertexts are laid out in a byte array, padded with random bytes to a
+    /// multiple of 31 so it divides evenly into fields:
+    ///
+    /// ```text
+    /// +---------+------------+-------------------------+---------+
+    /// | pk sign | header ct  |        body ct          | byte pad|
+    /// |   1 B   |   16 B     | PlaintextLen*32 + 16 B  | (random)|
+    /// +---------+------------+-------------------------+---------+
+    /// |<----------- padded to a multiple of 31 B ------------->|
+    /// ```
+    ///
+    /// **Step 2 -- Pack into fields.** The byte array is split into 31-byte chunks, each stored in one field. The
+    /// ephemeral public key x-coordinate is prepended as its own field. Any remaining fields (up to 15 total) are
+    /// filled with random data so that all messages are the same size:
+    ///
+    /// ```text
+    /// +----------+-------------------------+-------------------+
+    /// | eph_pk.x |  message-byte fields    | random field pad  |
+    /// |          | (packed 31 B per field)  |  (fills to 15)   |
+    /// +----------+-------------------------+-------------------+
+    /// |<---------- MESSAGE_CIPHERTEXT_LEN = 15 fields ------->|
+    /// ```
+    ///
+    /// ## Key Derivation
+    ///
+    /// Two (key, IV) pairs are derived from the ECDH shared secret via Poseidon2 hashing with different domain
+    /// separators: one pair for the body ciphertext and one for the header ciphertext.
     fn encrypt<let PlaintextLen: u32>(
         plaintext: [Field; PlaintextLen],
         recipient: AztecAddress,
     ) -> [Field; MESSAGE_CIPHERTEXT_LEN] {
+        std::static_assert(
+            PlaintextLen <= MESSAGE_PLAINTEXT_LEN,
+            "Plaintext length exceeds MESSAGE_PLAINTEXT_LEN",
+        );
+
         // AES 128 operates on bytes, not fields, so we need to convert the fields to bytes. (This process is then
         // reversed when processing the message in `process_message_ciphertext`)
         let plaintext_bytes = fields_to_bytes(plaintext);
 
-        // ***************************************************************************** Compute the shared secret
-        // *****************************************************************************
-
+        // Derive ECDH shared secret with recipient using a fresh ephemeral keypair.
         let (eph_sk, eph_pk) = generate_ephemeral_key_pair();
 
         let eph_pk_sign_byte: u8 = get_sign_of_point(eph_pk) as u8;
@@ -189,15 +273,7 @@ impl MessageEncryption for AES128 {
         );
         // TODO: also use this shared secret for deriving note randomness.
 
-        // ***************************************************************************** Convert the plaintext into
-        // whatever format the encryption function expects
-        // *****************************************************************************
-
-        // Already done for this strategy: AES expects bytes.
-
-        // ***************************************************************************** Encrypt the plaintext
-        // *****************************************************************************
-
+        // AES128-CBC encrypt the plaintext bytes.
         // It is safe to call the `unsafe` function here, because we know the `shared_secret` was derived using an
         // AztecAddress (the recipient). See the block comment at the start of this unsafe target function for more
         // info.
@@ -209,22 +285,15 @@ impl MessageEncryption for AES128 {
 
         let ciphertext_bytes = aes128_encrypt(plaintext_bytes, body_iv, body_sym_key);
 
-        // |full_pt| = |pt_length| + |pt|
-        // |pt_aes_padding| = 16 - (|full_pt| % 16)
-        // or... since a % b is the same as a - b * (a // b) (integer division), so:
-        // |pt_aes_padding| = 16 - (|full_pt| - 16 * (|full_pt| // 16))
-        // |ct| = |full_pt| + |pt_aes_padding|
-        //      = |full_pt| + 16 - (|full_pt| - 16 * (|full_pt| // 16)) = 16 + 16 * (|full_pt| // 16) = 16 * (1 +
-        // |full_pt| // 16)
+        // Each plaintext field is 32 bytes (a multiple of the 16-byte AES block
+        // size), so PKCS#7 always appends a full 16-byte padding block:
+        //   |ciphertext| = PlaintextLen*32 + 16 = 16 * (1 + PlaintextLen*32 / 16)
         std::static_assert(
             ciphertext_bytes.len() == 16 * (1 + (PlaintextLen * 32) / 16),
             "unexpected ciphertext length",
         );
 
-        // ***************************************************************************** Compute the header ciphertext
-        // *****************************************************************************
-
-        // Header contains only the length of the ciphertext stored in 2 bytes.
+        // Encrypt a 2-byte header containing the body ciphertext length.
         let mut header_plaintext: [u8; 2] = [0 as u8; 2];
         let ciphertext_bytes_length = ciphertext_bytes.len();
         header_plaintext[0] = (ciphertext_bytes_length >> 8) as u8;
@@ -233,16 +302,14 @@ impl MessageEncryption for AES128 {
         // Note: the aes128_encrypt builtin fn automatically appends bytes to the input, according to pkcs#7; hence why
         // the output `header_ciphertext_bytes` is 16 bytes larger than the input in this case.
         let header_ciphertext_bytes = aes128_encrypt(header_plaintext, header_iv, header_sym_key);
-        // I recall that converting a slice to an array incurs constraints, so I'll check the length this way instead:
+        // Verify expected header ciphertext size at compile time.
         std::static_assert(
             header_ciphertext_bytes.len() == HEADER_CIPHERTEXT_SIZE_IN_BYTES,
             "unexpected ciphertext header length",
         );
 
-        // ***************************************************************************** Prepend / append more bytes of
-        // data to the ciphertext, before converting back to fields.
-        // *****************************************************************************
-
+        // Assemble the message byte array:
+        //   [eph_pk_sign (1B)] [header_ct (16B)] [body_ct] [padding to mult of 31]
         let mut message_bytes_padding_to_mult_31 =
             get_arr_of_size__message_bytes_padding__from_PT::<PlaintextLen * 32>();
         // Safety: this randomness won't be constrained to be random. It's in the interest of the executor of this fn
@@ -285,17 +352,12 @@ impl MessageEncryption for AES128 {
         );
         assert(offset == message_bytes.len(), "unexpected encrypted message length");
 
-        // ***************************************************************************** Convert bytes back to fields
-        // *****************************************************************************
-
+        // Pack message bytes into fields (31 bytes per field) and prepend eph_pk.x.
         // TODO(#12749): As Mike pointed out, we need to make messages produced by different encryption schemes
         // indistinguishable from each other and for this reason the output here and in the last for-loop of this
         // function should cover a full field.
         let message_bytes_as_fields = bytes_to_fields(message_bytes);
 
-        // ***************************************************************************** Prepend / append fields, to
-        // create the final message *****************************************************************************
-
         let mut ciphertext: [Field; MESSAGE_CIPHERTEXT_LEN] = [0; MESSAGE_CIPHERTEXT_LEN];
 
         ciphertext[0] = eph_pk.x;
@@ -368,16 +430,16 @@ impl MessageEncryption for AES128 {
 
             // Extract and decrypt main ciphertext
             let ciphertext_start = header_start + HEADER_CIPHERTEXT_SIZE_IN_BYTES;
-            let ciphertext_with_padding: [u8; (MESSAGE_CIPHERTEXT_LEN - EPH_PK_X_SIZE_IN_FIELDS) * 31 - HEADER_CIPHERTEXT_SIZE_IN_BYTES - EPH_PK_SIGN_BYTE_SIZE_IN_BYTES] =
+            let ciphertext_with_padding: [u8; MESSAGE_PLAINTEXT_SIZE_IN_BYTES] =
                 array::subarray(ciphertext_without_eph_pk_x.storage(), ciphertext_start);
-            let ciphertext: BoundedVec<u8, (MESSAGE_CIPHERTEXT_LEN - EPH_PK_X_SIZE_IN_FIELDS) * 31 - HEADER_CIPHERTEXT_SIZE_IN_BYTES - EPH_PK_SIGN_BYTE_SIZE_IN_BYTES> =
+            let ciphertext: BoundedVec<u8, MESSAGE_PLAINTEXT_SIZE_IN_BYTES> =
                 BoundedVec::from_parts(ciphertext_with_padding, ciphertext_length);
 
             // Decrypt main ciphertext and return it
             let plaintext_bytes = aes128_decrypt_oracle(ciphertext, body_iv, body_sym_key);
 
-            // Each field of the original note message was serialized to 32 bytes so we convert the bytes back to
-            // fields.
+            // Each field of the original message was serialized to 32 bytes so we convert
+            // the bytes back to fields.
             fields_from_bytes(plaintext_bytes)
         })
     }
@@ -489,6 +551,48 @@ mod test {
         let _ = AES128::encrypt([1, 2, 3, 4], invalid_address);
     }
 
+    // Documents the PKCS#7 padding behavior that `encrypt` relies on (see its static_assert).
+    #[test]
+    fn pkcs7_padding_always_adds_at_least_one_byte() {
+        let key = [0 as u8; 16];
+        let iv = [0 as u8; 16];
+
+        // 1 byte input + 15 bytes padding = 16 bytes
+        assert_eq(std::aes128::aes128_encrypt([0; 1], iv, key).len(), 16);
+
+        // 15 bytes input + 1 byte padding = 16 bytes
+        assert_eq(std::aes128::aes128_encrypt([0; 15], iv, key).len(), 16);
+
+        // 16 bytes input (block-aligned) + full 16-byte padding block = 32 bytes
+        assert_eq(std::aes128::aes128_encrypt([0; 16], iv, key).len(), 32);
+    }
+
+    #[test]
+    unconstrained fn encrypt_decrypt_max_size_plaintext() {
+        let mut env = TestEnvironment::new();
+        let recipient = env.create_light_account();
+
+        env.private_context(|_| {
+            let mut plaintext = [0; MESSAGE_PLAINTEXT_LEN];
+            for i in 0..MESSAGE_PLAINTEXT_LEN {
+                plaintext[i] = i as Field;
+            }
+            let ciphertext = AES128::encrypt(plaintext, recipient);
+
+            assert_eq(
+                AES128::decrypt(BoundedVec::from_array(ciphertext), recipient).unwrap(),
+                BoundedVec::from_array(plaintext),
+            );
+        });
+    }
+
+    #[test(should_fail_with = "Plaintext length exceeds MESSAGE_PLAINTEXT_LEN")]
+    unconstrained fn encrypt_oversized_plaintext() {
+        let address = AztecAddress { inner: 3 };
+        let plaintext: [Field; MESSAGE_PLAINTEXT_LEN + 1] = [0; MESSAGE_PLAINTEXT_LEN + 1];
+        let _ = AES128::encrypt(plaintext, address);
+    }
+
     #[test]
     unconstrained fn random_address_point_produces_valid_points() {
         // About half of random addresses are invalid, so testing just a couple gives us high confidence that

noir-projects/aztec-nr/aztec/src/messages/logs/note.nr

@@ -89,7 +89,7 @@ mod test {
     use crate::{
         messages::{
             encoding::decode_message,
-            logs::note::{decode_private_note_message, encode_private_note_message},
+            logs::note::{decode_private_note_message, encode_private_note_message, MAX_NOTE_PACKED_LEN},
             msg_type::PRIVATE_NOTE_MSG_TYPE_ID,
         },
         note::note_interface::NoteType,
@@ -121,4 +121,55 @@ mod test {
         assert_eq(randomness, RANDOMNESS);
         assert_eq(packed_note, BoundedVec::from_array(note.pack()));
     }
+
+    #[derive(Packable)]
+    struct MaxSizeNote {
+        data: [Field; MAX_NOTE_PACKED_LEN],
+    }
+
+    impl NoteType for MaxSizeNote {
+        fn get_id() -> Field {
+            0
+        }
+    }
+
+    #[test]
+    unconstrained fn encode_decode_max_size_note() {
+        let mut data = [0; MAX_NOTE_PACKED_LEN];
+        for i in 0..MAX_NOTE_PACKED_LEN {
+            data[i] = i as Field;
+        }
+        let note = MaxSizeNote { data };
+
+        let encoded = encode_private_note_message(note, OWNER, STORAGE_SLOT, RANDOMNESS);
+        let (msg_type_id, msg_metadata, msg_content) = decode_message(BoundedVec::from_array(encoded));
+
+        assert_eq(msg_type_id, PRIVATE_NOTE_MSG_TYPE_ID);
+
+        let (note_type_id, owner, storage_slot, randomness, packed_note) =
+            decode_private_note_message(msg_metadata, msg_content);
+
+        assert_eq(note_type_id, MaxSizeNote::get_id());
+        assert_eq(owner, OWNER);
+        assert_eq(storage_slot, STORAGE_SLOT);
+        assert_eq(randomness, RANDOMNESS);
+        assert_eq(packed_note, BoundedVec::from_array(data));
+    }
+
+    #[derive(Packable)]
+    struct OversizedNote {
+        data: [Field; MAX_NOTE_PACKED_LEN + 1],
+    }
+
+    impl NoteType for OversizedNote {
+        fn get_id() -> Field {
+            0
+        }
+    }
+
+    #[test(should_fail_with = "Invalid message content: it must have a length of at most MAX_MESSAGE_CONTENT_LEN")]
+    fn encode_oversized_note_fails() {
+        let note = OversizedNote { data: [0; MAX_NOTE_PACKED_LEN + 1] };
+        let _ = encode_private_note_message(note, OWNER, STORAGE_SLOT, RANDOMNESS);
+    }
 }

noir-projects/aztec-nr/aztec/src/messages/processing/event_validation_request.nr

@@ -42,7 +42,7 @@ mod test {
             3, // randomness
             4, // serialized_event[0]
             5, // serialized_event[1]
-            0, 0, 0, 0, 0, 0, 0, 0, 0, // serialized_event padding
+            0, 0, 0, 0, 0, 0, 0, 0, // serialized_event padding
             2, // bounded_vec_len
             6, // event_commitment
             7, // tx_hash