diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index b5f431073..76db38c13 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -54,6 +54,7 @@ import ( "tailscale.com/types/ptr" "tailscale.com/types/tkatype" "tailscale.com/util/clientmetric" + "tailscale.com/util/cmpx" "tailscale.com/util/multierr" "tailscale.com/util/singleflight" "tailscale.com/util/systemd" @@ -1098,6 +1099,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap setControlKnobsFromNodeAttrs(resp.Node.Capabilities) } + // Call Node.InitDisplayNames on any changed nodes. + initDisplayNames(cmpx.Or(resp.Node, sess.lastNode).View(), &resp) + nm := sess.netmapForResponse(&resp) if nm.SelfNode == nil { c.logf("MapResponse lacked node") @@ -1122,14 +1126,16 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap c.lastPrintMap = now c.logf("[v1] new network map[%d]:\n%s", i, nm.VeryConcise()) } - newPersist := persist.AsStruct() - newPersist.NodeID = nm.SelfNode.StableID - newPersist.UserProfile = nm.UserProfiles[nm.User] c.mu.Lock() // If we are the ones who last updated persist, then we can update it - // again. Otherwise, we should not touch it. - if persist == c.persist { + // again. Otherwise, we should not touch it. Also, it's only worth + // change it if the Node info changed. + if persist == c.persist && resp.Node != nil { + newPersist := persist.AsStruct() + newPersist.NodeID = nm.SelfNode.StableID + newPersist.UserProfile = nm.UserProfiles[nm.User()] + c.persist = newPersist.View() persist = c.persist } @@ -1144,6 +1150,28 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap return nil } +// initDisplayNames mutates any tailcfg.Nodes in resp to populate their display names, +// calling InitDisplayNames on each. +// +// The magicDNSSuffix used is based on selfNode. +func initDisplayNames(selfNode tailcfg.NodeView, resp *tailcfg.MapResponse) { + if resp.Node == nil && len(resp.Peers) == 0 && len(resp.PeersChanged) == 0 { + // Fast path for a common case (delta updates). No need to compute + // magicDNSSuffix. + return + } + magicDNSSuffix := netmap.MagicDNSSuffixOfNodeName(selfNode.Name()) + if resp.Node != nil { + resp.Node.InitDisplayNames(magicDNSSuffix) + } + for _, n := range resp.Peers { + n.InitDisplayNames(magicDNSSuffix) + } + for _, n := range resp.PeersChanged { + n.InitDisplayNames(magicDNSSuffix) + } +} + // decode JSON decodes the res.Body into v. If serverNoiseKey is not specified, // it uses the serverKey and mkey to decode the message from the NaCl-crypto-box. func decode(res *http.Response, v any, serverKey, serverNoiseKey key.MachinePublic, mkey key.MachinePrivate) error { diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 8975f0577..9ee923e8e 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -67,6 +67,9 @@ func newMapSession(privateNodeKey key.NodePrivate) *mapSession { } func (ms *mapSession) addUserProfile(userID tailcfg.UserID) { + if userID == 0 { + return + } nm := ms.netMapBuilding if _, dup := nm.UserProfiles[userID]; dup { // Already populated it from a previous peer. @@ -184,7 +187,6 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo nm.Expiry = node.KeyExpiry nm.Name = node.Name nm.Addresses = filterSelfAddresses(node.Addresses) - nm.User = node.User if node.Hostinfo.Valid() { nm.Hostinfo = *node.Hostinfo.AsStruct() } @@ -195,16 +197,9 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo } } - ms.addUserProfile(nm.User) - magicDNSSuffix := nm.MagicDNSSuffix() - if nm.SelfNode != nil { - nm.SelfNode.InitDisplayNames(magicDNSSuffix) - } + ms.addUserProfile(nm.User()) for _, peer := range resp.Peers { - peer.InitDisplayNames(magicDNSSuffix) - if !peer.Sharer.IsZero() { - ms.addUserProfile(peer.Sharer) - } + ms.addUserProfile(peer.Sharer) ms.addUserProfile(peer.User) } if DevKnob.ForceProxyDNS() { diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 2880f276c..81b75f96c 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -446,10 +446,11 @@ func TestNetmapForResponse(t *testing.T) { ComputedNameWithHost: "foo", } ms := newTestMapSession(t) - - nm1 := ms.netmapForResponse(&tailcfg.MapResponse{ + mapRes := &tailcfg.MapResponse{ Node: someNode, - }) + } + initDisplayNames(mapRes.Node.View(), mapRes) + nm1 := ms.netmapForResponse(mapRes) if nm1.SelfNode == nil { t.Fatal("nil Node in 1st netmap") } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 6724e523c..0397ce765 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -703,7 +703,7 @@ func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func ss.InNetworkMap = true ss.HostName = b.netMap.Hostinfo.Hostname ss.DNSName = b.netMap.Name - ss.UserID = b.netMap.User + ss.UserID = b.netMap.User() if sn := b.netMap.SelfNode; sn != nil { peerStatusFromNode(ss, sn.View()) if c := sn.Capabilities; len(c) > 0 { @@ -2781,7 +2781,7 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn } } if netMap != nil { - newProfile := netMap.UserProfiles[netMap.User] + newProfile := netMap.UserProfiles[netMap.User()] if newLoginName := newProfile.LoginName; newLoginName != "" { if !oldp.Persist().Valid() { b.logf("active login: %s", newLoginName) @@ -3972,7 +3972,7 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { b.dialer.SetNetMap(nm) var login string if nm != nil { - login = cmpx.Or(nm.UserProfiles[nm.User].LoginName, "") + login = cmpx.Or(nm.UserProfiles[nm.User()].LoginName, "") } b.netMap = nm if login != b.activeLogin { @@ -4323,7 +4323,7 @@ func (b *LocalBackend) peerIsTaildropTargetLocked(p tailcfg.NodeView) bool { if b.netMap == nil || !p.Valid() { return false } - if b.netMap.User == p.User() { + if b.netMap.User() == p.User() { return true } if p.Addresses().Len() > 0 && diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index e0ab02de8..d32d4a940 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -30,12 +30,22 @@ type NetworkMap struct { PrivateKey key.NodePrivate Expiry time.Time // Name is the DNS name assigned to this node. - Name string - Addresses []netip.Prefix // same as tailcfg.Node.Addresses (IP addresses of this Node directly) + // It is the MapResponse.Node.Name value and ends with a period. + Name string + + // Addresses is SelfNode.Addresses. (IP addresses of this Node directly) + // + // TODO(bradfitz): remove this field and make this a method. + Addresses []netip.Prefix + + // MachineStatus is either tailcfg.MachineAuthorized or tailcfg.MachineUnauthorized, + // depending on SelfNode.MachineAuthorized. + // TODO(bradfitz): remove this field and make it a method. MachineStatus tailcfg.MachineStatus - MachineKey key.MachinePublic - Peers []tailcfg.NodeView // sorted by Node.ID - DNS tailcfg.DNSConfig + + MachineKey key.MachinePublic + Peers []tailcfg.NodeView // sorted by Node.ID + DNS tailcfg.DNSConfig // TODO(maisem) : replace with View. Hostinfo tailcfg.Hostinfo PacketFilter []filter.Match @@ -66,10 +76,6 @@ type NetworkMap struct { // hash of the latest update message to tick through TKA). TKAHead tka.AUMHash - // ACLs - - User tailcfg.UserID - // Domain is the current Tailnet name. Domain string @@ -81,6 +87,15 @@ type NetworkMap struct { UserProfiles map[tailcfg.UserID]tailcfg.UserProfile } +// User returns nm.SelfNode.User if nm.SelfNode is non-nil, otherwise it returns +// 0. +func (nm *NetworkMap) User() tailcfg.UserID { + if nm.SelfNode != nil { + return nm.SelfNode.User + } + return 0 +} + // AnyPeersAdvertiseRoutes reports whether any peer is advertising non-exit node routes. func (nm *NetworkMap) AnyPeersAdvertiseRoutes() bool { for _, p := range nm.Peers { @@ -111,18 +126,26 @@ func (nm *NetworkMap) PeerByTailscaleIP(ip netip.Addr) (peer tailcfg.NodeView, o return tailcfg.NodeView{}, false } -// MagicDNSSuffix returns the domain's MagicDNS suffix (even if -// MagicDNS isn't necessarily in use). +// MagicDNSSuffix returns the domain's MagicDNS suffix (even if MagicDNS isn't +// necessarily in use) of the provided Node.Name value. // // It will neither start nor end with a period. -func (nm *NetworkMap) MagicDNSSuffix() string { - name := strings.Trim(nm.Name, ".") +func MagicDNSSuffixOfNodeName(nodeName string) string { + name := strings.Trim(nodeName, ".") if _, rest, ok := strings.Cut(name, "."); ok { return rest } return name } +// MagicDNSSuffix returns the domain's MagicDNS suffix (even if +// MagicDNS isn't necessarily in use). +// +// It will neither start nor end with a period. +func (nm *NetworkMap) MagicDNSSuffix() string { + return MagicDNSSuffixOfNodeName(nm.Name) +} + // SelfCapabilities returns SelfNode.Capabilities if nm and nm.SelfNode are // non-nil. This is a method so we can use it in envknob/logknob without a // circular dependency. @@ -171,12 +194,12 @@ func (nm *NetworkMap) PeerWithStableID(pid tailcfg.StableNodeID) (_ tailcfg.Node func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) { fmt.Fprintf(buf, "netmap: self: %v auth=%v", nm.NodeKey.ShortString(), nm.MachineStatus) - login := nm.UserProfiles[nm.User].LoginName + login := nm.UserProfiles[nm.User()].LoginName if login == "" { - if nm.User.IsZero() { + if nm.User().IsZero() { login = "?" } else { - login = fmt.Sprint(nm.User) + login = fmt.Sprint(nm.User()) } } fmt.Fprintf(buf, " u=%s", login) @@ -189,7 +212,7 @@ func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) { func (a *NetworkMap) equalConciseHeader(b *NetworkMap) bool { if a.NodeKey != b.NodeKey || a.MachineStatus != b.MachineStatus || - a.User != b.User || + a.User() != b.User() || len(a.Addresses) != len(b.Addresses) { return false }