- 07 Sep, 2019 1 commit
-
-
Billy Brumley authored
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:
Matt Caswell <matt@openssl.org> Reviewed-by:
Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by:
Nicola Tuveri <nic.tuv@gmail.com> (Merged from https://github.com/openssl/openssl/pull/9781)
-
- 06 Sep, 2019 10 commits
-
-
Nicola Tuveri authored
Reviewed-by:
Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511) (cherry picked from commit 4fe2ee3a449a8ca2886584e221f34ff0ef5de119)
-
Nicola Tuveri authored
Replace flip_endian() by using the little endian specific BN_bn2lebinpad() and BN_lebin2bn(). Reviewed-by:
-
Nicola Tuveri authored
Reviewed-by:
-
Nicola Tuveri authored
This issue was partially addressed by commit 972c87df , 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:
-
Nicola Tuveri authored
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:
-
Bernd Edlinger authored
Reviewed-by:
-
Cesar Pereida Garcia authored
Reviewed-by:
-
Cesar Pereida Garcia authored
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:
-
Matt Caswell authored
We also use this in test_tls13messages to check that the extensions we expect to see in a CertificateRequest are there. Reviewed-by:
-
Matt Caswell authored
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:
-
- 05 Sep, 2019 3 commits
-
-
Bernd Edlinger authored
Reviewed-by:
Richard Levitte <levitte@openssl.org> Reviewed-by:
-
Bernd Edlinger authored
Fixes #9757 Reviewed-by:
-
Bernd Edlinger authored
This will never be the case for 1.1.1 so removed. Fixes: comment 1 of #9757 Reviewed-by:
Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9762)
-
- 04 Sep, 2019 4 commits
-
-
Billy Brawner authored
Fixes #9080 Signed-off-by:
Billy Brawner <billy@wbrawner.com> Reviewed-by:
-
raja-ashok authored
Reviewed-by:
-
raja-ashok authored
Reviewed-by:
-
David Woodhouse authored
In commit 6aca8d1a ("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. Reviewed-by:
Ben Kaduk <kaduk@mit.edu> Reviewed-by:
-
- 30 Aug, 2019 1 commit
-
-
Pauli authored
Reviewed-by:
-
- 29 Aug, 2019 2 commits
-
-
Pauli authored
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:
-
Pauli authored
Reviewed-by:
-
- 27 Aug, 2019 1 commit
-
-
Cesar Pereida Garcia authored
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:
-
- 24 Aug, 2019 1 commit
-
-
Pauli authored
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:
-
- 23 Aug, 2019 1 commit
-
-
Richard Levitte authored
Parsing functions are at liberty to return: 1: when parsing on processing of the parsed value succeeded 0: when the parsed keyword is unknown -1: when the parsed value processing failed Some parsing functions didn't do this quite right, they returned 0 when they should have returned -1, causing a message like this: Line 123: unknown keyword PeerKey When this message (which is displayed when the parsing function returns -1) would have been more appropriate: Line 123: error processing keyword PeerKey = ffdhe2048-2-pub Reviewed-by:
-
- 22 Aug, 2019 1 commit
-
-
Richard Levitte authored
The input reading loop in 'openssl dgst' and 'openssl enc' doesn't check for end of input, and because of the way BIO works, it thereby won't detect that the end is reached before the read is an error. With the FILE BIO, an error occurs when trying to read past EOF, which is fairly much ok, except when the command is used interactively, at least on Unix. The result in that case is that the user has to press Ctrl-D twice for the command to terminate. The issue is further complicated because both these commands use filter BIOs on top of the FILE BIO, so a naïve attempt to check BIO_eof() doesn't quite solve it, since that only checks the state of the source/sink BIO, and the filter BIO may have some buffered data that still needs to be read. Fortunately, there's BIO_pending() that checks exactly that, if any filter BIO has pending data that needs to be processed. We end up having to check both BIO_pending() and BIO_eof(). Thanks to Zsigmond Lőrinczy for the initial effort and inspiration. Fixes #9355 Reviewed-by:
-
- 20 Aug, 2019 2 commits
-
-
Johannes authored
CLA: trivial Reviewed-by:
-
Pauli authored
Improve handling of low entropy at start up from /dev/urandom by waiting for a read(2) call on /dev/random to succeed. Once one such call has succeeded, a shared memory segment is created and persisted as an indicator to other processes that /dev/urandom is properly seeded. This does not fully prevent against attacks weakening the entropy source. An attacker who has control of the machine early in its boot sequence could create the shared memory segment preventing detection of low entropy conditions. However, this is no worse than the current situation. An attacker would also be capable of removing the shared memory segment and causing seeding to reoccur resulting in a denial of service attack. This is partially mitigated by keeping the shared memory alive for the duration of the process's existence. Thus, an attacker would not only need to have called call shmctl(2) with the IPC_RMID command but the system must subsequently enter a state where no instances of libcrypto exist in any process. Even one long running process will prevent this attack. The System V shared memory calls used here go back at least as far as Linux kernel 2.0. Linux kernels 4.8 and later, don't have a reliable way to detect that /dev/urandom has been properly seeded, so a failure is raised for this case (i.e. the getentropy(2) call has already failed). Reviewed-by:
-
- 19 Aug, 2019 4 commits
-
-
Bernd Edlinger authored
Reviewed-by:
-
Rich Salz authored
Backport of https://github.com/openssl/openssl/pull/9602 Reviewed-by:
-
Mykola Baibuz authored
Fixes #9622 CLA: trivial Reviewed-by:
-
Bernd Edlinger authored
Reviewed-by:
-
- 18 Aug, 2019 3 commits
-
-
Patrick Steuer authored
Signed-off-by:
Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by:
-
Patrick Steuer authored
Requesting zero bytes from shake previously led to out-of-bounds write on some platforms. Signed-off-by:
-
Rich Salz authored
Reviewed-by:
-
- 17 Aug, 2019 1 commit
-
-
Bernd Edlinger authored
Reviewed-by:
Kurt Roeckx <kurt@roeckx.be> Reviewed-by:
-
- 15 Aug, 2019 3 commits
-
-
Dr. Matthias St. Pierre authored
Reviewed-by:
-
Omid Najafi authored
The error was from the alignment syntax of the code. More details: https://stackoverflow.com/questions/57316823/arm-assembly-syntax-in-vst-vld-commands?noredirect=1#comment101133590_57316823 CLA: trivial Fixes: #9518 Reviewed-by:
-
Richard Levitte authored
When OpenSSL is configured with 'no-stdio', TEST_ENG_OPENSSL_RC4_P_INIT shouldn't be defined, as that test uses stdio. Fixes #9597 Reviewed-by:
-
- 14 Aug, 2019 2 commits
-
-
Matt Caswell authored
Actually supply a chain and then test: 1) A successful check of both the ee and chain certs 2) A failure to check the ee cert 3) A failure to check a chain cert Reviewed-by:
-
Matt Caswell authored
The function SSL_check_chain() can be used by applications to check that a cert and chain is compatible with the negotiated parameters. This could be useful (for example) from the certificate callback. Unfortunately this function was applying TLSv1.2 sig algs rules and did not work correctly if TLSv1.3 was negotiated. We refactor tls_choose_sigalg to split it up and create a new function find_sig_alg which can (optionally) take a certificate and key as parameters and find an appropriate sig alg if one exists. If the cert and key are not supplied then we try to find a cert and key from the ones we have available that matches the shared sig algs. Reviewed-by:
-
