Pull request overview

Fixes a session-ticket encryption key rotation edge case where ticketHint can exceed the default ticket key lifetime, previously causing TicketEncCbCtx_ChooseKey() to return BAD_STATE_E and break subsequent handshakes when reusing a server WOLFSSL_CTX.

Changes:

• Update default session ticket key selection/rotation logic to avoid returning BAD_STATE_E when both keys are decrypt-valid but neither covers ticketHint.
• Add a new API regression test that reuses server/client contexts across multiple handshakes with an oversized ticket hint.
• Register the new test in the ssl extension test list.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Skoll Multi-Scan Review

Modes: review + review-securityOverall recommendation: REQUEST_CHANGES
Findings: 5 total — 4 posted, 1 skipped
2 finding(s) posted as inline comments (see file-level comments below)
2 finding(s) not tied to a diff line (full detail below)

Posted findings

• [Medium] [review+review-security] Hint rejection also fires with a custom TicketEncCb -- macro gate reflects macro presence, not the default callback — src/ssl_api_ext.c:1073-1080
• [Low] [review] Compile-time and runtime hint checks disagree by one for odd WOLFSSL_TICKET_KEY_LIFETIME — wolfssl/internal.h:2050-2052

Findings not tied to a diff line

New hint cap regresses OpenSSL-compat SSL_CTX_set_timeout for common timeouts (= 30 min)

File: src/ssl_api_ext.c:1073-1080 (manifests via src/ssl_sess.c:958-968)
Function: wolfSSL_CTX_set_TicketHint / wolfSSL_CTX_set_timeout
Severity: High
Category: Regression

wolfSSL_CTX_set_TicketHint is also called internally by the OpenSSL-compat wolfSSL_CTX_set_timeout (SSL_CTX_set_timeout). Under OPENSSL_EXTRA && HAVE_SESSION_TICKET && !NO_WOLFSSL_SERVER, wolfSSL_CTX_set_timeout(ctx, to) forwards the session timeout straight into wolfSSL_CTX_set_TicketHint(ctx, (int)to) (ssl_sess.c:968). The new check rejects any hint >= WOLFSSL_TICKET_KEY_LIFETIME / 2, i.e. any value >= 1800 seconds (30 min) with the default 3600s key lifetime. Session timeouts of 30+ minutes are the norm (OpenSSL's own default is 7200s; values like 3600/7200/86400 are routine), so SSL_CTX_set_timeout(ctx, 7200) -- which previously succeeded (only >604800 was rejected) -- now returns BAD_FUNC_ARG. This is a backward-incompatible break of a widely-used public compat API. Worse, ctx->timeout is already assigned at ssl_sess.c:958 BEFORE the ticket-hint call fails, so the session-cache timeout is silently applied while the function reports failure, leaving inconsistent state for callers that check the return code. Severity views differ (review: High/BLOCK; review-security: Medium) -- the stricter High is kept because this breaks a public API for common inputs; the session-cache timeout is conceptually independent of ticket-key rotation. Not covered by CI: test_wolfSSL_CTX_set_timeout only uses values below 1800.

Recommendation: Decouple the session-cache timeout from the ticket-hint constraint. In wolfSSL_CTX_set_timeout, clamp the value passed to wolfSSL_CTX_set_TicketHint (e.g. min(to, WOLFSSL_TICKET_KEY_LIFETIME/2 - 1)) or ignore a ticket-hint rejection when setting the session timeout, so a large session timeout still succeeds and ctx state stays consistent. Keep the hard rejection only for the direct public setter if desired. Add a test_wolfSSL_CTX_set_timeout case with a value >= WOLFSSL_TICKET_KEY_LIFETIME/2 (e.g. 3600) to lock in the intended behavior.

Referenced code: src/ssl_api_ext.c:1073-1080 (manifests via src/ssl_sess.c:958-968) (10 lines)

No test covers the wolfSSL_CTX_set_timeout - set_TicketHint regression path

File: tests/api.c:27884-27913
Function: test_wolfSSL_CTX_set_timeout
Severity: Medium
Category: test

The new tests in test_ssl_ext.c exercise the direct setter, but the highest-impact behavior change is via the internal caller wolfSSL_CTX_set_timeout. The existing test_wolfSSL_CTX_set_timeout only uses small values (20, 30, 100), all below the new 1800s cap, so it will neither catch the regression nor lock in whatever the intended behavior becomes. Since the change silently alters what SSL_CTX_set_timeout returns for common values, this path needs explicit coverage.

Recommendation: Add a case under OPENSSL_EXTRA + HAVE_SESSION_TICKET + server that calls wolfSSL_CTX_set_timeout with a value >= WOLFSSL_TICKET_KEY_LIFETIME/2 (e.g. 3600) and asserts the intended outcome (success with clamped hint per the fix above), and verify ctx state is consistent.

Skipped findings

• [Info] Stricter compile-time #error may break previously-valid custom lifetime/hint configurations

Review generated by Skoll

#10822 (review)

Skoll Multi-Scan Review

Modes: review + review-securityOverall recommendation: COMMENT
Findings: 4 total — 4 posted, 0 skipped
3 finding(s) posted as inline comments (see file-level comments below)
1 finding(s) not tied to a diff line (full detail below)

Posted findings

• [Medium] [review+review-security] Oversized ticket hint returns FATAL and tears down the whole handshake instead of degrading to a ticket-less connection — src/internal.c:41749
• [Low] [review] New failure uses WOLFSSL_TICKET_RET_FATAL while other encrypt-side returns in this function use REJECT — src/internal.c:41751
• [Info] [review] Trivial custom ticket-encryption callback duplicated across two test files — tests/api/test_ssl_ext.c:203

Findings not tied to a diff line

Compile-time ticket lifetime guard does not cover the new runtime 2*hint constraint

File: wolfssl/internal.h:2050
Function: DefTicketEncCb
Severity: Low
Category: Convention

internal.h statically enforces WOLFSSL_TICKET_KEY_LIFETIME > SESSION_TICKET_HINT_DEFAULT, but the new runtime rule requires 2 * ticketHint < WOLFSSL_TICKET_KEY_LIFETIME. With the defaults (lifetime 3600, default hint 300) there is no problem (600 < 3600). However, a user who overrides WOLFSSL_TICKET_KEY_LIFETIME to a value in the range (SESSION_TICKET_HINT_DEFAULT, 2*SESSION_TICKET_HINT_DEFAULT] (e.g. 400-600) passes the compile-time #error check yet now has every default-hint ticket handshake fail at runtime, with no build-time warning. This is a narrow, already-degenerate configuration, but the failure surfaces only at runtime.

Recommendation: Consider tightening the static assert to #if WOLFSSL_TICKET_KEY_LIFETIME <= 2 * SESSION_TICKET_HINT_DEFAULT (or documenting the 2*hint requirement near WOLFSSL_TICKET_KEY_LIFETIME) so users who shrink the key lifetime are warned at build time rather than seeing runtime handshake failures.

Referenced code: wolfssl/internal.h:2050-2052 (3 lines)

Review generated by Skoll