Skip to content

feat(native-rust): message-header inline structures (EDKs, encryption context)#901

Open
lucasmcdonald3 wants to merge 19 commits into
aws-crypto-rust/mainlinefrom
aws-crypto-rust/header-inline-review
Open

feat(native-rust): message-header inline structures (EDKs, encryption context)#901
lucasmcdonald3 wants to merge 19 commits into
aws-crypto-rust/mainlinefrom
aws-crypto-rust/header-inline-review

Conversation

@lucasmcdonald3
Copy link
Copy Markdown
Contributor

Implements the reusable sub-structures inside the message header — encrypted data keys and encryption context — per message-header.md.

Scope:

  • esdk/src/message/encrypted_data_keys.rs — EDK serialization/deserialization.
  • esdk/src/message/encryption_context.rs — canonical EC + header AAD sub-section.
  • esdk/src/message/serializable_types.rs — EC/EDK shape predicates and length helpers.
  • esdk/src/message/serialize_functions.rs — low-level big-endian read/write primitives.
  • Tests: esdk/tests/test_encrypted_data_keys.rs, esdk/tests/test_encryption_context_aad.rs.

The header body itself (V1/V2 header, header auth, message-ID, etc.) is reviewed separately; this PR covers only the inline structures the header embeds.


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/unreviewed branch.

If you want more context or to actually run the tests, see aws-crypto-rust/unreviewed.

Related spec: data-format/message-header.md

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Implements Rust serialization/deserialization for the message header’s inline sub-structures (Encrypted Data Keys and Encryption Context AAD encoding) per the message-header spec, plus accompanying spec-driven tests.

Changes:

  • Add low-level big-endian read/write primitives used by message serialization.
  • Add EDK and Encryption Context (canonical form + AAD subsection) serializers/deserializers and shape/length predicates.
  • Add spec-referenced tests for EDK and EC-AAD encoding behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
esdk/src/message/serialize_functions.rs Adds shared byte-level read/write helpers with “raw” mirroring support.
esdk/src/message/serializable_types.rs Adds EC/EDK type aliases plus length/shape predicates and helpers.
esdk/src/message/encryption_context.rs Implements canonical EC read + AAD section write logic.
esdk/src/message/encrypted_data_keys.rs Implements EDK section write/read for header serialization.
esdk/tests/test_encryption_context_aad.rs Adds spec-driven tests around EC AAD length/order/serialization.
esdk/tests/test_encrypted_data_keys.rs Adds spec-driven tests around EDK count/entry ordering and length fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread esdk/src/message/serializable_types.rs Outdated
Comment thread esdk/src/message/serializable_types.rs
Comment thread esdk/src/message/encrypted_data_keys.rs Outdated
Comment thread esdk/tests/test_encryption_context_aad.rs
Comment thread esdk/tests/test_encrypted_data_keys.rs
Comment thread esdk/src/message/encryption_context.rs
Comment thread esdk/src/message/encryption_context.rs Outdated
Comment thread esdk/src/message/encrypted_data_keys.rs
Comment thread esdk/tests/test_encryption_context_aad.rs Outdated
lucasmcdonald3 pushed a commit that referenced this pull request May 4, 2026
…efensive EDK count checks

Addresses Copilot review on PR #901.

- write_aad_section now emits key-value-pairs length = 2 (count) + pair bytes,
  matching Python/Java/C/JS. Previous output excluded the count field and
  diverged from every other ESDK on the wire for any non-empty encryption
  context.
- read_canonical_ec now enforces that the declared length equals the bytes
  consumed by deserialization; previously ignored the length entirely.
- write_edks rejects empty EDK lists (spec: count MUST be greater than 0).
- read_edks rejects count == 0 (defense in depth; previously surfaced as a
  downstream SerializationError from misaligned parsing).
- is_esdk_encryption_context / is_esdk_encrypted_data_keys: change >=
  boundary to > so u16::MAX values are valid (matches u16::try_from checks
  elsewhere).
- Tests updated for the new convention (kvp_len 12 -> 14 for single pair).
- Shared test_helpers (and one cross-scope test_decrypt_parse_header assertion)
  updated to match the on-wire convention.

Verified: test_encrypted_data_keys (9), test_encryption_context_aad (6),
test_decrypt_parse_header (11), and test_vectors_integration
(test_python_decrypt, test_java_decrypt, test_rust_encrypt_decrypt) all pass.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread esdk/src/message/encryption_context.rs Outdated
Comment thread esdk/src/message/serializable_types.rs
Comment thread esdk/tests/test_encryption_context_aad.rs
Comment thread esdk/tests/test_encrypted_data_keys.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants