Restrict raw_output to fw sources and reject unmatched prerouting dest zones

internal/nftables/compiler.go

@@ -225,11 +225,14 @@ func (c *Compiler) compileConntrack(state *FirewallState) error {
 		}
 
 		for _, src := range srcs {
+			// raw_output only sees locally generated traffic, so it applies to an fw (or omitted) source only.
 			chains := []string{"raw_prerouting"}
 			switch {
-			case ct.Chain == config.ConntrackOutput, ct.Chain == "" && src.Zone == fwZone:
+			case ct.Chain == config.ConntrackOutput && src.Zone != fwZone && src.Zone != "":
+				return fmt.Errorf("conntrack[%d]: chain output needs SOURCE %s, got %q", i, fwZone, src.Zone)
+			case ct.Chain != config.ConntrackPrerouting && src.Zone == fwZone, ct.Chain == config.ConntrackOutput:
 				chains = []string{"raw_output"}
-			case ct.Chain == config.ConntrackBoth:
+			case ct.Chain == config.ConntrackBoth && src.Zone == "":
 				chains = []string{"raw_prerouting", "raw_output"}
 			}
 			for _, srcAddr := range splitAddrs(src.Addr) {
@@ -254,6 +257,10 @@ func (c *Compiler) compileConntrackPair(state *FirewallState, tag, chain string,
 	if ct.Action == config.ConntrackHelper {
 		return nil
 	}
+	if z, ok := c.cfg.Zones[dstZone]; ok && z.Type != config.ZoneFirewall && chain == "raw_prerouting" &&
+		(dstAddr == "" || strings.HasPrefix(dstAddr, "!")) {
+		return fmt.Errorf("conntrack DEST zone %q needs an address in prerouting", dstZone)
+	}
 	srcIfaces, dstIfaces := c.resolveZoneInterfaces(srcZone, srcAddr), []string{""}
 	if chain == "raw_output" {
 		srcIfaces, dstIfaces = []string{""}, c.resolveZoneInterfaces(dstZone, dstAddr)

internal/nftables/compiler_test.go

@@ -1871,6 +1871,9 @@ func describeRule(r ManagedRule) string {
 				}
 				parts = append(parts, name+"="+net.IP(cmp.Data).String())
 			}
+			if m.Base == expr.PayloadBaseTransportHeader && m.Offset == 0 && m.Len == 2 && cmp.Op == expr.CmpOpEq {
+				parts = append(parts, fmt.Sprintf("sport=%d", binary.BigEndian.Uint16(cmp.Data)))
+			}
 		}
 	}
 	return strings.Join(parts, " ")
@@ -2419,9 +2422,10 @@ func TestCompile_OrigDestForwardRejected(t *testing.T) {
 
 func TestCompile_ConntrackZones(t *testing.T) {
 	tests := []struct {
-		name string
-		ct   config.ConntrackRule
-		want map[string][]string
+		name    string
+		ct      config.ConntrackRule
+		want    map[string][]string
+		wantErr string
 	}{
 		{
 			name: "source zone matches iif",
@@ -2448,15 +2452,62 @@ func TestCompile_ConntrackZones(t *testing.T) {
 			ct:   config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "dmz", Dest: "fw"},
 			want: map[string][]string{},
 		},
+		{
+			name: "chain output with fw source matches dest oif",
+			ct:   config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "fw", Dest: "net", Chain: config.ConntrackOutput},
+			want: map[string][]string{"raw_output": {"oif=eth0"}},
+		},
+		{
+			name:    "chain output with non-fw source is rejected",
+			ct:      config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "net", Dest: "fw", Chain: config.ConntrackOutput},
+			wantErr: "chain output needs SOURCE fw",
+		},
+		{
+			name: "chain both with non-fw source emits prerouting only",
+			ct:   config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "net", Dest: "fw", Proto: "udp", DPort: config.PortSpec{"53"}, Chain: config.ConntrackBoth},
+			want: map[string][]string{"raw_prerouting": {"iif=eth0"}},
+		},
+		{
+			name: "chain both with fw source emits output only",
+			ct:   config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "fw", Dest: "lan", Chain: config.ConntrackBoth},
+			want: map[string][]string{"raw_output": {"oif=eth1"}},
+		},
+		{
+			name: "negated addresses stay one AND-ed match",
+			ct:   config.ConntrackRule{Action: config.ConntrackDrop, Source: "net:!192.0.2.1,198.51.100.1", Dest: "fw"},
+			want: map[string][]string{"raw_prerouting": {"iif=eth0 !saddr=192.0.2.1 !saddr=198.51.100.1"}},
+		},
+		{
+			name: "sport",
+			ct:   config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "net", Dest: "fw", Proto: "udp", SPort: config.PortSpec{"123"}},
+			want: map[string][]string{"raw_prerouting": {"iif=eth0 sport=123"}},
+		},
+		{
+			name:    "dest zone without address is rejected in prerouting",
+			ct:      config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "net", Dest: "lan"},
+			wantErr: `conntrack DEST zone "lan" needs an address in prerouting`,
+		},
+		{
+			name: "dest zone with address matches daddr",
+			ct:   config.ConntrackRule{Action: config.ConntrackNoTrack, Source: "net", Dest: "lan:203.0.113.10"},
+			want: map[string][]string{"raw_prerouting": {"iif=eth0 daddr=203.0.113.10"}},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			state := mustCompile(t, listCfg(func(cfg *config.Config) {
+			cfg := listCfg(func(cfg *config.Config) {
 				cfg.Zones["lan"] = config.Zone{Type: config.ZoneIP}
 				cfg.Zones["dmz"] = config.Zone{Type: config.ZoneIP}
 				cfg.Interfaces = append(cfg.Interfaces, config.Interface{Zone: "lan", Interface: "eth1"})
 				cfg.Conntrack = []config.ConntrackRule{tt.ct}
-			}))
+			})
+			if tt.wantErr != "" {
+				if _, err := NewCompiler(cfg).Compile(); err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+					t.Fatalf("Compile() error = %v, want %q", err, tt.wantErr)
+				}
+				return
+			}
+			state := mustCompile(t, cfg)
 			got := map[string][]string{}
 			for _, chain := range []string{"raw_prerouting", "raw_output"} {
 				for _, r := range taggedRules(state, chain, "conntrack:0:"+chain) {