Skip to content

Conversation

@Elvrarin
Copy link
Contributor

@Elvrarin Elvrarin commented Oct 2, 2025

Issues:

See SIM P307101963

Description of changes:

Currently, aws-lc-rs only supports HMAC
This commit adds CMAC, with support for AES 128, 192, 256 and TDES keys.
https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38b.pdf

Call-outs:

  • Some APIs like Key::new(), sign(), Context::update(), Context::sign() may panic. This is done to be consistent with the API of HMAC
  • According to aws-lc's service indicator, AES 192 and TDES aren't FIPS approved. (Question: which fips document?)

Testing:

  • Unit tests added.
  • Integration Test data is copied from here in aws-lc

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@Elvrarin Elvrarin requested a review from a team as a code owner October 2, 2025 00:33
@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 91.13300% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.34%. Comparing base (c358484) to head (c661b48).
⚠️ Report is 275 commits behind head on main.

Files with missing lines Patch % Lines
aws-lc-rs/src/cmac.rs 91.13% 14 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #903      +/-   ##
==========================================
- Coverage   95.80%   92.34%   -3.46%     
==========================================
  Files          61       74      +13     
  Lines        8143     9862    +1719     
  Branches        0     9862    +9862     
==========================================
+ Hits         7801     9107    +1306     
- Misses        342      464     +122     
- Partials        0      291     +291     

☔ View full report in Codecov by Sentry.
📢 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.

@Elvrarin Elvrarin closed this Oct 7, 2025
@Elvrarin Elvrarin deleted the cmac-2 branch October 7, 2025 18:08
@Elvrarin Elvrarin reopened this Oct 7, 2025
@justsmth justsmth self-requested a review October 14, 2025 18:45
let signature = cmac::sign(&cmac_key, &input).unwrap();

// Truncate to tlen
let truncated_sig = &signature.as_ref()[..std::cmp::min(signature.as_ref().len(), tlen)];
Copy link
Contributor

Choose a reason for hiding this comment

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

The truncation used here and in the other functions is not needed -- it would actually hide a bug were we to produce too long of a signature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied these tests data, and test functions from the aws-lc. The original tests also does a truncation. Without the truncation, the tests start to fail.
https://github.com/aws/aws-lc/blob/main/crypto/fipsmodule/cmac/cmac_test.cc#L226-L227

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, we should just trancate it to tlen then -- it should never be longer than the actual tag:

let truncated_sig = &signature.as_ref()[..tlen];

@justsmth justsmth self-requested a review October 22, 2025 23:43
@justsmth justsmth requested a review from skmcgrail November 5, 2025 17:01
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