From 2b794f03c1fa42ff9677a1e26c49fe8e70466825 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Feb 2022 11:48:40 -0800 Subject: [PATCH 1/7] Fixes for multi-test pass. Breaks from PR #4807. --- configure.ac | 2 +- src/ssl.c | 8 ++++++-- wolfssl/internal.h | 5 ++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index bdb72994b..1c076891a 100644 --- a/configure.ac +++ b/configure.ac @@ -6843,7 +6843,7 @@ AS_CASE(["$CFLAGS $CPPFLAGS"],[*'WOLFSSL_TRUST_PEER_CERT'*],[ENABLED_TRUSTED_PEE AS_CASE(["$CFLAGS $CPPFLAGS $AM_CFLAGS"],[*'OPENSSL_COMPATIBLE_DEFAULTS'*], - [ENABLED_OPENSSL_COMPATIBLE_DEFAULTS=yes]) + [ENABLED_OPENSSL_COMPATIBLE_DEFAULTS=yes]) if test "x$ENABLED_OPENSSL_COMPATIBLE_DEFAULTS" = "xyes" then AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_TRUST_PEER_CERT" diff --git a/src/ssl.c b/src/ssl.c index bf45f2f5d..9e3a83039 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -5330,11 +5330,15 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify) #error CLIENT_SESSION_ROWS too big #endif - typedef struct ClientSession { + struct ClientSession { word16 serverRow; /* SessionCache Row id */ word16 serverIdx; /* SessionCache Idx (column) */ word32 sessionIDHash; - } ClientSession; + }; + #ifndef WOLFSSL_CLIENT_SESSION_DEFINED + typedef struct ClientSession ClientSession; + #define WOLFSSL_CLIENT_SESSION_DEFINED + #endif typedef struct ClientRow { int nextIdx; /* where to place next one */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index fa0b911f6..adf49dad3 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1691,7 +1691,10 @@ typedef WOLFSSL_BUFFER_INFO buffer; typedef struct Suites Suites; /* Declare opaque struct for API to use */ -typedef struct ClientSession ClientSession; +#ifndef WOLFSSL_CLIENT_SESSION_DEFINED + typedef struct ClientSession ClientSession; + #define WOLFSSL_CLIENT_SESSION_DEFINED +#endif /* defaults to client */ WOLFSSL_LOCAL void InitSSL_Method(WOLFSSL_METHOD* method, ProtocolVersion pv); From 6e24e21d5a48a92963579e1e48488e7424ca90a3 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Feb 2022 12:56:39 -0800 Subject: [PATCH 2/7] Fix for heap pointer in `wolfSSL_DupSession`. --- src/ssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 9e3a83039..7df0b69ff 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -24095,10 +24095,10 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, } else { tmp = (byte*)XREALLOC(ticBuff, input->ticketLen, - output->heap, DYNAMIC_TYPE_SESSION_TICK); + input->heap, DYNAMIC_TYPE_SESSION_TICK); if (tmp == NULL) { WOLFSSL_MSG("Failed to allocate memory for ticket"); - XFREE(ticBuff, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); + XFREE(ticBuff, input->heap, DYNAMIC_TYPE_SESSION_TICK); output->ticket = NULL; output->ticketLen = 0; output->ticketLenAlloc = 0; From 6dd7a289e739fca2457c87940bec211bed21829b Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Feb 2022 13:38:23 -0800 Subject: [PATCH 3/7] Fix for "set but not used". --- examples/client/client.c | 4 ++-- examples/server/server.c | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 3775df940..86e8ecb29 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -3191,8 +3191,8 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) #endif /* HAVE_ECC */ #if defined(WOLFSSL_TRUST_PEER_CERT) && !defined(NO_FILESYSTEM) if (trustCert) { - if ((ret = wolfSSL_CTX_trust_peer_cert(ctx, trustCert, - WOLFSSL_FILETYPE_PEM)) != WOLFSSL_SUCCESS) { + if (wolfSSL_CTX_trust_peer_cert(ctx, trustCert, + WOLFSSL_FILETYPE_PEM) != WOLFSSL_SUCCESS) { wolfSSL_CTX_free(ctx); ctx = NULL; err_sys("can't load trusted peer cert file"); } diff --git a/examples/server/server.c b/examples/server/server.c index 64e30f4e8..23835a1cb 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -2568,9 +2568,8 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) } #ifdef WOLFSSL_TRUST_PEER_CERT if (trustCert) { - if ((ret = wolfSSL_CTX_trust_peer_cert(ctx, trustCert, - WOLFSSL_FILETYPE_PEM)) - != WOLFSSL_SUCCESS) { + if (wolfSSL_CTX_trust_peer_cert(ctx, trustCert, + WOLFSSL_FILETYPE_PEM) != WOLFSSL_SUCCESS) { err_sys_ex(runWithErrors, "can't load trusted peer cert file"); } } @@ -2790,8 +2789,8 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) } #ifdef WOLFSSL_TRUST_PEER_CERT if (trustCert) { - if ((ret = wolfSSL_trust_peer_cert(ssl, trustCert, - WOLFSSL_FILETYPE_PEM)) != WOLFSSL_SUCCESS) { + if (wolfSSL_trust_peer_cert(ssl, trustCert, + WOLFSSL_FILETYPE_PEM) != WOLFSSL_SUCCESS) { err_sys_ex(runWithErrors, "can't load trusted peer cert " "file"); } From c2987a9ef9f3c051bf83bf5bd18ac79b294dc160 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Feb 2022 14:09:08 -0800 Subject: [PATCH 4/7] Fix for IPv6 `sockaddr_len` set but not read. --- src/wolfio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wolfio.c b/src/wolfio.c index 24c21fa29..1507e949c 100644 --- a/src/wolfio.c +++ b/src/wolfio.c @@ -834,10 +834,12 @@ int wolfIO_TcpConnect(SOCKET_T* sockfd, const char* ip, word16 port, int to_sec) return -1; } +#if !defined(HAVE_GETADDRINFO) #ifdef WOLFSSL_IPV6 sockaddr_len = sizeof(SOCKADDR_IN6); #else sockaddr_len = sizeof(SOCKADDR_IN); +#endif #endif XMEMSET(&addr, 0, sizeof(addr)); From 269ab860024a6f7a58d67d2e48705f196ba34990 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Feb 2022 14:28:50 -0800 Subject: [PATCH 5/7] Fixes for `DoClientTicket` changes. --- src/internal.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5d88d920c..aff16e83f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -30626,7 +30626,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, id = ssl->session->altSessionID; idSz = ID_LEN; } - XMEMCPY(it.id, id, ID_LEN); + XMEMCPY(it.id, id, idSz); } #endif @@ -30765,20 +30765,20 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* get master secret */ if (ret == WOLFSSL_TICKET_RET_OK || ret == WOLFSSL_TICKET_RET_CREATE) { if (ssl->version.minor < it->pv.minor) { - ForceZero(&it, sizeof(it)); + ForceZero(it, sizeof(*it)); WOLFSSL_MSG("Ticket has greater version"); return VERSION_ERROR; } else if (ssl->version.minor > it->pv.minor) { if (IsAtLeastTLSv1_3(it->pv) != IsAtLeastTLSv1_3(ssl->version)) { - ForceZero(&it, sizeof(it)); + ForceZero(it, sizeof(*it)); WOLFSSL_MSG("Tickets cannot be shared between " "TLS 1.3 and TLS 1.2 and lower"); return VERSION_ERROR; } if (!ssl->options.downgrade) { - ForceZero(&it, sizeof(it)); + ForceZero(it, sizeof(*it)); WOLFSSL_MSG("Ticket has lesser version"); return VERSION_ERROR; } @@ -30786,7 +30786,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, WOLFSSL_MSG("Downgrading protocol due to ticket"); if (it->pv.minor < ssl->options.minDowngrade) { - ForceZero(&it, sizeof(it)); + ForceZero(it, sizeof(*it)); return VERSION_ERROR; } ssl->version.minor = it->pv.minor; @@ -30837,7 +30837,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } } - ForceZero(&it, sizeof(it)); + ForceZero(it, sizeof(*it)); WOLFSSL_LEAVE("DoClientTicket", ret); WOLFSSL_END(WC_FUNC_TICKET_DO); From a39a1c1d87eb4bb32447d7f8c150c6a051ba13db Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Feb 2022 16:18:42 -0800 Subject: [PATCH 6/7] More fixups from cppcheck and clang-tidy. --- src/ssl.c | 11 +++++------ tests/api.c | 2 ++ wolfssl/internal.h | 4 ++-- wolfssl/ssl.h | 3 ++- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 7df0b69ff..3fcb151e4 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -5266,7 +5266,7 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify) #define SESSIONS_PER_ROW 3 #define SESSION_ROWS 11 #endif - #define INVALID_SESSION_ROW -1 + #define INVALID_SESSION_ROW (-1) #ifdef NO_SESSION_CACHE_ROW_LOCK #undef ENABLE_SESSION_CACHE_ROW_LOCK @@ -15511,6 +15511,7 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) return WOLFSSL_FAILURE; #endif + XMEMSET(bogusID, 0, sizeof(bogusID)); if (!IsAtLeastTLSv1_3(ssl->version) && ssl->arrays != NULL) id = ssl->arrays->sessionID; else if (ssl->session->haveAltSessionID) { @@ -15767,8 +15768,6 @@ int wolfSSL_SetSession(WOLFSSL* ssl, WOLFSSL_SESSION* session) SESSION_ROW_UNLOCK(sessRow); sessRow = NULL; } - /* Make sure we don't access this anymore */ - session = NULL; if (ret != WOLFSSL_SUCCESS) return ret; @@ -24095,10 +24094,10 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, } else { tmp = (byte*)XREALLOC(ticBuff, input->ticketLen, - input->heap, DYNAMIC_TYPE_SESSION_TICK); + output->heap, DYNAMIC_TYPE_SESSION_TICK); if (tmp == NULL) { WOLFSSL_MSG("Failed to allocate memory for ticket"); - XFREE(ticBuff, input->heap, DYNAMIC_TYPE_SESSION_TICK); + XFREE(ticBuff, output->heap, DYNAMIC_TYPE_SESSION_TICK); output->ticket = NULL; output->ticketLen = 0; output->ticketLenAlloc = 0; @@ -24142,7 +24141,7 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, } else { if (ticBuff != NULL) - XFREE(ticBuff, input->heap, DYNAMIC_TYPE_SESSION_TICK); + XFREE(ticBuff, output->heap, DYNAMIC_TYPE_SESSION_TICK); output->ticket = output->_staticTicket; output->ticketLenAlloc = 0; } diff --git a/tests/api.c b/tests/api.c index f8336579f..8a0cf5f72 100644 --- a/tests/api.c +++ b/tests/api.c @@ -39320,6 +39320,8 @@ static void test_wolfSSL_SESSION(void) #else AssertIntEQ(wolfSSL_SESSION_has_ticket(sess), 0); #endif +#else + (void)sess; #endif /* OPENSSL_EXTRA */ /* Retain copy of the session for later testing */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index adf49dad3..286c54838 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -4611,13 +4611,13 @@ struct WOLFSSL { */ #ifdef WOLFSSL_HAVE_ERROR_QUEUE #define CLEAR_ASN_NO_PEM_HEADER_ERROR(err) \ - err = wolfSSL_ERR_peek_last_error(); \ + (err) = wolfSSL_ERR_peek_last_error(); \ if (ERR_GET_LIB(err) == ERR_LIB_PEM && \ ERR_GET_REASON(err) == PEM_R_NO_START_LINE) { \ wc_RemoveErrorNode(-1); \ } #else -#define CLEAR_ASN_NO_PEM_HEADER_ERROR(err) (void)err; +#define CLEAR_ASN_NO_PEM_HEADER_ERROR(err) (void)(err); #endif /* diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index b25942dd8..bd82cf1e6 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -2852,7 +2852,8 @@ WOLFSSL_API int wolfSSL_make_eap_keys(WOLFSSL* ssl, void* key, unsigned int len, WOLFSSL_API int wolfSSL_Unload_trust_peers(WOLFSSL* ssl); #endif WOLFSSL_API int wolfSSL_CTX_trust_peer_buffer(WOLFSSL_CTX* ctx, - const unsigned char*, long, int); + const unsigned char* in, + long sz, int format); #endif WOLFSSL_API int wolfSSL_CTX_load_verify_buffer_ex(WOLFSSL_CTX* ctx, const unsigned char* in, long sz, int format, From 821fd3c89823467f2fd2c975ed2a71e0975c4db1 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 25 Feb 2022 10:58:19 -0800 Subject: [PATCH 7/7] Peer review fixes. Check idSz and add comment about session variable use. --- src/internal.c | 3 +++ src/ssl.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/internal.c b/src/internal.c index aff16e83f..25577319c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -30626,6 +30626,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, id = ssl->session->altSessionID; idSz = ID_LEN; } + /* make sure idSz is not larger than ID_LEN */ + if (idSz > ID_LEN) + idSz = ID_LEN; XMEMCPY(it.id, id, idSz); } #endif diff --git a/src/ssl.c b/src/ssl.c index 3fcb151e4..d2905de19 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -15769,6 +15769,9 @@ int wolfSSL_SetSession(WOLFSSL* ssl, WOLFSSL_SESSION* session) sessRow = NULL; } + /* Note: the `session` variable cannot be used below, since the row is + * un-locked */ + if (ret != WOLFSSL_SUCCESS) return ret;