From dd28d53cfb1aa4f71fa855dfb5467f02c743d56e Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 1 Apr 2016 09:23:46 -0700 Subject: [PATCH 1/3] Fix build issues with new async changes. Fixed issue with unused args preSigSz and preSigIdx with PSK enabled and ECC + RSA disabled. Fixed issue with missing qsSz variable in DoClientKeyExchange. Fixed missing DhAgree and DhKeyGen with NO_CERTS and PSK enabled. Fixed a couple scan-build warnings with "Value stored to '' is never read". --- src/internal.c | 26 ++++++++++++++++---------- wolfcrypt/benchmark/benchmark.c | 8 ++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/internal.c b/src/internal.c index 9a3eb703d..b48343699 100755 --- a/src/internal.c +++ b/src/internal.c @@ -2061,6 +2061,9 @@ int EccMakeTempKey(WOLFSSL* ssl) #endif /* HAVE_ECC */ +#endif /* !NO_CERTS */ + +#if !defined(NO_CERTS) || !defined(NO_PSK) #if !defined(NO_DH) int DhGenKeyPair(WOLFSSL* ssl, @@ -2145,8 +2148,8 @@ int DhAgree(WOLFSSL* ssl, } #endif /* !NO_DH */ +#endif /* !NO_CERTS || !NO_PSK */ -#endif /* NO_CERTS */ /* This function inherits a WOLFSSL_CTX's fields into an SSL object. @@ -14445,6 +14448,13 @@ int DoSessionTicket(WOLFSSL* ssl, word32 exportSz = 0; #endif + #ifdef HAVE_QSH + word32 qshSz = 0; + if (ssl->peerQSHKeyPresent) { + qshSz = QSH_KeyGetSize(ssl); + } + #endif + (void)ssl; (void)sigSz; @@ -14602,14 +14612,9 @@ int DoSessionTicket(WOLFSSL* ssl, case KEYSHARE_BUILD: { + #if (!defined(NO_DH) && !defined(NO_RSA)) || defined(HAVE_ECC) word32 preSigSz, preSigIdx; - - #ifdef HAVE_QSH - word32 qshSz = 0; - if (ssl->peerQSHKeyPresent && ssl->options.haveQSH) { - qshSz = QSH_KeyGetSize(ssl); - } - #endif + #endif switch(ssl->specs.kea) { @@ -14731,7 +14736,8 @@ int DoSessionTicket(WOLFSSL* ssl, idx += LENGTH_SZ; XMEMCPY(output + idx, ssl->buffers.serverDH_Pub.buffer, ssl->buffers.serverDH_Pub.length); - idx += ssl->buffers.serverDH_Pub.length; + /* No need to update idx, since sizes are already set */ + /* idx += ssl->buffers.serverDH_Pub.length; */ break; } #endif /* !defined(NO_DH) && !defined(NO_PSK) */ @@ -17441,7 +17447,6 @@ int DoSessionTicket(WOLFSSL* ssl, { #ifdef HAVE_QSH word16 name; - int qshSz; if (ssl->options.haveQSH) { /* extension name */ @@ -17449,6 +17454,7 @@ int DoSessionTicket(WOLFSSL* ssl, idx += OPAQUE16_LEN; if (name == TLSX_QUANTUM_SAFE_HYBRID) { + int qshSz; /* if qshSz is larger than 0 it is the length of buffer used */ if ((qshSz = TLSX_QSHCipher_Parse(ssl, diff --git a/wolfcrypt/benchmark/benchmark.c b/wolfcrypt/benchmark/benchmark.c index 31b2e7bea..fbb79c3e7 100644 --- a/wolfcrypt/benchmark/benchmark.c +++ b/wolfcrypt/benchmark/benchmark.c @@ -1391,16 +1391,16 @@ void bench_dh(void) (void)tmp; -#ifdef USE_CERT_BUFFERS_1024 +#if defined(NO_ASN) + dhKeySz = 1024; + /* do nothing, but don't use default FILE */ +#elif defined(USE_CERT_BUFFERS_1024) tmp = dh_key_der_1024; bytes = sizeof_dh_key_der_1024; dhKeySz = 1024; #elif defined(USE_CERT_BUFFERS_2048) tmp = dh_key_der_2048; bytes = sizeof_dh_key_der_2048; -#elif defined(NO_ASN) - dhKeySz = 1024; - /* do nothing, but don't use default FILE */ #else #error "need to define a cert buffer size" #endif /* USE_CERT_BUFFERS */ From 19f0769ec494ade6cbf8d8451ca7e31adaac168d Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 1 Apr 2016 10:55:01 -0700 Subject: [PATCH 2/3] Fix for scan-build warning where async changes make it appear like the output buffer could be NULL (even though its not). Added NULL check on the AddRecordHeader function. --- src/internal.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index b48343699..de74ae50b 100755 --- a/src/internal.c +++ b/src/internal.c @@ -3532,6 +3532,9 @@ static void AddRecordHeader(byte* output, word32 length, byte type, WOLFSSL* ssl /* record layer header */ rl = (RecordLayerHeader*)output; + if (rl == NULL) { + return; + } rl->type = type; rl->pvMajor = ssl->version.major; /* type and version same in each */ rl->pvMinor = ssl->version.minor; @@ -3539,13 +3542,15 @@ static void AddRecordHeader(byte* output, word32 length, byte type, WOLFSSL* ssl #ifdef WOLFSSL_ALTERNATIVE_DOWNGRADE if (ssl->options.side == WOLFSSL_CLIENT_END && ssl->options.connectState == CONNECT_BEGIN - && !ssl->options.resuming) + && !ssl->options.resuming) { rl->pvMinor = ssl->options.downgrade ? ssl->options.minDowngrade : ssl->version.minor; + } #endif - if (!ssl->options.dtls) + if (!ssl->options.dtls) { c16toa((word16)length, rl->length); + } else { #ifdef WOLFSSL_DTLS DtlsRecordLayerHeader* dtls; From 2d4aa1bbb5e7e33db8a40449c4c481b38dbbba0f Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 1 Apr 2016 12:57:33 -0700 Subject: [PATCH 3/3] Better fix for scan-build warning regarding possible use of NULL in AddRecordHeader. Scan-build considers paths where output is set to NULL, but ssl->spec.kea is corrupted/changed, which could result in output == NULL (even though it should never happen). So added proper NULL check in SendServerKeyExchange on AddHeader to make sure output isn't NULL. --- src/internal.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index de74ae50b..f072d8181 100755 --- a/src/internal.c +++ b/src/internal.c @@ -15468,8 +15468,15 @@ int DoSessionTicket(WOLFSSL* ssl, #endif #if defined(HAVE_ECC) - if (ssl->specs.kea == ecdhe_psk_kea || ssl->specs.kea == ecc_diffie_hellman_kea) { - AddHeaders(output, length, server_key_exchange, ssl); + if (ssl->specs.kea == ecdhe_psk_kea || + ssl->specs.kea == ecc_diffie_hellman_kea) { + /* Check output to make sure it was set */ + if (output) { + AddHeaders(output, length, server_key_exchange, ssl); + } + else { + ERROR_OUT(BUFFER_ERROR, exit_sske); + } } #endif /* HAVE_ECC */