internal/firewall/firewall.go

@@ -109,16 +109,31 @@ func statusActive(out string) bool {
 // drop or reject, or a base chain whose policy drops — and that is neither the mesh's own nor the
 // container runtime's. With ufw active, the tables iptables-nft manages are ufw's and the runtime's
 // and are not counted.
+//
+// **A ban is not a firewall.** fail2ban refuses the sources it banned and passes everything else;
+// captured on a lab machine with both of its backends (testdata/fail2ban-nftables.nft,
+// testdata/fail2ban-iptables.nft). The mesh opens nothing through it and it closes nothing the
+// mesh needs, so a refusal that names the sources it refuses, in a table or a chain that accepts
+// nothing and is entered only from chains whose policy accepts, is not counted.
 func Refusing(ruleset string, ufwActive bool) []string {
-	var refusing []string
+	type rule struct{ table, chain, line string }
+	type chainOf struct {
+		base, dropping, accepts bool
+		policyLine            string
+		jumpedFrom            []string
+	}
+	chains := map[string]*chainOf{} // by "table\x00chain"
+	tableAccepts := map[string]bool{}
+	var tables []string
+	var refusals []rule
 	managed := map[string]bool{}
 	var table, chain string
-	counted := map[string]bool{}
-	note := func() {
-		if !counted[table] {
-			counted[table] = true
-			refusing = append(refusing, "table "+table)
+	get := func(t, c string) *chainOf {
+		k := t + "\x00" + c
+		if chains[k] == nil {
+			chains[k] = &chainOf{}
 		}
+		return chains[k]
 	}
 	for _, raw := range strings.Split(ruleset, "\n") {
 		line := strings.TrimSpace(raw)
@@ -131,34 +146,108 @@ func Refusing(ruleset string, ufwActive bool) []string {
 		case strings.HasPrefix(line, "table "):
 			table = strings.TrimSuffix(strings.TrimSpace(strings.TrimPrefix(line, "table ")), "{")
 			table = strings.TrimSpace(table)
+			tables = append(tables, table)
 			chain = ""
 			continue
 		case strings.HasPrefix(line, "chain "):
 			chain = strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(line, "chain "), "{"))
+			get(table, chain)
+			continue
+		case strings.HasPrefix(line, "set ") || strings.HasPrefix(line, "map ") ||
+			strings.HasPrefix(line, "flowtable "):
+			chain = ""
 			continue
 		case line == "" || line == "}" || strings.HasPrefix(line, "#") || chain == "":
 			continue
 		}
-		if table == "inet mesh" || table == "inet mesh_guard" {
-			continue
-		}
-		iptables := managed[table] || iptablesTable(table)
-		if iptables && ufwActive {
-			continue
-		}
+		c := get(table, chain)
 		if strings.HasPrefix(line, "type ") {
-			if strings.Contains(line, "policy drop") && !(iptables && runtimes(table, chain, line)) {
-				note()
+			c.base = true
+			c.policyLine = line
+			c.dropping = strings.Contains(line, "policy drop")
+			continue
+		}
+		for _, verb := range []string{"jump ", "goto "} {
+			if i := strings.Index(line, verb); i >= 0 {
+				target := strings.Fields(line[i+len(verb):])
+				if len(target) > 0 {
+					get(table, target[0]).jumpedFrom = append(get(table, target[0]).jumpedFrom, chain)
+				}
 			}
+		}
+		if accepts(line) {
+			c.accepts = true
+			tableAccepts[table] = true
+		}
+		if verdictRefuses(line) {
+			refusals = append(refusals, rule{table, chain, line})
+		}
+	}
+
+	skipped := func(table string) bool {
+		if table == "inet mesh" || table == "inet mesh_guard" {
+			return true
+		}
+		return (managed[table] || iptablesTable(table)) && ufwActive
+	}
+	// onlyBans is whether a refusal only refuses the sources it names: in a table that accepts
+	// nothing and whose base chains all accept by default, or in a chain that accepts nothing and
+	// is entered only from base chains that accept by default.
+	onlyBans := func(r rule) bool {
+		if !bansSources(r.line) {
+			return false
+		}
+		allAccepting := true
+		for k, c := range chains {
+			if strings.HasPrefix(k, r.table+"\x00") && c.base && !strings.Contains(c.policyLine, "policy accept") {
+				allAccepting = false
+			}
+		}
+		if !tableAccepts[r.table] && allAccepting {
+			return true
+		}
+		c := get(r.table, r.chain)
+		if c.base || c.accepts || len(c.jumpedFrom) == 0 {
+			return false
+		}
+		for _, from := range c.jumpedFrom {
+			caller := get(r.table, from)
+			if !caller.base || !strings.Contains(caller.policyLine, "policy accept") {
+				return false
+			}
+		}
+		return true
+	}
+
+	counted := map[string]bool{}
+	for k, c := range chains {
+		t, name, _ := strings.Cut(k, "\x00")
+		if skipped(t) || !c.dropping {
 			continue
 		}
-		if !verdictRefuses(line) {
+		if (managed[t] || iptablesTable(t)) && runtimes(t, name, c.policyLine) {
 			continue
 		}
-		if iptables && runtimes(table, chain, line) {
+		counted[t] = true
+	}
+	for _, r := range refusals {
+		if skipped(r.table) || counted[r.table] {
 			continue
 		}
-		note()
+		if (managed[r.table] || iptablesTable(r.table)) && runtimes(r.table, r.chain, r.line) {
+			continue
+		}
+		if onlyBans(r) {
+			continue
+		}
+		counted[r.table] = true
+	}
+	var refusing []string
+	for _, t := range tables {
+		if counted[t] {
+			counted[t] = false
+			refusing = append(refusing, "table "+t)
+		}
 	}
 	return refusing
 }
@@ -194,15 +283,73 @@ func runtimes(table, chain, line string) bool {
 	return false
 }
 
-var verdict = regexp.MustCompile(`(^|\s)(drop|reject)(\s|$)`)
+// iptables-nft prints a REJECT target it cannot translate as `xt target "REJECT"`, measured in
+// testdata/fail2ban-iptables.nft; a refusal written that way is a refusal too.
+var verdict = regexp.MustCompile(`(^|\s)(drop|reject)(\s|$)|xt target "(DROP|REJECT)"`)
 
 func verdictRefuses(line string) bool {
 	return verdict.MatchString(line)
 }
 
+var acceptVerdict = regexp.MustCompile(`(^|\s)accept(\s|;|$)|xt target "ACCEPT"`)
+
+func accepts(line string) bool {
+	return acceptVerdict.MatchString(line)
+}
+
+// bansSources is whether a refusal names the sources it refuses — a set or an address — rather
+// than refusing everyone but some.
+func bansSources(line string) bool {
+	f := strings.Fields(line)
+	for i, w := range f {
+		if (w == "saddr" || w == "-s") && i+1 < len(f) && f[i+1] != "!=" && !strings.HasPrefix(f[i+1], "!") {
+			return i == 0 || f[i-1] != "!"
+		}
+	}
+	return false
+}
+
 // RefusingLegacy names the chains of an `iptables-legacy -S` that refuse traffic outside the
-// container runtime's own.
+// container runtime's own. A ban — a refusal of the sources it names, in a chain that accepts
+// nothing and is entered only from built-in chains whose policy accepts — is not counted, as in
+// Refusing (testdata/fail2ban-iptables-S.txt).
 func RefusingLegacy(rules string) []string {
+	policy := map[string]string{}
+	accepting := map[string]bool{}
+	jumpedFrom := map[string][]string{}
+	for _, line := range strings.Split(rules, "\n") {
+		fields := strings.Fields(line)
+		if len(fields) < 3 {
+			continue
+		}
+		switch fields[0] {
+		case "-P":
+			policy[fields[1]] = fields[2]
+		case "-A":
+			for i, f := range fields {
+				if (f == "-j" || f == "-g") && i+1 < len(fields) {
+					switch fields[i+1] {
+					case "ACCEPT":
+						accepting[fields[1]] = true
+					case "DROP", "REJECT", "RETURN", "LOG":
+					default:
+						jumpedFrom[fields[i+1]] = append(jumpedFrom[fields[i+1]], fields[1])
+					}
+				}
+			}
+		}
+	}
+	ban := func(chain, line string) bool {
+		if !bansSources(line) || accepting[chain] || len(jumpedFrom[chain]) == 0 {
+			return false
+		}
+		for _, from := range jumpedFrom[chain] {
+			if policy[from] != "ACCEPT" {
+				return false
+			}
+		}
+		return true
+	}
 	var refusing []string
 	seen := map[string]bool{}
 	for _, line := range strings.Split(rules, "\n") {
@@ -218,7 +365,7 @@ func RefusingLegacy(rules string) []string {
 		case "-A":
 			for i, f := range fields {
 				if f == "-j" && i+1 < len(fields) && (fields[i+1] == "DROP" || fields[i+1] == "REJECT") {
-					refuses = !strings.HasPrefix(chain, "DOCKER")
+					refuses = !strings.HasPrefix(chain, "DOCKER") && !ban(chain, line)
 				}
 			}
 		}

internal/firewall/firewall_test.go

@@ -500,3 +500,53 @@ func TestRetiringUfwOnAMachineWithoutIptablesStillRetiresIt(t *testing.T) {
 		t.Fatalf("disable: %v, active %v", err, f.active)
 	}
 }
+
+// Captured on a lab machine with fail2ban banning one documentation address in its sshd jail,
+// once with its nftables backend and once with its iptables backend (iptables-nft), ufw inactive.
+
+func captured(t *testing.T, name string) string {
+	t.Helper()
+	raw, err := os.ReadFile("testdata/" + name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return string(raw)
+}
+
+func TestFail2bansBansAreNotAFirewall(t *testing.T) {
+	for _, name := range []string{"fail2ban-nftables.nft", "fail2ban-iptables.nft"} {
+		ruleset := captured(t, name)
+		if !strings.Contains(ruleset, "192.0.2.55") {
+			t.Fatalf("%s holds no ban", name)
+		}
+		if got := Refusing(ruleset, false); len(got) != 0 {
+			t.Errorf("%s: fail2ban's bans read as a firewall: %v", name, got)
+		}
+		kind, what, err := Detect(context.Background(), (&fakeUFW{ruleset: ruleset}).run)
+		if err != nil || kind != None {
+			t.Errorf("%s: a machine with only fail2ban detected as %s (%s) %v", name, kind, what, err)
+		}
+	}
+	if got := RefusingLegacy(captured(t, "fail2ban-iptables-S.txt")); len(got) != 0 {
+		t.Errorf("fail2ban's iptables bans read as a firewall: %v", got)
+	}
+}
+
+func TestARefusalOfEveryoneButSomeIsStillAFirewall(t *testing.T) {
+	// A ban names the sources it refuses. A table that refuses every source but some, or every
+	// port but some, closes what the mesh would open, whatever its policy says.
+	for name, table := range map[string]string{
+		"all but a range":              "table inet own {\n\tchain input {\n\t\ttype filter hook input priority filter; policy accept;\n\t\tip saddr != 10.0.0.0/8 drop\n\t}\n}\n",
+		"all but ssh":                  "table inet own {\n\tchain input {\n\t\ttype filter hook input priority filter; policy accept;\n\t\ttcp dport != 22 drop\n\t}\n}\n",
+		"iptables reject":              "# Warning: table ip filter is managed by iptables-nft, do not touch!\ntable ip filter {\n\tchain INPUT {\n\t\ttype filter hook input priority filter; policy accept;\n\t\tcounter packets 0 bytes 0 xt target \"REJECT\"\n\t}\n}\n",
+		"ban beside a dropping policy": "table inet own {\n\tchain input {\n\t\ttype filter hook input priority filter; policy drop;\n\t\tip saddr 192.0.2.9 drop\n\t}\n}\n",
+	} {
+		if got := Refusing(dockerOnly(t)+table, false); len(got) == 0 {
+			t.Errorf("%s: not counted as a firewall", name)
+		}
+	}
+	legacy := "-P INPUT ACCEPT\n-N own\n-A INPUT -j own\n-A own ! -s 10.0.0.0/8 -j DROP\n"
+	if got := RefusingLegacy(legacy); len(got) == 0 {
+		t.Error("a legacy refusal of all but a range was not counted")
+	}
+}

internal/firewall/testdata/fail2ban-iptables-S.txt

@@ -0,0 +1,22 @@
+-P INPUT ACCEPT
+-P FORWARD DROP
+-P OUTPUT ACCEPT
+-N DOCKER
+-N DOCKER-BRIDGE
+-N DOCKER-CT
+-N DOCKER-FORWARD
+-N DOCKER-INTERNAL
+-N DOCKER-USER
+-N f2b-sshd
+-A INPUT -p tcp -m multiport --dports 22 -j f2b-sshd
+-A FORWARD -j DOCKER-USER
+-A FORWARD -j DOCKER-FORWARD
+-A DOCKER ! -i docker0 -o docker0 -j DROP
+-A DOCKER-BRIDGE -o docker0 -j DOCKER
+-A DOCKER-CT -o docker0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
+-A DOCKER-FORWARD -j DOCKER-CT
+-A DOCKER-FORWARD -j DOCKER-INTERNAL
+-A DOCKER-FORWARD -j DOCKER-BRIDGE
+-A DOCKER-FORWARD -i docker0 -j ACCEPT
+-A f2b-sshd -s 192.0.2.55/32 -j REJECT --reject-with icmp-port-unreachable
+-A f2b-sshd -j RETURN

internal/firewall/testdata/fail2ban-iptables.nft

@@ -0,0 +1,103 @@
+table ip nat {
+	chain DOCKER {
+	}
+
+	chain PREROUTING {
+		type nat hook prerouting priority dstnat; policy accept;
+		xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain OUTPUT {
+		type nat hook output priority dstnat; policy accept;
+		ip daddr != 127.0.0.0/8 xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain POSTROUTING {
+		type nat hook postrouting priority srcnat; policy accept;
+		ip saddr 172.17.0.0/16 oifname != "docker0" counter packets 0 bytes 0 xt target "MASQUERADE"
+	}
+}
+table ip filter {
+	chain DOCKER {
+		iifname != "docker0" oifname "docker0" counter packets 0 bytes 0 drop
+	}
+
+	chain DOCKER-FORWARD {
+		counter packets 0 bytes 0 jump DOCKER-CT
+		counter packets 0 bytes 0 jump DOCKER-INTERNAL
+		counter packets 0 bytes 0 jump DOCKER-BRIDGE
+		iifname "docker0" counter packets 0 bytes 0 accept
+	}
+
+	chain DOCKER-BRIDGE {
+		oifname "docker0" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain DOCKER-CT {
+		oifname "docker0" xt match "conntrack" counter packets 0 bytes 0 accept
+	}
+
+	chain DOCKER-INTERNAL {
+	}
+
+	chain FORWARD {
+		type filter hook forward priority filter; policy drop;
+		counter packets 0 bytes 0 jump DOCKER-USER
+		counter packets 0 bytes 0 jump DOCKER-FORWARD
+	}
+
+	chain DOCKER-USER {
+	}
+
+	chain f2b-sshd {
+		ip saddr 192.0.2.55 counter packets 0 bytes 0 xt target "REJECT"
+		counter packets 0 bytes 0 return
+	}
+
+	chain INPUT {
+		type filter hook input priority filter; policy accept;
+		ip protocol tcp xt match "multiport" counter packets 0 bytes 0 jump f2b-sshd
+	}
+}
+table ip6 nat {
+	chain DOCKER {
+	}
+
+	chain PREROUTING {
+		type nat hook prerouting priority dstnat; policy accept;
+		xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain OUTPUT {
+		type nat hook output priority dstnat; policy accept;
+		ip6 daddr != ::1 xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+}
+table ip6 filter {
+	chain DOCKER {
+	}
+
+	chain DOCKER-FORWARD {
+		counter packets 0 bytes 0 jump DOCKER-CT
+		counter packets 0 bytes 0 jump DOCKER-INTERNAL
+		counter packets 0 bytes 0 jump DOCKER-BRIDGE
+	}
+
+	chain DOCKER-BRIDGE {
+	}
+
+	chain DOCKER-CT {
+	}
+
+	chain DOCKER-INTERNAL {
+	}
+
+	chain FORWARD {
+		type filter hook forward priority filter; policy accept;
+		counter packets 0 bytes 0 jump DOCKER-USER
+		counter packets 0 bytes 0 jump DOCKER-FORWARD
+	}
+
+	chain DOCKER-USER {
+	}
+}

internal/firewall/testdata/fail2ban-nftables.nft

@@ -0,0 +1,105 @@
+table ip nat {
+	chain DOCKER {
+	}
+
+	chain PREROUTING {
+		type nat hook prerouting priority dstnat; policy accept;
+		xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain OUTPUT {
+		type nat hook output priority dstnat; policy accept;
+		ip daddr != 127.0.0.0/8 xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain POSTROUTING {
+		type nat hook postrouting priority srcnat; policy accept;
+		ip saddr 172.17.0.0/16 oifname != "docker0" counter packets 0 bytes 0 xt target "MASQUERADE"
+	}
+}
+table ip filter {
+	chain DOCKER {
+		iifname != "docker0" oifname "docker0" counter packets 0 bytes 0 drop
+	}
+
+	chain DOCKER-FORWARD {
+		counter packets 0 bytes 0 jump DOCKER-CT
+		counter packets 0 bytes 0 jump DOCKER-INTERNAL
+		counter packets 0 bytes 0 jump DOCKER-BRIDGE
+		iifname "docker0" counter packets 0 bytes 0 accept
+	}
+
+	chain DOCKER-BRIDGE {
+		oifname "docker0" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain DOCKER-CT {
+		oifname "docker0" xt match "conntrack" counter packets 0 bytes 0 accept
+	}
+
+	chain DOCKER-INTERNAL {
+	}
+
+	chain FORWARD {
+		type filter hook forward priority filter; policy drop;
+		counter packets 0 bytes 0 jump DOCKER-USER
+		counter packets 0 bytes 0 jump DOCKER-FORWARD
+	}
+
+	chain DOCKER-USER {
+	}
+}
+table ip6 nat {
+	chain DOCKER {
+	}
+
+	chain PREROUTING {
+		type nat hook prerouting priority dstnat; policy accept;
+		xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+
+	chain OUTPUT {
+		type nat hook output priority dstnat; policy accept;
+		ip6 daddr != ::1 xt match "addrtype" counter packets 0 bytes 0 jump DOCKER
+	}
+}
+table ip6 filter {
+	chain DOCKER {
+	}
+
+	chain DOCKER-FORWARD {
+		counter packets 0 bytes 0 jump DOCKER-CT
+		counter packets 0 bytes 0 jump DOCKER-INTERNAL
+		counter packets 0 bytes 0 jump DOCKER-BRIDGE
+	}
+
+	chain DOCKER-BRIDGE {
+	}
+
+	chain DOCKER-CT {
+	}
+
+	chain DOCKER-INTERNAL {
+	}
+
+	chain FORWARD {
+		type filter hook forward priority filter; policy accept;
+		counter packets 0 bytes 0 jump DOCKER-USER
+		counter packets 0 bytes 0 jump DOCKER-FORWARD
+	}
+
+	chain DOCKER-USER {
+	}
+}
+table inet f2b-table {
+	set addr-set-sshd {
+		type ipv4_addr
+		flags interval
+		elements = { 192.0.2.55 }
+	}
+
+	chain f2b-chain {
+		type filter hook input priority filter - 1; policy accept;
+		tcp dport 22 ip saddr @addr-set-sshd reject with icmp port-unreachable
+	}
+}