Check staged receive replays on the listener

connection_state.go

@@ -97,21 +97,16 @@ func (cs *ConnectionState) Curve() cert.Curve {
 }
 
 func (cs *ConnectionState) Decrypt(l *slog.Logger, messageCounter uint64, packet []byte, nb []byte) ([]byte, error) {
-	if !cs.checkReplay(l, messageCounter) {
+	cs.decryptLock.Lock()
+	result := cs.window.Check(l, messageCounter)
+	cs.decryptLock.Unlock()
+	if !result {
 		return nil, ErrAlreadySeen
 	}
 	out, err := cs.decryptPacket(messageCounter, packet, nb)
 	return cs.acceptDecrypted(l, messageCounter, out, err)
 }
 
-// checkReplay reports whether the replay window would still accept messageCounter, without recording it.
-func (cs *ConnectionState) checkReplay(l *slog.Logger, messageCounter uint64) bool {
-	cs.decryptLock.Lock()
-	result := cs.window.Check(l, messageCounter)
-	cs.decryptLock.Unlock()
-	return result
-}
-
 // decryptPacket authenticates and decrypts packet in place, leaving the replay window to the caller.
 func (cs *ConnectionState) decryptPacket(messageCounter uint64, packet, nb []byte) ([]byte, error) {
 	return cs.dKey.DecryptDanger(packet[header.Len:header.Len], packet[:header.Len], packet[header.Len:], messageCounter, nb)

flow_rx.go

@@ -1,7 +1,6 @@
 package nebula
 
 import (
-	"log/slog"
 	"net/netip"
 	"sync"
 
@@ -37,17 +36,12 @@ type rxChunk struct {
 	done  chan struct{}
 }
 
-// decryptRxItems checks the data packets among items against the replay window and decrypts the ones it accepts in
+// decryptRxItems decrypts the data packets among items in place, skipping the replays checkReplays marked. Recording
-// place, as Decrypt does. Recording the counters is left to readOutsidePackets on the writer, which also handles
+// the counters is left to readOutsidePackets on the writer, which also handles the packets without a tunnel.
-// the packets without a tunnel.
+func decryptRxItems(items []rxItem, nb []byte) {
-func decryptRxItems(l *slog.Logger, items []rxItem, nb []byte) {
 	for i := range items {
 		it := &items[i]
-		if it.ci == nil {
+		if it.ci == nil || it.err != nil {
-			continue
-		}
-		if !it.ci.checkReplay(l, it.counter) {
-			it.err = ErrAlreadySeen
 			continue
 		}
 		it.out, it.err = it.ci.decryptPacket(it.counter, it.packet, nb)
@@ -135,7 +129,7 @@ func newStagedRx(f *Interface, q int, wrxc *rxContext) *stagedRx {
 func (p *stagedRx) decrypt() {
 	nb := make([]byte, 12)
 	for c := range p.jobs {
-		decryptRxItems(p.f.l, c.items, nb)
+		decryptRxItems(c.items, nb)
 		c.done <- struct{}{}
 	}
 }
@@ -205,6 +199,7 @@ func (p *stagedRx) endBatch() {
 		b.reset()
 		return
 	}
+	p.checkReplays(b.items)
 
 	if p.inFlight == 0 && n < p.f.staged.inlineBelow {
 		// The writer is idle with nothing in flight, so the listener may commit with its context and flush the tun
@@ -239,6 +234,27 @@ func (p *stagedRx) endBatch() {
 	p.free = p.free[:len(p.free)-1]
 }
 
+// checkReplays marks the replays among items before anything decrypts them, as Decrypt checks before it decrypts,
+// taking each tunnel's lock once per run of its packets rather than once per packet. The window only moves forward
+// until the writer records these counters, so a stale check can only let a replay through to acceptDecrypted, which
+// rejects it.
+func (p *stagedRx) checkReplays(items []rxItem) {
+	for i := 0; i < len(items); {
+		ci := items[i].ci
+		if ci == nil {
+			i++
+			continue
+		}
+		ci.decryptLock.Lock()
+		for ; i < len(items) && items[i].ci == ci; i++ {
+			if !ci.window.Check(p.f.l, items[i].counter) {
+				items[i].err = ErrAlreadySeen
+			}
+		}
+		ci.decryptLock.Unlock()
+	}
+}
+
 // reap takes back every batch the writer has finished with, without waiting.
 func (p *stagedRx) reap() {
 	for {
@@ -260,7 +276,7 @@ func (p *stagedRx) release(b *rxBatch) {
 // order, and flush the tun batcher.
 func (p *stagedRx) process(b *rxBatch, rxc *rxContext) {
 	if b.nChunks == 0 {
-		decryptRxItems(p.f.l, b.items, rxc.nb)
+		decryptRxItems(b.items, rxc.nb)
 		p.commit(b.items, rxc)
 	} else {
 		// Commit chunk by chunk, so the commit overlaps the workers still on later chunks