The branch master has been updated via 8c08c8b37cab0eb66ca74fc65a40af3ccec77c00 (commit) via 50938aec35fd57fb3bec707ead2eee381fcfaf04 (commit) from c9007bda79291179ed2df31b3dfd9f1311102847 (commit)
- Log ----------------------------------------------------------------- commit 8c08c8b37cab0eb66ca74fc65a40af3ccec77c00 Author: Matt Caswell <m...@openssl.org> Date: Mon Nov 15 12:24:05 2021 +0000 Add a test for creating ECX private keys that are too short We expect attempting to create such short keys to fail Reviewed-by: Tomas Mraz <to...@openssl.org> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17041) commit 50938aec35fd57fb3bec707ead2eee381fcfaf04 Author: Matt Caswell <m...@openssl.org> Date: Mon Nov 15 12:14:03 2021 +0000 Don't create an ECX key with short keys If an ECX key is created and the private key is too short, a fromdata call would create the key, and then later detect the error and report it after freeing the key. However freeing the key was calling OPENSSL_secure_clear_free() and assuming that the private key was of the correct length. If it was actually too short this will write over memory that it shouldn't. Fixes #17017 Reviewed-by: Tomas Mraz <to...@openssl.org> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17041) ----------------------------------------------------------------------- Summary of changes: crypto/ec/ecx_backend.c | 25 ++++++++++++++++++------- test/evp_extra_test.c | 24 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/crypto/ec/ecx_backend.c b/crypto/ec/ecx_backend.c index a0144d5a86..2ab7611be9 100644 --- a/crypto/ec/ecx_backend.c +++ b/crypto/ec/ecx_backend.c @@ -70,11 +70,23 @@ int ossl_ecx_key_fromdata(ECX_KEY *ecx, const OSSL_PARAM params[], if (param_pub_key == NULL && param_priv_key == NULL) return 0; - if (param_priv_key != NULL - && !OSSL_PARAM_get_octet_string(param_priv_key, - (void **)&ecx->privkey, ecx->keylen, - &privkeylen)) - return 0; + if (param_priv_key != NULL) { + if (!OSSL_PARAM_get_octet_string(param_priv_key, + (void **)&ecx->privkey, ecx->keylen, + &privkeylen)) + return 0; + if (privkeylen != ecx->keylen) { + /* + * Invalid key length. We will clear what we've received now. We + * can't leave it to ossl_ecx_key_free() because that will call + * OPENSSL_secure_clear_free() and assume the correct key length + */ + OPENSSL_secure_clear_free(ecx->privkey, privkeylen); + ecx->privkey = NULL; + return 0; + } + } + pubkey = ecx->pubkey; if (param_pub_key != NULL @@ -83,8 +95,7 @@ int ossl_ecx_key_fromdata(ECX_KEY *ecx, const OSSL_PARAM params[], sizeof(ecx->pubkey), &pubkeylen)) return 0; - if ((param_pub_key != NULL && pubkeylen != ecx->keylen) - || (param_priv_key != NULL && privkeylen != ecx->keylen)) + if ((param_pub_key != NULL && pubkeylen != ecx->keylen)) return 0; if (param_pub_key == NULL && !ossl_ecx_public_from_private(ecx)) diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index d026ef0c1c..0f280e9f82 100644 --- a/test/evp_extra_test.c +++ b/test/evp_extra_test.c @@ -4231,6 +4231,28 @@ static int test_cipher_with_engine(void) # endif /* OPENSSL_NO_DYNAMIC_ENGINE */ #endif /* OPENSSL_NO_DEPRECATED_3_0 */ +static int ecxnids[] = { + NID_X25519, + NID_X448, + NID_ED25519, + NID_ED448 +}; + +/* Test that creating ECX keys with a short private key fails as expected */ +static int test_ecx_short_keys(int tst) +{ + unsigned char ecxkeydata = 1; + EVP_PKEY *pkey; + + + pkey = EVP_PKEY_new_raw_private_key(ecxnids[tst], NULL, &ecxkeydata, 1); + if (!TEST_ptr_null(pkey)) { + EVP_PKEY_free(pkey); + return 0; + } + return 1; +} + typedef enum OPTION_choice { OPT_ERR = -1, OPT_EOF = 0, @@ -4374,6 +4396,8 @@ int setup_tests(void) # endif #endif + ADD_ALL_TESTS(test_ecx_short_keys, OSSL_NELEM(ecxnids)); + return 1; }