Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions noir-projects/aztec-nr/aztec/src/messages/encoding.nr
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,25 @@ 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 = 417. 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
- HEADER_CIPHERTEXT_SIZE_IN_BYTES
- EPH_PK_SIGN_BYTE_SIZE_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
pub global MESSAGE_PLAINTEXT_LEN: u32 = MESSAGE_PLAINTEXT_SIZE_IN_BYTES / 32;
// each Field to 32 bytes. To convert the plaintext bytes back to fields, we divide by 32. We must also account for
// AES PKCS#7 padding expansion that is always appended. (417 - 16) / 32 = 12
pub global MESSAGE_PLAINTEXT_LEN: u32 = (MESSAGE_PLAINTEXT_SIZE_IN_BYTES - AES128_PKCS7_EXPANSION_IN_BYTES) / 32;

pub global MESSAGE_EXPANDED_METADATA_LEN: u32 = 1;

Expand Down Expand Up @@ -244,4 +248,16 @@ mod tests {
assert_eq(original_msg_type, unpacked_msg_type);
assert_eq(original_msg_metadata, unpacked_msg_metadata);
}

#[test]
fn encode_max_size_message() {
let msg_content = [0; MAX_MESSAGE_CONTENT_LEN];
let _ = encode_message(0, 0, msg_content);
}

#[test(should_fail_with = "Invalid message content: it must have a length of at most MAX_MESSAGE_CONTENT_LEN")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very surprised this is caught by a test. Static asserts trigger compilation errors, not runtime errors, and I thought #[test] would not catch these - I expected nargo test to fail instead. Can you check with the Noir team that this is correct?

fn encode_oversized_message_fails() {
let msg_content = [0; MAX_MESSAGE_CONTENT_LEN + 1];
let _ = encode_message(0, 0, msg_content);
}
}
40 changes: 37 additions & 3 deletions noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,13 @@ impl MessageEncryption for AES128 {
// 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.
fields_from_bytes(plaintext_bytes)
// Each field of the original message was serialized to 32 bytes so we convert back to fields.
// `fields_from_bytes` returns BoundedVec<Field, MESSAGE_PLAINTEXT_SIZE_IN_BYTES / 32> which
// may be larger than MESSAGE_PLAINTEXT_LEN, because MESSAGE_PLAINTEXT_SIZE_IN_BYTES is the
// raw byte capacity *before* subtracting AES PKCS#7 padding. After decryption the padding is
// stripped, so the actual content always fits in MESSAGE_PLAINTEXT_LEN fields. We shrink the
// capacity with subbvec to match the return type.
array::subbvec(fields_from_bytes(plaintext_bytes), 0)
})
}
}
Expand Down Expand Up @@ -407,6 +411,7 @@ mod test {
keys::ecdh_shared_secret::derive_ecdh_shared_secret,
messages::{encoding::MESSAGE_PLAINTEXT_LEN, encryption::message_encryption::MessageEncryption},
test::helpers::test_environment::TestEnvironment,
utils::conversion::fields_to_bytes::fields_to_bytes,
};
use crate::protocol::{address::AztecAddress, traits::FromField};
use super::{AES128, random_address_point};
Expand Down Expand Up @@ -489,6 +494,35 @@ mod test {
let _ = AES128::encrypt([1, 2, 3, 4], invalid_address);
}

#[test]
fn aes128_pkcs7_adds_full_block_for_field_aligned_input() {
// `fields_to_bytes` serializes each field to exactly 32 bytes, which is a multiple of
// the 16-byte AES block size. PKCS#7 therefore always appends a full 16-byte padding
// block.
let key = [0 as u8; 16];
let iv = [0 as u8; 16];

let one_field_bytes = fields_to_bytes([0; 1]);
assert_eq(std::aes128::aes128_encrypt(one_field_bytes, iv, key).len(), 32 + 16);

let two_field_bytes = fields_to_bytes([0; 2]);
assert_eq(std::aes128::aes128_encrypt(two_field_bytes, iv, key).len(), 64 + 16);
}

#[test]
unconstrained fn encrypt_max_size_plaintext() {
let address = AztecAddress { inner: 3 };
let plaintext: [Field; MESSAGE_PLAINTEXT_LEN] = [0; MESSAGE_PLAINTEXT_LEN];
let _ = AES128::encrypt(plaintext, address);
}

#[test(should_fail)]
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
Expand Down
36 changes: 35 additions & 1 deletion noir-projects/aztec-nr/aztec/src/messages/logs/note.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -121,4 +121,38 @@ 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]
fn encode_max_size_note() {
let note = MaxSizeNote { data: [0; MAX_NOTE_PACKED_LEN] };
let _ = encode_private_note_message(note, OWNER, STORAGE_SLOT, RANDOMNESS);
}

