diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 976f0d8..1ccb3ca 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1443,7 +1443,7 @@ func applyPackage(ctx context.Context, sys system.System, r *declaration.Package return out, err } if r.Absent { - // Declared absent (novox/hq ADR 0175): removed when it is here, left alone when it is not. + // Declared absent (novox/hq ADR 0180): removed when it is here, left alone when it is not. if !installed { out.Action = "unchanged" out.Detail = "not installed, as declared" diff --git a/internal/apply/left_out_test.go b/internal/apply/left_out_test.go index 4f91d3c..897655d 100644 --- a/internal/apply/left_out_test.go +++ b/internal/apply/left_out_test.go @@ -174,7 +174,7 @@ func TestACapabilityReachesTheRuntimeAndTheSpec(t *testing.T) { } } -// A package may be declared absent (novox/hq ADR 0175): removed when it is installed, read back, +// A package may be declared absent (novox/hq ADR 0180): removed when it is installed, read back, // left alone when it is not. func TestAPackageDeclaredAbsentIsRemovedWhenPresentAndLeftWhenNot(t *testing.T) { installed := true diff --git a/internal/apply/opening.go b/internal/apply/opening.go index 4c89239..bc34789 100644 --- a/internal/apply/opening.go +++ b/internal/apply/opening.go @@ -77,7 +77,7 @@ func retireFirewall(ctx context.Context, d *declaration.Declaration, origin stri return "", nil } if !firewall.Installed(ctx, run) { - // Uninstalled (novox/hq ADR 0175): retired for good, by the module that replaced it. Said + // Uninstalled (novox/hq ADR 0180): retired for good, by the module that replaced it. Said // once, and nothing is asked of a command that is not there. if rec.RetiredBy != firewall.RetiredRemoved { rec.RetiredBy = firewall.RetiredRemoved diff --git a/internal/apply/opening_test.go b/internal/apply/opening_test.go index 1a57764..cc97156 100644 --- a/internal/apply/opening_test.go +++ b/internal/apply/opening_test.go @@ -525,7 +525,7 @@ func TestUfwIsNotRetiredUntilTheMeshsOwnFilterIsLoaded(t *testing.T) { } // A front end that is no longer installed is recorded as removed, said once, and asked nothing of -// (novox/hq ADR 0175). +// (novox/hq ADR 0180). func TestAnUninstalledFrontEndIsRetiredForGood(t *testing.T) { dir := t.TempDir() u := &ufwMachine{installed: false, ruleset: "table inet mesh\n"} diff --git a/internal/apply/stayed_running_test.go b/internal/apply/stayed_running_test.go index 9ea13f3..d08fb24 100644 --- a/internal/apply/stayed_running_test.go +++ b/internal/apply/stayed_running_test.go @@ -2,6 +2,7 @@ package apply import ( "context" + "os" "strings" "testing" @@ -89,3 +90,11 @@ func TestAServiceAskedToStopIsNotWaitedOn(t *testing.T) { t.Fatalf("stopping a service was reported as a failure: %v", err) } } + +// The settle between a unit's two read-backs is a real pause on a machine and nothing in a test: +// no test here drives a service manager that takes time, so paying it would only slow the suite +// (novox/hq ADR 0184). +func TestMain(m *testing.M) { + serviceSettle = 0 + os.Exit(m.Run()) +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 23640c0..734ac1a 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -870,7 +870,7 @@ type Package struct { ID string `json:"id"` Type Type `json:"type"` Package string `json:"package"` - // Absent declares that the package is NOT installed (novox/hq ADR 0175): the host removes it + // Absent declares that the package is NOT installed (novox/hq ADR 0180): the host removes it // when it is, and leaves a machine that never had it alone. For the one case a module replaces // software the machine was found with and the operator has decided it does not come back — the // firewall front end a converged machine's filter module retired. Nothing to undo when the diff --git a/internal/firewall/filters.go b/internal/firewall/filters.go index 45888b1..13166b0 100644 --- a/internal/firewall/filters.go +++ b/internal/firewall/filters.go @@ -179,27 +179,18 @@ func legacyFilters(rules, tool string, ufwActive bool) []Filter { } } } - var entered func(chain string, seen map[string]bool) bool - entered = func(chain string, seen map[string]bool) bool { - if seen[chain] || accepting[chain] || len(jumpedFrom[chain]) == 0 { - return false - } - seen[chain] = true - for _, from := range jumpedFrom[chain] { - if p, builtIn := policy[from]; builtIn { - if p != "ACCEPT" { - return false - } - continue - } - if !entered(from, seen) { - return false - } - } - return true - } + // A chain of refusals is a ban list when every refusal names the sources it refuses and the + // chain accepts nothing — the same rule the nftables side applies, and no more. + // + // **The policy of the chains that jump to it says nothing about what it is.** An earlier cut + // required every path into the chain to come from a built-in whose policy accepts, and the + // mesh's own intrusion prevention then read as a foreign rule set on the home server: its ban + // chain hangs off the container runtime's user chain as well as INPUT, and that machine's + // forward policy is DROP because the runtime set it. The machine reported "NOT the mesh alone" + // about a chain the mesh had just written (novox/hq ADR 0186). The policy is already classified + // where it belongs — as the runtime's — so requiring it here counted it twice. ban := func(chain, line string) bool { - return bansSources(line) && entered(chain, map[string]bool{}) + return bansSources(line) && !accepting[chain] && len(jumpedFrom[chain]) > 0 } type seen struct { owner string @@ -320,7 +311,7 @@ func Active(ctx context.Context, run Runner) bool { } // Installed says whether ufw is on this machine at all: a command that is not there is a front end -// that was uninstalled (novox/hq ADR 0175), not one that is silent. +// that was uninstalled (novox/hq ADR 0180), not one that is silent. func Installed(ctx context.Context, run Runner) bool { _, err := run(ctx, "ufw", "status") return !missing(err) @@ -330,6 +321,6 @@ func Installed(ctx context.Context, run Runner) bool { const ( RetiredByMesh = "mesh" RetiredFoundSo = "found-inactive" - // RetiredRemoved is a front end uninstalled by the module that replaced it (ADR 0175). + // RetiredRemoved is a front end uninstalled by the module that replaced it (ADR 0180). RetiredRemoved = "removed" ) diff --git a/internal/firewall/neighbour_ban_test.go b/internal/firewall/neighbour_ban_test.go new file mode 100644 index 0000000..1e1a241 --- /dev/null +++ b/internal/firewall/neighbour_ban_test.go @@ -0,0 +1,60 @@ +package firewall + +import ( + "os" + "testing" +) + +// The mesh's own ban list is a ban wherever it hangs (novox/hq ADR 0186). +// +// Captured from the home server after the intrusion prevention had banned four addresses: its ban +// chain is jumped to from INPUT, whose policy accepts, and from the container runtime's user chain, +// which hangs off a FORWARD the runtime set to DROP. Requiring every path to come from an accepting +// built-in made the machine report "NOT the mesh alone" about a chain the mesh had just written. +func TestTheMeshsOwnBanChainIsABanBehindADroppingForward(t *testing.T) { + legacy, err := os.ReadFile("testdata/home-server-bans-S.txt") + if err != nil { + t.Fatal(err) + } + filters := Filters("", map[string]string{"iptables-legacy": string(legacy)}, false) + + var ban, other []string + for _, f := range filters { + switch f.Owner { + case OwnerBan: + ban = append(ban, f.Where) + case OwnerOther: + other = append(other, f.Where) + } + } + if len(other) > 0 { + t.Errorf("the machine reports %v as rule sets the mesh did not write", other) + } + found := false + for _, w := range ban { + if w == "chain f2b-route-proxy (iptables-legacy)" { + found = true + } + } + if !found { + t.Errorf("the intrusion prevention's own chain was not read as a ban; bans were %v", ban) + } + if !Alone(filters) { + t.Error("a machine filtered by the mesh and its own bans does not read as the mesh alone") + } +} + +// A chain that accepts anything is doing more than banning, and is still not a ban — which is what +// keeps a predecessor's allow-and-drop chain classified as something the operator must look at. +func TestAChainThatAcceptsIsNotABan(t *testing.T) { + rules := "-P INPUT ACCEPT\n" + + "-A INPUT -j HAL-MESH-ONLY\n" + + "-N HAL-MESH-ONLY\n" + + "-A HAL-MESH-ONLY -s 10.0.0.0/8 -j ACCEPT\n" + + "-A HAL-MESH-ONLY -s 203.0.113.7/32 -j DROP\n" + for _, f := range Filters("", map[string]string{"iptables-legacy": rules}, false) { + if f.Where == "chain HAL-MESH-ONLY (iptables-legacy)" && f.Owner != OwnerOther { + t.Errorf("a chain that accepts was classified as %s", f.Owner) + } + } +} diff --git a/internal/firewall/testdata/home-server-bans-S.txt b/internal/firewall/testdata/home-server-bans-S.txt new file mode 100644 index 0000000..d53cae8 --- /dev/null +++ b/internal/firewall/testdata/home-server-bans-S.txt @@ -0,0 +1,147 @@ +-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-route-proxy +-N ufw-after-forward +-N ufw-after-input +-N ufw-after-logging-forward +-N ufw-after-logging-input +-N ufw-after-logging-output +-N ufw-after-output +-N ufw-before-forward +-N ufw-before-input +-N ufw-before-logging-forward +-N ufw-before-logging-input +-N ufw-before-logging-output +-N ufw-before-output +-N ufw-reject-forward +-N ufw-reject-input +-N ufw-reject-output +-N ufw-track-forward +-N ufw-track-input +-N ufw-track-output +-A INPUT -p tcp -j f2b-route-proxy +-A INPUT -j ufw-before-logging-input +-A INPUT -j ufw-before-input +-A INPUT -j ufw-after-input +-A INPUT -j ufw-after-logging-input +-A INPUT -j ufw-reject-input +-A INPUT -j ufw-track-input +-A FORWARD -j DOCKER-USER +-A FORWARD -j DOCKER-FORWARD +-A FORWARD -j ufw-before-logging-forward +-A FORWARD -j ufw-before-forward +-A FORWARD -j ufw-after-forward +-A FORWARD -j ufw-after-logging-forward +-A FORWARD -j ufw-reject-forward +-A FORWARD -j ufw-track-forward +-A OUTPUT -j ufw-before-logging-output +-A OUTPUT -j ufw-before-output +-A OUTPUT -j ufw-after-output +-A OUTPUT -j ufw-after-logging-output +-A OUTPUT -j ufw-reject-output +-A OUTPUT -j ufw-track-output +-A DOCKER -d 172.17.0.18/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8686 -j ACCEPT +-A DOCKER -d 172.17.0.14/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8989 -j ACCEPT +-A DOCKER -d 172.17.0.15/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 7878 -j ACCEPT +-A DOCKER -d 172.17.0.5/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 9117 -j ACCEPT +-A DOCKER -d 172.17.0.13/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 6789 -j ACCEPT +-A DOCKER -d 172.19.0.2/32 ! -i br-32062158f584 -o br-32062158f584 -p tcp -m tcp --dport 8080 -j ACCEPT +-A DOCKER -d 172.17.0.2/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 5432 -j ACCEPT +-A DOCKER -d 172.27.0.2/32 ! -i br-0910a98c6158 -o br-0910a98c6158 -p tcp -m tcp --dport 5678 -j ACCEPT +-A DOCKER -d 172.17.0.21/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 3579 -j ACCEPT +-A DOCKER -d 172.17.0.19/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8181 -j ACCEPT +-A DOCKER -d 172.17.0.17/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8787 -j ACCEPT +-A DOCKER -d 172.17.0.16/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 6767 -j ACCEPT +-A DOCKER -d 172.17.0.12/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 3000 -j ACCEPT +-A DOCKER -d 172.17.0.11/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 80 -j ACCEPT +-A DOCKER -d 172.17.0.10/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 9443 -j ACCEPT +-A DOCKER -d 172.17.0.10/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 9000 -j ACCEPT +-A DOCKER -d 172.17.0.9/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 3000 -j ACCEPT +-A DOCKER -d 172.17.0.7/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 1880 -j ACCEPT +-A DOCKER -d 172.28.0.2/32 ! -i br-b11461b5b028 -o br-b11461b5b028 -p tcp -m tcp --dport 80 -j ACCEPT +-A DOCKER -d 172.23.0.14/32 ! -i br-66ffa5c1cba5 -o br-66ffa5c1cba5 -p tcp -m tcp --dport 6543 -j ACCEPT +-A DOCKER -d 172.23.0.14/32 ! -i br-66ffa5c1cba5 -o br-66ffa5c1cba5 -p tcp -m tcp --dport 5432 -j ACCEPT +-A DOCKER -d 172.23.0.5/32 ! -i br-66ffa5c1cba5 -o br-66ffa5c1cba5 -p tcp -m tcp --dport 8000 -j ACCEPT +-A DOCKER -d 172.26.0.3/32 ! -i br-b0fec361ccaa -o br-b0fec361ccaa -p tcp -m tcp --dport 6167 -j ACCEPT +-A DOCKER -d 172.26.0.2/32 ! -i br-b0fec361ccaa -o br-b0fec361ccaa -p tcp -m tcp --dport 80 -j ACCEPT +-A DOCKER -d 172.17.0.8/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8000 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p udp -m udp --dport 10001 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8880 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8843 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8443 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8080 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 6789 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p udp -m udp --dport 5514 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p udp -m udp --dport 3478 -j ACCEPT +-A DOCKER -d 172.17.0.6/32 ! -i docker0 -o docker0 -p udp -m udp --dport 1900 -j ACCEPT +-A DOCKER -d 172.25.0.3/32 ! -i br-b98821f7dc38 -o br-b98821f7dc38 -p tcp -m tcp --dport 8000 -j ACCEPT +-A DOCKER -d 172.18.0.3/32 ! -i br-442a0bfc65f8 -o br-442a0bfc65f8 -p tcp -m tcp --dport 1433 -j ACCEPT +-A DOCKER -d 172.20.0.3/32 ! -i br-afa37ac8b33d -o br-afa37ac8b33d -p tcp -m tcp --dport 8081 -j ACCEPT +-A DOCKER -d 172.20.0.3/32 ! -i br-afa37ac8b33d -o br-afa37ac8b33d -p tcp -m tcp --dport 1883 -j ACCEPT +-A DOCKER -d 172.17.0.4/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 8086 -j ACCEPT +-A DOCKER -d 172.21.0.2/32 ! -i br-df15d8e19ec7 -o br-df15d8e19ec7 -p tcp -m tcp --dport 6379 -j ACCEPT +-A DOCKER -d 172.30.0.3/32 ! -i br-521eab9a3a5e -o br-521eab9a3a5e -p tcp -m tcp --dport 8283 -j ACCEPT +-A DOCKER -d 172.17.0.3/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 3000 -j ACCEPT +-A DOCKER ! -i br-32062158f584 -o br-32062158f584 -j DROP +-A DOCKER ! -i docker0 -o docker0 -j DROP +-A DOCKER ! -i br-521eab9a3a5e -o br-521eab9a3a5e -j DROP +-A DOCKER ! -i br-df15d8e19ec7 -o br-df15d8e19ec7 -j DROP +-A DOCKER ! -i br-afa37ac8b33d -o br-afa37ac8b33d -j DROP +-A DOCKER ! -i br-442a0bfc65f8 -o br-442a0bfc65f8 -j DROP +-A DOCKER ! -i br-b98821f7dc38 -o br-b98821f7dc38 -j DROP +-A DOCKER ! -i br-b0fec361ccaa -o br-b0fec361ccaa -j DROP +-A DOCKER ! -i br-66ffa5c1cba5 -o br-66ffa5c1cba5 -j DROP +-A DOCKER ! -i br-b11461b5b028 -o br-b11461b5b028 -j DROP +-A DOCKER ! -i br-2df4e541b877 -o br-2df4e541b877 -j DROP +-A DOCKER ! -i br-0910a98c6158 -o br-0910a98c6158 -j DROP +-A DOCKER-BRIDGE -o br-32062158f584 -j DOCKER +-A DOCKER-BRIDGE -o docker0 -j DOCKER +-A DOCKER-BRIDGE -o br-521eab9a3a5e -j DOCKER +-A DOCKER-BRIDGE -o br-df15d8e19ec7 -j DOCKER +-A DOCKER-BRIDGE -o br-afa37ac8b33d -j DOCKER +-A DOCKER-BRIDGE -o br-442a0bfc65f8 -j DOCKER +-A DOCKER-BRIDGE -o br-b98821f7dc38 -j DOCKER +-A DOCKER-BRIDGE -o br-b0fec361ccaa -j DOCKER +-A DOCKER-BRIDGE -o br-66ffa5c1cba5 -j DOCKER +-A DOCKER-BRIDGE -o br-b11461b5b028 -j DOCKER +-A DOCKER-BRIDGE -o br-2df4e541b877 -j DOCKER +-A DOCKER-BRIDGE -o br-0910a98c6158 -j DOCKER +-A DOCKER-CT -o br-32062158f584 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o docker0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-521eab9a3a5e -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-df15d8e19ec7 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-afa37ac8b33d -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-442a0bfc65f8 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-b98821f7dc38 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-b0fec361ccaa -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-66ffa5c1cba5 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-b11461b5b028 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-2df4e541b877 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT +-A DOCKER-CT -o br-0910a98c6158 -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 br-32062158f584 -j ACCEPT +-A DOCKER-FORWARD -i docker0 -j ACCEPT +-A DOCKER-FORWARD -i br-521eab9a3a5e -j ACCEPT +-A DOCKER-FORWARD -i br-df15d8e19ec7 -j ACCEPT +-A DOCKER-FORWARD -i br-afa37ac8b33d -j ACCEPT +-A DOCKER-FORWARD -i br-442a0bfc65f8 -j ACCEPT +-A DOCKER-FORWARD -i br-b98821f7dc38 -j ACCEPT +-A DOCKER-FORWARD -i br-b0fec361ccaa -j ACCEPT +-A DOCKER-FORWARD -i br-66ffa5c1cba5 -j ACCEPT +-A DOCKER-FORWARD -i br-b11461b5b028 -j ACCEPT +-A DOCKER-FORWARD -i br-2df4e541b877 -j ACCEPT +-A DOCKER-FORWARD -i br-0910a98c6158 -j ACCEPT +-A DOCKER-USER -p tcp -j f2b-route-proxy +-A f2b-route-proxy -s 13.70.107.184/32 -j REJECT --reject-with icmp-port-unreachable +-A f2b-route-proxy -s 45.138.12.51/32 -j REJECT --reject-with icmp-port-unreachable +-A f2b-route-proxy -s 20.214.191.94/32 -j REJECT --reject-with icmp-port-unreachable +-A f2b-route-proxy -j RETURN diff --git a/internal/system/system.go b/internal/system/system.go index a630344..d8d36d2 100644 --- a/internal/system/system.go +++ b/internal/system/system.go @@ -52,7 +52,7 @@ type System interface { PackageInstalled(ctx context.Context, run Runner, name string) (bool, error) InstallPackage(ctx context.Context, run Runner, name string) error // RemovePackage uninstalls one package, leaving its dependencies and anything the operator - // changed in its configuration where the package manager leaves them (novox/hq ADR 0175). + // changed in its configuration where the package manager leaves them (novox/hq ADR 0180). RemovePackage(ctx context.Context, run Runner, name string) error // ServiceState is "running" or "stopped". A unit that does not exist is an error, never