fix(mesh): avoid NAT between allowed location IPs

pkg/mesh/routes.go

@@ -400,11 +400,18 @@ func (t *Topology) Rules(cni, iptablesForwardRule bool) iptables.RuleSet {
 			rules.AddToPrepend(iptables.NewRule(iptables.GetProtocol(aip.IP), "nat", "KILO-NAT", "-d", aip.String(), "-m", "comment", "--comment", "Kilo: do not NAT packets destined for known IPs", "-j", "RETURN"))
 		}
 		// Make sure packets to allowed location IPs go through the KILO-NAT chain, so they can be MASQUERADEd,
-		// Otherwise packets to these destinations will reach the destination, but never find their way back.
-		// We only want to NAT in locations of the corresponding allowed location IPs.
+		// otherwise packets to these destinations will reach the destination, but never find their way back.
+		// We only want to NAT in locations of the corresponding allowed location IPs, and never between
+		// allowed location IPs in the same location.
 		if t.location == s.location {
-			for _, alip := range s.allowedLocationIPs {
-				rules.AddToPrepend(iptables.NewRule(iptables.GetProtocol(alip.IP), "nat", "POSTROUTING", "-d", alip.String(), "-m", "comment", "--comment", "Kilo: jump to NAT chain", "-j", "KILO-NAT"))
+			for _, destination := range s.allowedLocationIPs {
+				rules.AddToPrepend(iptables.NewRule(iptables.GetProtocol(destination.IP), "nat", "POSTROUTING", "-d", destination.String(), "-m", "comment", "--comment", "Kilo: jump to NAT chain", "-j", "KILO-NAT"))
+				for _, source := range s.allowedLocationIPs {
+					if iptables.GetProtocol(source.IP) != iptables.GetProtocol(destination.IP) {
+						continue
+					}
+					rules.AddToPrepend(iptables.NewRule(iptables.GetProtocol(destination.IP), "nat", "KILO-NAT", "-s", source.String(), "-d", destination.String(), "-m", "comment", "--comment", "Kilo: do not NAT between allowed location IPs", "-j", "RETURN"))
+				}
 			}
 		}
 	}

pkg/mesh/routes_test.go

@@ -15,13 +15,16 @@
 package mesh
 
 import (
+	"net"
+	"strings"
 	"testing"
 
 	"github.com/kylelemons/godebug/pretty"
 	"github.com/vishvananda/netlink"
 	"golang.org/x/sys/unix"
 
 	"github.com/squat/kilo/pkg/encapsulation"
+	"github.com/squat/kilo/pkg/iptables"
 )
 
 func TestRoutes(t *testing.T) {
@@ -1205,3 +1208,65 @@ func TestRoutes(t *testing.T) {
 		}
 	}
 }
+
+func TestRulesDoNotNATBetweenAllowedLocationIPs(t *testing.T) {
+	allowedLocationIPs := []net.IPNet{
+		mustParseCIDR("192.168.102.0/23"),
+		mustParseCIDR("10.10.0.0/16"),
+	}
+	topology := &Topology{
+		location: "same-location",
+		segments: []*segment{
+			{
+				location:           "same-location",
+				wireGuardIP:        net.ParseIP("10.4.0.1"),
+				allowedLocationIPs: allowedLocationIPs,
+			},
+		},
+	}
+	client := &recordingIPTablesClient{}
+	controller, err := iptables.New(iptables.WithClients(client, client))
+	if err != nil {
+		t.Fatalf("failed to create iptables controller: %v", err)
+	}
+	if err := controller.Set(topology.Rules(false, false)); err != nil {
+		t.Fatalf("failed to apply topology rules: %v", err)
+	}
+
+	for _, expected := range []string{
+		"nat KILO-NAT -s 10.10.0.0/16 -d 192.168.102.0/23 -m comment --comment Kilo: do not NAT between allowed location IPs -j RETURN",
+		"nat KILO-NAT -s 192.168.102.0/23 -d 10.10.0.0/16 -m comment --comment Kilo: do not NAT between allowed location IPs -j RETURN",
+	} {
+		if !containsString(client.inserted, expected) {
+			t.Errorf("expected rule %q, got %q", expected, client.inserted)
+		}
+	}
+}
+
+type recordingIPTablesClient struct {
+	inserted []string
+}
+
+func (c *recordingIPTablesClient) AppendUnique(string, string, ...string) error { return nil }
+func (c *recordingIPTablesClient) InsertUnique(table, chain string, _ int, rule ...string) error {
+	c.inserted = append(c.inserted, strings.Join(append([]string{table, chain}, rule...), " "))
+	return nil
+}
+func (c *recordingIPTablesClient) Delete(string, string, ...string) error { return nil }
+func (c *recordingIPTablesClient) Exists(string, string, ...string) (bool, error) {
+	return false, nil
+}
+func (c *recordingIPTablesClient) List(string, string) ([]string, error) { return nil, nil }
+func (c *recordingIPTablesClient) ClearChain(string, string) error       { return nil }
+func (c *recordingIPTablesClient) DeleteChain(string, string) error      { return nil }
+func (c *recordingIPTablesClient) NewChain(string, string) error         { return nil }
+func (c *recordingIPTablesClient) ListChains(string) ([]string, error)   { return nil, nil }
+
+func containsString(haystack []string, needle string) bool {
+	for _, value := range haystack {
+		if value == needle {
+			return true
+		}
+	}
+	return false
+}