diff --git a/cmd/tailscale/tailscale.go b/cmd/tailscale/tailscale.go index b63935af1..eeaf885b4 100644 --- a/cmd/tailscale/tailscale.go +++ b/cmd/tailscale/tailscale.go @@ -28,6 +28,7 @@ import ( "tailscale.com/paths" "tailscale.com/safesocket" "tailscale.com/tailcfg" + "tailscale.com/wgengine/router" ) // globalStateKey is the ipn.StateKey that tailscaled loads on @@ -59,8 +60,7 @@ func main() { if runtime.GOOS == "linux" { upf.StringVar(&upArgs.advertiseRoutes, "advertise-routes", "", "routes to advertise to other nodes (comma-separated, e.g. 10.0.0.0/8,192.168.0.0/24)") upf.BoolVar(&upArgs.noSNAT, "no-snat", false, "disable SNAT of traffic to local routes advertised with -advertise-routes") - upf.BoolVar(&upArgs.noNetfilterCalls, "no-netfilter-calls", false, "don't call Tailscale netfilter chains from the main netfilter chains") - upf.BoolVar(&upArgs.noNetfilter, "no-netfilter", false, "disable all netfilter rule management") + upf.StringVar(&upArgs.netfilterMode, "netfilter-mode", "on", "netfilter mode (one of on, nodivert, off)") } upCmd := &ffcli.Command{ Name: "up", @@ -104,16 +104,15 @@ change in the future. } var upArgs struct { - server string - acceptRoutes bool - noSingleRoutes bool - shieldsUp bool - advertiseRoutes string - advertiseTags string - noSNAT bool - noNetfilterCalls bool - noNetfilter bool - authKey string + server string + acceptRoutes bool + noSingleRoutes bool + shieldsUp bool + advertiseRoutes string + advertiseTags string + noSNAT bool + netfilterMode string + authKey string } // parseIPOrCIDR parses an IP address or a CIDR prefix. If the input @@ -139,6 +138,10 @@ func parseIPOrCIDR(s string) (wgcfg.CIDR, bool) { } } +func warning(format string, args ...interface{}) { + fmt.Printf("Warning: "+format+"\n", args...) +} + // checkIPForwarding prints warnings on linux if IP forwarding is not // enabled, or if we were unable to verify the state of IP forwarding. func checkIPForwarding() { @@ -147,16 +150,16 @@ func checkIPForwarding() { } bs, err := ioutil.ReadFile("/proc/sys/net/ipv4/ip_forward") if err != nil { - fmt.Printf("Warning: couldn't check if IP forwarding is enabled (%v). IP forwarding must be enabled for subnet routes to work.", err) + warning("couldn't check if IP forwarding is enabled (%v). IP forwarding must be enabled for subnet routes to work.", err) return } on, err := strconv.ParseBool(string(bytes.TrimSpace(bs))) if err != nil { - fmt.Printf("Warning: couldn't check if IP forwarding is enabled (%v). IP forwarding must be enabled for subnet routes to work.", err) + warning("couldn't check if IP forwarding is enabled (%v). IP forwarding must be enabled for subnet routes to work.", err) return } if !on { - fmt.Printf("Warning: IP forwarding is disabled, subnet routes will not work.") + warning("IP forwarding is disabled, subnet routes will not work.") } } @@ -200,8 +203,20 @@ func runUp(ctx context.Context, args []string) error { prefs.AdvertiseRoutes = routes prefs.AdvertiseTags = tags prefs.NoSNAT = upArgs.noSNAT - prefs.NoNetfilter = upArgs.noNetfilter - prefs.NoNetfilterCalls = upArgs.noNetfilterCalls + if runtime.GOOS == "linux" { + switch upArgs.netfilterMode { + case "on": + prefs.NetfilterMode = router.NetfilterOn + case "nodivert": + prefs.NetfilterMode = router.NetfilterNoDivert + warning("netfilter in nodivert mode, you must add calls to Tailscale netfilter chains manually") + case "off": + prefs.NetfilterMode = router.NetfilterOff + warning("netfilter management disabled, you must write a secure packet filter yourself") + default: + log.Fatalf("invalid value --netfilter-mode: %q", upArgs.netfilterMode) + } + } c, bc, ctx, cancel := connect(ctx) defer cancel() diff --git a/ipn/local.go b/ipn/local.go index 6217d80f3..0164fe5be 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -739,14 +739,7 @@ func routerConfig(cfg *wgcfg.Config, prefs *Prefs, dnsDomains []string) *router. DNSDomains: dnsDomains, SubnetRoutes: wgCIDRToNetaddr(prefs.AdvertiseRoutes), SNATSubnetRoutes: !prefs.NoSNAT, - } - switch { - case prefs.NoNetfilter: - rs.NetfilterMode = router.NetfilterOff - case prefs.NoNetfilterCalls: - rs.NetfilterMode = router.NetfilterNoDivert - default: - rs.NetfilterMode = router.NetfilterOn + NetfilterMode: prefs.NetfilterMode, } for _, peer := range cfg.Peers { diff --git a/ipn/prefs.go b/ipn/prefs.go index 16be292f4..a53fca052 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -15,6 +15,7 @@ import ( "github.com/tailscale/wireguard-go/wgcfg" "tailscale.com/atomicfile" "tailscale.com/control/controlclient" + "tailscale.com/wgengine/router" ) // Prefs are the user modifiable settings of the Tailscale node agent. @@ -79,16 +80,9 @@ type Prefs struct { // // Linux-only. NoSNAT bool - // NoNetfilter, if set, disables all management of firewall rules - // for Tailscale traffic. The resulting configuration is not - // secure, and it is the user's responsibility to correct that. - NoNetfilter bool - // NoNetfilterDivert, if set, disables calling Tailscale netfilter - // chains from the main netfilter chains, but still manages the - // contents of the Tailscale chains. The resulting configuration - // is not secure, and it is the user's responsibility to insert - // calls to Tailscale's chains at the right place. - NoNetfilterCalls bool + // NetfilterMode specifies how much to manage netfilter rules for + // Tailscale, if at all. + NetfilterMode router.NetfilterMode // The Persist field is named 'Config' in the file for backward // compatibility with earlier versions. @@ -108,9 +102,9 @@ func (p *Prefs) Pretty() string { } else { pp = "Persist=nil" } - return fmt.Sprintf("Prefs{ra=%v mesh=%v dns=%v want=%v notepad=%v derp=%v shields=%v routes=%v snat=%v nf=%v nfd=%v %v}", + return fmt.Sprintf("Prefs{ra=%v mesh=%v dns=%v want=%v notepad=%v derp=%v shields=%v routes=%v snat=%v nf=%v %v}", p.RouteAll, p.AllowSingleHosts, p.CorpDNS, p.WantRunning, - p.NotepadURLs, !p.DisableDERP, p.ShieldsUp, p.AdvertiseRoutes, !p.NoSNAT, !p.NoNetfilter, !p.NoNetfilterCalls, pp) + p.NotepadURLs, !p.DisableDERP, p.ShieldsUp, p.AdvertiseRoutes, !p.NoSNAT, p.NetfilterMode, pp) } func (p *Prefs) ToBytes() []byte { @@ -139,8 +133,7 @@ func (p *Prefs) Equals(p2 *Prefs) bool { p.DisableDERP == p2.DisableDERP && p.ShieldsUp == p2.ShieldsUp && p.NoSNAT == p2.NoSNAT && - p.NoNetfilter == p2.NoNetfilter && - p.NoNetfilterCalls == p2.NoNetfilterCalls && + p.NetfilterMode == p2.NetfilterMode && compareIPNets(p.AdvertiseRoutes, p2.AdvertiseRoutes) && compareStrings(p.AdvertiseTags, p2.AdvertiseTags) && p.Persist.Equals(p2.Persist) @@ -180,6 +173,7 @@ func NewPrefs() *Prefs { AllowSingleHosts: true, CorpDNS: true, WantRunning: true, + NetfilterMode: router.NetfilterOn, } } diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index d5c1deae4..a7510a283 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -11,6 +11,7 @@ import ( "github.com/tailscale/wireguard-go/wgcfg" "tailscale.com/control/controlclient" "tailscale.com/tstest" + "tailscale.com/wgengine/router" ) func fieldsOf(t reflect.Type) (fields []string) { @@ -23,7 +24,7 @@ func fieldsOf(t reflect.Type) (fields []string) { func TestPrefsEqual(t *testing.T) { tstest.PanicOnLog() - prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "ShieldsUp", "AdvertiseTags", "NotepadURLs", "DisableDERP", "AdvertiseRoutes", "NoSNAT", "NoNetfilter", "NoNetfilterCalls", "Persist"} + prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "ShieldsUp", "AdvertiseTags", "NotepadURLs", "DisableDERP", "AdvertiseRoutes", "NoSNAT", "NetfilterMode", "Persist"} if have := fieldsOf(reflect.TypeOf(Prefs{})); !reflect.DeepEqual(have, prefsHandles) { t.Errorf("Prefs.Equal check might be out of sync\nfields: %q\nhandled: %q\n", have, prefsHandles) @@ -174,24 +175,13 @@ func TestPrefsEqual(t *testing.T) { }, { - &Prefs{NoNetfilter: true}, - &Prefs{NoNetfilter: false}, + &Prefs{NetfilterMode: router.NetfilterOff}, + &Prefs{NetfilterMode: router.NetfilterOn}, false, }, { - &Prefs{NoNetfilter: true}, - &Prefs{NoNetfilter: true}, - true, - }, - - { - &Prefs{NoNetfilterCalls: true}, - &Prefs{NoNetfilterCalls: false}, - false, - }, - { - &Prefs{NoNetfilterCalls: true}, - &Prefs{NoNetfilterCalls: true}, + &Prefs{NetfilterMode: router.NetfilterOn}, + &Prefs{NetfilterMode: router.NetfilterOn}, true, }, diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 0ddc4038a..64354f5e7 100644 --- a/wgengine/router/router.go +++ b/wgengine/router/router.go @@ -35,6 +35,8 @@ func New(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, err return newUserspaceRouter(logf, wgdev, tundev) } +// NetfilterMode is the firewall management mode to use when +// programming the Linux network stack. type NetfilterMode int const ( @@ -43,6 +45,19 @@ const ( NetfilterOn // manage tailscale chains and call them from main chains ) +func (m NetfilterMode) String() string { + switch m { + case NetfilterOff: + return "off" + case NetfilterNoDivert: + return "nodivert" + case NetfilterOn: + return "on" + default: + return "???" + } +} + // Config is the subset of Tailscale configuration that is relevant to // the OS's network stack. type Config struct { diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index 38265c2cd..ba53505da 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -718,6 +718,24 @@ func (r *linuxRouter) delNetfilterHooks() error { del := func(table, chain string) error { tsChain := tsChain(chain) + chains, err := r.ipt4.ListChains(table) + if err != nil { + return fmt.Errorf("listing iptables chains: %v", err) + } + found := false + for _, chain := range chains { + if chain == tsChain { + found = true + break + } + } + if !found { + // The divert rule can't exist if the chain doesn't exist, + // and querying for a jump to a non-existent chain errors + // out. + return nil + } + args := []string{"-j", tsChain} exists, err := r.ipt4.Exists(table, chain, args...) if err != nil { diff --git a/wgengine/router/router_linux_test.go b/wgengine/router/router_linux_test.go index 82609d4b3..820e36f4b 100644 --- a/wgengine/router/router_linux_test.go +++ b/wgengine/router/router_linux_test.go @@ -49,7 +49,8 @@ ip rule add fwmark 0x20000/0x20000 priority 10000 table main suppress_ifgroup 10 { name: "local addr only", in: &Config{ - LocalAddrs: mustCIDRs("100.101.102.103/10"), + LocalAddrs: mustCIDRs("100.101.102.103/10"), + NetfilterMode: NetfilterOff, }, want: ` up @@ -61,8 +62,9 @@ ip rule add fwmark 0x20000/0x20000 priority 10000 table main suppress_ifgroup 10 { name: "addr and routes", in: &Config{ - LocalAddrs: mustCIDRs("100.101.102.103/10"), - Routes: mustCIDRs("100.100.100.100/32", "192.168.16.0/24"), + LocalAddrs: mustCIDRs("100.101.102.103/10"), + Routes: mustCIDRs("100.100.100.100/32", "192.168.16.0/24"), + NetfilterMode: NetfilterOff, }, want: ` up @@ -76,9 +78,10 @@ ip rule add fwmark 0x20000/0x20000 priority 10000 table main suppress_ifgroup 10 { name: "addr and routes and subnet routes", in: &Config{ - LocalAddrs: mustCIDRs("100.101.102.103/10"), - Routes: mustCIDRs("100.100.100.100/32", "192.168.16.0/24"), - SubnetRoutes: mustCIDRs("200.0.0.0/8"), + LocalAddrs: mustCIDRs("100.101.102.103/10"), + Routes: mustCIDRs("100.100.100.100/32", "192.168.16.0/24"), + SubnetRoutes: mustCIDRs("200.0.0.0/8"), + NetfilterMode: NetfilterOff, }, want: ` up