Skip to content

crypto: AI generated audit feedback#3536

Open
smalis-msft wants to merge 1 commit into
microsoft:mainfrom
smalis-msft:crypto-audit-again
Open

crypto: AI generated audit feedback#3536
smalis-msft wants to merge 1 commit into
microsoft:mainfrom
smalis-msft:crypto-audit-again

Conversation

@smalis-msft
Copy link
Copy Markdown
Contributor

Some small cleanups:

  • Rename kbkdf 'salt' to 'label' to match the algorithm's official definition
  • Add some comments
  • Set xts_aes_256's tweak size to u64 everywhere to match the bcrypt restriction, so we don't accidentally depend on a larger tweak in the future

Copilot AI review requested due to automatic review settings May 20, 2026 19:23
@smalis-msft smalis-msft requested review from a team as code owners May 20, 2026 19:23
@smalis-msft smalis-msft enabled auto-merge (squash) May 20, 2026 19:23
@github-actions github-actions Bot added the unsafe Related to unsafe code label May 20, 2026
@github-actions
Copy link
Copy Markdown

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

@smalis-msft smalis-msft changed the title crypto: Ai generated audit feedback crypto: AI generated audit feedback May 20, 2026
Copy link
Copy Markdown
Contributor

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

This PR tightens and clarifies parts of the support/crypto APIs and their consumers by aligning parameter naming with the SP800-108 KBKDF spec and standardizing XTS-AES-256 tweak handling to a 64-bit tweak across all backends (matching Windows BCrypt constraints).

Changes:

  • Rename KBKDF HMAC-SHA256 parameter from salt to label across backends and the public wrapper.
  • Restrict XTS-AES-256 cipher tweak type to u64 in the public API and backend implementations; update disk encryption call sites accordingly.
  • Add clarifying documentation/comments (notably around X509Certificate::issued semantics and RSA error TODOs).

Reviewed changes

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

Show a summary per file
File Description
vm/devices/storage/disk_crypt/src/lib.rs Updates XTS cipher calls to pass u64 tweaks directly (no .into() conversion).
support/crypto/src/xts_aes_256/win.rs Changes BCrypt XTS tweak parameter type to u64 and simplifies IV construction.
support/crypto/src/xts_aes_256/ossl.rs Changes OpenSSL XTS tweak parameter type to u64 and zero-extends to 128-bit IV bytes.
support/crypto/src/xts_aes_256/mod.rs Updates the public XTS cipher API to u64 tweaks and adjusts tests/comments accordingly.
support/crypto/src/x509/mod.rs Documents that issued() is structural-only and does not verify signatures (points to verify()).
support/crypto/src/rsa/mod.rs Adds TODO notes about making RSA errors Clone once upstream supports it.
support/crypto/src/kbkdf/symcrypt.rs Renames KBKDF parameter saltlabel to match SP800-108 terminology.
support/crypto/src/kbkdf/ossl.rs Renames KBKDF parameter saltlabel in the OpenSSL-based implementation.
support/crypto/src/kbkdf/mod.rs Renames KBKDF public API parameter saltlabel and forwards accordingly.

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

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants