Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor AWS_LC_FIPS_failure to always exist #2200

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

andrewhop
Copy link
Contributor

Description of changes:

This change does two things:

  1. Update bcm.c to always build a AWS_LC_FIPS_failure function that either prints the message and aborts or just prints the message
  2. Update the remaining spots in self_check.c to always use that function and not print directly to stderr

Call-outs:

This does not change any of the expected output from AWS-LC.

Testing:

The existing tests that check the FIPS failure messages ensure there is no change in behavior with this change

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.

@andrewhop andrewhop requested a review from a team as a code owner February 18, 2025 00:22
… if the library is build in FIPS mode or not
@andrewhop andrewhop force-pushed the fips_failure_cleanup branch from 6790e8a to 0270ce7 Compare February 18, 2025 00:25
@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 68.62745% with 16 lines in your changes missing coverage. Please review.

Project coverage is 79.04%. Comparing base (ca99b6f) to head (0270ce7).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
crypto/fipsmodule/self_check/self_check.c 74.46% 12 Missing ⚠️
crypto/fipsmodule/bcm.c 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2200      +/-   ##
==========================================
+ Coverage   79.03%   79.04%   +0.01%     
==========================================
  Files         612      612              
  Lines      106159   106167       +8     
  Branches    15003    15004       +1     
==========================================
+ Hits        83901    83919      +18     
+ Misses      21605    21595      -10     
  Partials      653      653              

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

fprintf(stderr, "CPU Jitter entropy RNG initialization failed.\n");
AWS_LC_FIPS_failure("CPU Jitter failed to initilize");
AWS_LC_FIPS_failure("CPU Jitter entropy RNG initialization failed");
Copy link
Member

@skmcgrail skmcgrail Feb 18, 2025

Choose a reason for hiding this comment

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

This does not change any of the expected output from AWS-LC.

Doesn't this change the output? Or was this an error from the earlier refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct to both:

  1. The message printing out the error to stderr "CPU Jitter entropy RNG initialization failed" has been there since we added jitter
  2. In 5553a20#diff-eb881f93cebfbb804479d55b9d097763dbb1d6e85da9d3d9bfbbf2d9cb1db04d I passed this new slightly different message to AWS_LC_FIPS_failure

@andrewhop andrewhop merged commit 8dd51c0 into aws:main Feb 19, 2025
114 of 119 checks passed
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.

4 participants