Browse Source

firewall: Key conntrack by the peer's first vpn address

JackDoan 4 days ago
parent
commit
7aa6f6c7da
4 changed files with 10 additions and 11 deletions
  1. 2 3
      firewall.go
  2. 1 1
      firewall_test.go
  3. 2 2
      interface.go
  4. 5 5
      interface_test.go

+ 2 - 3
firewall.go

@@ -463,9 +463,8 @@ func (f *Firewall) Drop(fp firewall.Packet, incoming bool, h *HostInfo, caPool *
 		return ErrInvalidLocalIP
 	}
 
-	// The highest vpn address identifies the peer. v2 certs sort their networks and v1 certs have only one,
-	// so this is the peer's v6 address when it has one, which stays put while v4 addresses come and go.
-	key := firewall.ConntrackKey{Packet: fp, Peer: h.vpnAddrs[len(h.vpnAddrs)-1]}
+	// The first vpn address identifies the peer, as it does elsewhere. A host's v1 and v2 certs must agree on it
+	key := firewall.ConntrackKey{Packet: fp, Peer: h.vpnAddrs[0]}
 
 	// Check if we spoke to this tuple, if we did then allow this packet
 	if f.inConns(key, h, caPool, localCache) {

+ 1 - 1
firewall_test.go

@@ -1198,7 +1198,7 @@ func TestFirewall_ConntrackPeerRehandshake(t *testing.T) {
 		RemotePort: 55000,
 		Protocol:   iputil.IPProtocolTCP,
 	}
-	key := firewall.ConntrackKey{Packet: flow, Peer: netip.MustParseAddr("2001:db8::2")}
+	key := firewall.ConntrackKey{Packet: flow, Peer: netip.MustParseAddr("192.0.2.2")}
 
 	t.Run("inbound flow follows the peer's groups", func(t *testing.T) {
 		fw := NewFirewall(l, time.Second, time.Minute, time.Hour, owner)

+ 2 - 2
interface.go

@@ -718,7 +718,7 @@ func (f *Interface) Close() error {
 }
 
 // siblingPeers implements gatewaySiblings from the routes and tunnels we have now. Routes name a gateway by any of its
-// vpn addresses, conntrack keys a peer by its highest one, so each gateway is resolved through its tunnel. A gateway
+// vpn addresses, conntrack keys a peer by its first one, so each gateway is resolved through its tunnel. A gateway
 // we have no tunnel to can't be carrying a live flow.
 func (f *Interface) siblingPeers(remote netip.Addr, h *HostInfo, buf []netip.Addr) []netip.Addr {
 	gateways := f.inside.RoutesFor(remote)
@@ -732,7 +732,7 @@ func (f *Interface) siblingPeers(remote netip.Addr, h *HostInfo, buf []netip.Add
 			continue
 		}
 		if s := f.hostMap.QueryVpnAddr(g.Addr()); s != nil && s != h {
-			buf = append(buf, s.vpnAddrs[len(s.vpnAddrs)-1])
+			buf = append(buf, s.vpnAddrs[0])
 		}
 	}
 	return buf

+ 5 - 5
interface_test.go

@@ -158,10 +158,10 @@ func (t routesTun) RoutesFor(ip netip.Addr) routing.Gateways {
 func TestInterface_SiblingPeers(t *testing.T) {
 	addr := netip.MustParseAddr
 	routes := new(bart.Table[routing.Gateways])
-	// a and b carry 198.51.100.0/24, the route names a by its v4 address
+	// a and b carry 198.51.100.0/24, the route names b by its v6 address
 	routes.Insert(netip.MustParsePrefix("198.51.100.0/24"), routing.Gateways{
 		routing.NewGateway(addr("192.0.2.2"), 1),
-		routing.NewGateway(addr("192.0.2.3"), 1),
+		routing.NewGateway(addr("fd00::3"), 1),
 		routing.NewGateway(addr("192.0.2.5"), 1), // no tunnel to this one
 	})
 	routes.Insert(netip.MustParsePrefix("203.0.113.0/24"), routing.Gateways{routing.NewGateway(addr("192.0.2.2"), 1)})
@@ -175,9 +175,9 @@ func TestInterface_SiblingPeers(t *testing.T) {
 	}
 	f := &Interface{inside: routesTun{routes: routes}, hostMap: hm}
 
-	// Siblings come back as their highest vpn address, the one conntrack keys on
-	assert.Equal(t, []netip.Addr{addr("fd00::3")}, f.siblingPeers(addr("198.51.100.5"), a, nil))
-	assert.Equal(t, []netip.Addr{addr("fd00::2")}, f.siblingPeers(addr("198.51.100.5"), b, nil))
+	// Siblings come back as their first vpn address, the one conntrack keys on, whichever address the route names
+	assert.Equal(t, []netip.Addr{addr("192.0.2.3")}, f.siblingPeers(addr("198.51.100.5"), a, nil))
+	assert.Equal(t, []netip.Addr{addr("192.0.2.2")}, f.siblingPeers(addr("198.51.100.5"), b, nil))
 	assert.Empty(t, f.siblingPeers(addr("198.51.100.5"), c, nil), "not one of our gateways for the network")
 	assert.Empty(t, f.siblingPeers(addr("203.0.113.5"), a, nil), "a single gateway route has no siblings")
 	assert.Empty(t, f.siblingPeers(addr("192.0.2.3"), a, nil), "no route")