From 4feedb72cc6a72cf63fc430da486b15a4ce4df62 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Thu, 20 May 2021 19:14:35 +0900 Subject: [PATCH 1/7] simulate set_ciphersuites comp. API --- src/ssl.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++- tests/api.c | 16 +++++ 2 files changed, 195 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 579e13fcc..34b9cd45f 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11943,6 +11943,153 @@ static int wolfSSL_remove_ciphers(char* list, int sz, const char* toRemove) return totalSz; } +/* */ +/* build enabled cipher list w/ TLS13 or w/o TLS13 suites */ +/* @param ctx a pointer to WOLFSSL_CTX structure */ +/* @param suites currently enabled suites */ +/* @param onlytlsv13suites flag whether correcting w/ TLS13 suites */ +/* or w/o TLS13 suties */ +/* @param list suites list that user wants to update */ +/* @return suites list on successs, otherwise NULL */ +static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, + int onlytlsv13suites, const char* list) +{ + int idx = 0; + int listsz = 0; + int len = 0; + int ianasz = 0; + const char* enabledcs = NULL; + char* locallist = NULL; + char* head = NULL; + byte cipherSuite0; + byte cipherSuite; + + /* sanity check */ + if (ctx == NULL || suites == NULL || list == NULL) + return NULL; + + if (!suites->setSuites) + return NULL; + + listsz = XSTRLEN(list); + + /* calculate necessary buffer length */ + for(idx = 0; idx < suites->suiteSz; idx++) { + + cipherSuite0 = suites->suites[idx]; + cipherSuite = suites->suites[++idx]; + + if (onlytlsv13suites && cipherSuite0 == TLS13_BYTE) { + enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); + } else if (!onlytlsv13suites && cipherSuite0 != TLS13_BYTE) { + enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); + } else + continue; + + if (XSTRNCMP(enabledcs, "None", XSTRLEN(enabledcs)) != 0) { + len += XSTRLEN(enabledcs) + 2; + } + } + + len += listsz + 1; + + /* build string */ + if (len > 0) { + locallist = (char*)XMALLOC(len, ctx->heap, + DYNAMIC_TYPE_TMP_BUFFER); + /* sanity check */ + if (!locallist) + return NULL; + + XMEMSET(locallist, 0, len); + + head = locallist; + + for(idx = 0; idx < suites->suiteSz; idx++) { + cipherSuite0 = suites->suites[idx]; + cipherSuite = suites->suites[++idx]; + + if (onlytlsv13suites && cipherSuite0 == TLS13_BYTE) { + enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); + } else if (!onlytlsv13suites && cipherSuite0 != TLS13_BYTE) { + enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); + } else + continue; + + ianasz = (int)XSTRLEN(enabledcs); + if (ianasz + 1 < len) { + XSTRNCPY(locallist, enabledcs, len); + locallist += ianasz; + + *locallist++ = ':'; + *locallist = 0; + len -= ianasz + 1; + } + else{ + XFREE(locallist, ctx-heap, DYNAMIC_TYPE_TMP_BUFFER); + return NULL; + } + } + XSTRNCPY(locallist, list, len); + locallist += listsz; + *locallist = 0; + + return head; + } else + return NULL; +} + +/* */ +/* check if the list has TLS13 and pre-TLS13 suites */ +/* @param list cipher suite list that user want to set */ +/* @return mixed: 0, only pre-TLS13: 1, only TLS13: 2 */ +static int CheckcipherList(const char* list) +{ + int findTLSv13Suites = 0; + int findbeforeSuites = 0; + const int suiteSz = GetCipherNamesSize(); + const CipherSuiteInfo* names = GetCipherNames(); + + char* next = (char*)list; + + do { + char* current = next; + char name[MAX_SUITE_NAME + 1]; + int i; + word32 length; + + next = XSTRSTR(next, ":"); + length = min(sizeof(name), !next ? (word32)XSTRLEN(current) /* last */ + : (word32)(next - current)); + XSTRNCPY(name, current, length); + name[(length == sizeof(name)) ? length - 1 : length] = 0; + + for (i = 0; i < suiteSz; i++) { + if (XSTRNCMP(name, names[i].name, sizeof(name)) == 0) + { + if (names[i].cipherSuite0 == TLS13_BYTE) { + /* TLSv13 suite */ + findTLSv13Suites = 1; + break; + } else { + findbeforeSuites = 1; + break; + } + } + } + + if (findTLSv13Suites == 1 && findbeforeSuites == 1) + /* list has mixed suites */ + return 0; + } while (next++); /* ++ needed to skip ':' */ + + if (findTLSv13Suites == 0 && findbeforeSuites == 1) + return 1;/* only before TLSv13 sutes */ + else if (findTLSv13Suites == 1 && findbeforeSuites == 0) + return 2;/* only TLSv13 suties */ + else + return 0;/* handle as mixed */ +} /* parse some bulk lists like !eNULL / !aNULL * @@ -11957,7 +12104,10 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, const CipherSuiteInfo* names = GetCipherNames(); char* localList = NULL; int sz = 0; - + int listattribute = 0; + char* buildcipherList = NULL; + int onlytls13suites = 0; + if (suites == NULL || list == NULL) { WOLFSSL_MSG("NULL argument"); return WOLFSSL_FAILURE; @@ -12006,8 +12156,35 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, return (ret)? WOLFSSL_SUCCESS : WOLFSSL_FAILURE; } else { - return (SetCipherList(ctx, suites, list)) ? WOLFSSL_SUCCESS : + + listattribute = CheckcipherList(list); + + if (listattribute == 0) { + /* list has mixed(pre-TLSv13 and TLSv13) suites */ + /* update cipher suites the same as before */ + return (SetCipherList(ctx, suites, list)) ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE; + } else if (listattribute == 1) { + /* list has only pre-TLSv13 suites. Only update before TLSv13 suites.*/ + onlytls13suites = 1; + } else if (listattribute == 2) { + /* list has only TLSv13 suites. Only update TLv13 suites */ + /* simulate set_ciphersuites() comatibility layer API */ + onlytls13suites = 0; + } + + buildcipherList = buildEnabledCipherList(ctx, ctx->suites, + onlytls13suites, list); + + if (buildcipherList) { + + ret = SetCipherList(ctx, suites, buildcipherList); + XFREE(buildcipherList, ctx->heap, DYNAMIC_TYPE_TMP_BUFFER); + } + else + ret = SetCipherList(ctx, suites, list); + + return ret; } } diff --git a/tests/api.c b/tests/api.c index ad2ad6dc1..3f415fe51 100644 --- a/tests/api.c +++ b/tests/api.c @@ -756,6 +756,14 @@ static void test_for_double_Free(void) AssertTrue(wolfSSL_CTX_use_certificate_file(ctx, testCertFile, WOLFSSL_FILETYPE_PEM)); AssertTrue(wolfSSL_CTX_use_PrivateKey_file(ctx, testKeyFile, WOLFSSL_FILETYPE_PEM)); AssertTrue(wolfSSL_CTX_set_cipher_list(ctx, optionsCiphers)); +#ifdef WOLFSSL_TLS13 + /* only update TLSv13 suites */ + AssertTrue(wolfSSL_CTX_set_cipher_list(ctx, "TLS13-AES256-GCM-SHA384")); +#endif +#ifndef WOLFSSL_NO_TLS12 + /* only update pre-TLSv13 suites */ + AssertTrue(wolfSSL_CTX_set_cipher_list(ctx, "ECDHE-RSA-AES128-GCM-SHA256")); +#endif AssertNotNull(ssl = wolfSSL_new(ctx)); wolfSSL_CTX_free(ctx); wolfSSL_free(ssl); @@ -773,6 +781,14 @@ static void test_for_double_Free(void) AssertNotNull(ssl); /* test setting ciphers at SSL level */ AssertTrue(wolfSSL_set_cipher_list(ssl, optionsCiphers)); +#ifdef WOLFSSL_TLS13 + /* only update TLSv13 suites */ + AssertTrue(wolfSSL_set_cipher_list(ssl, "TLS13-AES256-GCM-SHA384")); +#endif +#ifndef WOLFSSL_NO_TLS12 + /* only update pre-TLSv13 suites */ + AssertTrue(wolfSSL_set_cipher_list(ssl, "ECDHE-RSA-AES128-GCM-SHA256")); +#endif wolfSSL_CTX_free(ctx); wolfSSL_free(ssl); } From a4ff5de369cf20558c8aa78fa61db4dc1ed174b0 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 21 May 2021 14:54:11 +0900 Subject: [PATCH 2/7] always tls13 suites in the front position --- src/ssl.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 34b9cd45f..6e65256f0 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12005,6 +12005,15 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, head = locallist; + if (!onlytlsv13suites) + { + /* always tls13 suites in the head position */ + XSTRNCPY(locallist, list, len); + locallist += listsz; + *locallist = 0; + len -= listsz; + } + for(idx = 0; idx < suites->suiteSz; idx++) { cipherSuite0 = suites->suites[idx]; cipherSuite = suites->suites[++idx]; @@ -12030,9 +12039,12 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, return NULL; } } - XSTRNCPY(locallist, list, len); - locallist += listsz; - *locallist = 0; + + if (onlytlsv13suites) { + XSTRNCPY(locallist, list, len); + locallist += listsz; + *locallist = 0; + } return head; } else @@ -12079,8 +12091,8 @@ static int CheckcipherList(const char* list) } if (findTLSv13Suites == 1 && findbeforeSuites == 1) - /* list has mixed suites */ - return 0; + /* list has mixed suites */ + return 0; } while (next++); /* ++ needed to skip ':' */ if (findTLSv13Suites == 0 && findbeforeSuites == 1) From 1ebb4a47f6b77ea71612ec0d7445b5ca0e674b85 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 21 May 2021 16:46:02 +0900 Subject: [PATCH 3/7] addressed jenkins failure --- src/ssl.c | 14 +++++++------- tests/api.c | 14 ++++++++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 6e65256f0..9eaf2980f 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11954,10 +11954,10 @@ static int wolfSSL_remove_ciphers(char* list, int sz, const char* toRemove) static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, int onlytlsv13suites, const char* list) { - int idx = 0; - int listsz = 0; - int len = 0; - int ianasz = 0; + word32 idx = 0; + word32 listsz = 0; + word32 len = 0; + word32 ianasz = 0; const char* enabledcs = NULL; char* locallist = NULL; char* head = NULL; @@ -11971,7 +11971,7 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, if (!suites->setSuites) return NULL; - listsz = XSTRLEN(list); + listsz = (word32)XSTRLEN(list); /* calculate necessary buffer length */ for(idx = 0; idx < suites->suiteSz; idx++) { @@ -11987,7 +11987,7 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, continue; if (XSTRNCMP(enabledcs, "None", XSTRLEN(enabledcs)) != 0) { - len += XSTRLEN(enabledcs) + 2; + len += (word32)XSTRLEN(enabledcs) + 2; } } @@ -12035,7 +12035,7 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, len -= ianasz + 1; } else{ - XFREE(locallist, ctx-heap, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(locallist, ctx->heap, DYNAMIC_TYPE_TMP_BUFFER); return NULL; } } diff --git a/tests/api.c b/tests/api.c index 3f415fe51..c2a13993f 100644 --- a/tests/api.c +++ b/tests/api.c @@ -756,11 +756,14 @@ static void test_for_double_Free(void) AssertTrue(wolfSSL_CTX_use_certificate_file(ctx, testCertFile, WOLFSSL_FILETYPE_PEM)); AssertTrue(wolfSSL_CTX_use_PrivateKey_file(ctx, testKeyFile, WOLFSSL_FILETYPE_PEM)); AssertTrue(wolfSSL_CTX_set_cipher_list(ctx, optionsCiphers)); -#ifdef WOLFSSL_TLS13 +#if defined(OPENSSL_EXTRA) && defined(WOLFSSL_TLS13) && defined(HAVE_AESGCM) && \ + defined(WOLFSSL_SHA384) && defined(WOLFSSL_AES_256) /* only update TLSv13 suites */ AssertTrue(wolfSSL_CTX_set_cipher_list(ctx, "TLS13-AES256-GCM-SHA384")); #endif -#ifndef WOLFSSL_NO_TLS12 +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && defined(HAVE_AESGCM) && \ + !defined(NO_SHA256) && !defined(WOLFSSL_NO_TLS12) && \ + defined(WOLFSSL_AES_128) && !defined(NO_RSA) /* only update pre-TLSv13 suites */ AssertTrue(wolfSSL_CTX_set_cipher_list(ctx, "ECDHE-RSA-AES128-GCM-SHA256")); #endif @@ -781,11 +784,14 @@ static void test_for_double_Free(void) AssertNotNull(ssl); /* test setting ciphers at SSL level */ AssertTrue(wolfSSL_set_cipher_list(ssl, optionsCiphers)); -#ifdef WOLFSSL_TLS13 +#if defined(OPENSSL_EXTRA) && defined(WOLFSSL_TLS13) && defined(HAVE_AESGCM) && \ + defined(WOLFSSL_SHA384) && defined(WOLFSSL_AES_256) /* only update TLSv13 suites */ AssertTrue(wolfSSL_set_cipher_list(ssl, "TLS13-AES256-GCM-SHA384")); #endif -#ifndef WOLFSSL_NO_TLS12 +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && defined(HAVE_AESGCM) && \ + !defined(NO_SHA256) && !defined(WOLFSSL_NO_TLS12) && \ + defined(WOLFSSL_AES_128) && !defined(NO_RSA) /* only update pre-TLSv13 suites */ AssertTrue(wolfSSL_set_cipher_list(ssl, "ECDHE-RSA-AES128-GCM-SHA256")); #endif From 23a3c7f5f50d7285bfcfe0b761e3c870b51d3d5b Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 4 Jun 2021 10:15:11 +0900 Subject: [PATCH 4/7] fixed no-termination --- src/ssl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 9eaf2980f..98c6626db 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11991,7 +11991,7 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, } } - len += listsz + 1; + len += listsz + 2; /* build string */ if (len > 0) { @@ -12010,8 +12010,9 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, /* always tls13 suites in the head position */ XSTRNCPY(locallist, list, len); locallist += listsz; + *locallist++ = ':'; *locallist = 0; - len -= listsz; + len -= listsz + 1; } for(idx = 0; idx < suites->suiteSz; idx++) { From 368dd7b5016bcd87d3a2541f1d918755147c26a7 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Thu, 10 Jun 2021 10:48:45 +0900 Subject: [PATCH 5/7] address review comments part1 --- src/ssl.c | 114 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 63 insertions(+), 51 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 98c6626db..e29d6a46c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11943,16 +11943,17 @@ static int wolfSSL_remove_ciphers(char* list, int sz, const char* toRemove) return totalSz; } -/* */ -/* build enabled cipher list w/ TLS13 or w/o TLS13 suites */ -/* @param ctx a pointer to WOLFSSL_CTX structure */ -/* @param suites currently enabled suites */ -/* @param onlytlsv13suites flag whether correcting w/ TLS13 suites */ -/* or w/o TLS13 suties */ -/* @param list suites list that user wants to update */ -/* @return suites list on successs, otherwise NULL */ +/* + * build enabled cipher list w/ TLS13 or w/o TLS13 suites + * @param ctx a pointer to WOLFSSL_CTX structure + * @param suites currently enabled suites + * @param onlytlsv13suites flag whether correcting w/ TLS13 suites + * or w/o TLS13 suties + * @param list suites list that user wants to update + * @return suites list on successs, otherwise NULL + */ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, - int onlytlsv13suites, const char* list) + int tls13Only, const char* list) { word32 idx = 0; word32 listsz = 0; @@ -11979,11 +11980,13 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, cipherSuite0 = suites->suites[idx]; cipherSuite = suites->suites[++idx]; - if (onlytlsv13suites && cipherSuite0 == TLS13_BYTE) { + if (tls13Only && cipherSuite0 == TLS13_BYTE) { enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); - } else if (!onlytlsv13suites && cipherSuite0 != TLS13_BYTE) { + } + else if (!tls13Only && cipherSuite0 != TLS13_BYTE) { enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); - } else + } + else continue; if (XSTRNCMP(enabledcs, "None", XSTRLEN(enabledcs)) != 0) { @@ -11994,7 +11997,7 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, len += listsz + 2; /* build string */ - if (len > 0) { + if (len > (listsz + 2)) { locallist = (char*)XMALLOC(len, ctx->heap, DYNAMIC_TYPE_TMP_BUFFER); /* sanity check */ @@ -12005,7 +12008,7 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, head = locallist; - if (!onlytlsv13suites) + if (!tls13Only) { /* always tls13 suites in the head position */ XSTRNCPY(locallist, list, len); @@ -12019,11 +12022,13 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, cipherSuite0 = suites->suites[idx]; cipherSuite = suites->suites[++idx]; - if (onlytlsv13suites && cipherSuite0 == TLS13_BYTE) { + if (tls13Only && cipherSuite0 == TLS13_BYTE) { enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); - } else if (!onlytlsv13suites && cipherSuite0 != TLS13_BYTE) { + } + else if (!tls13Only && cipherSuite0 != TLS13_BYTE) { enabledcs = GetCipherNameInternal(cipherSuite0, cipherSuite); - } else + } + else continue; ianasz = (int)XSTRLEN(enabledcs); @@ -12041,34 +12046,36 @@ static char* buildEnabledCipherList(WOLFSSL_CTX* ctx, Suites* suites, } } - if (onlytlsv13suites) { + if (tls13Only) { XSTRNCPY(locallist, list, len); locallist += listsz; *locallist = 0; } return head; - } else + } + else return NULL; } -/* */ -/* check if the list has TLS13 and pre-TLS13 suites */ -/* @param list cipher suite list that user want to set */ -/* @return mixed: 0, only pre-TLS13: 1, only TLS13: 2 */ +/* + * check if the list has TLS13 and pre-TLS13 suites + * @param list cipher suite list that user want to set + * @return mixed: 0, only pre-TLS13: 1, only TLS13: 2 + */ static int CheckcipherList(const char* list) { + int ret; int findTLSv13Suites = 0; int findbeforeSuites = 0; - const int suiteSz = GetCipherNamesSize(); - const CipherSuiteInfo* names = GetCipherNames(); - + byte cipherSuite0; + byte cipherSuite1; + int flags; char* next = (char*)list; do { char* current = next; char name[MAX_SUITE_NAME + 1]; - int i; word32 length; next = XSTRSTR(next, ":"); @@ -12077,20 +12084,19 @@ static int CheckcipherList(const char* list) XSTRNCPY(name, current, length); name[(length == sizeof(name)) ? length - 1 : length] = 0; - for (i = 0; i < suiteSz; i++) { - if (XSTRNCMP(name, names[i].name, sizeof(name)) == 0) - { - if (names[i].cipherSuite0 == TLS13_BYTE) { - /* TLSv13 suite */ - findTLSv13Suites = 1; - break; - } else { - findbeforeSuites = 1; - break; - } + ret = wolfSSL_get_cipher_suite_from_name(name, &cipherSuite0, + &cipherSuite1, &flags); + if (ret == 0) { + if (cipherSuite0 == TLS13_BYTE) { + /* TLSv13 suite */ + findTLSv13Suites = 1; + break; + } + else { + findbeforeSuites = 1; + break; } - } - + } if (findTLSv13Suites == 1 && findbeforeSuites == 1) /* list has mixed suites */ return 0; @@ -12119,7 +12125,7 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, int sz = 0; int listattribute = 0; char* buildcipherList = NULL; - int onlytls13suites = 0; + int tls13Only = 0; if (suites == NULL || list == NULL) { WOLFSSL_MSG("NULL argument"); @@ -12173,21 +12179,27 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, listattribute = CheckcipherList(list); if (listattribute == 0) { - /* list has mixed(pre-TLSv13 and TLSv13) suites */ - /* update cipher suites the same as before */ + /* list has mixed(pre-TLSv13 and TLSv13) suites + * update cipher suites the same as before + */ return (SetCipherList(ctx, suites, list)) ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE; - } else if (listattribute == 1) { - /* list has only pre-TLSv13 suites. Only update before TLSv13 suites.*/ - onlytls13suites = 1; - } else if (listattribute == 2) { - /* list has only TLSv13 suites. Only update TLv13 suites */ - /* simulate set_ciphersuites() comatibility layer API */ - onlytls13suites = 0; + } + else if (listattribute == 1) { + /* list has only pre-TLSv13 suites. + * Only update before TLSv13 suites. + */ + tls13Only = 1; + } + else if (listattribute == 2) { + /* list has only TLSv13 suites. Only update TLv13 suites + * simulate set_ciphersuites() comatibility layer API + */ + tls13Only = 0; } buildcipherList = buildEnabledCipherList(ctx, ctx->suites, - onlytls13suites, list); + tls13Only, list); if (buildcipherList) { From b52ff200de51be165bc8c3217c866b8549840b95 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Thu, 17 Jun 2021 16:26:51 +0900 Subject: [PATCH 6/7] addressed code review part2 --- src/ssl.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index e29d6a46c..174f015a2 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12097,17 +12097,21 @@ static int CheckcipherList(const char* list) break; } } - if (findTLSv13Suites == 1 && findbeforeSuites == 1) + if (findTLSv13Suites == 1 && findbeforeSuites == 1) { /* list has mixed suites */ return 0; + } } while (next++); /* ++ needed to skip ':' */ - if (findTLSv13Suites == 0 && findbeforeSuites == 1) + if (findTLSv13Suites == 0 && findbeforeSuites == 1) { return 1;/* only before TLSv13 sutes */ - else if (findTLSv13Suites == 1 && findbeforeSuites == 0) + } + else if (findTLSv13Suites == 1 && findbeforeSuites == 0) { return 2;/* only TLSv13 suties */ - else + } + else { return 0;/* handle as mixed */ + } } /* parse some bulk lists like !eNULL / !aNULL @@ -12179,22 +12183,22 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, listattribute = CheckcipherList(list); if (listattribute == 0) { - /* list has mixed(pre-TLSv13 and TLSv13) suites - * update cipher suites the same as before - */ + /* list has mixed(pre-TLSv13 and TLSv13) suites + * update cipher suites the same as before + */ return (SetCipherList(ctx, suites, list)) ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE; } else if (listattribute == 1) { - /* list has only pre-TLSv13 suites. - * Only update before TLSv13 suites. - */ + /* list has only pre-TLSv13 suites. + * Only update before TLSv13 suites. + */ tls13Only = 1; } else if (listattribute == 2) { - /* list has only TLSv13 suites. Only update TLv13 suites - * simulate set_ciphersuites() comatibility layer API - */ + /* list has only TLSv13 suites. Only update TLv13 suites + * simulate set_ciphersuites() comatibility layer API + */ tls13Only = 0; } @@ -12202,13 +12206,13 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, tls13Only, list); if (buildcipherList) { - ret = SetCipherList(ctx, suites, buildcipherList); XFREE(buildcipherList, ctx->heap, DYNAMIC_TYPE_TMP_BUFFER); } - else + else { ret = SetCipherList(ctx, suites, list); - + } + return ret; } } From fbb7a40295b32d4b3a4b2e39d955f2e4bdab0dd6 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 18 Jun 2021 11:55:09 +0900 Subject: [PATCH 7/7] simplified string parse --- src/ssl.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 174f015a2..66c0cb2a5 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12076,13 +12076,19 @@ static int CheckcipherList(const char* list) do { char* current = next; char name[MAX_SUITE_NAME + 1]; - word32 length; - + word32 length = MAX_SUITE_NAME; + word32 current_length; + next = XSTRSTR(next, ":"); - length = min(sizeof(name), !next ? (word32)XSTRLEN(current) /* last */ - : (word32)(next - current)); + + current_length = (!next) ? (word32)XSTRLEN(current) + : (word32)(next - current); + + if (current_length < length) { + length = current_length; + } XSTRNCPY(name, current, length); - name[(length == sizeof(name)) ? length - 1 : length] = 0; + name[length] = 0; ret = wolfSSL_get_cipher_suite_from_name(name, &cipherSuite0, &cipherSuite1, &flags); @@ -12142,14 +12148,19 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, Suites* suites, char* current = next; char name[MAX_SUITE_NAME + 1]; int i; - word32 length; + word32 length = MAX_SUITE_NAME; + word32 current_length; next = XSTRSTR(next, ":"); - length = min(sizeof(name), !next ? (word32)XSTRLEN(current) /*last*/ - : (word32)(next - current)); - + + current_length = (!next) ? (word32)XSTRLEN(current) + : (word32)(next - current); + + if (current_length < length) { + length = current_length; + } XSTRNCPY(name, current, length); - name[(length == sizeof(name)) ? length - 1 : length] = 0; + name[length] = 0; /* check for "not" case */ if (name[0] == '!' && suiteSz > 0) {