Firstly, FIPS 186-4 C.3.2 is broken for w=3. In step 4.1 it generates a
random, 2-bit number but in step 4.2 it rejects all four possible values
and loops forever.
Secondly, BN_is_prime_fasttext_ex is broken when trial division is
requested and the prime is small. It finds that the prime is a multiple
of a known prime and rejects it. We inherited this from OpenSSL.
Thirdly, we were missing a BN_CTX_start/end in
BN_enhanced_miller_rabin_primality_test, which didn't matter but could
have mattered in the future.
Change-Id: Ie988e37b14bb22acb005fc0652860be6bbd2a55f
Reviewed-on: https://boringssl-review.googlesource.com/15264
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
If all the inputs are given as assembly files then we can skip rewriting
symbols for the first file. If this file is bcm.s (i.e. the large
compiler output), this can save a few seconds of build time.
Change-Id: I4e4ea114acb86cd93e831b23b58f8c3401bc711c
Reviewed-on: https://boringssl-review.googlesource.com/15149
Reviewed-by: Adam Langley <agl@google.com>
delocate.go was adding redirector functions for the “_bss_get”
functions. (And they were going via the PLT too.)
Change-Id: I86bc9f0516a128a769068182cc280499f89b6c29
Reviewed-on: https://boringssl-review.googlesource.com/15148
Reviewed-by: Adam Langley <agl@google.com>
These relocations can be emitted for thread-local data. BoringSSL itself
doesn't include any thread-local variables that need linker support, but
ASAN and MSAN may inject these references in order to handle their own
bookkeeping.
Change-Id: I0c6e61d244be84d6bee5ccbf7c4ff4ea0f0b90fd
Reviewed-on: https://boringssl-review.googlesource.com/15147
Reviewed-by: Adam Langley <agl@google.com>
This is a version of PKCS7_get_certificates but does not require
crypto/x509.
BUG=54
Change-Id: I20152a8d1f3ed866d47e41fe576ea9f442490224
Reviewed-on: https://boringssl-review.googlesource.com/15129
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
A follow-up change will add a CRYPTO_BUFFER variant. This makes the
naming match the header and doesn't require including x509.h. (Though
like ssl.h and pkcs8.h, some of the functions are implemented with code
that depends on crypto/x509.)
Change-Id: I5a7de209f4f775fe0027893f711326d89699ca1f
Reviewed-on: https://boringssl-review.googlesource.com/15128
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=76
Change-Id: I8b754ba17b3e0beee425929e4b53785b2e95f0ae
Reviewed-on: https://boringssl-review.googlesource.com/15164
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I had a brain-fart and had in mind that strings.Index(x[i:], _) would
return a value relative to the beginning of |x|, which is impossible.
Change-Id: I905ea1fa3469ea13f2e3b782c4baf2431b615a2f
Reviewed-on: https://boringssl-review.googlesource.com/15146
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This also fixes TestGetUint to actually test CBS_get_last_u8's behavior.
Right now it can't distinguish CBS_get_last_u8 and CBS_get_u8.
BUG=129
Change-Id: Ie431bb1a828f1c6877938ba7e75c82305b54cf13
Reviewed-on: https://boringssl-review.googlesource.com/15007
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BUG=129
Change-Id: If91d97ea653177d55d5c703f091366ddce24da60
Reviewed-on: https://boringssl-review.googlesource.com/15006
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is not actually sensible, but it seemed really funny. PEM files
sometimes carry private keys so, in principle, we'd probably prefer not
to leak the contents when we encode or decode them?
Change-Id: I7b056612bd7f22c28853bc89f56aee1f5103b8fb
Reviewed-on: https://boringssl-review.googlesource.com/15047
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
FIPS 186-4 prescribes a particular ECDSA nonce selection algorithm,
implemented by BN_range_range_ex. Recast our nonce hardening mechanism
as additional data to be passed into the RBG during that algorithm.
Change-Id: Ic16a10cd58fd7deb7461f0c109a698ea80faff00
Reviewed-on: https://boringssl-review.googlesource.com/15046
Reviewed-by: Adam Langley <agl@google.com>
Rather than comparing against both min and max, FIPS prefers comparing
with max - min and adding min. It also does not believe in using
3*range. Align with it, though our old algorithm trivially produces the
same probability distribution on values.
Change-Id: I447cc3608b92ba93706489d702b8d6a68047f491
Reviewed-on: https://boringssl-review.googlesource.com/15045
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
FIPS requires that the output of the entropy source be checked to ensure
that no two n-bit blocks are equal.
Change-Id: Ia086ca5c888770e0fd71ee052278f77b544b9983
Reviewed-on: https://boringssl-review.googlesource.com/14926
Reviewed-by: Adam Langley <agl@google.com>
We already do this in the case that getrandom is supported. This change
adds a polling loop for the case where we are using /dev/urandom.
This makes FIPS imply Linux, which I think is fine for the time being.
Change-Id: I9bf5c0f51a908621655cbcc47fc86b0366168b97
Reviewed-on: https://boringssl-review.googlesource.com/14925
Reviewed-by: Adam Langley <agl@google.com>
Fork-unsafe buffering was a mode that could be enabled by applications
that were sure that they didn't need to worry about state duplication.
It saved reads to urandom.
Since everything is now going through the CTR-DRBG, we can get the same
effect by simply not reading additional data from urandom in this case.
This change drops the buffering from urandom.c and, instead, implements
fork-unsafe buffering as a mode that skips reading additional data from
urandom, which only happened when RDRAND wasn't available anyway.
Since we expect the power-on self-tests to call into the PRNG, this
change also makes the flag capable of changing at any point by using a
mutex rather than a once. This is split into a separate file so that it
doesn't have to go into the FIPS module—since it uses r/w data that
would be a pain.
Change-Id: I5fd0ead0422e770e35758f080bb1cffa70d0c8da
Reviewed-on: https://boringssl-review.googlesource.com/14924
Reviewed-by: Adam Langley <agl@google.com>
This isn't actually used yet, but implements CTR-DRBG from SP 800-90Ar1.
Specifically, it always uses AES-256 and no derivation function.
Change-Id: Ie82b829590226addd7c165eac410a5d584858bfd
Reviewed-on: https://boringssl-review.googlesource.com/14891
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I73213b5d9f3ac67bab70e3d9a36a4b67c558f3f5
Reviewed-on: https://boringssl-review.googlesource.com/15044
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Otherwise the order changes each time, which will make the build
egregiously non-deterministic.
Change-Id: Idd501ecd118c61a27566eafc61157715e48758bc
Reviewed-on: https://boringssl-review.googlesource.com/15026
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
References to global symbols generate relocations, which breaks the
integrity check.
Change-Id: If6fa06d5d924294ab496c32e7f082a1ae60fdb24
Reviewed-on: https://boringssl-review.googlesource.com/15025
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Some assembly code references “OPENSSL_ia32cap_P+4(%rip)” etc, which
slipped by the previous check.
Change-Id: I22c3fbf9883aea695e8584857bf9c0e3113f9a77
Reviewed-on: https://boringssl-review.googlesource.com/15024
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Since only the consumers knows whether an EC key will be used for
ECDSA or ECDHE, it is part of the FIPS policy for the consumer to
check the validity of the generated key before signing with it.
Change-Id: Ie250f655c8fcb6a59cc7210def1e87eb958e9349
Reviewed-on: https://boringssl-review.googlesource.com/14745
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This moves the kinv computation next to k generation and adds a check for group
size as per 186-4 B.5.2.
Change-Id: I8744080d3961bc9e29054985280fc835e3f1e25c
Reviewed-on: https://boringssl-review.googlesource.com/14944
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It's not obvious how to make ASAN happy with the integrity test but this
will let us test FIPS-only code with ASAN at least.
Change-Id: Iac983787e04cb86a158e4416c410d9b2d1e5e03f
Reviewed-on: https://boringssl-review.googlesource.com/14965
Reviewed-by: Adam Langley <agl@google.com>
FIPS is not compatible with multiprime RSA. Any multiprime RSA private
keys will fail to parse after this change.
Change-Id: I8d969d668bf0be4f66c66a30e56f0e7f6795f3e9
Reviewed-on: https://boringssl-review.googlesource.com/14984
Reviewed-by: Adam Langley <agl@google.com>
FIPS prescribes a slightly different key generation algorithm than we
use. Specifically:
- Rather than using BN_RAND_TOP_TWO (so using 1.5 as an upper bound for
sqrt(2)), it prescribes using sqrt(2) itself. To avoid unnecessary
squaring, we do a comparison against a hard-coded approximation for
sqrt(2) good enough for the largest FIPS key size. I went ahead and
made it constant-time since it was easy, but all this is far from
constant-time.
- FIPS requires a check that |p-q| is sufficiently large.
- FIPS requires a check that d is sufficiently large.
- BN_generate_prime_ex adds some delta to clear a table of prime
numbers. FIPS does not specify any of that, so implement a separate
routine here.
The primality test itself will be aligned in a follow-up. For now, it is
left unchanged, except that trial division is turned back on. That makes
things faster and is analogous the original algorithm's delta-munging
logic.
Change-Id: If32f0635bfb67a8c4740dedd7781d00647bbf60b
Reviewed-on: https://boringssl-review.googlesource.com/14948
Reviewed-by: Adam Langley <agl@google.com>
Previously, inject-hash would run the FIPS module in order to trigger a
failure and then extract the calculated hash value from the output. This
makes cross-compiling difficult because the build process needs to run a
binary for the target platform.
This change drops this step. Instead, inject-hash.go parses the object
file itself and calculates the hash without needing to run the module.
Change-Id: I2593daa03094b0a17b498c2e8be6915370669596
Reviewed-on: https://boringssl-review.googlesource.com/14964
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The FIPS RSA generation algorithm is unkind to keys with funny bit
sizes. Odd numbers of bits are especially inconvenient, but the sqrt(2)
bound is much simpler if the key size is a multiple of 128 (thus giving
prime sizes a multiple of 64, so the sqrt(2) bound is easier to work
with).
Also impose a minimum RSA key size. 255-bit RSA is far too small as it
is and gives small enough primes that the p-q FIPS bound (2^(n/2-100))
starts risking underflow.
Change-Id: I4583c90b67385e53641ccee9b29044e79e94c920
Reviewed-on: https://boringssl-review.googlesource.com/14947
Reviewed-by: Adam Langley <agl@google.com>
This is just some idle cleanup. The padding functions already must
handle size checks. Swap out the error code in the low-level portions to
keep that unchanged.
Also remove an old TODO(fork) about constant-time-ness. Signature
verification padding checks don't need to be constant time, and
decryption ones should be resolved now.
Change-Id: I20e7affdb7f2dce167a304afe707bfd537dd412a
Reviewed-on: https://boringssl-review.googlesource.com/14946
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I9f7f1dd609c38d1f4be536daff94a4ba002582d0
Reviewed-on: https://boringssl-review.googlesource.com/14888
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Later CLs will unwind the rest of multiprime RSA support. Start with key
generation.
Change-Id: Id20473fd55cf32c27ea4a57f2d2ea11daaffedeb
Reviewed-on: https://boringssl-review.googlesource.com/14870
Reviewed-by: Adam Langley <agl@google.com>
That file was getting too huge and we only need to de-static a single
function to do it.
Change-Id: Ie2c0bc90a7e538a74318c364a136c337ce8d9bbb
Reviewed-on: https://boringssl-review.googlesource.com/14884
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
As a precursor to removing the code entirely later, disable the protocol
by default. Callers must use SSL_CTX_set_min_version to enable it.
This change also makes SSLv3_method *not* enable SSL 3.0. Normally
version-specific methods set the minimum and maximum version to their
version. SSLv3_method leaves the minimum at the default, so we will
treat it as all versions disabled. To help debugging, the error code is
switched from WRONG_SSL_VERSION to a new NO_SUPPORTED_VERSIONS_ENABLED.
This also defines OPENSSL_NO_SSL3 and OPENSSL_NO_SSL3_METHOD to kick in
any no-ssl3 build paths in consumers which should provide a convenient
hook for any upstreaming changes that may be needed. (OPENSSL_NO_SSL3
existed in older versions of OpenSSL, so in principle one may encounter
an OpenSSL with the same settings.)
Change-Id: I96a8f2f568eb77b2537b3a774b2f7108bd67dd0c
Reviewed-on: https://boringssl-review.googlesource.com/14031
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I92419b7d2d8ded8f4868588ad3c24b70ac7f7b1b
Reviewed-on: https://boringssl-review.googlesource.com/14864
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I3134b2ed1b2000bf2413c066c6560832c0ff03ae
Reviewed-on: https://boringssl-review.googlesource.com/14704
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The built-in CMake support seems to basically work, though it believes
you want to build a fat binary which doesn't work with how we build
perlasm. (We'd need to stop conditioning on CMAKE_SYSTEM_PROCESSOR at
all, wrap all the generated assembly files in ifdefs, and convince the
build to emit more than one. Probably not worth bothering for now.)
We still, of course, need to actually test the assembly on iOS before
this can be shipped anywhere.
BUG=48
Change-Id: I6ae71d98d706be03142b82f7844d1c9b02a2b832
Reviewed-on: https://boringssl-review.googlesource.com/14645
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These two functions behave identically if the input is a word, which is
true if bits <= BN_BITS2. This also matches upstream's version of the
function. I'm guessing the patch was originally submitted as we have it,
perhaps because we didn't notice BN_get_word at the time, and it got
switched to the existing BN_get_word function in review.
Change-Id: I7847e3086aab871c5aa28e15fae6f89c964862d1
Reviewed-on: https://boringssl-review.googlesource.com/14331
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
