multiport: validate the peer's port range, warn when a lane never comes up

- Ignore a lane advert with a zero base port or a range running past 65535,
  with a warning, instead of aiming lanes at ports that wrapped.
- Warn once when a lane that has never come up misses 3 probes in a row.
  Until now that was silent above debug, including the common case of a
  firewall that only opens the base port.

design/multiport.md

@@ -415,10 +415,13 @@ all.
 
 ### Per-tunnel: back to a single session
 
-`newLaneSet` returns `nil`, and the tunnel is an ordinary one, when:
+The tunnel gets no lane set and is an ordinary one when:
 
-- the peer advertised no port count (vanilla peer, or multiport off there), or
-- `sessions < 2`, i.e. neither side offers a lane.
+- the peer advertised no port count (vanilla peer, or multiport off there),
+- the peer's advertised range is invalid -- a zero base port, or a range running
+  past 65535. `maybeAllocLanes` logs `Ignoring multiport advert with an invalid
+  port range` rather than aiming lanes at ports that wrapped, or
+- `sessions < 2`, i.e. neither side offers a lane (`newLaneSet` returns `nil`).
 
 `txLanes` can also land at 1 -- we bound ports but the peer bound only one -- in
 which case the set exists for RX but we never send on a lane.
@@ -520,7 +523,9 @@ up never demotes them.
 Logs worth grepping: `multiport enabled` and `multiport routines` at startup, the
 `lanes` attr on handshake completion (`tx`, `sessions`, `peerBasePort`,
 `peerPorts`, `portOffset`), and `Multiport lane up` / `Multiport lane demoted`,
-both of which name the `udpAddr` involved.
+both of which name the `udpAddr` involved. A lane that has never come up logs
+`Multiport lane not coming up` at Warn, once, after 3 unanswered probes in a
+row -- usually a sign that only the base port is open.
 
 ## Known gaps
 

handshake/machine.go

