From fa65da7bb028cef084d1603a389a1db7d455f24d Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 17 Oct 2024 00:06:32 -0500 Subject: [PATCH 1/2] analyzer-driven cleanups of --enable-dtls13 --enable-dtls-mtu --enable-dtls-frag-ch: Dtls13HashClientHello(): fix wc_HashType handling; Dtls13SendFragment(): fix identicalConditionAfterEarlyExit; GetDtlsRecordHeader(): fix error handling around GetDtls13RecordHeader() (incorrectLogicOperator); test_wolfSSL_dtls_stateless_maxfrag(): fix a clang-analyzer-core.NullDereference, test_dtls_frag_ch(): fix a clang-diagnostic-embedded-directive, test_AEAD_limit_client(): fix an united-data defect found by valgrind. --- src/dtls13.c | 20 ++++++++++---------- src/internal.c | 4 ++-- tests/api.c | 13 +++++++------ 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index f091ed62ff..6f2f014895 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -495,22 +495,25 @@ int Dtls13HashClientHello(const WOLFSSL* ssl, byte* hash, int* hashSz, wc_HashAlg hashCtx; int type = wolfSSL_GetHmacType_ex(specs); + if (type < 0) + return type; + header[0] = (byte)client_hello; c32to24(length, header + 1); - ret = wc_HashInit_ex(&hashCtx, type, ssl->heap, ssl->devId); + ret = wc_HashInit_ex(&hashCtx, (enum wc_HashType)type, ssl->heap, ssl->devId); if (ret == 0) { - ret = wc_HashUpdate(&hashCtx, type, header, OPAQUE32_LEN); + ret = wc_HashUpdate(&hashCtx, (enum wc_HashType)type, header, OPAQUE32_LEN); if (ret == 0) - ret = wc_HashUpdate(&hashCtx, type, body, length); + ret = wc_HashUpdate(&hashCtx, (enum wc_HashType)type, body, length); if (ret == 0) - ret = wc_HashFinal(&hashCtx, type, hash); + ret = wc_HashFinal(&hashCtx, (enum wc_HashType)type, hash); if (ret == 0) { - *hashSz = wc_HashGetDigestSize(type); + *hashSz = wc_HashGetDigestSize((enum wc_HashType)type); if (*hashSz < 0) ret = *hashSz; } - wc_HashFree(&hashCtx, type); + wc_HashFree(&hashCtx, (enum wc_HashType)type); } return ret; } @@ -568,9 +571,6 @@ static int Dtls13SendFragment(WOLFSSL* ssl, byte* output, word16 output_size, else { msg = output + recordHeaderLength; - if (length <= recordHeaderLength) - return BUFFER_ERROR; - if (hashOutput) { ret = Dtls13HashHandshake(ssl, msg, recordLength); if (ret != 0) @@ -1713,7 +1713,7 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, isFirst = fragOff == 0; isComplete = isFirst && fragLength == messageLength; - if (!isComplete && !Dtls13AcceptFragmented(ssl, handshakeType)) { + if (!isComplete && !Dtls13AcceptFragmented(ssl, (enum HandShakeType)handshakeType)) { #ifdef WOLFSSL_DTLS_CH_FRAG byte tls13 = 0; /* check if the first CH fragment contains a valid cookie */ diff --git a/src/internal.c b/src/internal.c index 38a79997a9..011dce6105 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11471,8 +11471,8 @@ static int GetDtlsRecordHeader(WOLFSSL* ssl, word32* inOutIdx, if (ssl->options.tls1_3) { ret = GetDtls13RecordHeader(ssl, inOutIdx, rh, size); if (ret == 0 || - ret != WC_NO_ERR_TRACE(SEQUENCE_ERROR) || - ret != WC_NO_ERR_TRACE(DTLS_CID_ERROR)) + ((ret != WC_NO_ERR_TRACE(SEQUENCE_ERROR)) && + (ret != WC_NO_ERR_TRACE(DTLS_CID_ERROR)))) return ret; } diff --git a/tests/api.c b/tests/api.c index 8ce4bb97f1..cf2fa25fb7 100644 --- a/tests/api.c +++ b/tests/api.c @@ -87647,6 +87647,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl) /* Test the sending limit for AEAD ciphers */ Dtls13GetEpoch(ssl, ssl->dtls13Epoch)->nextSeqNumber = sendLimit; test_AEAD_seq_num = 1; + XMEMSET(msgBuf, 0, sizeof(msgBuf)); ret = wolfSSL_write(ssl, msgBuf, sizeof(msgBuf)); AssertIntGT(ret, 0); didReKey = 0; @@ -90812,14 +90813,13 @@ static int test_wolfSSL_dtls_stateless_maxfrag(void) XMEMSET(&test_ctx, 0, sizeof(test_ctx)); ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method), 0); + ExpectNotNull(ssl_s); ExpectNotNull(ssl_c2 = wolfSSL_new(ctx_c)); ExpectIntEQ(wolfSSL_UseMaxFragment(ssl_c2, WOLFSSL_MFL_2_8), WOLFSSL_SUCCESS); wolfSSL_SetIOWriteCtx(ssl_c2, &test_ctx); wolfSSL_SetIOReadCtx(ssl_c2, &test_ctx); - if (ssl_s != NULL) { - max_fragment = ssl_s->max_fragment; - } + max_fragment = ssl_s->max_fragment; /* send CH */ ExpectTrue((wolfSSL_connect(ssl_c2) == WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)) && (ssl_c2->error == WC_NO_ERR_TRACE(WANT_READ))); @@ -95173,11 +95173,12 @@ static int test_dtls_frag_ch(void) /* Limit options to make the CH a fixed length */ /* See wolfSSL_parse_cipher_list for reason why we provide 1.3 AND 1.2 * ciphersuite. This is only necessary when building with OPENSSL_EXTRA. */ - ExpectTrue(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384" #ifdef OPENSSL_EXTRA - ":DHE-RSA-AES256-GCM-SHA384" + ExpectTrue(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384" + ":DHE-RSA-AES256-GCM-SHA384")); +#else + ExpectTrue(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384")); #endif - )); /* CH1 */ ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); From 06de22e72be26cef007b43b0bbc70fa2e1f1c08d Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 17 Oct 2024 10:57:19 -0500 Subject: [PATCH 2/2] api.c:test_wolfSSL_dtls_stateless_maxfrag(): add missing condition (clang-analyzer-core.NullDereference). --- tests/api.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/api.c b/tests/api.c index cf2fa25fb7..6d765a8d52 100644 --- a/tests/api.c +++ b/tests/api.c @@ -90819,7 +90819,9 @@ static int test_wolfSSL_dtls_stateless_maxfrag(void) WOLFSSL_SUCCESS); wolfSSL_SetIOWriteCtx(ssl_c2, &test_ctx); wolfSSL_SetIOReadCtx(ssl_c2, &test_ctx); - max_fragment = ssl_s->max_fragment; + if (EXPECT_SUCCESS()) { + max_fragment = ssl_s->max_fragment; + } /* send CH */ ExpectTrue((wolfSSL_connect(ssl_c2) == WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)) && (ssl_c2->error == WC_NO_ERR_TRACE(WANT_READ)));