F-5606: don't enforce DTLS 1.3 2^48-1 epoch cap on the receive side#10627
Draft
julek-wolfssl wants to merge 12 commits into
Draft
F-5606: don't enforce DTLS 1.3 2^48-1 epoch cap on the receive side#10627julek-wolfssl wants to merge 12 commits into
julek-wolfssl wants to merge 12 commits into
Conversation
RFC 9147 Section 4.2.1 limits the DTLS 1.3 epoch to 2^48-1. SendTls13KeyUpdate now refuses to send a KeyUpdate when the sending epoch is already at the maximum, and Dtls13KeyUpdateAckReceived rejects an epoch that would exceed the limit after incrementing (previously only a full 64-bit wrap to zero was checked).
The size_t length was cast directly to the int taken by ConstantCompare. On 64-bit builds a size with a negative low 32-bit value (e.g. 0x80000000) became a negative length, so ConstantCompare ran zero iterations and returned 0 (equal) without comparing. Reject size > INT_MAX with a non-zero (mismatch) result before narrowing.
WOLFSSL_FAILURE is 0, which equals X509_V_OK, so a NULL ssl was indistinguishable from successful verification under the standard "SSL_get_verify_result(ssl) \!= X509_V_OK" idiom. Return WOLFSSL_X509_V_ERR_APPLICATION_VERIFICATION (50, matching the OpenSSL compat value) instead, and add it to the X509 verify-error enum.
TLSX_MFL_Parse only checked that the extension was requested and that the value was recognized, not that the server echoed the requested value. Per RFC 6066 Section 4, compare the ServerHello value against the locally stored request and abort with a fatal illegal_parameter alert on mismatch.
A response ProtocolNameList was passed whole to ALPN_find_match, which accepts the first configured match, so a server could return more than one protocol. Per RFC 7301 Section 3.1 the ServerHello list must contain exactly one name; enforce that the single name spans the whole list and reject otherwise with a fatal illegal_parameter alert.
DoTls13CertificateRequest parsed the extensions but never verified the mandatory signature_algorithms extension was present. A request with none left peerSuites.hashSigAlgoSz at zero and was accepted. Per RFC 8446 Section 4.3.2, reject such a request with a fatal missing_extension alert before selecting a certificate response.
DoTls13CertificateRequest rejected a non-empty context during the handshake but did the complementary check for post-handshake auth. Per RFC 8446 Section 4.3.2 a post-handshake certificate_request_context must be non-empty and unique; reject a zero-length context and one that duplicates a still-pending request context with a fatal illegal_parameter alert.
wolfSSL_CTX_set_groups/wolfSSL_set_groups only rejected counts above WOLFSSL_MAX_GROUP_COUNT; a negative count skipped the copy loop and was cast to byte (e.g. 255) into numGroups, which InitSSL later trusts for a fixed-size copy. Reject count <= 0 in both, and in the set1_groups OpenSSL-compat wrappers.
Dtls13NewEpochSlot never updated oldestNumber after picking a candidate, so every eligible epoch compared "older" than the sentinel and the last eligible slot was returned instead of the lowest epoch number. Update oldestNumber alongside oldest so the true minimum is evicted.
FreeCiphersSide freed cipher->cam with XFREE only, leaving the expanded key schedule and IV in freed heap memory. Call wc_CamelliaFree (which ForceZeros the context) before XFREE, matching the ARIA cleanup above.
The constant-time path of _DH_compute_key (DH_compute_key_padded) had the XMEMMOVE source/dest swapped and used (padded_keySz - keySz) as the length instead of keySz, overwriting the secret with junk when keySz < padded_keySz. Move key[0..keySz-1] to the high end, matching the idiom used in tls.c/sniffer.c.
RFC 9147 Section 8's 2^48-1 epoch ceiling is a sender-only rule; the same paragraph says receiving implementations MUST NOT enforce it. The KeyUpdate receive path was rejecting a peer epoch that crossed 2^48-1, violating that. Guard only the genuine wrap-to-zero (Section 4.2.1) and let the receiving epoch advance past 2^48-1. The sender-side gates are unchanged.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates wolfSSL’s DTLS 1.3 KeyUpdate receive-path epoch handling to align with RFC 9147 by no longer enforcing the 2^48-1 ceiling on received epochs (while still preventing the dangerous wrap-to-zero), and it also includes several TLS/DTLS conformance and crypto hardening follow-up changes that are noted as coming from the dependent PR.
Changes:
- DTLS 1.3: adjust KeyUpdate receive-side epoch advancement to only reject wrap-to-zero, while keeping sender-side epoch ceiling checks.
- TLS 1.3: tighten CertificateRequest validation (context rules, duplicate context rejection for post-handshake auth, and require
signature_algorithms). - OpenSSL-compat / protocol hardening fixes:
wolfSSL_get_verify_result(NULL)returns a non-zero X509-style error, chunkedwolfSSL_CRYPTO_memcmp, stricter ALPN/MFL response parsing, DH shared-secret right-justification fix, Camellia key schedule free, and DTLS 1.3 epoch slot eviction fix.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
wolfssl/ssl.h |
Adds an OpenSSL-compatible verify error code used by wolfSSL_get_verify_result(NULL). |
wolfssl/internal.h |
Introduces DTLS 1.3 epoch max constants used by sender-side KeyUpdate gating. |
tests/api.c |
Updates API test expectations for wolfSSL_get_verify_result(NULL). |
src/tls13.c |
Tightens TLS 1.3 CertificateRequest parsing and adjusts DTLS 1.3 KeyUpdate receive epoch logic. |
src/tls.c |
Adds stricter input validation for group setters and hardens ALPN/MFL response parsing. |
src/ssl.c |
Changes OpenSSL-compat behaviors (get_verify_result, CRYPTO_memcmp) and group count validation message. |
src/pk.c |
Fixes DH padded shared-secret right-justification for short outputs. |
src/internal.c |
Ensures Camellia key schedule is freed/zeroized before deallocation. |
src/dtls13.c |
Fixes epoch slot eviction selection and hardens sender epoch increment validation on KeyUpdate ACK. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5951
to
+5954
| /* RFC 9147 Section 4.2.1: the DTLS 1.3 epoch is a 48-bit value and must not | ||
| * exceed 2^48-1. Expressed as the high/low 32-bit halves of a w64wrapper. */ | ||
| #define DTLS13_EPOCH_MAX_HI32 0x0000FFFFU | ||
| #define DTLS13_EPOCH_MAX_LO32 0xFFFFFFFFU |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to F-5606 (#10575). RFC 9147 Section 8's
2^48-1epoch ceiling is a sender-only rule; the same paragraph states that receiving implementations MUST NOT enforce it ("In order to allow this value to be changed later").The KeyUpdate receive path in
DoTls13KeyUpdatewas rejecting a peer epoch that crossed2^48-1, which violates that requirement. This changes the receive-side guard to catch only the genuine wrap-to-zero (RFC 9147 Section 4.2.1) — which would alias epoch 0 and reuse keys — and otherwise lets the receiving (peer) epoch advance past2^48-1.The sender-side gates that keep our own epoch at or below
2^48-1(SendTls13KeyUpdate,Dtls13KeyUpdateAckReceived) are unchanged.Testing
./configure --enable-dtls13 --enable-dtlsand rebuild — compiles cleanly under-Wall -Wextra.Depends on #10575, which introduces the F-5606 code this corrects. The other commits shown here belong to #10575 and will drop out of this diff once it merges to
master.