Skip to content

[PM-35105] Add hazmat module and shared COSE symmetric encryption#1202

Open
quexten wants to merge 8 commits into
km/high-entropy-secretfrom
km/hazmat-cose-symmetric
Open

[PM-35105] Add hazmat module and shared COSE symmetric encryption#1202
quexten wants to merge 8 commits into
km/high-entropy-secretfrom
km/hazmat-cose-symmetric

Conversation

@quexten

@quexten quexten commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-35105

📔 Objective

Stacked PR 2/4 (on top of #1201).

Introduces a hazmat::symmetric_encryption module (moving XChaCha20-Poly1305 into it and adding AES-256-GCM), and a shared cose::symmetric layer that seals/unseals COSE Encrypt/Encrypt0 bodies with either cipher, declaring the content-encryption algorithm in the protected header. Migrates the data, symmetric-key, and password-protected key envelopes to the shared encrypt/decrypt helpers.

The password envelope's COSE byte layout shifts, so the pin corruption test now flips a byte at the envelope midpoint.

Base: km/high-entropy-secret (#1201).

Introduces a hazmat::symmetric_encryption module (moving the XChaCha20-Poly1305
primitive into it and adding AES-256-GCM), and a shared cose::symmetric layer
that seals/unseals COSE Encrypt/Encrypt0 bodies with either cipher, declaring
the content-encryption algorithm in the protected header.

Migrates the data, symmetric-key, and password-protected key envelopes to the
shared encrypt/decrypt helpers. The password envelope's COSE byte layout shifts,
so the pin corruption test now flips a byte at the envelope midpoint.
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🔍 SDK Breaking Change Detection

SDK Version: km/hazmat-cose-symmetric (51d850a)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client Status Details
typescript ✅ No breaking changes detected Compilation passed with new SDK version - View Details
android ✅ No breaking changes detected Compilation passed with new SDK version - View Details

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.92244% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.17%. Comparing base (53771bf) to head (61d6e36).

Files with missing lines Patch % Lines
crates/bitwarden-crypto/src/cose/symmetric.rs 96.80% 15 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           km/high-entropy-secret    #1202      +/-   ##
==========================================================
+ Coverage                   85.09%   85.17%   +0.07%     
==========================================================
  Files                         465      467       +2     
  Lines                       63947    64291     +344     
==========================================================
+ Hits                        54415    54757     +342     
- Misses                       9532     9534       +2     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@quexten quexten requested a review from eligrubb June 22, 2026 14:33
@quexten quexten marked this pull request as ready for review June 22, 2026 21:28
@quexten quexten requested review from a team as code owners June 22, 2026 21:28
@quexten quexten requested a review from dani-garcia June 22, 2026 21:28
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new hazmat::symmetric_encryption module (XChaCha20-Poly1305 + new AES-256-GCM), the shared cose::symmetric seal/unseal layer, and the migration of the data, symmetric-key, and password-protected key envelopes onto the shared encrypt/decrypt helpers. Focus was on backward compatibility of existing encrypted data, AEAD/AAD correctness, and the new dependency. No blocking issues found.

Code Review Details

No new findings.

Backward-compatibility verification:

  • Data and symmetric-key envelopes always wrote the content-encryption algorithm into the protected header (confirmed against the base branch), so decrypt_cose0(.., None, ..) requiring a declared algorithm is correct for existing data.
  • The password-protected key envelope historically omitted the algorithm from the protected header, and the migration correctly supplies XChaCha20Poly1305 as the decryption fallback for those legacy envelopes. The pin-corruption test was correctly retargeted to the ciphertext midpoint for the shifted COSE byte layout.
  • The shared helpers recompute AAD from the stored protected header, so re-framing does not break previously sealed messages.

Notes (not findings):

  • AES-256-GCM is implemented and dispatchable but not yet wired to any caller (all three envelopes pass XChaCha20-Poly1305); its short-nonce caveat is documented and applies to future per-message-key callers.
  • aes-gcm is a pre-release (0.11.0-rc.4), consistent with the existing argon2 and chacha20poly1305 RC pins. AppSec dependency review is already tracked in VULN-650 (existing thread).
  • The [patch.crates-io] pin of crypto-bigint to a git tag is a documented, temporary registry-propagation workaround.

Dependency Changes

Package Change Ecosystem
aes-gcm New (0.11.0-rc.4) Cargo
crypto-bigint Patched to git v0.7.5 Cargo

@quexten quexten marked this pull request as draft June 22, 2026 21:31
Comment thread crates/bitwarden-crypto/src/safe/symmetric_key_envelope.rs Outdated
Comment thread crates/bitwarden-crypto/src/safe/symmetric_key_envelope.rs Outdated
Comment thread crates/bitwarden-crypto/src/safe/symmetric_key_envelope.rs Outdated
Comment thread crates/bitwarden-crypto/src/hazmat/mod.rs Outdated
Comment thread crates/bitwarden-crypto/src/cose/mod.rs Outdated
Comment thread crates/bitwarden-crypto/src/cose/symmetric.rs Outdated
Comment thread crates/bitwarden-crypto/src/cose/symmetric.rs Outdated
Comment thread crates/bitwarden-crypto/src/safe/password_protected_key_envelope.rs Outdated
@quexten quexten added the ai-review Request a Claude code review label Jun 22, 2026

[dependencies]
aes = { version = "0.9.0", features = ["zeroize"] }
aes-gcm = { version = "0.11.0-rc.4", features = ["zeroize"] }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ IMPORTANT: New direct dependency aes-gcm added — confirm AppSec dependency review was completed.

Details

aes-gcm = "0.11.0-rc.4" is a net-new direct dependency for bitwarden-crypto. Bitwarden's Dependency Review and Approval process requires AppSec review (typically tracked via a VULN task) before introducing a new dependency, and the PR description references only PM-35105 with no approval signal.

Two specifics worth confirming with AppSec:

  • This is a release-candidate version (0.11.0-rc.4), not a stable release. Pre-release crypto dependencies warrant extra scrutiny on maintenance/stability.
  • It is published by the RustCrypto org (same as the existing aes and chacha20poly1305 deps), which should ease review.

If approval was already obtained out-of-band, linking the VULN task in the PR description resolves this.

Reference: Bitwarden Dependency Review and Approval process (Stage 1–4).

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.

@quexten quexten marked this pull request as ready for review June 23, 2026 00:31
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant