nick/amethyst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(quic): second-pass audit fixes for path validation

Browse Source 
Three correctness bugs surfaced by post-fix re-audit, plus minor
cleanups.

  - Bug A (validation hang): checkPathValidationTimeoutLocked was
    only called from handlePtoFired. PATH_CHALLENGE is ack-eliciting
    so the peer ACKs it; that ACK resets consecutivePtoCount, which
    means the PTO timer that used to host the budget check stops
    firing. Validation could hang indefinitely on a peer that ACKs
    but doesn't reply with PATH_RESPONSE. Fix: drive the budget
    check from drainOutbound (every send-loop wake).

  - Bug B (stale retire): under abrupt-migration semantics the
    prior CID is abandoned the moment we rotate. Queuing the retire
    only inside applyPathResponse meant two consecutive failed
    validations would leave the original seq=0 unretired forever.
    Fix: queue priorSeq in tryStartValidation; advance
    activeCidSequence at trigger time so it tracks the on-wire DCID.

  - Bug C (spec MUST violation): RFC 9000 §5.1.2 requires server-
    forced retirement of the active CID when the peer's
    retire_prior_to advances past it. Previously the parser silently
    accepted the offer and we kept stamping a now-retired CID.
    Fix: new PathValidator.forceRotateToHigherSequence; called from
    applyPeerNewConnectionIdLocked after a successful Stored result.
    No PATH_CHALLENGE needed (same path, just different CID).
    Closes connection with CONNECTION_ID_LIMIT_ERROR if the pool
    is empty when forced rotation is needed.

Concurrency:
  - Add @Volatile to consecutivePtoCount. The driver kdoc claimed
    it already was; it wasn't. The send-loop reads it lockless for
    backoff calculation while three writers mutate it (driver PTO
    fire, parser ACK reset, applyPeerPathResponseLocked reset).

Cleanup:
  - Drop redundant destinationConnectionId re-stamp in
    applyPeerPathResponseLocked (already rotated at challenge time).
  - Fix PathMigrationResult kdoc to acknowledge that NotConnected
    is produced only by the connection-level wrapper.
  - Update applyPeerNewConnectionIdLocked kdoc with the §5.1.2
    forced-rotation contract.

Tests:
  - PathValidatorTest:
    + triggerRetiresPriorSequenceImmediately (Bug B)
    + twoConsecutiveFailedValidationsRetireAllAbandonedSequences
      (Bug B regression — would have caught the original miss)
    + forceRotateRunsWhenWatermarkPassesActiveCid (Bug C)
    + forceRotateNoOpWhenWatermarkBelowActive (Bug C edge)
    + forceRotateRotatesAgainWhenNewerOfferAdvancesWatermark (Bug C cascading)
  - ClientPathMigrationTest:
    + newConnectionIdWithRetirePriorToPastActiveForcesRotationOnSamePath
      (Bug C wire-level)
    + Updated fullMigrationRoundTrip to assert RETIRE rides in the
      same packet as PATH_CHALLENGE under abrupt-migration semantics.
    + Updated pathResponseWithMismatchingPayloadKeepsValidatingAndDcid
      to reflect activeCidSequence advances at trigger time.

All :quic:jvmTest (39 tests in path-validation suite) and
:nestsClient:jvmTest pass.

