Add Hmac_DRBG by keks · Pull Request #1382 · cryspen/libcrux

keks · 2026-04-09T14:08:52Z

This PR adds a DRBG and a wrapper that allows it to implement rand::CryptoRng.

- rand: auto-seed - health-tests: health tests for entropy - hmac: incremental API - hmac: wycheproof tests

See pp. 8, 14 of https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-107r1.pdf

This reverts commit 248f96d.

franziskuskiefer · 2026-04-13T05:55:55Z

 }

 mod impl_hacl;
+mod incremental;


We should update the docs around this new, not yet verified version.

Should include a changelog entry, too.

The changelog entry should be under an ### Added heading. (We started doing a bit more fine-grained tracking of the change categories at some point, but didn't adapt do the categorization for earlier versions retroactively.)

franziskuskiefer · 2026-04-13T05:57:20Z

 libcrux-secrets = { version = "=0.0.5", path = "crates/utils/secrets" }

+# 3rd Party
+rand = { version = "0.9", default-features = false }


We may bump to 0.10 before merging this and need to update here then as well.

I've updated the dependabot PR for this here: #1385

jschneider-bensch

Just some initial comments on docs, will give it a more thorough review later.

jschneider-bensch · 2026-04-13T12:29:14Z

+/// [`HmacDrbg::generate`]: crate::HmacDrbg::generate
+#[derive(Debug, PartialEq)]
+pub enum GenerateError {
+    /// The reseed counter has exceeded [`RESEED_INTERVAL`]; call `reseed` before generating.


Suggested change

/// The reseed counter has exceeded [`RESEED_INTERVAL`]; call `reseed` before generating.

/// The reseed counter has exceeded [`RESEED_INTERVAL`](crate::RESEED_INTERVAL); call `reseed` before generating.

jschneider-bensch · 2026-04-13T12:29:33Z

+    /// The reseed counter has exceeded [`RESEED_INTERVAL`]; call `reseed` before generating.
+    ReseedRequired,
+
+    /// The requested output length is zero or exceeds [`MAX_GENERATE_BYTES`].


Suggested change

/// The requested output length is zero or exceeds [`MAX_GENERATE_BYTES`].

/// The requested output length is zero or exceeds [`MAX_GENERATE_BYTES`](crate::MAX_GENERATE_BYTES).

jschneider-bensch · 2026-04-13T12:30:21Z

+    ///   `V ≠ V_old`; equality signals a catastrophic HMAC fixed-point.
+    /// - **Inter-call / T4** (AIS 20/31): each new block must differ from the last
+    ///   block of the previous `generate` call.
+    /// - **RCT** (SP 800-90B §4.4.1): no run of ≥ [`RCT_C`] identical bytes.


The doc links here are broken if the health-tests feature is not enabled.

jschneider-bensch

Still didn't look too closely at the health checks yet, but have enough comments on the rest.

jschneider-bensch · 2026-04-15T07:25:43Z

+/// Requesting more returns [`GenerateError::RequestTooLarge`].
+pub const MAX_GENERATE_BYTES: usize = 65_536;
+
+/// Minimum entropy input length in bytes (SP 800-90A Table 2: security strength).


Should we reference Table 1 here as well? That is where the justification is that a maximum designed security strength of 256 bits allows for instantiating all the lower security strengths as well. I found it a bit confusing since the concrete number stated here is not found in Table 2, which instead references another recommendation document for the security strength.

jschneider-bensch · 2026-04-15T07:55:44Z

+
+    /// Instantiate an HMAC_DRBG from explicit entropy material (SP 800-90A §10.1.2.3).
+    ///
+    /// - `entropy_input`: must contain at least [`MIN_ENTROPY_BYTES`] bytes of entropy.


We do check the length of the provided entropy with the health-tests feature, but should we maybe do it in any case since it's relatively inexpensive?

I see that we don't have Error variants for these checks conditions failing outside of the health-tests feature...

I was thinking about this as well. This is how it's done in the standard and it was given to me like this by @franziskuskiefer, so I decided against changing it. I liked that the health check error was so opaque and didn't want to open it up. Maybe we can add the check to only run if we don't do the health checks? Then we only need the additional error variant in that case, and the error variants don't overlap.

We'll have to do the same changes in Reseed.

yeah sounds reasonable to me.

jschneider-bensch · 2026-04-15T09:01:11Z

+        for b in self.v.iter_mut() {
+            unsafe { core::ptr::write_volatile(b, 0) };
+        }
+        atomic::compiler_fence(atomic::Ordering::SeqCst);


For my understanding: What's the purpose of this atomic::compiler_fence here?

It tells the compiler to not move a read before the zeroization when it reorders for optimization.

I think.

I'm not sure about the reasoning here. From the documentation for std::sync::atomic::compiler_fence I understand that it applies to intra-thread synchronization only and for this to work the read/write operations should be atomic ones. But as far as I can tell we don't do any atomic reads. The write_volatile is guaranteed to not to be re-ordered wrt to other externally observable events but can not be considered atomic.

@franziskuskiefer What are the requirements for zeroization here?

This is stupid zeroization. I'd be totally fine with dropping this here, or moving into a separate crate to allow zerization across crates at some point.

jschneider-bensch · 2026-04-15T09:17:07Z

+
+    /// Block-level tests run on each generated V block.
+    ///
+    /// - **Test 1** (NIST 800-90C §9.3.3): `new_block ≠ v_before` (HMAC fixed-point).


Is that reference correct? I can't find section 9.3.3 in NIST SP-800-90C.

It also is not in SP-800-90Ar1, or in the Health Test section of that document. Not sure where that came from.

@franziskuskiefer I think you added this. Do you remember?

Oh, that looks hallucinated. 800-90A §10.1.2 may be more appropriate here.

That's the thing. I can't find the check anywhere in the document.Not in 10.1.2 and not in 11.3.

jschneider-bensch · 2026-04-15T09:24:55Z

 }

 mod impl_hacl;
