Keep rule extras off the DNAT implied accept, expand all/any DNAT sources and match SPORT

internal/nftables/compiler_test.go

@@ -1627,7 +1627,7 @@ func TestCompile_QueueRedirMatchKernelReadback(t *testing.T) {
 	for _, rules := range state.Rules {
 		for _, r := range rules {
 			w, ok := want[r.Tag]
-			if !ok || r.Chain != "prerouting" && r.Tag == "rule:3" {
+			if !ok {
 				continue
 			}
 			if got := r.Exprs[len(r.Exprs)-1]; !reflect.DeepEqual(got, w) {
@@ -1943,6 +1943,18 @@ func TestCompile_CommaZoneLists(t *testing.T) {
 			want: map[string][]string{"prerouting": {"iif=eth0", "iif=eth2"},
 				"forward": {"iif=eth0 oif=eth2 daddr=192.0.2.10", "iif=eth2 oif=eth2 daddr=192.0.2.10"}},
 		},
+		{
+			name: "dnat all source expands per zone and skips fw and the target zone",
+			rule: config.Rule{Action: config.RuleDNAT, Source: "all", Dest: "svr:192.0.2.10", Proto: "tcp", DPort: config.PortSpec{"80"}},
+			want: map[string][]string{"prerouting": {"iif=eth3", "iif=eth1", "iif=eth0"},
+				"forward": {"iif=eth3 oif=eth2 daddr=192.0.2.10", "iif=eth1 oif=eth2 daddr=192.0.2.10", "iif=eth0 oif=eth2 daddr=192.0.2.10"}},
+		},
+		{
+			name: "dnat any+ source keeps the target zone",
+			rule: config.Rule{Action: config.RuleDNAT, Source: "any+", Dest: "svr:192.0.2.10", Proto: "tcp", DPort: config.PortSpec{"80"}},
+			want: map[string][]string{"prerouting": {"iif=eth3", "iif=eth1", "iif=eth0", "iif=eth2"},
+				"forward": {"iif=eth3 oif=eth2 daddr=192.0.2.10", "iif=eth1 oif=eth2 daddr=192.0.2.10", "iif=eth0 oif=eth2 daddr=192.0.2.10", "iif=eth2 oif=eth2 daddr=192.0.2.10"}},
+		},
 		{
 			name: "dnat to fw accepts in input",
 			rule: config.Rule{Action: config.RuleDNAT, Source: "net", Dest: "fw:192.0.2.1", Proto: "tcp", DPort: config.PortSpec{"80"}},
@@ -2056,7 +2068,7 @@ func TestCompile_CommaZoneLists(t *testing.T) {
 			got := map[string][]string{}
 			for chain, rules := range state.Rules {
 				for _, r := range rules {
-					if r.Tag == tag {
+					if r.Tag == tag || r.Tag == tag+":accept" {
 						got[chain] = append(got[chain], describeRule(r))
 					}
 				}
@@ -2263,18 +2275,79 @@ func TestCompile_DNATImpliedAccept(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Compile() error: %v", err)
 	}
-	fwd := taggedRules(state, "forward", "rule:0")
+	fwd := taggedRules(state, "forward", "rule:0:accept")
 	if len(fwd) != 1 {
 		t.Fatalf("got %d forward accepts, want 1", len(fwd))
 	}
 	want := append(append(append(append(append(matchIfaceName(true, "eth0"), matchIfaceName(false, "eth2")...),
 		mustExprs(t)(matchDestCIDR("192.0.2.17"))...), mustExprs(t)(l4Exprs("tcp", "8080"))...),
-		matchCtBits(expr.CtKeySTATUS, ctStatusDNAT)...), &expr.Verdict{Kind: expr.VerdictAccept})
+		dnatStatusExprs...), &expr.Verdict{Kind: expr.VerdictAccept})
 	if !reflect.DeepEqual(fwd[0].Exprs, want) {
 		t.Errorf("forward accept = %#v, want %#v", fwd[0].Exprs, want)
 	}
 }
 
+// IPS_DST_NAT = 1<<5, hard-coded so a wrong ctStatusDNAT or ct key fails here.
+var dnatStatusExprs = []expr.Any{
+	&expr.Ct{Key: expr.CtKeySTATUS, Register: 1},
+	&expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: binary.NativeEndian.AppendUint32(nil, 32), Xor: []byte{0, 0, 0, 0}},
+	&expr.Cmp{Op: expr.CmpOpNeq, Register: 1, Data: []byte{0, 0, 0, 0}},
+}
+
+func TestCompile_DNATImpliedAcceptGetsNoRuleExtras(t *testing.T) {
+	compile := func(r config.Rule) *FirewallState {
+		state, err := NewCompiler(listCfg(func(c *config.Config) {
+			c.Zones["svr"] = config.Zone{Type: config.ZoneIP}
+			c.Interfaces = append(c.Interfaces, config.Interface{Zone: "svr", Interface: "eth2"})
+			c.Rules = []config.Rule{r}
+		})).Compile()
+		if err != nil {
+			t.Fatalf("Compile() error: %v", err)
+		}
+		return state
+	}
+	plain := config.Rule{Action: config.RuleDNAT, Source: "net", Dest: "svr:192.0.2.17", Proto: "tcp", DPort: config.PortSpec{"80"}}
+	extras := plain
+	extras.RateLimit, extras.Mark, extras.User = "10/sec:5", "0x1", "root"
+	want, got := compile(plain), compile(extras)
+	if len(taggedRules(got, "forward", "rule:0:accept")) != 1 {
+		t.Fatalf("want one forward accept, got %v", got.Rules["forward"])
+	}
+	if !reflect.DeepEqual(got.Rules, want.Rules) {
+		t.Errorf("ratelimit/mark/user changed the DNAT rules:\ngot  %#v\nwant %#v", got.Rules, want.Rules)
+	}
+}
+
+func TestCompile_DNATMatchesSport(t *testing.T) {
+	state, err := NewCompiler(listCfg(func(c *config.Config) {
+		c.Zones["svr"] = config.Zone{Type: config.ZoneIP}
+		c.Interfaces = append(c.Interfaces, config.Interface{Zone: "svr", Interface: "eth2"})
+		c.Rules = []config.Rule{{Action: config.RuleDNAT, Source: "net", Dest: "svr:192.0.2.17", Proto: "tcp",
+			DPort: config.PortSpec{"80"}, SPort: config.PortSpec{"1024"}}}
+	})).Compile()
+	if err != nil {
+		t.Fatalf("Compile() error: %v", err)
+	}
+	m, err := l4Matches("tcp", config.PortSpec{"80"}, config.PortSpec{"1024"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, r := range append(taggedRules(state, "prerouting", "rule:0"), taggedRules(state, "forward", "rule:0:accept")...) {
+		if !containsExprs(r.Exprs, m[0].exprs) {
+			t.Errorf("%s rule lacks the sport match: %#v", r.Chain, r.Exprs)
+		}
+	}
+}
+
+func containsExprs(haystack, needle []expr.Any) bool {
+	for i := 0; i+len(needle) <= len(haystack); i++ {
+		if reflect.DeepEqual(haystack[i:i+len(needle)], needle) {
+			return true
+		}
+	}
+	return false
+}
+
 func mustExprs(t *testing.T) func([]expr.Any, error) []expr.Any {
 	return func(e []expr.Any, err error) []expr.Any {
 		t.Helper()
@@ -2298,6 +2371,7 @@ func TestCompile_CommaZoneListLimitErrors(t *testing.T) {
 		{Action: config.RuleAccept, Source: "net", Dest: "fw,lan", RateLimit: "10/sec:5"},
 		{Action: config.RuleAccept, Source: "net,lan", Dest: "fw", ConnLimit: "10"},
 		{Action: config.RuleAccept, Source: "net", Dest: "fw:192.0.2.1,198.51.100.1", RateLimit: "10/sec"},
+		{Action: config.RuleDNAT, Source: "net,lan", Dest: "fw:192.0.2.1", RateLimit: "10/sec"},
 	} {
 		t.Run(r.Source+">"+r.Dest, func(t *testing.T) {
 			cfg := &config.Config{