From 39019c241819a72de8d934787593e43ad3ceb24e Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 21 Sep 2018 08:54:32 -0700 Subject: [PATCH 1/3] Re-order the default supported curve groups by strength. Some TLS servers pick the top choice instead of the strongest. --- src/tls.c | 195 +++++++++++++++++++++++++++--------------------------- 1 file changed, 98 insertions(+), 97 deletions(-) diff --git a/src/tls.c b/src/tls.c index 0b931cc7e..d394261de 100644 --- a/src/tls.c +++ b/src/tls.c @@ -8762,6 +8762,7 @@ static byte* TLSX_QSHKeyFind_Pub(QSHKey* qsh, word16* pubLen, word16 name) ((defined(HAVE_ECC) || defined(HAVE_CURVE25519)) && \ defined(HAVE_SUPPORTED_CURVES)) +/* Populates the default supported groups / curves */ static int TLSX_PopulateSupportedGroups(WOLFSSL* ssl, TLSX** extensions) { int ret = WOLFSSL_SUCCESS; @@ -8786,7 +8787,87 @@ static int TLSX_PopulateSupportedGroups(WOLFSSL* ssl, TLSX** extensions) #endif /* WOLFSSL_TLS13 */ #if defined(HAVE_ECC) && defined(HAVE_SUPPORTED_CURVES) + /* list in order by strength, since not all servers choose by stength */ + #if defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES) + #ifndef NO_ECC_SECP + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP521R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif + #if defined(HAVE_ECC512) || defined(HAVE_ALL_CURVES) + #ifdef HAVE_ECC_BRAINPOOL + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_BRAINPOOLP512R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif + #if defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES) + #ifndef NO_ECC_SECP + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP384R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #ifdef HAVE_ECC_BRAINPOOL + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_BRAINPOOLP384R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif + #if !defined(NO_ECC256) || defined(HAVE_ALL_CURVES) + #ifndef NO_ECC_SECP + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP256R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #ifdef HAVE_ECC_KOBLITZ + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP256K1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #ifdef HAVE_ECC_BRAINPOOL + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_BRAINPOOLP256R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif +#endif /* HAVE_ECC && HAVE_SUPPORTED_CURVES */ + + #ifndef HAVE_FIPS + #if defined(HAVE_CURVE25519) + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_X25519, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif /* HAVE_FIPS */ + +#if defined(HAVE_ECC) && defined(HAVE_SUPPORTED_CURVES) + #if defined(HAVE_ECC224) || defined(HAVE_ALL_CURVES) + #ifndef NO_ECC_SECP + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP224R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #ifdef HAVE_ECC_KOBLITZ + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP224K1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif + #ifndef HAVE_FIPS + #if defined(HAVE_ECC192) || defined(HAVE_ALL_CURVES) + #ifndef NO_ECC_SECP + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP192R1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #ifdef HAVE_ECC_KOBLITZ + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_ECC_SECP192K1, ssl->heap); + if (ret != WOLFSSL_SUCCESS) return ret; + #endif + #endif #if defined(HAVE_ECC160) || defined(HAVE_ALL_CURVES) #ifndef NO_ECC_SECP ret = TLSX_UseSupportedCurve(extensions, @@ -8804,107 +8885,15 @@ static int TLSX_PopulateSupportedGroups(WOLFSSL* ssl, TLSX** extensions) if (ret != WOLFSSL_SUCCESS) return ret; #endif #endif - #if defined(HAVE_ECC192) || defined(HAVE_ALL_CURVES) - #ifndef NO_ECC_SECP - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP192R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #ifdef HAVE_ECC_KOBLITZ - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP192K1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif - #endif - #if defined(HAVE_ECC224) || defined(HAVE_ALL_CURVES) - #ifndef NO_ECC_SECP - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP224R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #ifdef HAVE_ECC_KOBLITZ - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP224K1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif - #if !defined(NO_ECC256) || defined(HAVE_ALL_CURVES) - #ifndef NO_ECC_SECP - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP256R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif -#endif /* HAVE_ECC && HAVE_SUPPORTED_CURVES */ - - #ifndef HAVE_FIPS - #if defined(HAVE_CURVE25519) - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_X25519, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif /* HAVE_FIPS */ - -#if defined(HAVE_ECC) && defined(HAVE_SUPPORTED_CURVES) - #if !defined(NO_ECC256) || defined(HAVE_ALL_CURVES) - #ifdef HAVE_ECC_KOBLITZ - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP256K1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #ifdef HAVE_ECC_BRAINPOOL - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_BRAINPOOLP256R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif - #if defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES) - #ifndef NO_ECC_SECP - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP384R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #ifdef HAVE_ECC_BRAINPOOL - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_BRAINPOOLP384R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif - #if defined(HAVE_ECC512) || defined(HAVE_ALL_CURVES) - #ifdef HAVE_ECC_BRAINPOOL - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_BRAINPOOLP512R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif - #if defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES) - #ifndef NO_ECC_SECP - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_ECC_SECP521R1, ssl->heap); - if (ret != WOLFSSL_SUCCESS) return ret; - #endif - #endif + #endif /* HAVE_FIPS */ #endif /* HAVE_ECC && HAVE_SUPPORTED_CURVES */ #ifdef WOLFSSL_TLS13 if (IsAtLeastTLSv1_3(ssl->version)) { /* Add FFDHE supported groups. */ - #ifdef HAVE_FFDHE_2048 + #ifdef HAVE_FFDHE_8192 ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_FFDHE_2048, ssl->heap); - if (ret != WOLFSSL_SUCCESS) - return ret; - #endif - #ifdef HAVE_FFDHE_3072 - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_FFDHE_3072, ssl->heap); - if (ret != WOLFSSL_SUCCESS) - return ret; - #endif - #ifdef HAVE_FFDHE_4096 - ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_FFDHE_4096, ssl->heap); + WOLFSSL_FFDHE_8192, ssl->heap); if (ret != WOLFSSL_SUCCESS) return ret; #endif @@ -8914,9 +8903,21 @@ static int TLSX_PopulateSupportedGroups(WOLFSSL* ssl, TLSX** extensions) if (ret != WOLFSSL_SUCCESS) return ret; #endif - #ifdef HAVE_FFDHE_8192 + #ifdef HAVE_FFDHE_4096 ret = TLSX_UseSupportedCurve(extensions, - WOLFSSL_FFDHE_8192, ssl->heap); + WOLFSSL_FFDHE_4096, ssl->heap); + if (ret != WOLFSSL_SUCCESS) + return ret; + #endif + #ifdef HAVE_FFDHE_3072 + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_FFDHE_3072, ssl->heap); + if (ret != WOLFSSL_SUCCESS) + return ret; + #endif + #ifdef HAVE_FFDHE_2048 + ret = TLSX_UseSupportedCurve(extensions, + WOLFSSL_FFDHE_2048, ssl->heap); if (ret != WOLFSSL_SUCCESS) return ret; #endif From 24f9f1284494b85882c759b2f637ae72f5a943eb Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 21 Sep 2018 09:27:48 -0700 Subject: [PATCH 2/3] Fix for the curve logic to pick the hightest strength, not just the default 256-bit. Added test for setting user curve. `./examples -H useSupCurve`. --- examples/client/client.c | 31 +++++++++++++++++++++++++------ src/tls.c | 2 +- tests/test.conf | 9 +++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index faf425f0d..0c5e06bea 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -990,6 +990,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) int doSTARTTLS = 0; char* starttlsProt = NULL; int useVerifyCb = 0; + int useSupCurve = 0; #ifdef WOLFSSL_TRUST_PEER_CERT const char* trustCert = NULL; @@ -1088,6 +1089,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) (void)useX25519; (void)helloRetry; (void)onlyKeyShare; + (void)useSupCurve; StackTrap(); @@ -1220,6 +1222,10 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) printf("Verify should fail\n"); myVerifyFail = 1; } + else if (XSTRNCMP(myoptarg, "useSupCurve", 11) == 0) { + printf("Test use supported curve\n"); + useSupCurve = 1; + } else { Usage(); XEXIT_T(MY_EX_USAGE); @@ -1440,6 +1446,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) case 't' : #ifdef HAVE_CURVE25519 useX25519 = 1; + useSupCurve = 1; #if defined(WOLFSSL_TLS13) && defined(HAVE_ECC) onlyKeyShare = 2; #endif @@ -1917,22 +1924,34 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) err_sys("DisableExtendedMasterSecret failed"); } #endif -#if defined(HAVE_CURVE25519) && defined(HAVE_SUPPORTED_CURVES) +#if defined(HAVE_SUPPORTED_CURVES) + #if defined(HAVE_CURVE25519) if (useX25519) { if (wolfSSL_CTX_UseSupportedCurve(ctx, WOLFSSL_ECC_X25519) != WOLFSSL_SUCCESS) { err_sys("unable to support X25519"); } - if (wolfSSL_CTX_UseSupportedCurve(ctx, WOLFSSL_ECC_SECP256R1) - != WOLFSSL_SUCCESS) { - err_sys("unable to support secp256r1"); - } + } + #endif /* HAVE_CURVE25519 */ + #ifdef HAVE_ECC + if (useSupCurve) { + #if !defined(NO_ECC_SECP) && \ + (defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES)) if (wolfSSL_CTX_UseSupportedCurve(ctx, WOLFSSL_ECC_SECP384R1) != WOLFSSL_SUCCESS) { err_sys("unable to support secp384r1"); } + #endif + #if !defined(NO_ECC_SECP) && \ + (!defined(NO_ECC256) || defined(HAVE_ALL_CURVES)) + if (wolfSSL_CTX_UseSupportedCurve(ctx, WOLFSSL_ECC_SECP256R1) + != WOLFSSL_SUCCESS) { + err_sys("unable to support secp256r1"); + } + #endif } -#endif /* HAVE_CURVE25519 && HAVE_SUPPORTED_CURVES */ + #endif /* HAVE_ECC */ +#endif /* HAVE_SUPPORTED_CURVES */ #ifdef WOLFSSL_TLS13 if (noPskDheKe) diff --git a/src/tls.c b/src/tls.c index d394261de..12f3adeab 100644 --- a/src/tls.c +++ b/src/tls.c @@ -3932,7 +3932,7 @@ int TLSX_ValidateSupportedCurves(WOLFSSL* ssl, byte first, byte second) { defSz = octets; } - if (currOid == 0 && ssl->eccTempKeySz == octets) + if (currOid == 0 && ssl->eccTempKeySz <= octets) currOid = oid; if ((nextOid == 0 || nextSz > octets) && ssl->eccTempKeySz <= octets) { nextOid = oid; diff --git a/tests/test.conf b/tests/test.conf index e6f72bfea..a678f52c4 100644 --- a/tests/test.conf +++ b/tests/test.conf @@ -2355,3 +2355,12 @@ -h localhost -A ./certs/test/server-localhost.pem -m + +# server TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 +-v 3 +-l ECDHE-RSA-AES256-GCM-SHA384 + +# client TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 with user curve (384 or 256) +-v 3 +-l ECDHE-RSA-AES256-GCM-SHA384 +-H useSupCurve From 038b5e8a66c0e2d0d0053f500e12377ee253048c Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 24 Sep 2018 07:23:54 -0700 Subject: [PATCH 3/3] Fix comment spelling error. --- src/tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tls.c b/src/tls.c index 12f3adeab..bcf32a13e 100644 --- a/src/tls.c +++ b/src/tls.c @@ -8787,7 +8787,7 @@ static int TLSX_PopulateSupportedGroups(WOLFSSL* ssl, TLSX** extensions) #endif /* WOLFSSL_TLS13 */ #if defined(HAVE_ECC) && defined(HAVE_SUPPORTED_CURVES) - /* list in order by strength, since not all servers choose by stength */ + /* list in order by strength, since not all servers choose by strength */ #if defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES) #ifndef NO_ECC_SECP ret = TLSX_UseSupportedCurve(extensions,