I mistakenly believed that only RDSEED could fail. However, the Intel
manuals state that RDRAND can fail too.
I can't actually observe it failing, even with all cores running RDRAND in a
tight loop. In any case, the ChaCha20 masking means that it wouldn't be
a big deal anyway.
Still, this change tests the carry flag after RDRAND and the code falls
back to |CRYPTO_sysrand| if RDRAND has a hiccup. (The Intel manuals
suggest[1] calling RDRAND in a loop, ten times, before considering it to
have failed. But a single failure appears to be such a rare event that
the complexity in the asm code doesn't seem worth it.)
This change also adds an asm function to fill a buffer with random data.
Otherwise the overhead of calling |CRYPTO_rdrand|, and bouncing the data
in and out of memory starts to add up.
Thanks to W. Mark Kubacki, who may have reported this. (There's some
confusion in the bug report.)
Before:
Did 6148000 RNG (16 bytes) operations in 1000080us: 98.4 MB/s
Did 649000 RNG (256 bytes) operations in 1000281us: 166.1 MB/s
Did 22000 RNG (8192 bytes) operations in 1033538us: 174.4 MB/s
After:
Did 6573000 RNG (16 bytes) operations in 1000002us: 105.2 MB/s
Did 693000 RNG (256 bytes) operations in 1000127us: 177.4 MB/s
Did 24000 RNG (8192 bytes) operations in 1028466us: 191.2 MB/s
[1] Intel Reference Manual, section 7.3.17.1.
Change-Id: Iba7f82e844ebacef535472a31f2dd749aad1190a
Reviewed-on: https://boringssl-review.googlesource.com/5180
Reviewed-by: Adam Langley <agl@google.com>
We can't actually catch this with MSan because it requires all code be
instrumented, so it needs a NO_ASM build which no disables that code. valgrind
doesn't notice either, possibly because there's some computation being done on
it. Still, we shouldn't use uninitialized memory.
Also get us closer to being instrumentable by MSan, but the runner tests will
need to build against an instrumented STL and I haven't tried that yet.
Change-Id: I2d65697a3269b5b022899f361730a85c51ecaa12
Reviewed-on: https://boringssl-review.googlesource.com/4760
Reviewed-by: Adam Langley <agl@google.com>
MSVC doesn't like |const size_t len| in a function definition where the
declaration was just |size_t len| without the |const|. Also, MSVC needs
declarations of parameterless functions to have a |void| parameter list.
Change-Id: I91e01a12aca657b2ee1d653926f09cc52da2faed
Reviewed-on: https://boringssl-review.googlesource.com/4329
Reviewed-by: Adam Langley <agl@google.com>
OpenSSH, especially, does some terrible things that mean that it needs
the EVP_CIPHER structure to be exposed ☹. Damian is open to a better API
to replace this, but only if OpenSSL agree too. Either way, it won't be
happening soon.
Change-Id: I393b7a6af6694d4d2fe9ebcccd40286eff4029bd
Reviewed-on: https://boringssl-review.googlesource.com/4330
Reviewed-by: Adam Langley <agl@google.com>
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