+mod incremental;


Should include a changelog entry, too.

keks · 2026-04-16T08:27:35Z

How do you all feel about renaming the crate and module to hmac_drbg? It seems DRBG would be the scheme/primitive, but the construction we put into the algorithms folder is HMAC_DRBG.

jschneider-bensch · 2026-04-16T08:29:54Z

How do you all feel about renaming the crate and module to hmac_drbg? It seems DRBG would be the scheme/primitive, but the construction we put into the algorithms folder is HMAC_DRBG.

That sounds reasonable 👍

…l enum variant also conditional

Review re-requested

jschneider-bensch · 2026-05-05T08:17:50Z

@keks Are the remaining comments all related to health tests? If so, we could also address those in a follow up.

franziskuskiefer and others added 19 commits March 31, 2026 16:10

drbg: implement hmac drbg

a9a7fb7

- rand: auto-seed - health-tests: health tests for entropy - hmac: incremental API - hmac: wycheproof tests

drbg: move hmac trait to submodule, split errors

af45967

split errors into submodule, add trait proposal

4dfdc40

drop errors banner

782702c

drbg: use rand-0.10

d5fedcd

split out errors for instantiating from rng

c7e91f0

drop unneeded error variant

e20412f

fix SECURITY_STRENGTH for HmacSha384, HmacSha512

298d5cc

See pp. 8, 14 of https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-107r1.pdf

Revert "fix SECURITY_STRENGTH for HmacSha384, HmacSha512"

1f0cf44

This reverts commit 248f96d.

move DrbgError into errors module

7cfc896

Rename rng_trait module to rand

37569bd

move minimum entropy length out of trait, into module const

fb5b5fd

move around code

61cd50f

add simple Rng

21ec595

add tests

e875086

drbg: rename and move auto-reseeding wrapper

2afb980

drbg: add second rng constructor that only uses inner rng for reseeding

c6b5b8f

improve comments

3237c37

drop DrbgError

6bc4199

keks requested review from a team as code owners April 9, 2026 14:08

keks requested a review from jschneider-bensch April 9, 2026 14:08

fmt

e27e03a

franziskuskiefer reviewed Apr 13, 2026

View reviewed changes

jschneider-bensch previously requested changes Apr 13, 2026

View reviewed changes

jschneider-bensch previously requested changes Apr 15, 2026

View reviewed changes

jschneider-bensch added the waiting-on-author Status: This is awaiting some action from the author. label Apr 15, 2026

add drbg module to libcrux crate

29c758d

keks added 8 commits April 16, 2026 11:19

add drbg crate readme

7e4a8d9

update comment to ref table 1; fix const

add73b0

directly write to self.key

5e0c91e

update nonce comment

005b482

impl Reseed for TryCryptoRng

ad7b201

fix drbg doc by moving around constants and making link to conditiona…

3a49dca

…l enum variant also conditional

note addition of incremental hmac in hmac changelog

8621120

rename module and crate

17ce7dc

keks requested a review from jschneider-bensch April 16, 2026 15:53

simplify additional input

efc964a

keks changed the title ~~Add DRBG~~ Add Hmac_DRBG Apr 27, 2026

keks linked an issue Apr 27, 2026 that may be closed by this pull request

Add HMAC_DRBG #1396

Open

keks added 4 commits April 30, 2026 18:26

rename HMAC changelog heading

54f7d60

update hmac_drbg crate name in fuzzing code

8603598

clarify changelog entry

9b3c6b7

rename error variant

7aa5bab

	/// The reseed counter has exceeded [`RESEED_INTERVAL`]; call `reseed` before generating.
	/// The reseed counter has exceeded [`RESEED_INTERVAL`](crate::RESEED_INTERVAL); call `reseed` before generating.

	/// The requested output length is zero or exceeds [`MAX_GENERATE_BYTES`].
	/// The requested output length is zero or exceeds [`MAX_GENERATE_BYTES`](crate::MAX_GENERATE_BYTES).

Conversation

keks commented Apr 9, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschneider-bensch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jschneider-bensch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keks Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keks Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keks commented Apr 16, 2026

Uh oh!

jschneider-bensch commented Apr 16, 2026

Uh oh!

jschneider-bensch commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

keks Apr 16, 2026 •

edited

Loading

keks Apr 22, 2026 •

edited

Loading