Add Entropy Source and DRNG Manager (ESDM) RNG support #4309

thillux · 2024-08-15T11:13:45Z

ESDM is a Linux-based user-space PRNG daemon, with configurable entropy sources.

It currently gets used, when a high level of control over entropy sources is desirable, e.g. for VPN appliance solutions.

In order to use this source, the ESDM server daemon has to be running and botan must be configured --with-esdm.

ESDM currently works only on Linux.

thillux · 2024-08-15T11:14:00Z

cc @smuellerDD

thillux · 2024-08-15T12:03:49Z

I tested the FFI-Bindings against https://github.com/thillux/botan-rs/tree/esdm-rng.

e.g. the following worked as expected:

let mut rng = RandomNumberGenerator::new_esdm_pr().unwrap();
for _ in 0..10 {
    let bytes = rng.read(32);
    println!("{bytes:x?}");
}

thillux · 2024-08-15T12:16:12Z

@reneme I reformatted the code with clang-format, after looking at the CI results.

coveralls · 2024-08-15T12:18:48Z

coverage: 91.257% (-0.01%) from 91.271%
when pulling 8a5a8cc on thillux:esdm-rng
into ac23bcb on randombit:master.

reneme · 2024-08-15T12:22:18Z

I reformatted the code with clang-format, after looking at the CI results.

Thanks! FYI: There are some Emacs, VS Code and .editorconfig config files in src/editors. In case you use any of this, you might want to symlink them into the repo's root dir and benefit from things like "format on save".

reneme

This seems interesting and the code looks good already. Thanks for the contribution! I've left a few suggestions down below.

Is there a way to test this in CI? I'd hope that we could just install and configure the daemon in src/scripts/ci/setup_gh_actions.sh and run a few basic smoke tests against it in the unit tests.

src/lib/rng/esdm_rng/esdm_rng.cpp

src/lib/rng/esdm_rng/info.txt

src/lib/rng/esdm_rng/esdm_rng.h

src/lib/rng/esdm_rng/esdm_rng.cpp

src/lib/rng/esdm_rng/esdm_rng.h

src/lib/rng/esdm_rng/esdm_rng.cpp

src/lib/rng/esdm_rng/esdm_rng.h

src/lib/utils/esdm/info.txt

randombit · 2024-08-16T02:47:11Z

So the change already looks pretty good, I think reneme already hit most of the points I would raise on it, but I am going to have to be a downer here and ask

Can you expand more on where and how ESDM is used? It does not seem to be widely distributed, eg per https://repology.org/project/esdm/versions it is only included in AUR and nix (the Debian experimental listing is a name collision with another package). In general our integrations with other systems have been for very widely deployed (eg TPM, PKCS11, OS provided interfaces, or widely deployed libs like zlib or sqlite3) and I'm hesitant to accept something that is so niche.

mouse07410 · 2024-08-16T02:51:23Z

I'm pretty sure it doesn't work on Mac, for example. No idea if it runs on Windows.

thillux · 2024-08-16T09:43:51Z

So the change already looks pretty good, I think reneme already hit most of the points I would raise on it, but I am going to have to be a downer here and ask

Can you expand more on where and how ESDM is used? It does not seem to be widely distributed, eg per https://repology.org/project/esdm/versions it is only included in AUR and nix (the Debian experimental listing is a name collision with another package). In general our integrations with other systems have been for very widely deployed (eg TPM, PKCS11, OS provided interfaces, or widely deployed libs like zlib or sqlite3) and I'm hesitant to accept something that is so niche.

You're right with ESDM being niche from a major Linux distribution point of view. I cannot weaken/invalidate this argument.

ESDM is currently mostly used under the public radar I would say. My company uses it to provide randomness in a regulated/certified market, on an installed base of >= 100k Linux-based notebooks and plans to use ESDM in more products in the future.

For such a usage in a regulated market or for governmental customers, it offers the following key benefits from my point of view:

