Put back the forward policy ufw disable opens when the found firewall is retired, as measured on a lab machine (hq ADR 0100)
This commit is contained in:
@@ -419,11 +419,61 @@ func Enable(ctx context.Context, run Runner) error {
|
||||
|
||||
// Disable retires ufw without flushing it: its configuration stays on disk, and the container
|
||||
// runtime's rules are not its to remove.
|
||||
//
|
||||
// **Nor is the forward policy ufw's to open.** Measured on a lab machine running the container
|
||||
// runtime with a published port (testdata/ufw-disable-iptables-before.txt and -after.txt):
|
||||
// `ufw disable` sets every built-in chain's policy to accept, the forward chain's among them. The
|
||||
// runtime had set that one to drop when it turned forwarding on, and it does not set it again while
|
||||
// forwarding stays on — not even on a restart. Left so, a retired ufw turns the machine into a
|
||||
// router for anyone who can reach it. So each family's forward policy is read before, and one that
|
||||
// was drop is put back and read back.
|
||||
func Disable(ctx context.Context, run Runner) error {
|
||||
type family struct{ tool, policy string }
|
||||
var before []family
|
||||
for _, tool := range []string{"iptables", "ip6tables"} {
|
||||
if policy, ok := forwardPolicy(ctx, run, tool); ok {
|
||||
before = append(before, family{tool, policy})
|
||||
}
|
||||
}
|
||||
if _, err := run(ctx, "ufw", "disable"); err != nil {
|
||||
return fmt.Errorf("disabling ufw: %w", err)
|
||||
}
|
||||
return expectActive(ctx, run, false)
|
||||
if err := expectActive(ctx, run, false); err != nil {
|
||||
return err
|
||||
}
|
||||
for _, f := range before {
|
||||
if f.policy != "DROP" {
|
||||
continue
|
||||
}
|
||||
if now, ok := forwardPolicy(ctx, run, f.tool); ok && now == "DROP" {
|
||||
continue
|
||||
}
|
||||
if _, err := run(ctx, f.tool, "-P", "FORWARD", "DROP"); err != nil {
|
||||
return fmt.Errorf("ufw is disabled, and %s's forward policy, which was drop, could not be put back: %w",
|
||||
f.tool, err)
|
||||
}
|
||||
if now, ok := forwardPolicy(ctx, run, f.tool); !ok || now != "DROP" {
|
||||
return fmt.Errorf("ufw is disabled, and %s's forward policy was put back to drop and reads %q",
|
||||
f.tool, now)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// forwardPolicy reads the forward chain's policy the way iptables prints it: "-P FORWARD DROP".
|
||||
// Not ok when the tool is absent or says nothing readable.
|
||||
func forwardPolicy(ctx context.Context, run Runner, tool string) (string, bool) {
|
||||
out, err := run(ctx, tool, "-S", "FORWARD")
|
||||
if err != nil {
|
||||
return "", false
|
||||
}
|
||||
for _, line := range strings.Split(out, "\n") {
|
||||
f := strings.Fields(line)
|
||||
if len(f) == 3 && f[0] == "-P" && f[1] == "FORWARD" {
|
||||
return f[2], true
|
||||
}
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
func expectActive(ctx context.Context, run Runner, want bool) error {
|
||||
|
||||
@@ -114,6 +114,41 @@ type fakeUFW struct {
|
||||
ruleset string
|
||||
firewalld bool
|
||||
asked []string
|
||||
|
||||
// iptablesActive and iptablesInactive are what `iptables -S` prints with ufw active and
|
||||
// after it is disabled; empty is a machine without iptables. forward is a policy set since.
|
||||
iptablesActive, iptablesInactive string
|
||||
forward string
|
||||
}
|
||||
|
||||
// iptables answers `iptables -S FORWARD` from the captured output for ufw's state, and records
|
||||
// a forward policy set with -P.
|
||||
func (f *fakeUFW) iptables(name string, args []string) (string, error) {
|
||||
if f.iptablesActive == "" {
|
||||
return "", &exec.Error{Name: name, Err: exec.ErrNotFound}
|
||||
}
|
||||
if name == "ip6tables" {
|
||||
return "", &exec.Error{Name: name, Err: exec.ErrNotFound}
|
||||
}
|
||||
if len(args) == 3 && args[0] == "-P" && args[1] == "FORWARD" {
|
||||
f.forward = args[2]
|
||||
return "", nil
|
||||
}
|
||||
captured := f.iptablesInactive
|
||||
if f.active {
|
||||
captured = f.iptablesActive
|
||||
}
|
||||
var out []string
|
||||
for _, line := range strings.Split(captured, "\n") {
|
||||
fields := strings.Fields(line)
|
||||
if len(fields) >= 2 && fields[1] == "FORWARD" {
|
||||
if fields[0] == "-P" && f.forward != "" && !f.active {
|
||||
line = "-P FORWARD " + f.forward
|
||||
}
|
||||
out = append(out, line)
|
||||
}
|
||||
}
|
||||
return strings.Join(out, "\n") + "\n", nil
|
||||
}
|
||||
|
||||
func canonical(args []string) string {
|
||||
@@ -155,6 +190,8 @@ func (f *fakeUFW) run(_ context.Context, name string, args ...string) (string, e
|
||||
return f.ruleset, nil
|
||||
case "iptables-legacy", "ip6tables-legacy":
|
||||
return "", &exec.Error{Name: name, Err: exec.ErrNotFound}
|
||||
case "iptables", "ip6tables":
|
||||
return f.iptables(name, args)
|
||||
case "ufw":
|
||||
default:
|
||||
return "", fmt.Errorf("unexpected %s", name)
|
||||
@@ -179,6 +216,7 @@ func (f *fakeUFW) run(_ context.Context, name string, args ...string) (string, e
|
||||
return "Firewall is active and enabled on system startup\n", nil
|
||||
case args[0] == "disable":
|
||||
f.active = false
|
||||
f.forward = ""
|
||||
return "Firewall stopped and disabled on system startup\n", nil
|
||||
case args[0] == "delete" && len(args) > 1 && args[1] == "route":
|
||||
// As the real ufw answers it (captured in testdata/ufw-delete.txt): a route rule is
|
||||
@@ -424,3 +462,41 @@ func TestARealUfwRulesetIsUfw(t *testing.T) {
|
||||
t.Error("ufw's drop chains, with ufw not known to be active, read as refusing nothing")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRetiringUfwKeepsTheMachineFromRoutingForOthers(t *testing.T) {
|
||||
// Captured on a lab machine running the container runtime with a published port: ufw active,
|
||||
// then `ufw disable`. Disabling set the forward policy the runtime had set to drop to accept.
|
||||
before, err := os.ReadFile("testdata/ufw-disable-iptables-before.txt")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
after, err := os.ReadFile("testdata/ufw-disable-iptables-after.txt")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(string(before), "-P FORWARD DROP") || !strings.Contains(string(after), "-P FORWARD ACCEPT") {
|
||||
t.Fatal("the captures no longer show ufw disable opening the forward policy")
|
||||
}
|
||||
f := &fakeUFW{installed: true, active: true, iptablesActive: string(before), iptablesInactive: string(after)}
|
||||
if err := Disable(context.Background(), f.run); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if f.active {
|
||||
t.Fatal("ufw is still active")
|
||||
}
|
||||
if f.forward != "DROP" {
|
||||
t.Errorf("the forward policy was left open after ufw was retired: %v", f.asked)
|
||||
}
|
||||
for _, a := range f.asked {
|
||||
if strings.Contains(a, "-F") || strings.Contains(a, "flush") || strings.Contains(a, "reset") {
|
||||
t.Errorf("retiring ufw flushed something: %s", a)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRetiringUfwOnAMachineWithoutIptablesStillRetiresIt(t *testing.T) {
|
||||
f := &fakeUFW{installed: true, active: true}
|
||||
if err := Disable(context.Background(), f.run); err != nil || f.active {
|
||||
t.Fatalf("disable: %v, active %v", err, f.active)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
-P INPUT ACCEPT
|
||||
-P FORWARD ACCEPT
|
||||
-P OUTPUT ACCEPT
|
||||
-N DOCKER
|
||||
-N DOCKER-BRIDGE
|
||||
-N DOCKER-CT
|
||||
-N DOCKER-FORWARD
|
||||
-N DOCKER-INTERNAL
|
||||
-N DOCKER-USER
|
||||
-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 -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.2/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 80 -j ACCEPT
|
||||
-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
|
||||
@@ -0,0 +1,119 @@
|
||||
-P INPUT DROP
|
||||
-P FORWARD DROP
|
||||
-P OUTPUT ACCEPT
|
||||
-N DOCKER
|
||||
-N DOCKER-BRIDGE
|
||||
-N DOCKER-CT
|
||||
-N DOCKER-FORWARD
|
||||
-N DOCKER-INTERNAL
|
||||
-N DOCKER-USER
|
||||
-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-logging-allow
|
||||
-N ufw-logging-deny
|
||||
-N ufw-not-local
|
||||
-N ufw-reject-forward
|
||||
-N ufw-reject-input
|
||||
-N ufw-reject-output
|
||||
-N ufw-skip-to-policy-forward
|
||||
-N ufw-skip-to-policy-input
|
||||
-N ufw-skip-to-policy-output
|
||||
-N ufw-track-forward
|
||||
-N ufw-track-input
|
||||
-N ufw-track-output
|
||||
-N ufw-user-forward
|
||||
-N ufw-user-input
|
||||
-N ufw-user-limit
|
||||
-N ufw-user-limit-accept
|
||||
-N ufw-user-logging-forward
|
||||
-N ufw-user-logging-input
|
||||
-N ufw-user-logging-output
|
||||
-N ufw-user-output
|
||||
-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.2/32 ! -i docker0 -o docker0 -p tcp -m tcp --dport 80 -j ACCEPT
|
||||
-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 ufw-after-input -p udp -m udp --dport 137 -j ufw-skip-to-policy-input
|
||||
-A ufw-after-input -p udp -m udp --dport 138 -j ufw-skip-to-policy-input
|
||||
-A ufw-after-input -p tcp -m tcp --dport 139 -j ufw-skip-to-policy-input
|
||||
-A ufw-after-input -p tcp -m tcp --dport 445 -j ufw-skip-to-policy-input
|
||||
-A ufw-after-input -p udp -m udp --dport 67 -j ufw-skip-to-policy-input
|
||||
-A ufw-after-input -p udp -m udp --dport 68 -j ufw-skip-to-policy-input
|
||||
-A ufw-after-input -m addrtype --dst-type BROADCAST -j ufw-skip-to-policy-input
|
||||
-A ufw-after-logging-forward -m limit --limit 3/min --limit-burst 10 -j LOG --log-prefix "[UFW BLOCK] "
|
||||
-A ufw-after-logging-input -m limit --limit 3/min --limit-burst 10 -j LOG --log-prefix "[UFW BLOCK] "
|
||||
-A ufw-before-forward -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
|
||||
-A ufw-before-forward -p icmp -m icmp --icmp-type 3 -j ACCEPT
|
||||
-A ufw-before-forward -p icmp -m icmp --icmp-type 11 -j ACCEPT
|
||||
-A ufw-before-forward -p icmp -m icmp --icmp-type 12 -j ACCEPT
|
||||
-A ufw-before-forward -p icmp -m icmp --icmp-type 8 -j ACCEPT
|
||||
-A ufw-before-forward -j ufw-user-forward
|
||||
-A ufw-before-input -i lo -j ACCEPT
|
||||
-A ufw-before-input -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
|
||||
-A ufw-before-input -m conntrack --ctstate INVALID -j ufw-logging-deny
|
||||
-A ufw-before-input -m conntrack --ctstate INVALID -j DROP
|
||||
-A ufw-before-input -p icmp -m icmp --icmp-type 3 -j ACCEPT
|
||||
-A ufw-before-input -p icmp -m icmp --icmp-type 11 -j ACCEPT
|
||||
-A ufw-before-input -p icmp -m icmp --icmp-type 12 -j ACCEPT
|
||||
-A ufw-before-input -p icmp -m icmp --icmp-type 8 -j ACCEPT
|
||||
-A ufw-before-input -p udp -m udp --sport 67 --dport 68 -j ACCEPT
|
||||
-A ufw-before-input -j ufw-not-local
|
||||
-A ufw-before-input -d 224.0.0.251/32 -p udp -m udp --dport 5353 -j ACCEPT
|
||||
-A ufw-before-input -d 239.255.255.250/32 -p udp -m udp --dport 1900 -j ACCEPT
|
||||
-A ufw-before-input -j ufw-user-input
|
||||
-A ufw-before-output -o lo -j ACCEPT
|
||||
-A ufw-before-output -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
|
||||
-A ufw-before-output -j ufw-user-output
|
||||
-A ufw-logging-allow -m limit --limit 3/min --limit-burst 10 -j LOG --log-prefix "[UFW ALLOW] "
|
||||
-A ufw-logging-deny -m conntrack --ctstate INVALID -m limit --limit 3/min --limit-burst 10 -j RETURN
|
||||
-A ufw-logging-deny -m limit --limit 3/min --limit-burst 10 -j LOG --log-prefix "[UFW BLOCK] "
|
||||
-A ufw-not-local -m addrtype --dst-type LOCAL -j RETURN
|
||||
-A ufw-not-local -m addrtype --dst-type MULTICAST -j RETURN
|
||||
-A ufw-not-local -m addrtype --dst-type BROADCAST -j RETURN
|
||||
-A ufw-not-local -m limit --limit 3/min --limit-burst 10 -j ufw-logging-deny
|
||||
-A ufw-not-local -j DROP
|
||||
-A ufw-skip-to-policy-forward -j DROP
|
||||
-A ufw-skip-to-policy-input -j DROP
|
||||
-A ufw-skip-to-policy-output -j ACCEPT
|
||||
-A ufw-track-output -p tcp -m conntrack --ctstate NEW -j ACCEPT
|
||||
-A ufw-track-output -p udp -m conntrack --ctstate NEW -j ACCEPT
|
||||
-A ufw-user-input -p tcp -m tcp --dport 22 -j ACCEPT
|
||||
-A ufw-user-input -p tcp -m tcp --dport 5671 -j ACCEPT
|
||||
-A ufw-user-input -i mesh0 -p tcp -m tcp --dport 5432 -j ACCEPT
|
||||
-A ufw-user-limit -m limit --limit 3/min -j LOG --log-prefix "[UFW LIMIT BLOCK] "
|
||||
-A ufw-user-limit -j REJECT --reject-with icmp-port-unreachable
|
||||
-A ufw-user-limit-accept -j ACCEPT
|
||||
Reference in New Issue
Block a user