I think you should just use the EVP interface instead of the AES interface. So instead of calling AES_encrypt() you call EVP_EncryptUpdate(). Does something like EVP_aes_128_ctr() do the right thing?

It was my first attempt to implement ctr_generate cleanly using EVP_aes_x_ctr. However, all other drbg functions turned out not to be a natural fit for the aes-ctr evp interfaces, best example is probably ctr_update. After all i found it cleaner to "export" the required functions from the e_aes.c module such that the subsequent changes in drbg_rand.c become trivial.

Looking at these things again, i noticed i could use CRYPTO_ctr128_encrypt_ctr32 from modes.h which already implements the 32-to-128 bit counter-overflow instead of re-implementing this. However, the functions from e_aes.c would still be needed.

edit:
push -f 'ed my branch with a cleaner implementation using CRYPTO_ctr128_encrypt_ctr32.

In general, using aes-ctr functions to implement ctr_drbg generate requires the drbg counter to be incremented after operations (instead of before operations, as in the nist standard's pseudo code). I put these changes in an extra patch.

@dot-asm: can you comment on this?

I'm a bit limited in communications right now, so I'll only provide first impressions. First of all, wha-a-a-a-t? it wasn't using platform support, and was wired to bare AES_encrypt? [Well, this is obviously not question to Patrick.] But on more related note, so far the choice of platform-specific subroutine was so to say single if. I mean single if tied together key setup, and single-block and/or "stream" subroutines(*). I'd like to argue in favour of keeping this approach. My initial reaction also is why not EVP_aes_xxx_ctr32, so I'll have to look closer at rationale behind counter-argument, later that is...

(*) On s390x platform this if resides in assembly key setup procedure, so it's hard to spot. Nor is there need to tie single-block or "stream" subroutines as run-time switch is performed based on chosen key. Bottom line is that s390x is special, so we are rather talking about other platforms here.

First of all, wha-a-a-a-t? it wasn't using platform support, and was wired to bare AES_encrypt?

It' ancient code from the FIPS 2.0 object module. So that's probably the reason why platform support was never added.

As for changing the counter preincrement to a postincrement: Looks like a trivial change, but it is an obvious deviation from the NIST document. So maybe one should ask the FIPS experts whether this could be a potential issue in future FIPS validations.

The single-if thing would be easy to solve once we have agreed to use or not to use the evp-interfaces @ Andy.

So my rationale behind not using evp was the following:

• The pre- to post-increment change is independent of the interface choice. its merely a mismatch between the aes-ctr and ctr_drbg specifications.
• The four interfaces that im using now (keysetup, block, stream and CRYPTO_ctr128_encrypt_ctr32) are exactly what i need: the changes in the drbg code become trivial using them.
• ctr_drbg's generate is essentially an aes-ctr keystream. however, the update operation's "encrypt the counter and replace it" for example is straight-forward using plain aes instead of aes-ctr. using evp aes-ctr interface would need to encrypt a tmp-buffer and (re)init the evp context with the new iv (or work on evp internal data structures directly).

edit: using internal functions instead of EVP_... also saves you all the argument checking (partially overlapping etc) that is not needed here.

Reworked this one. On drbg init, aes platform support is checked using single 'if'.

As for changing the counter preincrement to a postincrement: Looks like a trivial change, but it is an obvious deviation from the NIST document. So maybe one should ask the FIPS experts whether this could be a potential issue in future FIPS validations.

First words of chapter 9 in NIST SP 800-90A Rev. 1 are:

All DRBG mechanisms and algorithms are described in this document in pseudocode, which is intended to explain functionality. The pseudocode is not intended to constrain real-world implementations.

Thats why i think that functional equivalence is enough to be compliant.
edit: not considering myself a FIPS expert, though

edit: not considering myself a FIPS expert, though

Neither do I. Just spent a lot of time reading throught the NIST document. ;)

using internal functions instead of EVP_... also saves you all the argument checking (partially overlapping etc) that is not needed here.

+1: The RAND_RBG api is a low level api so imho it's perfectly ok to use the low level crypto api. There is no gain from the abstraction provided by the EVP interface.

When you use the EVP_ interface it will use aesni automatically if supported, which isn't the case when you use the AES_ interface.

So I would like to clarify this a little. When we add a new method of doing things based on CPU capability, we only need to modify 1 place in the EVP layer to select the best method based on the CPU. This selection can depend on both what the CPU is capable of and the specific CPU model. This change seems to move such logic to 2 places.

For each platform the following holds: What 'get_aes' does for DRBG is analoguous to what 'EVP_aes_keylen_mode' does for AES-keylen-mode, except that it returns the 3 func-ptrs directly instead of returning ptr to a struct holding the 3 func-ptrs (what could easily be changed).

edit: to clarify: One could add a macro like BLOCK_CIPHER_custom/generic and the corresponding data structures and have DRBG analoguous to an AES mode.

The logic in the platform specific code is for BLOCK_CIPHER_custom and for BLOCK_CIPHER_generic, so why not for DRBG?

The logic in the non-platform specific code (eg ifdef HW_AES_CAPABLE) is for each AES mode's init, so why not for DRBG init?

This functionality must use the EVP level API.

This one was rejected due to concerns regarding FIPS. Since FIPS is a major feature of next release, maybe its the right time to have a look at it again.

Summary: Idea was to use optimized/hw-supported aes-ctr implementations for drbg generate which results in performance gains (see initial comment). This requires to store counter+1 in the state (instead of counter). This is behavioural equivalent to the nist spec but a derivation from its pseudo-code. The spec says: "All DRBG mechanisms and algorithms are described in this document in pseudocode, which is intended to explain functionality. The pseudocode is not intended to constrain real-world implementations."

Closing this one. The optimizations i originally proposed here have meanwhile been implemented using the EVP interface and merged to 1.1.1 53eb05b and master 40d5bae.