In combination with ESDM Botan can offer a NIST SP800-90B/C compatible RNG
ESDM offers more control over configured entropy sources, when needed, without the need for a developer to change Botan
ESDM can also use Botan for its internal cryptographic primitives, so less certification/code review work necessary for the developer
ESDM is currently the only way to conform to the upcoming NTG.1 definition of the AIS 20/31 2024 from BSI in Germany
It is easier to comply with changing governmental requirements in user-space in ESDM, than in kernel space (e.g. change /dev/random)

thillux · 2024-08-16T09:45:56Z

I'm pretty sure it doesn't work on Mac, for example. No idea if it runs on Windows.

That's correct, it runs on Unix/Linux.

thillux · 2024-08-16T10:22:51Z

I started working on your suggestions and will squash them together again, when finished.

thillux · 2024-08-16T10:48:12Z

This seems interesting and the code looks good already. Thanks for the contribution! I've left a few suggestions down below.

Is there a way to test this in CI? I'd hope that we could just install and configure the daemon in src/scripts/ci/setup_gh_actions.sh and run a few basic smoke tests against it in the unit tests.

How can I test this by myself? I think I'll need some iterations to get this running.

reneme · 2024-08-16T10:59:12Z

How can I test this by myself? I think I'll need some iterations to get this running.

The repository policy doesn't let first-time contributors run the CI pipeline. You could iterate in a branch on your private fork, though. For a full end-to-end test, unfortunately the process is commit->push->fail->fix->commit 😢

You can, however, run the underlying CI script locally (that assumes that setup_gh_actions.sh -- or something else -- has already prepared the environment). Simply run: src/scripts/ci_build.py shared to run most things the CI would for the 'shared' (library) target.

mouse07410 · 2024-08-16T11:14:43Z

That's correct, it runs on Unix/Linux.

Does Botan aim to only run on Linux? And at that, on one (possibly major) distribution of Linux?

thillux · 2024-08-16T11:17:37Z

That's correct, it runs on Unix/Linux.

Does Botan aim to only run on Linux? And at that, on one (possibly major) distribution of Linux?

The ESDM_RNG class is added as optional dependency. On Windows and/or MacOS it just can't be used. This PR does not limit Botan to only Linux.

src/lib/rng/esdm_rng/esdm_rng.cpp

src/lib/rng/esdm_rng/esdm_rng.h

src/lib/rng/esdm_rng/esdm_rng.cpp

thillux · 2024-08-16T14:42:50Z

I'll probably try to add the test this weekend or start of next week.

reneme

One more suggestion for our little initialization-concurrency rabbit hole 🕳️, otherwise looks good to me now. Looking forward to a test.

Regarding the utility of this: I tend to agree that this is rather niche. Nevertheless, this has sparked some interest in our company as well, for similar reasons as pointed out by @thillux. FWIW.

src/lib/rng/esdm_rng/esdm_rng.cpp

thillux · 2024-08-16T20:19:54Z

@reneme Smoke tests added and PR squashed again.

thillux · 2024-08-17T07:55:51Z

I'm maybe too stupid to get esdm-server started for the coverage and sanitizer targets. I'll pushed a fix that hopefully works.

thillux · 2024-08-17T13:58:05Z

All tests green in my repo now.

thillux · 2024-11-07T16:34:06Z

I started rebasing, will push an updated version for 3.7 soon.

thillux · 2024-11-08T18:13:20Z

@reneme @randombit CI was green on my testing branch yesterday.

thillux · 2024-11-26T15:13:25Z

@reneme @randombit Is there something left in this PR which I have to adapt to and was not aware of?

reneme

Thank you for your patience and effort that went into that! I left a few minor nits but generally this looks really good to me and also works fine on my local machine. I'll prepare a patch integrating the CI dependency as mentioned in one of the comments.

src/lib/rng/esdm_rng/esdm_rng.cpp

src/lib/rng/esdm_rng/esdm_rng.h

src/scripts/ci/setup_gh_actions.sh

src/scripts/test_python.py

thillux · 2024-12-12T08:02:41Z