#[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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ mod test {
0x0000000000000000000000000000000000000000000000000000000000000000,
0x0000000000000000000000000000000000000000000000000000000000000000,
0x0000000000000000000000000000000000000000000000000000000000000000,
0x0000000000000000000000000000000000000000000000000000000000000000,
0x0000000000000000000000000000000000000000000000000000000000000002,
0x0000000000000000000000000000000000000000000000000000000000000006,
0x0000000000000000000000000000000000000000000000000000000000000007,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('EventValidationRequest', () => {
0,
0,
0,
0,
0, // serialized_event padding end
2, // bounded_vec_len
6, // event_commitment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { AztecAddress } from '@aztec/stdlib/aztec-address';
import { TxHash } from '@aztec/stdlib/tx';

// TODO(#14617): should we compute this from constants? This value is aztec-nr specific.
const MAX_EVENT_SERIALIZED_LEN = 11;
const MAX_EVENT_SERIALIZED_LEN = 10;

/**
* Intermediate struct used to perform batch event validation by PXE. The `utilityValidateAndStoreEnqueuedNotesAndEvents` oracle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ describe('NoteValidationRequest', () => {
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000', // content end (MAX_NOTE_PACKED_LEN = 10)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment was wrong, the value used to be 9

'0x0000000000000000000000000000000000000000000000000000000000000000', // content end (MAX_NOTE_PACKED_LEN = 8)
'0x0000000000000000000000000000000000000000000000000000000000000002', // content length
'0x0000000000000000000000000000000000000000000000000000000000000006', // note hash
'0x0000000000000000000000000000000000000000000000000000000000000007', // nullifier
Expand Down Expand Up @@ -57,9 +56,8 @@ describe('NoteValidationRequest', () => {
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000000', // content end (MAX_NOTE_PACKED_LEN = 10)
'0x0000000000000000000000000000000000000000000000000000000000000000', // extra item, this is a malformed serialization
'0x0000000000000000000000000000000000000000000000000000000000000000', // content end (MAX_NOTE_PACKED_LEN = 8)
'0x0000000000000000000000000000000000000000000000000000000000000000', // extra field beyond MAX_NOTE_PACKED_LEN, this is a malformed serialization
'0x0000000000000000000000000000000000000000000000000000000000000002', // content length
'0x0000000000000000000000000000000000000000000000000000000000000006', // note hash
'0x0000000000000000000000000000000000000000000000000000000000000007', // nullifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { AztecAddress } from '@aztec/stdlib/aztec-address';
import { TxHash } from '@aztec/stdlib/tx';

// TODO(#14617): should we compute this from constants? This value is aztec-nr specific.
export const MAX_NOTE_PACKED_LEN = 9;
export const MAX_NOTE_PACKED_LEN = 8;

/**
* Intermediate struct used to perform batch note validation by PXE. The `utilityValidateAndStoreEnqueuedNotesAndEvents` oracle
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/txe/src/rpc_translator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
toSingle,
} from './util/encoding.js';

const MAX_EVENT_LEN = 12; // This is MAX_MESSAGE_CONTENT_LEN - PRIVATE_EVENT_RESERVED_FIELDS
const MAX_EVENT_LEN = 10; // This is MAX_MESSAGE_CONTENT_LEN - PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why MAX_EVENT_LEN in rpc_translator.ts went from 12 to 10

Two corrections are stacked here:

  1. Pre-existing off-by-one: The old value of 12 was MAX_MESSAGE_CONTENT_LEN itself, not MAX_MESSAGE_CONTENT_LEN - PRIVATE_EVENT_MSG_PLAINTEXT_RESERVED_FIELDS_LEN as the comment claimed. The reserved fields subtraction (- 1) was never applied, so the old value should have been 11.
  2. PKCS#7 padding fix (the actual bug fix in this PR): MESSAGE_PLAINTEXT_LEN was computed without accounting for the 16-byte AES PKCS#7 padding block that is always appended when encrypting field-aligned data. Subtracting this reduces MESSAGE_PLAINTEXT_LEN by 1, which propagates down to MAX_MESSAGE_CONTENT_LEN (12 -> 11) and consequently MAX_EVENT_LEN (11 -> 10).

const MAX_PRIVATE_EVENTS_PER_TXE_QUERY = 5;

export class UnavailableOracleError extends Error {
Expand Down
Loading