1
0
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-10-24 04:09:07 +03:00

Close the remaining paths that write after close_notify

Review of the previous commit found more records, state changes and
misleading errors after our close_notify or a fatal alert:

- A TLS 1.2 server that sent close_notify still reset its handshake
  state for a renegotiation ClientHello, and with
  WOLFSSL_OP_NO_RENEGOTIATION, or when a DTLS epoch would wrap, still
  sent a no_renegotiation alert. It now skips the message, as the
  client does for a HelloRequest.
- An ignored post-handshake CertificateRequest was still parsed, and
  its context stayed linked on ssl->certReqCtx, so a peer could grow
  that list without bound while we waited for its close_notify. The
  message is now skipped before parsing.
- Dtls13DoScheduledWork() still sent the KeyUpdate that
  Dtls13CheckAEADFailLimit() schedules after too many failed
  decryptions.
- SendAlert_ex() only marked the connection closed for alert_fatal.
  In TLS 1.3 every alert except close_notify and user_canceled is
  fatal, which DoAlert() already applies on receipt. An SNI callback
  returning warning_return sends such an alert, after which the server
  could still send 0.5-RTT application data.
- A refused write reported the positive WOLFSSL_ERROR_WANT_READ, which
  wolfSSL_peek() leaves behind after a post-handshake message, instead
  of SSL_SHUTDOWN_ALREADY_DONE_E or SOCKET_PEER_CLOSED_E, so an event
  loop would wait for readability and retry forever.

Add IsWriteClosed() for the sentNotify || fatalAlert check that now
guards every one of these paths.

Extend test_tls12_no_renegotiation_after_close_notify (handshake state
and WOLFSSL_OP_NO_RENEGOTIATION) and
test_tls13_no_pha_response_after_close_notify (certReqCtx), and add
test_tls13_refused_write_after_peek,
test_dtls13_no_key_update_after_close_notify and
test_tls13_sent_warning_error_alert_is_fatal.
This commit is contained in:
Tobias Frauenschläger
2026-10-21 11:58:27 +02:00
committed by Paul Adelsbach
parent 2fd8dc2375
commit d76eb72c5b
7 changed files with 192 additions and 22 deletions

View File

@@ -3444,9 +3444,11 @@ int Dtls13DoScheduledWork(WOLFSSL* ssl)
if (ssl->dtls13DoKeyUpdate) {
ssl->dtls13DoKeyUpdate = 0;
ret = Tls13UpdateKeys(ssl);
if (ret != 0)
return ret;
if (!IsWriteClosed(ssl)) {
ret = Tls13UpdateKeys(ssl);
if (ret != 0)
return ret;
}
}
return 0;

View File

