diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 7c19ebb42..410ae00bc 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -799,8 +799,9 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo if runtime.GOOS == "plan9" { // TODO(bradfitz): why don't we do this on all platforms? + // TODO(barnstar): we do it on sandboxed darwin now // We should. Doing it just on plan9 for now conservatively. - sys.NetMon.Get().SetTailscaleInterfaceName(devName) + netmon.SetTailscaleInterfaceProps(devName, 0) } r, err := router.New(logf, dev, sys.NetMon.Get(), sys.HealthTracker.Get(), sys.Bus.Get()) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 44b12826b..066d8ba0a 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -565,7 +565,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo // Call our linkChange code once with the current state. // Following changes are triggered via the eventbus. - cd, err := netmon.NewChangeDelta(nil, b.interfaceState, false, netMon.TailscaleInterfaceName(), false) + cd, err := netmon.NewChangeDelta(nil, b.interfaceState, false, false) if err != nil { b.logf("[unexpected] setting initial netmon state failed: %v", err) } else { @@ -5321,7 +5321,11 @@ func (b *LocalBackend) initPeerAPIListenerLocked() { var err error skipListen := i > 0 && isNetstack if !skipListen { - ln, err = ps.listen(a.Addr(), b.interfaceState.TailscaleInterfaceIndex) + // We don't care about the error here. Not all platforms set this. + // If ps.listen needs it, it will check for zero values and error out. + tsIfIndex, _ := netmon.TailscaleInterfaceIndex() + + ln, err = ps.listen(a.Addr(), tsIfIndex) if err != nil { if peerAPIListenAsync { b.logf("[v1] possibly transient peerapi listen(%q) error, will try again on linkChange: %v", a.Addr(), err) @@ -5329,6 +5333,11 @@ func (b *LocalBackend) initPeerAPIListenerLocked() { // ("peerAPIListeners too low"). continue } + // Sandboxed macOS specifically requires the interface index to be non-zero. + if version.IsSandboxedMacOS() && tsIfIndex == 0 { + b.logf("[v1] peerapi listen(%q) error: interface index is 0 on darwin; try restarting tailscaled", a.Addr()) + continue + } b.logf("[unexpected] peerapi listen(%q) error: %v", a.Addr(), err) continue } diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 20c61c0ec..318d9bf6b 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -41,6 +41,8 @@ import ( "tailscale.com/wgengine/filter" ) +// initListenConfig, if non-nil, is called during peerAPIListener setup. It is used only +// on iOS and macOS to set socket options to bind the listener to the Tailscale interface. var initListenConfig func(config *net.ListenConfig, addr netip.Addr, tunIfIndex int) error // peerDNSQueryHandler is implemented by tsdns.Resolver. @@ -69,6 +71,13 @@ func (s *peerAPIServer) listen(ip netip.Addr, tunIfIndex int) (ln net.Listener, // On iOS/macOS, this sets the lc.Control hook to // setsockopt the interface index to bind to, to get // out of the network sandbox. + + // A zero tunIfIndex is invalid for peerapi. A zero value will not get us + // out of the network sandbox. Caller should log and retry. + if tunIfIndex == 0 { + return nil, fmt.Errorf("peerapi: cannot listen on %s with tunIfIndex 0", ipStr) + } + if err := initListenConfig(&lc, ip, tunIfIndex); err != nil { return nil, err } diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 4d6055bbd..9fca3db69 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -36,6 +36,7 @@ import ( "github.com/pires/go-proxyproto" "go4.org/mem" "tailscale.com/ipn" + "tailscale.com/net/netmon" "tailscale.com/net/netutil" "tailscale.com/syncs" "tailscale.com/tailcfg" @@ -166,16 +167,24 @@ func (s *localListener) Run() { var lc net.ListenConfig if initListenConfig != nil { + ifIndex, err := netmon.TailscaleInterfaceIndex() + if err != nil { + s.logf("localListener failed to get Tailscale interface index %v, backing off: %v", s.ap, err) + s.bo.BackOff(s.ctx, err) + continue + } + // On macOS, this sets the lc.Control hook to // setsockopt the interface index to bind to. This is - // required by the network sandbox to allow binding to - // a specific interface. Without this hook, the system - // chooses a default interface to bind to. - if err := initListenConfig(&lc, ip, s.b.interfaceState.TailscaleInterfaceIndex); err != nil { + // required by the network sandbox which will not automatically + // bind to the tailscale interface to prevent routing loops. + // Explicit binding allows us to bypass that restriction. + if err := initListenConfig(&lc, ip, ifIndex); err != nil { s.logf("localListener failed to init listen config %v, backing off: %v", s.ap, err) s.bo.BackOff(s.ctx, err) continue } + // On macOS (AppStore or macsys) and if we're binding to a privileged port, if version.IsSandboxedMacOS() && s.ap.Port() < 1024 { // On macOS, we need to bind to ""/all-interfaces due to diff --git a/net/netmon/interfaces.go b/net/netmon/interfaces.go new file mode 100644 index 000000000..4cf93973c --- /dev/null +++ b/net/netmon/interfaces.go @@ -0,0 +1,103 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package netmon + +import ( + "errors" + "net" + + "tailscale.com/syncs" +) + +type ifProps struct { + mu syncs.Mutex + name string // interface name, if known/set + index int // interface index, if known/set +} + +// tsIfProps tracks the properties (name and index) of the tailscale interface. +// There is only one tailscale interface per tailscaled instance. +var tsIfProps ifProps + +func (p *ifProps) tsIfName() string { + p.mu.Lock() + defer p.mu.Unlock() + return p.name +} + +func (p *ifProps) tsIfIndex() int { + p.mu.Lock() + defer p.mu.Unlock() + return p.index +} + +func (p *ifProps) set(ifName string, ifIndex int) { + p.mu.Lock() + defer p.mu.Unlock() + p.name = ifName + p.index = ifIndex +} + +// TODO (barnstar): This doesn't need the Monitor receiver anymore but we're +// keeping it for API compatibility to avoid a breaking change.  This can be +// removed when the various clients have switched to SetTailscaleInterfaceProps +func (m *Monitor) SetTailscaleInterfaceName(ifName string) { + SetTailscaleInterfaceProps(ifName, 0) +} + +// SetTailscaleInterfaceProps sets the name of the Tailscale interface and +// its index for use by various listeners/dialers. If the index is zero, +// an attempt will be made to look it up by name. This makes no attempt +// to validate that the interface exists at the time of calling. +// +// If this method is called, it is the responsibility of the caller to +// update the interface name and index if they change. +// +// This should be called as early as possible during tailscaled startup. +func SetTailscaleInterfaceProps(ifName string, ifIndex int) { + if ifIndex != 0 { + tsIfProps.set(ifName, ifIndex) + return + } + + ifaces, err := net.Interfaces() + if err != nil { + return + } + + for _, iface := range ifaces { + if iface.Name == ifName { + ifIndex = iface.Index + break + } + } + + tsIfProps.set(ifName, ifIndex) +} + +// TailscaleInterfaceName returns the name of the Tailscale interface. +// For example, "tailscale0", "tun0", "utun3", etc or an error if unset. +// +// Callers must handle errors, as the Tailscale interface +// name may not be set in some environments. +func TailscaleInterfaceName() (string, error) { + name := tsIfProps.tsIfName() + if name == "" { + return "", errors.New("Tailscale interface name not set") + } + return name, nil +} + +// TailscaleInterfaceIndex returns the index of the Tailscale interface or +// an error if unset. +// +// Callers must handle errors, as the Tailscale interface +// index may not be set in some environments. +func TailscaleInterfaceIndex() (int, error) { + index := tsIfProps.tsIfIndex() + if index == 0 { + return 0, errors.New("Tailscale interface index not set") + } + return index, nil +} diff --git a/net/netmon/loghelper_test.go b/net/netmon/loghelper_test.go index 968c2fd41..468a12505 100644 --- a/net/netmon/loghelper_test.go +++ b/net/netmon/loghelper_test.go @@ -64,7 +64,7 @@ func syncTestLinkChangeLogLimiter(t *testing.T) { // InjectEvent doesn't work because it's not a major event, so we // instead inject the event ourselves. injector := eventbustest.NewInjector(t, bus) - cd, err := NewChangeDelta(nil, &State{}, true, "tailscale0", true) + cd, err := NewChangeDelta(nil, &State{}, true, true) if err != nil { t.Fatal(err) } diff --git a/net/netmon/netmon.go b/net/netmon/netmon.go index 49fb426ae..e18bc392d 100644 --- a/net/netmon/netmon.go +++ b/net/netmon/netmon.go @@ -78,8 +78,7 @@ type Monitor struct { goroutines sync.WaitGroup wallTimer *time.Timer // nil until Started; re-armed AfterFunc per tick lastWall time.Time - timeJumped bool // whether we need to send a changed=true after a big time jump - tsIfName string // tailscale interface name, if known/set ("tailscale0", "utun3", ...) + timeJumped bool // whether we need to send a changed=true after a big time jump } // ChangeFunc is a callback function registered with Monitor that's called when the @@ -103,10 +102,6 @@ type ChangeDelta struct { // come out of sleep. TimeJumped bool - // The tailscale interface name, e.g. "tailscale0", "utun3", etc. Not all - // platforms know this or set it. Copied from netmon.Monitor.tsIfName. - TailscaleIfaceName string - DefaultRouteInterface string // Computed Fields @@ -134,12 +129,11 @@ func (cd *ChangeDelta) CurrentState() *State { // NewChangeDelta builds a ChangeDelta and eagerly computes the cached fields. // forceViability, if true, forces DefaultInterfaceMaybeViable to be true regardless of the // actual state of the default interface. This is useful in testing. -func NewChangeDelta(old, new *State, timeJumped bool, tsIfName string, forceViability bool) (*ChangeDelta, error) { +func NewChangeDelta(old, new *State, timeJumped bool, forceViability bool) (*ChangeDelta, error) { cd := ChangeDelta{ - old: old, - new: new, - TimeJumped: timeJumped, - TailscaleIfaceName: tsIfName, + old: old, + new: new, + TimeJumped: timeJumped, } if cd.new == nil { @@ -162,8 +156,10 @@ func NewChangeDelta(old, new *State, timeJumped bool, tsIfName string, forceViab cd.DefaultRouteInterface = new.DefaultRouteInterface defIf := new.Interface[cd.DefaultRouteInterface] + tsIfName, err := TailscaleInterfaceName() + // The default interface is not viable if it is down or it is the Tailscale interface itself. - if !forceViability && (!defIf.IsUp() || cd.DefaultRouteInterface == tsIfName) { + if !forceViability && (!defIf.IsUp() || (err == nil && cd.DefaultRouteInterface == tsIfName)) { cd.DefaultInterfaceMaybeViable = false } else { cd.DefaultInterfaceMaybeViable = true @@ -223,10 +219,11 @@ func (cd *ChangeDelta) isInterestingInterfaceChange() bool { } // Compare interfaces in both directions. Old to new and new to old. + tsIfName, ifNameErr := TailscaleInterfaceName() for iname, oldInterface := range cd.old.Interface { - if iname == cd.TailscaleIfaceName { - // Ignore changes in the Tailscale interface itself. + if ifNameErr == nil && iname == tsIfName { + // Ignore changes in the Tailscale interface itself continue } oldIps := filterRoutableIPs(cd.old.InterfaceIPs[iname]) @@ -259,7 +256,8 @@ func (cd *ChangeDelta) isInterestingInterfaceChange() bool { } for iname, newInterface := range cd.new.Interface { - if iname == cd.TailscaleIfaceName { + if ifNameErr == nil && iname == tsIfName { + // Ignore changes in the Tailscale interface itself continue } newIps := filterRoutableIPs(cd.new.InterfaceIPs[iname]) @@ -360,24 +358,7 @@ func (m *Monitor) InterfaceState() *State { } func (m *Monitor) interfaceStateUncached() (*State, error) { - return getState(m.tsIfName) -} - -// SetTailscaleInterfaceName sets the name of the Tailscale interface. For -// example, "tailscale0", "tun0", "utun3", etc. -// -// This must be called only early in tailscaled startup before the monitor is -// used. -func (m *Monitor) SetTailscaleInterfaceName(ifName string) { - m.mu.Lock() - defer m.mu.Unlock() - m.tsIfName = ifName -} - -func (m *Monitor) TailscaleInterfaceName() string { - m.mu.Lock() - defer m.mu.Unlock() - return m.tsIfName + return getState(tsIfProps.tsIfName()) } // GatewayAndSelfIP returns the current network's default gateway, and @@ -598,7 +579,7 @@ func (m *Monitor) handlePotentialChange(newState *State, forceCallbacks bool) { return } - delta, err := NewChangeDelta(oldState, newState, timeJumped, m.tsIfName, false) + delta, err := NewChangeDelta(oldState, newState, timeJumped, false) if err != nil { m.logf("[unexpected] error creating ChangeDelta: %v", err) return diff --git a/net/netmon/netmon_test.go b/net/netmon/netmon_test.go index 8fbf512dd..50519b4a9 100644 --- a/net/netmon/netmon_test.go +++ b/net/netmon/netmon_test.go @@ -159,7 +159,7 @@ func TestMonitorMode(t *testing.T) { // tests (*ChangeDelta).RebindRequired func TestRebindRequired(t *testing.T) { - // s1 cannot be nil by definition + // s1 must not be nil by definition tests := []struct { name string s1, s2 *State @@ -478,9 +478,11 @@ func TestRebindRequired(t *testing.T) { withIsInterestingInterface(t, func(ni Interface, pfxs []netip.Prefix) bool { return !strings.HasPrefix(ni.Name, "boring") }) + saveAndRestoreTailscaleIfaceProps(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Populate dummy interfaces where missing. for _, s := range []*State{tt.s1, tt.s2} { if s == nil { @@ -495,7 +497,8 @@ func TestRebindRequired(t *testing.T) { } } - cd, err := NewChangeDelta(tt.s1, tt.s2, false, tt.tsIfName, true) + SetTailscaleInterfaceProps(tt.tsIfName, 1) + cd, err := NewChangeDelta(tt.s1, tt.s2, false, true) if err != nil { t.Fatalf("NewChangeDelta error: %v", err) } @@ -507,6 +510,15 @@ func TestRebindRequired(t *testing.T) { } } +func saveAndRestoreTailscaleIfaceProps(t *testing.T) { + t.Helper() + index, _ := TailscaleInterfaceIndex() + name, _ := TailscaleInterfaceName() + t.Cleanup(func() { + SetTailscaleInterfaceProps(name, index) + }) +} + func withIsInterestingInterface(t *testing.T, fn func(Interface, []netip.Prefix) bool) { t.Helper() old := IsInterestingInterface diff --git a/net/netmon/state.go b/net/netmon/state.go index aefbbb22d..79dd8a01b 100644 --- a/net/netmon/state.go +++ b/net/netmon/state.go @@ -287,9 +287,6 @@ type State struct { // PAC is the URL to the Proxy Autoconfig URL, if applicable. PAC string - - // TailscaleInterfaceIndex is the index of the Tailscale interface - TailscaleInterfaceIndex int } func (s *State) String() string { @@ -473,15 +470,22 @@ func hasTailscaleIP(pfxs []netip.Prefix) bool { } func isTailscaleInterface(name string, ips []netip.Prefix) bool { + // Sandboxed macOS and Plan9 (and anything else that explicitly calls SetTailscaleInterfaceProps). + tsIfName, err := TailscaleInterfaceName() + if err == nil { + // If we've been told the Tailscale interface name, use that. + return name == tsIfName + } + + // The sandboxed app should (as of 1.92) set the tun interface name via SetTailscaleInterfaceProps + // early in the startup process. The non-sandboxed app does not. + // TODO (barnstar): If Wireguard created the tun device on darwin, it should know the name and it should + // be explicitly set instead checking addresses here. if runtime.GOOS == "darwin" && strings.HasPrefix(name, "utun") && hasTailscaleIP(ips) { - // On macOS in the sandboxed app (at least as of - // 2021-02-25), we often see two utun devices - // (e.g. utun4 and utun7) with the same IPv4 and IPv6 - // addresses. Just remove all utun devices with - // Tailscale IPs until we know what's happening with - // macOS NetworkExtensions and utun devices. return true } + + // Windows, Linux... return name == "Tailscale" || // as it is on Windows strings.HasPrefix(name, "tailscale") // TODO: use --tun flag value, etc; see TODO in method doc } @@ -505,18 +509,15 @@ func getState(optTSInterfaceName string) (*State, error) { s.Interface[ni.Name] = ni s.InterfaceIPs[ni.Name] = append(s.InterfaceIPs[ni.Name], pfxs...) - // Skip uninteresting interfaces. + // Skip uninteresting interfaces if IsInterestingInterface != nil && !IsInterestingInterface(ni, pfxs) { return } - if isTailscaleInterface(ni.Name, pfxs) { - s.TailscaleInterfaceIndex = ni.Index - } - if !ifUp || isTSInterfaceName || isTailscaleInterface(ni.Name, pfxs) { return } + for _, pfx := range pfxs { if pfx.Addr().IsLoopback() { continue @@ -803,8 +804,7 @@ func (m *Monitor) HasCGNATInterface() (bool, error) { hasCGNATInterface := false cgnatRange := tsaddr.CGNATRange() err := ForeachInterface(func(i Interface, pfxs []netip.Prefix) { - isTSInterfaceName := m.tsIfName != "" && i.Name == m.tsIfName - if hasCGNATInterface || !i.IsUp() || isTSInterfaceName || isTailscaleInterface(i.Name, pfxs) { + if hasCGNATInterface || !i.IsUp() || isTailscaleInterface(i.Name, pfxs) { return } for _, pfx := range pfxs {