summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Fix no-ec2m in ec_curve.c (1.1.0)OpenSSL_1_1_0-stableNicola Tuveri2019-09-121-2/+2
| | | | | | | | | I made a mistake in d4a5dac9f9242c580fb9d0a4389440eccd3494a7 and inverted the GF2m and GFp calls in ec_point_get_affine_coordinates, this fixes it. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9873)
* Prepare for 1.1.0m-devMatt Caswell2019-09-104-4/+12
| | | | Reviewed-by: Richard Levitte <levitte@openssl.org>
* Prepare for 1.1.0l releaseOpenSSL_1_1_0lMatt Caswell2019-09-104-6/+6
| | | | Reviewed-by: Richard Levitte <levitte@openssl.org>
* Update copyright yearMatt Caswell2019-09-1022-22/+22
| | | | | Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9848)
* Remove duplicate CHANGES entry (1.1.0)Matt Caswell2019-09-102-13/+2
| | | | | | Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9845)
* Fix a padding oracle in PKCS7_dataDecode and CMS_decrypt_set1_pkeyBernd Edlinger2019-09-105-5/+45
| | | | | | | | | | | | | | | | | | | | | An attack is simple, if the first CMS_recipientInfo is valid but the second CMS_recipientInfo is chosen ciphertext. If the second recipientInfo decodes to PKCS #1 v1.5 form plaintext, the correct encryption key will be replaced by garbage, and the message cannot be decoded, but if the RSA decryption fails, the correct encryption key is used and the recipient will not notice the attack. As a work around for this potential attack the length of the decrypted key must be equal to the cipher default key length, in case the certifiate is not given and all recipientInfo are tried out. The old behaviour can be re-enabled in the CMS code by setting the CMS_DEBUG_DECRYPT flag. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9777) (cherry picked from commit 5840ed0cd1e6487d247efbc1a04136a41d7b3a37)
* Update CHANGES and NEWS for the new releaseMatt Caswell2019-09-102-1/+19
| | | | | Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9842)
* Use BN_clear_free in DH_set0_keyBernd Edlinger2019-09-091-2/+2
| | | | | | | Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9796) (cherry picked from commit fa01370f7dc8f0a379483bbe74de11225857e5fe)
* [ec] Match built-in curves on EC_GROUP_new_from_ecparametersNicola Tuveri2019-09-094-5/+236
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Description ----------- Upon `EC_GROUP_new_from_ecparameters()` check if the parameters match any of the built-in curves. If that is the case, return a new `EC_GROUP_new_by_curve_name()` object instead of the explicit parameters `EC_GROUP`. This affects all users of `EC_GROUP_new_from_ecparameters()`: - direct calls to `EC_GROUP_new_from_ecparameters()` - direct calls to `EC_GROUP_new_from_ecpkparameters()` with an explicit parameters argument - ASN.1 parsing of explicit parameters keys (as it eventually ends up calling `EC_GROUP_new_from_ecpkparameters()`) A parsed explicit parameter key will still be marked with the `OPENSSL_EC_EXPLICIT_CURVE` ASN.1 flag on load, so, unless programmatically forced otherwise, if the key is eventually serialized the output will still be encoded with explicit parameters, even if internally it is treated as a named curve `EC_GROUP`. Before this change, creating any `EC_GROUP` object using `EC_GROUP_new_from_ecparameters()`, yielded an object associated with the default generic `EC_METHOD`, but this was never guaranteed in the documentation. After this commit, users of the library that intentionally want to create an `EC_GROUP` object using a specific `EC_METHOD` can still explicitly call `EC_GROUP_new(foo_method)` and then manually set the curve parameters using `EC_GROUP_set_*()`. Motivation ---------- This has obvious performance benefits for the built-in curves with specialized `EC_METHOD`s and subtle but important security benefits: - the specialized methods have better security hardening than the generic implementations - optional fields in the parameter encoding, like the `cofactor`, cannot be leveraged by an attacker to force execution of the less secure code-paths for single point scalar multiplication - in general, this leads to reducing the attack surface Check the manuscript at https://arxiv.org/abs/1909.01785 for an in depth analysis of the issues related to this commit. It should be noted that `libssl` does not allow to negotiate explicit parameters (as per RFC 8422), so it is not directly affected by the consequences of using explicit parameters that this commit fixes. On the other hand, we detected external applications and users in the wild that use explicit parameters by default (and sometimes using 0 as the cofactor value, which is technically not a valid value per the specification, but is tolerated by parsers for wider compatibility given that the field is optional). These external users of `libcrypto` are exposed to these vulnerabilities and their security will benefit from this commit. Related commits --------------- While this commit is beneficial for users using built-in curves and explicit parameters encoding for serialized keys, commit b783beeadf6b80bc431e6f3230b5d5585c87ef87 (and its equivalents for the 1.0.2, 1.1.0 and 1.1.1 stable branches) fixes the consequences of the invalid cofactor values more in general also for other curves (CVE-2019-1547). The following list covers commits in `master` that are related to the vulnerabilities presented in the manuscript motivating this commit: - d2baf88c43 [crypto/rsa] Set the constant-time flag in multi-prime RSA too - 311e903d84 [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation. - b783beeadf [crypto/ec] for ECC parameters with NULL or zero cofactor, compute it - 724339ff44 Fix SCA vulnerability when using PVK and MSBLOB key formats Note that the PRs that contributed the listed commits also include other commits providing related testing and documentation, in addition to links to PRs and commits backporting the fixes to the 1.0.2, 1.1.0 and 1.1.1 branches. This commit includes a partial backport of https://github.com/openssl/openssl/pull/8555 (commit 8402cd5f75f8c2f60d8bd39775b24b03dd8b3b38) for which the main author is Shane Lontis. Responsible Disclosure ---------------------- This and the other issues presented in https://arxiv.org/abs/1909.01785 were reported by Cesar Pereida García, Sohaib ul Hassan, Nicola Tuveri, Iaroslav Gridin, Alejandro Cabrera Aldaya and Billy Bob Brumley from the NISEC group at Tampere University, FINLAND. The OpenSSL Security Team evaluated the security risk for this vulnerability as low, and encouraged to propose fixes using public Pull Requests. _______________________________________________________________________________ Co-authored-by: Shane Lontis <shane.lontis@oracle.com> (Backport from https://github.com/openssl/openssl/pull/9808) Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9810)
* Configure: clang: add -Wno-unknown-warning-optionDr. Matthias St. Pierre2019-09-081-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | Fixes travis build errors due to clang error: unknown warning option '-Wno-extended-offsetof' It seems like '-Wextended-offsetof' was removed from clang in version 6.0.0, (see [1], [2]). While gcc ignores unknown options of the type '-Wno-xxx', clang by default issues a warning [-Wunknown-warning-option] (see [3]), which together with '-Werror' causes the build to fail. This commit adds the '-Wno-unknown-warning-option' option to make clang behave more relaxed like gcc. [1] https://reviews.llvm.org/D40267 [2] https://github.com/llvm/llvm-project/commit/52a3ca9e2909 [3] https://clang.llvm.org/docs/DiagnosticsReference.html#wunknown-warning-option [extended tests] Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9804)
* [test/recipes/30-test_evp_data] computing ECC cofactors: regression testBilly Brumley2019-09-071-0/+50
| | | | | | Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9795)
* [crypto/ec] for ECC parameters with NULL or zero cofactor, compute itBilly Brumley2019-09-074-8/+105
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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/9795)
* [ec/ecp_nistp*.c] restyle: use {} around `else` tooNicola Tuveri2019-09-073-18/+18
| | | | | | | | Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511) (cherry picked from commit 4fe2ee3a449a8ca2886584e221f34ff0ef5de119)
* [ec/ecp_nistp*.c] remove flip_endian()Nicola Tuveri2019-09-073-84/+62
| | | | | | | | | | | Replace flip_endian() by using the little endian specific BN_bn2lebinpad() and BN_lebin2bn(). Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511) (cherry picked from commit e0b660c27d8d97b4ad9e2098cc957de26872c0ef)
* Uniform BN_bn2binpad() and BN_bn2lebinpad() implementationsNicola Tuveri2019-09-071-20/+17
| | | | | | | | Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511) (cherry picked from commit 1b338abe3abb8c73f004c34d4b8a9272b89dfd5d)
* Make BN_num_bits() consttime upon BN_FLG_CONSTTIMENicola Tuveri2019-09-071-0/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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) (cherry picked from commit 8b44198b916015f77bef1befa26edb48ad8a0238)
* Fix a SCA leak using BN_bn2bin()Nicola Tuveri2019-09-073-21/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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) (cherry picked from commit 805315d3a20f7274195eed75b06c391dacf3b197)
* Fix a SCA leak in BN_generate_dsa_nonceBernd Edlinger2019-09-061-4/+1
| | | | | | | | Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> (Merged from https://github.com/openssl/openssl/pull/9782) (cherry picked from commit 31ca19403d56ad71d823cf62990518dfc6905bb4)
* [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.Cesar Pereida Garcia2019-09-062-3/+20
| | | | | | | | | | | | | | 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) (cherry picked from commit 311e903d8468e2a380d371609a10eda71de16c0e)
* Fix SCA vulnerability when using PVK and MSBLOB key formatsCesar Pereida Garcia2019-08-271-0/+3
| | | | | | | | | | | | | | | | | 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) (cherry picked from commit 724339ff44235149c4e8ddae614e1dda6863e23e)
* Fix error handling in X509_chain_up_refBernd Edlinger2019-08-171-1/+9
| | | | | | | | Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/9614) (cherry picked from commit cae665dfa6ccec743a7f39cf80676d7d2d787e56)
* Makefile.shared: fix to allow strings and spaces in passed variablesRichard Levitte2019-07-271-58/+58
| | | | | | | | | | | | | | | | | | | | | The previous change for mingw, which now defaults to OPENSSLDIR and ENGINESDIR definitions that include a space, a long standing issue was revealed again; our builds for Unix like environment were never very tolerant of spaces in these definitions, because the quotes were interpreted along the way. New analysis of Makefile.shared showed that our use of quotes in there wasn't quite right. A lot of double quotes could safely be replaced with single quotes, thus protecting the diverse values we pass down to this build file (remember that make variables are expanded before passing the command to the shell, unconditionally), reserving double quotes to the places where absolutely needed (to protect the expansion of shell variables to commands). CVE-2019-1552 Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9469)
* Fix default installation paths on mingwRichard Levitte2019-07-254-1/+120
| | | | | | | | | | | | | | | | | | | | | | | | Mingw config targets assumed that resulting programs and libraries are installed in a Unix-like environment and the default installation prefix was therefore set to '/usr/local'. However, mingw programs are installed in a Windows environment, and the installation directories should therefore have Windows defaults, i.e. the same kind of defaults as the VC config targets. A difficulty is, however, that a "cross compiled" build can't figure out the system defaults from environment the same way it's done when building "natively", so we have to fall back to hard coded defaults in that case. Tests can still be performed when cross compiled on a non-Windows platform, since all tests only depend on the source and build directory, and otherwise relies on normal local paths. CVE-2019-1552 Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9460)
* Add value_barriers in constant time select functionsBernd Edlinger2019-07-191-1/+19
| | | | | | | | | | The barriers prevent the compiler from narrowing down the possible value range of the mask and ~mask in the select statements, which avoids the recognition of the select and turning it into a conditional load or branch. Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9418)
* Fix wrong lock claimed in x509 dir lookup.Krists Krilovs2019-07-151-2/+2
| | | | | | | | | | | | | | | x509 store's objects cache can get corrupted when using dir lookup method in multithreaded application. Claim x509 store's lock when accessing objects cache. CLA: trivial Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9326)
* Avoid NULL pointer dereference.Pauli2019-07-084-3/+19
| | | | | | | | | [manual merge from #9059 to 1.1.0] Fixes: #9043 Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> (Merged from https://github.com/openssl/openssl/pull/9322)
* Prepare for 1.1.0l-devRichard Levitte2019-05-284-4/+12
| | | | Reviewed-by: Matt Caswell <matt@openssl.org>
* Prepare for 1.1.0k releaseOpenSSL_1_1_0kRichard Levitte2019-05-284-6/+6
| | | | Reviewed-by: Matt Caswell <matt@openssl.org>
* Update copyright yearRichard Levitte2019-05-2823-23/+23
| | | | | Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9033)
* Add CHANGES and NEWS for 1.1.0kRichard Levitte2019-05-272-1/+32
| | | | | Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9018)
* Change default RSA, DSA and DH size to 2048 bitKurt Roeckx2019-05-215-10/+16
| | | | | | | | | Fixes: #8737 Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Richard Levitte <levitte@openssl.org> GH: #8741 (cherry picked from commit 70b0b977f73cd70e17538af3095d18e0cf59132e)
* fixed public range check in ec_GF2m_simple_oct2pointShane Lontis2019-03-301-4/+5
| | | | | | | | Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/8607) (cherry picked from commit cad8347be23c5e0c0d9eea02d090d42daf2dd7a9)
* Modify the RSA_private_decrypt functions to check the padding inBernd Edlinger2019-03-223-48/+48
| | | | | | | | | | | | constant time with a memory access pattern that does not depend on secret information. [extended tests] Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/8543) (cherry picked from commit 9c0cf214e7836eb5aaf1ea5d3cbf6720533f86b5)
* Make err_clear_constant_time really constant timeBernd Edlinger2019-03-223-27/+25
| | | | | | | | | [extended tests] Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/8542) (cherry picked from commit 94dc53a3f7549040dd9e61a25485070c14b41c49)
* Clear the point S before freeing in ec_mul_consttimeBernd Edlinger2019-03-181-1/+1
| | | | | | | | The secret point R can be recovered from S using the equation R = S - P. The X and Z coordinates should be sufficient for that. Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/8505)
* Clear the secret point in ecdh_simple_compute_keyBernd Edlinger2019-03-181-1/+1
| | | | | | | Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/8501) (cherry picked from commit 1ff2c992c24c330c0d40708b4169b862563d6aab)
* Fix memory overrun in rsa padding check functionsBernd Edlinger2019-03-073-41/+44
| | | | | | | | | Fixes #8364 and #8357 Reviewed-by: Kurt Roeckx <kurt@roeckx.be> (Merged from https://github.com/openssl/openssl/pull/8365) (cherry picked from commit d7f5e5ae6d53f1387a42d210806cf5e9ed0882d6)
* Avoid an underflow in ecp_nistp521.cMatt Caswell2019-03-071-3/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function felem_diff_128_64 in ecp_nistp521.c substracts the number |in| from |out| mod p. In order to avoid underflow it first adds 32p mod p (which is equivalent to 0 mod p) to |out|. The comments and variable naming suggest that the original author intended to add 64p mod p. In fact it has been shown that with certain unusual co-ordinates it is possible to cause an underflow in this function when only adding 32p mod p while performing a point double operation. By changing this to 64p mod p the underflow is avoided. It turns out to be quite difficult to construct points that satisfy the underflow criteria although this has been done and the underflow demonstrated. However none of these points are actually on the curve. Finding points that satisfy the underflow criteria and are also *on* the curve is considered significantly more difficult. For this reason we do not believe that this issue is currently practically exploitable and therefore no CVE has been assigned. This only impacts builds using the enable-ec_nistp_64_gcc_128 Configure option. With thanks to Bo-Yin Yang, Billy Brumley and Dr Liu for their significant help in investigating this issue. Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> (Merged from https://github.com/openssl/openssl/pull/8405) (cherry picked from commit 13fbce17fc9f02e2401fc3868f3f8e02d6647e5f)
* Test an overlong ChaCha20-Poly1305 nonceMatt Caswell2019-03-061-0/+8
| | | | | | | | Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8406) (cherry picked from commit a4f0b50eafb256bb802f2724fc7f7580fb0fbabc)
* Prevent over long nonces in ChaCha20-Poly1305Matt Caswell2019-03-061-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ChaCha20-Poly1305 is an AEAD cipher, and requires a unique nonce input for every encryption operation. RFC 7539 specifies that the nonce value (IV) should be 96 bits (12 bytes). OpenSSL allows a variable nonce length and front pads the nonce with 0 bytes if it is less than 12 bytes. However it also incorrectly allows a nonce to be set of up to 16 bytes. In this case only the last 12 bytes are significant and any additional leading bytes are ignored. It is a requirement of using this cipher that nonce values are unique. Messages encrypted using a reused nonce value are susceptible to serious confidentiality and integrity attacks. If an application changes the default nonce length to be longer than 12 bytes and then makes a change to the leading bytes of the nonce expecting the new value to be a new unique nonce then such an application could inadvertently encrypt messages with a reused nonce. Additionally the ignored bytes in a long nonce are not covered by the integrity guarantee of this cipher. Any application that relies on the integrity of these ignored leading bytes of a long nonce may be further affected. Any OpenSSL internal use of this cipher, including in SSL/TLS, is safe because no such use sets such a long nonce value. However user applications that use this cipher directly and set a non-default nonce length to be longer than 12 bytes may be vulnerable. CVE-2019-1543 Fixes #8345 Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8406) (cherry picked from commit 2a3d0ee9d59156c48973592331404471aca886d6)
* Clarify that SSL_shutdown() must not be called after a fatal errorMatt Caswell2019-02-262-5/+12
| | | | | | Follow on from CVE-2019-1559 Reviewed-by: Richard Levitte <levitte@openssl.org>
* Go into the error state if a fatal alert is sent or receivedMatt Caswell2019-02-264-3/+9
| | | | | | | | | | 1.1.0 is not impacted by CVE-2019-1559, but this commit is a follow on from that. That CVE was a result of applications calling SSL_shutdown after a fatal alert has occurred. By chance 1.1.0 is not vulnerable to that issue, but this change is additional hardening to prevent other similar issues. Reviewed-by: Richard Levitte <levitte@openssl.org>
* Ensure bn_cmp_words can handle the case where n == 0Matt Caswell2019-02-251-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Thanks to David Benjamin who reported this, performed the analysis and suggested the patch. I have incorporated some of his analysis in the comments below. This issue can cause an out-of-bounds read. It is believed that this was not reachable until the recent "fixed top" changes. Analysis has so far only identified one code path that can encounter this - although it is possible that others may be found. The one code path only impacts 1.0.2 in certain builds. The fuzzer found a path in RSA where iqmp is too large. If the input is all zeros, the RSA CRT logic will multiply a padded zero by iqmp. Two mitigating factors: - Private keys which trip this are invalid (iqmp is not reduced mod p). Only systems which take untrusted private keys care. - In OpenSSL 1.1.x, there is a check which rejects the oversize iqmp, so the bug is only reproducible in 1.0.2 so far. Fortunately, the bug appears to be relatively harmless. The consequences of bn_cmp_word's misbehavior are: - OpenSSL may crash if the buffers are page-aligned and the previous page is non-existent. - OpenSSL will incorrectly treat two BN_ULONG buffers as not equal when they are equal. - Side channel concerns. The first is indeed a concern and is a DoS bug. The second is fine in this context. bn_cmp_word and bn_cmp_part_words are used to compute abs(a0 - a1) in Karatsuba. If a0 = a1, it does not matter whether we use a0 - a1 or a1 - a0. The third would be worth thinking about, but it is overshadowed by the entire Karatsuba implementation not being constant time. Due to the difficulty of tripping this and the low impact no CVE is felt necessary for this issue. Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Viktor Dukhovni <viktor@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8326) (cherry picked from commit 576129cd72ae054d246221f111aabf42b9c6d76d)
* apps/speed: fix segfault while looking up algorithm nameJeff Mahoney2019-02-241-1/+0
| | | | | | | | | | | | | | | | | | The backport of master commit 5c6a69f539a (apps/speed: fix possible OOB access in some EC arrays) as 1.1.0 commit 4e07941373a introduced a regression. The ecdh_choices array is iterated using an element count but is NULL terminated. This means that running 'openssl speed somealgo' will result in a segfault when opt_found hits the NULL entry. Fixes #8243 CLA: trivial Signed-off-by: Jeff Mahoney <jeffm@suse.com> Reviewed-by: Paul Yang <yang.yang@baishancloud.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8244)
* Clear BN_FLG_CONSTTIME on BN_CTX_get()Nicola Tuveri2019-02-211-1/+3
| | | | | | | (cherry picked from commit c8147d37ccaaf28c430d3fb45a14af36597e48b8) Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8294)
* Test for constant-time flag leakage in BN_CTXNicola Tuveri2019-02-211-3/+108
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a simple unit test to make sure that the constant-time flag does not "leak" among BN_CTX frames: - test_ctx_consttime_flag() initializes (and later frees before returning) a BN_CTX object, then it calls in sequence test_ctx_set_ct_flag() and test_ctx_check_ct_flag() using the same BN_CTX object. The process is run twice, once with a "normal" BN_CTX_new() object, then with a BN_CTX_secure_new() one. - test_ctx_set_ct_flag() starts a frame in the given BN_CTX and sets the BN_FLG_CONSTTIME flag on some of the BIGNUMs obtained from the frame before ending it. - test_ctx_check_ct_flag() then starts a new frame and gets a number of BIGNUMs from it. In absence of leaks, none of the BIGNUMs in the new frame should have BN_FLG_CONSTTIME set. In actual BN_CTX usage inside libcrypto the leak could happen at any depth level in the BN_CTX stack, with varying results depending on the patterns of sibling trees of nested function calls sharing the same BN_CTX object, and the effect of unintended BN_FLG_CONSTTIME on the called BN_* functions. This simple unit test abstracts away this complexity and verifies that the leak does not happen between two sibling functions sharing the same BN_CTX object at the same level of nesting. (manually cherry picked from commit fe16ae5f95fa86ddb049a8d1e2caee0b80b32282) Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8294)
* [test] unit test for field_inv function pointer in EC_METHODNicola Tuveri2019-02-203-0/+248
| | | | | | | | | | This is a rewrite of commit 8f58ede09572dcc6a7e6c01280dd348240199568 for the 1.1.0-stable branch. Co-authored-by: Billy Brumley <bbrumley@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8263)
* SCA hardening for mod. field inversion in EC_GROUPBilly Brumley2019-02-2012-74/+214
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a dedicated function in `EC_METHOD` to access a modular field inversion implementation suitable for the specifics of the implemented curve, featuring SCA countermeasures. The new pointer is defined as: `int (*field_inv)(const EC_GROUP*, BIGNUM *r, const BIGNUM *a, BN_CTX*)` and computes the multiplicative inverse of `a` in the underlying field, storing the result in `r`. Three implementations are included, each including specific SCA countermeasures: - `ec_GFp_simple_field_inv()`, featuring SCA hardening through blinding. - `ec_GFp_mont_field_inv()`, featuring SCA hardening through Fermat's Little Theorem (FLT) inversion. - `ec_GF2m_simple_field_inv()`, that uses `BN_GF2m_mod_inv()` which already features SCA hardening through blinding. From a security point of view, this also helps addressing a leakage previously affecting conversions from projective to affine coordinates. This commit also adds a new error reason code (i.e., `EC_R_CANNOT_INVERT`) to improve consistency between the three implementations as all of them could fail for the same reason but through different code paths resulting in inconsistent error stack states. Co-authored-by: Nicola Tuveri <nic.tuv@gmail.com> (cherry picked from commit e0033efc30b0f00476bba8f0fa5512be5dc8a3f1) Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> (Merged from https://github.com/openssl/openssl/pull/8263)
* cygwin: drop explicit O_TEXTCorinna Vinschen2019-02-181-4/+5
| | | | | | | | | | | Cygwin binaries should not enforce text mode these days, just use text mode if the underlying mount point requests it Signed-off-by: Corinna Vinschen <vinschen@redhat.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8275)
* crypto/engine/eng_cryptodev.c: fix bignum<->crp conversionRichard Levitte2019-02-111-10/+4
| | | | | | | | | | | | | | bn2crparam() incorrectly delivered a big endian byte string to cryptodev. Using BN_bn2lebinpad() instead of BN_bn2bin() fixes this. crparam2bn() had a hack that avoided this issue in the other direction, but allocated an intermediary chunk of memory to get correct endianness. Using BN_lebin2bn() avoids this allocation. Fixes #8202 Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/8204)
View Lorry Controller Status