multiport: carry a lost install race's counter to the surviving lane window

Two routines can race on a lane's first packet and derive a session each. Only
one gets installed, and the loser was dropped along with the replay-window entry
for the packet it had just accepted, leaving that one counter replayable.

The two sessions hold identical keys, so the winner's window is the same window
in every respect that matters: mark the counter seen on it instead. Closes the
gap without putting a lock on the decrypt path.

connection_state.go

@@ -168,6 +168,15 @@ func (cs *ConnectionState) Decrypt(l *slog.Logger, messageCounter uint64, packet
 	return out, nil
 }
 
+// noteSeen records a counter that some other session for the same keys already
+// accepted, so a packet doesn't become replayable just because the session that
+// decrypted it was thrown away. See laneSet.installSession, its only caller.
+func (cs *ConnectionState) noteSeen(l *slog.Logger, messageCounter uint64) {
+	cs.decryptLock.Lock()
+	cs.window.Update(l, messageCounter)
+	cs.decryptLock.Unlock()
+}
+
 func (cs *ConnectionState) VerifyRelay(l *slog.Logger, messageCounter uint64, packet []byte, nb []byte) error {
 	cs.decryptLock.Lock()
 	result := cs.window.Check(l, messageCounter)

lanes.go

@@ -240,8 +240,8 @@ func lanePortOffset(myAddr, peerAddr netip.Addr, peerPortCount uint16) uint16 {
 // deliberately left out of it: anyone who can spoof this tunnel's local index
 // can name any lane, and installing on sight would let them make us hold a
 // replay window and two cipher states per lane without authenticating anything.
-// The caller installs with installSession once the packet decrypts, which is the
-// first moment the lane is known to be real.
+// The caller must install with installSession once the packet decrypts, which is
+// the first moment the lane is known to be real.
 func (i *HostInfo) laneSession(s uint8) (ci *ConnectionState, cached bool, err error) {
 	ls := i.lanes
 	if ls == nil || s == 0 || int(s) >= len(ls.sessions) {
@@ -260,15 +260,18 @@ func (i *HostInfo) laneSession(s uint8) (ci *ConnectionState, cached bool, err e
 }
 
 // installSession publishes a session derived by laneSession, so the next packet
-// on the lane doesn't have to derive it again. cs must have already decrypted a
-// packet from the peer.
+// on the lane doesn't have to derive it again. cs must have already decrypted the
+// packet at messageCounter.
 //
-// A loser of the race keeps the session it decrypted with and drops it after,
-// which loses that one packet's replay-window entry. The alternative is holding
-// a lock across a decrypt to close a window that is one packet wide and only
-// open on the first packet of a lane.
-func (ls *laneSet) installSession(s uint8, cs *ConnectionState) {
-	ls.sessions[s].CompareAndSwap(nil, cs)
+// Two routines can race on a lane's first packet and derive a session each. The
+// loser's is dropped, and with it the replay-window entry for the packet it just
+// accepted, so hand that counter to the session that survives — the keys are
+// identical, so it is the same window in every respect that matters.
+func (ls *laneSet) installSession(l *slog.Logger, s uint8, cs *ConnectionState, messageCounter uint64) {
+	if ls.sessions[s].CompareAndSwap(nil, cs) {
+		return
+	}
+	ls.sessions[s].Load().noteSeen(l, messageCounter)
 }
 
 // session returns lane s's session for our own use, deriving and installing it

lanes_test.go

@@ -206,7 +206,7 @@ func TestLaneSessionRxDerivation(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []byte("real lane traffic"), pt)
 
-	respLS.installSession(2, ci)
+	respLS.installSession(test.NewLogger(), 2, ci, 1)
 	assert.Same(t, ci, respLS.sessions[2].Load())
 
 	// Now it is a hit, and the replay window the decrypt above advanced is the
@@ -217,11 +217,16 @@ func TestLaneSessionRxDerivation(t *testing.T) {
 	assert.Same(t, ci, got)
 
 	// A racing install loses rather than swapping the session out, which would
-	// throw away the replay window the live one has been accumulating.
+	// throw away the replay window the live one has been accumulating. The loser's
+	// packet still has to be marked seen on the winner, or dropping its session
+	// would make that one counter replayable.
 	other, err := newLaneConnectionState(&respLS.material, 2)
 	require.NoError(t, err)
-	respLS.installSession(2, other)
+	require.True(t, ci.window.Check(test.NewLogger(), 7), "counter 7 seen before the race")
+	respLS.installSession(test.NewLogger(), 2, other, 7)
 	assert.Same(t, ci, respLS.sessions[2].Load())
+	assert.False(t, ci.window.Check(test.NewLogger(), 7),
+		"the loser's counter was not carried to the surviving window")
 }
 
 func TestLaneTxGate(t *testing.T) {

outside.go

@@ -177,7 +177,7 @@ func (f *Interface) readOutsidePackets(via ViaSender, packet []byte, rxc *rxCont
 	if !laneCached {
 		// The packet decrypted, so the peer really is using this lane and the
 		// session we derived for it is worth keeping.
-		hostinfo.lanes.installSession(lane, ci)
+		hostinfo.lanes.installSession(f.l, lane, ci, h.MessageCounter)
 	}
 
 	// Roam before we respond, but only on the base tunnel: a lane's source