Support for platforms that we don't support FIPS on doesn't need to be
in the module. Also, functions for dealing with whether fork-unsafe
buffering is enabled are left out because they aren't implementing any
cryptography and they use global r/w state, making their inclusion
painful.
Change-Id: I71a0123db6f5449e9dfc7ec7dea0944428e661aa
Reviewed-on: https://boringssl-review.googlesource.com/15084
Reviewed-by: Adam Langley <agl@google.com>
With some optimisation settings, Clang was loading
BORINGSSL_bcm_text_hash with AVX2 instructions, which weren't getting
translated correctly. This seems to work and is less fragile.
The compiler just emits an leaq here. This is because it knows the
symbol is hidden (in the shared library sense), so it needn't go through
GOTPCREL. The assembler would have added a relocation, were the symbol
left undefined, but since we define the symbol later on, it all works
out without a relocation.
Were the symbol not hidden, the compiler would have emitted a movq by
way of GOTPCREL, but we can now translate those away anyway.
Change-Id: I442a22f4f8afaadaacbab7044f946a963ebfc46c
Reviewed-on: https://boringssl-review.googlesource.com/15384
Reviewed-by: Adam Langley <agl@google.com>
We BN_cmp with 1 at the top, so the absolute value code never runs.
This simplifies the BN_CTX business considerably. Also add a test for
negative prime numbers.
Change-Id: I500a56bc285c2f75576947cfb518e75c9e6861ce
Reviewed-on: https://boringssl-review.googlesource.com/15367
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>
Thanks to Rob Sloan for clearing out Android's uses of these functions.
I forgot we can hide these now.
BUG=97
Change-Id: I9bc7bf5ca379d3345743151e606f3e911367b4ed
Reviewed-on: https://boringssl-review.googlesource.com/15364
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Robert Sloan <varomodt@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: Ibd6b9b12b3b622f67f69da5c2add8b1b040882f1
Reviewed-on: https://boringssl-review.googlesource.com/15344
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 changes to delocate.go are needed because modes/ does things like
return the address of a module function. Both of these need to be
changed from referencing the GOT to using local symbols.
Rather than testing whether |ghash| is |gcm_ghash_avx|, we can just keep
that information in a flag.
The test for |aesni_ctr32_encrypt_blocks| is more problematic, but I
believe that it's superfluous and can be dropped: if you passed in a
stream function that was semantically different from
|aesni_ctr32_encrypt_blocks| you would already have a bug because
|CRYPTO_gcm128_[en|de]crypt_ctr32| will handle a block at the end
themselves, and assume a big-endian, 32-bit counter anyway.
Change-Id: I68a84ebdab6c6006e11e9467e3362d7585461385
Reviewed-on: https://boringssl-review.googlesource.com/15064
Reviewed-by: Adam Langley <agl@google.com>
SP 800-89 5.3.3 references FIPS 186 for the bounds on e. I /think/
that's section B.3.1 which says:
(b) The exponent e shall be an odd positive integer such that 2¹⁶ < e < 2²⁵⁶.
But that means that e has to be at least 17 bits. The check for
BN_is_odd ensures that 2¹⁶ itself is rejected.
Change-Id: Ib39f9d43032cbfe33317651c7b6eceb41b123291
Reviewed-on: https://boringssl-review.googlesource.com/15324
Reviewed-by: Steven Valdez <svaldez@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>
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>
Windows doesn't like uninitialized function-level static consts and
Android complains we're casting away a volatile.
Change-Id: I7c53de45cff9fa2ef298f015cf3f5ecca82194d0
Reviewed-on: https://boringssl-review.googlesource.com/14807
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This restores the original version of delocate.go, with the subsequent
bugfixes patched in. With this, the FIPS module builds with GCC and
Clang, with and without optimizations. I did patch over a variant of the
macro though, since it was otherwise really wordy.
Playing games with sections was a little overly clever and relied on the
compiler not performing a number of optimizations. Clang blew threw all
of those assumptions.
Change-Id: Ib4da468a5925998457994f9e392cf0c04573fe91
Reviewed-on: https://boringssl-review.googlesource.com/14805
Reviewed-by: Adam Langley <agl@google.com>
This fixes two issues in clang.
- clang emits callq instead of call.
- clang emits .cfi_endproc after .size for the dummy functions. This
causes it to get confused as there is no matching .cfi_startproc.
Don't bother trying to omit the dummy functions.
Alas, clang seems to compile the DEFINE_METHOD_FUNCTION hooks in a way
that brings the .rel.ro back AND isn't honoring the noinline. We'll
probably need to go back to the original CL's setup there.
Change-Id: Ic21ea99e54a93cdc739e4f67dc308d83083607d6
Reviewed-on: https://boringssl-review.googlesource.com/14804
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
In typical style I forgot to push a new revision before
landing fd49993c3b. That change accidently
dropped patchset eight when I squashed David's changes in, so this
restores that and fixes a couple of 80-char issues in a Python script.
Change-Id: I7e9338a715c68ae5c89d9d1f7d03782b99af2aa8
Reviewed-on: https://boringssl-review.googlesource.com/14784
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>
This is the only single-shot hash function which pretends it has a
failure case.
Change-Id: Ibf45e197eafc63c368be3783dfeec8ccb95589ab
Reviewed-on: https://boringssl-review.googlesource.com/14584
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BUG=187
Change-Id: I5775ce0886041b0c12174a7d665f3af1e8bce511
Reviewed-on: https://boringssl-review.googlesource.com/14505
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
It's amazing how short p_ed25519.c is.
BUG=187
Change-Id: Ib2a5fa7a4acf2087ece954506f81e91a1ed483e1
Reviewed-on: https://boringssl-review.googlesource.com/14449
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The resulting EVP_PKEYs do not do anything useful yet, but we are able
to parse them. Teaching them to sign will be done in a follow-up.
Creating these from in-memory keys is also slightly different from other
types. We don't have or need a public ED25519_KEY struct in
curve25519.h, so I've added tighter constructor functions which should
hopefully be easier to use anyway.
BUG=187
Change-Id: I0bbeea37350d4fdca05b6c6c0f152c15e6ade5bb
Reviewed-on: https://boringssl-review.googlesource.com/14446
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Right now this is just a wrapper over EVP_Digest and EVP_PKEY_sign. A
later change will introduce a sign_message hook to EVP_PKEY_METHOD which
Ed25519 and other single-shot-only algorithms can implement.
(EVP_PKEY_sign does not quite work for this purpose as all the other key
types believe EVP_PKEY_sign acts on a pre-hashed input.)
BUG=187
Change-Id: Ia4bbf61b25cc4a0d64bcb4364805fe9b5a6e829c
Reviewed-on: https://boringssl-review.googlesource.com/14447
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
On malloc error, CRYPTO_set_ex_data may fail. (See upstream's
62f488d31733e5dc77b339f905b44f165550e47d.)
It also failed to copy the reserved slots when we revised the app-data
machinery, although this is unreachable as EC_KEY is the only thing
which uses this function and EC_KEY has no reserved slots. (We probably
can/should also take CRYPTO_dup_ex_data out of there, as it's a little
bit weird...)
Change-Id: I60bbc301f919d4c0ee7fff362f979f6ec18d73b7
Reviewed-on: https://boringssl-review.googlesource.com/14604
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
(Thanks to Sam Panzer for the patch.)
At least some linkers will drop constructor functions if no symbols from
that translation unit are used elsewhere in the program. On POWER, since
the cached capability value isn't a global in crypto.o (like other
platforms), the constructor function is getting discarded.
The C++11 spec says (3.6.2, paragraph 4):
It is implementation-defined whether the dynamic initialization of a
non-local variable with static storage duration is done before the
first statement of main. If the initialization is deferred to some
point in time after the first statement of main, it shall occur
before the first odr-use (3.2) of any function or variable defined
in the same translation unit as the variable to be initialized.
Compilers appear to interpret that to mean they are allowed to drop
(i.e. indefinitely defer) constructors that occur in translation units
that are never used, so they can avoid initializing some part of a
library if it's dropped on the floor.
This change makes the hardware capability value for POWER a global in
crypto.c, which should prevent the constructor function from being
ignored.
Change-Id: I43ebe492d0ac1491f6f6c2097971a277f923dd3e
Reviewed-on: https://boringssl-review.googlesource.com/14664
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>
This was a mess. HMAC_CTX_copy_ex would avoid having to cleanup and init
the HMAC_CTX repeatedly, but even that is unnecessary. hctx_tpl was just
to reuse the key. Instead, HMAC_CTX already can be reset with the same
key. (Alas, with a slightly odd API, but so it goes.) Do that, and use
goto err to cleanup the error-handling.
Thanks to upstream's b98530d6e09f4cb34c791b8840e936c1fc1467cf for
drawing attention to this. (Though we've diverged significantly from
upstream with all the heap-allocated bits, so I didn't use the change
itself.)
While I'm here, tidy up some variable names and cite the newer RFC.
Change-Id: Ic1259f46b7c5a14dc341b8cee385be5508ac4daf
Reviewed-on: https://boringssl-review.googlesource.com/14605
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>
These static output buffers are a legacy from a time before processes
had threads. This change drops support and callers who were depending on
this (of which there are hopefully none) will crash.
Change-Id: I7b8eb3440def507f92543e55465f821dfa02c7da
Reviewed-on: https://boringssl-review.googlesource.com/14528
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>
Change-Id: I32b37306265e89afca568f20bfba2e04559c4f0b
Reviewed-on: https://boringssl-review.googlesource.com/14527
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>
and relying on a compiler to generate code for unaligned access. Both gcc
and llvm currently do that but llvm is going to change to generate code for
aligned access. The change in llvm will break SHA-1 on POWER without this fix.
Change-Id: If9393968288cf94b684ad340e3ea295e03174aa9
Reviewed-on: https://boringssl-review.googlesource.com/14378
Reviewed-by: Adam Langley <agl@google.com>
There are a few test vectors which were not imported from djb's. Mirror
those. Also as RFC 8032 uses a slightly different private key
representation, document this in curve25519.h.
BUG=187
Change-Id: I119381168ba1af9b332365fd8f974fba41759d57
Reviewed-on: https://boringssl-review.googlesource.com/14445
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is a remnant of a previous iteration of the SSL client certificate
bridging logic in Chromium.
Change-Id: Ifa8e15cc970395f179e2f6db65c97a342af5498d
Reviewed-on: https://boringssl-review.googlesource.com/14444
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is slightly tidier than casting through function pointers. (Also
more defined? But we cast T* => void* within a function pointer all over
the place, so that's probably a lost cause.)
Change-Id: I8f435906f3066d1377eababf940e3db34c626acd
Reviewed-on: https://boringssl-review.googlesource.com/14313
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We only need the size_t ones now.
BUG=22
Change-Id: Ie6935656bbc4bd2b602b8fad78effc401c493416
Reviewed-on: https://boringssl-review.googlesource.com/14312
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Not that this is remotely necessary since the code bounds to 1MB, the
caller bounds to INT_MAX (due to EVP_CIPHER) and the grandcaller bounds
to 16k (due to TLS).
BUG=22
Change-Id: Ia75990a30bac26ca617532630340ff94a88e4e20
Reviewed-on: https://boringssl-review.googlesource.com/14311
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is redundant because these "AEAD"s are not meant to be used outside
of TLS, but since we've moved them into their own layer, they should
check internally.
Change-Id: Ieb3541b2e494902527c2bb56a816cef620cb237b
Reviewed-on: https://boringssl-review.googlesource.com/14310
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This makes it a bit easier to see what is what.
Change-Id: I0f73f6ffa84bd30de3efcbf2bd34e1d3a889d1ee
Reviewed-on: https://boringssl-review.googlesource.com/14309
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BUG=22
Change-Id: I9f392eef44e83efb4b13931acb2a3c642cbf1f29
Reviewed-on: https://boringssl-review.googlesource.com/14308
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BUG=22
Change-Id: I5bfa543c261623d125e7a25cea905e3b90b0c014
Reviewed-on: https://boringssl-review.googlesource.com/14307
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
These will be used in follow-up commits. The _s names are taken from
upstream, to ease importing code. I've also promoted the CONSTTIME_*
macros from the test. None of them are really necessary except
~0u cannot substitute for CONSTTIME_TRUE_S on 64-bit platforms, so
having the macros seems safer.
Once everything is converted, I expect the unsigned versions can be
removed, so I've made the _8 and _int functions act on size_t rather
than unsigned. The users of these functions basically only believe that
array indices and bytes exist.
BUG=22
Change-Id: I987bfb0c708dc726a6f2afcb05b6619bbd600564
Reviewed-on: https://boringssl-review.googlesource.com/14306
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This pulls in upstream's 0822d41b6d54132df96c02cc6f6fa9b179378351 and a
portion of a285992763f3961f69a8d86bf7dfff020a08cef9. The former, in
particular, fixes a crash on iOS.
Change-Id: I3c083975d8d11e58b5a2919fcabbf83628f36340
Reviewed-on: https://boringssl-review.googlesource.com/14383
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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 ends up under half the size of the original file.
BUG=129
Change-Id: Idec69d9517bd57cee6b3b83bc0cce05396565b70
Reviewed-on: https://boringssl-review.googlesource.com/14305
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>
OPENSSL_free will handle NULL.
Change-Id: I18593a015cd4a081c2eeebf0cd738a024d02a97d
Reviewed-on: https://boringssl-review.googlesource.com/14373
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It still depends on crypto/x509, but we will need a CRYPTO_BUFFER
version of PKCS7_get_certificates for Chromium. Start with this.
BUG=54
Change-Id: I62dcb9ba768091ce37dc9fe819f4f14ac025219c
Reviewed-on: https://boringssl-review.googlesource.com/14372
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Channel ID is incompatible with 0-RTT, so we gracefully decline 0-RTT
as a server and forbid their combination as a client. We'll keep this
logic around until Channel ID is removed.
Channel ID will be replaced by tokbind which currently uses custom
extensions. Those will need additional logic to work with 0-RTT.
This is not implemented yet so, for now, fail if both are ever
configured together at all. A later change will allow the two to
combine.
BUG=183
Change-Id: I46c5ba883ccd47930349691fb08074a1fab13d5f
Reviewed-on: https://boringssl-review.googlesource.com/14370
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>
These will be used by Chromium's crypto::ECPrivateKey to work with
EncryptedPrivateKeyInfo structures.
Note this comes with a behavior change: PKCS8_encrypt and PKCS8_decrypt
will no longer preserve PKCS#8 PrivateKeyInfo attributes. However, those
functions are only called by Chromium which does not care. They are also
called by the PEM code, but not in a way which exposes attributes.
The PKCS#12 PFX code is made to use PKCS8_parse_encrypted_private_key
because it's cleaner (no more tossing X509_SIG around) and to ease
decoupling that in the future.
crypto/pkcs8's dependency on the legacy ASN.1 stack is now limited to
pkcs8_x509.c.
BUG=54
Change-Id: I173e605d175e982c6b0250dd22187b73aca15b1a
Reviewed-on: https://boringssl-review.googlesource.com/14215
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>
PKCS8_encrypt and PKCS8_decrypt still need to be split. The code for
processing PKCS#12 files is, for now, placed entirely in pkcs8_x509.c.
If we need to split it up, it should be straightforward to do so.
(Introduce a CRYPTO_BUFFER version of PKCS12_get_key_and_certs and go
from there.)
BUG=54
Change-Id: I9c87e916ec29ee14dbbd81c4d3fc10ac8a461f1a
Reviewed-on: https://boringssl-review.googlesource.com/14214
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The padding check functions will need to tweak their calling conventions
and the constant-time helpers, so leaving those alone for now. These
were the easy ones.
BUG=22
Change-Id: Ia00e41e26a134de17d56be3def5820cb042794e1
Reviewed-on: https://boringssl-review.googlesource.com/14265
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>
With PKCS8_encrypt_pbe and PKCS8_decrypt_pbe gone in
3e8b782c0c, we can restore the old
arrangement where the password encoding was handled in pkcs12_key_gen.
This simplifies the interface for the follow-up crypto/asn1 split.
Note this change is *not* a no-op for PKCS#12 files which use PBES2.
Before, we would perform the PKCS#12 password encoding for all parts of
PKCS#12 processing. The new behavior is we only perform it for the parts
that go through the PKCS#12 KDF. For such a file, it would only be the
MAC.
I believe the specification supports our new behavior. Although RFC 7292
B.1 says something which implies that the transformation is about
converting passwords to byte strings and would thus be universal,
appendix B itself is prefaced with:
Note that this method for password privacy mode is not recommended
and is deprecated for new usage. The procedures and algorithms
defined in PKCS #5 v2.1 [13] [22] should be used instead.
Specifically, PBES2 should be used as encryption scheme, with PBKDF2
as the key derivation function.
"This method" refers to the key derivation and not the password
formatting, but it does give support to the theory that password
formatting is tied to PKCS#12 key derivation.
(Of course, if one believes PKCS#12's assertion that their inane
encoding (NUL-terminated UTF-16!) is because PKCS#5 failed to talk about
passwords as Unicode strings, one would think that PBES2 (also in
PKCS#5) would have the same issue and thus need PKCS#12 to valiantly
save the day with an encoding...)
This matches OpenSSL's behavior and that of recent versions of NSS. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1268141. I was unable to
figure out what variants, if any, macOS accepts.
BUG=54
Change-Id: I9a1bb4d5e168e6e76b82241e4634b1103e620b9b
Reviewed-on: https://boringssl-review.googlesource.com/14213
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 isn't strictly necessary for Chromium yet, but we already have a
decoupled version of hash algorithm parsing available. For now, don't
export it but eventually we may wish to use it for OCSP.
BUG=54
Change-Id: If460d38d48bd47a2b4a853779f210c0cf7ee236b
Reviewed-on: https://boringssl-review.googlesource.com/14211
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This adds support on the server and client to accept data-less early
data. The server will still fail to parse early data with any
contents, so this should remain disabled.
BUG=76
Change-Id: Id85d192d8e0360b8de4b6971511b5e8a0e8012f7
Reviewed-on: https://boringssl-review.googlesource.com/12921
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 OPTIONAL prf field is an AlgorithmIdentifier, not an OID. I messed
this up in the recent rewrite.
Fix the parsing and add a test, produced by commenting out the logic in
OpenSSL to omit the field for hmacWithSHA1. (We don't currently support
any other PBKDF2, or I'd just add a test for that.)
Change-Id: I7d258bb01b93cd203a6fc1b8cccbddfdbc4dbbad
Reviewed-on: https://boringssl-review.googlesource.com/14330
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 isn't something we need to fix, just an explanatory comment.
Change-Id: I284e6580d176f981c6b161e9951f367fef1b1be6
Reviewed-on: https://boringssl-review.googlesource.com/14264
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>
