diff --git a/client/tailscale/tailscale.go b/client/tailscale/tailscale.go index f2ac76b7b..89c7bcf91 100644 --- a/client/tailscale/tailscale.go +++ b/client/tailscale/tailscale.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -198,7 +199,34 @@ func GetWaitingFile(ctx context.Context, baseName string) (rc io.ReadCloser, siz if res.StatusCode != 200 { body, _ := ioutil.ReadAll(res.Body) res.Body.Close() - return nil, 0, fmt.Errorf("expected 204 No Content; got HTTP %s: %s", res.Status, body) + return nil, 0, fmt.Errorf("HTTP %s: %s", res.Status, body) } return res.Body, res.ContentLength, nil } + +func CheckIPForwarding(ctx context.Context) error { + req, err := http.NewRequestWithContext(ctx, "GET", "http://local-tailscaled.sock/localapi/v0/check-ip-forwarding", nil) + if err != nil { + return err + } + res, err := DoLocalRequest(req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != 200 { + body, _ := ioutil.ReadAll(res.Body) + res.Body.Close() + return fmt.Errorf("HTTP %s: %s", res.Status, body) + } + var jres struct { + Warning string + } + if err := json.NewDecoder(res.Body).Decode(&jres); err != nil { + return fmt.Errorf("invalid JSON from check-ip-forwarding: %w", err) + } + if jres.Warning != "" { + return errors.New(jres.Warning) + } + return nil +} diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index 0a66eb139..e3d7da919 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -5,17 +5,14 @@ package cli import ( - "bytes" "context" "errors" "flag" "fmt" "log" "os" - "os/exec" "runtime" "sort" - "strconv" "strings" "sync" @@ -97,34 +94,6 @@ func warnf(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() { - var key string - - if runtime.GOOS == "linux" { - key = "net.ipv4.ip_forward" - } else if isBSD(runtime.GOOS) { - key = "net.inet.ip.forwarding" - } else { - return - } - - bs, err := exec.Command("sysctl", "-n", key).Output() - if err != nil { - warnf("couldn't check %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) - return - } - on, err := strconv.ParseBool(string(bytes.TrimSpace(bs))) - if err != nil { - warnf("couldn't parse %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) - return - } - if !on { - warnf("%s is disabled. Subnet routes won't work.", key) - } -} - var ( ipv4default = netaddr.MustParseIPPrefix("0.0.0.0/0") ipv6default = netaddr.MustParseIPPrefix("::/0") @@ -181,9 +150,8 @@ func runUp(ctx context.Context, args []string) error { routeMap[netaddr.MustParseIPPrefix("::/0")] = true } if len(routeMap) > 0 { - checkIPForwarding() - if isBSD(runtime.GOOS) { - warnf("Subnet routing and exit nodes only work with additional manual configuration on %v, and is not currently officially supported.", runtime.GOOS) + if err := tailscale.CheckIPForwarding(context.Background()); err != nil { + warnf("%v", err) } } routes := make([]netaddr.IPPrefix, 0, len(routeMap)) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 30daefbf0..e9554f7ee 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -66,6 +66,7 @@ type Direct struct { getMachinePrivKey func() (wgkey.Private, error) debugFlags []string keepSharerAndUserSplit bool + skipIPForwardingCheck bool mu sync.Mutex // mutex guards the following fields serverKey wgkey.Key @@ -98,6 +99,11 @@ type Options struct { // KeepSharerAndUserSplit controls whether the client // understands Node.Sharer. If false, the Sharer is mapped to the User. KeepSharerAndUserSplit bool + + // SkipIPForwardingCheck declares that the host's IP + // forwarding works and should not be double-checked by the + // controlclient package. + SkipIPForwardingCheck bool } type Decompressor interface { @@ -159,6 +165,7 @@ func NewDirect(opts Options) (*Direct, error) { debugFlags: opts.DebugFlags, keepSharerAndUserSplit: opts.KeepSharerAndUserSplit, linkMon: opts.LinkMonitor, + skipIPForwardingCheck: opts.SkipIPForwardingCheck, } if opts.Hostinfo == nil { c.SetHostinfo(NewHostinfo()) @@ -577,7 +584,8 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm OmitPeers: cb == nil, } var extraDebugFlags []string - if hostinfo != nil && c.linkMon != nil && ipForwardingBroken(hostinfo.RoutableIPs, c.linkMon.InterfaceState()) { + if hostinfo != nil && c.linkMon != nil && !c.skipIPForwardingCheck && + ipForwardingBroken(hostinfo.RoutableIPs, c.linkMon.InterfaceState()) { extraDebugFlags = append(extraDebugFlags, "warn-ip-forwarding-off") } if health.RouterHealth() != nil { @@ -1181,6 +1189,11 @@ func TrimWGConfig() opt.Bool { // and will definitely not work for the routes provided. // // It should not return false positives. +// +// TODO(bradfitz): merge this code into LocalBackend.CheckIPForwarding +// and change controlclient.Options.SkipIPForwardingCheck into a +// func([]netaddr.IPPrefix) error signature instead. Then we only have +// one copy of this code. func ipForwardingBroken(routes []netaddr.IPPrefix, state *interfaces.State) bool { if len(routes) == 0 { // Nothing to route, so no need to warn. diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 202a53afa..cc9e1113c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -12,6 +12,7 @@ "io" "net" "os" + "os/exec" "path/filepath" "runtime" "strconv" @@ -640,6 +641,10 @@ func (b *LocalBackend) Start(opts ipn.Options) error { DiscoPublicKey: discoPublic, DebugFlags: controlDebugFlags, LinkMonitor: b.e.GetLinkMonitor(), + + // Don't warn about broken Linux IP forwading when + // netstack is being used. + SkipIPForwardingCheck: wgengine.IsNetstack(b.e), }) if err != nil { return err @@ -2018,3 +2023,45 @@ func (b *LocalBackend) OpenFile(name string) (rc io.ReadCloser, size int64, err } return apiSrv.OpenFile(name) } + +func isBSD(s string) bool { + return s == "dragonfly" || s == "freebsd" || s == "netbsd" || s == "openbsd" +} + +func (b *LocalBackend) CheckIPForwarding() error { + if wgengine.IsNetstack(b.e) { + return nil + } + if isBSD(runtime.GOOS) { + //lint:ignore ST1005 output to users as is + return fmt.Errorf("Subnet routing and exit nodes only work with additional manual configuration on %v, and is not currently officially supported.", runtime.GOOS) + } + + var keys []string + + if runtime.GOOS == "linux" { + keys = append(keys, "net.ipv4.ip_forward", "net.ipv6.conf.all.forwarding") + } else if isBSD(runtime.GOOS) { + keys = append(keys, "net.inet.ip.forwarding") + } else { + return nil + } + + for _, key := range keys { + bs, err := exec.Command("sysctl", "-n", key).Output() + if err != nil { + //lint:ignore ST1005 output to users as is + return fmt.Errorf("couldn't check %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) + } + on, err := strconv.ParseBool(string(bytes.TrimSpace(bs))) + if err != nil { + //lint:ignore ST1005 output to users as is + return fmt.Errorf("couldn't parse %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) + } + if !on { + //lint:ignore ST1005 output to users as is + return fmt.Errorf("%s is disabled. Subnet routes won't work.", key) + } + } + return nil +} diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index b88dc3643..ce257bf39 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -67,6 +67,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.serveGoroutines(w, r) case "/localapi/v0/status": h.serveStatus(w, r) + case "/localapi/v0/check-ip-forwarding": + h.serveCheckIPForwarding(w, r) default: io.WriteString(w, "tailscaled\n") } @@ -121,6 +123,23 @@ func (h *Handler) serveGoroutines(w http.ResponseWriter, r *http.Request) { w.Write(buf) } +func (h *Handler) serveCheckIPForwarding(w http.ResponseWriter, r *http.Request) { + if !h.PermitRead { + http.Error(w, "IP forwarding check access denied", http.StatusForbidden) + return + } + var warning string + if err := h.b.CheckIPForwarding(); err != nil { + warning = err.Error() + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(struct { + Warning string + }{ + Warning: warning, + }) +} + func (h *Handler) serveStatus(w http.ResponseWriter, r *http.Request) { if !h.PermitRead { http.Error(w, "status access denied", http.StatusForbidden)