From 355b3dcbdbcd69c4a8707dd1625575873c7ab008 Mon Sep 17 00:00:00 2001 From: JackDoan Date: Fri, 25 Sep 2026 14:47:13 -0500 Subject: [PATCH] make all lane epochs match --- connection_state.go | 9 ++++++--- handshake_manager.go | 2 +- lanes.go | 6 +++++- lanes_test.go | 33 +++++++++++++++++++++++++++++---- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/connection_state.go b/connection_state.go index 5a3e3b64..46f38f1a 100644 --- a/connection_state.go +++ b/connection_state.go @@ -29,12 +29,15 @@ const ( // RehandshakeAfterMessages must stay below RejectAfterMessages so tunnels roll before the hard send stop. const _ = RejectAfterMessages - RehandshakeAfterMessages -// sessionEpoch hands out a receiver-local ordinal to every ConnectionState at creation. The RX +// sessionEpoch hands out a receiver-local ordinal to every tunnel at creation. The RX // staging sort (overlay/batch) orders packets by (epoch, message counter). A re-handshake never // rekeys an existing tunnel; it brings up a new hostinfo and ConnectionState with a counter space // starting near zero, while the old tunnel keeps decrypting until torn down. During that cutover // one flush batch can hold packets from both tunnels, and the epoch keeps the old tunnel's // packets sorted first. +// +// It is drawn once per tunnel, by the base session: a tunnel's multiport lane sessions copy it +// rather than draw their own. var sessionEpoch atomic.Uint64 type ConnectionState struct { @@ -85,7 +88,7 @@ func newConnectionStateFromResult(r *handshake.Result) (*ConnectionState, error) // // The lane gets its own counter and replay window starting from zero. No // handshake messages were spent on it, so unlike the base session there is -// nothing to seed. +// nothing to seed. The epoch is the tunnel's, not the lane's; see sessionEpoch. func newLaneConnectionState(m *laneMaterial, lane uint8) (*ConnectionState, error) { if lane == 0 { return nil, fmt.Errorf("lane 0 is the base session") @@ -107,7 +110,7 @@ func newLaneConnectionState(m *laneMaterial, lane uint8) (*ConnectionState, erro eKey: noiseutil.NewCipherStateFromKey(eKey, m.cipher), dKey: noiseutil.NewCipherStateFromKey(dKey, m.cipher), window: NewBits(ReplayWindow), - epoch: sessionEpoch.Add(1), + epoch: m.epoch, }, nil } diff --git a/handshake_manager.go b/handshake_manager.go index 8e4763fc..430f3f09 100644 --- a/handshake_manager.go +++ b/handshake_manager.go @@ -733,7 +733,7 @@ func (hm *HandshakeManager) maybeAllocLanes(hostinfo *HostInfo, result *handshak return } - hostinfo.lanes = newLaneSet(result, hm.config.laneCount, hm.f.myVpnAddrs[0], hostinfo.vpnAddrs[0]) + hostinfo.lanes = newLaneSet(result, hm.config.laneCount, hostinfo.ConnectionState.epoch, hm.f.myVpnAddrs[0], hostinfo.vpnAddrs[0]) } // beginHandshake handles an incoming handshake packet that doesn't match any diff --git a/lanes.go b/lanes.go index bc18487b..8a3420a1 100644 --- a/lanes.go +++ b/lanes.go @@ -145,6 +145,7 @@ type laneMaterial struct { myCert cert.Certificate peerCert *cert.CachedCertificate initiator bool + epoch uint64 } type laneProbeState struct { @@ -174,7 +175,9 @@ type laneProbeState struct { // returns nil when the pair has no lane beyond the base tunnel, which is the // normal answer for a peer running without multiport. No session is derived // here; each is derived on the first packet that needs it. -func newLaneSet(r *handshake.Result, myLanes int, myAddr, peerAddr netip.Addr) *laneSet { +// +// epoch is the base session's, which every lane session will share. +func newLaneSet(r *handshake.Result, myLanes int, epoch uint64, myAddr, peerAddr netip.Addr) *laneSet { // PeerPortCount and PeerBasePort are already bounded to uint16 by the // handshake payload parser. A zero port count is a peer that did not // advertise multiport at all, so there is no lane to be had in either @@ -218,6 +221,7 @@ func newLaneSet(r *handshake.Result, myLanes int, myAddr, peerAddr netip.Addr) * myCert: r.MyCert, peerCert: r.RemoteCert, initiator: r.Initiator, + epoch: epoch, }, txAddr: make([]atomic.Pointer[netip.AddrPort], txLanes), demand: make([]atomic.Bool, txLanes), diff --git a/lanes_test.go b/lanes_test.go index 2f5043ad..768aae18 100644 --- a/lanes_test.go +++ b/lanes_test.go @@ -27,12 +27,17 @@ var ( // newTestLaneSet builds a lane set from a real handshake result so the sessions // it derives hold usable keys. +// testLaneEpoch stands in for the base session's epoch. It is deliberately not a +// value sessionEpoch would hand out early in a test run, so a lane session that +// went back to drawing its own cannot match it by accident. +const testLaneEpoch = uint64(0xe90c) + func newTestLaneSet(t *testing.T, r *handshake.Result, myLanes int, peerPorts, peerBase, peerTxLanes uint32) *laneSet { t.Helper() r.PeerPortCount = peerPorts r.PeerBasePort = peerBase r.PeerTxLanes = peerTxLanes - return newLaneSet(r, myLanes, testMyAddr, testPeerAddr) + return newLaneSet(r, myLanes, testLaneEpoch, testMyAddr, testPeerAddr) } // laneSessionFor derives lane s's session and installs it, standing in for the @@ -153,10 +158,10 @@ func TestNewLaneSetSizing(t *testing.T) { initR, _ := runTestHandshake(t) // A peer with no multiport advert gets no lanes at all. - assert.Nil(t, newLaneSet(&handshake.Result{}, 4, testMyAddr, testPeerAddr)) + assert.Nil(t, newLaneSet(&handshake.Result{}, 4, testLaneEpoch, testMyAddr, testPeerAddr)) // One lane means only the base tunnel, which is not a lane set. - assert.Nil(t, newLaneSet(&handshake.Result{PeerPortCount: 4, PeerTxLanes: 1}, 1, testMyAddr, testPeerAddr)) + assert.Nil(t, newLaneSet(&handshake.Result{PeerPortCount: 4, PeerTxLanes: 1}, 1, testLaneEpoch, testMyAddr, testPeerAddr)) // Sessions cover both directions: enough for everything the peer may send, // even though we may only send on a few. @@ -262,6 +267,26 @@ func TestLaneSessionRxDerivation(t *testing.T) { "the loser's counter was not carried to the surviving window") } +// Every session on a tunnel reports the tunnel's epoch. The RX staging sort keys +// off (epoch, counter) and lanes derive lazily, so a lane drawing its own ordinal +// would be ordered by when its first packet landed: during a rehandshake cutover +// a late-derived lane of the *old* tunnel would outrank the new tunnel and sort +// its packets last, the exact inversion the epoch exists to prevent. +func TestLaneSessionsShareTheTunnelEpoch(t *testing.T) { + initR, _ := runTestHandshake(t) + ls := newTestLaneSet(t, initR, 4, 4, 4242, 4) + + for s := 1; s < 4; s++ { + // Move the global on between derivations, so a session that drew its own + // would land on a different value for every lane. + sessionEpoch.Add(1) + + cs, err := ls.session(s) + require.NoError(t, err) + assert.Equal(t, testLaneEpoch, cs.epoch, "lane %d did not inherit the tunnel's epoch", s) + } +} + func TestLaneTxGate(t *testing.T) { initR, _ := runTestHandshake(t) ls := newTestLaneSet(t, initR, 4, 4, 4242, 4) @@ -976,7 +1001,7 @@ func TestLaneFlowSymmetry(t *testing.T) { const ourBase, peerBase = 4242, 5353 ourLS := newTestLaneSet(t, initR, 4, 4, peerBase, 4) respR.PeerPortCount, respR.PeerBasePort, respR.PeerTxLanes = 4, ourBase, 4 - peerLS := newLaneSet(respR, 4, testPeerAddr, testMyAddr) + peerLS := newLaneSet(respR, 4, testLaneEpoch, testPeerAddr, testMyAddr) require.NotNil(t, peerLS) require.Equal(t, uint16(0), ourLS.laneBias, "the low address hashes straight")