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

Tree DRBG with Jitter Entropy root #1958

Merged

Conversation

torben-hansen
Copy link
Contributor

@torben-hansen torben-hansen commented Oct 31, 2024

Description of changes:

Implements a tree-DRBG using CPU Jitter as root. I had to implement some implementation-defined state, so switched from an immutable entropy source object entropy_source to a mutable one. Instead I factored out the vtable into a immutable object that defines the API to the entropy source.

The tree-DRBG is quite straightforward. The painful part is managing memory.

  • Steady-state is straightforward. No global locks, just thread-local ones except if one goes to the global DRBG.
  • In the graceful thread exit, the callback free_thread will manage thread-local memory.
  • When the process exists, we make sure to zeroize all thread-local data zeroize_thread in case they haven't been properly closed. A destructor in the tree-DRBG implementation will make sure to free the global DRBG + CPU Jitter object. This should also mean that we can safely unload DSO's.

Changes to CPU Jitter code entails:

  • __int64 is a language extension, old compilers aren't happy with that. Fix by replacing with portableint64_t

  • time is shadowing a global declaration from some imported header file. Fix by renaming parameters and local variables.

  • There are warnings, that turns into errors, originating from -Wconversion on the oldest GCC. These are false-positives because prior to GCC 4.3, -Wconversion didn't have anything to do with finding troublesome implicit conversions, it was an aid in converting from old C to modern C. Disable on those old compilers.

  • jitterentropy-base-windows.h distributes definitions of jent_get_nstime() throughout compilation units. However, many of these doesn't use the function causing unused function warnings for Windows clang builds. Tried different things but inlining was the only thing that worked.

Call-outs:

The requirement to zeroize memory before exiting is handled a little special. When all frontend DRBGs have been locked and can't produce anymore output, we kick off zeroizing the associated entropy source. We do this bottom-up: first the global DRBG is zeroized with random data, that thread-local DRBGs can use as seeds and randomise their state. This is needed because the front-end DRBGs might still try to reseed requiring access to the entropy source. Note, any data from the entropy source is just random and will never be used to generate any output to a consumer - because the frontend DRBGs has been locked and will spin forever if scheduled.

Testing:

There are currently some gaps in testing. I will implement comprehensive testing later.

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-commenter commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 75.40107% with 46 lines in your changes missing coverage. Please review.

Project coverage is 78.73%. Comparing base (3b6651c) to head (cfd04e6).

Files with missing lines Patch % Lines
...fipsmodule/rand/entropy/tree_drbg_jitter_entropy.c 71.72% 41 Missing ⚠️
crypto/fipsmodule/rand/entropy/entropy_sources.c 82.60% 4 Missing ⚠️
crypto/fipsmodule/rand/new_rand.c 90.00% 1 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##           randomness_generation    #1958      +/-   ##
=========================================================
- Coverage                  78.78%   78.73%   -0.05%     
=========================================================
  Files                        595      605      +10     
  Lines                     101780   102679     +899     
  Branches                   14424    14562     +138     
=========================================================
+ Hits                       80183    80845     +662     
- Misses                     20959    21133     +174     
- Partials                     638      701      +63     

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

@torben-hansen torben-hansen marked this pull request as ready for review November 8, 2024 18:17
@torben-hansen torben-hansen requested a review from a team as a code owner November 8, 2024 18:17
@torben-hansen torben-hansen enabled auto-merge (squash) November 8, 2024 18:17
@torben-hansen torben-hansen merged commit ddaf6d3 into aws:randomness_generation Nov 13, 2024
113 of 116 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