ipn/ipnlocal: set the push device token correctly

It would end up resetting whatever hostinfo we had constructed
and leave the backend statemachine in a broken state.
This fixes that by storing the PushDeviceToken on the LocalBackend
and populating it on Hostinfo before passing it to controlclient.

Updates tailscale/corp#8940
Updates tailscale/corp#15367

Signed-off-by: Maisem Ali <maisem@tailscale.com>

hostinfo/hostinfo.go

@@ -53,7 +53,6 @@ func New() *tailcfg.Hostinfo {
 		GoVersion:       runtime.Version(),
 		Machine:         condCall(unameMachine),
 		DeviceModel:     deviceModel(),
-		PushDeviceToken: pushDeviceToken(),
 		Cloud:           string(cloudenv.Get()),
 		NoLogsNoSupport: envknob.NoLogsNoSupport(),
 		AllowsUpdate:    envknob.AllowsRemoteUpdate(),
@@ -166,18 +165,14 @@ func GetEnvType() EnvType {
 }
 
 var (
-	pushDeviceTokenAtomic atomic.Value // of string
+	deviceModelAtomic atomic.Value // of string
-	deviceModelAtomic     atomic.Value // of string
+	osVersionAtomic   atomic.Value // of string
-	osVersionAtomic       atomic.Value // of string
+	desktopAtomic     atomic.Value // of opt.Bool
-	desktopAtomic         atomic.Value // of opt.Bool
+	packagingType     atomic.Value // of string
-	packagingType         atomic.Value // of string
+	appType           atomic.Value // of string
-	appType               atomic.Value // of string
+	firewallMode      atomic.Value // of string
-	firewallMode          atomic.Value // of string
 )
 
-// SetPushDeviceToken sets the device token for use in Hostinfo updates.
-func SetPushDeviceToken(token string) { pushDeviceTokenAtomic.Store(token) }
-
 // SetDeviceModel sets the device model for use in Hostinfo updates.
 func SetDeviceModel(model string) { deviceModelAtomic.Store(model) }
 
@@ -203,11 +198,6 @@ func deviceModel() string {
 	return s
 }
 
-func pushDeviceToken() string {
-	s, _ := pushDeviceTokenAtomic.Load().(string)
-	return s
-}
-
 // FirewallMode returns the firewall mode for the app.
 // It is empty if unset.
 func FirewallMode() string {

ipn/ipnlocal/local.go

@@ -157,6 +157,7 @@ type LocalBackend struct {
 	e                     wgengine.Engine // non-nil; TODO(bradfitz): remove; use sys
 	store                 ipn.StateStore  // non-nil; TODO(bradfitz): remove; use sys
 	dialer                *tsdial.Dialer  // non-nil; TODO(bradfitz): remove; use sys
+	pushDeviceToken       syncs.AtomicValue[string]
 	backendLogID          logid.PublicID
 	unregisterNetMon      func()
 	unregisterHealthWatch func()
@@ -2035,22 +2036,19 @@ func (b *LocalBackend) readPoller() {
 	}
 }
 
-// ResendHostinfoIfNeeded is called to recompute the Hostinfo and send
+// GetPushDeviceToken returns the push notification device token.
-// the new version to the control server.
+func (b *LocalBackend) GetPushDeviceToken() string {
-func (b *LocalBackend) ResendHostinfoIfNeeded() {
+	return b.pushDeviceToken.Load()
-	// TODO(maisem,bradfitz): this is all wrong. hostinfo has been modified
+}
-	// a dozen ways elsewhere that this omits. This method should be rethought.
-	hi := hostinfo.New()
 
-	b.mu.Lock()
+// SetPushDeviceToken sets the push notification device token and informs the
-	applyConfigToHostinfo(hi, b.conf)
+// controlclient of the new value.
-	if b.hostinfo != nil {
+func (b *LocalBackend) SetPushDeviceToken(tk string) {
-		hi.Services = b.hostinfo.Services
+	old := b.pushDeviceToken.Swap(tk)
+	if old == tk {
+		return
 	}
-	b.hostinfo = hi
+	b.doSetHostinfoFilterServices()
-	b.mu.Unlock()
-
-	b.doSetHostinfoFilterServices(hi)
 }
 
 func applyConfigToHostinfo(hi *tailcfg.Hostinfo, c *conffile.Config) {
@@ -3172,6 +3170,7 @@ func (b *LocalBackend) doSetHostinfoFilterServices() {
 	// the slice with no free capacity.
 	c := len(hi.Services)
 	hi.Services = append(hi.Services[:c:c], peerAPIServices...)
+	hi.PushDeviceToken = b.pushDeviceToken.Load()
 	cc.SetHostinfo(&hi)
 }
 

ipn/localapi/localapi.go

@@ -1553,8 +1553,7 @@ func (h *Handler) serveSetPushDeviceToken(w http.ResponseWriter, r *http.Request
 		http.Error(w, "invalid JSON body", http.StatusBadRequest)
 		return
 	}
-	hostinfo.SetPushDeviceToken(params.PushDeviceToken)
+	h.b.SetPushDeviceToken(params.PushDeviceToken)
-	h.b.ResendHostinfoIfNeeded()
 	w.WriteHeader(http.StatusOK)
 }
 

ipn/localapi/localapi_test.go

@@ -15,7 +15,6 @@ import (
 	"testing"
 
 	"tailscale.com/client/tailscale/apitype"
-	"tailscale.com/hostinfo"
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
@@ -77,7 +76,7 @@ func TestSetPushDeviceToken(t *testing.T) {
 	if res.StatusCode != 200 {
 		t.Errorf("res.StatusCode=%d, want 200. body: %s", res.StatusCode, body)
 	}
-	if got := hostinfo.New().PushDeviceToken; got != want {
+	if got := h.b.GetPushDeviceToken(); got != want {
 		t.Errorf("hostinfo.PushDeviceToken=%q, want %q", got, want)
 	}
 }