diff --git a/.github/workflows/os-check.yml b/.github/workflows/os-check.yml index f06e361fde..08134c4a21 100644 --- a/.github/workflows/os-check.yml +++ b/.github/workflows/os-check.yml @@ -16,11 +16,14 @@ jobs: '--enable-all --enable-asn=original', '--enable-harden-tls', '--enable-tls13 --enable-session-ticket --enable-dtls --enable-dtls13 - --enable-opensslextra --enable-sessioncerts - CPPFLAGS=''-DWOLFSSL_DTLS_NO_HVR_ON_RESUME -DHAVE_EXT_CACHE - -DWOLFSSL_TICKET_HAVE_ID -DHAVE_EX_DATA -DSESSION_CACHE_DYNAMIC_MEM'' ', + --enable-opensslextra --enable-sessioncerts + CPPFLAGS=''-DWOLFSSL_DTLS_NO_HVR_ON_RESUME -DHAVE_EXT_CACHE + -DWOLFSSL_TICKET_HAVE_ID -DHAVE_EX_DATA -DSESSION_CACHE_DYNAMIC_MEM'' ', '--enable-all --enable-secure-renegotiation', '--enable-all --enable-haproxy --enable-quic', + '--enable-dtls --enable-dtls13 --enable-earlydata + --enable-session-ticket --enable-psk + CPPFLAGS=''-DWOLFSSL_DTLS13_NO_HRR_ON_RESUME'' ', ] name: make check runs-on: ${{ matrix.os }} diff --git a/examples/client/client.c b/examples/client/client.c index c7789e454d..0850a89a04 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -466,26 +466,6 @@ static void EarlyData(WOLFSSL_CTX* ctx, WOLFSSL* ssl, const char* msg, wolfSSL_CTX_free(ctx); ctx = NULL; err_sys("SSL_write_early_data failed"); } - do { - err = 0; /* reset error */ - ret = wolfSSL_write_early_data(ssl, msg, msgSz, &msgSz); - 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; - } - #endif - } - } while (err == WC_PENDING_E); - if (ret != msgSz) { - LOG_ERROR("SSL_write_early_data msg error %d, %s\n", err, - wolfSSL_ERR_error_string(err, buffer)); - wolfSSL_free(ssl); - wolfSSL_CTX_free(ctx); - err_sys("SSL_write_early_data failed"); - } } #endif diff --git a/examples/server/server.c b/examples/server/server.c index 2e28a624e0..33a6e4eaa6 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -3358,7 +3358,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) err = 0; /* reset error */ ret = wolfSSL_read_early_data(ssl, input, sizeof(input)-1, &len); - if (ret != WOLFSSL_SUCCESS) { + if (ret <= 0) { err = SSL_get_error(ssl, 0); #ifdef WOLFSSL_ASYNC_CRYPT if (err == WC_PENDING_E) { diff --git a/src/dtls.c b/src/dtls.c index 8b5234fee3..155beb25f4 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -21,11 +21,13 @@ /* * WOLFSSL_DTLS_NO_HVR_ON_RESUME + * WOLFSSL_DTLS13_NO_HRR_ON_RESUME * If defined, a DTLS server will not do a cookie exchange on successful * client resumption: the resumption will be faster (one RTT less) and - * will consume less bandwidth (one ClientHello and one HelloVerifyRequest - * less). On the other hand, if a valid SessionID is collected, forged - * clientHello messages will consume resources on the server. + * will consume less bandwidth (one ClientHello and one + * HelloVerifyRequest/HelloRetryRequest less). On the other hand, if a valid + * SessionID/ticket/psk is collected, forged clientHello messages will + * consume resources on the server. * WOLFSSL_DTLS_CH_FRAG * Allow a server to process a fragmented second/verified (one containing a * valid cookie response) ClientHello message. The first/unverified (one @@ -769,6 +771,15 @@ static int SendStatelessReplyDtls13(const WOLFSSL* ssl, WolfSSL_CH* ch) } } +#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME + if (ssl->options.dtls13NoHrrOnResume && usePSK && pskInfo.isValid && + !cs.doHelloRetry) { + /* Skip HRR on resumption */ + ((WOLFSSL*)ssl)->options.dtlsStateful = 1; + goto dtls13_cleanup; + } +#endif + #ifdef HAVE_SUPPORTED_CURVES if (cs.doHelloRetry) { ret = TLSX_KeyShare_SetSupported(ssl, &parsedExts); @@ -949,7 +960,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, ret = COOKIE_ERROR; else #endif - ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); + ret = SendStatelessReply(ssl, &ch, isTls13); } else { byte cookieGood; @@ -970,7 +981,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, ret = COOKIE_ERROR; else #endif - ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); + ret = SendStatelessReply(ssl, &ch, isTls13); } else { ssl->options.dtlsStateful = 1; diff --git a/src/dtls13.c b/src/dtls13.c index 6efc7bb46d..d41e426769 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -2844,5 +2844,15 @@ int wolfSSL_dtls13_allow_ch_frag(WOLFSSL *ssl, int enabled) } #endif +#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME +int wolfSSL_dtls13_no_hrr_on_resume(WOLFSSL *ssl, int enabled) +{ + if (ssl->options.side == WOLFSSL_CLIENT_END) { + return WOLFSSL_FAILURE; + } + ssl->options.dtls13NoHrrOnResume = !!enabled; + return WOLFSSL_SUCCESS; +} +#endif #endif /* WOLFSSL_DTLS13 */ diff --git a/src/internal.c b/src/internal.c index 8bc7cdd1f8..703993877c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -20185,20 +20185,8 @@ static int DtlsShouldDrop(WOLFSSL* ssl, int retcode) #ifndef NO_WOLFSSL_SERVER if (ssl->options.side == WOLFSSL_SERVER_END && ssl->curRL.type != handshake && !IsSCR(ssl)) { - int beforeCookieVerified = 0; - if (!IsAtLeastTLSv1_3(ssl->version)) { - beforeCookieVerified = - ssl->options.acceptState < ACCEPT_FIRST_REPLY_DONE; - } -#ifdef WOLFSSL_DTLS13 - else { - beforeCookieVerified = - ssl->options.acceptState < TLS13_ACCEPT_SECOND_REPLY_DONE; - } -#endif /* WOLFSSL_DTLS13 */ - - if (beforeCookieVerified) { - WOLFSSL_MSG("Drop non-handshake record before handshake"); + if (!ssl->options.dtlsStateful) { + WOLFSSL_MSG("Drop non-handshake record when not stateful"); return 1; } } @@ -34441,6 +34429,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #if defined(WOLFSSL_TLS13) && defined(HAVE_SUPPORTED_CURVES) if (cs.doHelloRetry) { + /* Make sure we don't send HRR twice */ + if (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) + return INVALID_PARAMETER; ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE; return TLSX_KeyShare_SetSupported(ssl, &ssl->extensions); } diff --git a/src/ssl.c b/src/ssl.c index 4fad504e48..5c7c51c062 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16793,11 +16793,13 @@ int wolfSSL_set_compression(WOLFSSL* ssl) #endif /* OPENSSL_EXTRA || WOLFSSL_EXTRA || WOLFSSL_WPAS_SMALL */ /* return true if connection established */ - int wolfSSL_is_init_finished(WOLFSSL* ssl) + int wolfSSL_is_init_finished(const WOLFSSL* ssl) { if (ssl == NULL) return 0; + /* Can't use ssl->options.connectState and ssl->options.acceptState because + * they differ in meaning for TLS <=1.2 and 1.3 */ if (ssl->options.handShakeState == HANDSHAKE_DONE) return 1; @@ -31970,12 +31972,7 @@ int wolfSSL_SSL_in_init(WOLFSSL *ssl) { WOLFSSL_ENTER("wolfSSL_SSL_in_init"); - if (ssl == NULL) - return WOLFSSL_FAILURE; - - /* Can't use ssl->options.connectState and ssl->options.acceptState because - * they differ in meaning for TLS <=1.2 and 1.3 */ - return ssl->options.handShakeState != HANDSHAKE_DONE; + return !wolfSSL_is_init_finished(ssl); } int wolfSSL_SSL_in_connect_init(WOLFSSL* ssl) diff --git a/src/tls13.c b/src/tls13.c index 07c7158ef7..ac24dda0e9 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6204,6 +6204,8 @@ static int CheckPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz, if ((ret = SetKeysSide(ssl, DECRYPT_SIDE_ONLY)) != 0) return ret; + ssl->keys.encryptionOn = 1; + #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { ret = Dtls13NewEpoch(ssl, @@ -6916,7 +6918,11 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } } else { - ERROR_OUT(HRR_COOKIE_ERROR, exit_dch); +#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_DTLS13_NO_HRR_ON_RESUME) + /* Don't error out as we may be resuming. We confirm this later. */ + if (!ssl->options.dtls) +#endif + ERROR_OUT(HRR_COOKIE_ERROR, exit_dch); } } #endif @@ -6982,7 +6988,6 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, goto exit_dch; } } - else #endif #ifdef HAVE_SUPPORTED_CURVES if (args->usingPSK == 2) { @@ -6990,6 +6995,9 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, int doHelloRetry = 0; ret = TLSX_KeyShare_Establish(ssl, &doHelloRetry); if (doHelloRetry) { + /* Make sure we don't send HRR twice */ + if (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) + ERROR_OUT(INVALID_PARAMETER, exit_dch); ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE; if (ret != WC_PENDING_E) ret = 0; /* for hello_retry return 0 */ @@ -7082,32 +7090,58 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ret = INPUT_CASE_ERROR; } /* switch (ssl->options.asyncState) */ -#if defined(WOLFSSL_SEND_HRR_COOKIE) - if (ret == 0 && ssl->options.sendCookie && ssl->options.cookieGood && - (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE +#ifdef WOLFSSL_SEND_HRR_COOKIE + if (ret == 0 && ssl->options.sendCookie) { + if (ssl->options.cookieGood && + ssl->options.acceptState == TLS13_ACCEPT_FIRST_REPLY_DONE) { + /* Processing second ClientHello. Clear HRR state. */ + ssl->options.serverState = NULL_STATE; + } + + if (ssl->options.cookieGood && + ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) { + /* If we already verified the peer with a cookie then we can't + * do another HRR for cipher negotiation. Send alert and restart + * the entire handshake. */ + ERROR_OUT(INVALID_PARAMETER, exit_dch); + } #ifdef WOLFSSL_DTLS13 - /* DTLS cookie exchange should be done in stateless code in - * DoClientHelloStateless. If we verified the cookie then - * always advance the state. */ - || ssl->options.dtls + if (ssl->options.dtls && + ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) { + /* Cookie and key share negotiation should be handled in + * DoClientHelloStateless. If we enter here then something went + * wrong in our logic. */ + ERROR_OUT(BAD_HELLO, exit_dch); + } #endif - )) - ssl->options.serverState = SERVER_HELLO_COMPLETE; + /* Send a cookie */ + if (!ssl->options.cookieGood && + ssl->options.serverState != SERVER_HELLO_RETRY_REQUEST_COMPLETE) { +#ifdef WOLFSSL_DTLS13 + if (ssl->options.dtls) { +#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME + /* We can skip cookie on resumption */ + if (!ssl->options.dtls || !ssl->options.dtls13NoHrrOnResume || + !args->usingPSK) +#endif + ERROR_OUT(BAD_HELLO, exit_dch); + } + else #endif + { + /* Need to remove the keyshare ext if we found a common group + * and are not doing curve negotiation. */ + TLSX_Remove(&ssl->extensions, TLSX_KEY_SHARE, ssl->heap); + ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE; + } -#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE) - if (ret == 0 && ssl->options.dtls && ssl->options.sendCookie && - ssl->options.serverState <= SERVER_HELLO_RETRY_REQUEST_COMPLETE) { - /* Cookie and key share negotiation should be handled in - * DoClientHelloStateless. If we enter here then something went wrong - * in our logic. */ - ERROR_OUT(BAD_HELLO, exit_dch); + } } #endif /* WOLFSSL_DTLS13 */ #ifdef WOLFSSL_DTLS_CID /* do not modify CID state if we are sending an HRR */ - if (ssl->options.useDtlsCID && + if (ret == 0 && ssl->options.dtls && ssl->options.useDtlsCID && ssl->options.serverState != SERVER_HELLO_RETRY_REQUEST_COMPLETE) DtlsCIDOnExtensionsParsed(ssl); #endif /* WOLFSSL_DTLS_CID */ diff --git a/tests/api.c b/tests/api.c index 9bf41456bd..6ec9ae216b 100644 --- a/tests/api.c +++ b/tests/api.c @@ -67135,7 +67135,7 @@ static int test_dtls_empty_keyshare_with_cookie(void) return EXPECT_RESULT(); } -#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ +#if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(HAVE_LIBOQS) static void test_tls13_pq_groups_ctx_ready(WOLFSSL_CTX* ctx) { @@ -67152,7 +67152,7 @@ static void test_tls13_pq_groups_on_result(WOLFSSL* ssl) static int test_tls13_pq_groups(void) { EXPECT_DECLS; -#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ +#if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ defined(HAVE_LIBOQS) callback_functions func_cb_client; callback_functions func_cb_server; @@ -67174,6 +67174,86 @@ static int test_tls13_pq_groups(void) return EXPECT_RESULT(); } +static int test_dtls13_early_data(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) && \ + defined(WOLFSSL_EARLY_DATA) && defined(HAVE_SESSION_TICKET) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + WOLFSSL_SESSION *sess = NULL; + int written = 0; + int read = 0; + char msg[] = "This is early data"; + char msg2[] = "This is client data"; + char msg3[] = "This is server data"; + char msgBuf[50]; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); + + /* Get a ticket so that we can do 0-RTT on the next connection */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectNotNull(sess = wolfSSL_get1_session(ssl_c)); + + wolfSSL_free(ssl_c); + ssl_c = NULL; + wolfSSL_free(ssl_s); + ssl_s = NULL; + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); + ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS); +#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME + ExpectIntEQ(wolfSSL_dtls13_no_hrr_on_resume(ssl_s, 1), WOLFSSL_SUCCESS); +#else + /* Let's test this but we generally don't recommend turning off the + * cookie exchange */ + ExpectIntEQ(wolfSSL_disable_hrr_cookie(ssl_s), WOLFSSL_SUCCESS); +#endif + + ExpectIntEQ(wolfSSL_write_early_data(ssl_c, msg, sizeof(msg), + &written), sizeof(msg)); + ExpectIntEQ(written, sizeof(msg)); + + ExpectIntEQ(wolfSSL_read_early_data(ssl_s, msgBuf, sizeof(msgBuf), + &read), sizeof(msg)); + ExpectIntEQ(read, sizeof(msg)); + ExpectStrEQ(msg, msgBuf); + + /* Complete handshake */ + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + /* Use wolfSSL_is_init_finished to check if handshake is complete. Normally + * a user would loop until it is true but here we control both sides so we + * just assert the expected value. wolfSSL_read_early_data does not provide + * handshake status to us with non-blocking IO and we can't use + * wolfSSL_accept as TLS layer may return ZERO_RETURN due to early data + * parsing logic. */ + ExpectFalse(wolfSSL_is_init_finished(ssl_s)); + ExpectIntEQ(wolfSSL_read_early_data(ssl_s, msgBuf, sizeof(msgBuf), + &read), WOLFSSL_FAILURE); + ExpectTrue(wolfSSL_is_init_finished(ssl_s)); + + ExpectIntEQ(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS); + + /* Test bi-directional write */ + ExpectIntEQ(wolfSSL_write(ssl_c, msg2, sizeof(msg2)), sizeof(msg2)); + ExpectIntEQ(wolfSSL_read(ssl_s, msgBuf, sizeof(msgBuf)), sizeof(msg2)); + ExpectStrEQ(msg2, msgBuf); + ExpectIntEQ(wolfSSL_write(ssl_s, msg3, sizeof(msg3)), sizeof(msg3)); + ExpectIntEQ(wolfSSL_read(ssl_c, msgBuf, sizeof(msgBuf)), sizeof(msg3)); + ExpectStrEQ(msg3, msgBuf); + + ExpectTrue(wolfSSL_session_reused(ssl_c)); + ExpectTrue(wolfSSL_session_reused(ssl_s)); +#endif + return EXPECT_RESULT(); +} + /*----------------------------------------------------------------------------* | Main *----------------------------------------------------------------------------*/ @@ -68456,6 +68536,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_dtls13_frag_ch_pq), TEST_DECL(test_dtls_empty_keyshare_with_cookie), TEST_DECL(test_tls13_pq_groups), + TEST_DECL(test_dtls13_early_data), /* This test needs to stay at the end to clean up any caches allocated. */ TEST_DECL(test_wolfSSL_Cleanup) }; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index cdb33c5d8f..c4805ee7aa 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -4732,6 +4732,9 @@ struct Options { #ifdef WOLFSSL_DTLS13 word16 dtls13SendMoreAcks:1; /* Send more acks during the * handshake process */ +#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME + word16 dtls13NoHrrOnResume:1; +#endif #ifdef WOLFSSL_DTLS_CH_FRAG word16 dtls13ChFrag:1; #endif diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 730571645a..a05cf262c4 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -1657,7 +1657,7 @@ WOLFSSL_API int wolfSSL_CTX_add_session(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* session); WOLFSSL_API int wolfSSL_SESSION_set_cipher(WOLFSSL_SESSION* session, const WOLFSSL_CIPHER* cipher); -WOLFSSL_API int wolfSSL_is_init_finished(WOLFSSL* ssl); +WOLFSSL_API int wolfSSL_is_init_finished(const WOLFSSL* ssl); WOLFSSL_API const char* wolfSSL_get_version(const WOLFSSL* ssl); WOLFSSL_API int wolfSSL_get_current_cipher_suite(WOLFSSL* ssl); @@ -5227,6 +5227,9 @@ WOLFSSL_API int wolfSSL_dtls_cid_get_tx(WOLFSSL* ssl, unsigned char* buffer, #ifdef WOLFSSL_DTLS_CH_FRAG WOLFSSL_API int wolfSSL_dtls13_allow_ch_frag(WOLFSSL *ssl, int enabled); #endif +#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME + WOLFSSL_API int wolfSSL_dtls13_no_hrr_on_resume(WOLFSSL *ssl, int enabled); +#endif /* */ #define SSL2_VERSION 0x0002