wgengine: don't create duplicate iptables rules on Linux, clean up

Fixes #131

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

go.mod

@@ -5,6 +5,7 @@ go 1.13
 require (
 	github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 // indirect
 	github.com/apenwarr/fixconsole v0.0.0-20191012055117-5a9f6489cc29
+	github.com/coreos/go-iptables v0.4.5
 	github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 // indirect
 	github.com/gliderlabs/ssh v0.2.2
 	github.com/go-ole/go-ole v1.2.4

go.sum

@@ -19,6 +19,8 @@ github.com/blakesmith/ar v0.0.0-20190502131153-809d4375e1fb h1:m935MPodAbYS46DG4
 github.com/blakesmith/ar v0.0.0-20190502131153-809d4375e1fb/go.mod h1:PkYb9DJNAwrSvRx5DYA+gUcOIgTGVMNkfSCbZM8cWpI=
 github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e h1:hHg27A0RSSp2Om9lubZpiMgVbvn39bsUmW9U5h0twqc=
 github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e/go.mod h1:oDpT4efm8tSYHXV5tHSdRvBet/b/QzxZ+XyyPehvm3A=
+github.com/coreos/go-iptables v0.4.5 h1:DpHb9vJrZQEFMcVLFKAAGMUVX0XoRC0ptCthinRYm38=
+github.com/coreos/go-iptables v0.4.5/go.mod h1:/mVI274lEDI2ns62jHCDnCyBF9Iwsmekav8Dbxlm1MU=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

wgengine/router_linux.go

@@ -14,6 +14,7 @@
 	"path/filepath"
 	"strings"
 
+	"github.com/coreos/go-iptables/iptables"
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"github.com/tailscale/wireguard-go/wgcfg"
@@ -26,6 +27,8 @@ type linuxRouter struct {
 	tunname string
 	local   wgcfg.CIDR
 	routes  map[wgcfg.CIDR]struct{}
+
+	ipt4 *iptables.IPTables
 }
 
 func newUserspaceRouter(logf logger.Logf, _ *device.Device, tunDev tun.Device) (Router, error) {
@@ -34,9 +37,15 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tunDev tun.Device) (
 		return nil, err
 	}
 
+	ipt4, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	if err != nil {
+		return nil, err
+	}
+
 	return &linuxRouter{
 		logf:    logf,
 		tunname: tunname,
+		ipt4:    ipt4,
 	}, nil
 }
 
@@ -55,22 +64,13 @@ func (r *linuxRouter) Up() error {
 		log.Fatalf("running ip link failed: %v\n%s", err, out)
 	}
 
-	// TODO(apenwarr): This never cleans up after itself!
+	err = r.ipt4.AppendUnique("filter", "FORWARD", r.forwardRule()...)
-	out, err = cmd("iptables",
-		"-A", "FORWARD",
-		"-i", r.tunname,
-		"-j", "ACCEPT").CombinedOutput()
 	if err != nil {
-		r.logf("iptables forward failed: %v\n%s", err, out)
+		r.logf("iptables forward failed: %v", err)
 	}
-	// TODO(apenwarr): hardcoded eth0 interface is obviously not right.
+	err = r.ipt4.AppendUnique("nat", "POSTROUTING", r.natRule()...)
-	out, err = cmd("iptables",
-		"-t", "nat",
-		"-A", "POSTROUTING",
-		"-o", "eth0",
-		"-j", "MASQUERADE").CombinedOutput()
 	if err != nil {
-		r.logf("iptables nat failed: %v\n%s", err, out)
+		r.logf("iptables nat failed: %v", err)
 	}
 	return nil
 }
@@ -158,15 +158,39 @@ func (r *linuxRouter) SetRoutes(rs RouteSettings) error {
 	return errq
 }
 
+func (r *linuxRouter) forwardRule() []string {
+	return []string{
+		"-m", "comment", "--comment", "tailscale",
+		"-i", r.tunname,
+		"-j", "ACCEPT",
+	}
+}
+
+func (r *linuxRouter) natRule() []string {
+	// TODO(apenwarr): hardcoded eth0 interface is obviously not right.
+	return []string{
+		"-m", "comment", "--comment", "tailscale",
+		"-o", "eth0",
+		"-j", "MASQUERADE",
+	}
+}
+
 func (r *linuxRouter) Close() error {
 	var ret error
-	if err := r.restoreResolvConf(); err != nil {
+	set := func(err error) {
-		r.logf("failed to restore system resolv.conf: %v", err)
+		if ret == nil && err != nil {
-		if ret == nil {
 			ret = err
 		}
 	}
-	// TODO(apenwarr): clean up iptables etc.
+	if err := r.restoreResolvConf(); err != nil {
+		r.logf("failed to restore system resolv.conf: %v", err)
+		set(err)
+	}
+	set(r.ipt4.Delete("filter", "FORWARD", r.forwardRule()...))
+	set(r.ipt4.Delete("nat", "POSTROUTING", r.natRule()...))
+
+	// TODO(apenwarr): clean up routes etc.
+
 	return ret
 }