Nicola Tuveri [Thu, 1 Aug 2019 23:08:34 +0000 (02:08 +0300)]
Make BN_num_bits() consttime upon BN_FLG_CONSTTIME
This issue was partially addressed by commit 972c87dfc7e765bd28a4964519c362f0d3a58ca4, which hardened its callee
BN_num_bits_word() to avoid leaking the most-significant word of its
argument via branching and memory access pattern.
The commit message also reported:
> There are a few places where BN_num_bits is called on an input where
> the bit length is also secret. This does *not* fully resolve those
> cases as we still only look at the top word.
BN_num_bits() is called directly or indirectly (e.g., through
BN_num_bytes() or BN_bn2binpad() ) in various parts of the `crypto/ec`
code, notably in all the currently supported implementations of scalar
multiplication (in the generic path through ec_scalar_mul_ladder() as
well as in dedicated methods like ecp_nistp{224,256,521}.c and
ecp_nistz256.c).
Under the right conditions, a motivated SCA attacker could retrieve the
secret bitlength of a secret nonce through this vulnerability,
potentially leading, ultimately, to recover a long-term secret key.
With this commit, exclusively for BIGNUMs that are flagged with
BN_FLG_CONSTTIME, instead of accessing only bn->top, all the limbs of
the BIGNUM are accessed up to bn->dmax and bitwise masking is used to
avoid branching.
Memory access pattern still leaks bn->dmax, the size of the lazily
allocated buffer for representing the BIGNUM, which is inevitable with
the current BIGNUM architecture: reading past bn->dmax would be an
out-of-bound read.
As such, it's the caller responsibility to ensure that bn->dmax does not
leak secret information, by explicitly expanding the internal BIGNUM
buffer to a public value sufficient to avoid any lazy reallocation
while manipulating it: this should be already done at the top level
alongside setting the BN_FLG_CONSTTIME.
Thanks to David Schrammel and Samuel Weiser for reporting this issue
through responsible disclosure.
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)
Nicola Tuveri [Thu, 1 Aug 2019 22:33:05 +0000 (01:33 +0300)]
Fix a SCA leak using BN_bn2bin()
BN_bn2bin() is not constant-time and leaks the number of bits in the
processed BIGNUM.
The specialized methods in ecp_nistp224.c, ecp_nistp256.c and
ecp_nistp521.c internally used BN_bn2bin() to convert scalars into the
internal fixed length representation.
This can leak during ECDSA/ECDH key generation or handling the nonce
while generating an ECDSA signature, when using these implementations.
The amount and risk of leaked information useful for a SCA attack
varies for each of the three curves, as it depends mainly on the
ratio between the bitlength of the curve subgroup order (governing the
size of the secret nonce/key) and the limb size for the internal BIGNUM
representation (which depends on the compilation target architecture).
To fix this, we replace BN_bn2bin() with BN_bn2binpad(), bounding the
output length to the width of the internal representation buffer: this
length is public.
Internally the final implementation of both BN_bn2binpad() and
BN_bn2bin() already has masking in place to avoid leaking bn->top
through memory access patterns.
Memory access pattern still leaks bn->dmax, the size of the lazily
allocated buffer for representing the BIGNUM, which is inevitable with
the current BIGNUM architecture: reading past bn->dmax would be an
out-of-bound read.
As such, it's the caller responsibility to ensure that bn->dmax does not
leak secret information, by explicitly expanding the internal BIGNUM
buffer to a public value sufficient to avoid any lazy reallocation
while manipulating it: this is already done at the top level alongside
setting the BN_FLG_CONSTTIME.
Finally, the internal implementation of BN_bn2binpad() indirectly calls
BN_num_bits() via BN_num_bytes(): the current implementation of
BN_num_bits() can leak information to a SCA attacker, and is addressed
in the next commit.
Thanks to David Schrammel and Samuel Weiser for reporting this issue
through responsible disclosure.
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9511)
[crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.
This commit addresses multiple side-channel vulnerabilities present
during RSA key validation.
Private key parameters are re-computed using variable-time functions.
This issue was discovered and reported by the NISEC group at TAU Finland.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9779)
Richard Levitte [Fri, 30 Aug 2019 14:54:47 +0000 (16:54 +0200)]
Deal with BUF_MEM_grow ambiguity
BUF_MEM_grow() returns the passed length, but also zero on error. If
the passed length was zero, an extra check to see if a returned zero
was an error or not is needed.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9662)
Richard Levitte [Fri, 30 Aug 2019 13:36:20 +0000 (15:36 +0200)]
More KDF cleanup
The EVP_KDF_ definitions are no longer needed, and neither is
EVP_get_kdfbyname()
test/evp_kdf_test.c tried to use a EVP_get_kdfbyname() that was rewritten
to use EVP_KDF_fetch() without ever freeing the resulting KDF method.
It's better to refactor the test to use EVP_KDF_fetch directly.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9662)
Richard Levitte [Fri, 30 Aug 2019 12:32:55 +0000 (14:32 +0200)]
crypto/evp/pkey_kdf.c: Redo parameter processing
Undo the caching scheme, pass through most controls as parameters, except
for SEED and INFO, where we keep supporting adding data through additional
ctrl calls by collecting the data, and only passing it to the EVP_KDF
before calling its derive function.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9662)
Matt Caswell [Thu, 5 Sep 2019 15:43:57 +0000 (16:43 +0100)]
Don't send a status_request extension in a CertificateRequest message
If a TLSv1.3 server configured to respond to the status_request extension
also attempted to send a CertificateRequest then it was incorrectly
inserting a non zero length status_request extension into that message.
The TLSv1.3 RFC does allow that extension in that message but it must
always be zero length.
In fact we should not be sending the extension at all in that message
because we don't support it.
Fixes #9767
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9780)
Viktor Dukhovni [Sun, 9 Dec 2018 22:10:29 +0000 (17:10 -0500)]
Undeprecate OpenSSL_version_num and OPENSSL_VERSION_NUMBER
The OpenSSL_version_num() function returns at runtime the
OPENSSL_VERSION_NUMBER of the compiled OpenSSL library. This is a
used and useful interface, and should not (at least yet) be
deprecated, we just introduced the new versioning schema, it seems
too early to deprecate the old.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7853)
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9776)
Billy Brumley [Mon, 2 Sep 2019 12:02:30 +0000 (15:02 +0300)]
[crypto/ec] for ECC parameters with NULL or zero cofactor, compute it
The cofactor argument to EC_GROUP_set_generator is optional, and SCA mitigations for ECC currently use it. So the library currently falls back to very old SCA-vulnerable code if the cofactor is not present.
This PR allows EC_GROUP_set_generator to compute the cofactor for all curves of cryptographic interest. Steering scalar multiplication to more SCA-robust code.
This issue affects persisted private keys in explicit parameter form, where the (optional) cofactor field is zero or absent.
It also affects curves not built-in to the library, but constructed programatically with explicit parameters, then calling EC_GROUP_set_generator with a nonsensical value (NULL, zero).
The very old scalar multiplication code is known to be vulnerable to local uarch attacks, outside of the OpenSSL threat model. New results suggest the code path is also vulnerable to traditional wall clock timing attacks.
CVE-2019-1547
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9827)
Rich Salz [Sun, 18 Aug 2019 13:04:17 +0000 (09:04 -0400)]
Make failed messages easier to find
Now that we use travis_terminate, we can make the status messages
simpler to find, and we don't need the "OK" output.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9707)
Bernd Edlinger [Mon, 19 Aug 2019 15:12:22 +0000 (17:12 +0200)]
Fix error handling in x509_lu.c
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9639)
Rich Salz [Thu, 29 Aug 2019 16:12:17 +0000 (12:12 -0400)]
Handle the renamed command POD files in find-doc-nits
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9729)
Billy Brawner [Wed, 28 Aug 2019 00:07:17 +0000 (17:07 -0700)]
Suppress 'No server certificate CA names sent' message
Fixes #9080
Signed-off-by: Billy Brawner <billy@wbrawner.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9710)
Richard Levitte [Tue, 3 Sep 2019 16:11:49 +0000 (18:11 +0200)]
New function EVP_CIPHER_free()
This function re-implements EVP_CIPHER_meth_free(), but has a name that
isn't encumbered by legacy EVP_CIPHER construction functionality.
We also refactor most of EVP_CIPHER_meth_new() into an internal
evp_cipher_new() that's used when creating fetched methods.
EVP_CIPHER_meth_new() and EVP_CIPHER_meth_free() are rewritten in terms of
evp_cipher_new() and EVP_CIPHER_free(). This means that at any time, we can
deprecate all the EVP_CIPHER_meth_ functions with no harmful consequence.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9758)
Richard Levitte [Tue, 3 Sep 2019 15:47:13 +0000 (17:47 +0200)]
New function EVP_MD_free()
This function re-implements EVP_MD_meth_free(), but has a name that
isn't encumbered by legacy EVP_MD construction functionality.
We also refactor most of EVP_MD_meth_new() into an internal
evp_md_new() that's used when creating fetched methods.
EVP_MD_meth_new() and EVP_MD_meth_free() are rewritten in terms of
evp_md_new() and EVP_MD_free(). This means that at any time, we can
deprecate all the EVP_MD_meth_ functions with no harmful consequence.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9758)
Richard Levitte [Tue, 3 Sep 2019 13:10:43 +0000 (15:10 +0200)]
test/errtest.c: more conditions for checking __FILE__ and __LINE__
When at least one of OPENSSL_NO_ERR or OPENSSL_NO_FILENAMES is
defined, __FILE__ and __LINE__ are not saved with the error record.
This test only checked OPENSSL_NO_FILENAMES. Now fixed.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9755)
Richard Levitte [Fri, 23 Aug 2019 12:03:28 +0000 (14:03 +0200)]
Refactor how KEYMGMT methods get associated with other methods
KEYMGMT methods were attached to other methods after those were fully
created and registered, thereby creating a potential data race, if two
threads tried to create the exact same method at the same time.
Instead of this, we change the method creating function to take an
extra data parameter, passed all the way from the public fetching
function. In the case of EVP_KEYEXCH, we pass all the necessary data
that evp_keyexch_from_dispatch() needs to be able to fetch the
appropriate KEYMGMT method on the fly.
Fixes #9592
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9678)
Richard Levitte [Sun, 1 Sep 2019 08:58:19 +0000 (10:58 +0200)]
Diverse ERR fixes
1. There are still references to the removed ERR_put_func_error().
2. ERR_put_error() is deprecated as off version 3.0, so should
ERR_PUT_error().
3. 'no-err' didn't affect what was passed to ERR_set_debug().
Fixes #9522
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9747)
Richard Levitte [Wed, 28 Aug 2019 07:13:21 +0000 (09:13 +0200)]
Rework the documentation of our individual MAC implementations
We now describe parameters instead of controls.
Also, since macros like EVP_MAC_CMAC do not exist any more, we rename
the pod files from EVP_MAC_{algo}.pod to EVP_MAC-{algo}.pod. This
allows getting the documentation like this:
man EVP_MAC CMAC
[skip ci]
Fixes #9709
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9713)
Bernd Edlinger [Sat, 24 Aug 2019 09:28:19 +0000 (11:28 +0200)]
Remove extern declarations of OPENSSL_ia32cap_P
Use the header file internal/cryptlib.h instead.
Remove checks for OPENSSL_NO_ASM and I386_ONLY
in cryptlib.c, to match the checks in other
places where OPENSSL_ia32cap_P is used and
assumed to be initialized.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/9688)
Matt Caswell [Thu, 29 Aug 2019 16:15:16 +0000 (17:15 +0100)]
Fix pkeyutl -verifyrecover
When performing a pkeyutl -verifyrecover operation the input file is not
a hash - it is the signature itself. Therefore don't do the check to make
sure it looks like a hash.
Fixes #9658
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9731)
Pauli [Thu, 29 Aug 2019 21:38:58 +0000 (07:38 +1000)]
Don't include the DEVRANDOM being seeded logic on Android.
It lacks exposure of the `shm*` functions and should prefer the GETRANDOM
source.
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9735)
Matt Caswell [Wed, 28 Aug 2019 13:31:39 +0000 (14:31 +0100)]
Fix no-ec
A test in sslapitest.c was failing in a no-ec build because we were using
an EC based ciphersuite. That particular test doesn't require EC
specifically, so we swap to a non EC based ciphersuite.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9716)
Matt Caswell [Wed, 28 Aug 2019 16:30:14 +0000 (17:30 +0100)]
Update fuzz README.md
Building with enable-fuzz-afl has always required no-shared. We now also
need no-module for a successful build. Therefore update the README
accordingly.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9721)
Matt Caswell [Wed, 14 Aug 2019 17:09:28 +0000 (18:09 +0100)]
Fix data races in EVP_CIPHER_fetch and EVP_MD_fetch
Don't modify the cipher/md we just fetched - it could be shared by multiple
threads.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)
Matt Caswell [Wed, 14 Aug 2019 14:00:35 +0000 (15:00 +0100)]
Make sure we pre-initialise properties
Simplify the initialisation of the core by pre-initialising properties.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)
Matt Caswell [Wed, 14 Aug 2019 13:43:11 +0000 (14:43 +0100)]
Fix context locking
Some parts of OPENSSL_CTX intialisation can get quite complex (e.g. RAND).
This can lead to complex interactions where different parts of the library
try to initialise while other parts are still initialising. This can lead
to deadlocks because both parts want to obtain the init lock.
We separate out the init lock so that it is only used to manage the
dynamic list of indexes. Each part of the library gets its own
initialisation lock.
Fixes #9454
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)
Richard Levitte [Sat, 24 Aug 2019 10:40:10 +0000 (12:40 +0200)]
OPENSSL_info(): add the item OPENSSL_INFO_SEED_SOURCE and use it
'openssl version -r' prints the seed source based on compiler macros.
This does not necessarily reflect the library's idea of what seed
sources to use, so we reimplement the list of seed sources as a
OPENSSL_info() item and display that instead.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9689)
David Woodhouse [Thu, 22 Aug 2019 17:09:11 +0000 (18:09 +0100)]
Fix bogus check for EVP_PKEY_supports_digest_nid() in check_cert_usable()
In commit 2d263a4a73 ("Honour mandatory digest on private key in
has_usable_cert()" I added two checks for the capabilities of the
EVP_PKEY being used. One of them was wrong, as it should only be
checking the signature of the X.509 cert (by its issuer) against the
sigalgs given in a TLS v1.3 signature_algorithms_cert extension.
Remove it and provide the code comments which, if they'd been present
in the first place, would hopefully have prevented the mistake.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9672)
Fix SCA vulnerability when using PVK and MSBLOB key formats
This commit addresses a side-channel vulnerability present when
PVK and MSBLOB key formats are loaded into OpenSSL.
The public key was not computed using a constant-time exponentiation
function.
This issue was discovered and reported by the NISEC group at TAU Finland.
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9587)
Rich Salz [Fri, 16 Aug 2019 12:34:16 +0000 (08:34 -0400)]
Do not have duplicate section heads
Change find-doc-nits to complain if a section header is repeated,
within a parent header (i.e., duplicate =head2 within a =head1).
In almost all cases, we just remove the duplicate header, as
it was a "continuation" of the =head1 that was already in affect.
In some cases, just remove "=head1 NOTES", possibly moving text
around, because the "NOTES" were really important parts of the
DESCRIPTION section.
No =headX sections should end with a period.
All =head1 labels should be in all uppercase.
No sub-head (=head2, etc) should be in all uppercase.
Update find-doc-nits to reject the above.
Fixup an internal POD link
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Yang <kaishen.yy@antfin.com> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9631)
Shane Lontis [Mon, 26 Aug 2019 07:05:08 +0000 (17:05 +1000)]
Cleanup ciphers and Add 3des ciphers.
Moved the relevant ciphers into default and restructed headers to allow the move.
This removed most of the cases of #ifdef NO_XXX (which are now specified in build.info)
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9482)
Shane Lontis [Sun, 25 Aug 2019 07:10:48 +0000 (17:10 +1000)]
Fix Issue OSS-Fuzz: Branch on uninitialized memory (in ccm code).
This would also happen for aes-ccm. There was one branch path where it just returned 1
without setting *padlen, It now branches so that the value is set to 0.
Fixes #9691
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9692)
Richard Levitte [Thu, 22 Aug 2019 10:50:00 +0000 (12:50 +0200)]
Get rid of the diversity of names for MAC parameters
The EVP_PKEY MAC implementations had a diversity of controls that were
really the same thing. We did reproduce that for the provider based
MACs, but are changing our minds on this. Instead of that, we now use
one parameter name for passing the name of the underlying ciphers or
digests to a MAC implementation, "cipher" and "digest", and one
parameter name for passing the output size of the MAC, "size".
Then we leave it to the EVP_PKEY->EVP_MAC bridge to translate "md"
to "digest", and "digestsize" to "size".
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9667)
Pauli [Sat, 24 Aug 2019 06:13:24 +0000 (16:13 +1000)]
Avoid overflowing FDSET when using select(2).
There is a problem in the rand_unix.c code when the random seed fd is greater
than or equal to FD_SETSIZE and the FDSET overruns its limit and walks the
stack.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9686)