Package: rng-tools Version: 2-unofficial-mt.14-1 Severity: normal Tags: patch
Hi, 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). You can pull it from here: http://anonscm.debian.org/cgit/users/ron/rng-tools.git/commit/?id=27c260ae79e3b02f81062328497648b1e5f46613 And if reportbug --attach actually worked, the git-format-patch export should be included in this mail. There's a full description of the bugs in the commit message of it. This might be a good one to also push upstream, if they haven't found and fixed it already. I've only looked at the code in your fork so far. Cheers, Ron
>From 27c260ae79e3b02f81062328497648b1e5f46613 Mon Sep 17 00:00:00 2001 From: Ron <[email protected]> Date: Sat, 24 Jan 2015 19:12:50 +1030 Subject: [PATCH] Fix the broken FIPS 140-2 runs test There were two (related) bugs in the Runs test that this patch fixes. The first is it was not handling the special case of ctx->rlength == -1 so if the first bit of a block was not equal to ctx->last_bit (which is either carried over 'uselessly' from the previous block or set to 0 for the very first bit of a testing session, but either way is 'random' at the start of a new block), then it will try to record that event into ctx->runs[-1 + (6 * ctx->current_bit)]. Which in the 'best' case, will record it as a run of 0's when it should have been a run of one 1's, and in the worst case will instead increment ctx->poker[15] (when the fips_ctx_t struct isn't padded between those two fields). In any reasonable length run it will hit both cases and break both the Runs test and the Poker test results. The second bug is that the same code was wrongly incrementing the count for ctx->current_bit, when the run it observed is actually instead for ctx->last_bit. This wouldn't make a whole lot of difference to the end result, except for the code outside of that loop which records the final run of the block (correctly) for ctx->current_bit. That means the final run will record 0's or 1's into the opposite set of bins that the main processing loop does, again (in the worst case) resulting in a wrong count for some run length of both 0's and 1's, which some portion of the time will cause a false (positive or negative) Runs test result. Signed-off-by: Ron <[email protected]> --- fips.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 */ -- 2.1.1
