From 5d5aa129ca2ee85c6ec0f444ac7b2fb89786f479 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 20 Jul 2020 16:14:53 -0700 Subject: [PATCH 1/4] When attempting to send a message with DTLS, if it is too large, return an error rather than splitting it across records. (ZD 10602) --- src/internal.c | 13 +++++++++---- wolfssl/error-ssl.h | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index 36eed125f..39af97f69 100644 --- a/src/internal.c +++ b/src/internal.c @@ -17709,9 +17709,11 @@ int SendData(WOLFSSL* ssl, const void* data, int sz) len = wolfSSL_GetMaxRecordSize(ssl, sz - sent); -#ifdef WOLFSSL_DTLS - if (IsDtlsNotSctpMode(ssl)) { - len = min(len, MAX_UDP_SIZE); +#if defined(WOLFSSL_DTLS) && !defined(WOLFSSL_NO_DTLS_SIZE_CHECK) + if (ssl->options.dtls && (len < sz - sent)) { + ssl->error = DTLS_SIZE_ERROR; + WOLFSSL_ERROR(ssl->error); + return ssl->error; } #endif buffSz = len; @@ -18439,6 +18441,9 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e) case TLS13_SECRET_CB_E: return "TLS1.3 Secret Callback Error"; + case DTLS_SIZE_ERROR: + return "DTLS trying to send too much in single datagram error"; + default : return "unknown error number"; } @@ -29977,7 +29982,7 @@ int wolfSSL_GetMaxRecordSize(WOLFSSL* ssl, int maxFragment) } #endif /* HAVE_MAX_FRAGMENT */ #ifdef WOLFSSL_DTLS - if ((ssl->options.dtls) && (maxFragment > MAX_UDP_SIZE)) { + if (IsDtlsNotSctpMode(ssl) && (maxFragment > MAX_UDP_SIZE)) { maxFragment = MAX_UDP_SIZE; } #endif diff --git a/wolfssl/error-ssl.h b/wolfssl/error-ssl.h index 9b44326e7..9478242aa 100644 --- a/wolfssl/error-ssl.h +++ b/wolfssl/error-ssl.h @@ -167,6 +167,7 @@ enum wolfSSL_ErrorCodes { CLIENT_CERT_CB_ERROR = -436, /* Client cert callback error */ SSL_SHUTDOWN_ALREADY_DONE_E = -437, /* Shutdown called redundantly */ TLS13_SECRET_CB_E = -438, /* TLS1.3 secret Cb fcn failure */ + DTLS_SIZE_ERROR = -439, /* Trying to send too much data */ /* add strings to wolfSSL_ERR_reason_error_string in internal.c !!!!! */ From 98ae3a2352dd42e4c40f518fcbbbdda6c6c55e94 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 22 Jul 2020 13:20:23 -0700 Subject: [PATCH 2/4] Added a suite test use case to cover the new error check. Also fixed and issue with passing a couple flags to the test case runner, and some other changes to support the new test. --- examples/client/client.c | 22 ++++++++++++++++++---- examples/server/server.c | 18 ++++++++++++++++-- tests/include.am | 1 + tests/suites.c | 17 ++++++++++++++++- tests/test-dtls-fails.conf | 16 ++++++++++++++++ tests/test-fails.conf | 9 ++++++++- 6 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 tests/test-dtls-fails.conf diff --git a/examples/client/client.c b/examples/client/client.c index fcab9d5f2..c87429b77 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -486,7 +486,7 @@ static int ClientBenchmarkConnections(WOLFSSL_CTX* ctx, char* host, word16 port, /* Measures throughput in kbps. Throughput = number of bytes */ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, int dtlsUDP, int dtlsSCTP, int block, size_t throughput, int useX25519, - int useX448) + int useX448, int exitWithRet) { double start, conn_time = 0, tx_time = 0, rx_time = 0; SOCKET_T sockfd; @@ -591,7 +591,10 @@ 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); - err_sys("SSL_write failed"); + if (!exitWithRet) + err_sys("SSL_write failed"); + ret = err; + goto doExit; } tx_time += current_time(0) - start; @@ -645,6 +648,7 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, else { err_sys("Client buffer malloc failed"); } +doExit: if(tx_buffer) XFREE(tx_buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER); if(rx_buffer) XFREE(rx_buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER); } @@ -656,6 +660,9 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, wolfSSL_free(ssl); ssl = NULL; CloseSocket(sockfd); + if (exitWithRet) + return err; + #if !defined(__MINGW32__) printf("wolfSSL Client Benchmark %zu bytes\n" #else @@ -1595,6 +1602,9 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) StackTrap(); + /* Reinitialize the global myVerifyAction. */ + myVerifyAction = VERIFY_OVERRIDE_ERROR; + #ifndef WOLFSSL_VXWORKS /* Not used: All used */ while ((ch = mygetopt(argc, argv, "?:" @@ -2613,9 +2623,13 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) if (throughput) { ((func_args*)args)->return_code = ClientBenchmarkThroughput(ctx, host, port, dtlsUDP, dtlsSCTP, - block, throughput, useX25519, useX448); + block, throughput, useX25519, useX448, + exitWithRet); wolfSSL_CTX_free(ctx); ctx = NULL; - XEXIT_T(EXIT_SUCCESS); + if (!exitWithRet) + XEXIT_T(EXIT_SUCCESS); + else + goto exit; } #if defined(WOLFSSL_MDK_ARM) diff --git a/examples/server/server.c b/examples/server/server.c index c575f2f88..5394c1681 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -381,6 +381,8 @@ int ServerEchoData(SSL* ssl, int clientfd, int echoData, int block, err_sys_ex(runWithErrors, "SSL_read failed"); break; } + if (err == WOLFSSL_ERROR_ZERO_RETURN) + return WOLFSSL_ERROR_ZERO_RETURN; } else { rx_pos += ret; @@ -438,7 +440,7 @@ int ServerEchoData(SSL* ssl, int clientfd, int echoData, int block, ); } - return EXIT_SUCCESS; + return 0; } static void ServerRead(WOLFSSL* ssl, char* input, int inputLen) @@ -1097,6 +1099,10 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) #ifdef WOLFSSL_VXWORKS useAnyAddr = 1; #else + + /* Reinitialize the global myVerifyAction. */ + myVerifyAction = VERIFY_OVERRIDE_ERROR; + /* Not Used: h, z, F, T, V, W, X */ while ((ch = mygetopt(argc, argv, "?:" "abc:defgijk:l:mnop:q:rstuv:wxy" @@ -2446,7 +2452,15 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) #endif } else if (err == 0 || err == WOLFSSL_ERROR_ZERO_RETURN) { - ServerEchoData(ssl, clientfd, echoData, block, throughput); + err = ServerEchoData(ssl, clientfd, echoData, block, throughput); + if (err != 0) { + SSL_free(ssl); ssl = NULL; + SSL_CTX_free(ctx); ctx = NULL; + CloseSocket(clientfd); + CloseSocket(sockfd); + ((func_args*)args)->return_code = err; + goto exit; + } } #if defined(WOLFSSL_MDK_SHELL) && defined(HAVE_MDK_RTX) diff --git a/tests/include.am b/tests/include.am index 07230abf3..1ef0a7cdf 100644 --- a/tests/include.am +++ b/tests/include.am @@ -31,6 +31,7 @@ EXTRA_DIST += tests/test.conf \ tests/test-psk-no-id.conf \ tests/test-psk-no-id-sha2.conf \ tests/test-dtls.conf \ + tests/test-dtls-fails.conf \ tests/test-dtls-group.conf \ tests/test-dtls-reneg-client.conf \ tests/test-dtls-reneg-server.conf \ diff --git a/tests/suites.c b/tests/suites.c index d4e4107af..72ae8fddf 100644 --- a/tests/suites.c +++ b/tests/suites.c @@ -455,6 +455,7 @@ static int execute_test_case(int svr_argc, char** svr_argv, return NOT_BUILT_IN; } printf("trying client command line[%d]: %s\n", tests, commandLine); + tests++; /* determine based on args if this test is expected to fail */ if (XSTRSTR(commandLine, exitWithRetFlag) != NULL) { @@ -881,6 +882,20 @@ int SuiteTest(int argc, char** argv) goto exit; } #endif +#ifndef WOLFSSL_NO_DTLS_SIZE_CHECK + /* failure tests */ + args.argc = 3; + strcpy(argv0[1], "tests/test-dtls-fails.conf"); + strcpy(argv0[2], "expFail"); /* tests are expected to fail */ + printf("starting dtls tests that expect failure\n"); + test_harness(&args); + if (args.return_code != 0) { + printf("error from script %d\n", args.return_code); + args.return_code = EXIT_FAILURE; + goto exit; + } + strcpy(argv0[2], ""); +#endif #endif #ifdef WOLFSSL_SCTP /* add dtls-sctp extra suites */ @@ -1038,7 +1053,7 @@ int SuiteTest(int argc, char** argv) args.argc = 3; strcpy(argv0[1], "tests/test-dhprime.conf"); strcpy(argv0[2], "doDH"); /* add DH prime flag */ - printf("starting tests that expect failure\n"); + printf("starting dh prime tests\n"); test_harness(&args); if (args.return_code != 0) { printf("error from script %d\n", args.return_code); diff --git a/tests/test-dtls-fails.conf b/tests/test-dtls-fails.conf new file mode 100644 index 000000000..07492f2f7 --- /dev/null +++ b/tests/test-dtls-fails.conf @@ -0,0 +1,16 @@ +# DTLS test +# server DTLSv1.2 too big test +-v 3 +-l ECDHE-ECDSA-AES128-SHA256 +-c ./certs/server-ecc.pem +-k ./certs/ecc-key.pem +-u +-B 9000 + +# client DTLSv1.2 too big test +-v 3 +-l ECDHE-ECDSA-AES128-SHA256 +-A ./certs/ca-ecc-cert.pem +-u +-B 9000 + diff --git a/tests/test-fails.conf b/tests/test-fails.conf index d1dd44417..40afb54e0 100644 --- a/tests/test-fails.conf +++ b/tests/test-fails.conf @@ -114,6 +114,7 @@ # server -v 3 -l ECDHE-RSA-AES128-GCM-SHA256 +-H verifyFail # client verify should fail -v 3 @@ -128,10 +129,12 @@ # client -v 3 -l ECDHE-RSA-AES128-GCM-SHA256 +-H verifyFail # server -v 3 -l ECDHE-ECDSA-AES128-GCM-SHA256 +-H verifyFail # client verify should fail -v 3 @@ -146,6 +149,7 @@ # client -v 3 -l ECDHE-ECDSA-AES128-GCM-SHA256 +-H verifyFail # error going into callback, return error # server @@ -153,6 +157,7 @@ -l ECDHE-RSA-AES128-GCM-SHA256 -c ./certs/test/server-cert-rsa-badsig.pem -k ./certs/server-key.pem +-H verifyFail # client verify should fail -v 3 @@ -164,6 +169,7 @@ -l ECDHE-ECDSA-AES128-GCM-SHA256 -c ./certs/test/server-cert-ecc-badsig.pem -k ./certs/ecc-key.pem +-H verifyFail # client verify should fail -v 3 @@ -173,10 +179,12 @@ # server send alert on no mutual authentication -v 3 -F +-H verifyFail # client send alert on no mutual authentication -v 3 -x +-H verifyFail # server TLSv1.3 fail on no client certificate # server always sets WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT unless using -d @@ -187,4 +195,3 @@ -v 4 -l TLS13-AES128-GCM-SHA256 -x - From 839044d9e11c4897b0f5c9c3ec50f9ceee3417cb Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 23 Jul 2020 12:26:49 -0700 Subject: [PATCH 3/4] 1. Remove dead assignment from client test. 2. Fix memory leak in example server test. 3. Use verify callback on certificates to allow callback to fail them. 4. Restore the forced failure test cases. 5. Make the verify action thread local. --- examples/client/client.c | 1 - examples/server/server.c | 7 +++++-- src/internal.c | 2 ++ tests/test-fails.conf | 9 +-------- wolfssl/test.h | 2 +- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index c87429b77..486508629 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -593,7 +593,6 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port, printf("SSL_write bench error %d!\n", err); if (!exitWithRet) err_sys("SSL_write failed"); - ret = err; goto doExit; } tx_time += current_time(0) - start; diff --git a/examples/server/server.c b/examples/server/server.c index 5394c1681..44bc31348 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -381,8 +381,10 @@ int ServerEchoData(SSL* ssl, int clientfd, int echoData, int block, err_sys_ex(runWithErrors, "SSL_read failed"); break; } - if (err == WOLFSSL_ERROR_ZERO_RETURN) + if (err == WOLFSSL_ERROR_ZERO_RETURN) { + free(buffer); return WOLFSSL_ERROR_ZERO_RETURN; + } } else { rx_pos += ret; @@ -1813,7 +1815,8 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) SSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER | (usePskPlus ? WOLFSSL_VERIFY_FAIL_EXCEPT_PSK : WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT), - myVerifyAction == VERIFY_OVERRIDE_DATE_ERR ? myVerify : NULL); + (myVerifyAction == VERIFY_OVERRIDE_DATE_ERR || + myVerifyAction == VERIFY_FORCE_FAIL) ? myVerify : NULL); #ifdef TEST_BEFORE_DATE verify_flags |= WOLFSSL_LOAD_FLAG_DATE_ERR_OKAY; diff --git a/src/internal.c b/src/internal.c index 39af97f69..29d9a5748 100644 --- a/src/internal.c +++ b/src/internal.c @@ -10007,6 +10007,8 @@ int DoVerifyCallback(WOLFSSL_CERT_MANAGER* cm, WOLFSSL* ssl, int ret, /* Determine if verify was okay */ if (ret == 0) { verify_ok = 1; + use_cb = 1; /* use verify callback on success, in case callback + * could force fail a cert */ } /* Determine if verify callback should be used */ diff --git a/tests/test-fails.conf b/tests/test-fails.conf index 40afb54e0..d1dd44417 100644 --- a/tests/test-fails.conf +++ b/tests/test-fails.conf @@ -114,7 +114,6 @@ # server -v 3 -l ECDHE-RSA-AES128-GCM-SHA256 --H verifyFail # client verify should fail -v 3 @@ -129,12 +128,10 @@ # client -v 3 -l ECDHE-RSA-AES128-GCM-SHA256 --H verifyFail # server -v 3 -l ECDHE-ECDSA-AES128-GCM-SHA256 --H verifyFail # client verify should fail -v 3 @@ -149,7 +146,6 @@ # client -v 3 -l ECDHE-ECDSA-AES128-GCM-SHA256 --H verifyFail # error going into callback, return error # server @@ -157,7 +153,6 @@ -l ECDHE-RSA-AES128-GCM-SHA256 -c ./certs/test/server-cert-rsa-badsig.pem -k ./certs/server-key.pem --H verifyFail # client verify should fail -v 3 @@ -169,7 +164,6 @@ -l ECDHE-ECDSA-AES128-GCM-SHA256 -c ./certs/test/server-cert-ecc-badsig.pem -k ./certs/ecc-key.pem --H verifyFail # client verify should fail -v 3 @@ -179,12 +173,10 @@ # server send alert on no mutual authentication -v 3 -F --H verifyFail # client send alert on no mutual authentication -v 3 -x --H verifyFail # server TLSv1.3 fail on no client certificate # server always sets WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT unless using -d @@ -195,3 +187,4 @@ -v 4 -l TLS13-AES128-GCM-SHA256 -x + diff --git a/wolfssl/test.h b/wolfssl/test.h index 385a3be21..aff5c4d65 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1684,7 +1684,7 @@ enum { VERIFY_USE_PREVERFIY, VERIFY_OVERRIDE_DATE_ERR, }; -static int myVerifyAction = VERIFY_OVERRIDE_ERROR; +static THREAD_LS_T int myVerifyAction = VERIFY_OVERRIDE_ERROR; /* The verify callback is called for every certificate only when * --enable-opensslextra is defined because it sets WOLFSSL_ALWAYS_VERIFY_CB and From fd1a1bd0f77df159cfd080bf27f60afb12b1cbb2 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 23 Jul 2020 14:32:48 -0700 Subject: [PATCH 4/4] Add some missing frees to the example client when using in the return-not-exit mode for tests. --- examples/client/client.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/client/client.c b/examples/client/client.c index 486508629..fd62cb59f 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -3165,12 +3165,16 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) err = ClientWrite(ssl, msg, msgSz, "", exitWithRet); if (exitWithRet && (err != 0)) { ((func_args*)args)->return_code = err; + wolfSSL_free(ssl); ssl = NULL; + wolfSSL_CTX_free(ctx); ctx = NULL; goto exit; } err = ClientRead(ssl, reply, sizeof(reply)-1, 1, "", exitWithRet); if (exitWithRet && (err != 0)) { ((func_args*)args)->return_code = err; + wolfSSL_free(ssl); ssl = NULL; + wolfSSL_CTX_free(ctx); ctx = NULL; goto exit; }