From 0441aae3f22b822cc1a7656dee8ab9c44e03046c Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 4 Sep 2026 11:49:32 -0600 Subject: [PATCH 1/6] fenrir: 11089 Client throughput benchmark compares an unread receive buffer after timeout error out when tcp_select does not return ready to recive --- src/client/client.c | 66 ++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/src/client/client.c b/src/client/client.c index e4364b33..4cf3c21d 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -1162,40 +1162,52 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, /* Perform RX */ select_ret = tcp_select(sockfd, DEFAULT_TIMEOUT_SEC); - if (select_ret == TEST_RECV_READY) { - start = current_time(1); - rx_pos = 0; - while (rx_pos < len) { - ret = wolfSSL_read(ssl, &rx_buffer[rx_pos], - len - rx_pos); - if (ret <= 0) { - err = wolfSSL_get_error(ssl, 0); - #ifdef WOLFSSL_ASYNC_CRYPT - if (err == WC_PENDING_E) { - ret = wolfSSL_AsyncPoll(ssl, WOLF_POLL_FLAG_CHECK_HW); - if (ret < 0) break; - } - else - #endif - if (err != WOLFSSL_ERROR_WANT_READ) { - printf("SSL_read bench error %d\n", err); - err_sys("SSL_read failed"); - } + if (select_ret != TEST_RECV_READY) { + printf("SSL_read bench select error %d!\n", select_ret); + if (!exitWithRet) + err_sys("SSL_read timeout"); + err = WOLFSSL_FATAL_ERROR; + goto doExit; + } + + start = current_time(1); + rx_pos = 0; + while (rx_pos < len) { + ret = wolfSSL_read(ssl, &rx_buffer[rx_pos], + len - rx_pos); + if (ret <= 0) { + err = wolfSSL_get_error(ssl, 0); + #ifdef WOLFSSL_ASYNC_CRYPT + if (err == WC_PENDING_E) { + ret = wolfSSL_AsyncPoll(ssl, WOLF_POLL_FLAG_CHECK_HW); + if (ret < 0) break; } - else { - rx_pos += ret; + else + #endif + if (err != WOLFSSL_ERROR_WANT_READ) { + break; } } - rx_time += current_time(0) - start; + else { + rx_pos += ret; + } + } + rx_time += current_time(0) - start; + + /* Only compare once the full block has been received */ + if (rx_pos != len) { + printf("SSL_read bench error %d!\n", err); + if (!exitWithRet) + err_sys("SSL_read failed"); + goto doExit; } /* Compare TX and RX buffers */ if (XMEMCMP(tx_buffer, rx_buffer, len) != 0) { - free(tx_buffer); - tx_buffer = NULL; - free(rx_buffer); - rx_buffer = NULL; - err_sys("Compare TX and RX buffers failed"); + if (!exitWithRet) + err_sys("Compare TX and RX buffers failed"); + err = WOLFSSL_FATAL_ERROR; + goto doExit; } /* Update overall position */ From c2f26ee1a70cada4424c93eaaf1b2d60d129aa9f Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 4 Sep 2026 11:52:44 -0600 Subject: [PATCH 2/6] Fenrir: 9844 Client throughput sizes above INT_MAX become negative record lengths fix: increased the size of the variable to size_t --- src/client/client.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/client/client.c b/src/client/client.c index 4cf3c21d..54651024 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -1133,9 +1133,13 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, xfer_bytes = 0; while (throughput > xfer_bytes) { int len, rx_pos, select_ret; + size_t remain; - /* Determine packet size */ - len = min(block, (int)(throughput - xfer_bytes)); + /* Determine packet size. Bound the size_t remainder by + * the block size before narrowing it to int, otherwise a + * remainder above INT_MAX becomes a negative length. */ + remain = throughput - xfer_bytes; + len = (remain > (size_t)block) ? block : (int)remain; /* Perform TX */ start = current_time(1); @@ -2610,18 +2614,24 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) break; case 'B' : - throughput = atol(myoptarg); + { + long throughputArg = atol(myoptarg); + for (; *myoptarg != '\0'; myoptarg++) { if (*myoptarg == ',') { block = atoi(myoptarg + 1); break; } } - if (throughput == 0 || block <= 0) { + /* Reject non-positive and out-of-range values here so the + * benchmark never runs with a wrapped size_t throughput. */ + if (throughputArg <= 0 || block <= 0) { Usage(); XEXIT_T(MY_EX_USAGE); } + throughput = (size_t)throughputArg; break; + } case 'N' : nonBlocking = 1; From 43258f97338bb3b587a66c3f423101feb87be83b Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 4 Sep 2026 12:10:26 -0600 Subject: [PATCH 3/6] Fenrir: 5359 s_client port parser silently truncates ports above 65535 parse into long before loading in to word16 and report/error if it does not fit --- src/client/client.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/client/client.c b/src/client/client.c index 54651024..9934a4df 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -2502,12 +2502,19 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) break; case 'p' : - port = (word16)atoi(myoptarg); + { + long portArg; + if (wolfCLU_parseDecimalBounded(myoptarg, 0, 65535, &portArg) == + WOLFCLU_FATAL_ERROR) { + err_sys("port number must be between 0 and 65535"); + } + port = (word16)portArg; #if !defined(NO_MAIN_DRIVER) || defined(USE_WINDOWS_API) if (port == 0) err_sys("port number cannot be 0"); #endif break; + } case 'v' : if (myoptarg[0] == 'd') { From e4e3f3326b0876276f596cc9d784b33abe282f51 Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 4 Sep 2026 12:28:54 -0600 Subject: [PATCH 4/6] Fenrir: 9839 Ipv6 DTLS and SCTP connections use IPv4 sockets and omit the DTLS peer Fix: set the correct socket family and set the DTLS peer correctly --- src/client/client.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/client/client.c b/src/client/client.c index 9934a4df..aa4edb6d 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -311,22 +311,20 @@ static WC_INLINE void clu_build_addr(SOCKADDR_IN4_T* addr, SOCKADDR_IN6_T* ipv6, static WC_INLINE void clu_tcp_socket(SOCKET_T* sockfd, int udp, int sctp, int isIpv6) { + /* The address built by clu_build_addr() is AF_INET6 whenever isIpv6 is + * set, so every socket type has to be opened in the matching family. */ + int family = isIpv6 ? AF_INET6_V : AF_INET_V; + (void)sctp; if (udp) - *sockfd = socket(AF_INET_V, SOCK_DGRAM, IPPROTO_UDP); + *sockfd = socket(family, SOCK_DGRAM, IPPROTO_UDP); #ifdef WOLFSSL_SCTP else if (sctp) - *sockfd = socket(AF_INET_V, SOCK_STREAM, IPPROTO_SCTP); + *sockfd = socket(family, SOCK_STREAM, IPPROTO_SCTP); #endif - else { - if (isIpv6) { - *sockfd = socket(AF_INET6_V, SOCK_STREAM, IPPROTO_TCP); - } - else { - *sockfd = socket(AF_INET_V, SOCK_STREAM, IPPROTO_TCP); - } - } + else + *sockfd = socket(family, SOCK_STREAM, IPPROTO_TCP); if(WOLFSSL_SOCKET_IS_INVALID(*sockfd)) { err_sys_with_errno("socket failed\n"); } @@ -371,7 +369,11 @@ static WC_INLINE void clu_tcp_connect(SOCKET_T* sockfd, const char* ip, if (isIpv6) { clu_build_addr(NULL, &ipv6, ip, port, udp, sctp); + if (udp) { + wolfSSL_dtls_set_peer(ssl, &ipv6, sizeof(ipv6)); + } clu_tcp_socket(sockfd, udp, sctp, isIpv6); + if (!udp) { if (WOLFSSL_SOCKET_IS_INVALID(*sockfd)) err_sys_with_errno("tcp bad socket"); From 40996c481f9c2df3b49e178d46ea442520d3a63f Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 4 Sep 2026 12:59:11 -0600 Subject: [PATCH 5/6] Fenrir: 8074 Added free for allocation in zephr build swapped free for XFREE --- src/client/client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/client.c b/src/client/client.c index aa4edb6d..d8b1e535 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -236,6 +236,7 @@ static WC_INLINE void clu_build_addr(SOCKADDR_IN4_T* addr, SOCKADDR_IN6_T* ipv6, if (getaddrinfo((char*)peer, portStr, &hints, &addrInfo) == 0) { XMEMCPY(addr, addrInfo->ai_addr, sizeof(*addr)); useLookup = 1; + zsock_freeaddrinfo(addrInfo); } #endif else From d827339d9ef6374b3b120eab3dcf49a5654eadd3 Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 4 Sep 2026 13:52:45 -0600 Subject: [PATCH 6/6] Clinet.c bug fixes --- src/client/client.c | 81 +++++++++++++++++++++++++++---------- tests/client/client-test.py | 68 ++++++++++++++++++++++++++++--- tests/wolfclu_test.py | 14 +++++++ 3 files changed, 135 insertions(+), 28 deletions(-) diff --git a/src/client/client.c b/src/client/client.c index d8b1e535..3a1b794c 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -35,6 +35,7 @@ #include #include +#include #include @@ -95,6 +96,11 @@ static const char *wolfsentry_config_path = NULL; #define MAX_NON_BLOCK_SEC 10 #endif +/* Scratch buffer for a decimal command line argument. LONG_MAX is 19 decimal + * digits, so 24 leaves room for that plus a NUL and a little slack; anything + * longer is out of range for the values parsed with it. */ +#define MAX_DECIMAL_ARG_LEN 24 + #define OCSP_STAPLING 1 #define OCSP_STAPLINGV2 2 #define OCSP_STAPLINGV2_MULTI 3 @@ -236,7 +242,7 @@ static WC_INLINE void clu_build_addr(SOCKADDR_IN4_T* addr, SOCKADDR_IN6_T* ipv6, if (getaddrinfo((char*)peer, portStr, &hints, &addrInfo) == 0) { XMEMCPY(addr, addrInfo->ai_addr, sizeof(*addr)); useLookup = 1; - zsock_freeaddrinfo(addrInfo); + freeaddrinfo(addrInfo); } #endif else @@ -1106,6 +1112,7 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, if (ret == WOLFSSL_SUCCESS) { /* Perform throughput test */ char *tx_buffer, *rx_buffer; + const char* errMsg = NULL; /* Record connection time */ conn_time = current_time(0) - start; @@ -1161,8 +1168,9 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, } while (err == WC_PENDING_E); if (ret != len) { printf("SSL_write bench error %d!\n", err); - if (!exitWithRet) - err_sys("SSL_write failed"); + if (err == 0) + err = WOLFSSL_FATAL_ERROR; + errMsg = "SSL_write failed"; goto doExit; } tx_time += current_time(0) - start; @@ -1171,9 +1179,9 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, select_ret = tcp_select(sockfd, DEFAULT_TIMEOUT_SEC); if (select_ret != TEST_RECV_READY) { printf("SSL_read bench select error %d!\n", select_ret); - if (!exitWithRet) - err_sys("SSL_read timeout"); err = WOLFSSL_FATAL_ERROR; + errMsg = (select_ret == TEST_TIMEOUT) ? + "SSL_read timeout" : "SSL_read select failed"; goto doExit; } @@ -1204,16 +1212,19 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, /* Only compare once the full block has been received */ if (rx_pos != len) { printf("SSL_read bench error %d!\n", err); - if (!exitWithRet) - err_sys("SSL_read failed"); + /* wolfSSL_read() can return <= 0 without setting an + * error code, so normalize it here. Otherwise a short + * read would be returned as EXIT_SUCCESS under -H. */ + if (err == 0) + err = WOLFSSL_FATAL_ERROR; + errMsg = "SSL_read failed"; goto doExit; } /* Compare TX and RX buffers */ if (XMEMCMP(tx_buffer, rx_buffer, len) != 0) { - if (!exitWithRet) - err_sys("Compare TX and RX buffers failed"); err = WOLFSSL_FATAL_ERROR; + errMsg = "Compare TX and RX buffers failed"; goto doExit; } @@ -1232,6 +1243,10 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, doExit: if (tx_buffer) XFREE(tx_buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (rx_buffer) XFREE(rx_buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER); + /* err_sys() does not return, so only report the failure once the + * buffers have been released */ + if (errMsg != NULL && !exitWithRet) + err_sys(errMsg); } else { err_sys("wolfSSL_connect failed"); @@ -2506,10 +2521,14 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) case 'p' : { - long portArg; - if (wolfCLU_parseDecimalBounded(myoptarg, 0, 65535, &portArg) == - WOLFCLU_FATAL_ERROR) { - err_sys("port number must be between 0 and 65535"); + long portArg = 0; + /* 0 is not a usable port to connect to, and the port == 0 + * check below is compiled out of every wolfCLU build + * (wolfclu/client.h defines NO_MAIN_DRIVER), so reject it + * here. */ + if (wolfCLU_parseDecimalBounded(myoptarg, 1, 65535, &portArg) + != WOLFCLU_SUCCESS) { + err_sys("port number must be between 1 and 65535"); } port = (word16)portArg; #if !defined(NO_MAIN_DRIVER) || defined(USE_WINDOWS_API) @@ -2625,21 +2644,39 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) case 'B' : { - long throughputArg = atol(myoptarg); + long throughputArg = 0; + long blockArg = (long)block; + const char* comma = XSTRSTR(myoptarg, ","); + size_t numLen = (comma != NULL) ? + (size_t)(comma - myoptarg) : XSTRLEN(myoptarg); + char numBuf[MAX_DECIMAL_ARG_LEN]; + + /* Parse both halves with the bounded decimal parser so that + * negative, out-of-range and trailing-junk input is rejected + * instead of silently saturating or truncating the way + * atol()/atoi() do, which would let the benchmark run with a + * wrapped size_t throughput. */ + if (numLen == 0 || numLen >= sizeof(numBuf)) { + Usage(); + XEXIT_T(MY_EX_USAGE); + } + XMEMCPY(numBuf, myoptarg, numLen); + numBuf[numLen] = '\0'; - for (; *myoptarg != '\0'; myoptarg++) { - if (*myoptarg == ',') { - block = atoi(myoptarg + 1); - break; - } + if (wolfCLU_parseDecimalBounded(numBuf, 1, LONG_MAX, + &throughputArg) != WOLFCLU_SUCCESS) { + Usage(); + XEXIT_T(MY_EX_USAGE); } - /* Reject non-positive and out-of-range values here so the - * benchmark never runs with a wrapped size_t throughput. */ - if (throughputArg <= 0 || block <= 0) { + if (comma != NULL && + wolfCLU_parseDecimalBounded(comma + 1, 1, INT_MAX, + &blockArg) != WOLFCLU_SUCCESS) { Usage(); XEXIT_T(MY_EX_USAGE); } + throughput = (size_t)throughputArg; + block = (int)blockArg; break; } diff --git a/tests/client/client-test.py b/tests/client/client-test.py index 4fbca71d..c83d08e2 100644 --- a/tests/client/client-test.py +++ b/tests/client/client-test.py @@ -7,7 +7,8 @@ import unittest sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) -from wolfclu_test import WOLFSSL_BIN, CERTS_DIR, run_wolfssl, test_main +from wolfclu_test import (WOLFSSL_BIN, CERTS_DIR, run_wolfssl, + skip_if_no_filesystem, test_main) class ClientTest(unittest.TestCase): @@ -17,11 +18,7 @@ def setUpClass(cls): if not os.path.isdir(CERTS_DIR): raise unittest.SkipTest("certs directory not found") - config_log = os.path.join(".", "config.log") - if os.path.isfile(config_log): - with open(config_log, "r") as f: - if "disable-filesystem" in f.read(): - raise unittest.SkipTest("filesystem support disabled") + skip_if_no_filesystem() def test_s_client_x509(self): """Connect to a TLS server, extract cert, and verify PEM output.""" @@ -66,6 +63,65 @@ def test_client_help(self): self.assertEqual(r.returncode, 0, r.stderr) self.assertIn("s_client" , r.stderr, "help menu was not printed") +class PortArgTest(unittest.TestCase): + """Regression tests for the port half of -connect :. + + -connect forwards the text after ':' to the client's -p handler, which + used to be (word16)atoi(): "99999" silently truncated to 34463 and the + client happily connected to the wrong port. The port is now parsed with + wolfCLU_parseDecimalBounded(), so anything outside 1-65535, and anything + that is not a plain decimal number, is rejected before any connection is + attempted. + """ + + PORT_ERROR = "port number must be between 1 and 65535" + + @classmethod + def setUpClass(cls): + # s_client is compiled out under --disable-filesystem, so it never + # reaches the port parser and never prints PORT_ERROR. + skip_if_no_filesystem() + + def _assert_rejected(self, port, description): + """Run s_client against localhost: and require that the port + itself was rejected -- a non-zero exit alone is not enough, since a + truncated port would also fail to connect.""" + r = run_wolfssl("s_client", "-connect", "localhost:" + port, + timeout=30) + self.assertNotEqual(r.returncode, 0, + f"{description} port {port!r} was accepted") + self.assertIn(self.PORT_ERROR, r.stdout + r.stderr, + f"{description} port {port!r} was not rejected as an " + f"out-of-range port: {r.stdout + r.stderr}") + + def test_port_zero(self): + """0 must be rejected: it is not a port that can be connected to.""" + self._assert_rejected("0", "zero") + + def test_port_above_word16(self): + """99999 must be rejected, not truncated to 34463.""" + self._assert_rejected("99999", "out-of-range") + + def test_port_just_above_word16(self): + """65536 must be rejected, not truncated to 0.""" + self._assert_rejected("65536", "out-of-range") + + def test_port_negative(self): + """A negative port must be rejected, not wrapped.""" + self._assert_rejected("-1", "negative") + + def test_port_non_numeric(self): + """A non-numeric port must be rejected, not read as 0.""" + self._assert_rejected("abc", "non-numeric") + + def test_port_trailing_junk(self): + """Trailing junk must be rejected, not silently ignored.""" + self._assert_rejected("443abc", "trailing junk") + + def test_port_empty(self): + """An empty port must be rejected.""" + self._assert_rejected("", "empty") + class ShellInjectionTest(unittest.TestCase): """Regression tests for shell command injection via hostname. diff --git a/tests/wolfclu_test.py b/tests/wolfclu_test.py index 6909c2e3..5a13a3ee 100644 --- a/tests/wolfclu_test.py +++ b/tests/wolfclu_test.py @@ -88,6 +88,20 @@ def run_wolfssl(*args, stdin_data=None, timeout=60): return subprocess.run(cmd, **kwargs) +def skip_if_no_filesystem(): + """Raise SkipTest when wolfCLU was built with --disable-filesystem. + + WOLFCLU_NO_FILESYSTEM compiles out the front ends that need files (e.g. + wolfCLU_Client), so those commands only print "No filesystem support" + instead of doing any work. + """ + config_log = os.path.join(".", "config.log") + if os.path.isfile(config_log): + with open(config_log, "r") as f: + if "disable-filesystem" in f.read(): + raise unittest.SkipTest("filesystem support disabled") + + def is_fips(): """True when linked against a FIPS wolfSSL build (per `wolfssl -v`).""" r = run_wolfssl("-v")