Skip to content

Add WOLFSSL_STRICT_CIPHER_LIST for TLS 1.3-only cipher lists on a non-TLS1.3 ctx/ssl #10963

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
miyazakh wants to merge 1 commit into
base: master
Choose a base branch
from
+41 −0
Open

Add WOLFSSL_STRICT_CIPHER_LIST for TLS 1.3-only cipher lists on a non-TLS1.3 ctx/ssl #10963

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 .wolfssl_known_macro_extras
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,7 @@ WOLFSSL_STM32F427_RNG
WOLFSSL_STM32U0
WOLFSSL_STM32_DHUK_UNWRAP
WOLFSSL_STM32_USE_SAES
WOLFSSL_STRICT_CIPHER_LIST
WOLFSSL_STRONGEST_HASH_SIG
WOLFSSL_STSAFE_TAKES_SLOT
WOLFSSL_TELIT_M2MB
Expand Down
4 changes: 4 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
* Enable SHA-2 cipher suites for pre-TLS 1.2 default: off
* WOLFSSL_NO_STRICT_CIPHER_SUITE:
* Relax strict cipher suite validation default: off
* WOLFSSL_STRICT_CIPHER_LIST: Fail wolfSSL_CTX/SSL_set_cipher_list()
* when given a TLS 1.3-only suite list but TLS 1.3
* is not negotiable, instead of leaving the prior
* suites untouched and returning success default: off
* NO_RESUME_SUITE_CHECK: Skip cipher suite check on resume default: off
* NO_FORCE_SCR_SAME_SUITE: Allow different suite in renegotiation default: off
* CIPHER_NONCE: Per-record cipher nonce for AEAD default: off
Expand Down
9 changes: 9 additions & 0 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3895,8 +3895,17 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
tls13Only = 1;

@Frauschi Frauschi Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 [Low] WOLFSSL_STRICT_CIPHER_LIST can now fail ctx creation under WOLFSSL_SYS_CRYPTO_POLICY
💡 SUGGEST question

Beyond the two public setters named in the PR, wolfSSL_parse_cipher_list() is also called during ctx construction when WOLFSSL_SYS_CRYPTO_POLICY is enabled. With WOLFSSL_STRICT_CIPHER_LIST defined, a system crypto policy that lists only TLS 1.3 suites applied to a <=TLS1.2 method would now make wolfSSL_parse_cipher_list() return WOLFSSL_FAILURE, causing wolfSSL_CTX_new_ex to free the ctx and return NULL where it previously succeeded. This is a plausibly-intended consequence of opting into strict behavior (fail loudly), but it is a broader blast radius than the two setters described in the PR body and is not covered by tests. Worth confirming this interaction is intended and documenting it.

Recommendation: Confirm the WOLFSSL_SYS_CRYPTO_POLICY + WOLFSSL_STRICT_CIPHER_LIST interaction (ctx creation can now fail) is intended, and note it in the macro documentation.

if ((ctx != NULL && !IsAtLeastTLSv1_3(ctx->method->version)) ||
(ssl != NULL && !IsAtLeastTLSv1_3(ssl->version))) {
#ifdef WOLFSSL_STRICT_CIPHER_LIST
/* TLS 1.3 is not negotiable on this ctx/ssl: fail instead of
* leaving the previously configured <= TLS 1.2 suites
* untouched while reporting success. */
WOLFSSL_MSG("Cipher list has only TLS 1.3 suites but TLS 1.3 "
"is not negotiable");
return WOLFSSL_FAILURE;
#else
Comment thread
miyazakh marked this conversation as resolved.
/* Silently ignore TLS 1.3 ciphers if we don't support it. */
return WOLFSSL_SUCCESS;
#endif
}
}

Expand Down
25 changes: 25 additions & 0 deletions tests/api/test_tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,31 @@ int test_tls13_cipher_suites(void)
return EXPECT_RESULT();
}

