SP 800-90A deterministic Random Bit Generator
Closed, ResolvedPublic

Description

Considering SP800-131A, the ANSI X9.31 DRNG present in the current libgcrypt
code base is not allowed in FIPS mode any more starting from next year on. Even
this year, a FIPS 140-2 validation with an X9.31 DRNG is not allowed any more.

The following set of patches against the current GIT development tree of
libgcrypt implements the SP800-90A DRBG and integrates it with libgcrypt.

This submission collects all individual patches submitted over the last
weeks.

This DRBG is now also part of the Linux kernel 3.17 RC1. During the
time in the staging tree several patches were added. All applicable
patches are ported to the libgcrypt DRBG implementation. One of the
fixes covers a buffer overrun when using AES192.

The DRBG has been tested with the CAVS tool. The CAVS test driver is available
at [1].

[1] http://www.chronox.de/drbg.html

On 32 bit, a problem was just discovered in the kernel development branch: see
discussion in https://lkml.org/lkml/2014/8/26/59.

The base line is that the bit shift in drbg_max_addtl and drbg_max_requests are
stored in a size_t which is 32 bit on 32 bit machines. Yet, the bit shift is
larger than 32 bit. It will be fixed in the next installment of the patch.

werner claimed this task.Aug 29 2014, 11:43 AM
werner added a subscriber: werner.

Update of the entire patch set to version 8:
Fix the functions drbg_max_addtl, and drbg_max_requests to not overflow
size_t in 32 bit. Furthermore, the per-DRBG option for maximum requests,
maximum request bits and maximum length of additional information is removed
in favor of a global setting. The change only affects drbg.c

Note: only the patch 0001 is changed compared to version 7 of the patch set.

v8 does not compile on 32 bit

Changes v9:
drbg_int2byte replaced by drbg_cpu_to_be32 and the use of be_bswap32
and be_bswap64 for converting an integer into a character string.
Besides performance increase, it fixes the conversion on 32 bit machines.

Tested:

  • on 64 and 32 bit
  • CAVS on both arches
  • sanity tests on 32 and 64 bit
werner added a comment.Sep 3 2014, 9:03 AM

I have alsready pushed the GCRYCTL_DRBG_REINIT constant so that the value is
reserved.

The patch needs some rework: At a first glance gcrypt.h has new strucures using
symbols not from the gcrypt name space (_gcry or gcry prefixes). I noticed
quite some other Linux specific stuff like __u8 instead of unsigned character,
different indentation, and remove of page breaks (^L).

I have not looked at the API but I wonder why you don't re-use the existing
random API. Adding new functions for your RNG is not a good idea - unless there
is a real good reason for it. Exposing internals in the API is a no-go.

werner added a comment.Sep 3 2014, 9:04 AM

I would also prefer one patch and not a set of patches.

One last thing: Libcrypt is under the LGPLv2+ but your alternative license is
under an unspecified version of the GPL. Can you change the alternative license
to the "GNU Lesser General Public License as published by the Free Software
Foundation; either version 2.1 of the License, or (at your option) any later
version."?

re GPL: will do

re one patch: will do

I will make also the requested code changes. Though, the indentation makes me
wonder. As I am not used to this indentation, I used the help of indent wit the
following command as specified on the GNU home page: indent -nbad -bap -nbc -bbo
-bl -bli2 -bls -ncdb -nce -cp1 -cs -di2 -ndj -nfc1 -nfca -hnl -i2 -ip5 -lp -pcs
-psl -nsc -nsob. Now, what is wrong with the indentation?

Re reusing the API: I am wondering where I do not reuse the API? The normal
usage is via the gcry_randomize function. The external hook is used for:

  1. changing the type of DRBG (note, the code implements many random number

generators)

  1. allowing the use of the personalization string / additional info string (I

would not know how to use that with gcry_randomize.

  1. allow the CAVS testing to be performed.

If you have suggestions on how to cover that using existing APIs, I would be
very much interested in it.

werner added a comment.Sep 3 2014, 5:08 PM

Thanks.

re: indent: You mixed prototype and functions and thus by quickly browsing the
source I noticed the prototype - which are correct.

re: API it is a bit hard to check from just the patches. Thus I suggest that I
apply your next patch and then look again at it.

re: reregssion test: We can use a secret API for that so that it is not part of
the stable ABI. See for example tests/fipsdrv.c:init_external_rng_test

Please do not use C99 feature like // and struct init using symbols. I am
willing to fix that, though.

The patch v10 should now cover all change requests from Werner as documented in
the cover-letter.

However, I am not fully sure about the interface yet: the GCRY_DRBG_REINIT is
now solely limited to normal DRBG use. I do not see how that can be merged to
existing random interfaces.

The CAVS test interface is now isolated to the control value 75 similarly to the
X9.31 testing approach. However, the current approach triggers a compile time
warning about the undefined enum 75.

See [1] in libgcrypt/test/ for a test application that uses the DRBG in normal
mode and in CAVS test mode -- search for gcry_control.

Tested:

  • 32 / 64 bit
  • CAVS testing on both arches
  • brief stess testing by creating 200 MB of data and checking it with ent to see

that the output function is not broken

[1] http://www.chronox.de/drbg/drbg-20140907.tar.bz2

werner added a comment.Sep 7 2015, 6:27 PM

To be considered for 1.7

civ added a subscriber: civ.Nov 5 2015, 2:38 PM
werner closed this task as Resolved.Mar 18 2016, 8:50 AM

Applied to master will go into 1.7.