diff --git a/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr b/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr index 888d208b7bb7..2681085b6651 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr @@ -18,7 +18,7 @@ use crate::{ get_arr_of_size__message_bytes__from_PT, get_arr_of_size__message_bytes_padding__from_PT, }, }, - oracle::{aes128_decrypt::aes128_decrypt, random::random, shared_secret::get_shared_secret}, + oracle::{aes128_decrypt::try_aes128_decrypt, random::random, shared_secret::get_shared_secret}, utils::{ array, conversion::{ @@ -414,27 +414,22 @@ impl MessageEncryption for AES128 { let header_ciphertext_bvec = BoundedVec::::from_array(header_ciphertext); - // Decrypt header - let header_plaintext = aes128_decrypt(header_ciphertext_bvec, header_iv, header_sym_key); - + try_aes128_decrypt(header_ciphertext_bvec, header_iv, header_sym_key) // Extract ciphertext length from header (2 bytes, big-endian) - extract_ciphertext_length(header_plaintext) + .and_then(|header_plaintext| extract_ciphertext_length(header_plaintext)) .filter(|ciphertext_length| ciphertext_length <= MESSAGE_PLAINTEXT_SIZE_IN_BYTES) - .and_then(|ciphertext_length| { + .map(|ciphertext_length| { // Extract and decrypt main ciphertext let ciphertext_start = header_start + HEADER_CIPHERTEXT_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 = - BoundedVec::from_parts(ciphertext_with_padding, ciphertext_length); - - // Decrypt main ciphertext and return it - let plaintext_bytes = aes128_decrypt(ciphertext, body_iv, body_sym_key); - - // Convert bytes back to fields (32 bytes per field). Returns None if the actual bytes are - // not valid. - try_fields_from_bytes(plaintext_bytes) + BoundedVec::from_parts(ciphertext_with_padding, ciphertext_length) }) + // Decrypt main ciphertext and return it + .and_then(|ciphertext| try_aes128_decrypt(ciphertext, body_iv, body_sym_key)) + // Convert bytes back to fields (32 bytes per field). Returns None if the actual bytes are + // not valid. + .and_then(|plaintext_bytes| try_fields_from_bytes(plaintext_bytes)) }) }) } @@ -680,7 +675,7 @@ mod test { let ciphertext = BoundedVec::from_array(AES128::encrypt(plaintext, recipient)); let empty_header = BoundedVec::::new(); - let _ = OracleMock::mock("aztec_utl_aes128Decrypt").returns(empty_header).times(1); + let _ = OracleMock::mock("aztec_utl_tryAes128Decrypt").returns(Option::some(empty_header)).times(1); assert(AES128::decrypt(ciphertext, recipient).is_none()); }); @@ -700,7 +695,7 @@ mod test { let bad_header = BoundedVec::::from_array(encode_header( MESSAGE_PLAINTEXT_SIZE_IN_BYTES + 1, )); - let _ = OracleMock::mock("aztec_utl_aes128Decrypt").returns(bad_header).times(1); + let _ = OracleMock::mock("aztec_utl_tryAes128Decrypt").returns(Option::some(bad_header)).times(1); assert(AES128::decrypt(ciphertext, recipient).is_none()); }); 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 0569dd438dc1..b59ca4c0b0d5 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/aes128_decrypt.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/aes128_decrypt.nr @@ -1,30 +1,30 @@ use crate::utils::array::assert_bounded_vec_trimmed; -#[oracle(aztec_utl_aes128Decrypt)] -unconstrained fn aes128_decrypt_oracle( +#[oracle(aztec_utl_tryAes128Decrypt)] +unconstrained fn try_aes128_decrypt_oracle( ciphertext: BoundedVec, iv: [u8; 16], sym_key: [u8; 16], -) -> BoundedVec {} +) -> Option> {} -/// Decrypts a ciphertext, using AES128. +/// Attempts to decrypt a ciphertext using AES128. /// -/// 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. +/// Returns `Option::some(plaintext)` on success, or `Option::none()` if decryption fails (e.g. due to malformed +/// ciphertext). Note that decryption with the wrong key will still return `Some` with garbage data, it's up to +/// the calling function to verify correctness (e.g. via a MAC check). /// /// Note that we accept ciphertext as a BoundedVec, not as an array. This is because this function is typically used -/// when processing logs and at that point we don't have a comptime information about the length of the ciphertext as +/// when processing logs and at that point we don't have comptime information about the length of the ciphertext as /// the log is not specific to any individual note. -pub unconstrained fn aes128_decrypt( +pub unconstrained fn try_aes128_decrypt( ciphertext: BoundedVec, iv: [u8; 16], sym_key: [u8; 16], -) -> BoundedVec { - let result = aes128_decrypt_oracle(ciphertext, iv, sym_key); - assert_bounded_vec_trimmed(result); - result +) -> Option> { + try_aes128_decrypt_oracle(ciphertext, iv, sym_key).map(|result: BoundedVec| { + assert_bounded_vec_trimmed(result); + result + }) } mod test { @@ -33,7 +33,7 @@ mod test { utils::{array::subarray::subarray, point::point_from_x_coord}, }; use crate::test::helpers::test_environment::TestEnvironment; - use super::aes128_decrypt; + use super::try_aes128_decrypt; use poseidon::poseidon2::Poseidon2; use std::aes128::aes128_encrypt; @@ -56,13 +56,9 @@ mod test { let ciphertext: [u8; TEST_CIPHERTEXT_LENGTH] = aes128_encrypt(plaintext, iv, sym_key); - // We need to convert the array to a BoundedVec because the oracle expects a BoundedVec as it's designed to - // work with logs with unknown length at compile time. This would not be necessary here as the header - // ciphertext length is fixed. But we do it anyway to not have to have duplicate oracles. let ciphertext_bvec = BoundedVec::::from_array(ciphertext); - let received_plaintext = aes128_decrypt(ciphertext_bvec, iv, sym_key); - + let received_plaintext = try_aes128_decrypt(ciphertext_bvec, iv, sym_key).unwrap(); 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); @@ -80,21 +76,17 @@ mod test { let env = TestEnvironment::new(); env.utility_context(|_| { - // 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. + // The AES decryption oracle will not fail for any valid ciphertext; it will always return + // `Some` with some data. Whether the decryption was successful is up to the app to check 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. if it's a note that's been encrypted, upon decryption the app can check whether the + // note hash exists onchain. If it doesn't, that's a strong indicator that decryption failed. // // 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 this plaintext to get a - // ciphertext. We broadcast the [ciphertext, mac] tuple. The eventual decryptor will expect the mac in the - // decrypted plaintext to match the mac that was broadcast. If not, the recipient knows that decryption has - // failed. + // (https://en.wikipedia.org/wiki/Message_authentication_code). We demonstrate this approach in + // this test: we compute a MAC, include it in the plaintext, encrypt, and then verify that + // decryption with a bad key produces a MAC mismatch. let ciphertext_shared_secret = point_from_x_coord(1).unwrap(); let (sym_key, iv) = derive_aes_symmetric_key_and_iv_from_ecdh_shared_secret_using_poseidon2_unsafe::<1>( @@ -135,12 +127,12 @@ mod test { // We need to convert the array to a BoundedVec because the oracle expects a BoundedVec as it's designed to // work with logs of unknown length. let ciphertext_bvec = BoundedVec::::from_array(ciphertext); - let received_plaintext = aes128_decrypt(ciphertext_bvec, iv, bad_sym_key); + // Decryption with wrong key still returns Some (with garbage). + let received_plaintext = try_aes128_decrypt(ciphertext_bvec, iv, bad_sym_key).unwrap(); 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/noir-projects/aztec-nr/aztec/src/oracle/version.nr b/noir-projects/aztec-nr/aztec/src/oracle/version.nr index bb733bb42fae..2adcca12621a 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/version.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/version.nr @@ -4,7 +4,7 @@ /// /// @dev Whenever a contract function or Noir test is run, the `aztec_utl_assertCompatibleOracleVersion` oracle is /// called and if the oracle version is incompatible an error is thrown. -pub global ORACLE_VERSION: Field = 18; +pub global ORACLE_VERSION: Field = 19; /// Asserts that the version of the oracle is compatible with the version expected by the contract. pub fn assert_compatible_oracle_version() { diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/legacy_oracle_mappings.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/legacy_oracle_mappings.ts index d8c7297824db..ccf8b110c5a6 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/legacy_oracle_mappings.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/legacy_oracle_mappings.ts @@ -74,13 +74,6 @@ export function buildLegacyOracleCallbacks(oracle: Oracle): ACIRCallback { ): Promise => oracle.aztec_utl_copyCapsule(contractAddress, srcSlot, dstSlot, numEntries), utilityDeleteCapsule: (contractAddress: ACVMField[], slot: ACVMField[]): Promise => oracle.aztec_utl_deleteCapsule(contractAddress, slot), - utilityAes128Decrypt: ( - ciphertextBVecStorage: ACVMField[], - ciphertextLength: ACVMField[], - iv: ACVMField[], - symKey: ACVMField[], - ): Promise<(ACVMField | ACVMField[])[]> => - oracle.aztec_utl_aes128Decrypt(ciphertextBVecStorage, ciphertextLength, iv, symKey), utilityGetSharedSecret: ( address: ACVMField[], ephPKField0: ACVMField[], diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/oracle.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/oracle.ts index 4ae37c0cee92..5625f75b9835 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/oracle.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/oracle.ts @@ -604,9 +604,8 @@ export class Oracle { return []; } - // TODO(F-452): Return Option and wrap in try/catch so BB exceptions don't crash PXE. // eslint-disable-next-line camelcase - async aztec_utl_aes128Decrypt( + async aztec_utl_tryAes128Decrypt( ciphertextBVecStorage: ACVMField[], [ciphertextLength]: ACVMField[], iv: ACVMField[], @@ -616,8 +615,15 @@ export class Oracle { const ivBuffer = fromUintArray(iv, 8); const symKeyBuffer = fromUintArray(symKey, 8); - const plaintext = await this.handlerAsUtility().aes128Decrypt(ciphertext, ivBuffer, symKeyBuffer); - return bufferToBoundedVec(plaintext, ciphertextBVecStorage.length); + // Noir Option is encoded as [is_some: Field, storage: Field[], length: Field]. + try { + const plaintext = await this.handlerAsUtility().aes128Decrypt(ciphertext, ivBuffer, symKeyBuffer); + const [storage, length] = bufferToBoundedVec(plaintext, ciphertextBVecStorage.length); + return [toACVMField(1), storage, length]; + } catch { + const zeroStorage = Array(ciphertextBVecStorage.length).fill(toACVMField(0)); + return [toACVMField(0), zeroStorage, toACVMField(0)]; + } } // eslint-disable-next-line camelcase diff --git a/yarn-project/pxe/src/oracle_version.ts b/yarn-project/pxe/src/oracle_version.ts index b027d2b7e182..8d3e39230464 100644 --- a/yarn-project/pxe/src/oracle_version.ts +++ b/yarn-project/pxe/src/oracle_version.ts @@ -4,9 +4,9 @@ /// /// @dev Whenever a contract function or Noir test is run, the `aztec_utl_assertCompatibleOracleVersion` oracle is called /// and if the oracle version is incompatible an error is thrown. -export const ORACLE_VERSION = 18; +export const ORACLE_VERSION = 19; /// This hash is computed as by hashing the Oracle interface and it is used to detect when the Oracle interface changes, /// which in turn implies that you need to update the ORACLE_VERSION constant in this file and in /// `noir-projects/aztec-nr/aztec/src/oracle/version.nr`. -export const ORACLE_INTERFACE_HASH = 'ab9ba6bb6675a4663d66af494bec195ab1d18fa2b646549323e0f54cac6af1de'; +export const ORACLE_INTERFACE_HASH = '038f85d2e84afb5f688ce868249a0c8c9f94d605690bd2f1b59b1eb978b9c670'; diff --git a/yarn-project/txe/src/rpc_translator.ts b/yarn-project/txe/src/rpc_translator.ts index 04b8cf8d4479..ecaee7e8b3b7 100644 --- a/yarn-project/txe/src/rpc_translator.ts +++ b/yarn-project/txe/src/rpc_translator.ts @@ -878,9 +878,8 @@ export class RPCTranslator { // The compiler didn't throw an error, so it took me a while to learn of the existence of this file, and that I need // to implement this function here. Isn't there a way to programmatically identify that this is missing, given the // existence of a txe_oracle method? - // TODO(F-452): Return Option and wrap in try/catch so BB exceptions don't crash TXE. // eslint-disable-next-line camelcase - async aztec_utl_aes128Decrypt( + async aztec_utl_tryAes128Decrypt( foreignCiphertextBVecStorage: ForeignCallArray, foreignCiphertextLength: ForeignCallSingle, foreignIv: ForeignCallArray, @@ -890,11 +889,18 @@ export class RPCTranslator { const iv = fromUintArray(foreignIv, 8); const symKey = fromUintArray(foreignSymKey, 8); - const plaintextBuffer = await this.handlerAsUtility().aes128Decrypt(ciphertext, iv, symKey); - - return toForeignCallResult( - arrayToBoundedVec(bufferToU8Array(plaintextBuffer), foreignCiphertextBVecStorage.length), - ); + // Noir Option is encoded as [is_some: Field, storage: Field[], length: Field]. + try { + const plaintextBuffer = await this.handlerAsUtility().aes128Decrypt(ciphertext, iv, symKey); + const [storage, length] = arrayToBoundedVec( + bufferToU8Array(plaintextBuffer), + foreignCiphertextBVecStorage.length, + ); + return toForeignCallResult([toSingle(new Fr(1)), storage, length]); + } catch { + const zeroStorage = toArray(Array(foreignCiphertextBVecStorage.length).fill(new Fr(0))); + return toForeignCallResult([toSingle(new Fr(0)), zeroStorage, toSingle(new Fr(0))]); + } } // eslint-disable-next-line camelcase