diff --git a/hscontrol/state/auth_tagged_expiry_test.go b/hscontrol/state/auth_tagged_expiry_test.go index c626fb009..4fa33cfd3 100644 --- a/hscontrol/state/auth_tagged_expiry_test.go +++ b/hscontrol/state/auth_tagged_expiry_test.go @@ -2201,3 +2201,52 @@ func TestPAKReregisterConcurrentAdminExpire(t *testing.T) { require.True(t, n.IsExpired(), "iteration %d: the spent key revived an expired node", i) } } + +func TestPreAuthKeyRegistrationOwnsHostinfo(t *testing.T) { + for _, name := range []string{"new", "reregister", "preserve NetInfo"} { + t.Run(name, func(t *testing.T) { + s := newRetagTestState(t) + user := s.CreateUserForTest("hostinfo-user") + pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + + machineKey := key.NewMachine().Public() + + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "original", + Services: []tailcfg.Service{{Proto: "tcp", Port: 80}}, + NetInfo: &tailcfg.NetInfo{PreferredDERP: 1}, + }, + } + if name != "new" { + _, _, err = s.HandleNodeFromPreAuthKey(regReq, machineKey) + require.NoError(t, err) + } + + if name == "preserve NetInfo" { + regReq.Hostinfo.NetInfo = nil + } + + original := regReq.Hostinfo.Clone() + node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey) + require.NoError(t, err) + require.Equal(t, original, regReq.Hostinfo, "registration must not modify its request") + + regReq.Hostinfo.Hostname = "changed" + + regReq.Hostinfo.Services[0].Port = 443 + if regReq.Hostinfo.NetInfo != nil { + regReq.Hostinfo.NetInfo.PreferredDERP = 2 + } + + stored, ok := s.GetNodeByID(node.ID()) + require.True(t, ok) + require.Equal(t, "original", stored.Hostinfo().Hostname()) + require.Equal(t, uint16(80), stored.Hostinfo().Services().At(0).Port) + require.Equal(t, tailcfg.DERPRegionID(1), stored.Hostinfo().NetInfo().PreferredDERP()) + }) + } +} diff --git a/hscontrol/state/maprequest_test.go b/hscontrol/state/maprequest_test.go index fe384abdb..f5ba839ce 100644 --- a/hscontrol/state/maprequest_test.go +++ b/hscontrol/state/maprequest_test.go @@ -21,6 +21,43 @@ import ( "tailscale.com/types/opt" ) +func TestMapRequestOwnsRequestData(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + node, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + + req := tailcfg.MapRequest{ + NodeKey: node.NodeKey(), + DiscoKey: node.DiscoKey(), + Endpoints: []netip.AddrPort{netip.MustParseAddrPort("192.0.2.1:41641")}, + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "original", + Services: []tailcfg.Service{{Proto: "tcp", Port: 80}}, + NetInfo: &tailcfg.NetInfo{ + PreferredDERP: 1, + DERPLatency: map[string]float64{"1-v4": 0.1}, + }, + }, + } + original := req.Hostinfo.Clone() + _, err := s.UpdateNodeFromMapRequest(nodeID, req) + require.NoError(t, err) + require.Equal(t, original, req.Hostinfo, "map update must not modify its request") + + req.Hostinfo.Hostname = "changed" + req.Hostinfo.Services[0].Port = 443 + req.Hostinfo.NetInfo.PreferredDERP = 2 + req.Hostinfo.NetInfo.DERPLatency["1-v4"] = 0.2 + req.Endpoints[0] = netip.MustParseAddrPort("192.0.2.2:41641") + + stored, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + require.Equal(t, original, stored.Hostinfo().AsStruct()) + require.Equal(t, netip.MustParseAddrPort("192.0.2.1:41641"), stored.Endpoints().At(0)) +} + func TestNetInfoFromMapRequest(t *testing.T) { nodeID := types.NodeID(1) diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 152ecf643..09f4fd51d 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -2776,8 +2776,7 @@ func (s *State) HandleNodeFromPreAuthKey( } // Ensure we have valid hostinfo - validHostinfo := cmp.Or(regReq.Hostinfo, &tailcfg.Hostinfo{}) - validHostinfo.Hostname = hostname + validHostinfo := cmp.Or(regReq.Hostinfo.Clone(), &tailcfg.Hostinfo{}) log.Debug(). Caller(). @@ -3520,7 +3519,8 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest currentNode.ApplyPeerChange(&delta.peerChange) if hostinfoToStore { - currentNode.Hostinfo = newHostinfo + // The snapshot outlives the request; deep-copy so its slices and NetInfo are never shared. + currentNode.Hostinfo = newHostinfo.Clone() } // Only a real hostname change may re-derive GivenName: it is peer diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 06d255ebe..e85f80b57 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -661,7 +661,7 @@ func (node *Node) PeerChangeFromMapRequest(req tailcfg.MapRequest) tailcfg.PeerC // Compare endpoints using order-independent comparison if EndpointsChanged(node.Endpoints, req.Endpoints) { - ret.Endpoints = req.Endpoints + ret.Endpoints = slices.Clone(req.Endpoints) } now := time.Now()