From 0bfa399b6cfb8e4a300e2eb3ef7fcea72a9b255a Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Tue, 9 Jan 2018 16:13:46 -0700 Subject: [PATCH 1/2] fix check key pair match with ECC --- tests/api.c | 32 +++++++++++++++++++++++++++++++- wolfcrypt/src/asn.c | 39 ++++++++++++++++++++++++++++++++------- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/tests/api.c b/tests/api.c index e91fde627..794ae541b 100644 --- a/tests/api.c +++ b/tests/api.c @@ -13327,7 +13327,7 @@ static void test_wolfSSL_ASN1_TIME_print() static void test_wolfSSL_private_keys(void) { #if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && \ - !defined(NO_FILESYSTEM) && !defined(NO_RSA) + !defined(NO_FILESYSTEM) WOLFSSL* ssl; WOLFSSL_CTX* ctx; EVP_PKEY* pkey = NULL; @@ -13337,6 +13337,7 @@ static void test_wolfSSL_private_keys(void) OpenSSL_add_all_digests(); OpenSSL_add_all_algorithms(); +#ifndef NO_RSA AssertNotNull(ctx = SSL_CTX_new(wolfSSLv23_server_method())); AssertTrue(SSL_CTX_use_certificate_file(ctx, svrCertFile, WOLFSSL_FILETYPE_PEM)); AssertTrue(SSL_CTX_use_PrivateKey_file(ctx, svrKeyFile, WOLFSSL_FILETYPE_PEM)); @@ -13376,12 +13377,41 @@ static void test_wolfSSL_private_keys(void) EVP_PKEY_free(pkey); SSL_free(ssl); /* frees x509 also since loaded into ssl */ SSL_CTX_free(ctx); +#endif /* end of RSA private key match tests */ + + +#ifdef HAVE_ECC + AssertNotNull(ctx = SSL_CTX_new(wolfSSLv23_server_method())); + AssertTrue(SSL_CTX_use_certificate_file(ctx, eccCertFile, + WOLFSSL_FILETYPE_PEM)); + AssertTrue(SSL_CTX_use_PrivateKey_file(ctx, eccKeyFile, + WOLFSSL_FILETYPE_PEM)); + AssertNotNull(ssl = SSL_new(ctx)); + + AssertIntEQ(wolfSSL_check_private_key(ssl), WOLFSSL_SUCCESS); + SSL_free(ssl); + + + AssertTrue(SSL_CTX_use_PrivateKey_file(ctx, cliEccKeyFile, + WOLFSSL_FILETYPE_PEM)); + AssertNotNull(ssl = SSL_new(ctx)); + + AssertIntNE(wolfSSL_check_private_key(ssl), WOLFSSL_SUCCESS); + + SSL_free(ssl); + SSL_CTX_free(ctx); +#endif /* end of ECC private key match tests */ + /* test existence of no-op macros in wolfssl/openssl/ssl.h */ CONF_modules_free(); ENGINE_cleanup(); CONF_modules_unload(); + (void)ssl; + (void)ctx; + (void)pkey; + printf(resultFmt, passed); #endif /* defined(OPENSSL_EXTRA) && !defined(NO_CERTS) */ } diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 9ebbb0112..4981d5555 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -1803,23 +1803,48 @@ int wc_CheckPrivateKey(byte* key, word32 keySz, DecodedCert* der) #ifdef HAVE_ECC if (der->keyOID == ECDSAk) { - word32 keyIdx = 0; ecc_key key_pair; + byte* privDer; + word32 privSz; + word32 keyIdx = 0; if ((ret = wc_ecc_init(&key_pair)) < 0) return ret; + if ((ret = wc_EccPrivateKeyDecode(key, &keyIdx, &key_pair, keySz)) == 0) { WOLFSSL_MSG("Checking ECC key pair"); - keyIdx = 0; - if ((ret = wc_ecc_import_x963(der->publicKey, der->pubKeySize, - &key_pair)) == 0) { - /* public and private extracted successfuly no check if is + + if ((privSz = wc_ecc_size(&key_pair)) <= 0) { + return WC_KEY_SIZE_E; + } + + privDer = (byte*)XMALLOC(privSz, der->heap, DYNAMIC_TYPE_KEY); + if (privDer == NULL) { + return MEMORY_E; + } + + if ((ret = wc_ecc_export_private_only(&key_pair, privDer, &privSz)) + == 0) { + wc_ecc_free(&key_pair); + ret = wc_ecc_init(&key_pair); + if (ret == 0) { + ret = wc_ecc_import_private_key((const byte*)privDer, + privSz, (const byte*)der->publicKey, + der->pubKeySize, &key_pair); + } + + /* public and private extracted successfuly now check if is * a pair and also do sanity checks on key. wc_ecc_check_key * checks that private * base generator equals pubkey */ - if ((ret = wc_ecc_check_key(&key_pair)) == 0) - ret = 1; + if (ret == 0) { + if ((ret = wc_ecc_check_key(&key_pair)) == 0) { + ret = 1; + } + } } + XFREE(privDer, der->heap, DYNAMIC_TYPE_KEY); + } wc_ecc_free(&key_pair); } From 59b9ab9097517fd180ad883a6ece31716e988254 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 10 Jan 2018 13:36:03 -0700 Subject: [PATCH 2/2] place buffer on stack instead and zero it when done --- wolfcrypt/src/asn.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 4981d5555..d79fbda85 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -1804,8 +1804,8 @@ int wc_CheckPrivateKey(byte* key, word32 keySz, DecodedCert* der) #ifdef HAVE_ECC if (der->keyOID == ECDSAk) { ecc_key key_pair; - byte* privDer; - word32 privSz; + byte privDer[MAX_ECC_BYTES]; + word32 privSz = MAX_ECC_BYTES; word32 keyIdx = 0; if ((ret = wc_ecc_init(&key_pair)) < 0) @@ -1815,15 +1815,6 @@ int wc_CheckPrivateKey(byte* key, word32 keySz, DecodedCert* der) keySz)) == 0) { WOLFSSL_MSG("Checking ECC key pair"); - if ((privSz = wc_ecc_size(&key_pair)) <= 0) { - return WC_KEY_SIZE_E; - } - - privDer = (byte*)XMALLOC(privSz, der->heap, DYNAMIC_TYPE_KEY); - if (privDer == NULL) { - return MEMORY_E; - } - if ((ret = wc_ecc_export_private_only(&key_pair, privDer, &privSz)) == 0) { wc_ecc_free(&key_pair); @@ -1842,9 +1833,8 @@ int wc_CheckPrivateKey(byte* key, word32 keySz, DecodedCert* der) ret = 1; } } + ForceZero(privDer, privSz); } - XFREE(privDer, der->heap, DYNAMIC_TYPE_KEY); - } wc_ecc_free(&key_pair); }