I applied your suggestions and squashed them together with my last commit as suggested. Thanks for your review @reneme!

randombit

@thillux Generally looks good and we can probably merge for 3.7.0, some comments to address first though.

randombit · 2024-12-23T17:34:36Z

src/lib/rng/esdm_rng/esdm_rng.cpp

+      */
+      esdm_invoke(esdm_rpcc_write_data(in.data(), in.size()));
+      if(ret != 0) {
+         throw Botan::System_Error("Writing additional input to ESDM failed");


This is pretty confusing on first read - is esdm_invoke a macro that sets ret? 😬 If so please add a block comment here calling out this.

The comment mentioning this, was already at the initial declaration of ret in line 64. I added a short note above every check against ret nevertheless, as this behavior is probably not expected.

Sorry I missed this

randombit · 2024-12-23T17:36:17Z

src/scripts/config_for_oss_fuzz.py

@@ -29,7 +29,7 @@
    "--build-fuzzers=libfuzzer",
    "--build-targets=static",
    "--without-os-features=getrandom,getentropy",
-    "--disable-modules=system_rng,processor_rng",
+    "--disable-modules=system_rng,processor_rng,esdm_rng",


This shouldn’t be necessary - this is only built if explicitly requested. (Right??)

Done. I removed this, as you are right here.

randombit · 2024-12-23T17:38:16Z

src/scripts/test_python.py

@@ -179,6 +179,27 @@ def test_rng(self):

        user_rng.add_entropy('seed material...')

+        if platform.system() != "Linux":


Don’t do this. This breaks anyone (such as distributions or end users) which want to run the Python tests but are not using ESDM. Instead try to call ESDM. If it succeeds, great. If not, verify that we throw something indicating not implemented / not available.

Done, I moved this to a own unit test function, which checks for "not implemented" and gets skipped accordingly.

Perfect thanks

randombit · 2024-12-23T17:41:13Z

src/lib/rng/esdm_rng/esdm_rng.cpp

+      }
+
+   private:
+      std::mutex m_mutex;


Couple of things here

If using std::mutex then include <mutex> in the file

This then requires threads, which is ok BUT
2a) If we require threads here then add os_features block listing threads
2b) If it’s ok to use this on a machine without threads then use the wrappers in utils/mutex.h

I think the context ESDM is used in it probably doesn’t make sense to support systems without threads. This is typically just for people doing very niche baremetal/RTOS/etc kind of things so probably the correct fix is 2a.

Done. I second to use 1 & 2a.

thillux · 2025-01-02T22:41:07Z

@randombit Thanks for your feedback! If you can live with the current state, I'll squash the WIP commit. I just left it for now, in order to make it easier to look at the changes related to your suggestions.

randombit

Thanks @thillux LGTM pending a final squash

ESDM is a Linux-based user-space PRNG daemon, with configurable entropy sources. See: https://github.com/smuellerDD/esdm It currently gets used, when a high level of control over entropy sources is desirable, e.g. for VPN appliance solutions. In order to use this source, the ESDM server daemon has to be running and botan must be configured --with-esdm. ESDM currently works only on Linux. Signed-off-by: Markus Theil <[email protected]>

thillux · 2025-01-06T09:10:29Z

Thanks @thillux LGTM pending a final squash

Thanks for your review, squash is now pushed.

thillux force-pushed the esdm-rng branch 3 times, most recently from b031eb7 to 4151fd1 Compare August 15, 2024 11:52

thillux force-pushed the esdm-rng branch 2 times, most recently from 756c117 to df2d5fc Compare August 15, 2024 12:50

reneme reviewed Aug 15, 2024

View reviewed changes

reneme reviewed Aug 16, 2024

View reviewed changes

src/lib/rng/esdm_rng/esdm_rng.cpp Show resolved Hide resolved

src/lib/rng/esdm_rng/esdm_rng.h Outdated Show resolved Hide resolved

src/lib/rng/esdm_rng/esdm_rng.h Outdated Show resolved Hide resolved

