From aeeeb666a7ee43b32d7b42ac4cedab77529f92ea Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 9 Oct 2020 13:43:24 -0700 Subject: [PATCH 1/6] Maintenance Fixes 1. The test_wolfSSL_X509V3_EXT_print() test was using stderr for output, changed to stdout. 2. A call to XFREAD wasn't typecasting its output to the size of the variable getting the output in decodedCertCache_test(). --- tests/api.c | 3 +-- wolfcrypt/test/test.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/api.c b/tests/api.c index 866000ecb..6a7c75474 100644 --- a/tests/api.c +++ b/tests/api.c @@ -33329,8 +33329,7 @@ static void test_wolfSSL_X509V3_EXT_print(void) }; int* n; - printf(testingFmt, "wolfSSL_X509V3_EXT_print"); - AssertNotNull(bio = BIO_new_fp(stderr, BIO_NOCLOSE)); + AssertNotNull(bio = BIO_new_fp(stdout, BIO_NOCLOSE)); AssertNotNull(x509 = wolfSSL_X509_load_certificate_file(cliCertFileExt, WOLFSSL_FILETYPE_PEM)); diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 8a81754e9..fee483566 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -11087,7 +11087,7 @@ static int decodedCertCache_test(void) /* load cert.der */ file = XFOPEN(certDerFile, "rb"); if (file != NULL) { - derSz = XFREAD(der, 1, FOURK_BUF, file); + derSz = (word32)XFREAD(der, 1, FOURK_BUF, file); XFCLOSE(file); } else From d8299e2764c0a3a5f90bc58a0a2361fe1a788b6b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 9 Oct 2020 13:48:08 -0700 Subject: [PATCH 2/6] Maintenance Fixes When building the list of ciphers with wolfSSL_get_ciphers_compat(), skip the fake indicator ciphers like the renegotiation indication and the quantum-safe hybrid since they do not have encryption or mac algorithms associated to them. --- src/ssl.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index 70574db35..64aea9ca0 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -43501,6 +43501,21 @@ unsigned long wolfSSL_ERR_peek_error_line_data(const char **file, int *line, #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) + +static WC_INLINE int SKIP_SUITE(byte suite0, byte suite) +{ +#ifdef HAVE_RENEGOTIATION_INDICATION + if (suite0 == CIPHER_BYTE && suite == TLS_EMPTY_RENEGOTIATION_INFO_SCSV) + return 1; +#endif +#ifdef BUILD_TLS_QSH + if (suite0 == QSH_BYTE && suite == TLS_QSH) + return 1; +#endif + return 0; +} + + /* returns a pointer to internal cipher suite list. Should not be free'd by * caller. */ @@ -43532,7 +43547,15 @@ WOLF_STACK_OF(WOLFSSL_CIPHER) *wolfSSL_get_ciphers_compat(const WOLFSSL *ssl) int j; #endif for (i = 0; i < suites->suiteSz; i+=2) { - WOLFSSL_STACK* add = wolfSSL_sk_new_node(ssl->heap); + WOLFSSL_STACK* add; + + /* A couple of suites are placeholders for special options, + * skip those. */ + if (SKIP_SUITE(suites->suites[i], suites->suites[i+1])) { + continue; + } + + add = wolfSSL_sk_new_node(ssl->heap); if (add != NULL) { add->type = STACK_TYPE_CIPHER; add->data.cipher.cipherSuite0 = suites->suites[i]; From 2d85061c47936aa5e61a5cc4bda055628baaf1f6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 9 Oct 2020 14:40:19 -0700 Subject: [PATCH 3/6] Maintenance Fixes Improve the reporting of the NTRU based cipher suites with the function wolfSSL_sk_CIPHER_description(). --- src/internal.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 9122c43d2..e68427670 100644 --- a/src/internal.c +++ b/src/internal.c @@ -19059,6 +19059,11 @@ const char* GetCipherKeaStr(char n[][MAX_SEGMENT_SZ]) { n3 = n[3]; n4 = n[4]; +#if HAVE_NTRU + if (XSTRNCMP(n0,"NTRU",4) == 0) + return "NTRU"; +#endif + if (XSTRNCMP(n0,"ECDHE",5) == 0 && XSTRNCMP(n1,"PSK",3) == 0) keaStr = "ECDHEPSK"; else if (XSTRNCMP(n0,"ECDH",4) == 0) @@ -19094,6 +19099,11 @@ const char* GetCipherAuthStr(char n[][MAX_SEGMENT_SZ]) { n1 = n[1]; n2 = n[2]; +#ifdef HAVE_NTRU + if (XSTRNCMP(n0,"NTRU",4) == 0) + return "NTRU"; +#endif + if ((XSTRNCMP(n0,"AES128",6) == 0) || (XSTRNCMP(n0,"AES256",6) == 0) || ((XSTRNCMP(n0,"TLS13",5) == 0) && ((XSTRNCMP(n1,"AES128",6) == 0) || (XSTRNCMP(n1,"AES256",6) == 0) || (XSTRNCMP(n1,"CHACHA20",8) == 0))) || @@ -19158,10 +19168,13 @@ const char* GetCipherEncStr(char n[][MAX_SEGMENT_SZ]) { else if ((XSTRNCMP(n0,"CAMELLIA128",11) == 0) || (XSTRNCMP(n2,"CAMELLIA128",11) == 0)) encStr = "CAMELLIA(128)"; - else if ((XSTRNCMP(n0,"RC4",3) == 0) || (XSTRNCMP(n2,"RC4",3) == 0)) + else if ((XSTRNCMP(n0,"RC4",3) == 0) || (XSTRNCMP(n1,"RC4",3) == 0) || + (XSTRNCMP(n2,"RC4",3) == 0)) encStr = "RC4"; - else if (((XSTRNCMP(n0,"DES",3) == 0) || (XSTRNCMP(n2,"DES",3) == 0)) && - ((XSTRNCMP(n1,"CBC3",4) == 0) || (XSTRNCMP(n3,"CBC3",4) == 0))) + else if (((XSTRNCMP(n0,"DES",3) == 0) || (XSTRNCMP(n1,"DES",3) == 0) || + (XSTRNCMP(n2,"DES",3) == 0)) && + ((XSTRNCMP(n1,"CBC3",4) == 0) || (XSTRNCMP(n2,"CBC3",4) == 0) || + (XSTRNCMP(n3,"CBC3",4) == 0))) encStr = "3DES"; else if ((XSTRNCMP(n1,"CHACHA20",8) == 0 && XSTRNCMP(n2,"POLY1305",8) == 0) || (XSTRNCMP(n2,"CHACHA20",8) == 0 && XSTRNCMP(n3,"POLY1305",8) == 0)) From 6cfb038d11ded154d0c4cf395e30a12ee739d15d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 9 Oct 2020 15:54:44 -0700 Subject: [PATCH 4/6] Fix a bad ifdef. --- src/internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index e68427670..9cf1023fe 100644 --- a/src/internal.c +++ b/src/internal.c @@ -19059,7 +19059,7 @@ const char* GetCipherKeaStr(char n[][MAX_SEGMENT_SZ]) { n3 = n[3]; n4 = n[4]; -#if HAVE_NTRU +#ifdef HAVE_NTRU if (XSTRNCMP(n0,"NTRU",4) == 0) return "NTRU"; #endif From a05a305d70031278eb3faec7904f47e318b8d927 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 9 Oct 2020 15:59:14 -0700 Subject: [PATCH 5/6] Fix unused parameters in SKIP_SUITE. --- src/ssl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ssl.c b/src/ssl.c index 64aea9ca0..2a7377923 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -43504,6 +43504,8 @@ unsigned long wolfSSL_ERR_peek_error_line_data(const char **file, int *line, static WC_INLINE int SKIP_SUITE(byte suite0, byte suite) { + (void)suite0; + (void)suite; #ifdef HAVE_RENEGOTIATION_INDICATION if (suite0 == CIPHER_BYTE && suite == TLS_EMPTY_RENEGOTIATION_INFO_SCSV) return 1; From 0ca202f389171599f93dacdeab0fd6360247b6ce Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 12 Oct 2020 09:49:02 -0700 Subject: [PATCH 6/6] Rename SKIP_SUITE to something more descriptive. Add some comments. --- src/ssl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 2a7377923..1403acbbc 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -43502,7 +43502,8 @@ unsigned long wolfSSL_ERR_peek_error_line_data(const char **file, int *line, #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) -static WC_INLINE int SKIP_SUITE(byte suite0, byte suite) +/* Is the specified cipher suite a fake one used an an extension proxy? */ +static WC_INLINE int SCSV_Check(byte suite0, byte suite) { (void)suite0; (void)suite; @@ -43511,6 +43512,7 @@ static WC_INLINE int SKIP_SUITE(byte suite0, byte suite) return 1; #endif #ifdef BUILD_TLS_QSH + /* This isn't defined as a SCSV, but it acts like one. */ if (suite0 == QSH_BYTE && suite == TLS_QSH) return 1; #endif @@ -43553,7 +43555,7 @@ WOLF_STACK_OF(WOLFSSL_CIPHER) *wolfSSL_get_ciphers_compat(const WOLFSSL *ssl) /* A couple of suites are placeholders for special options, * skip those. */ - if (SKIP_SUITE(suites->suites[i], suites->suites[i+1])) { + if (SCSV_Check(suites->suites[i], suites->suites[i+1])) { continue; }