@@ -805,6 +805,11 @@ int IsAtLeastTLSv1_3(const ProtocolVersion pv)
return ret;
}
int IsWriteClosed(const WOLFSSL* ssl)
{
return ssl->options.sentNotify || ssl->options.fatalAlert;
}
int IsEncryptionOn(const WOLFSSL* ssl, int isSend)
{
if (ssl == NULL) {
@@ -20608,7 +20613,7 @@ static int DoHelloRequest(WOLFSSL* ssl, word32 size)
WOLFSSL_ERROR_VERBOSE(FATAL_ERROR);
return FATAL_ERROR;
}
else if (ssl->options.sentNotify || ssl->options.fatalAlert) {
else if (IsWriteClosed(ssl)) {
WOLFSSL_MSG("Ignoring HelloRequest after close_notify");
WOLFSSL_LEAVE("DoHelloRequest", 0);
WOLFSSL_END(WC_FUNC_HELLO_REQUEST_DO);
@@ -21439,6 +21444,16 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
expectedIdx = *inOutIdx + size;
#ifndef NO_WOLFSSL_SERVER
if (ssl->options.side == WOLFSSL_SERVER_END &&
ssl->options.handShakeDone && type == client_hello &&
IsWriteClosed(ssl)) {
WOLFSSL_MSG("Ignoring ClientHello after close_notify");
*inOutIdx = expectedIdx;
return 0;
}
#endif
#if !defined(NO_WOLFSSL_SERVER) && \
defined(HAVE_SECURE_RENEGOTIATION) && \
defined(HAVE_SERVER_RENEGOTIATION_INFO)
@@ -30388,9 +30403,7 @@ int SendData(WOLFSSL* ssl, const void* data, size_t sz)
/* We can send data without waiting on peer finished msg */
WOLFSSL_MSG("server sending data before receiving client finished");
}
/* No handshake records after our close_notify or a fatal alert */
else if (!ssl->options.sentNotify && !ssl->options.fatalAlert &&
ssl_in_handshake(ssl, 1)) {
else if (!IsWriteClosed(ssl) && ssl_in_handshake(ssl, 1)) {
int err;
WOLFSSL_MSG("handshake not complete, trying to finish");
if ( (err = wolfSSL_negotiate(ssl)) != WOLFSSL_SUCCESS) {
@@ -30416,8 +30429,7 @@ int SendData(WOLFSSL* ssl, const void* data, size_t sz)
}
#endif /* WOLFSSL_DTLS13 */
#ifdef WOLFSSL_TLS13
if (ssl->options.sendKeyUpdate && !ssl->options.sentNotify &&
!ssl->options.fatalAlert) {
if (ssl->options.sendKeyUpdate && !IsWriteClosed(ssl)) {
ssl->options.sendKeyUpdate = 0;
ret = SendTls13KeyUpdate(ssl);
if (ret != 0) {
@@ -30469,7 +30481,8 @@ int SendData(WOLFSSL* ssl, const void* data, size_t sz)
/* RFC 8446 6.2 - no new data write after a fatal alert */
if (ssl->options.fatalAlert) {
WOLFSSL_MSG("Connection is closed, not sending new data");
if (ssl->error == 0)
if (ssl->error == 0 || ssl->error == WC_NO_ERR_TRACE(WANT_READ) ||
ssl->error == WOLFSSL_ERROR_WANT_READ)
ssl->error = SOCKET_PEER_CLOSED_E;
return WOLFSSL_FATAL_ERROR;
}
@@ -30477,7 +30490,8 @@ int SendData(WOLFSSL* ssl, const void* data, size_t sz)
/* Reads and wolfSSL_shutdown() clear this; the peer may still send data */
if (ssl->options.sentNotify) {
WOLFSSL_MSG("close_notify already sent, not sending new data");
if (ssl->error == 0 || ssl->error == WC_NO_ERR_TRACE(WANT_READ))
if (ssl->error == 0 || ssl->error == WC_NO_ERR_TRACE(WANT_READ) ||
ssl->error == WOLFSSL_ERROR_WANT_READ)
ssl->error = SSL_SHUTDOWN_ALREADY_DONE_E;
return WOLFSSL_FATAL_ERROR;
}
@@ -30856,9 +30870,7 @@ int ReceiveData(WOLFSSL* ssl, byte* output, size_t sz, int peek)
else
#endif
{
/* No handshake records after our close_notify or a fatal alert */
if (!ssl->options.sentNotify && !ssl->options.fatalAlert &&
ssl_in_handshake(ssl, 0)) {
if (!IsWriteClosed(ssl) && ssl_in_handshake(ssl, 0)) {
int err;
WOLFSSL_MSG("Handshake not complete, trying to finish");
if ( (err = wolfSSL_negotiate(ssl)) != WOLFSSL_SUCCESS) {
@@ -30927,7 +30939,7 @@ startScr:
if (ssl->secure_renegotiation && ssl->secure_renegotiation->enabled &&
ssl->options.handShakeState != HANDSHAKE_DONE
&& ssl->buffers.clearOutputBuffer.length == 0
&& !ssl->options.sentNotify && !ssl->options.fatalAlert) {
&& !IsWriteClosed(ssl)) {
/* ProcessReply processed a handshake packet and not any APP DATA
* so let's move the handshake along */
int err;
@@ -31084,7 +31096,9 @@ static int SendAlert_ex(WOLFSSL* ssl, int severity, int type)
input[1] = (byte)type;
ssl->alert_history.last_tx.code = type;
ssl->alert_history.last_tx.level = severity;
if (severity == alert_fatal) {
/* RFC 9846 Section 6.2: TLS 1.3 error alerts are fatal at any level */
if (severity == alert_fatal || (IsAtLeastTLSv1_3(ssl->version) &&
type != close_notify && type != user_canceled)) {
#ifdef WOLFSSL_DTLS
/* Mark as closed in dtls only once we enter stateful mode. */
if (!ssl->options.dtls || ssl->options.dtlsStateful)

View File

@@ -142,7 +142,7 @@ static int wolfssl_write_dup_do_tls13_work(WOLFSSL* ssl)
{
int ret = 0;
if (ssl->options.sentNotify || ssl->options.fatalAlert) {
if (IsWriteClosed(ssl)) {
WOLFSSL_MSG("Dropping delegated responses after close_notify");
#ifdef WOLFSSL_POST_HANDSHAKE_AUTH
ssl->postHandshakeAuthPending = 0;

View File

@@ -6617,6 +6617,13 @@ static int DoTls13CertificateRequest(WOLFSSL* ssl, const byte* input,
WOLFSSL_START(WC_FUNC_CERTIFICATE_REQUEST_DO);
WOLFSSL_ENTER("DoTls13CertificateRequest");
#ifdef WOLFSSL_POST_HANDSHAKE_AUTH
if (ssl->options.handShakeState == HANDSHAKE_DONE && IsWriteClosed(ssl)) {
*inOutIdx = begin + size;
return 0;
}
#endif
XMEMSET(&peerSuites, 0, sizeof(Suites));
#if !defined(WOLFSSL_NO_SIGALG)
/* Post-handshake auth can deliver several requests; each one's cert
@@ -14445,7 +14452,7 @@ static int DoTls13KeyUpdate(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
#endif /* WOLFSSL_DTLS13 */
if (ssl->keys.keyUpdateRespond) {
if (ssl->options.sentNotify || ssl->options.fatalAlert) {
if (IsWriteClosed(ssl)) {
WOLFSSL_MSG("Not answering KeyUpdate after close_notify");
ssl->keys.keyUpdateRespond = 0;
return 0;
@@ -16401,7 +16408,7 @@ int DoTls13HandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
if (ssl->options.side == WOLFSSL_CLIENT_END) {
if (type == certificate_request &&
ssl->options.handShakeState == HANDSHAKE_DONE) {
if (ssl->options.sentNotify || ssl->options.fatalAlert) {
if (IsWriteClosed(ssl)) {
WOLFSSL_MSG("Ignoring CertificateRequest after "
"close_notify");
}

View File

@@ -13768,8 +13768,10 @@ int test_tls13_no_pha_response_after_close_notify(void)
(int)sizeof(msg));
ExpectBufEQ(readBuf, msg, sizeof(msg));
ExpectIntEQ(test_ctx.s_len, s_len_before);
if (ssl_c != NULL)
if (ssl_c != NULL) {
ExpectIntEQ(ssl_c->options.handShakeState, HANDSHAKE_DONE);
ExpectNull(ssl_c->certReqCtx);
}
wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
@@ -13855,14 +13857,18 @@ int test_tls12_no_renegotiation_after_close_notify(void)
int *closerOutLen;
int lenBefore;
int serverCloses;
int mode;
for (serverCloses = 0; serverCloses < 2 && EXPECT_SUCCESS();
serverCloses++) {
/* 0: client closes, 1: server closes, 2: also WOLFSSL_OP_NO_RENEGOTIATION */
for (mode = 0; mode < 3 && EXPECT_SUCCESS(); mode++) {
serverCloses = (mode > 0);
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c,
&ssl_s, wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
ExpectIntEQ(wolfSSL_UseSecureRenegotiation(ssl_c), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_UseSecureRenegotiation(ssl_s), WOLFSSL_SUCCESS);
if (mode == 2)
wolfSSL_set_options(ssl_s, WOLFSSL_OP_NO_RENEGOTIATION);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
closer = serverCloses ? ssl_s : ssl_c;
peer = serverCloses ? ssl_c : ssl_s;
@@ -13878,6 +13884,8 @@ int test_tls12_no_renegotiation_after_close_notify(void)
ExpectIntEQ(wolfSSL_get_error(closer,
WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)), WOLFSSL_ERROR_WANT_READ);
ExpectIntEQ(*closerOutLen, lenBefore);
if (closer != NULL)
ExpectIntEQ(closer->options.handShakeState, HANDSHAKE_DONE);
wolfSSL_free(ssl_c);
ssl_c = NULL;
@@ -13892,6 +13900,138 @@ int test_tls12_no_renegotiation_after_close_notify(void)
return EXPECT_RESULT();
}
int test_tls13_refused_write_after_peek(void)
{
EXPECT_DECLS;
#if defined(WOLFSSL_TLS13) && defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
defined(HAVE_SESSION_TICKET) && \
!defined(WOLFSSL_TLS13_NO_PEEK_HANDSHAKE_DONE) && \
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
WOLFSSL_CTX *ctx_c = NULL;
WOLFSSL_CTX *ctx_s = NULL;
WOLFSSL *ssl_c = NULL;
WOLFSSL *ssl_s = NULL;
struct test_memio_ctx test_ctx;
char buf[32];
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
ExpectIntEQ(wolfSSL_shutdown(ssl_c), WOLFSSL_SHUTDOWN_NOT_DONE);
ExpectIntEQ(wolfSSL_send_SessionTicket(ssl_s), WOLFSSL_SUCCESS);
/* The ticket leaves the positive WOLFSSL_ERROR_WANT_READ in ssl->error */
ExpectIntEQ(wolfSSL_peek(ssl_c, buf, (int)sizeof(buf)), 0);
ExpectIntEQ(wolfSSL_get_error(ssl_c, 0), WOLFSSL_ERROR_WANT_READ);
ExpectIntEQ(wolfSSL_write(ssl_c, buf, (int)sizeof(buf)),
WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
ExpectIntEQ(wolfSSL_get_error(ssl_c, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)),
WC_NO_ERR_TRACE(SSL_SHUTDOWN_ALREADY_DONE_E));
wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}
int test_dtls13_no_key_update_after_close_notify(void)
{
EXPECT_DECLS;
#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_RW_THREADED) && \
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
WOLFSSL_CTX *ctx_c = NULL;
WOLFSSL_CTX *ctx_s = NULL;
WOLFSSL *ssl_c = NULL;
WOLFSSL *ssl_s = NULL;
struct test_memio_ctx test_ctx;
char readBuf[32];
char msg[] = "after close";
int s_len_before;
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(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
ExpectIntEQ(wolfSSL_write(ssl_s, msg, (int)sizeof(msg)), (int)sizeof(msg));
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, (int)sizeof(readBuf)),
(int)sizeof(msg));
ExpectIntEQ(wolfSSL_write(ssl_c, msg, (int)sizeof(msg)), (int)sizeof(msg));
ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, (int)sizeof(readBuf)),
(int)sizeof(msg));
ExpectIntEQ(wolfSSL_shutdown(ssl_c), WOLFSSL_SHUTDOWN_NOT_DONE);
ExpectIntEQ(wolfSSL_write(ssl_s, msg, (int)sizeof(msg)), (int)sizeof(msg));
if (ssl_c != NULL)
ssl_c->dtls13DoKeyUpdate = 1;
s_len_before = test_ctx.s_len;
ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, (int)sizeof(readBuf)),
(int)sizeof(msg));
ExpectIntEQ(test_ctx.s_len, s_len_before);
wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}
#if defined(WOLFSSL_TLS13) && defined(HAVE_SNI) && \
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
static int test_sni_warn_cb(WOLFSSL* ssl, int* ad, void* arg)
{
(void)ssl;
(void)arg;
*ad = unrecognized_name;
return warning_return;
}
#endif
int test_tls13_sent_warning_error_alert_is_fatal(void)
{
EXPECT_DECLS;
#if defined(WOLFSSL_TLS13) && defined(HAVE_SNI) && \
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
!defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
WOLFSSL_CTX *ctx_c = NULL;
WOLFSSL_CTX *ctx_s = NULL;
WOLFSSL *ssl_c = NULL;
WOLFSSL *ssl_s = NULL;
struct test_memio_ctx test_ctx;
char msg[] = "early";
int c_len_before;
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
wolfSSL_CTX_set_servername_callback(ctx_s, test_sni_warn_cb);
ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, "example.com",
(word16)XSTRLEN("example.com")), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_connect(ssl_c), WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
ExpectIntEQ(wolfSSL_accept(ssl_s), WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
if (ssl_s != NULL)
ExpectIntEQ(ssl_s->options.fatalAlert, 1);
c_len_before = test_ctx.c_len;
ExpectIntEQ(wolfSSL_write(ssl_s, msg, (int)sizeof(msg)),
WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
if (ssl_s != NULL)
ExpectIntEQ(ssl_s->error, WC_NO_ERR_TRACE(SOCKET_PEER_CLOSED_E));
ExpectIntEQ(test_ctx.c_len, c_len_before);
wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}
/* RFC 9846 Section 4.7.3, write-duplicate path. The read side of a write dup
* cannot send, so a peer KeyUpdate(update_requested) is delegated to the write
* side via dupWrite->keyUpdateRespond and sent from

View File

@@ -167,6 +167,9 @@ int test_tls13_no_key_update_response_after_close_notify(void);
int test_tls13_no_pha_response_after_close_notify(void);
int test_tls13_write_dup_no_response_after_close_notify(void);
int test_tls12_no_renegotiation_after_close_notify(void);
int test_tls13_refused_write_after_peek(void);
int test_dtls13_no_key_update_after_close_notify(void);
int test_tls13_sent_warning_error_alert_is_fatal(void);
int test_tls13_pqc_hybrid_async_server(void);
int test_tls13_pha_status_request(void);
int test_tls13_x25519_keyshare_masks_reserved_bit(void);
@@ -342,6 +345,9 @@ int test_tls13_cks_hrr_reparse(void);
TEST_DECL_GROUP("tls13", test_tls13_no_pha_response_after_close_notify), \
TEST_DECL_GROUP("tls13", test_tls13_write_dup_no_response_after_close_notify), \
TEST_DECL_GROUP("tls13", test_tls12_no_renegotiation_after_close_notify), \
TEST_DECL_GROUP("tls13", test_tls13_refused_write_after_peek), \
TEST_DECL_GROUP("tls13", test_dtls13_no_key_update_after_close_notify), \
TEST_DECL_GROUP("tls13", test_tls13_sent_warning_error_alert_is_fatal), \
TEST_DECL_GROUP("tls13", test_tls13_pqc_hybrid_async_server), \
TEST_DECL_GROUP("tls13", test_tls13_x25519_keyshare_masks_reserved_bit), \
TEST_DECL_GROUP("tls13", test_tls13_is_init_finished_want_write), \

View File

@@ -7782,6 +7782,7 @@ WOLFSSL_LOCAL int IsTLS(const WOLFSSL* ssl);
WOLFSSL_LOCAL int IsTLS_ex(const ProtocolVersion pv);
WOLFSSL_LOCAL int IsAtLeastTLSv1_2(const WOLFSSL* ssl);
WOLFSSL_LOCAL int IsAtLeastTLSv1_3(const ProtocolVersion pv);
WOLFSSL_LOCAL int IsWriteClosed(const WOLFSSL* ssl);
WOLFSSL_LOCAL int IsEncryptionOn(const WOLFSSL* ssl, int isSend);
WOLFSSL_LOCAL int TLSv1_3_Capable(WOLFSSL* ssl);