From 567c5a6d9e6dabf042eadc83e0779b2f00055743 Mon Sep 17 00:00:00 2001 From: Sonia Appasamy Date: Mon, 25 Jan 2021 17:41:39 -0500 Subject: [PATCH 1/3] tailcfg, controlclient: add DisplayName field to tailcfg.Node and populate it from controlclient (#1191) Signed-off-by: Sonia Appasamy --- cmd/tailscaled/depaware.txt | 2 +- control/controlclient/direct.go | 7 +++++++ control/controlclient/netmap.go | 4 +++- tailcfg/tailcfg.go | 23 +++++++++++++++++++++++ tailcfg/tailcfg_clone.go | 1 + tailcfg/tailcfg_test.go | 2 +- util/dnsname/dnsname.go | 8 ++++++++ util/dnsname/dnsname_test.go | 18 ++++++++++++++++++ 8 files changed, 62 insertions(+), 3 deletions(-) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 2638d666c..19bcc78d6 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -113,7 +113,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/types/strbuilder from tailscale.com/net/packet tailscale.com/types/structs from tailscale.com/control/controlclient+ tailscale.com/types/wgkey from tailscale.com/control/controlclient+ - tailscale.com/util/dnsname from tailscale.com/wgengine/tsdns + tailscale.com/util/dnsname from tailscale.com/wgengine/tsdns+ LW tailscale.com/util/endian from tailscale.com/net/netns+ tailscale.com/util/lineread from tailscale.com/control/controlclient+ tailscale.com/util/pidowner from tailscale.com/ipn/ipnserver diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 85267fae6..b3a9f61ac 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -775,6 +775,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw MachineKey: machinePubKey, Expiry: resp.Node.KeyExpiry, Name: resp.Node.Name, + DisplayName: resp.Node.DisplayName, Addresses: resp.Node.Addresses, Peers: resp.Peers, LocalPort: localPort, @@ -799,6 +800,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw } addUserProfile(nm.User) for _, peer := range resp.Peers { + if peer.DisplayName == "" { + peer.DisplayName = peer.DefaultDisplayName() + } if !peer.Sharer.IsZero() { if c.keepSharerAndUserSplit { addUserProfile(peer.Sharer) @@ -808,6 +812,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw } addUserProfile(peer.User) } + if resp.Node.DisplayName == "" { + nm.DisplayName = resp.Node.DefaultDisplayName() + } if resp.Node.MachineAuthorized { nm.MachineStatus = tailcfg.MachineAuthorized } else { diff --git a/control/controlclient/netmap.go b/control/controlclient/netmap.go index ff574a39b..fe8553b66 100644 --- a/control/controlclient/netmap.go +++ b/control/controlclient/netmap.go @@ -28,7 +28,9 @@ type NetworkMap struct { PrivateKey wgkey.Private Expiry time.Time // Name is the DNS name assigned to this node. - Name string + Name string + // DisplayName is the title to show for the node in client UIs. + DisplayName string Addresses []netaddr.IPPrefix LocalPort uint16 // used for debugging MachineStatus tailcfg.MachineStatus diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index ba948125c..098c95d56 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -20,6 +20,7 @@ import ( "tailscale.com/types/key" "tailscale.com/types/opt" "tailscale.com/types/structs" + "tailscale.com/util/dnsname" ) // CurrentMapRequestVersion is the current MapRequest.Version value. @@ -160,6 +161,12 @@ type Node struct { StableID StableNodeID Name string // DNS + // DisplayName is the title to show for the node in client + // UIs. This field is assigned by default in controlclient, + // but can be overriden by providing this field non-empty + // in a MapResponse. + DisplayName string `json:",omitempty"` + // User is the user who created the node. If ACL tags are in // use for the node then it doesn't reflect the ACL identity // that the node is running as. @@ -185,6 +192,21 @@ type Node struct { MachineAuthorized bool `json:",omitempty"` // TODO(crawshaw): replace with MachineStatus } +// DefaultDisplayName returns a value suitable +// for using as the default value for n.DisplayName. +func (n *Node) DefaultDisplayName() string { + if n.Name != "" { + // Use the Magic DNS prefix as the default display name. + return dnsname.ToBaseName(n.Name) + } + if n.Hostinfo.Hostname != "" { + // When no Magic DNS name is present, use the hostname. + return n.Hostinfo.Hostname + } + // When we've exhausted all other name options, use the node's ID. + return n.ID.String() +} + type MachineStatus int const ( @@ -796,6 +818,7 @@ func (n *Node) Equal(n2 *Node) bool { n.ID == n2.ID && n.StableID == n2.StableID && n.Name == n2.Name && + n.DisplayName == n2.DisplayName && n.User == n2.User && n.Sharer == n2.Sharer && n.Key == n2.Key && diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 32e097bc5..3101048b4 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -64,6 +64,7 @@ var _NodeNeedsRegeneration = Node(struct { ID NodeID StableID StableNodeID Name string + DisplayName string User UserID Sharer UserID Key NodeKey diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 02bc499af..794f7d4a1 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -189,7 +189,7 @@ func TestHostinfoEqual(t *testing.T) { func TestNodeEqual(t *testing.T) { nodeHandles := []string{ - "ID", "StableID", "Name", "User", "Sharer", + "ID", "StableID", "Name", "DisplayName", "User", "Sharer", "Key", "KeyExpiry", "Machine", "DiscoKey", "Addresses", "AllowedIPs", "Endpoints", "DERP", "Hostinfo", "Created", "LastSeen", "KeepAlive", "MachineAuthorized", diff --git a/util/dnsname/dnsname.go b/util/dnsname/dnsname.go index 1488272a4..633471e2f 100644 --- a/util/dnsname/dnsname.go +++ b/util/dnsname/dnsname.go @@ -17,3 +17,11 @@ func HasSuffix(name, suffix string) bool { nameBase := strings.TrimSuffix(name, suffix) return len(nameBase) < len(name) && strings.HasSuffix(nameBase, ".") } + +// ToBaseName removes the domain ending from a DNS name of a node. +func ToBaseName(name string) string { + if i := strings.Index(name, "."); i != -1 { + return name[:i] + } + return name +} diff --git a/util/dnsname/dnsname_test.go b/util/dnsname/dnsname_test.go index da4e51384..a8c97ed8b 100644 --- a/util/dnsname/dnsname_test.go +++ b/util/dnsname/dnsname_test.go @@ -26,3 +26,21 @@ func TestHasSuffix(t *testing.T) { } } } + +func TestToBaseName(t *testing.T) { + tests := []struct { + name string + want string + }{ + {"foo", "foo"}, + {"foo.com", "foo"}, + {"foo.example.com.beta.tailscale.net", "foo"}, + {"computer-a.test.gmail.com.beta.tailscale.net", "computer-a"}, + } + for _, tt := range tests { + got := ToBaseName(tt.name) + if got != tt.want { + t.Errorf("ToBaseName(%q) = %q; want %q", tt.name, got, tt.want) + } + } +} From 0dde8fa0a8b22c2c80e4bab604a17bddd285f7f8 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 25 Jan 2021 15:46:14 -0800 Subject: [PATCH 2/3] ipn/ipnserver: rearrange some code No functional change. Make a future diff easier to read. --- ipn/ipnserver/server.go | 56 ++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go index c4fd95a2b..cfccc2e74 100644 --- a/ipn/ipnserver/server.go +++ b/ipn/ipnserver/server.go @@ -503,6 +503,34 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() ( }() logf("Listening on %v", listen.Addr()) + var store ipn.StateStore + if opts.StatePath != "" { + store, err = ipn.NewFileStore(opts.StatePath) + if err != nil { + return fmt.Errorf("ipn.NewFileStore(%q): %v", opts.StatePath, err) + } + if opts.AutostartStateKey == "" { + autoStartKey, err := store.ReadState(ipn.ServerModeStartKey) + if err != nil && err != ipn.ErrStateNotExist { + return fmt.Errorf("calling ReadState on %s: %w", opts.StatePath, err) + } + key := string(autoStartKey) + if strings.HasPrefix(key, "user-") { + uid := strings.TrimPrefix(key, "user-") + u, err := server.lookupUserFromID(uid) + if err != nil { + logf("ipnserver: found server mode auto-start key %q; failed to load user: %v", key, err) + } else { + logf("ipnserver: found server mode auto-start key %q (user %s)", key, u.Username) + server.serverModeUser = u + } + opts.AutostartStateKey = ipn.StateKey(key) + } + } + } else { + store = &ipn.MemoryStore{} + } + bo := backoff.NewBackoff("ipnserver", logf, 30*time.Second) var unservedConn net.Conn // if non-nil, accepted, but hasn't served yet @@ -538,34 +566,6 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() ( } } - var store ipn.StateStore - if opts.StatePath != "" { - store, err = ipn.NewFileStore(opts.StatePath) - if err != nil { - return fmt.Errorf("ipn.NewFileStore(%q): %v", opts.StatePath, err) - } - if opts.AutostartStateKey == "" { - autoStartKey, err := store.ReadState(ipn.ServerModeStartKey) - if err != nil && err != ipn.ErrStateNotExist { - return fmt.Errorf("calling ReadState on %s: %w", opts.StatePath, err) - } - key := string(autoStartKey) - if strings.HasPrefix(key, "user-") { - uid := strings.TrimPrefix(key, "user-") - u, err := server.lookupUserFromID(uid) - if err != nil { - logf("ipnserver: found server mode auto-start key %q; failed to load user: %v", key, err) - } else { - logf("ipnserver: found server mode auto-start key %q (user %s)", key, u.Username) - server.serverModeUser = u - } - opts.AutostartStateKey = ipn.StateKey(key) - } - } - } else { - store = &ipn.MemoryStore{} - } - b, err := ipn.NewLocalBackend(logf, logid, store, eng) if err != nil { return fmt.Errorf("NewLocalBackend: %v", err) From c3c59445ff5272355f4cce1a7171ec5c6672bb8a Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 25 Jan 2021 14:53:31 -0800 Subject: [PATCH 3/3] ipn/ipnserver: on Windows in unattended mode, wait for Engine forever Updates #1187 --- ipn/ipnserver/server.go | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go index cfccc2e74..7518c6e2a 100644 --- a/ipn/ipnserver/server.go +++ b/ipn/ipnserver/server.go @@ -537,6 +537,46 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() ( eng, err := getEngine() if err != nil { logf("ipnserver: initial getEngine call: %v", err) + + // Issue 1187: on Windows, in unattended mode, + // sometimes we try 5 times and fail to create the + // engine before the system's ready. Hack until the + // bug if fixed properly: if we're running in + // unattended mode on Windows, keep trying forever, + // waiting for the machine to be ready (networking to + // come up?) and then dial our own safesocket TCP + // listener to wake up the usual mechanism that lets + // us surface getEngine errors to UI clients. (We + // don't want to just call getEngine in a loop without + // the listener.Accept, as we do want to handle client + // connections so we can tell them about errors) + + bootRaceWaitForEngine, bootRaceWaitForEngineCancel := context.WithTimeout(context.Background(), time.Minute) + if runtime.GOOS == "windows" && opts.AutostartStateKey != "" { + logf("ipnserver: in unattended mode, waiting for engine availability") + getEngine = getEngineUntilItWorksWrapper(getEngine) + // Wait for it to be ready. + go func() { + defer bootRaceWaitForEngineCancel() + t0 := time.Now() + for { + time.Sleep(10 * time.Second) + if _, err := getEngine(); err != nil { + logf("ipnserver: unattended mode engine load: %v", err) + continue + } + c, err := net.Dial("tcp", listen.Addr().String()) + logf("ipnserver: engine created after %v; waking up Accept: Dial error: %v", time.Since(t0).Round(time.Second), err) + if err == nil { + c.Close() + } + break + } + }() + } else { + bootRaceWaitForEngineCancel() + } + for i := 1; ctx.Err() == nil; i++ { c, err := listen.Accept() if err != nil { @@ -544,6 +584,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() ( bo.BackOff(ctx, err) continue } + <-bootRaceWaitForEngine.Done() logf("ipnserver: try%d: trying getEngine again...", i) eng, err = getEngine() if err == nil { @@ -756,6 +797,27 @@ func FixedEngine(eng wgengine.Engine) func() (wgengine.Engine, error) { return func() (wgengine.Engine, error) { return eng, nil } } +// getEngineUntilItWorksWrapper returns a getEngine wrapper that does +// not call getEngine concurrently and stops calling getEngine once +// it's returned a working engine. +func getEngineUntilItWorksWrapper(getEngine func() (wgengine.Engine, error)) func() (wgengine.Engine, error) { + var mu sync.Mutex + var engGood wgengine.Engine + return func() (wgengine.Engine, error) { + mu.Lock() + defer mu.Unlock() + if engGood != nil { + return engGood, nil + } + e, err := getEngine() + if err != nil { + return nil, err + } + engGood = e + return e, nil + } +} + type dummyAddr string type oneConnListener struct { conn net.Conn