diff --git a/noir-projects/aztec-nr/aztec/src/oracle/aes128_decrypt.nr b/noir-projects/aztec-nr/aztec/src/oracle/aes128_decrypt.nr index 5ca72abf49c8..3b86686e752d 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/aes128_decrypt.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/aes128_decrypt.nr @@ -1,34 +1,28 @@ /// Decrypts a ciphertext, using AES128. /// -/// Returns a padded plaintext, of the same size as the input ciphertext. -/// Note that between 1-16 bytes at the end of the returned plaintext will be pkcs#7 padding. -/// It's up to the calling function to identify and remove that padding. -/// See the tests below for an example of how. +/// Returns a BoundedVec containing the plaintext. +/// /// It's up to the calling function to determine whether decryption succeeded or failed. /// See the tests below for an example of how. -unconstrained fn aes128_decrypt_oracle_wrapper( - ciphertext: [u8; N], - iv: [u8; 16], - sym_key: [u8; 16], -) -> [u8; N] { - aes128_decrypt_oracle(ciphertext, iv, sym_key) -} - #[oracle(aes128Decrypt)] -unconstrained fn aes128_decrypt_oracle( +pub unconstrained fn aes128_decrypt_oracle( ciphertext: [u8; N], iv: [u8; 16], sym_key: [u8; 16], -) -> [u8; N] {} +) -> BoundedVec {} mod test { use crate::{ encrypted_logs::encrypt::aes128::derive_aes_symmetric_key_and_iv_from_ecdh_shared_secret_using_sha256, - utils::point::point_from_x_coord, + utils::{array::subarray::subarray, point::point_from_x_coord}, }; - use super::aes128_decrypt_oracle_wrapper; + use super::aes128_decrypt_oracle; use std::aes128::aes128_encrypt; + global TEST_PLAINTEXT_LENGTH: u32 = 10; + global TEST_CIPHERTEXT_LENGTH: u32 = 16; + global TEST_PADDING_LENGTH: u32 = TEST_CIPHERTEXT_LENGTH - TEST_PLAINTEXT_LENGTH; + #[test] unconstrained fn aes_encrypt_then_decrypt() { let ciphertext_shared_secret = point_from_x_coord(1); @@ -37,23 +31,27 @@ mod test { ciphertext_shared_secret, ); - let plaintext: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - - let ciphertext = aes128_encrypt(plaintext, iv, sym_key); + let plaintext: [u8; TEST_PLAINTEXT_LENGTH] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - let received_plaintext = aes128_decrypt_oracle_wrapper(ciphertext, iv, sym_key); - let padding_length = received_plaintext[received_plaintext.len() - 1] as u32; + let ciphertext: [u8; TEST_CIPHERTEXT_LENGTH] = aes128_encrypt(plaintext, iv, sym_key); - // A BoundedVec could also be used. - let mut received_plaintext_with_padding_removed = std::collections::vec::Vec::new(); - for i in 0..received_plaintext.len() - padding_length { - received_plaintext_with_padding_removed.push(received_plaintext[i]); - } + let received_plaintext = aes128_decrypt_oracle(ciphertext, iv, sym_key); - assert_eq(received_plaintext_with_padding_removed.slice, plaintext.as_slice()); + assert_eq(received_plaintext.len(), TEST_PLAINTEXT_LENGTH); + assert_eq(received_plaintext.max_len(), TEST_CIPHERTEXT_LENGTH); + assert_eq( + subarray::<_, _, TEST_PLAINTEXT_LENGTH>(received_plaintext.storage(), 0), + plaintext, + ); + assert_eq( + subarray::<_, _, TEST_PADDING_LENGTH>( + received_plaintext.storage(), + TEST_PLAINTEXT_LENGTH, + ), + [0 as u8; TEST_PADDING_LENGTH], + ); } - global TEST_PLAINTEXT_LENGTH: u32 = 10; global TEST_MAC_LENGTH: u32 = 32; #[test(should_fail_with = "mac does not match")] @@ -61,10 +59,13 @@ mod test { // The AES decryption oracle will not fail for any ciphertext; it will always // return some data. As for whether the decryption was successful, it's up // to the app to check this in a custom way. + // // E.g. if it's a note that's been encrypted, then upon decryption, the app // can check to see if the note hash exists onchain. If it doesn't exist // onchain, then that's a strong indicator that decryption has failed. - // E.g. for non-note messages, the plaintext could include a MAC. We + // + // E.g. for non-note messages, the plaintext could include a MAC + // (https://en.wikipedia.org/wiki/Message_authentication_code). We // demonstrate what this could look like in this test. // // We compute a MAC and we include that MAC in the plaintext. We then encrypt @@ -86,7 +87,10 @@ mod test { // We append the mac to the plaintext. It doesn't necessarily have to be 32 bytes; // that's quite an extreme length. 16 bytes or 8 bytes might be sufficient, and would - // save on data broadcasting costs. + // save on data broadcasting costs. The shorter the mac, the more possibility + // of false positive decryptions (decryption seemingly succeeding, but the + // decrypted plaintext being garbage). + // Some projects use the `iv` (all 16 bytes or the first 8 bytes) as a mac. let mut plaintext_with_mac = [0 as u8; TEST_PLAINTEXT_LENGTH + TEST_MAC_LENGTH]; for i in 0..TEST_PLAINTEXT_LENGTH { plaintext_with_mac[i] = plaintext[i]; @@ -109,12 +113,10 @@ mod test { let mut bad_sym_key = sym_key; bad_sym_key[0] = 0; - let received_plaintext = aes128_decrypt_oracle_wrapper(ciphertext, iv, bad_sym_key); + let received_plaintext = aes128_decrypt_oracle(ciphertext, iv, bad_sym_key); - let mut extracted_mac_as_bytes = [0 as u8; TEST_MAC_LENGTH]; - for i in 0..TEST_MAC_LENGTH { - extracted_mac_as_bytes[i] = received_plaintext[TEST_PLAINTEXT_LENGTH + i]; - } + let extracted_mac_as_bytes: [u8; TEST_MAC_LENGTH] = + subarray(received_plaintext.storage(), TEST_PLAINTEXT_LENGTH); // We expect this assertion to fail, because we used a bad sym key. assert_eq(mac_as_bytes, extracted_mac_as_bytes, "mac does not match"); diff --git a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts index 1def8a9584b7..488e621190a9 100644 --- a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts @@ -264,6 +264,6 @@ export abstract class TypedOracle { } aes128Decrypt(_ciphertext: Buffer, _iv: Buffer, _symKey: Buffer): Promise { - throw new OracleMethodNotAvailableError('aes128Decrypt'); + return Promise.reject(new OracleMethodNotAvailableError('aes128Decrypt')); } } diff --git a/yarn-project/simulator/src/client/view_data_oracle.ts b/yarn-project/simulator/src/client/view_data_oracle.ts index e28c38076380..f942bd30734c 100644 --- a/yarn-project/simulator/src/client/view_data_oracle.ts +++ b/yarn-project/simulator/src/client/view_data_oracle.ts @@ -368,11 +368,7 @@ export class ViewDataOracle extends TypedOracle { // TODO(#11849): consider replacing this oracle with a pure Noir implementation of aes decryption. public override aes128Decrypt(ciphertext: Buffer, iv: Buffer, symKey: Buffer): Promise { - // Noir can't predict the amount of padding that gets trimmed, - // but it needs to know the length of the returned value. - // So we tell Noir that the length is the (predictable) length - // of the padded plaintext, we return that padded plaintext, and have Noir interpret the padding to do the trimming. const aes128 = new Aes128(); - return aes128.decryptBufferCBCKeepPadding(ciphertext, iv, symKey); + return aes128.decryptBufferCBC(ciphertext, iv, symKey); } } diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index b2adace9066a..7457039d7113 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -25,6 +25,8 @@ import { type ForeignCallArray, type ForeignCallSingle, addressFromSingle, + arrayToBoundedVec, + bufferToU8Array, fromArray, fromSingle, fromUintArray, @@ -587,14 +589,9 @@ export class TXEService { const ivBuffer = fromUintArray(iv, 8); const symKeyBuffer = fromUintArray(symKey, 8); - // TODO(AD): this is a hack to shut up linter - this shouldn't be async! - const paddedPlaintext = await Promise.resolve( - this.typedOracle.aes128Decrypt(ciphertextBuffer, ivBuffer, symKeyBuffer), - ); + const plaintextBuffer = await this.typedOracle.aes128Decrypt(ciphertextBuffer, ivBuffer, symKeyBuffer); - // We convert each byte of the buffer to its own Field, so that the Noir - // function correctly receives [u8; N]. - return toForeignCallResult([toArray(Array.from(paddedPlaintext).map(byte => new Fr(byte)))]); + return toForeignCallResult(arrayToBoundedVec(bufferToU8Array(plaintextBuffer), ciphertextBuffer.length)); } // AVM opcodes diff --git a/yarn-project/txe/src/util/encoding.ts b/yarn-project/txe/src/util/encoding.ts index 152470395067..f7b6868774c0 100644 --- a/yarn-project/txe/src/util/encoding.ts +++ b/yarn-project/txe/src/util/encoding.ts @@ -32,7 +32,7 @@ export function fromArray(obj: ForeignCallArray) { * @param uintBitSize If it's an array of Noir u8's, put `8`, etc. * @returns */ -export function fromUintArray(obj: ForeignCallArray, uintBitSize: number) { +export function fromUintArray(obj: ForeignCallArray, uintBitSize: number): Buffer { if (uintBitSize % 8 !== 0) { throw new Error(`u${uintBitSize} is not a supported type in Noir`); } @@ -40,14 +40,40 @@ export function fromUintArray(obj: ForeignCallArray, uintBitSize: number) { return Buffer.concat(obj.map(str => hexToBuffer(str).slice(-uintByteSize))); } -export function toSingle(obj: Fr | AztecAddress) { +export function toSingle(obj: Fr | AztecAddress): ForeignCallSingle { return obj.toString().slice(2); } -export function toArray(objs: Fr[]) { +export function toArray(objs: Fr[]): ForeignCallArray { return objs.map(obj => obj.toString()); } +export function bufferToU8Array(buffer: Buffer): ForeignCallArray { + return toArray(Array.from(buffer).map(byte => new Fr(byte))); +} + +/** + * Converts a ForeignCallArray into a tuple which represents a nr BoundedVec. + * If the input array is shorter than the maxLen, it pads the result with zeros, + * so that nr can correctly coerce this result into a BoundedVec. + * @param array + * @param maxLen - the max length of the BoundedVec. + * @returns a tuple representing a BoundedVec. + */ +export function arrayToBoundedVec(array: ForeignCallArray, maxLen: number): [ForeignCallArray, ForeignCallSingle] { + if (array.length > maxLen) { + throw new Error(`Array of length ${array.length} larger than maxLen ${maxLen}`); + } + const lengthDiff = maxLen - array.length; + // We pad the array to the maxLen of the BoundedVec. + const zeroPaddingArray = toArray(Array(lengthDiff).fill(new Fr(0))); + + // These variable names match with the BoundedVec members in nr: + const storage = array.concat(zeroPaddingArray); + const len = toSingle(new Fr(array.length)); + return [storage, len]; +} + export function toForeignCallResult(obj: (ForeignCallSingle | ForeignCallArray)[]) { return { values: obj }; }