Skip to content

Add optional AAD support to encrypt/decrypt - #159

Open
beardedeagle wants to merge 2 commits into
ecies:masterfrom
beardedeagle:aad-support
Open

Add optional AAD support to encrypt/decrypt#159
beardedeagle wants to merge 2 commits into
ecies:masterfrom
beardedeagle:aad-support

Conversation

@beardedeagle

Copy link
Copy Markdown

Summary

Adds optional additional authenticated data (AAD) support to the public API:

  • New encrypt_with_aad / decrypt_with_aad functions, wired through all three symmetric backends (aes-rust, aes-openssl, xchacha20).
  • The existing encrypt / decrypt delegate with empty AAD, so behavior and wire compatibility for current users are unchanged (all pre-existing known-answer vectors pass untouched).

Motivation

Nothing in an ECIES ciphertext binds it to the context it was produced for: a ciphertext copied into a different envelope, map entry, or label still decrypts fine. AAD closes that gap — decryption authenticates the context string alongside the ciphertext, so a ciphertext presented under the wrong context fails instead of silently resolving. Useful for protocols that store many ciphertexts in one container and want each bound to its own name/label, without changing the wire format.

The AAD is never stored in or transmitted with the ciphertext; the same value must be supplied at decryption time.

Compatibility

  • Fully additive — no changes to existing public signatures.
  • Empty AAD is the legacy path: legacy ciphertexts decrypt through decrypt_with_aad(.., b"") and vice versa, so mixed deployments interoperate.
  • No new dependencies.

Tests

  • Round-trip with non-empty AAD on every backend; decryption under a different AAD fails.
  • Empty-AAD equivalence with the legacy API.
  • Known-answer tests pinning the wire layout against a direct AEAD invocation.
  • Public-API integration test + README example (doctest).
  • Feature matrix green: aes-rust, xchacha20, aes-short-nonce, aes-openssl (default), x25519/ed25519 compile checks.
  • Live round-trip against demo.ecies.org still passes.

Additive API: encrypt_with_aad/decrypt_with_aad on all three symmetric
backends (aes-rust, aes-openssl, xchacha20). The legacy encrypt/decrypt
delegate with empty AAD, so existing behavior and wire compatibility are
unchanged.
Copilot AI lite review requested due to automatic review settings August 5, 2026 05:13

Copilot AI left a comment

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.

Pull request overview

This PR adds optional Additional Authenticated Data (AAD) support to the crate’s ECIES encryption/decryption public API, enabling callers to authenticate external context alongside ciphertext without changing the wire format or breaking existing users.

Changes:

  • Introduces encrypt_with_aad / decrypt_with_aad and wires AAD through the symmetric backends (pure Rust AEAD and OpenSSL AES-GCM).
  • Keeps legacy encrypt / decrypt behavior by delegating to the AAD variants with empty AAD.
  • Adds tests (including layout/KAT-style coverage) and expands README with an AAD usage example.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/integration.rs Adds an integration test covering correct AAD round-trip and failure on wrong/missing AAD.
src/symmetric/openssl_aes.rs Adds AES-256-GCM encrypt_with_aad / decrypt_with_aad and makes legacy wrappers use empty AAD.
src/symmetric/mod.rs Adds sym_encrypt_with_aad / sym_decrypt_with_aad wrappers and AAD-focused unit tests (incl. wire layout check).
src/symmetric/aead.rs Extends pure-Rust AEAD backend with AAD-capable encrypt/decrypt wrappers.
src/lib.rs Exposes public encrypt_with_aad / decrypt_with_aad and routes legacy APIs through empty-AAD delegation.
README.md Documents AAD usage with an example snippet.

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

Comment thread tests/integration.rs Outdated
Drops the redundant all(all(..)) nesting; rustfmt's canonical form
applies. Semantics unchanged.
@beardedeagle

Copy link
Copy Markdown
Author

Heads-up for when the workflows get approved: the check-msrv job will fail, but it's pre-existing and unrelated to this PR.

The repo has no committed Cargo.lock and CI runs cargo generate-lockfile, so the MSRV job resolves fresh. Current resolution pulls zeroize v1.9.0, whose manifest requires cargo's edition2024 feature — cargo 1.81.0 can't parse it, so the job fails during dependency download before compiling anything. I reproduced the identical failure on pristine master (no PR commits), so any PR would hit this today.

For what it's worth, the code in this PR builds cleanly on 1.81.0 in all four check-msrv feature combos once zeroize is pinned to 1.8.1 locally.

Possible fixes, maintainer's call: bump the MSRV toolchain to one whose cargo understands edition2024 (1.85+), or commit a lockfile and build the MSRV leg with --locked. Happy to open a separate PR for either if you'd like.

beardedeagle added a commit to beardedeagle/postmaster that referenced this pull request Aug 5, 2026
…tion

- keychain reads described as "via security(1)" -> Security.framework API
  (README, architecture/postmaster.md, getting-started/key-provisioning.md)
  [A17]
- launcher-contract key_var_for: lossy suffix mapping -> hard error for
  anything outside [A-Za-z0-9_] (no aliasing) [A20]
- threat-model fd-closure: drop the "5 sites" count; dotenvy no longer
  opens its own fds (from_read_iter on postmaster-opened fds) [A13]
- dependency policy: 8 -> 10 direct deps, add security-framework (macOS)
  and a note on the ecies fork git+rev pin (ecies/rs#159)
- materialization-abi conformance list: drop the cryptographically
  unbound "or profile" leg of the replay case
- cleanup: document the .postmaster-managed sentinel guard in the
  cleanup rustdoc, HELP_CLEANUP, Invocation::Cleanup, and cli.md [A19]
- ramdisk.rs: drop the obsolete /usr/bin/security analogy

mdbook build, fmt, clippy, full locked test suite green.
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2106a80) to head (81e1b84).

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #159   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          778       832   +54     
=========================================
+ Hits           778       832   +54     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kigawas

kigawas commented Aug 5, 2026

Copy link
Copy Markdown
Member

Thank you for the pr (it seems from Claude though). As it's a library not an app, cargo.lock is intentionally gitignored. I'll do some housekeeping before reconciling _with_aad functions.

@beardedeagle

Copy link
Copy Markdown
Author

Thank you for the pr (it seems from Claude though). As it's a library not an app, cargo.lock is intentionally gitignored. I'll do some housekeeping before reconciling _with_aad functions.

that's fair, I had gpt-5.6-sol handle the pr text, doc strings and comment replies, the rest was me. I'd be happy to adjust the new API surface however you'd prefer, or throw in similar pr's (ie, AAD support) to the other repo's in this org. ran a lap around it and I see that you like to keep things consistent between each as much as possible.

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.

3 participants