Skip to content

Conformance to RFC 9846: New version of TLS 1.3 specification - #11215

Open
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:tls13_rfc9846
Open

Conformance to RFC 9846: New version of TLS 1.3 specification#11215
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:tls13_rfc9846

Conversation

@SparkiDev

Copy link
Copy Markdown
Contributor

Description

RFC 9846, 5.5
CheckTLS13AEADSendLimit(): at the AEAD limit while sending early data, return TOO_MUCH_EARLY_DATA instead of calling Tls13UpdateKeys(). A KeyUpdate there would go out pre-handshake.

RFC 9846, 6.1
Three alert sites in DoAlert() / DoProcessAlertRecord(): TLS 1.3 user_canceled is now exempt from teardown and session invalidation at any AlertLevel, not just warning. TLS 1.2 unchanged.

RFC 9846, 4.7.3
New Tls13KeyUpdateLimitReached() helper shared by send and receive paths. At the 2^48-1 cap, DoTls13KeyUpdate() drops a peer's update_requested and continues, rather than failing the connection. App-initiated wolfSSL_update_keys() still returns BAD_STATE_E.

RFC 9846, 4.3
TranslateErrorToAlert() maps BUFFER_E to decode_error as well as BUFFER_ERROR — one case label covering 27 malformed-extension sites that previously aborted with no alert sent.

Tests added.

Testing

Added tests.

