feat(native-rust): message-header structure (V1/V2 header, header auth)#909
Conversation
There was a problem hiding this comment.
Pull request overview
Implements core Rust-side serialization/deserialization for AWS Encryption SDK message headers, focusing on V1/V2 header bodies, header authentication, and associated field types as defined in the message-header spec.
Changes:
- Adds V1/V2 header body read/write implementations and shared header field types (version/type/content type, suite ID, message ID).
- Adds header authentication (V1 IV+tag, V2 tag-only) read/write.
- Introduces a suite of spec-assertion tests for header structure, header types, header auth, and V1/V2 header body layouts.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| esdk/src/message/mod.rs | Introduces the message module layout plus helper writers (NoopWriter, DigestWriter). |
| esdk/src/message/header.rs | Top-level header read/write orchestration and validation helpers. |
| esdk/src/message/header_types.rs | Defines header field enums/structs and shared read/write helpers (version/type/content type, suite ID, message ID). |
| esdk/src/message/header_auth.rs | Implements header authentication tag serialization/deserialization for V1 and V2. |
| esdk/src/message/v1_header_body.rs | Implements V1 header body serialization/deserialization (reserved bytes, IV length, frame length, etc.). |
| esdk/src/message/v2_header_body.rs | Implements V2 header body serialization/deserialization (commitment/suite data handling, etc.). |
| esdk/tests/test_header_structure.rs | Adds structure-level tests (endianness, ordering, EDK count constraints, frame length constraints). |
| esdk/tests/test_header_types.rs | Adds tests for version/type/content-type parsing and negative cases (including Base64-like input). |
| esdk/tests/test_header_auth.rs | Adds tests for V1/V2 header auth layout and tamper detection. |
| esdk/tests/test_v1_header_body.rs | Adds detailed V1 header body layout/field-size/order and negative-tamper tests. |
| esdk/tests/test_v2_header_body.rs | Adds detailed V2 header body layout/field-size/order tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub(crate) fn generate_message_id(suite: &AlgorithmSuite) -> Result<MessageId, Error> { | ||
| let length = if suite.message_version == 1 { | ||
| MESSAGE_ID_LEN_V1 | ||
| } else { |
There was a problem hiding this comment.
Any suite whose message_version is not exactly 1 falls through to V2 ID — including a hypothetical V3 or a bad value. Should we have explicit match for all available version?
There was a problem hiding this comment.
Good catch, updated
| } | ||
|
|
||
| impl Default for HeaderAuth { | ||
| fn default() -> Self { |
There was a problem hiding this comment.
Is there a way the library uses default by error and wrongly uses all zero bytes IV and tag? Can we somehow enforce our code path never uses it?
There was a problem hiding this comment.
Good catch. No code path uses the default today but I removed the default so no future code can pick it up.
| //# If the message format version associated with the [algorithm suite](../framework/algorithm-suites.md#supported-algorithm-suites) is 1.0, | ||
| //# the remaining header fields MUST be serialized according to the | ||
| //# [Header Body Version 1.0](../data-format/message-header.md#header-body-version-10) specification: | ||
| HeaderBody::V1Body(x) => write_v1_header_body(w, x), |
There was a problem hiding this comment.
Do we need to enforce algorithm suite and the header body version is in agreement?
There was a problem hiding this comment.
This is enforced at construction so a mismatch can't happen on the encrypt path today. But I added a debug_assert! so any future drift gets caught in our test cases.
| encryption_context, | ||
| encrypted_data_keys, | ||
| content_type, | ||
| header_iv_length: u64::from(header_iv_length), |
There was a problem hiding this comment.
We are widening the iv length from u8 to u64 here. Will there be any code path which reads u64 and tries to convert to u8?
Also, can you help me understand why are we doing this here?
There was a problem hiding this comment.
No, this is cruft. Paths into this field convert to/from UInt64, but the value on the wire is always UInt8, so I'll just make the intermediate storage type a UInt8.
Implements the message header body itself — V1 and V2 header bodies, header authentication, header-level types, and shared header helpers — per message-header.md.
Scope:
esdk/src/message/mod.rs— shared machinery (DigestWriter,NoopWriter).esdk/src/message/header.rs— top-level header wrapper.esdk/src/message/header_types.rs— header-level types (version, algorithm suite id, content type, etc.).esdk/src/message/header_auth.rs— header IV + auth tag.esdk/src/message/v1_header_body.rs— V1 header body serialization/deserialization.esdk/src/message/v2_header_body.rs— V2 header body serialization/deserialization.esdk/src/message/shared_header_functions.rs— helpers shared between V1 and V2.esdk/tests/test_header_structure.rs,esdk/tests/test_header_types.rs,esdk/tests/test_header_auth.rs,esdk/tests/test_v1_header_body.rs,esdk/tests/test_v2_header_body.rs.The reusable sub-structures the header embeds (encrypted data keys, encryption context, low-level serialize helpers) are reviewed separately in #901.
data-format/message.mdcitations are absorbed into this PR since they mostly live alongside the header machinery and have no dedicated implementation file.Note: CI does not run on this PR. This branch is scoped for focused review and intentionally omits test helpers, scaffolding, and other supporting code. Full code — including helpers, test infrastructure, and working tests — lives on the
aws-crypto-rust/unreviewedbranch.If you want more context or to actually run the tests, see
aws-crypto-rust/unreviewed.Related spec: data-format/message-header.md