int test_tls13_cipher_list_no_tls13_ctx(void)

@Frauschi Frauschi Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 [Medium] Test only exercises the CTX path, not the SSL path
💡 SUGGEST test

The new macro gates the return value inside wolfSSL_parse_cipher_list(), which backs BOTH wolfSSL_CTX_set_cipher_list() (via ctx) and wolfSSL_set_cipher_list() (via ssl, src/ssl.c:4041). The PR body states the change and test cover both entry points, but the test only calls wolfSSL_CTX_set_cipher_list(). The ssl->version branch of the condition (ssl != NULL && !IsAtLeastTLSv1_3(ssl->version)) is never exercised, so a regression that broke only the SSL path would pass CI.

Suggestion: Add a parallel case that creates a WOLFSSL* from a TLS 1.2 ctx and calls wolfSSL_set_cipher_list(ssl, "TLS13-AES128-GCM-SHA256"), asserting WOLFSSL_FAILURE (strict) / WOLFSSL_SUCCESS (default), to cover the ssl->version branch.

{
EXPECT_DECLS;
#if defined(WOLFSSL_TLS13) && defined(OPENSSL_EXTRA) && \
!defined(NO_WOLFSSL_CLIENT) && !defined(WOLFSSL_NO_TLS12) && \
defined(BUILD_TLS_AES_128_GCM_SHA256)
WOLFSSL_CTX* ctx = NULL;

/* ctx->method caps the connection at TLS 1.2, so a cipher list that
* names only TLS 1.3 suites can never take effect on it. */
ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_client_method()));
#ifdef WOLFSSL_STRICT_CIPHER_LIST
ExpectIntEQ(wolfSSL_CTX_set_cipher_list(ctx, "TLS13-AES128-GCM-SHA256"),
WOLFSSL_FAILURE);
#else

@Frauschi Frauschi Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 [Low] Test asserts return value but not that suites are actually left untouched
💡 SUGGEST test

The default-path comment claims the call succeeds "but leave ctx->suites untouched," yet the test only checks the WOLFSSL_SUCCESS return code. The distinguishing behavior (that the prior <=TLS1.2 suites remain and the TLS1.3-only list did NOT overwrite them) is not asserted. This is the exact silent-no-op behavior the PR is calling out as a footgun, so it would be valuable to pin it down.

Suggestion: Capture ctx->suites->suiteSz (or the suite bytes) before the call and assert they are unchanged after, confirming the TLS1.3-only list was actually ignored rather than merely returning success.

/* Default OpenSSL-compat behavior: succeed but leave ctx->suites
* untouched since the TLS 1.3-only list is unusable on this ctx. */
ExpectIntEQ(wolfSSL_CTX_set_cipher_list(ctx, "TLS13-AES128-GCM-SHA256"),
WOLFSSL_SUCCESS);
#endif
wolfSSL_CTX_free(ctx);
Comment thread
miyazakh marked this conversation as resolved.
#endif
return EXPECT_RESULT();
}


#if defined(WOLFSSL_TLS13) && defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES)\
&& !defined(NO_PSK)
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_tls13.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

int test_tls13_apis(void);
int test_tls13_cipher_suites(void);
int test_tls13_cipher_list_no_tls13_ctx(void);
int test_tls13_bad_psk_binder(void);
int test_tls13_rpk_handshake(void);
int test_tls13_rpk_handshake_no_negotiation(void);
Expand Down Expand Up @@ -108,6 +109,7 @@ int test_tls13_pqc_hybrid_async_server(void);
#define TEST_TLS13_DECLS \
TEST_DECL_GROUP("tls13", test_tls13_apis), \
TEST_DECL_GROUP("tls13", test_tls13_cipher_suites), \
TEST_DECL_GROUP("tls13", test_tls13_cipher_list_no_tls13_ctx), \
TEST_DECL_GROUP("tls13", test_tls13_bad_psk_binder), \
TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \
TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \
Expand Down
Loading