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>
This allows us to move the code from Chrome into BoringSSL itself.
BUG=126
Change-Id: I04b4f63008a6de0a58dd6c685c78e9edd06deda6
Reviewed-on: https://boringssl-review.googlesource.com/14028
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 also adds a few missing assertions (X25519 returns true in normal
cases and, even when it returns zero, it still writes to out.)
BUG=129
Change-Id: I63f7e9025f88b2ec309382b66fc915acca6513a9
Reviewed-on: https://boringssl-review.googlesource.com/14030
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>
BUG=129
Change-Id: Ie64a445a42fb3a6d16818b1fabba8481e6e9ad94
Reviewed-on: https://boringssl-review.googlesource.com/14029
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>
Within the library, we never need to exponentiate modulo an even number.
In fact, all the remaining BN_mod_exp calls are modulo an odd prime.
This extends 617804adc5 to the rest of the
library.
Change-Id: I4273439faa6a516c99673b28f8ae38ddfff7e42d
Reviewed-on: https://boringssl-review.googlesource.com/14024
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Update the X509v3 name parsing to allow multiple xn-- international
domain name indicators in a name. Previously, only allowed one at
the beginning of a name, which was wrong.
(Imported from upstream's 31d1d3741f16bd80ec25f72dcdbf6bbdc5664374)
Change-Id: I93f1db7a5920305569af23f9f2b30ab5cc226521
Reviewed-on: https://boringssl-review.googlesource.com/13984
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>
These are only used by crypto/asn1 and not externally.
Change-Id: I2e6a28828fd81a4e3421eed1e98f0a65197f4b88
Reviewed-on: https://boringssl-review.googlesource.com/13868
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>
Noticed this comparing our and upstream's ASN.1 code. Somehow I missed
this line in cb852981cd. This change is a
no-op as our only ASN1_EX_COMBINE field is an ASN1_CHOICE which does not
read aclass.
Change-Id: I011f2f6eadd3939ec5f0b346c4eb7d14e406e3cd
Reviewed-on: https://boringssl-review.googlesource.com/13833
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_template_noexp_d2i call ASN1_item_ex_free(&skfield,...) on error.
Reworked error handling in asn1_item_ex_combine_new:
- call ASN1_item_ex_free and return the correct error code if
ASN1_template_new failed.
- dont call ASN1_item_ex_free if ASN1_OP_NEW_PRE failed.
Reworked error handing in x509_name_ex_d2i and x509_name_encode.
(Imported from upstream's 748cb9a17f4f2b77aad816cf658cd4025dc847ee.)
I believe the tasn1_new.c change is a no-op since we have no
ASN1_OP_NEW_PRE hooks anymore. I'm not sure what the commit message is
referring to with ASN1_template_new. It also seems odd as
ASN1_item_ex_free should probably be able to survive *pval being NULL.
Whatever.
We'd previously tried to fix x509_name_ex_d2i, but I think ours wasn't
quite right. (This thing is a mess...) I've aligned that function with
upstream.
Change-Id: Ie71521cd8a1ec357876caadd13be1ce247110f76
Reviewed-on: https://boringssl-review.googlesource.com/13831
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>
(Imported from upstream's 1222d273d36277f56c3603a757240c386d55f318.)
We'd fixed half of these, but the other half are probably unreachable
from code that ran under malloc tests, so we never noticed. It's
puzzling why upstream did both this and
166e365ed84dfabec3274baf8a9ef8aa4e677891. It seems you only need one of
them.
Change-Id: I08074358134180c6661600b66958ba861e7726fb
Reviewed-on: https://boringssl-review.googlesource.com/13832
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: Id7a92285601ff4276f4015eaee290bf77aa22b47
Reviewed-on: https://boringssl-review.googlesource.com/13628
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 copy fails, we shouldn't call cleanup. Also remove some pointless
NULL checks after EVP_PKEY_up_ref.
See also upstream's 748cb9a17f4f2b77aad816cf658cd4025dc847ee.
Change-Id: I2acb6892cde1ab662ca6a620d87179f9be609cba
Reviewed-on: https://boringssl-review.googlesource.com/13830
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 were added in an attempt to deal with the empty vs. NULL confusion
in PKCS#12. Instead, PKCS8_encrypt and PKCS8_decrypt already treated
NULL special. Since we're stuck with supporting APIs like those anyway,
Chromium has been converted to use that feature. This cuts down on the
number of APIs we need to decouple from crypto/asn1.
BUG=54
Change-Id: Ie2d4798d326c5171ea5d731da0a2c11278bc0241
Reviewed-on: https://boringssl-review.googlesource.com/13885
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>
