diff --git a/connection_state.go b/connection_state.go index 8f5ef52a..afadde87 100644 --- a/connection_state.go +++ b/connection_state.go @@ -77,8 +77,8 @@ func newConnectionStateFromResult(r *handshake.Result) (*ConnectionState, error) return ci, nil } -// newLaneConnectionState derives multiport lane s's session from the completed -// base handshake. Each key is an HKDF expansion of the base tunnel's matching +// newLaneConnectionState derives multiport lane s's session from the base +// tunnel's material. Each key is an HKDF expansion of the base tunnel's matching // key, labelled with the lane index, so the pair stays matched with no extra // negotiation: Noise leaves our send key equal to the peer's receive key, and // expanding both with the same label preserves that. @@ -86,26 +86,26 @@ 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. -func newLaneConnectionState(r *handshake.Result, lane uint8) (*ConnectionState, error) { +func newLaneConnectionState(m *laneMaterial, lane uint8) (*ConnectionState, error) { if lane == 0 { return nil, fmt.Errorf("lane 0 is the base session") } - eKey, err := deriveLaneKey(r.EKey.UnsafeKey(), lane) + eKey, err := deriveLaneKey(m.eKey, lane) if err != nil { return nil, err } - dKey, err := deriveLaneKey(r.DKey.UnsafeKey(), lane) + dKey, err := deriveLaneKey(m.dKey, lane) if err != nil { return nil, err } return &ConnectionState{ - myCert: r.MyCert, - initiator: r.Initiator, - peerCert: r.RemoteCert, - eKey: noiseutil.NewCipherStateFromKey(eKey, r.Cipher), - dKey: noiseutil.NewCipherStateFromKey(dKey, r.Cipher), + myCert: m.myCert, + initiator: m.initiator, + peerCert: m.peerCert, + eKey: noiseutil.NewCipherStateFromKey(eKey, m.cipher), + dKey: noiseutil.NewCipherStateFromKey(dKey, m.cipher), window: NewBits(ReplayWindow), epoch: sessionEpoch.Add(1), }, nil diff --git a/control.go b/control.go index 9928657a..2df54319 100644 --- a/control.go +++ b/control.go @@ -434,7 +434,7 @@ func copyLanes(h *HostInfo) []ControlLane { l.Up = true l.Remote = *addr } - if cs := ls.sessions[s]; cs != nil { + if cs := ls.sessions[s].Load(); cs != nil { l.MessageCounter = cs.messageCounter.Load() } lanes = append(lanes, l) diff --git a/handshake_manager.go b/handshake_manager.go index 4f65eeb6..9fe181e7 100644 --- a/handshake_manager.go +++ b/handshake_manager.go @@ -721,12 +721,10 @@ func (hm *HandshakeManager) laneAdvert() *handshake.LaneDetails { } } -// maybeAllocLanes derives the multiport lane sessions for a just-completed -// tunnel. Must run before the hostinfo becomes visible in the hostmap: the data -// plane reads hostinfo.lanes with no synchronization at all. -// -// A failure here costs the tunnel its lanes, not the tunnel: the base session is -// already usable and every lane the data plane wants falls back to it. +// maybeAllocLanes sets up the multiport lanes for a just-completed tunnel. Must +// run before the hostinfo becomes visible in the hostmap: the data plane reads +// hostinfo.lanes without synchronizing on it. The sessions themselves are derived +// later, on the first packet that needs each one. func (hm *HandshakeManager) maybeAllocLanes(hostinfo *HostInfo, result *handshake.Result) { if hm.config.laneCount == 0 || result.PeerPortCount == 0 { return @@ -735,12 +733,7 @@ func (hm *HandshakeManager) maybeAllocLanes(hostinfo *HostInfo, result *handshak return } - ls, err := newLaneSet(result, hm.config.laneCount, hm.f.myVpnAddrs[0], hostinfo.vpnAddrs[0]) - if err != nil { - hostinfo.logger(hm.l).Error("Failed to derive multiport lanes", "error", err) - return - } - hostinfo.lanes = ls + hostinfo.lanes = newLaneSet(result, hm.config.laneCount, 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 a4ce547f..e86288f7 100644 --- a/lanes.go +++ b/lanes.go @@ -9,6 +9,8 @@ import ( "sync/atomic" "time" + "github.com/flynn/noise" + "github.com/slackhq/nebula/cert" "github.com/slackhq/nebula/handshake" "github.com/slackhq/nebula/header" "github.com/slackhq/nebula/noiseutil" @@ -28,11 +30,18 @@ import ( // Lane 0 is the base tunnel itself: HostInfo.ConnectionState, socket 0, and the // peer's real remote address. Lane s > 0 egresses writers[s] (bound to // listen.port+s) toward the peer's advertised port range. Receiving on a lane -// needs no permission — the session exists as soon as the base handshake +// needs no permission — the keys are derivable the moment the base handshake // completes — but sending on one needs proof the new 5-tuple actually works, // since nothing else would notice a middlebox quietly dropping it. So a lane // stays down until a probe on it is acked, and falls back to the base tunnel the // moment it stops being acked. +// +// Both directions are pay-per-use. A lane session is derived on the first packet +// that needs it, because how many lanes exist is partly the peer's call: it +// advertises how many it sends on, and we have to be able to receive all of +// them. Deriving them all up front would let a peer advertising the maximum cost +// us a replay window and two cipher states per lane, per tunnel, for lanes it +// may never send on. const ( // laneKeyInfo is the HKDF label prefix for lane key expansion. Changing it @@ -65,28 +74,35 @@ const ( // laneSet holds a peer's lane sessions and the state deciding which lanes may // carry traffic. It is built when the base handshake completes and never // resized, so the slices and their lengths are immutable; mu guards the fields -// under it, and txAddr/demand are atomics read by the data plane without it. +// under it, and sessions/txAddr/demand are atomics read by the data plane +// without it. type laneSet struct { - // sessions[s] is lane s's session. sessions[0] is nil: lane 0 is the base - // tunnel's own ConnectionState. Immutable, so the RX path reads it with no - // synchronization at all. - sessions []*ConnectionState + // sessions[s] holds lane s's session once something has needed it, and nil + // until then. sessions[0] is never populated: lane 0 is the base tunnel's own + // ConnectionState. The length is immutable, so the data plane bounds-checks + // and loads with no locking. + sessions []atomic.Pointer[ConnectionState] + + // material is what a lane session is derived from, kept because sessions are + // derived lazily and the handshake result is long gone by then. + material laneMaterial // txAddr[s] holds lane s's remote address while the lane is proven usable // and nil otherwise. This single atomic is both the TX gate and the // destination, so a routine that loads non-nil has everything it needs. + // Sized txLanes: lanes above that never send. txAddr []atomic.Pointer[netip.AddrPort] // demand[s] is raised by the TX path when a routine riding lane s has // traffic for this peer and the lane is down. Probing is demand-driven: a // peer we exchange a trickle with never costs more than its base tunnel, no - // matter how many lanes are configured. + // matter how many lanes are configured. Sized txLanes. demand []atomic.Bool // txLanes is how many lanes we may send on — our lane count clamped to the - // ports the peer bound. Lanes from txLanes up exist only to receive, which - // is how a peer with more routines than us still spreads its own traffic. - // Immutable. + // ports the peer bound. Lanes from txLanes up can only receive, which is how + // a peer with more routines than us still spreads its own traffic. + // Immutable, and the length of every TX-side slice here. txLanes int mu sync.Mutex @@ -103,10 +119,22 @@ type laneSet struct { // a roam invalidates every lane rather than moving it. peerAddr netip.Addr - // probe[s] is lane s's probe and backoff state. + // probe[s] is lane s's probe and backoff state. Sized txLanes. probe []laneProbeState } +// laneMaterial is everything a lane session is derived from. The two base keys +// are the same secret the base tunnel's own cipher states already hold — the +// noiseutil.CipherState interface just doesn't hand them back, so a lane set +// keeps its own copy rather than a reference to the session. +type laneMaterial struct { + eKey, dKey [32]byte + cipher noise.CipherFunc + myCert cert.Certificate + peerCert *cert.CachedCertificate + initiator bool +} + type laneProbeState struct { // gen is the generation of the last probe sent, echoed in the ack so a late // ack cannot promote a lane on the strength of a superseded probe. @@ -128,46 +156,48 @@ type laneProbeState struct { retryAt time.Time } -// newLaneSet derives the lane sessions for a freshly completed base handshake. -// It returns nil when the pair has no lane beyond the base tunnel, which is the -// normal answer for a peer running without multiport. -func newLaneSet(r *handshake.Result, myLanes int, myAddr, peerAddr netip.Addr) (*laneSet, error) { +// newLaneSet sets up the lanes for a freshly completed base handshake. It +// 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 { // 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 // direction. peerPorts := uint16(r.PeerPortCount) if peerPorts == 0 { - return nil, nil + return nil } // Sessions have to cover both directions: we send on our lanes and receive // on the peer's, and one derived session serves both ends of a lane index. + // Only the session table is sized by the peer's advertised count; the TX-side + // state is sized by what we will actually send on. n := min(max(myLanes, int(r.PeerTxLanes)), header.MaxLane+1) if n < 2 { - return nil, nil + return nil } + txLanes := min(myLanes, int(peerPorts), n) - ls := &laneSet{ - sessions: make([]*ConnectionState, n), - txAddr: make([]atomic.Pointer[netip.AddrPort], n), - demand: make([]atomic.Bool, n), - probe: make([]laneProbeState, n), - txLanes: min(myLanes, int(peerPorts), n), + return &laneSet{ + sessions: make([]atomic.Pointer[ConnectionState], n), + material: laneMaterial{ + eKey: r.EKey.UnsafeKey(), + dKey: r.DKey.UnsafeKey(), + cipher: r.Cipher, + myCert: r.MyCert, + peerCert: r.RemoteCert, + initiator: r.Initiator, + }, + txAddr: make([]atomic.Pointer[netip.AddrPort], txLanes), + demand: make([]atomic.Bool, txLanes), + probe: make([]laneProbeState, txLanes), + txLanes: txLanes, peerPortCount: peerPorts, peerBasePort: uint16(r.PeerBasePort), portOffset: lanePortOffset(myAddr, peerAddr, peerPorts), } - - for s := 1; s < n; s++ { - cs, err := newLaneConnectionState(r, uint8(s)) - if err != nil { - return nil, err - } - ls.sessions[s] = cs - } - - return ls, nil } // lanePortOffset returns the rotation applied to this pair's lane target ports, @@ -202,13 +232,63 @@ func lanePortOffset(myAddr, peerAddr netip.Addr, peerPortCount uint16) uint16 { return o } -// laneSession returns lane s's session, or nil if this peer has no lane s. -func (i *HostInfo) laneSession(s uint8) *ConnectionState { +// laneSession returns the session to decrypt a lane s packet with, deriving one +// if this is the first packet to claim that lane. A nil session with no error +// means this tunnel has no lane s at all. +// +// cached reports whether the session was already in the table. A fresh one is +// 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. +func (i *HostInfo) laneSession(s uint8) (ci *ConnectionState, cached bool, err error) { ls := i.lanes - if ls == nil || int(s) >= len(ls.sessions) { - return nil + if ls == nil || s == 0 || int(s) >= len(ls.sessions) { + return nil, false, nil } - return ls.sessions[s] + + if cs := ls.sessions[s].Load(); cs != nil { + return cs, true, nil + } + + cs, err := newLaneConnectionState(&ls.material, s) + if err != nil { + return nil, false, err + } + return cs, false, nil +} + +// 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. +// +// 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) +} + +// session returns lane s's session for our own use, deriving and installing it +// if it doesn't exist yet. Unlike the RX path this needs no proof the lane is +// real: we only ask for lanes we chose to send on. s must be a lane this set +// covers. +func (ls *laneSet) session(s int) (*ConnectionState, error) { + if cs := ls.sessions[s].Load(); cs != nil { + return cs, nil + } + + cs, err := newLaneConnectionState(&ls.material, uint8(s)) + if err != nil { + return nil, err + } + + if !ls.sessions[s].CompareAndSwap(nil, cs) { + return ls.sessions[s].Load(), nil + } + return cs, nil } // maxMessageCounter returns the highest counter across the base session and @@ -222,8 +302,10 @@ func (i *HostInfo) maxMessageCounter() uint64 { } c := i.ConnectionState.messageCounter.Load() if ls := i.lanes; ls != nil { - for _, cs := range ls.sessions { + for s := range ls.sessions { + cs := ls.sessions[s].Load() if cs == nil { + // Never derived, so it has never sent anything either. continue } if lc := cs.messageCounter.Load(); lc > c { @@ -244,7 +326,13 @@ func (ls *laneSet) txLane(s int) (*ConnectionState, netip.AddrPort) { } if addr := ls.txAddr[s].Load(); addr != nil { - return ls.sessions[s], *addr + // The lane is only up because a probe was acked on it, and that probe + // derived the session, so this load cannot miss. Fall back rather than + // derive here anyway: this is the hot path and a nil is not worth an HKDF. + if cs := ls.sessions[s].Load(); cs != nil { + return cs, *addr + } + return nil, netip.AddrPort{} } // Load-guarded so the common case of a lane that will not come up is a @@ -395,7 +483,12 @@ func (ls *laneSet) resetLocked() { // keys we derived for this lane match the ones it derived. Reports whether the // probe made it onto the wire. func (f *Interface) sendLaneProbe(hostinfo *HostInfo, s int, gen uint8, addr netip.AddrPort, nb, out []byte) bool { - ci := hostinfo.lanes.sessions[s] + // The first probe on a lane is what derives its session. + ci, err := hostinfo.lanes.session(s) + if err != nil { + hostinfo.logger(f.l).Error("Failed to derive multiport lane session", "error", err, "lane", s) + return false + } if ci == nil || ci.eKey == nil { return false } @@ -413,7 +506,7 @@ func (f *Interface) sendLaneProbe(hostinfo *HostInfo, s int, gen uint8, addr net } b := header.EncodeLane(out[:0], header.Version, header.Test, header.LaneProbe, hostinfo.remoteIndexId, c, uint8(s)) - b, err := ci.eKey.EncryptDanger(b, b, []byte{uint8(s), gen}, c, nb) + b, err = ci.eKey.EncryptDanger(b, b, []byte{uint8(s), gen}, c, nb) if noiseutil.EncryptLockNeeded { ci.writeLock.Unlock() } diff --git a/lanes_test.go b/lanes_test.go index 0f869ae9..2947cc9e 100644 --- a/lanes_test.go +++ b/lanes_test.go @@ -24,16 +24,24 @@ var ( testPeerAddr = netip.MustParseAddr("10.0.0.2") ) -// newTestLaneSet derives a lane set from a real handshake result so the sessions -// hold usable keys. +// newTestLaneSet builds a lane set from a real handshake result so the sessions +// it derives hold usable keys. 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 - ls, err := newLaneSet(r, myLanes, testMyAddr, testPeerAddr) + return newLaneSet(r, myLanes, testMyAddr, testPeerAddr) +} + +// laneSessionFor derives lane s's session and installs it, standing in for the +// data-plane call that would normally be the first to need it. +func laneSessionFor(t *testing.T, ls *laneSet, s int) *ConnectionState { + t.Helper() + cs, err := ls.session(s) require.NoError(t, err) - return ls + require.NotNil(t, cs) + return cs } func newTestLaneHostInfo(t *testing.T, r *handshake.Result, ls *laneSet) *HostInfo { @@ -118,15 +126,15 @@ func TestLaneKeyDerivationSymmetry(t *testing.T) { initLS := newTestLaneSet(t, initR, 4, 4, 4242, 4) respLS := newTestLaneSet(t, respR, 4, 4, 4242, 4) require.Len(t, initLS.sessions, 4) - assert.Nil(t, initLS.sessions[0], "lane 0 is the base session, not a derived one") + assert.Nil(t, initLS.sessions[0].Load(), "lane 0 is the base session, not a derived one") nb := make([]byte, 12) for s := 1; s < 4; s++ { out := header.EncodeLane(make([]byte, 0, mtu), header.Version, header.Message, 0, 200, 1, uint8(s)) - ct, err := initLS.sessions[s].eKey.EncryptDanger(out, out, []byte("lane payload"), 1, nb) + ct, err := laneSessionFor(t, initLS, s).eKey.EncryptDanger(out, out, []byte("lane payload"), 1, nb) require.NoError(t, err) - pt, err := respLS.sessions[s].Decrypt(test.NewLogger(), 1, ct, nb) + pt, err := laneSessionFor(t, respLS, s).Decrypt(test.NewLogger(), 1, ct, nb) require.NoError(t, err, "lane %d keys did not match", s) assert.Equal(t, []byte("lane payload"), pt) } @@ -134,9 +142,9 @@ func TestLaneKeyDerivationSymmetry(t *testing.T) { // Distinct lanes get distinct keys: lane 2's session must not open lane 1's // ciphertext, or the header's lane index would be forgeable in effect. out := header.EncodeLane(make([]byte, 0, mtu), header.Version, header.Message, 0, 200, 7, 1) - ct, err := initLS.sessions[1].eKey.EncryptDanger(out, out, []byte("lane payload"), 7, nb) + ct, err := laneSessionFor(t, initLS, 1).eKey.EncryptDanger(out, out, []byte("lane payload"), 7, nb) require.NoError(t, err) - _, err = respLS.sessions[2].Decrypt(test.NewLogger(), 7, ct, nb) + _, err = laneSessionFor(t, respLS, 2).Decrypt(test.NewLogger(), 7, ct, nb) assert.Error(t, err) } @@ -144,21 +152,29 @@ func TestNewLaneSetSizing(t *testing.T) { initR, _ := runTestHandshake(t) // A peer with no multiport advert gets no lanes at all. - ls, err := newLaneSet(&handshake.Result{}, 4, testMyAddr, testPeerAddr) - require.NoError(t, err) - assert.Nil(t, ls) + assert.Nil(t, newLaneSet(&handshake.Result{}, 4, testMyAddr, testPeerAddr)) // One lane means only the base tunnel, which is not a lane set. - ls, err = newLaneSet(&handshake.Result{PeerPortCount: 4, PeerTxLanes: 1}, 1, testMyAddr, testPeerAddr) - require.NoError(t, err) - assert.Nil(t, ls) + assert.Nil(t, newLaneSet(&handshake.Result{PeerPortCount: 4, PeerTxLanes: 1}, 1, testMyAddr, testPeerAddr)) // Sessions cover both directions: enough for everything the peer may send, // even though we may only send on a few. - ls = newTestLaneSet(t, initR, 2, 8, 4242, 6) + ls := newTestLaneSet(t, initR, 2, 8, 4242, 6) assert.Len(t, ls.sessions, 6, "sessions must cover the peer's tx lanes") assert.Equal(t, 2, ls.txLanes, "we may only send on our own lanes") + // The peer's advert sizes the session table but nothing else. Its lane count + // is its own choice, so it must not be able to make us allocate per-lane tx + // state we will never use. + assert.Len(t, ls.txAddr, 2) + assert.Len(t, ls.demand, 2) + assert.Len(t, ls.probe, 2) + + // Nothing is derived up front, for the same reason. + for s := range ls.sessions { + assert.Nil(t, ls.sessions[s].Load(), "lane %d derived before anything needed it", s) + } + // Our tx lanes are clamped to the ports the peer actually bound: a lane // aimed past the peer's range would land on some unrelated socket. ls = newTestLaneSet(t, initR, 8, 3, 4242, 8) @@ -166,6 +182,48 @@ func TestNewLaneSetSizing(t *testing.T) { assert.Len(t, ls.sessions, 8) } +// The RX path must not cache a session for a lane until a packet on it has +// actually decrypted, or a spoofer naming lanes at random could make us hold a +// replay window and two cipher states per lane without authenticating anything. +func TestLaneSessionRxDerivation(t *testing.T) { + initR, respR := runTestHandshake(t) + respLS := newTestLaneSet(t, respR, 4, 4, 4242, 4) + hi := newTestLaneHostInfo(t, respR, respLS) + + ci, cached, err := hi.laneSession(2) + require.NoError(t, err) + require.NotNil(t, ci) + assert.False(t, cached, "the first packet on a lane derives, it does not hit") + assert.Nil(t, respLS.sessions[2].Load(), "an unauthenticated packet must not install a session") + + // The lane the peer really is using decrypts, and that is what installs it. + nb := make([]byte, 12) + out := header.EncodeLane(make([]byte, 0, mtu), header.Version, header.Message, 0, 200, 1, 2) + ct, err := laneSessionFor(t, newTestLaneSet(t, initR, 4, 4, 4242, 4), 2). + eKey.EncryptDanger(out, out, []byte("real lane traffic"), 1, nb) + require.NoError(t, err) + pt, err := ci.Decrypt(test.NewLogger(), 1, ct, nb) + require.NoError(t, err) + assert.Equal(t, []byte("real lane traffic"), pt) + + respLS.installSession(2, ci) + assert.Same(t, ci, respLS.sessions[2].Load()) + + // Now it is a hit, and the replay window the decrypt above advanced is the + // one the next packet sees. + got, cached, err := hi.laneSession(2) + require.NoError(t, err) + assert.True(t, cached) + 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. + other, err := newLaneConnectionState(&respLS.material, 2) + require.NoError(t, err) + respLS.installSession(2, other) + assert.Same(t, ci, respLS.sessions[2].Load()) +} + func TestLaneTxGate(t *testing.T) { initR, _ := runTestHandshake(t) ls := newTestLaneSet(t, initR, 4, 4, 4242, 4) @@ -178,12 +236,19 @@ func TestLaneTxGate(t *testing.T) { assert.True(t, ls.demand[1].Load()) assert.False(t, ls.demand[2].Load(), "demand raised on an untouched lane") - // Promotion publishes the session and destination together. + // An up lane with no session derived yet cannot happen — the probe that + // promoted it derived one — but it must fall back rather than send in the + // clear if it ever does. want := netip.MustParseAddrPort("192.0.2.1:4243") ls.txAddr[1].Store(&want) + ci, _ = ls.txLane(1) + assert.Nil(t, ci) + + // Promotion publishes the session and destination together. + sess := laneSessionFor(t, ls, 1) ls.demand[1].Store(false) ci, addr = ls.txLane(1) - assert.Equal(t, ls.sessions[1], ci) + assert.Same(t, sess, ci) assert.Equal(t, want, addr) assert.False(t, ls.demand[1].Load(), "a hit must not raise demand") @@ -199,14 +264,30 @@ func TestLaneSessionLookup(t *testing.T) { ls := newTestLaneSet(t, initR, 3, 4, 4242, 3) hi := newTestLaneHostInfo(t, initR, ls) - assert.Nil(t, hi.laneSession(0), "lane 0 is the base session") - assert.Equal(t, ls.sessions[2], hi.laneSession(2)) - assert.Nil(t, hi.laneSession(3), "a lane we never derived") - assert.Nil(t, hi.laneSession(255)) + assertNoLane := func(s uint8, msg string) { + t.Helper() + ci, cached, err := hi.laneSession(s) + require.NoError(t, err) + assert.Nil(t, ci, msg) + assert.False(t, cached, msg) + } + + assertNoLane(0, "lane 0 is the base session") + assertNoLane(3, "a lane beyond what this tunnel covers") + assertNoLane(255, "a lane beyond what this tunnel covers") + + // An already-derived lane is returned as a hit. + sess := laneSessionFor(t, ls, 2) + ci, cached, err := hi.laneSession(2) + require.NoError(t, err) + assert.Same(t, sess, ci) + assert.True(t, cached) // A peer without lanes answers nil for every lane rather than panicking. bare := &HostInfo{} - assert.Nil(t, bare.laneSession(1)) + ci, _, err = bare.laneSession(1) + require.NoError(t, err) + assert.Nil(t, ci) } func TestMaxMessageCounter(t *testing.T) { @@ -219,7 +300,7 @@ func TestMaxMessageCounter(t *testing.T) { // A lane past the base is what the rehandshake threshold has to notice: the // base counter would sit still while the lane burns through its nonces. - ls.sessions[2].messageCounter.Store(9000) + laneSessionFor(t, ls, 2).messageCounter.Store(9000) assert.Equal(t, uint64(9000), hi.maxMessageCounter()) assert.Equal(t, uint64(0), (&HostInfo{}).maxMessageCounter()) @@ -418,7 +499,7 @@ func TestHandleLaneProbe(t *testing.T) { assert.Equal(t, header.LaneProbeAck, h.Subtype) assert.Equal(t, uint8(0), h.Lane(), "the ack is base-tunnel traffic") - pt, err := newTestLaneSet(t, initR, 4, 4, 4242, 4).sessions[1].dKey.DecryptDanger( + pt, err := laneSessionFor(t, newTestLaneSet(t, initR, 4, 4, 4242, 4), 1).dKey.DecryptDanger( nil, sent.bufs[0][:header.Len], sent.bufs[0][header.Len:], h.MessageCounter, make([]byte, 12)) _ = pt assert.Error(t, err, "the ack must not be readable with a lane key") @@ -522,8 +603,11 @@ func TestSendInsideMessageLaneSwap(t *testing.T) { assert.Equal(t, uint8(0), h.Lane()) // Once the lane is up, slot-1 traffic rides the lane session, the lane - // socket and the lane's destination, tagged with the lane index. + // socket and the lane's destination, tagged with the lane index. Promotion + // normally happens on the ack of a probe, which is also what derived the + // session, so stand both up here. laneRemote := netip.MustParseAddrPort("192.0.2.1:5354") + laneSessionFor(t, ls, 1) ls.txAddr[1].Store(&laneRemote) ls.demand[1].Store(false) ifce.sendInsideMessage(hi, pkt, nb, tx1) @@ -537,7 +621,7 @@ func TestSendInsideMessageLaneSwap(t *testing.T) { assert.Equal(t, hi.remoteIndexId, h.RemoteIndex) // And the peer's derived lane-1 session is what opens it. - pt, err := peerLS.sessions[1].Decrypt(test.NewLogger(), h.MessageCounter, laneWriter.bufs[0], nb) + pt, err := laneSessionFor(t, peerLS, 1).Decrypt(test.NewLogger(), h.MessageCounter, laneWriter.bufs[0], nb) require.NoError(t, err) assert.Equal(t, pkt.Bytes, pt) diff --git a/outside.go b/outside.go index 1a0e8053..5abef4eb 100644 --- a/outside.go +++ b/outside.go @@ -111,10 +111,28 @@ func (f *Interface) readOutsidePackets(via ViaSender, packet []byte, rxc *rxCont // is the base tunnel; a higher lane is one of the sessions derived from it. ci := hostinfo.ConnectionState lane := h.Lane() + laneCached := true if lane != 0 { - ci = hostinfo.laneSession(lane) + if isMessageRelay { + // A relay carrier is always the base tunnel, so lane ciphertext can + // never legitimately arrive wrapped in one. Checked before the lookup + // below so a junk relay packet can't make us derive a session. + f.messageMetrics.RxInvalid(1) + if f.l.Enabled(context.Background(), slog.LevelDebug) { + hostinfo.logger(f.l).Debug("Refusing relayed multiport lane packet", "from", via, "header", h) + } + return + } + + var err error + ci, laneCached, err = hostinfo.laneSession(lane) + if err != nil { + f.messageMetrics.RxInvalid(1) + hostinfo.logger(f.l).Error("Failed to derive multiport lane session", "error", err, "lane", lane) + return + } if ci == nil { - // A lane we have no session for: a stale lane from a tunnel that has + // A lane this tunnel doesn't have: a stale lane from a tunnel that has // since rolled, or a peer sending above what it advertised. Dropping // silently is right for both — a recv_error would tear down a // perfectly good base tunnel on the strength of one odd packet. @@ -124,15 +142,6 @@ func (f *Interface) readOutsidePackets(via ViaSender, packet []byte, rxc *rxCont } return } - if isMessageRelay { - // A relay carrier is always the base tunnel, so lane ciphertext can - // never legitimately arrive wrapped in one. - f.messageMetrics.RxInvalid(1) - if f.l.Enabled(context.Background(), slog.LevelDebug) { - hostinfo.logger(f.l).Debug("Refusing relayed multiport lane packet", "from", via, "header", h) - } - return - } } if len(packet) < header.Len+ci.dKey.Overhead() { @@ -165,6 +174,12 @@ func (f *Interface) readOutsidePackets(via ViaSender, packet []byte, rxc *rxCont return } + 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) + } + // Roam before we respond, but only on the base tunnel: a lane's source // address is a per-lane 4-tuple, not the tunnel's remote, and letting it // roam the hostinfo would point every non-lane packet at a lane port.