From a31b76878fed857f8402e322d1922b6fa4861836 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 26 Apr 2022 15:51:50 +0200 Subject: [PATCH 1/3] DTLS fixes with WANT_WRITE simulations - WANT_WRITE could be returned in unexpected places. This patch takes care of that. - Change state after SendBuffered only if in a sending state to begin with. - Adapt client and server to simulate WANT_WRITE with DTLS --- examples/client/client.c | 13 ++++++++++--- examples/server/server.c | 18 +++++++++++++++--- src/internal.c | 20 ++++++++++++++++++-- src/ssl.c | 30 ++++++++++++++++++++++++------ src/tls13.c | 34 ++++++++++++++++++++++++++++------ wolfssl/test.h | 6 ++---- 6 files changed, 97 insertions(+), 24 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 3d2fbac33..1aa9c0b1d 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -232,9 +232,12 @@ static int NonBlockingSSL_Connect(WOLFSSL* ssl) } } #ifdef WOLFSSL_DTLS - else if (select_ret == TEST_TIMEOUT && wolfSSL_dtls(ssl) && - wolfSSL_dtls_got_timeout(ssl) >= 0) { - error = WOLFSSL_ERROR_WANT_READ; + else if (select_ret == TEST_TIMEOUT && wolfSSL_dtls(ssl)) { + ret = wolfSSL_dtls_got_timeout(ssl); + if (ret != WOLFSSL_SUCCESS) + error = wolfSSL_get_error(ssl, ret); + else + error = WOLFSSL_ERROR_WANT_READ; } #endif else { @@ -3561,6 +3564,10 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) err_sys("error in setting fd"); } + if (simulateWantWrite) { + wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + } + /* STARTTLS */ if (doSTARTTLS) { if (StartTLS_Init(&sockfd) != WOLFSSL_SUCCESS) { diff --git a/examples/server/server.c b/examples/server/server.c index a4335f10b..11945dec5 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -354,9 +354,12 @@ static int NonBlockingSSL_Accept(SSL* ssl) error = WOLFSSL_ERROR_WANT_READ; } #ifdef WOLFSSL_DTLS - else if (select_ret == TEST_TIMEOUT && wolfSSL_dtls(ssl) && - wolfSSL_dtls_got_timeout(ssl) >= 0) { - error = WOLFSSL_ERROR_WANT_READ; + else if (select_ret == TEST_TIMEOUT && wolfSSL_dtls(ssl)) { + ret = wolfSSL_dtls_got_timeout(ssl); + if (ret != WOLFSSL_SUCCESS) + error = wolfSSL_get_error(ssl, ret); + else + error = WOLFSSL_ERROR_WANT_READ; } #endif else { @@ -2958,6 +2961,15 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) else { wolfSSL_dtls_set_peer(ssl, &client_addr, client_len); } + if (simulateWantWrite) { + /* connect on a udp to associate peer with this fd to make it + * simpler for SimulateWantWriteIOSendCb */ + if (connect(clientfd, (struct sockaddr*)&client_addr, + client_len) != 0) { + err_sys_ex(catastrophic, "error in connecting to peer"); + } + wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + } } #endif diff --git a/src/internal.c b/src/internal.c index c2d631820..3b76a3c73 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8883,9 +8883,8 @@ static int SendHandshakeMsg(WOLFSSL* ssl, byte* input, word32 inputSz, dataSz += DTLS_HANDSHAKE_HEADER_SZ; AddHandShakeHeader(data, inputSz, ssl->fragOffset, fragSz, type, ssl); - } - if (ssl->options.dtls) ssl->keys.dtls_handshake_number--; + } if (IsDtlsNotSctpMode(ssl) && (ret = DtlsMsgPoolSave(ssl, data, fragSz + DTLS_HANDSHAKE_HEADER_SZ, type)) @@ -17479,6 +17478,11 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) #endif } if (ret != 0 + /* DoDtlsHandShakeMsg can return a WANT_WRITE when + * calling DtlsMsgPoolSend. This msg is done + * processing so let's move on. */ + && (!ssl->options.dtls + || ret != WANT_WRITE) #ifdef WOLFSSL_ASYNC_CRYPT /* In async case, on pending, move onto next message. * Current message should have been DtlsMsgStore'ed and @@ -26783,6 +26787,12 @@ int SendCertificateVerify(WOLFSSL* ssl) if (args->output == NULL) { ERROR_OUT(BUFFER_ERROR, exit_scv); } + #ifdef WOLFSSL_DTLS + /* We have re-entered this funtion after a WANT_WRITE. Make sure + * the handshake number stays the same. */ + if (ssl->options.dtls && ssl->fragOffset != 0) + ssl->keys.dtls_handshake_number--; + #endif AddHeaders(args->output, (word32)args->length + args->extraSz + VERIFY_HEADER, certificate_verify, ssl); @@ -28871,6 +28881,12 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, case TLS_ASYNC_FINALIZE: { + #ifdef WOLFSSL_DTLS + /* We have re-entered this funtion after a WANT_WRITE. Make sure + * the handshake number stays the same. */ + if (ssl->options.dtls && ssl->fragOffset != 0) + ssl->keys.dtls_handshake_number--; + #endif #if defined(HAVE_ECC) || defined(HAVE_CURVE25519) || \ defined(HAVE_CURVE448) if (ssl->specs.kea == ecdhe_psk_kea || diff --git a/src/ssl.c b/src/ssl.c index 02302eb84..83488770e 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11836,9 +11836,17 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, * fragment, fragOffset is zero again, and the state can be * advanced. */ if (ssl->fragOffset == 0) { - ssl->options.connectState++; - WOLFSSL_MSG("connect state: " - "Advanced from last buffered fragment send"); + if (ssl->options.connectState == CONNECT_BEGIN || + ssl->options.connectState == HELLO_AGAIN || + ssl->options.connectState == FIRST_REPLY_DONE || + ssl->options.connectState == FIRST_REPLY_FIRST || + ssl->options.connectState == FIRST_REPLY_SECOND || + ssl->options.connectState == FIRST_REPLY_THIRD || + ssl->options.connectState == FIRST_REPLY_FOURTH) { + ssl->options.connectState++; + WOLFSSL_MSG("connect state: " + "Advanced from last buffered fragment send"); + } } else { WOLFSSL_MSG("connect state: " @@ -12303,9 +12311,19 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, * fragment, fragOffset is zero again, and the state can be * advanced. */ if (ssl->fragOffset == 0) { - ssl->options.acceptState++; - WOLFSSL_MSG("accept state: " - "Advanced from last buffered fragment send"); + if (ssl->options.acceptState == ACCEPT_FIRST_REPLY_DONE || + ssl->options.acceptState == SERVER_HELLO_SENT || + ssl->options.acceptState == CERT_SENT || + ssl->options.acceptState == CERT_STATUS_SENT || + ssl->options.acceptState == KEY_EXCHANGE_SENT || + ssl->options.acceptState == CERT_REQ_SENT || + ssl->options.acceptState == ACCEPT_SECOND_REPLY_DONE || + ssl->options.acceptState == TICKET_SENT || + ssl->options.acceptState == CHANGE_CIPHER_SENT) { + ssl->options.acceptState++; + WOLFSSL_MSG("accept state: " + "Advanced from last buffered fragment send"); + } } else { WOLFSSL_MSG("accept state: " diff --git a/src/tls13.c b/src/tls13.c index 555c03ac2..ea00180b2 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -8637,9 +8637,18 @@ int wolfSSL_connect_TLSv13(WOLFSSL* ssl) * fragment, fragOffset is zero again, and the state can be * advanced. */ if (ssl->fragOffset == 0) { - ssl->options.connectState++; - WOLFSSL_MSG("connect state: " - "Advanced from last buffered fragment send"); + /* Only increment from states in which we send data */ + if (ssl->options.connectState == CONNECT_BEGIN || + ssl->options.connectState == HELLO_AGAIN || + ssl->options.connectState == FIRST_REPLY_DONE || + ssl->options.connectState == FIRST_REPLY_FIRST || + ssl->options.connectState == FIRST_REPLY_SECOND || + ssl->options.connectState == FIRST_REPLY_THIRD || + ssl->options.connectState == FIRST_REPLY_FOURTH) { + ssl->options.connectState++; + WOLFSSL_MSG("connect state: " + "Advanced from last buffered fragment send"); + } } else { WOLFSSL_MSG("connect state: " @@ -9592,9 +9601,22 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl) * fragment, fragOffset is zero again, and the state can be * advanced. */ if (ssl->fragOffset == 0) { - ssl->options.acceptState++; - WOLFSSL_MSG("accept state: " - "Advanced from last buffered fragment send"); + /* Only increment from states in which we send data */ + if (ssl->options.acceptState == TLS13_ACCEPT_CLIENT_HELLO_DONE || + ssl->options.acceptState == TLS13_ACCEPT_HELLO_RETRY_REQUEST_DONE || + ssl->options.acceptState == TLS13_ACCEPT_SECOND_REPLY_DONE || + ssl->options.acceptState == TLS13_SERVER_HELLO_SENT || + ssl->options.acceptState == TLS13_ACCEPT_THIRD_REPLY_DONE || + ssl->options.acceptState == TLS13_SERVER_EXTENSIONS_SENT || + ssl->options.acceptState == TLS13_CERT_REQ_SENT || + ssl->options.acceptState == TLS13_CERT_SENT || + ssl->options.acceptState == TLS13_CERT_VERIFY_SENT || + ssl->options.acceptState == TLS13_ACCEPT_FINISHED_SENT || + ssl->options.acceptState == TLS13_ACCEPT_FINISHED_DONE) { + ssl->options.acceptState++; + WOLFSSL_MSG("accept state: " + "Advanced from last buffered fragment send"); + } } else { WOLFSSL_MSG("accept state: " diff --git a/wolfssl/test.h b/wolfssl/test.h index 529b33419..d912cf967 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1847,10 +1847,8 @@ static WC_INLINE void tcp_connect(SOCKET_T* sockfd, const char* ip, word16 port, } tcp_socket(sockfd, udp, sctp); - if (!udp) { - if (connect(*sockfd, (const struct sockaddr*)&addr, sizeof(addr)) != 0) - err_sys_with_errno("tcp connect failed"); - } + if (connect(*sockfd, (const struct sockaddr*)&addr, sizeof(addr)) != 0) + err_sys_with_errno("tcp connect failed"); } #endif /* WOLFSSL_WOLFSENTRY_HOOKS */ From 9914da30467f9e766f8b1a7ca0edb190e11536d8 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 27 Apr 2022 16:55:27 +0200 Subject: [PATCH 2/3] Fix resumption failure and use range in connect state logic --- examples/client/client.c | 3 +++ examples/server/server.c | 2 ++ src/ssl.c | 7 ++----- src/tls13.c | 7 ++----- wolfssl/test.h | 4 ++-- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 1aa9c0b1d..acafd71a8 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -4104,6 +4104,9 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) wolfSSL_CTX_free(ctx); ctx = NULL; err_sys("error in setting fd"); } + if (simulateWantWrite) { + wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + } #ifdef HAVE_ALPN if (alpnList != NULL) { printf("ALPN accepted protocols list : %s\n", alpnList); diff --git a/examples/server/server.c b/examples/server/server.c index 11945dec5..bd7f6db4f 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -2962,6 +2962,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) wolfSSL_dtls_set_peer(ssl, &client_addr, client_len); } if (simulateWantWrite) { +#ifdef USE_WOLFSSL_IO /* connect on a udp to associate peer with this fd to make it * simpler for SimulateWantWriteIOSendCb */ if (connect(clientfd, (struct sockaddr*)&client_addr, @@ -2969,6 +2970,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) err_sys_ex(catastrophic, "error in connecting to peer"); } wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); +#endif } } #endif diff --git a/src/ssl.c b/src/ssl.c index 83488770e..e38decf5c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11838,11 +11838,8 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, if (ssl->fragOffset == 0) { if (ssl->options.connectState == CONNECT_BEGIN || ssl->options.connectState == HELLO_AGAIN || - ssl->options.connectState == FIRST_REPLY_DONE || - ssl->options.connectState == FIRST_REPLY_FIRST || - ssl->options.connectState == FIRST_REPLY_SECOND || - ssl->options.connectState == FIRST_REPLY_THIRD || - ssl->options.connectState == FIRST_REPLY_FOURTH) { + (ssl->options.connectState >= FIRST_REPLY_DONE && + ssl->options.connectState <= FIRST_REPLY_FOURTH)) { ssl->options.connectState++; WOLFSSL_MSG("connect state: " "Advanced from last buffered fragment send"); diff --git a/src/tls13.c b/src/tls13.c index ea00180b2..99656fb36 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -8640,11 +8640,8 @@ int wolfSSL_connect_TLSv13(WOLFSSL* ssl) /* Only increment from states in which we send data */ if (ssl->options.connectState == CONNECT_BEGIN || ssl->options.connectState == HELLO_AGAIN || - ssl->options.connectState == FIRST_REPLY_DONE || - ssl->options.connectState == FIRST_REPLY_FIRST || - ssl->options.connectState == FIRST_REPLY_SECOND || - ssl->options.connectState == FIRST_REPLY_THIRD || - ssl->options.connectState == FIRST_REPLY_FOURTH) { + (ssl->options.connectState >= FIRST_REPLY_DONE && + ssl->options.connectState <= FIRST_REPLY_FOURTH)) { ssl->options.connectState++; WOLFSSL_MSG("connect state: " "Advanced from last buffered fragment send"); diff --git a/wolfssl/test.h b/wolfssl/test.h index d912cf967..471c990f0 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -2050,7 +2050,7 @@ static WC_INLINE void udp_accept(SOCKET_T* sockfd, SOCKET_T* clientfd, if (bind(*sockfd, (const struct sockaddr*)&addr, sizeof(addr)) != 0) err_sys_with_errno("tcp bind failed"); - #if (defined(NO_MAIN_DRIVER) && !defined(USE_WINDOWS_API)) && !defined(WOLFSSL_TIRTOS) + #if !defined(USE_WINDOWS_API) && !defined(WOLFSSL_TIRTOS) if (port == 0) { socklen_t len = sizeof(addr); if (getsockname(*sockfd, (struct sockaddr*)&addr, &len) == 0) { @@ -2063,7 +2063,7 @@ static WC_INLINE void udp_accept(SOCKET_T* sockfd, SOCKET_T* clientfd, } #endif -#if defined(_POSIX_THREADS) && defined(NO_MAIN_DRIVER) && !defined(__MINGW32__) +#if defined(_POSIX_THREADS) && !defined(__MINGW32__) /* signal ready to accept data */ { tcp_ready* ready = args->signal; From 44be4e1cc8fac30f6bfe2c203c1d970fa0af5e35 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 12 May 2022 16:48:04 +0200 Subject: [PATCH 3/3] Reset ret in client and server after wolfSSL_dtls_got_timeout() - Do UDP connect only with simulateWantWrite to accommodate macOS that doesn't like sendto being called on connected UDP sockets - Call wolfSSL_dtls_get_current_timeout only on a DTLS connection --- examples/client/client.c | 14 +++++++++++--- examples/server/server.c | 4 +++- wolfssl/test.h | 12 ++++++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index acafd71a8..d0935fcfd 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -201,7 +201,8 @@ static int NonBlockingSSL_Connect(WOLFSSL* ssl) else { #ifdef WOLFSSL_DTLS - currTimeout = wolfSSL_dtls_get_current_timeout(ssl); + if (wolfSSL_dtls(ssl)) + currTimeout = wolfSSL_dtls_get_current_timeout(ssl); #endif select_ret = tcp_select(sockfd, currTimeout); } @@ -238,6 +239,7 @@ static int NonBlockingSSL_Connect(WOLFSSL* ssl) error = wolfSSL_get_error(ssl, ret); else error = WOLFSSL_ERROR_WANT_READ; + ret = WOLFSSL_FAILURE; /* Reset error so we loop */ } #endif else { @@ -3565,7 +3567,10 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) } if (simulateWantWrite) { - wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + if (dtlsUDP) { + wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + udp_connect(&sockfd, host, port); + } } /* STARTTLS */ @@ -4105,7 +4110,10 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) err_sys("error in setting fd"); } if (simulateWantWrite) { - wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + if (dtlsUDP) { + wolfSSL_SetIOWriteCtx(ssl, (void*)&sockfd); + udp_connect(&sockfd, host, port); + } } #ifdef HAVE_ALPN if (alpnList != NULL) { diff --git a/examples/server/server.c b/examples/server/server.c index bd7f6db4f..e69d8183b 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -330,7 +330,8 @@ static int NonBlockingSSL_Accept(SSL* ssl) } else { #ifdef WOLFSSL_DTLS - currTimeout = wolfSSL_dtls_get_current_timeout(ssl); + if (wolfSSL_dtls(ssl)) + currTimeout = wolfSSL_dtls_get_current_timeout(ssl); #endif select_ret = tcp_select(sockfd, currTimeout); } @@ -360,6 +361,7 @@ static int NonBlockingSSL_Accept(SSL* ssl) error = wolfSSL_get_error(ssl, ret); else error = WOLFSSL_ERROR_WANT_READ; + ret = WOLFSSL_FAILURE; /* Reset error so we loop */ } #endif else { diff --git a/wolfssl/test.h b/wolfssl/test.h index 471c990f0..daa739c4d 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1847,16 +1847,20 @@ static WC_INLINE void tcp_connect(SOCKET_T* sockfd, const char* ip, word16 port, } tcp_socket(sockfd, udp, sctp); - if (connect(*sockfd, (const struct sockaddr*)&addr, sizeof(addr)) != 0) - err_sys_with_errno("tcp connect failed"); + if (!udp) { + if (connect(*sockfd, (const struct sockaddr*)&addr, sizeof(addr)) != 0) + err_sys_with_errno("tcp connect failed"); + } } #endif /* WOLFSSL_WOLFSENTRY_HOOKS */ -static WC_INLINE void udp_connect(SOCKET_T* sockfd, void* addr, int addrSz) +static WC_INLINE void udp_connect(SOCKET_T* sockfd, const char* ip, word16 port) { - if (connect(*sockfd, (const struct sockaddr*)addr, addrSz) != 0) + SOCKADDR_IN_T addr; + build_addr(&addr, ip, port, 1, 0); + if (connect(*sockfd, (const struct sockaddr*)&addr, sizeof(addr)) != 0) err_sys_with_errno("tcp connect failed"); }