router_linux: work around terrible bugs in old iptables-compat versions.

Specifically, this sequence:
	iptables -N ts-forward
	iptables -A ts-forward -m mark --mark 0x10000 -j ACCEPT
	iptables -A FORWARD -j ts-forward
doesn't work on Debian-9-using-nftables, but this sequence:
	iptables -N ts-forward
	iptables -A FORWARD -j ts-forward
	iptables -A ts-forward -m mark --mark 0x10000 -j ACCEPT
does work.

I'm sure the reason why is totally fascinating, but it's an old version
of iptables and the bug doesn't seem to exist on modern nftables, so
let's refactor our code to add rules in the always-safe order and
pretend this never happened.

Fixes #401.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>

wgengine/router/router_linux.go

@@ -227,6 +227,12 @@ func (r *linuxRouter) setNetfilterMode(mode NetfilterMode) error {
 			if err := r.delNetfilterBase(); err != nil {
 				return err
 			}
+			if err := r.delNetfilterChains(); err != nil {
+				r.logf("note: %v", err)
+				// harmless, continue.
+				// This can happen if someone left a ref to
+				// this table somewhere else.
+			}
 		case NetfilterOn:
 			if err := r.delNetfilterHooks(); err != nil {
 				return err
@@ -234,12 +240,21 @@ func (r *linuxRouter) setNetfilterMode(mode NetfilterMode) error {
 			if err := r.delNetfilterBase(); err != nil {
 				return err
 			}
+			if err := r.delNetfilterChains(); err != nil {
+				r.logf("note: %v", err)
+				// harmless, continue.
+				// This can happen if someone left a ref to
+				// this table somewhere else.
+			}
 		}
 		r.snatSubnetRoutes = false
 	case NetfilterNoDivert:
 		switch r.netfilterMode {
 		case NetfilterOff:
 			reprocess = true
+			if err := r.addNetfilterChains(); err != nil {
+				return err
+			}
 			if err := r.addNetfilterBase(); err != nil {
 				return err
 			}
@@ -250,20 +265,40 @@ func (r *linuxRouter) setNetfilterMode(mode NetfilterMode) error {
 			}
 		}
 	case NetfilterOn:
+		// Because of bugs in old version of iptables-compat,
+		// we can't add a "-j ts-forward" rule to FORWARD
+		// while ts-forward contains an "-m mark" rule. But
+		// we can add the row *before* populating ts-forward.
+		// So we have to delNetFilterBase, then add the hooks,
+		// then re-addNetFilterBase, just in case.
 		switch r.netfilterMode {
 		case NetfilterOff:
 			reprocess = true
-			if err := r.addNetfilterBase(); err != nil {
+			if err := r.addNetfilterChains(); err != nil {
+				return err
+			}
+			if err := r.delNetfilterBase(); err != nil {
 				return err
 			}
 			if err := r.addNetfilterHooks(); err != nil {
 				return err
 			}
+			if err := r.addNetfilterBase(); err != nil {
+				return err
+			}
 			r.snatSubnetRoutes = false
 		case NetfilterNoDivert:
+			reprocess = true
+			if err := r.delNetfilterBase(); err != nil {
+				return err
+			}
 			if err := r.addNetfilterHooks(); err != nil {
 				return err
 			}
+			if err := r.addNetfilterBase(); err != nil {
+				return err
+			}
+			r.snatSubnetRoutes = false
 		}
 	default:
 		panic("unhandled netfilter mode")
@@ -618,10 +653,8 @@ func (r *linuxRouter) delBypassRule() error {
 	return rg.ErrAcc
 }
 
-// addNetfilterBase adds custom Tailscale chains to netfilter, along
+// addNetfilterChains creates custom Tailscale chains in netfilter.
-// with some basic processing rules to be supplemented by later calls
+func (r *linuxRouter) addNetfilterChains() error {
-// to other helpers.
-func (r *linuxRouter) addNetfilterBase() error {
 	create := func(table, chain string) error {
 		err := r.ipt4.ClearChain(table, chain)
 		if errCode(err) == 1 {
@@ -642,7 +675,12 @@ func (r *linuxRouter) addNetfilterBase() error {
 	if err := create("nat", "ts-postrouting"); err != nil {
 		return err
 	}
+	return nil
+}
 
+// addNetfilterBase adds with some basic processing rules to be supplemented
+// by later calls to other helpers.
+func (r *linuxRouter) addNetfilterBase() error {
 	// Only allow CGNAT range traffic to come from tailscale0. There
 	// is an exception carved out for ranges used by ChromeOS, for
 	// which we fall out of the Tailscale chain.
@@ -682,8 +720,8 @@ func (r *linuxRouter) addNetfilterBase() error {
 	return nil
 }
 
-// delNetfilterBase removes custom Tailscale chains from netfilter.
+// delNetfilterChains removes the custom Tailscale chains from netfilter.
-func (r *linuxRouter) delNetfilterBase() error {
+func (r *linuxRouter) delNetfilterChains() error {
 	del := func(table, chain string) error {
 		if err := r.ipt4.ClearChain(table, chain); err != nil {
 			if errCode(err) == 1 {
@@ -714,6 +752,34 @@ func (r *linuxRouter) delNetfilterBase() error {
 	return nil
 }
 
+// delNetfilterBase empties but does not remove custom Tailscale chains from
+// netfilter.
+func (r *linuxRouter) delNetfilterBase() error {
+	del := func(table, chain string) error {
+		if err := r.ipt4.ClearChain(table, chain); err != nil {
+			if errCode(err) == 1 {
+				// nonexistent chain. That's fine, since it's
+				// the desired state anyway.
+				return nil
+			}
+			return fmt.Errorf("flushing %s/%s: %w", table, chain, err)
+		}
+		return nil
+	}
+
+	if err := del("filter", "ts-input"); err != nil {
+		return err
+	}
+	if err := del("filter", "ts-forward"); err != nil {
+		return err
+	}
+	if err := del("nat", "ts-postrouting"); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // addNetfilterHooks inserts calls to tailscale's netfilter chains in
 // the relevant main netfilter chains. The tailscale chains must
 // already exist.