Skip to content

Ensure close_notify is sent after user_canceled during quiet shutdown #11225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
holtrop-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
+93 −1
Open

Ensure close_notify is sent after user_canceled during quiet shutdown #11225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5660,6 +5660,7 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out,
ssl->options.isClosed = 0;
ssl->options.connReset = 0;
ssl->options.sentNotify = 0;
ssl->options.sentUserCanceled = 0;
ssl->options.closeNotify = 0;
ssl->options.sendVerify = 0;
ssl->options.serverState = NULL_STATE;
Expand Down
16 changes: 15 additions & 1 deletion src/ssl_api_rw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,10 @@ int wolfSSL_SendUserCanceled(WOLFSSL* ssl)

if (ssl != NULL) {
ssl->error = SendAlert(ssl, alert_warning, user_canceled);
if ((ssl->error == 0) ||
(ssl->error == WC_NO_ERR_TRACE(WANT_WRITE))) {
ssl->options.sentUserCanceled = 1;
}
if (ssl->error < 0) {
WOLFSSL_ERROR(ssl->error);
}
Expand Down Expand Up @@ -1030,10 +1034,20 @@ int wolfSSL_shutdown(WOLFSSL* ssl)
if (ssl == NULL) {
ret = WOLFSSL_FATAL_ERROR;
}
else if (ssl->options.quietShutdown) {
else if (ssl->options.quietShutdown && (!ssl->options.sentUserCanceled)) {
WOLFSSL_MSG("quiet shutdown, no close notify sent");
ret = WOLFSSL_SUCCESS;
}
else if (ssl->options.quietShutdown) {
/* A "user_canceled" alert has gone out so we need a "close_notify" to
* follow it per RFC 9846 Section 6.1. */
Comment thread
holtrop-wolfssl marked this conversation as resolved.
if (!wolfssl_shutdown_flush_alert(ssl, &ret)) {
(void)wolfssl_shutdown_send_close_notify(ssl, &ret);

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quiet-shutdown user_canceled path returns WOLFSSL_FATAL_ERROR with no error code set · TLS protocol issues

In the new quietShutdown && sentUserCanceled branch, both helpers are no-ops when sentNotify is already set or isClosed/connReset is set, so ret keeps its initial WOLFSSL_FATAL_ERROR (line 1029) while ssl->error is left untouched. Callers get -1 with wolfSSL_get_error() returning 0. This is adjacent to known finding #10650 (which this PR fixes) but is a distinct defect in the new branch's no-op path.

Related known finding #10650 (similar but distinct): Both are in wolfSSL_shutdown's quiet-shutdown handling, but #10650 concerns omission of a required close_notify; this finding concerns the new user_canceled no-op path retaining an initial fatal return without setting ssl->error. The faulting operations, root causes, and required patches differ.

Fix: Handle the no-op case explicitly: return WOLFSSL_SUCCESS when close_notify was already sent, and set ssl->error = SOCKET_PEER_CLOSED_E as the non-quiet path does when the peer closed.

}
if (ret == WC_NO_ERR_TRACE(WOLFSSL_SHUTDOWN_NOT_DONE)) {

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quiet-shutdown user_canceled branch returns WOLFSSL_FATAL_ERROR with no error recorded · Incorrect error handling

When wolfssl_shutdown_send_close_notify() takes no action (isClosed, connReset, or sentNotify already set) it returns 0 and leaves *ret untouched, so the new quiet branch falls through with ret still at its WOLFSSL_FATAL_ERROR initializer while ssl->error stays WOLFSSL_ERROR_NONE — the caller gets a failure with nothing to query. The non-quiet branch at lines 1075-1102 records SOCKET_PEER_CLOSED_E for exactly this case; the quiet branch has no equivalent. It also makes a repeat wolfSSL_shutdown() return failure in builds where the wolfSSL_clear() reset of sentUserCanceled is not compiled in.

Related known finding #10650 (similar but distinct): Both affect wolfSSL_shutdown's quiet-shutdown path and close-notify handling, but #10650 faults by suppressing a required TLS 1.3 close_notify while this finding returns an unrecorded fatal error after the send helper takes no action. The root causes and required patches differ.

Fix: Decide ret explicitly in the quiet branch when the helpers leave it undecided: return WOLFSSL_SUCCESS when nothing remains to send, or record an error before returning failure.

ret = WOLFSSL_SUCCESS;
}
}
else {
int done;

Expand Down
72 changes: 72 additions & 0 deletions tests/api/test_ssl_rw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,78 @@ int test_wolfSSL_SendUserCanceled_paths(void)
return EXPECT_RESULT();
}

/* Test that quiet shutdown does not suppress the close_notify that the
* user_canceled alert obliges wolfSSL to send.
*
* RFC 9846 Section 6.1 has a "close_notify" following "user_canceled" and has
Comment thread
holtrop-wolfssl marked this conversation as resolved.
* the peer keep reading until it arrives. Quiet shutdown may drop the
* close_notify that stands alone - that is what the option is for - but not
* the one the peer has been told to wait for.
*
* @return TEST_SUCCESS on success.
*/
int test_wolfSSL_SendUserCanceled_quiet_shutdown(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(NO_TLS) && \
!defined(WOLFSSL_NO_TLS12) && (defined(OPENSSL_EXTRA) || \
defined(OPENSSL_EXTRA_X509_SMALL) || defined(WOLFSSL_EXTRA) || \
defined(WOLFSSL_WPAS_SMALL))
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 reply[16];

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(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

wolfSSL_set_quiet_shutdown(ssl_c, 1);
/* Both alerts go out. Waiting for the peer's reply is what quiet shutdown
* skips, so the shutdown is done as far as this side is concerned. */
ExpectIntEQ(wolfSSL_SendUserCanceled(ssl_c), WOLFSSL_SUCCESS);

/* The server reads the user_canceled and then the close_notify, which is
* what it reports. Without the close_notify it would still be waiting. */
ExpectIntEQ(wolfSSL_read(ssl_s, reply, (int)sizeof(reply)), 0);
ExpectIntEQ(wolfSSL_get_error(ssl_s, 0), WOLFSSL_ERROR_ZERO_RETURN);
ExpectIntEQ(wolfSSL_get_shutdown(ssl_s), WOLFSSL_RECEIVED_SHUTDOWN);

wolfSSL_free(ssl_c);
ssl_c = NULL;
wolfSSL_free(ssl_s);
ssl_s = NULL;
wolfSSL_CTX_free(ctx_c);
ctx_c = NULL;
wolfSSL_CTX_free(ctx_s);
ctx_s = NULL;

/* A quiet shutdown with no user_canceled behind it still sends nothing:
* that is the whole point of the option. */
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(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

wolfSSL_set_quiet_shutdown(ssl_c, 1);
ExpectIntEQ(wolfSSL_shutdown(ssl_c), WOLFSSL_SUCCESS);

/* Nothing arrived, so the server is still waiting for a record. */
ExpectIntLT(wolfSSL_read(ssl_s, reply, (int)sizeof(reply)), 0);
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
Comment thread
holtrop-wolfssl marked this conversation as resolved.
ExpectIntEQ(wolfSSL_get_shutdown(ssl_s), 0);

wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}

/* Test that an error the read side recorded is the one the write reports.
*
* With a write duplicate in use the read side hands errors over through
Expand Down
3 changes: 3 additions & 0 deletions tests/api/test_ssl_rw.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ int test_wolfSSL_shutdown_repeat_after_done(void);
int test_wolfSSL_shutdown_flush_no_notify(void);
int test_wolfSSL_shutdown_quic_alert_refused(void);
int test_wolfSSL_SendUserCanceled_paths(void);
int test_wolfSSL_SendUserCanceled_quiet_shutdown(void);
int test_wolfSSL_write_dup_err(void);

#define TEST_SSL_RW_DECLS \
Expand All @@ -56,6 +57,8 @@ int test_wolfSSL_write_dup_err(void);
TEST_DECL_GROUP("ssl_rw", test_wolfSSL_shutdown_flush_no_notify), \
TEST_DECL_GROUP("ssl_rw", test_wolfSSL_shutdown_quic_alert_refused), \
TEST_DECL_GROUP("ssl_rw", test_wolfSSL_SendUserCanceled_paths), \
TEST_DECL_GROUP("ssl_rw", \
test_wolfSSL_SendUserCanceled_quiet_shutdown), \
TEST_DECL_GROUP("ssl_rw", test_wolfSSL_write_dup_err)

#endif /* TESTS_API_SSL_RW_H */
2 changes: 2 additions & 0 deletions wolfssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5441,6 +5441,8 @@ struct Options {
word16 isClosed:1; /* if we consider conn closed */
word16 closeNotify:1; /* we've received a close notify */
word16 sentNotify:1; /* we've sent a close notify */
word16 sentUserCanceled:1; /* we've sent a user_canceled and
* owe the peer a close notify */
Comment thread
holtrop-wolfssl marked this conversation as resolved.
word16 usingCompression:1; /* are we using compression */
word16 haveRSA:1; /* RSA available */
word16 haveECC:1; /* ECC available */
Expand Down
Loading