src/lib/rng/esdm_rng/esdm_rng.cpp Outdated Show resolved Hide resolved

reneme reviewed Aug 16, 2024

View reviewed changes

src/lib/rng/esdm_rng/esdm_rng.cpp Outdated Show resolved Hide resolved

src/lib/rng/esdm_rng/esdm_rng.cpp Show resolved Hide resolved

src/lib/rng/esdm_rng/esdm_rng.cpp Show resolved Hide resolved

thillux force-pushed the esdm-rng branch 3 times, most recently from 9884c54 to 8343523 Compare August 16, 2024 20:19

thillux requested a review from reneme August 16, 2024 20:19

thillux force-pushed the esdm-rng branch 2 times, most recently from 5a91a9c to 26d69f8 Compare August 17, 2024 07:54

thillux force-pushed the esdm-rng branch from 26d69f8 to f2aa372 Compare August 17, 2024 13:57

randombit mentioned this pull request Aug 26, 2024

Add jitterentropy as RNG #4325

Merged

randombit added this to the Botan 3.7.0 milestone Oct 10, 2024

thillux force-pushed the esdm-rng branch from f2aa372 to 4574d17 Compare November 7, 2024 18:37

reneme approved these changes Dec 10, 2024

View reviewed changes

src/lib/rng/esdm_rng/esdm_rng.cpp Outdated Show resolved Hide resolved

src/lib/rng/esdm_rng/esdm_rng.h Outdated Show resolved Hide resolved

src/scripts/ci/setup_gh_actions.sh Outdated Show resolved Hide resolved

src/scripts/test_python.py Outdated Show resolved Hide resolved

thillux force-pushed the esdm-rng branch 2 times, most recently from 20c08cc to f3d19f7 Compare December 12, 2024 08:02

randombit requested changes Dec 23, 2024

View reviewed changes

thillux force-pushed the esdm-rng branch from f3d19f7 to 7d958f6 Compare January 2, 2025 22:34

randombit approved these changes Jan 4, 2025

View reviewed changes

thillux force-pushed the esdm-rng branch from 7d958f6 to 8a5a8cc Compare January 6, 2025 09:09

		@@ -179,6 +179,27 @@ def test_rng(self):

		user_rng.add_entropy('seed material...')

		if platform.system() != "Linux":

Add Entropy Source and DRNG Manager (ESDM) RNG support #4309

Are you sure you want to change the base?

Add Entropy Source and DRNG Manager (ESDM) RNG support #4309

Conversation

thillux commented Aug 15, 2024

thillux commented Aug 15, 2024

thillux commented Aug 15, 2024 • edited Loading

thillux commented Aug 15, 2024

coveralls commented Aug 15, 2024 • edited Loading

reneme commented Aug 15, 2024

reneme left a comment • edited Loading

Choose a reason for hiding this comment

randombit commented Aug 16, 2024

mouse07410 commented Aug 16, 2024

thillux commented Aug 16, 2024

thillux commented Aug 16, 2024

thillux commented Aug 16, 2024

thillux commented Aug 16, 2024

reneme commented Aug 16, 2024

mouse07410 commented Aug 16, 2024

thillux commented Aug 16, 2024

thillux commented Aug 16, 2024

reneme left a comment

Choose a reason for hiding this comment

thillux commented Aug 16, 2024

thillux commented Aug 17, 2024 • edited Loading

thillux commented Aug 17, 2024

thillux commented Nov 7, 2024

thillux commented Nov 8, 2024

thillux commented Nov 26, 2024

reneme left a comment

Choose a reason for hiding this comment

thillux commented Dec 12, 2024

randombit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thillux commented Jan 2, 2025

randombit left a comment

Choose a reason for hiding this comment

thillux commented Jan 6, 2025

thillux commented Aug 15, 2024 •

edited

Loading

coveralls commented Aug 15, 2024 •

edited

Loading

reneme left a comment •

edited

Loading

thillux commented Aug 17, 2024 •

edited

Loading