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

Possible issue with FIPS 140-2 test #105

Closed
diederikdehaas opened this issue Nov 23, 2020 · 4 comments
Closed

Possible issue with FIPS 140-2 test #105

diederikdehaas opened this issue Nov 23, 2020 · 4 comments

Comments

@diederikdehaas
Copy link
Contributor

I'm forwarding an issue that was reported to Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=847962
While it was reported against the Debian specific rng-tools implementation, it appears to be applicable to this implementation as well.
I have no idea or opinion on the validity of the bug/assertion, I'm only forwarding it.

Original bug report text (relevant parts anyway):

==============================================================
So I recently wrote some code to do the FIPS 140 tests, and though test
suites are good and all that, for something like this I also wanted to
run it against an independent implementation as a cross correlation on
sanity, and found the code in rng-tools which looked like it would be a
good candidate for that ...

Except they disagreed on a small, but large enough to be disturbing,
number of blocks for the Runs and Poker tests. At first glance we did
appear to be using the same thresholds, so clearly Something Was Wrong.

Happily (for me :), it turns out that my code was correct.
Also happily (for you, I hope ;), I've attached a patch that fixes two
bugs in the Runs test in rng-tools (which broke both tests).

==============================================================

The full .patch file can be downloaded here: https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=847962;filename=0001-Fix-the-broken-FIPS-140-2-runs-test.patch;msg=5 (apparently I couldn't attach it here)

Below you'll find the diff of that patch.

diff --git a/fips.c b/fips.c
index a4d5e32..f1c82ca 100644
--- a/fips.c
+++ b/fips.c
@@ -93,11 +93,10 @@ static void fips_test_store(fips_ctx_t *ctx, unsigned int rng_data)
 		if (ctx->current_bit != ctx->last_bit) {
 			/* If runlength is 1-6 count it in correct bucket. 0's go in
 			   runs[0-5] 1's go in runs[6-11] hence the 6*current_bit below */
-			if (ctx->rlength < 5) {
-				ctx->runs[ctx->rlength +
-				     (6 * ctx->current_bit)]++;
-			} else {
-				ctx->runs[5 + (6 * ctx->current_bit)]++;
+			if (ctx->rlength > 4) {
+				ctx->runs[5 + (6 * ctx->last_bit)]++;
+			} else if (ctx->rlength >= 0 ) {
+				ctx->runs[ctx->rlength + (6 * ctx->last_bit)]++;
 			}
 
 			/* Check if we just failed longrun test */
@nhorman
Copy link
Owner

nhorman commented Nov 23, 2020

can you please attach the full patch set as a pull request here

@diederikdehaas
Copy link
Contributor Author

I've asked the original patch author to submit it as that seems most appropriate.

JIC, I've added '.txt' to the patch file and attached it here
0001-Fix-the-broken-FIPS-140-2-runs-test-1.patch.txt

If he doesn't respond (in a 'reasonable' time), I could do it or you could do it yourself.

@diederikdehaas
Copy link
Contributor Author

Fixed in #106

@diederikdehaas
Copy link
Contributor Author

When is the next release planned?
Then I can prod the maintainer of the Debian package to upload a new version so it'll be part of Bullseye.

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

No branches or pull requests

2 participants