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, the firewall's own rules and the
identity a program looks up for 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
- identity's trust statement loses its overlap caveat; the example
  config says what an answer proves and the one limit left, that the
  connection must have arrived on the nebula interface

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

CHANGELOG.md

@@ -62,6 +62,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

examples/config.yml

@@ -265,6 +265,9 @@ punchy:
   #   GET /v1/whoami                              this node's identity
   #   GET /v1                                     describes what is served
   #
+  # NB: unless a connection arrived on the nebula interface, there's no guarantee
+  # that the identity returned belongs to the sender.
+  #
   #info:
     #enabled: false
     # The absolute path to listen on, or a full pipe name on Windows.

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
 
@@ -153,10 +156,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)
 	}
 
@@ -177,6 +182,7 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur
 		DefaultTimeout:   defaultTimeout,
 		routableNetworks: routableNetworks,
 		assignedNetworks: assignedNetworks,
+		vpnNetworks:      vpnNetworks,
 		unsafeNetworks:   unsafeNetworks,
 		l:                l,
 
@@ -444,7 +450,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

@@ -252,6 +252,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)

identity.go

@@ -34,17 +34,15 @@ func (f *identityFlags) setAddress(v string) error {
 // hostIdentity is what identity reports: the identity a certificate asserts for a host,
 // and nothing else. Every field is read from a certificate nebula verified when the
 // tunnel came up, or from our own, which is what lets a program on this host make an
-// authorization decision from it, within two limits. Nebula drops an inbound packet
-// whose source address is not in the sender's certificate, so for a connection that
-// arrived on the nebula interface the source address maps to a certificate that claims
-// it. That certificate is the sender's only while no peer's unsafe networks overlap the
-// overlay: a peer whose certificate carries an unsafe network containing another node's
-// vpn address may send as that address, and the lookup then reports the node, not the
-// sender. And a connection that arrived on any other interface with an overlay source
-// address was never checked by nebula at all. A program authorizing on this answer
-// should confirm the connection came in on the nebula interface and know whether its
-// network lets unsafe networks overlap the overlay. The field names are an API: the
-// info API serves this shape to programs that were not built against nebula.
+// authorization decision from it, within one limit. Nebula drops an inbound packet
+// whose source address is not in the sender's certificate, and one with an overlay
+// source address that arrived through a peer's unsafe network rather than from the node
+// holding it, so for a connection that arrived on the nebula interface the source
+// address is the sender's and this is the sender's identity. A connection that arrived
+// on any other interface with an overlay source address was never checked by nebula at
+// all, so a program authorizing on this answer should confirm the connection came in
+// on the nebula interface. The field names are an API: the info API serves this shape
+// to programs that were not built against nebula.
 type hostIdentity struct {
 	Name           string         `json:"name"`
 	VpnAddrs       []netip.Addr   `json:"vpnAddrs"`