From 11ece02f5246dee65418bb87c702f230ba2163f0 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 23 Aug 2023 11:48:05 -0700 Subject: [PATCH] net/{interfaces,netmon}: remove "interesting", EqualFiltered API This removes a lot of API from net/interfaces (including all the filter types, EqualFiltered, active Tailscale interface func, etc) and moves the "major" change detection to net/netmon which knows more about the world and the previous/new states. Updates #9040 Change-Id: I7fe66a23039c6347ae5458745b709e7ebdcce245 Signed-off-by: Brad Fitzpatrick --- net/dns/nm.go | 21 +-- net/interfaces/interfaces.go | 214 ++++++++++++------------------ net/interfaces/interfaces_test.go | 168 ++++++++++++++++------- net/netmon/netmon.go | 100 +++++++++++++- net/netmon/netmon_test.go | 132 ++++++++++++++++++ net/netns/netns_darwin.go | 31 ++++- net/netns/netns_darwin_test.go | 2 +- 7 files changed, 474 insertions(+), 194 deletions(-) diff --git a/net/dns/nm.go b/net/dns/nm.go index 664297c63..4d9fbca66 100644 --- a/net/dns/nm.go +++ b/net/dns/nm.go @@ -8,13 +8,14 @@ package dns import ( "context" "fmt" + "net" "net/netip" "sort" "time" "github.com/godbus/dbus/v5" "github.com/josharian/native" - "tailscale.com/net/interfaces" + "tailscale.com/net/tsaddr" "tailscale.com/util/dnsname" ) @@ -139,14 +140,18 @@ func (m *nmManager) trySet(ctx context.Context, config OSConfig) error { // tell it explicitly to keep it. Read out the current interface // settings and mirror them out to NetworkManager. var addrs6 []map[string]any - addrs, _, err := interfaces.Tailscale() - if err == nil { + if tsIf, err := net.InterfaceByName(m.interfaceName); err == nil { + addrs, _ := tsIf.Addrs() for _, a := range addrs { - if a.Is6() { - addrs6 = append(addrs6, map[string]any{ - "address": a.String(), - "prefix": uint32(128), - }) + if ipnet, ok := a.(*net.IPNet); ok { + nip, ok := netip.AddrFromSlice(ipnet.IP) + nip = nip.Unmap() + if ok && tsaddr.IsTailscaleIP(nip) && nip.Is6() { + addrs6 = append(addrs6, map[string]any{ + "address": nip.String(), + "prefix": uint32(128), + }) + } } } } diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index 792385317..1e00368d1 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -11,6 +11,7 @@ import ( "net/http" "net/netip" "runtime" + "slices" "sort" "strings" @@ -24,49 +25,6 @@ import ( // which HTTP proxy the system should use. var LoginEndpointForProxyDetermination = "https://controlplane.tailscale.com/" -// Tailscale returns the current machine's Tailscale interface, if any. -// If none is found, all zero values are returned. -// A non-nil error is only returned on a problem listing the system interfaces. -func Tailscale() ([]netip.Addr, *net.Interface, error) { - ifs, err := netInterfaces() - if err != nil { - return nil, nil, err - } - for _, iface := range ifs { - if !maybeTailscaleInterfaceName(iface.Name) { - continue - } - addrs, err := iface.Addrs() - if err != nil { - continue - } - var tsIPs []netip.Addr - for _, a := range addrs { - if ipnet, ok := a.(*net.IPNet); ok { - nip, ok := netip.AddrFromSlice(ipnet.IP) - nip = nip.Unmap() - if ok && tsaddr.IsTailscaleIP(nip) { - tsIPs = append(tsIPs, nip) - } - } - } - if len(tsIPs) > 0 { - return tsIPs, iface.Interface, nil - } - } - return nil, nil, nil -} - -// maybeTailscaleInterfaceName reports whether s is an interface -// name that might be used by Tailscale. -func maybeTailscaleInterfaceName(s string) bool { - return s == "Tailscale" || - strings.HasPrefix(s, "wg") || - strings.HasPrefix(s, "ts") || - strings.HasPrefix(s, "tailscale") || - strings.HasPrefix(s, "utun") -} - func isUp(nif *net.Interface) bool { return nif.Flags&net.FlagUp != 0 } func isLoopback(nif *net.Interface) bool { return nif.Flags&net.FlagLoopback != 0 } @@ -300,9 +258,9 @@ func (s *State) String() string { } } sb.WriteString("ifs={") - ifs := make([]string, 0, len(s.Interface)) + var ifs []string for k := range s.Interface { - if anyInterestingIP(s.InterfaceIPs[k]) { + if s.keepInterfaceInStringSummary(k) { ifs = append(ifs, k) } } @@ -318,23 +276,40 @@ func (s *State) String() string { if i > 0 { sb.WriteString(" ") } - if s.Interface[ifName].IsUp() { - fmt.Fprintf(&sb, "%s:[", ifName) - needSpace := false - for _, pfx := range s.InterfaceIPs[ifName] { - if !isInterestingIP(pfx.Addr()) { - continue - } - if needSpace { - sb.WriteString(" ") - } + iface := s.Interface[ifName] + if iface.Interface == nil { + fmt.Fprintf(&sb, "%s:nil", ifName) + continue + } + if !iface.IsUp() { + fmt.Fprintf(&sb, "%s:down", ifName) + continue + } + fmt.Fprintf(&sb, "%s:[", ifName) + needSpace := false + for _, pfx := range s.InterfaceIPs[ifName] { + a := pfx.Addr() + if a.IsMulticast() { + continue + } + fam := "4" + if a.Is6() { + fam = "6" + } + if needSpace { + sb.WriteString(" ") + } + needSpace = true + switch { + case a.IsLoopback(): + fmt.Fprintf(&sb, "lo%s", fam) + case a.IsLinkLocalUnicast(): + fmt.Fprintf(&sb, "llu%s", fam) + default: fmt.Fprintf(&sb, "%s", pfx) - needSpace = true } - sb.WriteString("]") - } else { - fmt.Fprintf(&sb, "%s:down", ifName) } + sb.WriteString("]") } sb.WriteString("}") @@ -351,18 +326,8 @@ func (s *State) String() string { return sb.String() } -// An InterfaceFilter indicates whether EqualFiltered should use i when deciding whether two States are equal. -// ips are all the IPPrefixes associated with i. -type InterfaceFilter func(i Interface, ips []netip.Prefix) bool - -// An IPFilter indicates whether EqualFiltered should use ip when deciding whether two States are equal. -// ip is an ip address associated with some interface under consideration. -type IPFilter func(ip netip.Addr) bool - -// EqualFiltered reports whether s and s2 are equal, -// considering only interfaces in s for which filter returns true, -// and considering only IPs for those interfaces for which filterIP returns true. -func (s *State) EqualFiltered(s2 *State, useInterface InterfaceFilter, useIP IPFilter) bool { +// Equal reports whether s and s2 are exactly equal. +func (s *State) Equal(s2 *State) bool { if s == nil && s2 == nil { return true } @@ -378,19 +343,16 @@ func (s *State) EqualFiltered(s2 *State, useInterface InterfaceFilter, useIP IPF return false } for iname, i := range s.Interface { - ips := s.InterfaceIPs[iname] - if !useInterface(i, ips) { - continue - } i2, ok := s2.Interface[iname] if !ok { return false } - ips2, ok := s2.InterfaceIPs[iname] - if !ok { + if !i.Equal(i2) { return false } - if !interfacesEqual(i, i2) || !prefixesEqualFiltered(ips, ips2, useIP) { + } + for iname, vv := range s.InterfaceIPs { + if !slices.Equal(vv, s2.InterfaceIPs[iname]) { return false } } @@ -413,60 +375,23 @@ func (s *State) HasIP(ip netip.Addr) bool { return false } -func interfacesEqual(a, b Interface) bool { - return a.Index == b.Index && +func (a Interface) Equal(b Interface) bool { + if (a.Interface == nil) != (b.Interface == nil) { + return false + } + if !(a.Desc == b.Desc && netAddrsEqual(a.AltAddrs, b.AltAddrs)) { + return false + } + if a.Interface != nil && !(a.Index == b.Index && a.MTU == b.MTU && a.Name == b.Name && a.Flags == b.Flags && - bytes.Equal([]byte(a.HardwareAddr), []byte(b.HardwareAddr)) -} - -func filteredIPPs(ipps []netip.Prefix, useIP IPFilter) []netip.Prefix { - // TODO: rewrite prefixesEqualFiltered to avoid making copies - x := make([]netip.Prefix, 0, len(ipps)) - for _, ipp := range ipps { - if useIP(ipp.Addr()) { - x = append(x, ipp) - } - } - return x -} - -func prefixesEqualFiltered(a, b []netip.Prefix, useIP IPFilter) bool { - return prefixesEqual(filteredIPPs(a, useIP), filteredIPPs(b, useIP)) -} - -func prefixesEqual(a, b []netip.Prefix) bool { - if len(a) != len(b) { + bytes.Equal([]byte(a.HardwareAddr), []byte(b.HardwareAddr))) { return false } - for i, v := range a { - if b[i] != v { - return false - } - } return true } -// UseInterestingInterfaces is an InterfaceFilter that reports whether i is an interesting interface. -// An interesting interface if it is (a) not owned by Tailscale and (b) routes interesting IP addresses. -// See UseInterestingIPs for the definition of an interesting IP address. -func UseInterestingInterfaces(i Interface, ips []netip.Prefix) bool { - return !isTailscaleInterface(i.Name, ips) && anyInterestingIP(ips) -} - -// UseInterestingIPs is an IPFilter that reports whether ip is an interesting IP address. -// An IP address is interesting if it is neither a loopback nor a link local unicast IP address. -func UseInterestingIPs(ip netip.Addr) bool { - return isInterestingIP(ip) -} - -// UseAllInterfaces is an InterfaceFilter that includes all interfaces. -func UseAllInterfaces(i Interface, ips []netip.Prefix) bool { return true } - -// UseAllIPs is an IPFilter that includes all IPs. -func UseAllIPs(ips netip.Addr) bool { return true } - func (s *State) HasPAC() bool { return s != nil && s.PAC != "" } // AnyInterfaceUp reports whether any interface seems like it has Internet access. @@ -477,6 +402,18 @@ func (s *State) AnyInterfaceUp() bool { return s != nil && (s.HaveV4 || s.HaveV6) } +func netAddrsEqual(a, b []net.Addr) bool { + if len(a) != len(b) { + return false + } + for i, av := range a { + if av.Network() != b[i].Network() || av.String() != b[i].String() { + return false + } + } + return true +} + func hasTailscaleIP(pfxs []netip.Prefix) bool { for _, pfx := range pfxs { if tsaddr.IsTailscaleIP(pfx.Addr()) { @@ -664,11 +601,23 @@ var ( v6Global1 = netip.MustParsePrefix("2000::/3") ) -// anyInterestingIP reports whether pfxs contains any IP that matches -// isInterestingIP. -func anyInterestingIP(pfxs []netip.Prefix) bool { - for _, pfx := range pfxs { - if isInterestingIP(pfx.Addr()) { +// keepInterfaceInStringSummary reports whether the named interface should be included +// in the String method's summary string. +func (s *State) keepInterfaceInStringSummary(ifName string) bool { + iface, ok := s.Interface[ifName] + if !ok || iface.Interface == nil { + return false + } + if ifName == s.DefaultRouteInterface { + return true + } + up := iface.IsUp() + for _, p := range s.InterfaceIPs[ifName] { + a := p.Addr() + if a.IsLinkLocalUnicast() || a.IsLoopback() { + continue + } + if up || a.IsGlobalUnicast() || a.IsPrivate() { return true } } @@ -677,9 +626,12 @@ func anyInterestingIP(pfxs []netip.Prefix) bool { // isInterestingIP reports whether ip is an interesting IP that we // should log in interfaces.State logging. We don't need to show -// localhost or link-local addresses. +// loopback, link-local addresses, or non-Tailscale ULA addresses. func isInterestingIP(ip netip.Addr) bool { - return !ip.IsLoopback() && !ip.IsLinkLocalUnicast() + if ip.IsLoopback() || ip.IsLinkLocalUnicast() { + return false + } + return true } var altNetInterfaces func() ([]Interface, error) diff --git a/net/interfaces/interfaces_test.go b/net/interfaces/interfaces_test.go index bb93d38bf..98448dbb2 100644 --- a/net/interfaces/interfaces_test.go +++ b/net/interfaces/interfaces_test.go @@ -23,19 +23,6 @@ func TestGetState(t *testing.T) { } t.Logf("Got: %s", j) t.Logf("As string: %s", st) - - st2, err := GetState() - if err != nil { - t.Fatal(err) - } - - if !st.EqualFiltered(st2, UseAllInterfaces, UseAllIPs) { - // let's assume nobody was changing the system network interfaces between - // the two GetState calls. - t.Fatal("two States back-to-back were not equal") - } - - t.Logf("As string:\n\t%s", st) } func TestLikelyHomeRouterIP(t *testing.T) { @@ -154,7 +141,7 @@ func TestIsUsableV6(t *testing.T) { {"first ULA", "fc00::1", true}, {"Tailscale", "fd7a:115c:a1e0::1", false}, {"Cloud Run", "fddf:3978:feb1:d745::1", true}, - {"zeros", "0000:0000:0000:0000:0000:0000:0000:0000", false}, + {"zeros", "0::0", false}, {"Link Local", "fe80::1", false}, {"Global", "2602::1", true}, {"IPv4 public", "192.0.2.1", false}, @@ -168,41 +155,6 @@ func TestIsUsableV6(t *testing.T) { } } -func TestStateEqualFilteredIPFilter(t *testing.T) { - // s1 and s2 are identical, except that an "interesting" interface - // has gained an "uninteresting" IP address. - - s1 := &State{ - InterfaceIPs: map[string][]netip.Prefix{"x": { - netip.MustParsePrefix("42.0.0.0/8"), - netip.MustParsePrefix("169.254.0.0/16"), // link local unicast - }}, - Interface: map[string]Interface{"x": {Interface: &net.Interface{Name: "x"}}}, - } - - s2 := &State{ - InterfaceIPs: map[string][]netip.Prefix{"x": { - netip.MustParsePrefix("42.0.0.0/8"), - netip.MustParsePrefix("169.254.0.0/16"), // link local unicast - netip.MustParsePrefix("127.0.0.0/8"), // loopback (added) - }}, - Interface: map[string]Interface{"x": {Interface: &net.Interface{Name: "x"}}}, - } - - // s1 and s2 are different... - if s1.EqualFiltered(s2, UseAllInterfaces, UseAllIPs) { - t.Errorf("%+v != %+v", s1, s2) - } - // ...and they look different if you only restrict to interesting interfaces... - if s1.EqualFiltered(s2, UseInterestingInterfaces, UseAllIPs) { - t.Errorf("%+v != %+v when restricting to interesting interfaces _but not_ IPs", s1, s2) - } - // ...but because the additional IP address is uninteresting, we should treat them as the same. - if !s1.EqualFiltered(s2, UseInterestingInterfaces, UseInterestingIPs) { - t.Errorf("%+v == %+v when restricting to interesting interfaces and IPs", s1, s2) - } -} - func TestStateString(t *testing.T) { tests := []struct { name string @@ -222,11 +174,15 @@ func TestStateString(t *testing.T) { "wlan0": { Interface: &net.Interface{}, }, + "lo": { + Interface: &net.Interface{}, + }, }, InterfaceIPs: map[string][]netip.Prefix{ "eth0": { netip.MustParsePrefix("10.0.0.2/8"), }, + "lo": {}, }, HaveV4: true, }, @@ -239,10 +195,13 @@ func TestStateString(t *testing.T) { Interface: map[string]Interface{ "foo": { Desc: "a foo thing", + Interface: &net.Interface{ + Flags: net.FlagUp, + }, }, }, }, - want: `interfaces.State{defaultRoute=foo (a foo thing) ifs={} v4=false v6=false}`, + want: `interfaces.State{defaultRoute=foo (a foo thing) ifs={foo:[]} v4=false v6=false}`, }, } for _, tt := range tests { @@ -254,3 +213,112 @@ func TestStateString(t *testing.T) { }) } } + +func TestIsInterestingIP(t *testing.T) { + tests := []struct { + ip string + want bool + }{ + {"fd7a:115c:a1e0:ab12:4843:cd96:624a:4603", true}, // Tailscale private ULA + {"fd15:bbfa:c583:4fce:f4fb:4ff:fe1a:4148", true}, // Other private ULA + {"10.2.3.4", true}, + {"127.0.0.1", false}, + {"::1", false}, + {"2001::2", true}, + {"169.254.1.2", false}, + {"fe80::1", false}, + } + for _, tt := range tests { + if got := isInterestingIP(netip.MustParseAddr(tt.ip)); got != tt.want { + t.Errorf("isInterestingIP(%q) = %v, want %v", tt.ip, got, tt.want) + } + } +} + +// tests (*State).Equal +func TestEqual(t *testing.T) { + tests := []struct { + name string + s1, s2 *State + want bool // implies !wantMajor + }{ + { + name: "eq_nil", + want: true, + }, + { + name: "nil_mix", + s2: new(State), + want: false, + }, + { + name: "eq", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + want: true, + }, + { + name: "default-route-changed", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "bar", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + want: false, + }, + { + name: "some-interface-ips-changed", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.3/16")}, + }, + }, + want: false, + }, + { + name: "altaddrs-changed", + s1: &State{ + Interface: map[string]Interface{ + "foo": {AltAddrs: []net.Addr{&net.TCPAddr{IP: net.ParseIP("1.2.3.4")}}}, + }, + }, + s2: &State{ + Interface: map[string]Interface{ + "foo": {AltAddrs: []net.Addr{&net.TCPAddr{IP: net.ParseIP("5.6.7.8")}}}, + }, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.s2.Equal(tt.s1); got != tt.want { + t.Errorf("Equal = %v; want %v", got, tt.want) + } + }) + } +} diff --git a/net/netmon/netmon.go b/net/netmon/netmon.go index a555a7c95..b37b25967 100644 --- a/net/netmon/netmon.go +++ b/net/netmon/netmon.go @@ -55,6 +55,10 @@ type Monitor struct { change chan bool // send false to wake poller, true to also force ChangeDeltas be sent stop chan struct{} // closed on Stop + // Things that must be set early, before use, + // and not change at runtime. + tsIfName string // tailscale interface name, if known/set ("tailscale0", "utun3", ...) + mu sync.Mutex // guards all following fields cbs set.HandleSet[ChangeFunc] ruleDelCB set.HandleSet[RuleDeleteCallback] @@ -76,6 +80,9 @@ type ChangeFunc func(*ChangeDelta) // ChangeDelta describes the difference between two network states. type ChangeDelta struct { + // Monitor is the network monitor that sent this delta. + Monitor *Monitor + // Old is the old interface state, if known. // It's nil if the old state is unknown. // Do not mutate it. @@ -145,6 +152,15 @@ func (m *Monitor) interfaceStateUncached() (*interfaces.State, error) { return interfaces.GetState() } +// 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.tsIfName = ifName +} + // GatewayAndSelfIP returns the current network's default gateway, and // the machine's default IP for that gateway. // @@ -320,7 +336,11 @@ func (m *Monitor) notifyRuleDeleted(rdm ipRuleDeletedMessage) { // considered when checking for network state changes. // The ips parameter should be the IPs of the provided interface. func (m *Monitor) isInterestingInterface(i interfaces.Interface, ips []netip.Prefix) bool { - return m.om.IsInterestingInterface(i.Name) && interfaces.UseInterestingInterfaces(i, ips) + if !m.om.IsInterestingInterface(i.Name) { + return false + } + + return true } // debounce calls the callback function with a delay between events @@ -358,18 +378,19 @@ func (m *Monitor) handlePotentialChange(newState *interfaces.State, forceCallbac defer m.mu.Unlock() oldState := m.ifState timeJumped := shouldMonitorTimeJump && m.checkWallTimeAdvanceLocked() - if !timeJumped && !forceCallbacks && oldState.EqualFiltered(newState, interfaces.UseAllInterfaces, interfaces.UseAllIPs) { + if !timeJumped && !forceCallbacks && oldState.Equal(newState) { // Exactly equal. Nothing to do. return } delta := &ChangeDelta{ + Monitor: m, Old: oldState, New: newState, TimeJumped: timeJumped, } - delta.Major = !newState.EqualFiltered(oldState, m.isInterestingInterface, interfaces.UseInterestingIPs) + delta.Major = m.IsMajorChangeFrom(oldState, newState) if delta.Major { m.gwValid = false m.ifState = newState @@ -394,6 +415,79 @@ func (m *Monitor) handlePotentialChange(newState *interfaces.State, forceCallbac } } +// IsMajorChangeFrom reports whether the transition from s1 to s2 is +// a "major" change, where major roughly means it's worth tearing down +// a bunch of connections and rebinding. +// +// TODO(bradiftz): tigten this definition. +func (m *Monitor) IsMajorChangeFrom(s1, s2 *interfaces.State) bool { + if s1 == nil && s2 == nil { + return false + } + if s1 == nil || s2 == nil { + return true + } + if s1.HaveV6 != s2.HaveV6 || + s1.HaveV4 != s2.HaveV4 || + s1.IsExpensive != s2.IsExpensive || + s1.DefaultRouteInterface != s2.DefaultRouteInterface || + s1.HTTPProxy != s2.HTTPProxy || + s1.PAC != s2.PAC { + return true + } + for iname, i := range s1.Interface { + if iname == m.tsIfName { + // Ignore changes in the Tailscale interface itself. + continue + } + ips := s1.InterfaceIPs[iname] + if !m.isInterestingInterface(i, ips) { + continue + } + i2, ok := s2.Interface[iname] + if !ok { + return true + } + ips2, ok := s2.InterfaceIPs[iname] + if !ok { + return true + } + if !i.Equal(i2) || !prefixesMajorEqual(ips, ips2) { + return true + } + } + return false +} + +// prefixesMajorEqual reports whether a and b are equal after ignoring +// boring things like link-local, loopback, and multicast addresses. +func prefixesMajorEqual(a, b []netip.Prefix) bool { + // trim returns a subslice of p with link local unicast, + // loopback, and multicast prefixes removed from the front. + trim := func(p []netip.Prefix) []netip.Prefix { + for len(p) > 0 { + a := p[0].Addr() + if a.IsLinkLocalUnicast() || a.IsLoopback() || a.IsMulticast() { + p = p[1:] + continue + } + break + } + return p + } + for { + a = trim(a) + b = trim(b) + if len(a) == 0 || len(b) == 0 { + return len(a) == 0 && len(b) == 0 + } + if a[0] != b[0] { + return false + } + a, b = a[1:], b[1:] + } +} + func jsonSummary(x any) any { j, err := json.Marshal(x) if err != nil { diff --git a/net/netmon/netmon_test.go b/net/netmon/netmon_test.go index b209fd730..9c08b0df1 100644 --- a/net/netmon/netmon_test.go +++ b/net/netmon/netmon_test.go @@ -5,9 +5,14 @@ package netmon import ( "flag" + "net" + "net/netip" "sync/atomic" "testing" "time" + + "tailscale.com/net/interfaces" + "tailscale.com/util/mak" ) func TestMonitorStartClose(t *testing.T) { @@ -108,3 +113,130 @@ func TestMonitorMode(t *testing.T) { t.Logf("%v callbacks", n) } } + +// tests (*State).IsMajorChangeFrom +func TestIsMajorChangeFrom(t *testing.T) { + type State = interfaces.State + type Interface = interfaces.Interface + tests := []struct { + name string + s1, s2 *State + want bool + }{ + { + name: "eq_nil", + want: false, + }, + { + name: "nil_mix", + s2: new(State), + want: true, + }, + { + name: "eq", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + want: false, + }, + { + name: "default-route-changed", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "bar", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + want: true, + }, + { + name: "some-interesting-ip-changed", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.3/16")}, + }, + }, + want: true, + }, + { + name: "ipv6-ula-addressed-appeared", + s1: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": {netip.MustParsePrefix("10.0.1.2/16")}, + }, + }, + s2: &State{ + DefaultRouteInterface: "foo", + InterfaceIPs: map[string][]netip.Prefix{ + "foo": { + netip.MustParsePrefix("10.0.1.2/16"), + // Brad saw this address coming & going on his home LAN, possibly + // via an Apple TV Thread routing advertisement? (Issue 9040) + netip.MustParsePrefix("fd15:bbfa:c583:4fce:f4fb:4ff:fe1a:4148/64"), + }, + }, + }, + want: true, // TODO(bradfitz): want false (ignore the IPv6 ULA address on foo) + }, + } + 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 { + continue + } + for name := range s.InterfaceIPs { + if _, ok := s.Interface[name]; !ok { + mak.Set(&s.Interface, name, Interface{Interface: &net.Interface{ + Name: name, + }}) + } + } + } + + var m Monitor + m.om = &testOSMon{ + Interesting: func(name string) bool { return true }, + } + if got := m.IsMajorChangeFrom(tt.s1, tt.s2); got != tt.want { + t.Errorf("IsMajorChange = %v; want %v", got, tt.want) + } + }) + } +} + +type testOSMon struct { + osMon + Interesting func(name string) bool +} + +func (m *testOSMon) IsInterestingInterface(name string) bool { + if m.Interesting == nil { + return true + } + return m.Interesting(name) +} diff --git a/net/netns/netns_darwin.go b/net/netns/netns_darwin.go index b32a744b7..b9395f734 100644 --- a/net/netns/netns_darwin.go +++ b/net/netns/netns_darwin.go @@ -20,6 +20,7 @@ import ( "tailscale.com/envknob" "tailscale.com/net/interfaces" "tailscale.com/net/netmon" + "tailscale.com/net/tsaddr" "tailscale.com/types/logger" ) @@ -110,7 +111,7 @@ func getInterfaceIndex(logf logger.Logf, netMon *netmon.Monitor, address string) // Verify that we didn't just choose the Tailscale interface; // if so, we fall back to binding from the default. - _, tsif, err2 := interfaces.Tailscale() + tsif, err2 := tailscaleInterface() if err2 == nil && tsif != nil && tsif.Index == idx { logf("[unexpected] netns: interfaceIndexFor returned Tailscale interface") return defaultIdx() @@ -119,6 +120,34 @@ func getInterfaceIndex(logf logger.Logf, netMon *netmon.Monitor, address string) return idx, err } +// tailscaleInterface returns the current machine's Tailscale interface, if any. +// If none is found, (nil, nil) is returned. +// A non-nil error is only returned on a problem listing the system interfaces. +func tailscaleInterface() (*net.Interface, error) { + ifs, err := net.Interfaces() + if err != nil { + return nil, err + } + for _, iface := range ifs { + if !strings.HasPrefix(iface.Name, "utun") { + continue + } + addrs, err := iface.Addrs() + if err != nil { + continue + } + for _, a := range addrs { + if ipnet, ok := a.(*net.IPNet); ok { + nip, ok := netip.AddrFromSlice(ipnet.IP) + if ok && tsaddr.IsTailscaleIP(nip.Unmap()) { + return &iface, nil + } + } + } + } + return nil, nil +} + // interfaceIndexFor returns the interface index that we should bind to in // order to send traffic to the provided address. func interfaceIndexFor(addr netip.Addr, canRecurse bool) (int, error) { diff --git a/net/netns/netns_darwin_test.go b/net/netns/netns_darwin_test.go index 0fc92f6f3..17d1f9945 100644 --- a/net/netns/netns_darwin_test.go +++ b/net/netns/netns_darwin_test.go @@ -55,7 +55,7 @@ func TestGetInterfaceIndex(t *testing.T) { } t.Run("NoTailscale", func(t *testing.T) { - _, tsif, err := interfaces.Tailscale() + tsif, err := tailscaleInterface() if err != nil { t.Fatal(err) }