Skip to content

[DRAFT] Panic on any FIPS-relevant cryptographic operation#987

Draft
justsmth wants to merge 1 commit intoaws:mainfrom
justsmth:panic-on-fips
Draft

[DRAFT] Panic on any FIPS-relevant cryptographic operation#987
justsmth wants to merge 1 commit intoaws:mainfrom
justsmth:panic-on-fips

Conversation

@justsmth
Copy link
Contributor

@justsmth justsmth commented Jan 2, 2026

Issues:

Resolves #ISSUE-NUMBER1
Addresses #ISSUE-NUMBER2

Description of changes:

Describe current behavior and how your code changes that behavior. If there are no issues this pr is resolving, explain why this change is necessary.

Call-outs:

Point out areas that need special attention or support during the review process. Discuss architecture or design changes.

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

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.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.22%. Comparing base (c358484) to head (c4f5b61).
⚠️ Report is 306 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #987      +/-   ##
==========================================
- Coverage   95.80%   92.22%   -3.58%     
==========================================
  Files          61       71      +10     
  Lines        8143     9446    +1303     
  Branches        0     9446    +9446     
==========================================
+ Hits         7801     8712     +911     
- Misses        342      451     +109     
- Partials        0      283     +283     

☔ 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.

test_logging = []
unstable = []
prebuilt-nasm = ["aws-lc-sys?/prebuilt-nasm"]
panic_on_fips = []
Copy link

Choose a reason for hiding this comment

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

This should almost certainly be a cfg flag rather than a feature, since features are transitive? I don't think we want to let dependencies enable the "panic on fips" mode in binaries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this would be needed when there are two (or more) versions of aws-lc-rs resolved to -- one is desired, the others are not. So we'd need to ensure that the one desired version remains unaffected.

One option might be to check some environment variable in the aws-lc-rs build script, like
AWS_LC_RS_PANIC_ON_FIPS_UNLESS_VERSION=1.11.1, and emit a cargo:rustc-cfg=panic_on_fips only if CARGO_PKG_VERSION != AWS_LC_RS_PANIC_ON_FIPS_UNLESS_VERSION.

Copy link

Choose a reason for hiding this comment

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

I was assuming it would be something like, --cfg aws-lc-panic-unless-version="1.11" (and presumably patch version bumps are still ok, idk).

But ENV works too, anyway main point is, seems sketchy as a (transitive from dependencies) cargo feature flag.

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