Drop a packet claiming an overlay address through a peer's unsafe network

The firewall accepted any inbound source inside the sender's unsafe
networks. One that overlapped the overlay let the peer send as any node
in it, and whatever trusted the source address, the firewall's own rules
and anything on the host reading it, was told the wrong node.

- a source inside this node's own networks is a node's address, which
  only that node sends from; through an unsafe network it is refused,
  counted as a dropped remote address
- the peer's own address and sources outside the overlay still pass
  through its unsafe network, and outbound, where the address is a
  destination, nothing changes

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

CHANGELOG.md

@@ -20,6 +20,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- A peer whose certificate carries an unsafe network overlapping the overlay could send packets as any node in
+  it: the firewall accepted any source inside the unsafe network. A source address inside this node's own
+  networks must now be the sending peer's own; through an unsafe network it is dropped, counted as a dropped
+  remote address.
+
 - Signing a v2 certificate with an empty name, a name longer than 253 bytes, or an empty group now fails.
   These certificates were signed successfully but could not be loaded.
 - `create-tunnel -address <ip:port>` panicked on a nil RemoteList and, over the ssh debug server, took nebula

firewall.go

@@ -60,6 +60,9 @@ type Firewall struct {
 
 	// assignedNetworks is a list of vpn networks assigned to us in the certificate.
 	assignedNetworks []netip.Prefix
+	// vpnNetworks holds assignedNetworks for lookup: a source address inside one of them
+	// belongs to a node of the overlay, and only that node may send as it.
+	vpnNetworks *bart.Lite
 	// unsafeNetworks is the list of unsafe networks issued to us in the certificate
 	unsafeNetworks []netip.Prefix
 
@@ -156,10 +159,12 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur
 	}
 
 	routableNetworks := new(bart.Lite)
+	vpnNetworks := new(bart.Lite)
 	var assignedNetworks []netip.Prefix
 	for _, network := range c.Networks() {
 		nprefix := netip.PrefixFrom(network.Addr(), network.Addr().BitLen())
 		routableNetworks.Insert(nprefix)
+		vpnNetworks.Insert(network)
 		assignedNetworks = append(assignedNetworks, network)
 	}
 
@@ -180,6 +185,7 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur
 		DefaultTimeout:   defaultTimeout,
 		routableNetworks: routableNetworks,
 		assignedNetworks: assignedNetworks,
+		vpnNetworks:      vpnNetworks,
 		unsafeNetworks:   unsafeNetworks,
 		l:                l,
 
@@ -462,7 +468,17 @@ func (f *Firewall) Drop(fp firewall.Packet, incoming bool, h *HostInfo, caPool *
 			f.metrics(incoming).droppedRemoteAddr.Inc(1)
 			return ErrPeerRejected // reject for now, one day this may have different FW rules
 		case NetworkTypeUnsafe:
-			break // nothing special, one day this may have different FW rules
+			// An unsafe network says what a peer routes for, never who it is. One that
+			// overlaps our own networks would let the peer send as any node of the overlay,
+			// and whatever trusts the source address, the firewall's own rules and the
+			// identity a program looks up for it, would be told the wrong node. A source
+			// inside our networks is a node's own address, which only that node sends
+			// from, as NetworkTypeVPN, matched first; through an unsafe network it is
+			// refused. Outbound the address is a destination and claims nothing.
+			if incoming && f.vpnNetworks.Contains(fp.RemoteAddr) {
+				f.metrics(incoming).droppedRemoteAddr.Inc(1)
+				return ErrInvalidRemoteIP
+			}
 		default:
 			f.metrics(incoming).droppedRemoteAddr.Inc(1)
 			return ErrUnknownNetworkType //should never happen

firewall_test.go

@@ -277,6 +277,58 @@ func TestFirewall_Drop(t *testing.T) {
 	require.NoError(t, fw.Drop(p, true, &h, cp, nil))
 }
 
+// TestFirewall_DropOverlaySourceThroughUnsafeNetwork: a peer whose unsafe network
+// overlaps our overlay may not send as another node of it. Its own address and sources
+// outside the overlay still pass through the unsafe network, and outbound to such an
+// address, where the address is a destination, is not the firewall's to refuse here.
+func TestFirewall_DropOverlaySourceThroughUnsafeNetwork(t *testing.T) {
+	l := test.NewLogger()
+	myVpnNetworksTable := new(bart.Lite)
+	myVpnNetworksTable.Insert(netip.MustParsePrefix("1.2.3.0/24"))
+
+	me := dummyCert{name: "me", networks: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/24")}}
+	// A router for 1.2.0.0/16, which contains the whole overlay.
+	peer := dummyCert{
+		name:           "router",
+		networks:       []netip.Prefix{netip.MustParsePrefix("1.2.3.9/24")},
+		unsafeNetworks: []netip.Prefix{netip.MustParsePrefix("1.2.0.0/16")},
+		groups:         []string{"default-group"},
+	}
+	h := HostInfo{
+		ConnectionState: &ConnectionState{
+			peerCert: &cert.CachedCertificate{Certificate: &peer, InvertedGroups: map[string]struct{}{"default-group": {}}},
+		},
+		vpnAddrs: []netip.Addr{netip.MustParseAddr("1.2.3.9")},
+	}
+	h.buildNetworks(myVpnNetworksTable, &peer)
+
+	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &me)
+	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	cp := cert.NewCAPool()
+	packet := func(remote string) firewall.Packet {
+		return firewall.Packet{
+			LocalAddr:  netip.MustParseAddr("1.2.3.4"),
+			RemoteAddr: netip.MustParseAddr(remote),
+			LocalPort:  10,
+			RemotePort: 90,
+			Protocol:   iputil.IPProtocolUDP,
+		}
+	}
+
+	// Another node's overlay address, through the unsafe network: refused.
+	assert.Equal(t, ErrInvalidRemoteIP, fw.Drop(packet("1.2.3.10"), true, &h, cp, nil))
+	assert.Equal(t, int64(1), fw.incomingMetrics.droppedRemoteAddr.Count())
+
+	// The peer's own address, and an address outside the overlay it routes for: served.
+	require.NoError(t, fw.Drop(packet("1.2.3.9"), true, &h, cp, nil))
+	require.NoError(t, fw.Drop(packet("1.2.5.5"), true, &h, cp, nil))
+
+	// Outbound to the overlay address via the router is not refused here; it meets the
+	// outbound rules as before.
+	resetConntrack(fw)
+	assert.Equal(t, ErrNoMatchingRule, fw.Drop(packet("1.2.3.10"), false, &h, cp, nil))
+}
+
 func TestFirewall_DropV6(t *testing.T) {
 	ob := &bytes.Buffer{}
 	l := test.NewLoggerWithOutput(ob)