TLS 1.3: compare the 0-RTT fresh start reference against a whole second - #11379
Open
Frauschi wants to merge 1 commit into
Open
TLS 1.3: compare the 0-RTT fresh start reference against a whole second#11379Frauschi wants to merge 1 commit into
Frauschi wants to merge 1 commit into
Conversation
The RFC 8446 section 8.2 check flags a resumption ticket as belonging to a previous server instance when ticketSeen is below ctx->ticketStartTime. The two are read from different clocks: ticketStartTime comes from TimeNowInMilliseconds(), while ticketSeen is sess->bornOn scaled up and bornOn comes from LowResTimer(). Where those are separate clocks - glibc reads time() from the coarse realtime seconds and gettimeofday() from the fine grained one - the coarse clock can still report the previous second for up to a tick after each second boundary. A ctx created in that window records the new second while a session born microseconds later records the old one, so a freshly minted ticket is dated before the ctx that minted it and 0-RTT is silently refused. The PSK is still accepted, so the handshake resumes and only the early data disappears. cbcaf31 already dropped a whole second for this, but guarded it to WOLFSSL_32BIT_MILLI_TIME for the unrelated reason that 2^32 is not a multiple of 1000. Apply it everywhere. A ticket minted in the second before the ctx is then accepted, which does not matter for a heuristic about freshly started servers. This is what makes test_tls13_early_data_0rtt_replay fail intermittently on Linux CI. It does not reproduce where time() and gettimeofday() are coherent, which covers macOS and the usual container VMs. Injecting the lag with an LD_PRELOAD time() shim reproduces it within a few hundred iterations, and the fix survives 20000 iterations under the same shim.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11379
Scan targets checked: wolfssl-bugs, wolfssl-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
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.
Problem
test_tls13_early_data_0rtt_replayfails intermittently on Linux CI, roughly once every few hundred runs, with:The
ExpectTrue(wolfSSL_session_reused(ssl_s))immediately above it passes, so the session resumes and only the early data goes missing. It has been written off as a flaky test twice (PR #11306 in August, PR #11352 this week, on unrelated configurations). It is not flaky - it is a real refusal of 0-RTT by the server.Root cause
The RFC 8446 section 8.2 fresh start check flags a resumption ticket as belonging to a previous server instance when
ticketSeenis belowctx->ticketStartTime(src/tls13.c~6727). Those two values are read from different clocks:ctx->ticketStartTimeTimeNowInMilliseconds()->gettimeofday(), floored to a whole second (src/internal.c~2665)ticketSeensess->bornOn * 1000, andbornOnisLowResTimer()->XTIME(0)->time()On glibc/Linux
time()reads the coarse realtime seconds (vDSOxtime_sec, refreshed on the timer tick) whilegettimeofday()adds the elapsed delta since that refresh. For up to one tick after each second boundary - 1 ms atCONFIG_HZ=1000, 4 ms at 250 -gettimeofday()already reports second S whiletime()still returns S-1.A
WOLFSSL_CTXcreated inside that window recordsticketStartTime = S * 1000. A session born microseconds later recordsbornOn = S - 1, soticketSeen = (S - 1) * 1000, which is lower.ticketPredatesCtxis set andDoPreSharedKeysskips early data atsrc/tls13.c:7000. The PSK itself is still accepted, which is why the handshake resumes and only the early data vanishes.The
&&chain short-circuits beforewolfSSL_SSL_CTX_remove_session(), so the single-use eviction never runs either.The check landed in 978d795 / 1e46767 / cbcaf31. cbcaf31 already added exactly this mitigation - drop a further whole second - but guarded it to
WOLFSSL_32BIT_MILLI_TIME, for the unrelated reason that 2^32 is not a multiple of 1000, so a wrapped 32 bit ms clock keeps its true sub-second part through the% 1000. The 64 bit path never got it.Fix
Lift the guard so the same-second tolerance applies on every platform:
The cost is that a ticket minted in the second before the ctx was created is now accepted for 0-RTT. That is immaterial for a heuristic about freshly started servers, and it is the tolerance the 32 bit path has been running with since cbcaf31.
Reproducing it
It does not reproduce where
time()andgettimeofday()are coherent, which covers macOS and the usual container VMs - hence 380+ runs in August and, this week, 100 group runs on arm64, 40 full--apisweeps on an x86_64 build whose skip/pass fingerprint matches the failing Jenkins build exactly, and 4 million in-process iterations, all clean.Injecting the coarse clock reproduces it on demand. An
LD_PRELOADshim that makestime()laggettimeofday()for the firstLAG_USmicroseconds of each second:unit.testdeduplicates repeated-<testname>arguments, so to get iteration density the test body was temporarily wrapped in a repeat loop (360 iterations/sec in process, against 7.6/sec one process per run). That harness change is not part of this PR.LAG_US=0, 300 iterationsLAG_US=3000(a realistic Linux tick), 20000 iterationsLAG_US=500000, 5000 iterationsLAG_US=500000withwolfSSL_CTX_no_early_data_fresh_start_check()The third row isolates the gate: disabling only the fresh start check makes the failure go away under the same injected skew.
Testing
--enable-all --enable-debugon macOS/arm64:tls13group 124 passed, 15 skipped, 0 failed;make checkclean.--enable-sp-math-all --enable-all --enable-intelasm --enable-sp-asm --enable-smallstack --enable-stacksize=verbose(the configuration that failed in CI): full--apisweep0/431/2068/2499.test_tls13_0rtt_fresh_startandtest_tls13_0rtt_fresh_start_check_argsstill pass, so the check itself still rejects tickets that genuinely predate the ctx.No new test: exercising this in the suite needs the clock injected, which the harness has no mechanism for.