From 393ca1b30cf889a7ab318e7e4a99d0fdb4d390c2 Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Mon, 15 Apr 2019 17:54:23 -0700 Subject: [PATCH 1/9] Increased test suite ciphers buffer size (ticket #5000)) Enhancement to support ECC domain param HEX string or unsigned bin comparison (ticket #5035) --- testsuite/testsuite.c | 2 +- wolfcrypt/src/ecc.c | 68 +++++++++++++++++++++++++++++++++-------- wolfssl/wolfcrypt/ecc.h | 2 ++ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c index e993297cf..ddfd8dd48 100644 --- a/testsuite/testsuite.c +++ b/testsuite/testsuite.c @@ -180,7 +180,7 @@ int testsuite_test(int argc, char** argv) /* show ciphers */ { - char ciphers[1024]; + char ciphers[1024*2]; XMEMSET(ciphers, 0, sizeof(ciphers)); wolfSSL_get_ciphers(ciphers, sizeof(ciphers)-1); printf("ciphers = %s\n", ciphers); diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index def11e8a3..623f78ba1 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3173,11 +3173,11 @@ int wc_ecc_get_curve_id_from_name(const char* curveName) } /* Compares a curve parameter (hex, from ecc_sets[]) to given input - * parameter (byte array) for equality. - * + * parameter for equality. + * encType is WC_TYPE_UNSIGNED_BIN or WC_TYPE_HEX_STR * Returns MP_EQ on success, negative on error */ static int wc_ecc_cmp_param(const char* curveParam, - const byte* param, word32 paramSz) + const byte* param, word32 paramSz, int encType) { int err = MP_OKAY; #ifdef WOLFSSL_SMALL_STACK @@ -3209,9 +3209,12 @@ static int wc_ecc_cmp_param(const char* curveParam, return err; } - if (err == MP_OKAY) - err = mp_read_unsigned_bin(a, param, paramSz); - + if (err == MP_OKAY) { + if (encType == WC_TYPE_HEX_STR) + err = mp_read_radix(a, (char*) param, MP_RADIX_HEX); + else + err = mp_read_unsigned_bin(a, param, paramSz); + } if (err == MP_OKAY) err = mp_read_radix(b, curveParam, MP_RADIX_HEX); @@ -3270,13 +3273,17 @@ int wc_ecc_get_curve_id_from_params(int fieldSize, for (idx = 0; ecc_sets[idx].size != 0; idx++) { if (curveSz == ecc_sets[idx].size) { if ((wc_ecc_cmp_param(ecc_sets[idx].prime, prime, - primeSz) == MP_EQ) && - (wc_ecc_cmp_param(ecc_sets[idx].Af, Af, AfSz) == MP_EQ) && - (wc_ecc_cmp_param(ecc_sets[idx].Bf, Bf, BfSz) == MP_EQ) && + primeSz, WC_TYPE_UNSIGNED_BIN) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Af, Af, AfSz, + WC_TYPE_UNSIGNED_BIN) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Bf, Bf, BfSz, + WC_TYPE_UNSIGNED_BIN) == MP_EQ) && (wc_ecc_cmp_param(ecc_sets[idx].order, order, - orderSz) == MP_EQ) && - (wc_ecc_cmp_param(ecc_sets[idx].Gx, Gx, GxSz) == MP_EQ) && - (wc_ecc_cmp_param(ecc_sets[idx].Gy, Gy, GySz) == MP_EQ) && + orderSz, WC_TYPE_UNSIGNED_BIN) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Gx, Gx, GxSz, + WC_TYPE_UNSIGNED_BIN) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Gy, Gy, GySz, + WC_TYPE_UNSIGNED_BIN) == MP_EQ) && (cofactor == ecc_sets[idx].cofactor)) { break; } @@ -3289,6 +3296,43 @@ int wc_ecc_get_curve_id_from_params(int fieldSize, return ecc_sets[idx].id; } +int wc_ecc_get_curve_id_from_dp_params(const ecc_set_type* dp) +{ + int idx; + + if (dp == NULL) + return BAD_FUNC_ARG; + + if (dp == NULL || dp->prime == NULL || dp->Af == NULL || + dp->Bf == NULL || dp->order == NULL || dp->Gx == NULL || dp->Gy == NULL) + return BAD_FUNC_ARG; + + for (idx = 0; ecc_sets[idx].size != 0; idx++) { + if (dp->size == ecc_sets[idx].size) { + if ((wc_ecc_cmp_param(ecc_sets[idx].prime, (const byte*)dp->prime, + (word32)XSTRLEN(dp->prime), WC_TYPE_HEX_STR) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Af, (const byte*)dp->Af, + (word32)XSTRLEN(dp->Af),WC_TYPE_HEX_STR) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Bf, (const byte*)dp->Bf, + (word32)XSTRLEN(dp->Bf),WC_TYPE_HEX_STR) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].order, (const byte*)dp->order, + (word32)XSTRLEN(dp->order),WC_TYPE_HEX_STR) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Gx, (const byte*)dp->Gx, + (word32)XSTRLEN(dp->Gx),WC_TYPE_HEX_STR) == MP_EQ) && + (wc_ecc_cmp_param(ecc_sets[idx].Gy, (const byte*)dp->Gy, + (word32)XSTRLEN(dp->Gy),WC_TYPE_HEX_STR) == MP_EQ) && + (dp->cofactor == ecc_sets[idx].cofactor)) { + break; + } + } + } + + if (ecc_sets[idx].size == 0) + return ECC_CURVE_INVALID; + + return ecc_sets[idx].id; +} + /* Returns the curve id that corresponds to a given OID, * as listed in ecc_sets[] of ecc.c. * diff --git a/wolfssl/wolfcrypt/ecc.h b/wolfssl/wolfcrypt/ecc.h index 49b6b1acb..3c543a0c4 100644 --- a/wolfssl/wolfcrypt/ecc.h +++ b/wolfssl/wolfcrypt/ecc.h @@ -501,6 +501,8 @@ int wc_ecc_get_curve_id_from_params(int fieldSize, const byte* prime, word32 primeSz, const byte* Af, word32 AfSz, const byte* Bf, word32 BfSz, const byte* order, word32 orderSz, const byte* Gx, word32 GxSz, const byte* Gy, word32 GySz, int cofactor); +WOLFSSL_API +int wc_ecc_get_curve_id_from_dp_params(const ecc_set_type* dp); WOLFSSL_API int wc_ecc_get_curve_id_from_oid(const byte* oid, word32 len); From f8c9285b924bcda169fdde800de75db1c56f0152 Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Mon, 22 Apr 2019 16:30:38 -0700 Subject: [PATCH 2/9] Added a WOLFSSL_CIPHER_LIST_MAX_SIZE macro --- examples/benchmark/tls_bench.c | 7 +++---- examples/client/client.c | 2 +- testsuite/testsuite.c | 2 +- wolfssl/test.h | 1 + 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/benchmark/tls_bench.c b/examples/benchmark/tls_bench.c index c52935ad7..2644fa6f3 100644 --- a/examples/benchmark/tls_bench.c +++ b/examples/benchmark/tls_bench.c @@ -1230,7 +1230,7 @@ static void Usage(void) static void ShowCiphers(void) { - char ciphers[4096]; + char ciphers[WOLFSSL_CIPHER_LIST_MAX_SIZE]; int ret = wolfSSL_get_ciphers(ciphers, (int)sizeof(ciphers)); @@ -1368,12 +1368,11 @@ int bench_tls(void* args) } else { /* Run for each cipher */ - const int ciphersSz = 4096; - ciphers = (char*)XMALLOC(ciphersSz, NULL, DYNAMIC_TYPE_TMP_BUFFER); + ciphers = (char*)XMALLOC(WOLFSSL_CIPHER_LIST_MAX_SIZE, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (ciphers == NULL) { goto exit; } - wolfSSL_get_ciphers(ciphers, ciphersSz); + wolfSSL_get_ciphers(ciphers, (int)sizeof(ciphers)); cipher = ciphers; } diff --git a/examples/client/client.c b/examples/client/client.c index 7dccfacd7..7fe72f57c 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -195,7 +195,7 @@ static int NonBlockingSSL_Connect(WOLFSSL* ssl) static void ShowCiphers(void) { - static char ciphers[4096]; + static char ciphers[WOLFSSL_CIPHER_LIST_MAX_SIZE]; int ret = wolfSSL_get_ciphers(ciphers, (int)sizeof(ciphers)); diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c index ddfd8dd48..d89eb15cc 100644 --- a/testsuite/testsuite.c +++ b/testsuite/testsuite.c @@ -180,7 +180,7 @@ int testsuite_test(int argc, char** argv) /* show ciphers */ { - char ciphers[1024*2]; + char ciphers[WOLFSSL_CIPHER_LIST_MAX_SIZE]; XMEMSET(ciphers, 0, sizeof(ciphers)); wolfSSL_get_ciphers(ciphers, sizeof(ciphers)-1); printf("ciphers = %s\n", ciphers); diff --git a/wolfssl/test.h b/wolfssl/test.h index b9771ad5f..e8a58e6b2 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -155,6 +155,7 @@ #pragma warning(disable:4244 4996) #endif +#define WOLFSSL_CIPHER_LIST_MAX_SIZE 4096 /* Buffer for benchmark tests */ #ifndef TEST_BUFFER_SIZE #define TEST_BUFFER_SIZE 16384 From 6b51f2d5b2dd7196540c98e38456ce46bfd0794d Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Tue, 23 Apr 2019 16:45:52 -0700 Subject: [PATCH 3/9] Added unit test for wc_ecc_get_curve_id_from_dp_params --- tests/api.c | 52 +++++++++++++++++++++++++++++++++++++++++++++----- wolfssl/test.h | 6 ++++-- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/tests/api.c b/tests/api.c index 87efb38b1..6753a45fb 100644 --- a/tests/api.c +++ b/tests/api.c @@ -2107,7 +2107,7 @@ static THREAD_RETURN WOLFSSL_THREAD test_server_loop(void* args) if (cbf != NULL && cbf->ctx_ready != NULL) { cbf->ctx_ready(ctx); } - + while(count != loop_count) { ssl = wolfSSL_new(ctx); if (ssl == NULL) { @@ -2125,7 +2125,7 @@ static THREAD_RETURN WOLFSSL_THREAD test_server_loop(void* args) "Please run from wolfSSL home dir");*/ goto done; } - + #if !defined(NO_FILESYSTEM) && !defined(NO_DH) wolfSSL_SetTmpDH_file(ssl, dhParamFile, WOLFSSL_FILETYPE_PEM); #elif !defined(NO_DH) @@ -2470,7 +2470,7 @@ static void test_client_reuse_WOLFSSLobj(void* args, void *cb, void* server_args if(wolfSSL_KeepHandshakeResources(ssl)) { /* err_sys("SSL_KeepHandshakeResources failed"); */ goto done; - } + } if (sharedCtx && wolfSSL_use_certificate_file(ssl, cliCertFile, WOLFSSL_FILETYPE_PEM) != WOLFSSL_SUCCESS) { /*err_sys("can't load client cert file, " @@ -22701,6 +22701,48 @@ static void test_wc_ecc_get_curve_id_from_name(void) #endif /* HAVE_ECC */ } +#if defined(OPENSSL_EXTRA) +static void test_wc_ecc_get_curve_id_from_dp_params(void) +{ +#ifdef HAVE_ECC + int id; + int curve_id; + int ret = 0; + WOLFSSL_EC_KEY *ecKey; + ecc_key* key; + const ecc_set_type* params; + + printf(testingFmt, "wc_ecc_get_curve_id_from_dp_params"); + + #if !defined(NO_ECC256) && !defined(NO_ECC_SECP) + id = wc_ecc_get_curve_id_from_name("SECP256R1"); + AssertIntEQ(id, ECC_SECP256R1); + #endif + + ecKey = wolfSSL_EC_KEY_new_by_curve_name(id); + AssertNotNull(ecKey); + + ret = wolfSSL_EC_KEY_generate_key(ecKey); + + if (ret == 0) { + /* normal test */ + key = (ecc_key*)ecKey->internal; + params = key->dp; + + curve_id = wc_ecc_get_curve_id_from_dp_params(params); + AssertIntEQ(curve_id, id); + } + + /* invalid case, NULL input*/ + + id = wc_ecc_get_curve_id_from_dp_params(NULL); + AssertIntEQ(id, BAD_FUNC_ARG); + + printf(resultFmt, passed); +#endif /* HAVE_ECC */ +} +#endif /* OPENSSL_EXTRA */ + static void test_wc_ecc_get_curve_id_from_params(void) { #ifdef HAVE_ECC @@ -24344,7 +24386,7 @@ void ApiTest(void) test_wolfSSL_read_write(); #if defined(OPENSSL_EXTRA) && !defined(NO_SESSION_CACHE) && !defined(WOLFSSL_TLS13) test_wolfSSL_reuse_WOLFSSLobj(); -#endif +#endif #endif test_wolfSSL_dtls_export(); AssertIntEQ(test_wolfSSL_SetMinVersion(), WOLFSSL_SUCCESS); @@ -24501,7 +24543,6 @@ void ApiTest(void) test_wc_ecc_get_curve_size_from_name(); test_wc_ecc_get_curve_id_from_name(); test_wc_ecc_get_curve_id_from_params(); - #ifdef WOLFSSL_TLS13 /* TLS v1.3 API tests */ test_tls13_apis(); @@ -24648,6 +24689,7 @@ void ApiTest(void) test_wolfSSL_EVP_get_cipherbynid(); test_wolfSSL_EC(); test_wolfSSL_ECDSA_SIG(); + test_wc_ecc_get_curve_id_from_dp_params(); #endif #ifdef HAVE_HASHDRBG diff --git a/wolfssl/test.h b/wolfssl/test.h index e8a58e6b2..7d2f9ad10 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -155,10 +155,12 @@ #pragma warning(disable:4244 4996) #endif -#define WOLFSSL_CIPHER_LIST_MAX_SIZE 4096 +#ifndef WOLFSSL_CIPHER_LIST_MAX_SIZE + #define WOLFSSL_CIPHER_LIST_MAX_SIZE 4096 +#endif /* Buffer for benchmark tests */ #ifndef TEST_BUFFER_SIZE -#define TEST_BUFFER_SIZE 16384 + #define TEST_BUFFER_SIZE 16384 #endif #ifndef WOLFSSL_HAVE_MIN From edef75c70f628752d1a0d3ded2b498b244f35200 Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Wed, 24 Apr 2019 10:02:20 -0700 Subject: [PATCH 4/9] Wrapped new unit test API with FIPS macros --- tests/api.c | 13 +++++++++---- wolfcrypt/src/ecc.c | 7 +++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/api.c b/tests/api.c index 6753a45fb..0567febcc 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22701,10 +22701,12 @@ static void test_wc_ecc_get_curve_id_from_name(void) #endif /* HAVE_ECC */ } -#if defined(OPENSSL_EXTRA) +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && \ + !defined(HAVE_SELFTEST) && \ + !(defined(HAVE_FIPS) || defined(HAVE_FIPS_VERSION)) + static void test_wc_ecc_get_curve_id_from_dp_params(void) { -#ifdef HAVE_ECC int id; int curve_id; int ret = 0; @@ -22739,9 +22741,8 @@ static void test_wc_ecc_get_curve_id_from_dp_params(void) AssertIntEQ(id, BAD_FUNC_ARG); printf(resultFmt, passed); -#endif /* HAVE_ECC */ } -#endif /* OPENSSL_EXTRA */ +#endif /* defined(OPENSSL_EXTRA) && defined(HAVE_ECC) */ static void test_wc_ecc_get_curve_id_from_params(void) { @@ -24689,6 +24690,10 @@ void ApiTest(void) test_wolfSSL_EVP_get_cipherbynid(); test_wolfSSL_EC(); test_wolfSSL_ECDSA_SIG(); +#endif +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && \ + !defined(HAVE_SELFTEST) && \ + !(defined(HAVE_FIPS) || defined(HAVE_FIPS_VERSION)) test_wc_ecc_get_curve_id_from_dp_params(); #endif diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 623f78ba1..faaf85759 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3296,6 +3296,13 @@ int wc_ecc_get_curve_id_from_params(int fieldSize, return ecc_sets[idx].id; } +/* Returns the curve id in ecc_sets[] that corresponds + * to a given domain parameters pointer. + * + * dp domain parameters pointer + * + * return curve id, from ecc_sets[] on success, negative on error + */ int wc_ecc_get_curve_id_from_dp_params(const ecc_set_type* dp) { int idx; From dfde631cb2f46080e37eb7a7c88f8719205d40e0 Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Wed, 24 Apr 2019 11:49:53 -0700 Subject: [PATCH 5/9] Free key at the end of the test --- tests/api.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/api.c b/tests/api.c index 0567febcc..acc6fa7f8 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22739,6 +22739,7 @@ static void test_wc_ecc_get_curve_id_from_dp_params(void) id = wc_ecc_get_curve_id_from_dp_params(NULL); AssertIntEQ(id, BAD_FUNC_ARG); + wolfSSL_EC_KEY_free(ecKey); printf(resultFmt, passed); } From 06eba2c1e29a16858c0754c01ca4ec8f8c84a0bf Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Thu, 2 May 2019 13:58:55 -0700 Subject: [PATCH 6/9] Removed a redundant check --- wolfcrypt/src/ecc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index faaf85759..0cf7fd0d8 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3307,9 +3307,6 @@ int wc_ecc_get_curve_id_from_dp_params(const ecc_set_type* dp) { int idx; - if (dp == NULL) - return BAD_FUNC_ARG; - if (dp == NULL || dp->prime == NULL || dp->Af == NULL || dp->Bf == NULL || dp->order == NULL || dp->Gx == NULL || dp->Gy == NULL) return BAD_FUNC_ARG; From 3e7a6054a9cd9356d493945f984cd1fdb981e5eb Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Thu, 2 May 2019 14:13:48 -0700 Subject: [PATCH 7/9] Use the macro instead of sizeof() --- examples/benchmark/tls_bench.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/benchmark/tls_bench.c b/examples/benchmark/tls_bench.c index 2644fa6f3..5941d7fc2 100644 --- a/examples/benchmark/tls_bench.c +++ b/examples/benchmark/tls_bench.c @@ -1372,7 +1372,7 @@ int bench_tls(void* args) if (ciphers == NULL) { goto exit; } - wolfSSL_get_ciphers(ciphers, (int)sizeof(ciphers)); + wolfSSL_get_ciphers(ciphers, WOLFSSL_CIPHER_LIST_MAX_SIZE); cipher = ciphers; } From ade8f780a9bb49118e2d2f718b5a0ab263396182 Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Tue, 7 May 2019 11:11:41 -0700 Subject: [PATCH 8/9] simplify to not expect null terminated strings --- wolfcrypt/src/ecc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 0cf7fd0d8..49204f20d 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3190,6 +3190,9 @@ static int wc_ecc_cmp_param(const char* curveParam, if (param == NULL || curveParam == NULL) return BAD_FUNC_ARG; + if (encType == WC_TYPE_HEX_STR) + return XSTRNCMP(curveParam, (char*) param, paramSz); + #ifdef WOLFSSL_SMALL_STACK a = (mp_int*)XMALLOC(sizeof(mp_int), NULL, DYNAMIC_TYPE_ECC); if (a == NULL) @@ -3210,10 +3213,7 @@ static int wc_ecc_cmp_param(const char* curveParam, } if (err == MP_OKAY) { - if (encType == WC_TYPE_HEX_STR) - err = mp_read_radix(a, (char*) param, MP_RADIX_HEX); - else - err = mp_read_unsigned_bin(a, param, paramSz); + err = mp_read_unsigned_bin(a, param, paramSz); } if (err == MP_OKAY) err = mp_read_radix(b, curveParam, MP_RADIX_HEX); From 25aeb8238effe23ad57aaa47e965ab92d0f61c8d Mon Sep 17 00:00:00 2001 From: Tesfa Mael Date: Tue, 4 Jun 2019 16:05:57 -0700 Subject: [PATCH 9/9] Addressed review comment about id being undefined --- tests/api.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/tests/api.c b/tests/api.c index acc6fa7f8..e823d87fa 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22719,22 +22719,21 @@ static void test_wc_ecc_get_curve_id_from_dp_params(void) #if !defined(NO_ECC256) && !defined(NO_ECC_SECP) id = wc_ecc_get_curve_id_from_name("SECP256R1"); AssertIntEQ(id, ECC_SECP256R1); + + ecKey = wolfSSL_EC_KEY_new_by_curve_name(id); + AssertNotNull(ecKey); + + ret = wolfSSL_EC_KEY_generate_key(ecKey); + + if (ret == 0) { + /* normal test */ + key = (ecc_key*)ecKey->internal; + params = key->dp; + + curve_id = wc_ecc_get_curve_id_from_dp_params(params); + AssertIntEQ(curve_id, id); + } #endif - - ecKey = wolfSSL_EC_KEY_new_by_curve_name(id); - AssertNotNull(ecKey); - - ret = wolfSSL_EC_KEY_generate_key(ecKey); - - if (ret == 0) { - /* normal test */ - key = (ecc_key*)ecKey->internal; - params = key->dp; - - curve_id = wc_ecc_get_curve_id_from_dp_params(params); - AssertIntEQ(curve_id, id); - } - /* invalid case, NULL input*/ id = wc_ecc_get_curve_id_from_dp_params(NULL);