https://claude.ai/code/session_01PVVhSQXvw4K4oQ46FzpgaT
This commit is contained in:
Claude
2026-05-08 19:57:55 +00:00
parent 9b9ede2e1e
commit 07ba23a71c
5 changed files with 369 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/PathValidator.kt
+118 -22
View File
@@ -116,8 +116,12 @@ sealed class PathValidationState {
}
/**
* Outcome of [PathValidator.tryStartValidation], used by the caller
* (driver / test) to know whether a probe was actually issued.
* Outcome of attempting to start a client-initiated path
* migration. Produced by [PathValidator.tryStartValidation] (which
* never returns [NotConnected]) and the connection-level wrapper
* [QuicConnection.triggerPathMigrationLocked] (which adds the
* pre-handshake gate). Used by the driver / application code that
* fires the trigger to decide whether to retry later.
*/
enum class PathMigrationResult {
/** A `PATH_CHALLENGE` was queued; state is [PathValidationState.Validating]. */
@@ -402,11 +406,16 @@ class PathValidator(
* sequence unused CID, generates a random 8-byte challenge,
* queues the challenge in [pendingChallenges], and transitions
* [state] to [PathValidationState.Validating]. The selected
* CID is removed from the unused pool; on validation success
* [applyPathResponse] commits it (callers swap the writer's
* DCID), and on timeout [checkValidationTimeout] queues it
* for RETIRE_CONNECTION_ID so the peer can drop the routing
* entry.
* CID is removed from the unused pool. The prior CID's
* sequence is queued for RETIRE_CONNECTION_ID immediately —
* we're abandoning that path and the peer can drop the
* routing entry now (RFC 9000 §5.1.2). [activeCidSequence]
* is bumped to the new sequence so it tracks what the writer
* is putting on the wire.
*
* On success the writer just promotes [PathValidationState] to
* [PathValidationState.Succeeded]; on timeout the failed CID
* is also queued for retire (see [checkValidationTimeout]).
*
* Caller is responsible for two follow-ups when this returns
* [PathMigrationResult.Started]:
@@ -428,12 +437,25 @@ class PathValidator(
require(it.size == 8) { "challenge payload supplier must return 8 bytes" }
}
pendingChallenges.addLast(payload)
val priorSeq = activeCidSequence
// Bug-B fix: retire the prior sequence at trigger time.
// Abrupt migration means we won't use the old DCID again,
// so the peer's routing entry for it is dead state. Without
// this, two consecutive failed validations leave the
// initial seq=0 unretired indefinitely.
if (priorSeq != seq) queueRetireSequence(priorSeq)
// [activeCidSequence] tracks "the seq the writer is currently
// stamping on the wire", so it advances at trigger time
// alongside `destinationConnectionId`. The
// [PathValidationState.Validating.priorCidSequence] field
// preserves the value for diagnostic / qlog purposes.
activeCidSequence = seq
state =
PathValidationState.Validating(
challengeData = payload,
newCidSequence = seq,
newConnectionId = entry.connectionId,
priorCidSequence = activeCidSequence,
priorCidSequence = priorSeq,
startedAtMillis = nowMillis,
priorPtoMillis = currentPtoMillis,
)
@@ -441,30 +463,33 @@ class PathValidator(
}
/**
* Process an inbound `PATH_RESPONSE` payload. Returns true when
* it matched the outstanding challenge and the migration just
* completed; false if there's no outstanding challenge or the
* payload doesn't match (an attacker echoing random bytes).
* Process an inbound `PATH_RESPONSE` payload. Confirms the
* outstanding challenge: state transitions to
* [PathValidationState.Succeeded] and a [ValidationOutcome.Validated]
* is returned with the new sequence + connection-id bytes for
* the connection-level wrapper to surface in qlog.
*
* Side effects on success:
* - [state] transitions to [PathValidationState.Succeeded].
* - [activeCidSequence] is bumped to the validated sequence.
* - The prior sequence is queued for RETIRE_CONNECTION_ID.
* - Returns the new entry so the connection can swap its
* `destinationConnectionId` field.
* Note that [activeCidSequence] and the retire-queue entry for
* the prior sequence were already mutated at challenge-issue
* time (see [tryStartValidation]) — under the abrupt-migration
* model the prior CID is abandoned the moment we rotate, not
* when the response arrives. So this method has no further
* cleanup work beyond marking the state as Succeeded.
*
* Returns [ValidationOutcome.NotValidating] if no challenge
* is outstanding, or [ValidationOutcome.PayloadMismatch] if
* the bytes don't match. Both are silently dropped at the
* caller per RFC 9000 §8.2.2.
*/
fun applyPathResponse(payload: ByteArray): ValidationOutcome {
val current = state as? PathValidationState.Validating ?: return ValidationOutcome.NotValidating
if (!payload.contentEquals(current.challengeData)) return ValidationOutcome.PayloadMismatch
val priorSeq = activeCidSequence
activeCidSequence = current.newCidSequence
if (priorSeq != current.newCidSequence) queueRetireSequence(priorSeq)
state = PathValidationState.Succeeded
successfulValidations += 1
return ValidationOutcome.Validated(
newSequence = current.newCidSequence,
connectionId = current.newConnectionId,
retiredSequence = priorSeq,
retiredSequence = current.priorCidSequence,
)
}
@@ -534,6 +559,77 @@ class PathValidator(
}
}
/**
* RFC 9000 §5.1.2: "If the active connection ID's sequence
* number is less than the Retire Prior To value, the endpoint
* MUST retire the active connection ID and adopt one with a
* higher sequence number."
*
* Server-forced retirement does NOT require path validation —
* it's the same 4-tuple, just a different CID. Pick the
* lowest-sequence entry from the pool, swap it in, queue the
* old active sequence for RETIRE_CONNECTION_ID. If no spare
* is available, returns null and the caller should close the
* connection (we have no CID to use that satisfies the peer's
* watermark).
*
* If a path validation happens to be in progress when this
* fires, the in-flight challenge's CID may itself fall under
* the watermark — abandon the validation and queue the failed
* sequence for retire alongside the swap.
*/
fun forceRotateToHigherSequence(): ForcedRotationResult? {
if (activeCidSequence >= retirePriorToWatermark) return null
val (seq, entry) = unusedCids.entries.firstOrNull() ?: return ForcedRotationResult.NoSpareCid
unusedCids.remove(seq)
val priorSeq = activeCidSequence
queueRetireSequence(priorSeq)
activeCidSequence = seq
// If we were mid-validation, the challenge we issued is
// for a CID that may now be retired (priorSeq could be
// the seq we were trying to validate). Abandon it.
val s = state
if (s is PathValidationState.Validating) {
queueRetireSequence(s.newCidSequence)
pendingChallenges.removeAll { it.contentEquals(s.challengeData) }
state = PathValidationState.Failed
failedValidations += 1
}
return ForcedRotationResult.Rotated(newSequence = seq, connectionId = entry.connectionId, retiredSequence = priorSeq)
}
sealed class ForcedRotationResult {
/**
* Watermark moved past active CID but the pool was empty —
* the caller cannot satisfy the spec MUST. RFC 9000 §5.1.2
* leaves the recovery to the caller; closing the connection
* with CONNECTION_ID_LIMIT_ERROR is the safe default.
*/
object NoSpareCid : ForcedRotationResult()
data class Rotated(
val newSequence: Long,
val connectionId: ByteArray,
val retiredSequence: Long,
) : ForcedRotationResult() {
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is Rotated) return false
return newSequence == other.newSequence &&
connectionId.contentEquals(other.connectionId) &&
retiredSequence == other.retiredSequence
}
override fun hashCode(): Int {
var h = newSequence.hashCode()
h = 31 * h + connectionId.contentHashCode()
h = 31 * h + retiredSequence.hashCode()
return h
}
}
}
companion object {
/**
* Default cap on the unused-CID pool. RFC 9000 §18.2 default
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt
+65 -26
View File
@@ -560,7 +560,17 @@ class QuicConnection(
* PTO expiration without an intervening ACK; reset to 0 when an
* inbound ACK acknowledges any ack-eliciting packet. The driver
* doubles its sleep between probes by `1 shl consecutivePtoCount`.
*
* `@Volatile` because the driver's send-loop reads this without
* holding `streamsLock` (in the backoff calculation in
* [QuicConnectionDriver.sendLoopBody]), while three writers
* mutate it: the driver's PTO firing path, the parser's ACK
* handler resetting on inbound ACK, and the
* [applyPeerPathResponseLocked] reset on successful migration.
* Without volatility the JIT can cache a stale value
* indefinitely and the backoff multiplier grows unbounded.
*/
@Volatile
internal var consecutivePtoCount: Int = 0
/**
@@ -1911,13 +1921,15 @@ class QuicConnection(
* RFC 9000 §19.15: store a peer-issued connection ID into the
* [pathValidator] pool, enforcing `retire_prior_to` semantics
* and capping pool size. On a peer protocol violation
* ([PathValidator.RecordResult.RetirePriorToExceedsSequence] /
* [PathValidator.RecordResult.RetirePriorToRegressed] /
* [PathValidator.RecordResult.DuplicateSequenceMismatch] /
* [PathValidator.RecordResult.InvalidCidLength] /
* [PathValidator.RecordResult.InvalidStatelessResetToken]) the
* connection is closed with FRAME_ENCODING_ERROR — these are
* MUSTs in the spec.
* the connection is closed with FRAME_ENCODING_ERROR /
* PROTOCOL_VIOLATION — these are MUSTs in the spec.
*
* After a successful store, if the peer's `retire_prior_to`
* has advanced past our currently-active CID sequence,
* RFC 9000 §5.1.2 obligates us to "retire the active
* connection ID and adopt one with a higher sequence number."
* That's a server-forced rotation on the same path — no
* PATH_CHALLENGE needed (Bug-C fix).
*
* Caller must hold [streamsLock].
*/
@@ -1927,23 +1939,18 @@ class QuicConnection(
connectionId: ByteArray,
statelessResetToken: ByteArray,
) {
when (
val recordResult =
pathValidator.recordPeerNewConnectionId(
sequenceNumber = sequenceNumber,
retirePriorTo = retirePriorTo,
connectionId = connectionId,
statelessResetToken = statelessResetToken,
)
) {
PathValidator.RecordResult.Stored -> {
Unit
}
PathValidator.RecordResult.Duplicate -> {
Unit
}
PathValidator.RecordResult.AlreadyRetired -> {
when (recordResult) {
PathValidator.RecordResult.Stored,
PathValidator.RecordResult.Duplicate,
PathValidator.RecordResult.AlreadyRetired,
-> {
Unit
}
@@ -1953,31 +1960,65 @@ class QuicConnection(
// we MAY treat this as CONNECTION_ID_LIMIT_ERROR. We
// only drop silently here — pinning memory is the
// real concern, and the cap defense already handled
// that.
// that. Skip the watermark check below; this offer
// wasn't accepted.
return
}
PathValidator.RecordResult.RetirePriorToExceedsSequence -> {
markClosedExternally(
"FRAME_ENCODING_ERROR: NEW_CONNECTION_ID retire_prior_to ($retirePriorTo) > sequence_number ($sequenceNumber)",
)
return
}
PathValidator.RecordResult.DuplicateSequenceMismatch -> {
markClosedExternally(
"PROTOCOL_VIOLATION: NEW_CONNECTION_ID seq=$sequenceNumber re-issued with different bytes",
)
return
}
PathValidator.RecordResult.InvalidCidLength -> {
markClosedExternally(
"FRAME_ENCODING_ERROR: NEW_CONNECTION_ID has invalid cid length",
)
return
}
PathValidator.RecordResult.InvalidStatelessResetToken -> {
markClosedExternally(
"FRAME_ENCODING_ERROR: NEW_CONNECTION_ID has invalid stateless reset token length",
)
return
}
}
// Bug-C fix: §5.1.2 server-forced rotation. The watermark
// may have advanced past our active CID as a side effect of
// [recordPeerNewConnectionId]. If so we MUST swap on the
// same path before the next outbound packet (which would
// otherwise stamp a now-retired CID).
when (val rotation = pathValidator.forceRotateToHigherSequence()) {
null -> {
Unit
}
// active CID is still valid; nothing to do.
PathValidator.ForcedRotationResult.NoSpareCid -> {
// Watermark forced retirement of the active CID but
// the pool is empty — we have nothing valid to use.
// RFC 9000 §5.1.2 leaves recovery to the endpoint;
// CONNECTION_ID_LIMIT_ERROR is the canonical close.
markClosedExternally(
"CONNECTION_ID_LIMIT_ERROR: peer retired our active CID with no spare available",
)
}
is PathValidator.ForcedRotationResult.Rotated -> {
destinationConnectionId = ConnectionId(rotation.connectionId)
qlogObserver.onConnectionIdActivated("peer", rotation.newSequence, rotation.connectionId)
qlogObserver.onConnectionIdRetired("peer", rotation.retiredSequence)
}
}
}
@@ -2014,14 +2055,12 @@ class QuicConnection(
qlogObserver.onConnectionIdActivated("peer", outcome.newSequence, outcome.connectionId)
qlogObserver.onConnectionIdRetired("peer", outcome.retiredSequence)
pathValidator.acknowledgeTerminal()
// Note: the writer's `destinationConnectionId` was
// already swapped to [outcome.connectionId] inside
// The writer's `destinationConnectionId` was already
// swapped at challenge time inside
// [triggerPathMigrationLocked] (abrupt-migration
// model — the challenge itself MUST go out on the
// new path per RFC 9000 §9.3). On success there's
// nothing more to swap; we just confirm the
// promotion is durable by re-stamping (idempotent).
destinationConnectionId = ConnectionId(outcome.connectionId)
// model — RFC 9000 §9.3 requires the challenge on
// the new path). On success there is no further
// mutation to make.
}
}
}
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt
+11
View File
@@ -91,6 +91,17 @@ fun drainOutbound(
return datagram
}
// Bug-A fix: drive the §8.2.4 3*PTO validation budget on every
// drain, not just on PTO expiration. The peer ACKs PATH_CHALLENGE
// (it's ack-eliciting), and ACK arrival resets consecutivePtoCount —
// so the PTO timer that previously hosted [checkPathValidationTimeoutLocked]
// can stop firing before the budget elapses, leaving validation
// hung indefinitely. Drain happens at least every PTO_BASE
// (~30-100ms) even when the application is idle, which covers the
// budget-elapsed condition reliably. Cheap (one type-cast +
// time comparison) when the validator is in [PathValidationState.Idle].
conn.checkPathValidationTimeoutLocked(nowMillis)
// Drain destructive frame sources into local lists, ONCE.
val initialContents = collectHandshakeLevelFrames(conn, EncryptionLevel.INITIAL, nowMillis)
val handshakeContents = collectHandshakeLevelFrames(conn, EncryptionLevel.HANDSHAKE, nowMillis)
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ClientPathMigrationTest.kt
+75 -21
View File
@@ -163,14 +163,24 @@ class ClientPathMigrationTest {
"DCID must rotate at challenge time so PATH_CHALLENGE goes out on the new path",
)
// Step 3 — the next outbound application packet must carry a
// PATH_CHALLENGE stamped with the NEW DCID.
// Step 3 — the next outbound application packet must carry
// BOTH the PATH_CHALLENGE (stamped with the NEW DCID) AND
// a RETIRE_CONNECTION_ID for the old sequence (Bug-B fix:
// abrupt migration retires the prior CID at trigger time,
// not at response time, so the writer drains both in the
// same packet).
val challengeOut = drainOutbound(client, nowMillis = 100L)
assertTrue(challengeOut != null, "client must emit a packet carrying PATH_CHALLENGE")
val outboundFrames = pipe.decryptClientApplicationFrames(challengeOut)
assertTrue(outboundFrames != null, "client outbound must decrypt with server keys")
val challenge = outboundFrames.firstOrNull { it is PathChallengeFrame } as? PathChallengeFrame
assertTrue(challenge != null, "outbound must contain PATH_CHALLENGE — got ${outboundFrames.map { it::class.simpleName }}")
val retire = outboundFrames.firstOrNull { it is RetireConnectionIdFrame } as? RetireConnectionIdFrame
assertTrue(
retire != null,
"outbound must contain RETIRE_CONNECTION_ID for the prior CID alongside PATH_CHALLENGE — got ${outboundFrames.map { it::class.simpleName }}",
)
assertEquals(0L, retire.sequenceNumber)
// Step 4 — server echoes the payload in PATH_RESPONSE.
val response =
@@ -179,26 +189,13 @@ class ClientPathMigrationTest {
)!!
feedDatagram(client, response, nowMillis = 200L)
// Step 5 — validation succeeded; DCID is still the new bytes
// (already rotated at step 2), the prior sequence (0) is
// queued for RETIRE_CONNECTION_ID, and activeCidSequence now
// reflects the new sequence.
// Step 5 — validation succeeded; state transitions to
// Idle (after acknowledgeTerminal), activeCidSequence is
// still the new sequence (already bumped at trigger),
// DCID still the new bytes.
assertContentEquals(newCidBytes, client.destinationConnectionId.bytes)
assertEquals(1L, client.pathValidator.activeCidSequence)
assertTrue(
client.pathValidator.pendingRetireSequences.contains(0L),
"old sequence number must be queued for retire",
)
// Step 6 — the next outbound packet should carry the
// RETIRE_CONNECTION_ID for the old sequence.
val retireOut = drainOutbound(client, nowMillis = 200L)
assertTrue(retireOut != null, "client must emit a packet after PATH_RESPONSE")
val frames = pipe.decryptClientApplicationFrames(retireOut)
assertTrue(frames != null, "outbound after rotation must decrypt")
val retire = frames.firstOrNull { it is RetireConnectionIdFrame } as? RetireConnectionIdFrame
assertTrue(retire != null, "outbound must contain RETIRE_CONNECTION_ID — got ${frames.map { it::class.simpleName }}")
assertEquals(0L, retire.sequenceNumber)
assertEquals(1L, client.pathValidator.successfulValidations)
// Sanity: connection still alive after the rotation.
assertEquals(QuicConnection.Status.CONNECTED, client.status)
@@ -243,7 +240,11 @@ class ClientPathMigrationTest {
assertContentEquals(newCid, client.destinationConnectionId.bytes, "mismatched response must not undo the rotation")
assertTrue(client.pathValidator.state is PathValidationState.Validating, "still validating")
assertEquals(0L, client.pathValidator.activeCidSequence, "active sequence still old until validation succeeds")
assertEquals(
1L,
client.pathValidator.activeCidSequence,
"active sequence tracks what the writer is putting on the wire — bumped at challenge time per Bug-B fix",
)
}
@Test
@@ -330,6 +331,59 @@ class ClientPathMigrationTest {
assertEquals(0, client.consecutivePtoCount, "successful path validation must reset consecutivePtoCount")
}
@Test
fun newConnectionIdWithRetirePriorToPastActiveForcesRotationOnSamePath() =
runBlocking {
// Bug-C regression: RFC 9000 §5.1.2 — a peer that
// advances retire_prior_to past our active CID forces
// us to rotate on the SAME path (no PATH_CHALLENGE).
// Pre-fix the parser silently accepted the offer and
// we'd keep stamping the now-retired seq=0 on outbound
// packets.
val (client, pipe) = newConnectedClient()
val originalDcid = client.destinationConnectionId
val replacementCid = ByteArray(8) { (0xB0 or it).toByte() }
val token = ByteArray(16) { 0x33 }
// First offer: seq=1, no retirement. Pool fills.
feedDatagram(
client,
pipe.buildServerApplicationDatagram(
listOf(NewConnectionIdFrame(1L, 0L, replacementCid, token)),
)!!,
nowMillis = 0L,
)
assertEquals(originalDcid, client.destinationConnectionId, "first offer doesn't force rotation")
// Second offer: seq=2 with retire_prior_to=1. Watermark
// advances past our active seq=0. MUST rotate on the
// same path to seq=1 (the lowest spare).
val secondToken = ByteArray(16) { 0x44 }
feedDatagram(
client,
pipe.buildServerApplicationDatagram(
listOf(NewConnectionIdFrame(2L, 1L, ByteArray(8) { 0xCC.toByte() }, secondToken)),
)!!,
nowMillis = 0L,
)
assertContentEquals(
replacementCid,
client.destinationConnectionId.bytes,
"DCID must rotate to the lowest spare (seq=1) when watermark advances past seq=0",
)
assertEquals(1L, client.pathValidator.activeCidSequence)
assertTrue(
client.pathValidator.pendingRetireSequences.contains(0L),
"old active seq=0 must be queued for RETIRE_CONNECTION_ID",
)
assertEquals(
QuicConnection.Status.CONNECTED,
client.status,
"server-forced rotation must NOT close the connection",
)
}
@Test
fun triggerPathMigrationBeforeHandshakeReturnsNotConnected() =
runBlocking {
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/PathValidatorTest.kt
+100
View File
@@ -173,6 +173,106 @@ class PathValidatorTest {
assertEquals(1L, v.successfulValidations)
}
@Test
fun triggerRetiresPriorSequenceImmediately() {
// Bug-B regression: under abrupt-migration semantics the
// prior CID is abandoned the moment we rotate (RFC 9000
// §5.1.2). Queuing the retire only at PATH_RESPONSE success
// means a failed first attempt followed by a never-validated
// second attempt would leave seq=0 unretired forever.
val v = PathValidator(challengePayloadFactory = { ByteArray(8) { 0x11 } })
v.recordPeerNewConnectionId(1L, 0L, ByteArray(8) { 0x01 }, ByteArray(16) { 0x10 })
v.tryStartValidation(0L, 100L)
assertTrue(
v.pendingRetireSequences.contains(0L),
"prior sequence must be queued for retire at trigger time, not at response time",
)
assertEquals(1L, v.activeCidSequence, "activeCidSequence advances at trigger to track the on-wire DCID")
}
@Test
fun twoConsecutiveFailedValidationsRetireAllAbandonedSequences() {
// Bug-B regression: prior to the fix, two failed validations
// would leave the original seq=0 unretired (the priorSeq
// queue entry was scheduled inside applyPathResponse, which
// never runs for failed validations).
val v = PathValidator(challengePayloadFactory = { ByteArray(8) { 0x22 } })
v.recordPeerNewConnectionId(1L, 0L, ByteArray(8) { 0x01 }, ByteArray(16) { 0x10 })
v.recordPeerNewConnectionId(2L, 0L, ByteArray(8) { 0x02 }, ByteArray(16) { 0x20 })
val pto = 1_000L
// First attempt fails by timeout.
v.tryStartValidation(0L, pto)
v.checkValidationTimeout(nowMillis = pto * 4L)
v.acknowledgeTerminal()
// Second attempt also fails.
v.tryStartValidation(nowMillis = 10_000L, currentPtoMillis = pto)
v.checkValidationTimeout(nowMillis = 10_000L + pto * 4L)
// All three sequences abandoned along the way (the original
// seq=0 plus seq=1 plus seq=2) MUST be queued for retire.
for (seq in listOf(0L, 1L, 2L)) {
assertTrue(
v.pendingRetireSequences.contains(seq),
"seq=$seq abandoned but not queued for retire — pending=${v.pendingRetireSequences}",
)
}
}
@Test
fun forceRotateRunsWhenWatermarkPassesActiveCid() {
// Bug-C regression: RFC 9000 §5.1.2 — "If the active
// connection ID's sequence number is less than the Retire
// Prior To value, the endpoint MUST retire the active
// connection ID and adopt one with a higher sequence number."
val v = PathValidator()
v.recordPeerNewConnectionId(1L, 0L, ByteArray(8) { 0x01 }, ByteArray(16) { 0x10 })
v.recordPeerNewConnectionId(2L, 0L, ByteArray(8) { 0x02 }, ByteArray(16) { 0x20 })
// Now the peer advances retire_prior_to past our active CID
// (seq 0), forcing same-path rotation.
v.recordPeerNewConnectionId(3L, retirePriorTo = 1L, ByteArray(8) { 0x03 }, ByteArray(16) { 0x30 })
val rotation = v.forceRotateToHigherSequence()
assertTrue(rotation is PathValidator.ForcedRotationResult.Rotated, "got $rotation")
assertEquals(1L, rotation.newSequence, "must pick the lowest spare sequence")
assertEquals(0L, rotation.retiredSequence)
assertContentEquals(ByteArray(8) { 0x01 }, rotation.connectionId)
assertEquals(1L, v.activeCidSequence)
assertTrue(v.pendingRetireSequences.contains(0L))
}
@Test
fun forceRotateNoOpWhenWatermarkBelowActive() {
val v = PathValidator()
v.recordPeerNewConnectionId(1L, 0L, ByteArray(8) { 0x01 }, ByteArray(16) { 0x10 })
// Watermark is still 0; active is also 0 (initial). Nothing
// to rotate.
assertEquals(null, v.forceRotateToHigherSequence())
assertEquals(0L, v.activeCidSequence)
}
@Test
fun forceRotateRotatesAgainWhenNewerOfferAdvancesWatermark() {
// Cascading server-forced rotation: peer issues seq=1 then
// (later) seq=2 with retire_prior_to=2, retiring seq=1
// immediately after we'd just adopted it.
val v = PathValidator()
v.recordPeerNewConnectionId(1L, 0L, ByteArray(8) { 0x01 }, ByteArray(16) { 0x10 })
// Tester adopts seq=1 via initial-state simulation: trigger
// a validation just to bump activeCidSequence past 0.
v.tryStartValidation(0L, 100L)
v.applyPathResponse(ByteArray(8) { 0x00 }) // mismatched, validation stays Validating
v.acknowledgeTerminal() // benign no-op (state isn't terminal)
assertEquals(1L, v.activeCidSequence)
// Peer now advances watermark to 2 with a fresh offer.
v.recordPeerNewConnectionId(2L, retirePriorTo = 2L, ByteArray(8) { 0x02 }, ByteArray(16) { 0x20 })
val rotation = v.forceRotateToHigherSequence()
assertTrue(rotation is PathValidator.ForcedRotationResult.Rotated, "got $rotation")
assertEquals(2L, rotation.newSequence)
assertEquals(1L, rotation.retiredSequence)
assertEquals(2L, v.activeCidSequence)
}
@Test
fun pathResponseWithMismatchedPayloadIsIgnored() {
val v = PathValidator(challengePayloadFactory = { ByteArray(8) { 0x77 } })