feat: Proper padding in ts AES and constrained AES in body and header…

… computations (#6269)

Fixes #6172. 

Also fixes an issue as the typescript AES was not doing proper padding. 

Battling noir, good with help from @Thunkar.

---------

Co-authored-by: thunkar <gregojquiros@gmail.com>
Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

noir-projects/aztec-nr/aztec/src/encrypted_logs/body.nr

@@ -1,7 +1,7 @@
 use crate::note::{note_interface::NoteInterface};
 use dep::protocol_types::{grumpkin_private_key::GrumpkinPrivateKey, grumpkin_point::GrumpkinPoint};
 
-use crate::oracle::encryption::aes128_encrypt;
+use dep::std::aes128::aes128_encrypt_slice;
 use crate::keys::point_to_symmetric_key::point_to_symmetric_key;
 
 struct EncryptedLogBody<Note> {
@@ -19,32 +19,30 @@ impl<Note> EncryptedLogBody<Note> {
         Self { storage_slot, note_type_id, note }
     }
 
-    pub fn compute_ciphertext<N, M>(
+    pub fn compute_ciphertext<N>(
         self,
         secret: GrumpkinPrivateKey,
         point: GrumpkinPoint
-    ) -> [u8; M] where Note: NoteInterface<N> {
-        // We need 32 bytes for every field in the note, and then we have 2 extra fields (storage_slot and note_type_id)
-        let serialized_note: [Field; N] = Note::serialize_content(self.note);
+    ) -> [u8] where Note: NoteInterface<N> {
+        let serialized_note: [Field; N] = self.note.serialize_content();
 
-        // Work around not being able to use N directly beyond the size of the array above.
-        let N_ = serialized_note.len();
-
-        assert(N_ * 32 + 64 == M, "Invalid size of encrypted log body");
-
-        let mut buffer: [u8; M] = [0; M];
+        let mut buffer_slice: [u8] = &[];
 
         let storage_slot_bytes = self.storage_slot.to_be_bytes(32);
         let note_type_id_bytes = self.note_type_id.to_be_bytes(32);
+
+        for i in 0..32 {
+            buffer_slice = buffer_slice.push_back(storage_slot_bytes[i]);
+        }
+
         for i in 0..32 {
-            buffer[i] = storage_slot_bytes[i];
-            buffer[32 + i] = note_type_id_bytes[i];
+            buffer_slice = buffer_slice.push_back(note_type_id_bytes[i]);
         }
 
-        for i in 0..N_ {
+        for i in 0..serialized_note.len() {
             let bytes = serialized_note[i].to_be_bytes(32);
             for j in 0..32 {
-                buffer[64 + i * 32 + j] = bytes[j];
+                buffer_slice = buffer_slice.push_back(bytes[j]);
             }
         }
 
@@ -56,35 +54,33 @@ impl<Note> EncryptedLogBody<Note> {
             sym_key[i] = full_key[i];
             iv[i] = full_key[i + 16];
         }
-
-        aes128_encrypt(buffer, iv, sym_key)
+        aes128_encrypt_slice(buffer_slice, iv, sym_key)
     }
 }
 
-/*
-// Test is semi broken, needs to be fixed along with #6172  
 mod test {
     use crate::encrypted_logs::body::EncryptedLogBody;
-    use dep::protocol_types::{address::AztecAddress, traits::Empty, constants::GENERATOR_INDEX__NOTE_NULLIFIER};
+    use dep::protocol_types::{
+        address::AztecAddress, traits::Empty, constants::GENERATOR_INDEX__NOTE_NULLIFIER,
+        grumpkin_private_key::GrumpkinPrivateKey, grumpkin_point::GrumpkinPoint
+    };
 
     use crate::{
         note::{note_header::NoteHeader, note_interface::NoteInterface, utils::compute_note_hash_for_consumption},
         oracle::{unsafe_rand::unsafe_rand, nullifier_key::get_app_nullifier_secret_key, get_public_key::get_public_key},
         context::PrivateContext, hash::poseidon2_hash
     };
 
-    use dep::protocol_types::{address::AztecAddress, grumpkin_private_key::GrumpkinPrivateKey, grumpkin_point::GrumpkinPoint};
-
     struct AddressNote {
         address: AztecAddress,
         owner: AztecAddress,
         randomness: Field,
         header: NoteHeader,
     }
 
-    global BIB_BOB_ADDRESS_NOTE_LEN: Field = 3;
+    global ADDRESS_NOTE_LEN: Field = 3;
 
-    impl NoteInterface<BIB_BOB_ADDRESS_N3OTE_LEN> for AddressNote {
+    impl NoteInterface<ADDRESS_NOTE_LEN> for AddressNote {
         fn compute_note_content_hash(self) -> Field {1}
 
         fn get_note_type_id() -> Field {2}
@@ -99,9 +95,9 @@ mod test {
 
         fn broadcast(self, context: &mut PrivateContext, slot: Field) {}
 
-        fn serialize_content(self) -> [Field; BIB_BOB_ADDRESS_NOTE_LEN] { [self.address.to_field(), self.owner.to_field(), self.randomness]}
+        fn serialize_content(self) -> [Field; ADDRESS_NOTE_LEN] { [self.address.to_field(), self.owner.to_field(), self.randomness]}
 
-        fn deserialize_content(fields: [Field; BIB_BOB_ADDRESS_NOTE_LEN]) -> Self {
+        fn deserialize_content(fields: [Field; ADDRESS_NOTE_LEN]) -> Self {
             AddressNote { address: AztecAddress::from_field(fields[0]), owner: AztecAddress::from_field(fields[1]), randomness: fields[2], header: NoteHeader::empty() }
         }
     }
@@ -110,10 +106,9 @@ mod test {
         pub fn new(address: AztecAddress, owner: AztecAddress, randomness: Field) -> Self {
             AddressNote { address, owner, randomness, header: NoteHeader::empty() }
         }
-        // docs:end:address_note_def
     }
 
-    // @todo Issue(#6172) This is to be run as a test. But it is currently using the AES oracle so will fail there.
+    #[test]
     fn test_encrypted_log_body() {
         let note = AddressNote::new(
             AztecAddress::from_field(0x1),
@@ -137,11 +132,12 @@ mod test {
         let ciphertext = body.compute_ciphertext(secret, point);
 
         let expected_body_ciphertext = [
-            131, 119, 105, 129, 244, 32, 151, 205, 12, 99, 93, 62, 10, 180, 72, 21, 36, 194, 14, 168, 0, 137, 126, 59, 151, 177, 136, 254, 153, 190, 92, 33, 40, 151, 178, 54, 34, 166, 124, 96, 117, 108, 168, 7, 147, 222, 81, 201, 254, 170, 244, 151, 60, 64, 226, 45, 156, 185, 53, 23, 121, 63, 243, 101, 134, 21, 167, 39, 226, 203, 162, 223, 28, 74, 244, 159, 54, 201, 192, 168, 19, 85, 103, 82, 148, 3, 153, 210, 89, 245, 171, 171, 12, 248, 40, 74, 199, 65, 96, 42, 84, 83, 48, 21, 188, 134, 45, 247, 134, 166, 109, 170, 68, 212, 99, 235, 74, 202, 162, 108, 130, 128, 122, 16, 79, 242, 30, 157, 26, 75, 57, 24, 18, 124, 217, 74, 155, 13, 171, 205, 194, 193, 103, 134, 224, 204, 46, 105, 135, 166, 192, 163, 186, 42, 71, 51, 156, 161, 8, 131
+            131, 119, 105, 129, 244, 32, 151, 205, 12, 99, 93, 62, 10, 180, 72, 21, 47, 232, 95, 17, 240, 230, 80, 129, 174, 158, 23, 76, 114, 185, 43, 18, 254, 148, 147, 230, 66, 216, 167, 62, 180, 213, 238, 33, 108, 29, 84, 139, 99, 206, 212, 253, 92, 116, 137, 31, 0, 104, 45, 91, 250, 109, 141, 114, 189, 53, 35, 60, 108, 156, 170, 206, 150, 114, 150, 187, 198, 13, 62, 153, 133, 13, 169, 167, 242, 221, 40, 168, 186, 203, 104, 82, 47, 238, 142, 179, 90, 37, 9, 70, 245, 176, 122, 247, 42, 87, 75, 7, 20, 89, 166, 123, 14, 26, 230, 156, 49, 94, 0, 94, 72, 58, 171, 239, 115, 174, 155, 7, 151, 17, 60, 206, 193, 134, 70, 87, 215, 88, 21, 194, 63, 26, 106, 105, 124, 213, 252, 152, 192, 71, 115, 13, 181, 5, 169, 15, 170, 196, 174, 228, 170, 192, 91, 76, 110, 220, 89, 47, 248, 144, 189, 251, 167, 149, 248, 226
         ];
 
-        assert_eq(ciphertext, expected_body_ciphertext);
+        for i in 0..expected_body_ciphertext.len() {
+            assert_eq(ciphertext[i], expected_body_ciphertext[i]);
+        }
+        assert_eq(expected_body_ciphertext.len(), ciphertext.len());
     }
 }
-
-*/

noir-projects/aztec-nr/aztec/src/encrypted_logs/header.nr

@@ -1,8 +1,9 @@
 use dep::protocol_types::{address::AztecAddress, grumpkin_private_key::GrumpkinPrivateKey, grumpkin_point::GrumpkinPoint};
 
-use crate::oracle::encryption::aes128_encrypt;
 use crate::keys::point_to_symmetric_key::point_to_symmetric_key;
 
+use dep::std::aes128::aes128_encrypt_slice;
+
 struct EncryptedLogHeader {
     address: AztecAddress,
 }
@@ -13,28 +14,22 @@ impl EncryptedLogHeader {
     }
 
     // @todo Issue(#5901) Figure out if we return the bytes or fields for the log
-    fn compute_ciphertext(self, secret: GrumpkinPrivateKey, point: GrumpkinPoint) -> [u8; 32] {
+    fn compute_ciphertext(self, secret: GrumpkinPrivateKey, point: GrumpkinPoint) -> [u8; 48] {
         let full_key = point_to_symmetric_key(secret, point);
         let mut sym_key = [0; 16];
         let mut iv = [0; 16];
-        let mut input = [0; 32];
-        let input_slice = self.address.to_field().to_be_bytes(32);
 
         for i in 0..16 {
             sym_key[i] = full_key[i];
             iv[i] = full_key[i + 16];
-
-            // We copy address on the following 2 lines in order to avoid having 2 loops  
-            input[i] = input_slice[i];
-            input[i + 16] = input_slice[i + 16];
         }
 
-        // @todo Issue(#6172) This encryption is currently using an oracle. It is not actually constrained atm.
-        aes128_encrypt(input, iv, sym_key)
+        let input: [u8] = self.address.to_field().to_be_bytes(32);
+        aes128_encrypt_slice(input, iv, sym_key).as_array()
     }
 }
 
-// @todo Issue(#6172) This is to be run as a test. But it is currently using the AES oracle so will fail there.
+#[test]
 fn test_encrypted_log_header() {
     let address = AztecAddress::from_field(0xdeadbeef);
     let header = EncryptedLogHeader::new(address);
@@ -50,7 +45,7 @@ fn test_encrypted_log_header() {
     let ciphertext = header.compute_ciphertext(secret, point);
 
     let expected_header_ciphertext = [
-        131, 119, 105, 129, 244, 32, 151, 205, 12, 99, 93, 62, 10, 180, 72, 21, 179, 36, 250, 95, 56, 167, 171, 16, 195, 164, 223, 57, 75, 5, 24, 119
+        131, 119, 105, 129, 244, 32, 151, 205, 12, 99, 93, 62, 10, 180, 72, 21, 179, 36, 250, 95, 56, 167, 171, 16, 195, 164, 223, 57, 75, 5, 24, 119, 198, 34, 99, 189, 193, 183, 227, 43, 79, 204, 214, 89, 221, 153, 246, 64
     ];
 
     assert_eq(ciphertext, expected_header_ciphertext);

noir-projects/noir-contracts/contracts/test_contract/src/main.nr

@@ -337,7 +337,7 @@ contract Test {
     }
 
     #[aztec(private)]
-    fn encrypt(input: [u8; 64], iv: [u8; 16], key: [u8; 16]) -> [u8; 64] {
+    fn encrypt(input: [u8; 64], iv: [u8; 16], key: [u8; 16]) -> [u8; 80] {
         aes128_encrypt(input, iv, key)
     }
 
@@ -347,20 +347,20 @@ contract Test {
     }
 
     #[aztec(private)]
-    fn compute_note_header_ciphertext(secret: GrumpkinPrivateKey, point: GrumpkinPoint) -> [u8; 32] {
+    fn compute_note_header_ciphertext(secret: GrumpkinPrivateKey, point: GrumpkinPoint) -> [u8; 48] {
         EncryptedLogHeader::new(context.this_address()).compute_ciphertext(secret, point)
     }
 
-    // 64 bytes + 32 * #fields = 96 bytes
+    // 64 bytes + 32 * #fields + 16 = 112 bytes
     #[aztec(private)]
     fn compute_note_body_ciphertext(
         secret: GrumpkinPrivateKey,
         point: GrumpkinPoint,
         storage_slot: Field,
         value: Field
-    ) -> [u8; 96] {
+    ) -> [u8; 112] {
         let note = TestNote::new(value);
-        EncryptedLogBody::new(storage_slot, TestNote::get_note_type_id(), note).compute_ciphertext(secret, point)
+        EncryptedLogBody::new(storage_slot, TestNote::get_note_type_id(), note).compute_ciphertext(secret, point).as_array()
     }
 
     #[aztec(public)]

noir/noir-repo/noir_stdlib/src/aes128.nr

@@ -2,3 +2,6 @@
 // docs:start:aes128
 pub fn aes128_encrypt<N>(input: [u8; N], iv: [u8; 16], key: [u8; 16]) -> [u8] {}
 // docs:end:aes128
+
+#[foreign(aes128_encrypt)]
+pub fn aes128_encrypt_slice(input: [u8], iv: [u8; 16], key: [u8; 16]) -> [u8] {}

yarn-project/circuit-types/src/logs/encrypted_log_body.test.ts

@@ -47,7 +47,7 @@ describe('encrypt log body', () => {
     const noteTypeId = new Fr(1);
     const storageSlot = new Fr(2);
 
-    const body = new EncryptedLogBody(noteTypeId, storageSlot, note);
+    const body = new EncryptedLogBody(storageSlot, noteTypeId, note);
 
     const encrypted = body.computeCiphertext(ephSecretKey, viewingPubKey);
 

yarn-project/circuits.js/src/barretenberg/crypto/aes128/index.test.ts

@@ -9,14 +9,33 @@ describe('aes128', () => {
     aes128 = new Aes128();
   });
 
+  // PKCS#7 padding
+  const pad = (data: Buffer): Buffer => {
+    const rawLength = data.length;
+    const numPaddingBytes = 16 - (rawLength % 16);
+    const paddingBuffer = Buffer.alloc(numPaddingBytes);
+    paddingBuffer.fill(numPaddingBytes);
+    return Buffer.concat([data, paddingBuffer]);
+  };
+
+  // PKCS#7 padding removal
+  const removePadding = (paddedBuffer: Buffer): Buffer => {
+    // We get padding length from the last byte - in PKCS#7 all the padded bytes contain padding length
+    // and there is always some padding.
+    const paddingToRemove = paddedBuffer[paddedBuffer.length - 1];
+    return paddedBuffer.subarray(0, paddedBuffer.length - paddingToRemove);
+  };
+
   it('should correctly encrypt input', () => {
     const data = randomBytes(32);
     const key = randomBytes(16);
     const iv = randomBytes(16);
 
+    const paddedData = pad(data);
+
     const cipher = createCipheriv('aes-128-cbc', key, iv);
     cipher.setAutoPadding(false);
-    const expected = Buffer.concat([cipher.update(data), cipher.final()]);
+    const expected = Buffer.concat([cipher.update(paddedData), cipher.final()]);
 
     const result: Buffer = aes128.encryptBufferCBC(data, iv, key);
 
@@ -28,13 +47,15 @@ describe('aes128', () => {
     const key = randomBytes(16);
     const iv = randomBytes(16);
 
+    const paddedData = pad(data);
+
     const cipher = createCipheriv('aes-128-cbc', key, iv);
     cipher.setAutoPadding(false);
-    const ciphertext = Buffer.concat([cipher.update(data), cipher.final()]);
+    const ciphertext = Buffer.concat([cipher.update(paddedData), cipher.final()]);
 
     const decipher = createDecipheriv('aes-128-cbc', key, iv);
     decipher.setAutoPadding(false);
-    const expected = Buffer.concat([decipher.update(ciphertext), decipher.final()]);
+    const expected = removePadding(Buffer.concat([decipher.update(ciphertext), decipher.final()]));
 
     const result: Buffer = aes128.decryptBufferCBC(ciphertext, iv, key);
 

yarn-project/circuits.js/src/barretenberg/crypto/aes128/index.ts

@@ -15,13 +15,11 @@ export class Aes128 {
    */
   public encryptBufferCBC(data: Uint8Array, iv: Uint8Array, key: Uint8Array) {
     const rawLength = data.length;
-    const numPaddingBytes = rawLength % 16 != 0 ? 16 - (rawLength % 16) : 0;
+    const numPaddingBytes = 16 - (rawLength % 16);
     const paddingBuffer = Buffer.alloc(numPaddingBytes);
-    // input num bytes needs to be a multiple of 16
+    // input num bytes needs to be a multiple of 16 and at least 1 byte
     // node uses PKCS#7-Padding scheme, where padding byte value = the number of padding bytes
-    if (numPaddingBytes != 0) {
-      paddingBuffer.fill(numPaddingBytes);
-    }
+    paddingBuffer.fill(numPaddingBytes);
     const input = Buffer.concat([data, paddingBuffer]);
 
     const api = BarretenbergSync.getSingleton();
@@ -39,8 +37,10 @@ export class Aes128 {
    */
   public decryptBufferCBC(data: Uint8Array, iv: Uint8Array, key: Uint8Array) {
     const api = BarretenbergSync.getSingleton();
-    return Buffer.from(
+    const paddedBuffer = Buffer.from(
       api.aesDecryptBufferCbc(new RawBuffer(data), new RawBuffer(iv), new RawBuffer(key), data.length),
     );
+    const paddingToRemove = paddedBuffer[paddedBuffer.length - 1];
+    return paddedBuffer.subarray(0, paddedBuffer.length - paddingToRemove);
   }
 }