@@ -345,8 +345,8 @@ func (m *Machine) processPayload(msg []byte, flags msgFlags) error {
 		// Multiport advert from the peer's side of the exchange. Out-of-range
 		// values mean a peer we can't pair lanes with; ignore the advert
 		// rather than failing the handshake — the tunnel itself is fine, it
-		// just won't get lanes. Semantic policing (port-count caps, lane
-		// clamping) belongs to the handshake manager.
+		// just won't get lanes. Semantic policing (the port range, lane
+		// clamping) belongs to the handshake manager and newLaneSet.
 		var peerLanes *LaneDetails
 		if m.result.Initiator {
 			peerLanes = payload.ResponderLanes

handshake_manager.go

@@ -732,6 +732,13 @@ func (hm *HandshakeManager) maybeAllocLanes(hostinfo *HostInfo, result *handshak
 	if len(hm.f.myVpnAddrs) == 0 || len(hostinfo.vpnAddrs) == 0 {
 		return
 	}
+	if !validLaneAdvert(result.PeerBasePort, result.PeerPortCount) {
+		// Lane targets are computed in uint16, so a range like this would wrap
+		// onto low ports. The tunnel itself is fine; it just gets no lanes.
+		hostinfo.logger(hm.l).Warn("Ignoring multiport advert with an invalid port range",
+			"peerBasePort", result.PeerBasePort, "peerPorts", result.PeerPortCount)
+		return
+	}
 
 	hostinfo.lanes = newLaneSet(result, hm.config.laneCount, hm.f.myVpnAddrs[0], hostinfo.vpnAddrs[0])
 }

lanes.go

@@ -64,6 +64,11 @@ const (
 	// laneMaxFails caps the failure counter; the backoff saturates well before.
 	laneMaxFails = 8
 
+	// laneWarnFails is how many unanswered probes a lane that has never come up
+	// takes before it is worth a warning. One or two can be loss; this many in a
+	// row is usually a firewall that only opens the base port.
+	laneWarnFails = 3
+
 	// laneProbeTimeout is how long a probe may go unacked before it counts as a
 	// failure. It is shorter than the connection manager's check interval on
 	// purpose: an outstanding probe is judged on the next tick either way, and a
@@ -170,6 +175,12 @@ type laneProbeState struct {
 	retryAt time.Time
 }
 
+// validLaneAdvert reports whether a peer's advertised port range is one we can
+// aim lanes at: a real base port, and every port in the range a valid one.
+func validLaneAdvert(basePort, portCount uint32) bool {
+	return basePort != 0 && portCount != 0 && basePort+portCount-1 <= 0xffff
+}
+
 // 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
@@ -592,6 +603,11 @@ func (f *Interface) probeLanes(hostinfo *HostInfo, now time.Time, nb, out []byte
 			if up {
 				ls.txAddr[s].Store(nil)
 				hostinfo.logger(f.l).Info("Multiport lane demoted, probe unanswered", "lane", s, "udpAddr", p.target)
+			} else if p.lastAck.IsZero() && p.fails == laneWarnFails {
+				// Said once per lane: fails only passes through this value again
+				// after an ack or a reset clears it.
+				hostinfo.logger(f.l).Warn("Multiport lane not coming up, probes unanswered; check that the peer's lane ports are reachable",
+					"lane", s, "udpAddr", p.target, "fails", p.fails)
 			}
 			continue
 		}

lanes_test.go

@@ -1,8 +1,10 @@
 package nebula
 
 import (
+	"bytes"
 	"log/slog"
 	"net/netip"
+	"strings"
 	"testing"
 	"time"
 
@@ -1107,3 +1109,67 @@ func TestHandshakeManagerVpnIpsIdentityDelete(t *testing.T) {
 	hm.DeleteHostInfo(pendingBase)
 	assert.Nil(t, hm.QueryVpnAddr(vpnIp))
 }
+
+func TestValidLaneAdvert(t *testing.T) {
+	assert.True(t, validLaneAdvert(4242, 4))
+	assert.True(t, validLaneAdvert(65535, 1))
+	assert.True(t, validLaneAdvert(65532, 4), "a range ending on 65535 is valid")
+	assert.False(t, validLaneAdvert(0, 4), "no base port")
+	assert.False(t, validLaneAdvert(4242, 0), "no ports")
+	assert.False(t, validLaneAdvert(65533, 4), "the range runs past 65535")
+}
+
+// A peer advertising a port range that would wrap gets a plain tunnel, and a
+// warning saying why, rather than lanes aimed at low ports.
+func TestMaybeAllocLanesRejectsInvalidRange(t *testing.T) {
+	hostMap := newHostMap(test.NewLogger())
+	ifce := newLaneTestInterface(hostMap)
+	ifce.myVpnAddrs = []netip.Addr{testMyAddr}
+	hm := ifce.handshakeManager
+	hm.config.laneCount = 4
+	var buf bytes.Buffer
+	hm.l = test.NewLoggerWithOutputAndLevel(&buf, slog.LevelWarn)
+
+	initR, _ := runTestHandshake(t)
+
+	hi := &HostInfo{vpnAddrs: []netip.Addr{testPeerAddr}}
+	initR.PeerBasePort, initR.PeerPortCount, initR.PeerTxLanes = 65530, 16, 4
+	hm.maybeAllocLanes(hi, initR)
+	assert.Nil(t, hi.lanes)
+	assert.Contains(t, buf.String(), "invalid port range")
+
+	hi = &HostInfo{vpnAddrs: []netip.Addr{testPeerAddr}}
+	initR.PeerBasePort = 4242
+	hm.maybeAllocLanes(hi, initR)
+	assert.NotNil(t, hi.lanes)
+}
+
+// A lane that has never come up warns once after laneWarnFails unanswered
+// probes, and not again while it keeps failing.
+func TestLaneNotComingUpWarnsOnce(t *testing.T) {
+	hostMap := newHostMap(test.NewLogger())
+	ifce := newLaneTestInterface(hostMap)
+	var buf bytes.Buffer
+	ifce.l = test.NewLoggerWithOutputAndLevel(&buf, slog.LevelWarn)
+
+	initR, _ := runTestHandshake(t)
+	ls := newTestLaneSet(t, initR, 2, 2, 4242, 2)
+	hi := newTestLaneHostInfo(t, initR, ls)
+
+	nb := make([]byte, 12)
+	out := make([]byte, mtu)
+	now := time.Now()
+	const msg = "Multiport lane not coming up"
+	for i := 1; i <= laneWarnFails+3; i++ {
+		ls.demand[1].Store(true)
+		ifce.probeLanes(hi, now, nb, out)
+		now = now.Add(laneProbeTimeout)
+		ifce.probeLanes(hi, now, nb, out)
+		now = now.Add(laneRetryMax)
+
+		if i < laneWarnFails {
+			require.NotContains(t, buf.String(), msg, "warned after %d failures", i)
+		}
+	}
+	assert.Equal(t, 1, strings.Count(buf.String(), msg))
+}