@SparkiDev SparkiDev self-assigned this Aug 20, 2026
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m3

  • FLASH: .text +28 B (+0.0%, 123,887 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4

  • FLASH: .text +64 B (+0.0%, 202,415 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +128 B (+0.1%, 183,804 B / 1,048,576 B, total: 18% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text +128 B (+0.0%, 776,668 B / 1,048,576 B, total: 74% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +128 B (+0.0%, 298,076 B / 1,048,576 B, total: 28% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text +64 B (+0.0%, 328,624 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text +64 B (+0.0%, 238,817 B / 262,144 B, total: 91% used)

gcc-arm-cortex-m7

  • FLASH: .text +64 B (+0.0%, 202,415 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +64 B (+0.0%, 298,972 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text +64 B (+0.0%, 238,881 B / 262,144 B, total: 91% used)

linuxkm-standard

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11215

Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/tls13.c
Comment thread src/tls13.c
Comment thread tests/api/test_tls13.c
Comment thread tests/api/test_tls13.c
Comment thread src/tls13.c
Comment thread src/tls13.c
Comment thread tests/api/test_tls13.c
Comment thread tests/api/test_tls13.c

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11215

Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/internal.c
Comment thread tests/api/test_tls13.c Outdated
Comment thread src/internal.c
Comment thread tests/api/test_tls13.c Outdated
RFC 9846, 5.5
CheckTLS13AEADSendLimit(): at the AEAD limit while sending early data,
return TOO_MUCH_EARLY_DATA instead of calling Tls13UpdateKeys(). A KeyUpdate
there would go out pre-handshake.

RFC 9846, 6.1
Three alert sites in DoAlert() / DoProcessAlertRecord(): TLS 1.3
user_canceled is now exempt from teardown and session invalidation at any
AlertLevel, not just warning. TLS 1.2 unchanged.

RFC 9846, 4.7.3
New Tls13KeyUpdateLimitReached() helper shared by send and receive
paths. At the 2^48-1 cap, DoTls13KeyUpdate() drops a peer's update_requested
and continues, rather than failing the connection. App-initiated
wolfSSL_update_keys() still returns BAD_STATE_E.

RFC 9846, 4.3
TranslateErrorToAlert() maps BUFFER_E to decode_error as well as
BUFFER_ERROR — one case label covering 27 malformed-extension sites that
previously aborted with no alert sent.

Tests added.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11215

Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/internal.c
* as either BUFFER_ERROR or the wolfCrypt BUFFER_E; both must map
* here, or the handshake aborts silently with no alert sent. */
case WC_NO_ERR_TRACE(BUFFER_ERROR):
case WC_NO_ERR_TRACE(BUFFER_E):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [Medium] BUFFER_E - decode_error makes the DTLS stateless ClientHello path send a fatal alert and stick alert_history at… · Incorrect error handling

CheckDtlsCookie() (src/dtls.c:291,294) returns BUFFER_E for a malformed DTLS 1.3 cookie extension, which reaches DoClientHello() at src/internal.c:40993. That error is now translated to decode_error, so a fatal alert is sent to an address-unverified peer and DtlsIgnoreError(BUFFER_E) then zeroes ret and the object keeps running with alert_history.last_tx.level == alert_fatal, which permanently suppresses later alerts via the guards at src/internal.c:20676, src/internal.c:34575 and src/tls13.c:14996.

Related known finding #7568 (similar but distinct): Both concern fatal-alert behavior on DoClientHello error paths, but #7568 maps VERSION_ERROR from version negotiation to a missing/wrong protocol_version alert, while this maps malformed-cookie BUFFER_E to decode_error before DTLS address validation and leaves alert history poisoned. The faulting operations, causes, and required fixes differ.

Fix: Suppress the alert on the pre-cookie stateless path in DoClientHello(), or clear alert_history.last_tx in DtlsResetState().

Comment thread src/internal.c
/* RFC 9846 Section 6.1: keep reading past a TLS 1.3 "user_canceled" until
* "close_notify" arrives, whatever AlertLevel the peer used. */
if (ret == alert_fatal &&
!(IsAtLeastTLSv1_3(ssl->version) && type == user_canceled))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 [Low] TLS 1.3 user_canceled exemption is applied before version negotiation, silently ignoring a pre-1.3 peer's fatal alert · Logic errors

IsAtLeastTLSv1_3(ssl->version) reflects the maximum supported version, not the negotiated one, until the ServerHello is processed. A fatal-level user_canceled arriving before negotiation is therefore exempted from teardown even when the peer is TLS 1.2, contradicting the PR's stated "TLS 1.2 unchanged" and leaving the connection in WANT_READ instead of failing. The same premature check exists at src/internal.c:24049.

Related known finding #6723 (similar but distinct): Both rely on IsAtLeastTLSv1_3(ssl->version) in early record processing, but #6723 applies TLS 1.3 CCS handling to DTLS 1.3 records, whereas this exempts a fatal user_canceled alert before TLS version negotiation. They affect different record types, functions, protocol roles, and require separate patches.

Fix: Gate the exemption on a negotiated TLS 1.3 connection (e.g. ssl->options.tls1_3) rather than on ssl->version alone.

Comment thread src/internal.c
* completed handshake, and it can only work out the remainder
* from the returned count. Report the short write; the next call
* re-runs this check with nothing sent yet and fails properly. */
if ((ret == WC_NO_ERR_TRACE(TOO_MUCH_EARLY_DATA)) && (sent > 0)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 [Low] SendData returns a short write without partialWrite enabled · TLS protocol issues

The new break exits the send loop with ssl->error == 0 and returns sent < sz. Every other short return in SendData is gated on ssl->options.partialWrite (line 29193), so a caller relying on the default all-or-error contract silently drops the untransmitted tail of the early data.

Fix: Record a sticky indicator (e.g. set ssl->error = TOO_MUCH_EARLY_DATA before breaking) so the caller can distinguish this short write from a complete one.

Comment thread tests/api/test_tls13.c
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);

ExpectIntEQ(test_memio_inject_message(&test_ctx, 1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 [Low] New user_canceled tests only cover the plaintext pre-handshake alert · Missing edge-case coverage on a function the PR also changed

Both new alert tests inject an unencrypted user_canceled record while the client has only sent ClientHello. The RFC 9846 6.1 case the PR targets — an authenticated fatal user_canceled after the handshake, where the peer keeps reading until close_notify — never reaches DoProcessAlertRecord with keys.decryptedCur set, and no test confirms the connection still carries data afterwards.

Fix: Add a case that completes the handshake, has the peer send a fatal user_canceled over the encrypted channel, then asserts subsequent read/write still succeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants