aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2019-09-09Check the DH modulus bit lengthBernd Edlinger4-5/+37
The check was missing in DH_check and DH_check_params. [extended tests] Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9796)
2019-09-09Uniform TEST_*() check usage in test/ectest.cNicola Tuveri1-13/+13
- Replace a `TEST_true()` with `!TEST_false()` to avoid reporting confusing errors - We tend to use `if (!TEST_foo() || !TEST_bar())` and it's a bit confusing to switch to `if(!(TEST_foo() && TEST_bar()))`: replace it with the more common style Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9813)
2019-09-09Fix spacing nit in test/ectest.cNicola Tuveri1-1/+1
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9813)
2019-09-09Fix typos in the OSSL_METHOD_STORE docJakub Zelenka1-5/+5
Reviewed-by: Paul Yang <kaishen.yy@antfin.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9825)
2019-09-09[ec] Match built-in curves on EC_GROUP_new_from_ecparametersNicola Tuveri3-4/+345
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. 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. _______________________________________________________________________________ Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9808)
2019-09-09Do no mention private headers in public headersRichard Levitte4-5/+3
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from https://github.com/openssl/openssl/pull/9812)
2019-09-08Configure: clang: move -Wno-unknown-warning-option to the frontDr. Matthias St. Pierre1-1/+1
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 turned out to be a problem on the 1.0.2 stable branch in the case of the '-Wextended-offsetof' option, which was removed in version 6.0.0, but needs to be kept here in order to support older clang versions, too (see #9446). Incidentally, master and 1.1.1 branch already contained the -Wno-unknown-warning-option option. Due to its special role and its importance, this commit adds an explaining commit message and moves the option to the front. [extended tests] Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9447)
2019-09-07CHANGES entry: for ECC parameters with NULL or zero cofactor, compute itBilly Brumley1-0/+7
This is a forward port from https://github.com/openssl/openssl/pull/9781 of the CHANGES entry for the functionality added in https://github.com/openssl/openssl/pull/9727 (cherry picked from commit 4b965086cb56c24cb5d2197fc04869b95f209a11) Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9797)
2019-09-07[test] computing ECC cofactors: regression testBilly Brumley1-5/+5
This is a forward port from https://github.com/openssl/openssl/pull/9781 for the test logic introduced by https://github.com/openssl/openssl/pull/9727 As @mattcaswell commented (https://github.com/openssl/openssl/pull/9781#discussion_r321621541): > These `TEST_true` calls should be `!TEST_false` because we are > *expecting* a failure. > The difference is that the test framework will print error details if > the test doesn't give the expected answer. > So by using `TEST_true` instead of `!TEST_false` we'll get error > details printed, but the test will succeed anyway. (cherry picked from commit e8aafc891d9bd7fa1cce0401d858ef842f09b49e) Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9797)
2019-09-07Use common digest getter for X942 KDFPauli1-28/+12
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Use common digest getter for SSH KDFPauli1-26/+9
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Use common digest getter for TLS1 PRFPauli1-39/+29
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Use common digest getter for single step KDFPauli1-32/+18
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Use common digest getter for PBKDF2Pauli1-27/+14
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Use common digest getter for HKDFPauli1-30/+14
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Use common define for properties, engine, cipher and digest paramsPauli1-6/+17
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Add 'engine' param to KDFsPauli1-8/+12
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07KMAC using common digest get codePauli1-45/+35
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07HMAC using common digest get codePauli1-82/+16
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07GMAC using common cipher get codePauli1-76/+16
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07CMAC using common cipher get codePauli1-82/+15
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07Unify the digest getting code inside providers.Pauli4-2/+234
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9770)
2019-09-07[ec/ecp_nistp*.c] restyle: use {} around `else` tooNicola Tuveri3-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)
2019-09-07[ec/ecp_nistp*.c] remove flip_endian()Nicola Tuveri3-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)
2019-09-07Uniform BN_bn2binpad() and BN_bn2lebinpad() implementationsNicola Tuveri1-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)
2019-09-07Make BN_num_bits() consttime upon BN_FLG_CONSTTIMENicola Tuveri1-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)
2019-09-07Fix a SCA leak using BN_bn2bin()Nicola Tuveri3-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)
2019-09-06Fix a SCA leak in BN_generate_dsa_nonceBernd Edlinger1-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)
2019-09-06[crypto/rsa] Set the constant-time flag in multi-prime RSA tooCesar Pereida Garcia1-3/+6
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9779)
2019-09-06[crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.Cesar Pereida Garcia2-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)
2019-09-06libcrypto.num entries for KDFsPauli1-7/+13
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Move OSSL_OP_KDF into its rightful place amongst the other OSSL_OP_ definitionsPauli1-1/+1
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Remove reference to legacy aliases for MAC and KDFPauli1-4/+0
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Clear collected_seed after freeing itPauli1-1/+3
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Lowercase command line 'N' argument since params have lower case namesPauli1-1/+1
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Update KDF documentation (section 3)Pauli2-274/+259
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Update KDF documentation (section 1)Pauli2-9/+15
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Update KDF documentation (section 7)Pauli13-974/+811
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Deal with BUF_MEM_grow ambiguityRichard Levitte1-2/+9
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)
2019-09-06crypto/evp/pkey_kdf.c: further special treatment of "seed" and "info"Richard Levitte1-9/+34
pkey_kdf_ctrl_str() has to do the same kind of special treatment as pkey_kdf_ctrl() does. Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06More KDF cleanupRichard Levitte2-22/+31
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)
2019-09-06PBKDF2 implementation: refactor to avoid memleakRichard Levitte1-5/+10
Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Fix memleaks in KDF implementationsRichard Levitte7-11/+9
Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06crypto/evp/pkey_kdf.c: Redo parameter processingRichard Levitte1-35/+83
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)
2019-09-06crypto/evp/kdf_meth.c: Add the reset function to the methodRichard Levitte1-0/+5
Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Update private.num for KDFs/PRFsPauli1-2/+0
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06ossl_provider_library_context(NULL) returns NULL.Pauli1-1/+2
This will only be required until everything is moved to providers and a NULL provider pointer won't be possible. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Params from text to allow zero length value fieldsPauli1-41/+44
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06Update EVP test data for KDFs and PRFs.Pauli1-13/+13
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)
2019-09-06KDF error codes reworkedPauli7-57/+101
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9662)