BUG=129
Change-Id: Ia8b0639489fea817be4bb24f0457629f0fd6a815
Reviewed-on: https://boringssl-review.googlesource.com/16947
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: I4e0da85857e820f8151e2fb50d699f14fedee97b
Reviewed-on: https://boringssl-review.googlesource.com/16966
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: I2e7b9e80419758a5ee4f53915f13334bbf8e0447
Reviewed-on: https://boringssl-review.googlesource.com/16965
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: Ic22ea72b0134aa7884f1e75433dd5c18247f57ab
Reviewed-on: https://boringssl-review.googlesource.com/16964
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>
When building with OPENSSL_NO_ASM do not try to enable_language(ASM).
Even though the assembly source isn't being built this still causes
CMake to look for the assembler which will fail on platforms where one
is not available.
Change-Id: Ie4893f606143e8f8ca0807114068e577dc1e23e9
Reviewed-on: https://boringssl-review.googlesource.com/16904
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>
Drop some redundant instructions in reduction in ecp_nistz256_sqr_montx.
(Imported from upstream's 8fc063dcc9668589fd95533d25932396d60987f9.)
I believe this is a no-op for us as we do not currently enable the
ADX-based optimizations.
Change-Id: I34a5f5ffb965d59c67f6b9f0ca7937e49ba6e820
Reviewed-on: https://boringssl-review.googlesource.com/16884
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>
BUG=129
Change-Id: I1fef45d662743e7210f93e4dc1bae0c55f75d3fe
Reviewed-on: https://boringssl-review.googlesource.com/16864
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: Ie88363c4f02016ee743b37a79e76432823b948a0
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/16844
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>
In order to use AES-GCM-SIV in the open-source QUIC boxer, it needs to
be moved out from OPENSSL_SMALL. (Hopefully the linker can still discard
it in the vast majority of cases.)
Additionally, the input to the key schedule function comes from outside
and may not be aligned, thus we need to use unaligned instructions to
read it.
Change-Id: I02c261fe0663d13a96c428174943c7e5ac8415a7
Reviewed-on: https://boringssl-review.googlesource.com/16824
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>
Without this, trying to trigger the CRNGT on a system with RDRAND won't
work.
Change-Id: I0658a1f045620a2800df36277f67305bc0efff8b
Reviewed-on: https://boringssl-review.googlesource.com/16766
Reviewed-by: Adam Langley <agl@google.com>
We want to clarify that this isn't the PWCT that FIPS generally means,
but rather the power-on self-test. Since ECDSA is non-deterministic, we
have to implement that power-on self-test as a PWCT, but we have a
different flag to break that actual PWCT.
Change-Id: I3e27c6a6b0483a6c04e764d6af8a4a863e0b8b77
Reviewed-on: https://boringssl-review.googlesource.com/16765
Reviewed-by: Adam Langley <agl@google.com>
FIPS requires that the CTR-DRBG state be zeroed on process exit, however
destructors for thread-local data aren't called when the process exits.
This change maintains a linked-list of thread-local state which is
walked on exit to zero each thread's PRNG state. Any concurrently
running threads block until the process finishes exiting.
Change-Id: Ie5dc18e1bb2941a569d8b309411cf20c9bdf52ef
Reviewed-on: https://boringssl-review.googlesource.com/16764
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>
Comments in CAVP are semantically important and we need to copy them
from the input to the output.
Change-Id: Ib798c4ad79de924487d0c4a0f8fc16b757e766d8
Reviewed-on: https://boringssl-review.googlesource.com/16725
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>
Most importantly, this version of delocate works for ppc64le. It should
also work for x86-64, but will need significant testing to make sure
that it covers all the cases that the previous delocate.go covered.
It's less stringtastic than the old code, however the parser isn't as
nice as I would have liked. I thought that the reason we put up with
AT&T syntax with Intel is so that assembly syntax could be somewhat
consistent across platforms. At least for ppc64le, that does not appear
to be the case.
Change-Id: Ic7e3c6acc3803d19f2c3ff5620c5e39703d74212
Reviewed-on: https://boringssl-review.googlesource.com/16464
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>
Change-Id: I1a17860245b7726a24576f5e1bddb0645171f28e
Reviewed-on: https://boringssl-review.googlesource.com/16486
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 symbol “rcon” should be local in order to avoid collisions and it's
much easier on delocate if some of the expressions are evalulated in
Perl rather than left in the resulting .S file.
Also fix the perlasm style so the symbols are actually local.
Change-Id: Iddfc661fc3a6504bcc5732abaa1174da89ad805e
Reviewed-on: https://boringssl-review.googlesource.com/16524
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 change replace the cmovq scheme with slightly faster SSE2 code.
The SSE2 code was first introduced in Go's curve25519 implementation.
See: https://go-review.googlesource.com/c/39693/
The implementation is basicly copied from the Go assembly.
Change-Id: I25931a421ba141ce33809875699f048b0941c061
Reviewed-on: https://boringssl-review.googlesource.com/16564
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>
The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).
In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)
Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.
(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)
BUG=21
Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344
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>
It shouldn't have been defined for variable-length nonces at all, but so
it goes. EVP_CIPHER rejected this by way of EVP_CTRL_GCM_SET_IVLEN
comparing <= 0, but the EVP_AEAD API did not.
I've done the test in a separate file on the assumption that aead_test
will become GTest shortly, at which point it will be easy to stick extra
tests into the same file as the FileTest ones.
Thanks to Daniel Bleichenbacher and Thanh Bui of Project Wycheproof for
the report.
Change-Id: Ic4616b39a1d7fe74a1f14fb58cccec2ce7c4f2f3
Reviewed-on: https://boringssl-review.googlesource.com/16544
Reviewed-by: Adam Langley <agl@google.com>
This introduces machinery to start embedding the test data files into
the crypto_test binary. Figuring out every CI's test data story is more
trouble than is worth it. The GTest FileTest runner is considerably
different from the old one:
- It returns void and expects failures to use the GTest EXPECT_* and
ASSERT_* macros, rather than ExpectBytesEqual. This is more monkey
work to convert, but ultimately less work to add new tests. I think
it's also valuable for our FileTest and normal test patterns to align
as much as possible. The line number is emitted via SCOPED_TRACE.
- I've intentionally omitted the Error attribute handling, since that
doesn't work very well with the new callback. This means evp_test.cc
will take a little more work to convert, but this is again to keep our
two test patterns aligned.
- The callback takes a std::function rather than a C-style void pointer.
This means we can go nuts with lambdas. It also places the path first
so clang-format doesn't go nuts.
BUG=129
Change-Id: I0d1920a342b00e64043e3ea05f5f5af57bfe77b3
Reviewed-on: https://boringssl-review.googlesource.com/16507
Reviewed-by: Adam Langley <agl@google.com>
In GTest, we'll just burn the files into the binary and not worry about
this. Apparently test files is a one of computer science's great
unsolved problems and everyone has their own special-snowflake way of
doing it. Burning them into the executable is easier.
BUG=129
Change-Id: Ib39759ed4dba6eb9ba97f0282f000739ddf931fe
Reviewed-on: https://boringssl-review.googlesource.com/16506
Reviewed-by: Adam Langley <agl@google.com>
Instead of a script which generates macros, emit static inlines in
individual header (or C files). This solves a few issues with the
original setup:
- The documentation was off. We match the documentation now.
- The stack macros did not check constness; see some of the fixes in
crypto/x509.
- Type errors did not look like usual type errors.
- Any type which participated in STACK_OF had to be made partially
public. This allows stack types to be defined an internal header or
even an individual file.
- One could not pass sk_FOO_free into something which expects a function
pointer.
Thanks to upstream's 411abf2dd37974a5baa54859c1abcd287b3c1181 for the
idea.
Change-Id: Ie5431390ccad761c17596b0e93941b0d7a68f904
Reviewed-on: https://boringssl-review.googlesource.com/16087
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I7bf485a9bfe0d7b7a3dc3081f86278fee87b8c74
Reviewed-on: https://boringssl-review.googlesource.com/16485
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: Iab7a738a8981de7c56d1585050e78699cb876dab
Reviewed-on: https://boringssl-review.googlesource.com/16467
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 change causes FIPS mode to use RDRAND in preference to the kernel's
entropy pool. This prevents issues where the ioctl that we have to do
when getrandom isn't supported transiently reports that the pool is
“empty” and causes us to block.
Change-Id: Iad50e443d88b168bf0b85fe1e91e153d79ab3703
Reviewed-on: https://boringssl-review.googlesource.com/16466
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>
Rather than comparing against both endpoints, subtract the minimum and
rely on unsigned wraparound to do both comparisons at once. This seems
to be slightly faster.
In addition, constant_time_lt_8 becomes much simpler if it can assume
that |a| and |b| have the same MSB. But we can arrange that by casting
up to |crypto_word_t| (which is otherwise happening anyway).
Change-Id: I82bd676e487eb7bb079ba7286df724c1c380bbb4
Reviewed-on: https://boringssl-review.googlesource.com/16445
Reviewed-by: Adam Langley <agl@google.com>
With the constant-time base64 decode, base64_ascii_to_bin is a bit more
expensive. This check is redundant with the one in base64_decode_quad,
though it does mean syntax error reporting will be slightly deferred by
four bytes.
Change-Id: I71f23ea23feba2ee5b41df79ce09026fb56996d3
Reviewed-on: https://boringssl-review.googlesource.com/16444
Reviewed-by: Adam Langley <agl@google.com>
Saves having it in several places.
Change-Id: I329e1bf4dd4a7f51396e36e2604280fcca32b58c
Reviewed-on: https://boringssl-review.googlesource.com/16026
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>
AES-GCM-SIV specifies that the counter is a 32-bit, unsigned number.
These test vectors are crafted to trigger a wrap-around and ensure that
corner of the spec is implemented correctly.
Change-Id: I911482ca0b6465a7623ee1b74a6cb1d5e54ddbea
Reviewed-on: https://boringssl-review.googlesource.com/16324
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 makes things a little easier for some of our tooling.
Change-Id: Ia7e73daf0a5150b106cf9b03b10cae194cb8fc5a
Reviewed-on: https://boringssl-review.googlesource.com/15104
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
43e5a26b53 removed the .file directive
from x86asm.pl. This removes the parameter from asm_init altogether. See
also upstream's e195c8a2562baef0fdcae330556ed60b1e922b0e.
Change-Id: I65761bc962d09f9210661a38ecf6df23eae8743d
Reviewed-on: https://boringssl-review.googlesource.com/16247
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>
We're not using the MASM output, so don't bother maintaining a diff on
it.
Change-Id: I7321e58c8b267be91d58849927139b74cc96eddc
Reviewed-on: https://boringssl-review.googlesource.com/16246
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>
Due to issues with CMake enable_language, we have to delay setting
CMAKE_ASM_FLAGS until after enable_language(ASM) has been called.
We also need to remove the '.file' macro from x86gas.pl to prevent the
filenames from being overridden from those provided by the build
system.
Change-Id: I436f57ec45e4751714af49e1211a0d7810e4e56a
Reviewed-on: https://boringssl-review.googlesource.com/16127
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 allows breaking Known Answer Tests for AES-GCM, DES, SHA-1,
SHA-256, SHA-512, RSA signing and DRBG as required by FIPS.
Change-Id: I8e59698a5048656021f296195229a09ca5cd767c
Reviewed-on: https://boringssl-review.googlesource.com/16088
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>
It's about time we got rid of this. As a first step, introduce a flag,
so that some consumers may stage this change in appropriately.
BUG=chromium:534766,chromium:532048
Change-Id: Id53f0bacf5bdbf85dd71d1262d9f3a9ce3c4111f
Reviewed-on: https://boringssl-review.googlesource.com/16104
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This has since been done.
Change-Id: I498f845fa4ba3d1c04a5892831be4b07f31536d4
Reviewed-on: https://boringssl-review.googlesource.com/16124
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>
This is needed when unrandom.c is compiled on its own.
Change-Id: Ia46e06d267c097e5fa0296092a7270a4cd0b2044
Reviewed-on: https://boringssl-review.googlesource.com/16085
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>
This is required by FIPS testing.
Change-Id: Ia399a0bf3d03182499c0565278a3713cebe771e3
Reviewed-on: https://boringssl-review.googlesource.com/16044
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>
SHA-512 is faster to calculate on 64-bit systems and that's what we were
using before. (Though, realistically, this doesn't show up at all.)
Change-Id: Id4f386ca0b5645a863b36405eef03bc62d0f29b3
Reviewed-on: https://boringssl-review.googlesource.com/16006
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>
SHA-512 is faster to calculate on 64-bit systems and we're only
targetting 64-bit systems with FIPS.
Change-Id: I5e9b8419ad4ddc72ec682c4193ffb17975d228e5
Reviewed-on: https://boringssl-review.googlesource.com/16025
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>
ASAN prevents the integrity test from running, so don't indicate FIPS
mode in that case.
Change-Id: I14c79e733e53ef16f164132bc1fded871ce3f133
Reviewed-on: https://boringssl-review.googlesource.com/16024
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>
Perl, multiple versions, for some reason occasionally takes issue with
letter b[?] in ox([0-9a-f]+) regex. As result some constants, such as
0xb1 came out wrong when generating code for MASM. Fixes upstream
GH#3241.
(Imported from upstream's c47aea8af1e28e46e1ad5e2e7468b49fec3f4f29.)
This does not affect of the configurations we generate and is imported
to avoid a diff against upstream.
Change-Id: Iacde0ca5220c3607681fad081fbe72d8d613518f
Reviewed-on: https://boringssl-review.googlesource.com/15985
Reviewed-by: Adam Langley <agl@google.com>
This avoids depending the FIPS module on crypto/bytestring and moves
ECDSA_SIG_{new,free} into the module.
Change-Id: I7b45ef07f1140873a0da300501141b6ae272a5d9
Reviewed-on: https://boringssl-review.googlesource.com/15984
Reviewed-by: Adam Langley <agl@google.com>
This dates to ded93581f1, but we have
since switched to building with nasm, to match upstream's supported
assemblers. Since this doesn't affect anything we generate, remove the
workaround to reduce the diff against upstream.
Change-Id: I549ae97ad6d6f28836f6c9d54dcf51c518de7521
Reviewed-on: https://boringssl-review.googlesource.com/15986
Reviewed-by: Adam Langley <agl@google.com>
EVP_AEAD_CTX is otherwise a pain to use from C++ when you need to keep
it around.
Change-Id: I1dff926b33a3246680be21b89b69dfb336d25cd5
Reviewed-on: https://boringssl-review.googlesource.com/15965
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: Icf1d6ec9d3fb33a124a9f61c75d29248a2582680
Reviewed-on: https://boringssl-review.googlesource.com/15964
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>
Nearly all of the assembly code was written by Shay and is submitted
under the CLA.
Change-Id: Ia70952d4ba2713ccc5e96a0952c22e5400c90f3a
Reviewed-on: https://boringssl-review.googlesource.com/15649
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>
The names in the P-224 code collided with the P-256 code and thus many
of the functions and constants in the P-224 code have been prefixed.
Change-Id: I6bcd304640c539d0483d129d5eaf1702894929a8
Reviewed-on: https://boringssl-review.googlesource.com/15847
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 wants d = e^-1 (mod lcm(p-1, q-1)), not (p-1)*(q-1).
Note this means the size of d might reveal information about p-1 and
q-1. However, we do operations with Chinese Remainder Theorem, so we
only use d (mod p-1) and d (mod q-1) as exponents. Using a minimal
totient does not affect those two values.
This removes RSA_recover_crt_params. Using a minimal d breaks (or rather
reveals an existing bug in) the function.
While I'm here, rename those ridiculous variable names.
Change-Id: Iaf623271d49cd664ba0eca24aa25a393f5666fac
Reviewed-on: https://boringssl-review.googlesource.com/15944
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>
Nothing is using them. For encrypt, there's generally no need to swap
out public key operations. keygen seems especially pointless as one
could just as easily call the other function directly.
The one behavior change is RSA_encrypt now gracefully detects if called
on an empty RSA, to match the other un-RSA_METHOD-ed functions which had
similar treatments. (Conscrypt was filling in the encrypt function
purely to provide a non-crashing no-op function. They leave the public
bits blank and pass their custom keys through sufficiently many layers
of Java crypto goo that it's not obvious whether this is reachable.)
We still can't take the function pointers out, but once
96bbe03dfd
trickles back into everything, we can finally prune RSA_METHOD.
Bump BORINGSSL_API_VERSION as a convenience so I can land the
corresponding removal in Conscrypt immediately.
Change-Id: Ia2ef4780a5dfcb869b224e1ff632daab8d378b2e
Reviewed-on: https://boringssl-review.googlesource.com/15864
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>
(Imported from upstream's 54538204d870b97c751d13efeefa876bd792a44b.)
Change-Id: If9967b67a74ab7dea175e97ea8bda195c3cd0478
Reviewed-on: https://boringssl-review.googlesource.com/15835
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>
ASN1_GENERALIZEDTIME and ASN1_UTCTIME may be specified using offsets,
even though that's not supported within certificates. [davidben: This
commit message seems off as crypto/x509 does not reject them. It merely
has a comment telling you that it's doing it wrong.]
To convert the offset time back to GMT, the offsets are supposed to be
subtracted, not added. e.g. 1759-0500 == 2359+0100 == 2259Z.
(Imported from upstream's d2335f30970ed3edc1c7c11700ab7f34396cf086.)
Change-Id: Id0d4c5b650e77db3b04b15e66b069807f6f31266
Reviewed-on: https://boringssl-review.googlesource.com/15834
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>
I forgot to scrub these files when they moved and their macros are
currently leaking into other files. This isn't a problem, but does
prevent ec/ code from being moved into the module at the moment.
Change-Id: I5433fb043e90a03ae3dc5c38cb3a69563aada007
Reviewed-on: https://boringssl-review.googlesource.com/15845
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>
Another synthesized function which may be referenced directly.
Change-Id: Ic75fe66ce7244246a2d4a707b6a5fee24cac6941
Reviewed-on: https://boringssl-review.googlesource.com/15831
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>
This will let us keep CBS/CBB out of the module. It also makes the PWCT
actually use a hard-coded public key since kEC was using the
private-key-only serialization.
Change-Id: I3769fa26fc789c4797a56534df73f810cf5441c4
Reviewed-on: https://boringssl-review.googlesource.com/15830
Reviewed-by: Adam Langley <agl@google.com>
This will let us keep CBS/CBB out of the module.
Change-Id: I780de0fa2c102cf27eee2cc242ee23740fbc16ce
Reviewed-on: https://boringssl-review.googlesource.com/15829
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>
RSA_verify_raw is the same as RSA_public_decrypt and fits the calling
convention better. This also avoids the extra copy.
Change-Id: Ib7e3152af26872440290a289f178c9a1d9bc673f
Reviewed-on: https://boringssl-review.googlesource.com/15826
Reviewed-by: Adam Langley <agl@google.com>
This allows us to implement RSA-PSS in the FIPS module without pulling
in EVP_PKEY. It also allows people to use RSA-PSS on an RSA*.
Empirically folks seem to use the low-level padding functions a lot,
which is unfortunate.
This allows us to remove a now redundant length check in p_rsa.c.
Change-Id: I5270e01c6999d462d378865db2b858103c335485
Reviewed-on: https://boringssl-review.googlesource.com/15825
Reviewed-by: Adam Langley <agl@google.com>
We check the length for MD5+SHA1 but not the normal cases. Instead,
EVP_PKEY_sign externally checks the length (largely because the silly
RSA-PSS padding function forces it). We especially should be checking
the length for these because otherwise the prefix built into the ASN.1
prefix is wrong.
The primary motivation is to avoid putting EVP_PKEY inside the FIPS
module. This means all logic for supported algorithms should live in
crypto/rsa.
This requires fixing up the verify_recover logic and some tests,
including bcm.c's KAT bits.
(evp_tests.txt is now this odd mixture of EVP-level and RSA-level error
codes. A follow-up change will add new APIs for RSA-PSS which will allow
p_rsa.c to be trimmed down and make things consistent.)
Change-Id: I29158e9695b28e8632b06b449234a5dded35c3e7
Reviewed-on: https://boringssl-review.googlesource.com/15824
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I167b7045c537d95294d387936f3d7bad530e1c6f
Reviewed-on: https://boringssl-review.googlesource.com/15844
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: I64533d2b4a6b075fa3ccea1abfd0ec5106673453
Reviewed-on: https://boringssl-review.googlesource.com/15704
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 works better with util/generate_build_files.py.
Change-Id: Icb55dc74e0a004aca3e09978640455b66f0473ff
Reviewed-on: https://boringssl-review.googlesource.com/15648
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 a remnant of the ECDSA code returning a tri-state -1, 0, 1.
Change-Id: I8bd1fcd94e07dbffc650f414ebc19f30236378bd
Reviewed-on: https://boringssl-review.googlesource.com/15667
Reviewed-by: Adam Langley <agl@google.com>
This CL adds utility code to process NIST CAVP test vectors using the
existing FileTest code.
Also add binaries for processing AESAVS (AES) and GCMVS (AES-GCM) vector
files.
Change-Id: I8e5ebf751d7d4b5504bbb52f3e087b0065babbe0
Reviewed-on: https://boringssl-review.googlesource.com/15484
Reviewed-by: Adam Langley <agl@google.com>
When code wants to push a pointer from the GOT onto the stack, we don't
have any registers to play with. We do, however, know that the stack is
viable and thankfully Intel has an “xchg” instruction that avoids the
need for an intermediate register.
Change-Id: Iba7e4f0f4c9b43b3d994cf6cfc92837b312c7728
Reviewed-on: https://boringssl-review.googlesource.com/15625
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 doesn't actually measure what we need(*) and, because of that, it's
way more noisy than expected.
(*) We want to know whether the pool has been initialised, not whether
it currently thinks it has a lot of bits, but we can't get what we want
without getrandom() support in the kernel.
Change-Id: I20accb99a592739c786a25c1656aeea050ae81a3
Reviewed-on: https://boringssl-review.googlesource.com/15624
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_ia32cap_addr avoids any relocations within the module, at the
cost of a runtime TEXTREL, which causes problems in some cases.
(Notably, if someone links us into a binary which uses the GCC "ifunc"
attribute, the loader crashes.)
We add a OPENSSL_ia32cap_addr_delta symbol (which is reachable
relocation-free from the module) stores the difference between
OPENSSL_ia32cap_P and its own address. Next, reference
OPENSSL_ia32cap_P in code as usual, but always doing LEAQ (or the
equivalent GOTPCREL MOVQ) into a register first. This pattern we can
then transform into a LEAQ and ADDQ on OPENSSL_ia32cap_addr_delta.
ADDQ modifies the FLAGS register, so this is only a safe transformation
if we safe and restore flags first. That, in turn, is only a safe
transformation if code always uses %rsp as a stack pointer (specifically
everything below the stack must be fair game for scribbling over). Linux
delivers signals on %rsp, so this should already be an ABI requirement.
Further, we must clear the red zone (using LEAQ to avoid touching FLAGS)
which signal handlers may not scribble over.
This also fixes the GOTTPOFF logic to clear the red zone.
Change-Id: I4ca6133ab936d5a13d5c8ef265a12ab6bd0073c9
Reviewed-on: https://boringssl-review.googlesource.com/15545
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>
Past the first word, the remaining arguments are usually separated by
commas. This avoids some of the awkward fixing up needed to extract
target registers, etc.
Change-Id: Id99b99e5160abf80e60afea96f2b46b53b55c9c5
Reviewed-on: https://boringssl-review.googlesource.com/15544
Reviewed-by: Adam Langley <agl@google.com>
OPENSSL_ia32cap_addr avoids any relocations within the module, at the
cost of a runtime TEXTREL, which causes problems in some cases.
(Notably, if someone links us into a binary which uses the GCC "ifunc"
attribute, the loader crashes.)
Fix C references of OPENSSL_ia32cap_addr with a function. This is
analogous to the BSS getters. A follow-up commit will fix perlasm with a
different scheme which avoids calling into a function (clobbering
registers and complicating unwind directives.)
Change-Id: I09d6cda4cec35b693e16b5387611167da8c7a6de
Reviewed-on: https://boringssl-review.googlesource.com/15525
Reviewed-by: Adam Langley <agl@google.com>
The test takes a little long to run. I've chopped it to primes up to
20,000. This ensures we still test some values out of range of the table
in crypto/bn/prime.c.
Also remove false comment in crypto/bn/prime.c.
Change-Id: I910015af9570b2f9f1c6c82dc61a0dbdfd24840b
Reviewed-on: https://boringssl-review.googlesource.com/15604
Reviewed-by: Adam Langley <agl@google.com>
We only ever compute it for odd (actually, prime) modulus as part of
BN_mod_sqrt.
If we cared, we could probably drop this from most binaries. This is
used to when modular square root needs Tonelli-Shanks. Modular square
root is only used for compressed coordinates. Of our supported curves
(I'm handwaiving away EC_GROUP_new_curve_GFp here[*]), only P-224 needs
the full Tonelli-Shanks algorithm (p is 1 mod 8). That computes the
Legendre symbol a bunch to find a non-square mod p. But p is known at
compile-time, so we can just hard-code a sample non-square.
Sadly, BN_mod_sqrt has some callers outside of crypto/ec, so there's
also that. Anyway, it's also not that large of a function.
[*] Glancing through SEC 2 and Brainpool, secp224r1 is the only curve
listed in either document whose prime is not either 3 mod 4 or 5 mod 8.
Even 5 mod 8 is rare: only secp224k1. It's unlikely anyone would notice
if we broke annoying primes. Though OpenSSL does support "WTLS" curves
which has an additional 1 mod 8 case.
Change-Id: If36aa78c0d41253ec024f2d90692949515356cd1
Reviewed-on: https://boringssl-review.googlesource.com/15425
Reviewed-by: Adam Langley <agl@google.com>
Not requiring the list of assembly sources to be comma-separated is
helpful to environments where the list would more naturally be
treated as a list.
Change-Id: I43b18cdbeed1dc7ad217ff61557ac55860f40733
Reviewed-on: https://boringssl-review.googlesource.com/15585
Reviewed-by: Matt Braithwaite <mab@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>
Chromium's test infrastruction does not actually support GTest. It
requires a custom test runner in //base. Split gtest_main.cc up into a
gtest_main.h which defines a support function we maintain and a default
runner. Chromium's build will swap that file out for a custom one.
BUG=129
Change-Id: I3e39fe3a931b3051a61d5f8eef514ca6a504f11c
Reviewed-on: https://boringssl-review.googlesource.com/15564
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>
Looks like this file was forgotten when the old suites were removed.
Change-Id: Ied8d82e23ae5db0257add3c18eee46ee1a366637
Reviewed-on: https://boringssl-review.googlesource.com/15444
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: I4e34dabe302f7dacdf04a89052ad9fe9254a1b81
Reviewed-on: https://boringssl-review.googlesource.com/15404
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>
Whether UCHAR_MAX expands to a signed or unsigned number appears to be a
matter of some debate. Or the Android headers are wrong. Just add a cast
and not think about it too hard.
Change-Id: I84e928bdfe459a9129cde276c82b60a318533552
Reviewed-on: https://boringssl-review.googlesource.com/15385
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This can just be a bog-standard loop. The compiler should be plenty
smart enough to transform it into whatever it likes.
Change-Id: I7b782dd2a11902f6e5c9902d9624be26eee5c959
Reviewed-on: https://boringssl-review.googlesource.com/15366
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>
CRYPTO_gcm128_init is currently assuming that it gets passed in
aesni_encrypt whenever it selects the AVX implementation. This is true,
but we can easily avoid this assumption by adding an extra boolean
input.
Change-Id: Ie7888323f0c93ff9df8f1cf3ba784fb35bb07076
Reviewed-on: https://boringssl-review.googlesource.com/15370
Reviewed-by: Adam Langley <agl@google.com>
These modes do internal random IV generation and are unsuitable for
non-testing purposes.
Change-Id: I14b98af8f6cf43b4fc835a2b04a9b0425b7651b7
Reviewed-on: https://boringssl-review.googlesource.com/15244
Reviewed-by: Adam Langley <agl@google.com>
This also fixes the comments regarding BN_prime_checks to match the
security level guarantees provided by BN_prime_checks.
Change-Id: I8032e88680bf51e8876e134b4253ed26c2072617
Reviewed-on: https://boringssl-review.googlesource.com/15304
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
C leaves some details as implementation-defined. We only support
implementations which define things consistent with the obvious
representation on current machines.
Change-Id: I255a2e54a86c8a7d987dea51ea7168ad66ad9ddd
Reviewed-on: https://boringssl-review.googlesource.com/15305
Reviewed-by: Adam Langley <agl@google.com>
CMake loves making archives, but that's not universal.
Change-Id: I5356b4701982748a46817e0094ad838605dcada6
Reviewed-on: https://boringssl-review.googlesource.com/15144
Reviewed-by: Adam Langley <agl@google.com>
Using |size_t| was correct, except for NaCl, which is a 64-bit build
with 32-bit pointers. In that configuration, |size_t| is smaller than
the native word size.
This change adds |crypto_word_t|, an unsigned type with native size and
switches constant-time functions to using it.
Change-Id: Ib275127063d5edbb7c55d413132711b7c74206b0
Reviewed-on: https://boringssl-review.googlesource.com/15325
Reviewed-by: Adam Langley <agl@google.com>
In some modes the compiler will emit a section for BSS symbols and
construct the values with labels, alignment and data instructions. This
change parses these sections and emits the local versions of each symbol
needed to make this work.
Change-Id: I8d43ffe4b5b734950aa4287a3dd7c0d2f191f2e4
Reviewed-on: https://boringssl-review.googlesource.com/15206
Reviewed-by: Adam Langley <agl@google.com>
We might want to back off on this in the future so that we don't upset
future compiler work but, for now, it's useful to know when we hit
something that we don't understand.
Change-Id: I763830b0ddcf5da20061fad673265d4a5855479c
Reviewed-on: https://boringssl-review.googlesource.com/15205
Reviewed-by: Adam Langley <agl@google.com>
In order to better handle BSS sections, rather than having a single loop
over the lines and state flags, pull lines as needed. This means that
subfunctions can process sections of the input.
Also, stop bothering to move the init_array to the end, it's already put
into its own section.
Change-Id: I0e62930c65d29baecb39ba0d8bbc21f2da3bde56
Reviewed-on: https://boringssl-review.googlesource.com/15204
Reviewed-by: Adam Langley <agl@google.com>
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>
We forgot to run the script at some point.
Change-Id: I0bd142fdd13d64c1ed81d9b1515449220d1c936b
Reviewed-on: https://boringssl-review.googlesource.com/14329
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>
EVP_DigestUpdate can tolerate zero length inputs. Also properly clean up
ctx in all codepaths.
Change-Id: I90889c6236f6bf74625ba9f967de36949a9a6f83
Reviewed-on: https://boringssl-review.googlesource.com/14327
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>
We still need to expose a suitable API for Chromium to consume, but the
core implementation itself should now be ready.
The supported cipher list is based on what EVP_get_cipherbynid currently
supports, excluding the entries which don't have OIDs.
BUG=54
Change-Id: I3befca0a34b330ec1f663a029a8fbf049a4406bd
Reviewed-on: https://boringssl-review.googlesource.com/14212
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>
Playing around with the code, we seem to have sufficient positive test
vectors for the logic around the high bits, but not negative test
vectors. Add some. Also add a negative test vector for the trailing
byte.
(For future reference, use openssl rsautl -raw for raw RSA operations
and openssl pkeyutil for EVP_PKEY_sign.)
Change-Id: I36eddf048e51e037fd924902cd13dcb3c62bfd02
Reviewed-on: https://boringssl-review.googlesource.com/14325
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>
To remove the OID table from Chromium, we'll need to decouple a lot of
this code. In preparation for that, detach the easy cases from the OID
table. What remains is PBES, cipher, and digest OIDs which will be doing
in follow-up changes.
BUG=54
Change-Id: Ie205d23d042e21114ca1faf68917fdc870969d09
Reviewed-on: https://boringssl-review.googlesource.com/14209
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>
If ret is allocated, it may be leaked on error.
(Imported from upstream's cdfb7809b6a365a0a7874afd8f8778c5c572f267 and
ffcdb0e6efb6fb7033b2cd29e8cca2e2fe355c14.)
Change-Id: I50ed9ad072cf80461d9527d0834b596a8c32e3d3
Reviewed-on: https://boringssl-review.googlesource.com/14315
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>
conf has the ability to expand variables in config files. Repeatedly doing
this can lead to an exponential increase in the amount of memory required.
This places a limit on the length of a value that can result from an
expansion.
Credit to OSS-Fuzz for finding this problem.
(Imported from upstream's 6a6213556a80ab0a9eb926a1d6023b8bf44f2afd. This
also import's upstream's ee1ccd0a41ad068957fe65ba7521e593b51bbad4 which
we had previously missed.)
Change-Id: I9be06a7e8a062b5adcd00c974a7b245226123563
Reviewed-on: https://boringssl-review.googlesource.com/14316
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>
(Imported from upstream's 04cf39207f94abf89b3964c7710f22f829a1a78f.)
The other half of the change was fixed earlier, but this logic was still
off. This code is kind of a mess and needs a rewrite, but import the
change to get it correct and sufficiently tested first.
(If we could take the sLen = -2 case away altogether, that would be
great...)
Change-Id: I5786e980f26648822633fc216315e8f77ed4d45b
Reviewed-on: https://boringssl-review.googlesource.com/14321
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>
One test case is commented out, to be fixed in a follow-up.
Change-Id: I543c7f54e63837c6e8088fdcbb03226e0144b2e5
Reviewed-on: https://boringssl-review.googlesource.com/14320
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 too appear to be unused now that the core parsers use CBS. They
also were buggy as they silently ignored sign bits. This removes all
ASN1_PRIMITIVE_FUNCS definitions. (The code to use them still exists as
we're not ready to diverge on tasn_*. Current thinking is we'll
eventually just ditch the code rather than do so.)
Change-Id: I8d20e2989460dd593d62368cfbd083d5de1ee2a1
Reviewed-on: https://boringssl-review.googlesource.com/14324
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 have no consumers remaining. Upstream recently had a long series
of bugfixes for these types (2cbd4d98673d99cd7cb10715656b6d3727342e77,
e5afec1831248c767be7c5844a88535dabecc01a,
9abe889702bdc73f9490f611f54bf9c865702554,
2e5adeb2904dd68780fb154dbeb6e3efafb418bb). Rather than worry about this,
just remove the code.
Change-Id: I90f896aad096fc4979877e2006131e76c9ff023b
Reviewed-on: https://boringssl-review.googlesource.com/14323
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>
Import test data from:
ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1/pkcs-1v2-1-vec.zip
This is a set of RSA-PSS and RSA-OAEP test vectors including some edge cases
with unusual key sizes.
(Imported from upstream's 946a515a2b370dbadb1f8c39e3586a8f1e3cff1a.)
Change-Id: I1d8aa85a8578e47b26c74bb4e4c246975619d574
Reviewed-on: https://boringssl-review.googlesource.com/14318
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>
This change adds support for setting an |SSL_TICKET_AEAD_METHOD| which
allows a caller to control ticket encryption and decryption to a greater
extent than previously possible and also permits asynchronous ticket
decryption.
This change only includes partial support: TLS 1.3 work remains to be
done.
Change-Id: Ia2e10ebb3257e1a119630c463b6bf389cf20ef18
Reviewed-on: https://boringssl-review.googlesource.com/14144
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This makes it easier to build a subset of BoringSSL which doesn't depend
on the filesystem (though perhaps it's worth a build define for that
now). This hook is also generally surprising. CONF hooks are bad enough
when they don't open arbitrary files.
Change-Id: Ibf791162dd3d4cec8117eb49ff0cd716a1c54abd
Reviewed-on: https://boringssl-review.googlesource.com/14166
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>
It's more consistent to have the helper function do the check that
its every caller already performs. This removes the error code
SSL_R_LIBRARY_HAS_NO_CIPHERS in favor of SSL_R_NO_CIPHER_MATCH.
Change-Id: I522239770dcb881d33d54616af386142ae41b29f
Reviewed-on: https://boringssl-review.googlesource.com/13964
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 allows a caller to configure a serving chain without dealing with
crypto/x509.
Change-Id: Ib42bb2ab9227d32071cf13ab07f92d029643a9a6
Reviewed-on: https://boringssl-review.googlesource.com/14126
Commit-Queue: Adam Langley <alangley@gmail.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 disabled because we couldn't test it. We now have SDE for
testing which, even if it's not running on a builder yet, confirms that
this passes tests for all current and past Intel chips.
Change-Id: Iad74cc9944ee85557bb45c981751f84f335fb6c8
Reviewed-on: https://boringssl-review.googlesource.com/14010
Commit-Queue: Adam Langley <alangley@gmail.com>
Reviewed-by: Adam Langley <agl@google.com>
On 32-bit x86, |bn_mul_mont| returns 0 when the modulus has less than
four limbs. Instead of calling |bn_mul_mont| and then falling back to
the |BN_mul|+|BN_from_montgomery_word| path for small moduli, just
avoid calling |bn_mul_mont| at all for small moduli.
This allows us to more clearly understand exactly when the fallback
code path, which is a timing side channel, is taken. This change makes
it easier to start minimizing this side channel.
The limit is set at 128 bits, which is four limbs on 32-bit and two
limbs on 64-bit platforms. Do this consistently on all platforms even
though it seems to be needed only for 32-bit x86, to minimize platform
variance: every platform uses the same cut-off in terms of input size.
128 bits is small enough to allow even questionably small curves, like
secp128r1, to use the |bn_mul_mont| path, and is way too small for RSA
and FFDH, so this change shouldn't have any security impact other than
the positive impact of simplifying the control flow.
Change-Id: I9b68ae33dc2c86b54ed4294839c7eca6a1dc11c0
Reviewed-on: https://boringssl-review.googlesource.com/14084
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>
