diff --git a/hscontrol/auth.go b/hscontrol/auth.go index cbb499e75..d703613fb 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/url" + "slices" "strings" "time" @@ -390,25 +391,32 @@ func (h *Headscale) reqToNewRegisterResponse( }, nil } -// registrationDataFromRequest builds the [types.RegistrationData] payload stored -// in the auth cache for a pending registration. The original [tailcfg.Hostinfo] is -// retained so that consumers (auth callback, observability) see the -// fields the client originally announced; the bounded-LRU cap on the -// cache is what bounds the unauthenticated cache-fill DoS surface. +// registrationDataFromRequest keeps only auth and confirmation metadata; +// operational metadata arrives with the node's first MapRequest. func registrationDataFromRequest( req tailcfg.RegisterRequest, machineKey key.MachinePublic, ) *types.RegistrationData { - var hostname string + var ( + hostname string + hostinfo *tailcfg.Hostinfo + ) + if req.Hostinfo != nil { hostname = req.Hostinfo.Hostname + + hostinfo = &tailcfg.Hostinfo{ + Hostname: hostname, + OS: req.Hostinfo.OS, + RequestTags: slices.Clone(req.Hostinfo.RequestTags), + } } regData := &types.RegistrationData{ MachineKey: machineKey, NodeKey: req.NodeKey, Hostname: hostname, - Hostinfo: req.Hostinfo, + Hostinfo: hostinfo, } if !req.Expiry.IsZero() { diff --git a/hscontrol/auth_test.go b/hscontrol/auth_test.go index c98912e88..12f8a9248 100644 --- a/hscontrol/auth_test.go +++ b/hscontrol/auth_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "net/netip" "net/url" "strings" "testing" @@ -1420,15 +1421,8 @@ func TestAuthenticationFlows(t *testing.T) { }, // === AUTH PROVIDER EDGE CASES === - // TEST: Interactive workflow preserves custom hostinfo - // WHAT: Tests that custom hostinfo fields are preserved through interactive flow - // INPUT: Interactive registration with detailed hostinfo (OS, version, model) - // EXPECTED: Node registers with all hostinfo fields preserved - // WHY: Ensures interactive flow doesn't lose custom hostinfo data - // NOTE: RequestTags are NOT tested here because tag authorization via - // advertise-tags requires the user to have existing nodes (for IP-based - // ownership verification). New users registering their first node cannot - // claim tags via RequestTags - they must use a tagged PreAuthKey instead. + // Interactive registration retains auth metadata. The first map request + // supplies the remaining Hostinfo fields. { name: "interactive_workflow_with_custom_hostinfo", setupFunc: func(t *testing.T, app *Headscale) (string, error) { //nolint:thelper @@ -1455,15 +1449,29 @@ func TestAuthenticationFlows(t *testing.T) { validateCompleteResponse: true, expectedAuthURLPattern: "/register/", validate: func(t *testing.T, resp *tailcfg.RegisterResponse, app *Headscale) { //nolint:thelper - // Verify custom hostinfo was preserved through interactive workflow + // Confirm the minimal cached report, then restore the operational fields. node, found := app.state.GetNodeByNodeKey(nodeKey1.Public()) assert.True(t, found, "node should be found after interactive registration") if found { assert.Equal(t, "custom-interactive-node", node.Hostname()) assert.Equal(t, "linux", node.Hostinfo().OS()) - assert.Equal(t, "20.04", node.Hostinfo().OSVersion()) - assert.Equal(t, "server", node.Hostinfo().DeviceModel()) + assert.Empty(t, node.Hostinfo().OSVersion()) + assert.Empty(t, node.Hostinfo().DeviceModel()) + + _, err := app.state.UpdateNodeFromMapRequest(node.ID(), tailcfg.MapRequest{ + NodeKey: node.NodeKey(), DiscoKey: node.DiscoKey(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: node.Hostname(), OS: "linux", + OSVersion: "20.04", DeviceModel: "server", + }, + }) + require.NoError(t, err) + + updated, ok := app.state.GetNodeByID(node.ID()) + require.True(t, ok) + assert.Equal(t, "20.04", updated.Hostinfo().OSVersion()) + assert.Equal(t, "server", updated.Hostinfo().DeviceModel()) } }, }, @@ -4120,14 +4128,14 @@ func TestHandleNodeFromAuthPath_ReloginWholeNode(t *testing.T) { tests := []struct { name string expired bool - services []tailcfg.Service + hostname string wantPatch bool }{ {name: "keys only", wantPatch: true}, {name: "expired", expired: true}, { - name: "peer-visible hostinfo", - services: []tailcfg.Service{{Proto: tailcfg.PeerAPI4, Port: 4242}}, + name: "peer-visible hostname", + hostname: "renamed-node", }, } @@ -4148,15 +4156,19 @@ func TestHandleNodeFromAuthPath_ReloginWholeNode(t *testing.T) { require.NoError(t, app.state.UpdatePolicyManagerUsersForTest()) require.NoError(t, app.state.UpdatePolicyManagerNodesForTest()) + hostname := node.Hostname + if tt.hostname != "" { + hostname = tt.hostname + } + newNodeKey := key.NewNode() authID := types.MustAuthID() app.state.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ MachineKey: node.MachineKey, NodeKey: newNodeKey.Public(), - Hostname: node.Hostname, + Hostname: hostname, Hostinfo: &tailcfg.Hostinfo{ - Hostname: node.Hostname, - Services: tt.services, + Hostname: hostname, }, })) @@ -4381,3 +4393,32 @@ func TestRegisterRejectsUnencodableExpiry(t *testing.T) { require.NoError(t, err) assert.True(t, latest.Equal(*stored.Expiry)) } + +// TestPendingRegistrationKeepsAuthMetadata pins that operational reports do +// not stay in the auth cache before the first MapRequest. +func TestPendingRegistrationKeepsAuthMetadata(t *testing.T) { + app := createTestApp(t) + hostinfo := &tailcfg.Hostinfo{ + Hostname: "pending-router", OS: "linux", OSVersion: "6.12.0", + RequestTags: []string{"tag:router"}, + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")}, + Services: []tailcfg.Service{{Proto: tailcfg.PeerAPI4, Port: 41641}}, + NetInfo: &tailcfg.NetInfo{PreferredDERP: 1}, + SSH_HostKeys: []string{"ssh-ed25519 key"}, + } + resp, err := app.handleRegister(context.Background(), tailcfg.RegisterRequest{ + NodeKey: key.NewNode().Public(), Hostinfo: hostinfo, + }, key.NewMachine().Public()) + require.NoError(t, err) + authID, err := extractRegistrationIDFromAuthURL(resp.AuthURL) + require.NoError(t, err) + + entry, ok := app.state.GetAuthCacheEntry(authID) + require.True(t, ok) + assert.Equal(t, &tailcfg.Hostinfo{ + Hostname: "pending-router", OS: "linux", RequestTags: []string{"tag:router"}, + }, entry.RegistrationData().Hostinfo) + hostinfo.RequestTags[0] = "tag:changed" + assert.Equal(t, []string{"tag:router"}, entry.RegistrationData().Hostinfo.RequestTags) + assert.Len(t, hostinfo.RoutableIPs, 1, "the request itself must be left alone") +} diff --git a/hscontrol/noise_test.go b/hscontrol/noise_test.go index a60de33bc..cb24c69a0 100644 --- a/hscontrol/noise_test.go +++ b/hscontrol/noise_test.go @@ -1421,6 +1421,42 @@ func serveRegister(t *testing.T, ns *noiseServer, body any) *httptest.ResponseRe func TestRegistrationHandler_ValidatesClientReport(t *testing.T) { t.Parallel() + t.Run("pending registration retains auth metadata", func(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + ns := &noiseServer{headscale: app, machineKey: key.NewMachine().Public()} + + rec := serveRegister(t, ns, tailcfg.RegisterRequest{ + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "report-pending-node", + OS: "linux", + OSVersion: "6.12", + RequestTags: []string{"tag:test"}, + Services: manyServices(300), + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, + NetInfo: &tailcfg.NetInfo{PreferredDERP: 1}, + }, + }) + require.Equal(t, http.StatusOK, rec.Code, "body=%q", rec.Body.String()) + + var resp tailcfg.RegisterResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + + authID, err := extractRegistrationIDFromAuthURL(resp.AuthURL) + require.NoError(t, err, "resp=%+v", resp) + + entry, ok := app.state.GetAuthCacheEntry(authID) + require.True(t, ok) + assert.Equal(t, &tailcfg.Hostinfo{ + Hostname: "report-pending-node", + OS: "linux", + RequestTags: []string{"tag:test"}, + }, entry.RegistrationData().Hostinfo) + }) + t.Run("oversized report", func(t *testing.T) { t.Parallel() diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 4039af50c..0d32203b8 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -999,6 +999,11 @@ func (a *AuthProviderOIDC) handleRegistration( util.RegisterMethodOIDC, ) if err != nil { + if errors.Is(err, types.ErrHostinfoTooLarge) { + return false, newHTTPUserError(http.StatusBadRequest, "node report too large", + "This device reports too much data. Ask your administrator to reduce its reported metadata, then sign in again.", err) + } + return false, fmt.Errorf("registering node: %w", err) } diff --git a/hscontrol/oidc_test.go b/hscontrol/oidc_test.go index bad068170..e1a9f8baa 100644 --- a/hscontrol/oidc_test.go +++ b/hscontrol/oidc_test.go @@ -2,12 +2,14 @@ package hscontrol import ( "context" + "encoding/json" "io" "net/http" "net/http/cookiejar" "net/http/httptest" "net/url" "regexp" + "strings" "testing" "time" @@ -17,6 +19,7 @@ import ( "github.com/oauth2-proxy/mockoidc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/tailcfg" "tailscale.com/types/key" ) @@ -616,3 +619,43 @@ func TestRouterMethodNotAllowedIncludesAllow(t *testing.T) { assert.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode) assert.Equal(t, []string{http.MethodGet}, resp.Header.Values("Allow")) } + +func TestOIDCRegistrationRejectsOversizedMergedHostinfo(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + user := app.state.CreateUserForTest("oidc-report-user") + node := app.state.CreateNodeForTest(user, "oidc-report-node") + node.Hostinfo = &tailcfg.Hostinfo{Hostname: node.Hostname, OSVersion: "v"} + encoded, err := json.Marshal(node.Hostinfo) + require.NoError(t, err) + + node.Hostinfo.OSVersion = strings.Repeat("v", types.MaxHostinfoSize-len(encoded)) + require.NoError(t, types.ValidateHostinfo(node.Hostinfo)) + before := app.state.PutNodeInStoreForTest(*node) + + hostname := strings.Repeat("h", types.MaxHostnameLength) + authID := types.MustAuthID() + app.state.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: node.MachineKey, NodeKey: key.NewNode().Public(), + Hostname: hostname, Hostinfo: &tailcfg.Hostinfo{Hostname: hostname}, + })) + + provider := &AuthProviderOIDC{h: app} + changed, err := provider.handleRegistration(user, authID, nil) + require.ErrorIs(t, err, types.ErrHostinfoTooLarge) + + var httpErr HTTPError + require.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusBadRequest, httpErr.Code) + assert.Contains(t, strings.ToLower(httpErr.UserMsg), "reduce") + assert.Contains(t, httpErr.UserMsg, "metadata") + assert.Contains(t, httpErr.UserMsg, "sign") + assert.False(t, changed) + + after, ok := app.state.GetNodeByID(node.ID) + require.True(t, ok) + assert.Equal(t, before.NodeKey(), after.NodeKey()) + assert.Equal(t, before.Hostname(), after.Hostname()) + assert.True(t, before.Hostinfo().AsStruct().Equal(after.Hostinfo().AsStruct())) +} diff --git a/hscontrol/servertest/lifecycle_test.go b/hscontrol/servertest/lifecycle_test.go index 2f33d4698..4bde06bcc 100644 --- a/hscontrol/servertest/lifecycle_test.go +++ b/hscontrol/servertest/lifecycle_test.go @@ -324,9 +324,8 @@ func TestReloginOfExpiredNodeClearsPeerExpiry(t *testing.T) { } // TestReloginPeerVisibleHostinfoReachesPeers covers the rest of the #3531 -// class: a relogin stores the RegisterRequest's Hostinfo, so the following -// MapRequest carries no Hostinfo delta, and a peer-visible change made at -// relogin reaches peers only if the relogin itself sends the whole node. +// class: peers must see changed services after the first MapRequest following +// relogin. A key-only patch cannot carry that change. func TestReloginPeerVisibleHostinfoReachesPeers(t *testing.T) { t.Parallel() diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go index a4e7e271c..03749165e 100644 --- a/hscontrol/state/persist_test.go +++ b/hscontrol/state/persist_test.go @@ -1,6 +1,7 @@ package state import ( + "encoding/json" "errors" "net/netip" "slices" @@ -325,7 +326,6 @@ func TestReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) { Hostname: "attacker", Hostinfo: &tailcfg.Hostinfo{}, }, - ValidHostinfo: &tailcfg.Hostinfo{}, Hostname: "attacker", User: attacker, RegisterMethod: util.RegisterMethodCLI, @@ -386,7 +386,6 @@ func TestReauthPreservesEndpointsWhenClientOmitsThem(t *testing.T) { Hostinfo: &tailcfg.Hostinfo{}, Endpoints: nil, }, - ValidHostinfo: &tailcfg.Hostinfo{}, Hostname: "node", User: user, RegisterMethod: util.RegisterMethodCLI, @@ -975,3 +974,176 @@ func TestBootKeepsHostinfoTheMapPathStored(t *testing.T) { osVersionLen := len(reloaded.Hostinfo().OSVersion()) assert.Equal(t, 60<<10, osVersionLen) } + +// TestReauthKeepsOperationalMetadata covers a relogin before the next map request. +// Pending registrations carry only auth metadata; live connectivity must survive. +func TestReauthKeepsOperationalMetadata(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + route := netip.MustParsePrefix("10.0.0.0/24") + + node, ok := s.nodeStore.UpdateNode(nodeID, func(n *types.Node) { + n.Hostinfo = &tailcfg.Hostinfo{ + Hostname: n.Hostname, RoutableIPs: []netip.Prefix{route}, + OS: "linux", OSVersion: "6.12.0", + Services: []tailcfg.Service{{Proto: tailcfg.PeerAPI4, Port: 41641}}, + SSH_HostKeys: []string{"ssh-ed25519 key"}, + NetInfo: &tailcfg.NetInfo{PreferredDERP: 1}, + } + n.DiscoKey = key.NewDisco().Public() + n.Endpoints = []netip.AddrPort{netip.MustParseAddrPort("192.0.2.1:41641")} + n.ApprovedRoutes = []netip.Prefix{route} + }) + require.True(t, ok) + + _, epoch := s.Connect(nodeID) + require.NotZero(t, epoch) + require.Equal(t, []netip.Prefix{route}, s.GetNodePrimaryRoutes(nodeID), + "precondition: the node serves its route") + + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: node.MachineKey(), + NodeKey: key.NewNode().Public(), + Hostname: node.Hostname(), + Hostinfo: &tailcfg.Hostinfo{Hostname: node.Hostname()}, + })) + + relogged, _, err := s.HandleNodeFromAuthPath( + authID, node.TypedUserID(), nil, util.RegisterMethodOIDC, + ) + require.NoError(t, err) + require.Equal(t, nodeID, relogged.ID(), "relogin must update the existing node") + + assert.Equal(t, []netip.Prefix{route}, relogged.AnnouncedRoutes()) + assert.Equal(t, []netip.Prefix{route}, s.GetNodePrimaryRoutes(nodeID)) + assert.True(t, node.Hostinfo().Equal(relogged.Hostinfo())) + assert.Equal(t, node.DiscoKey(), relogged.DiscoKey()) + assert.Equal(t, node.Endpoints(), relogged.Endpoints()) +} + +func TestReauthKeepsTaggedFutureExpiry(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + _, err := s.SetPolicy([]byte(`{"tagOwners":{"tag:ci":["persist-user@"]}}`)) + require.NoError(t, err) + + node, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + + userID := node.TypedUserID() + _, _, err = s.SetNodeTags(nodeID, []string{"tag:ci"}) + require.NoError(t, err) + + future := time.Now().Add(time.Hour).UTC() + node, ok = s.nodeStore.UpdateNode(nodeID, func(n *types.Node) { + n.Expiry = &future + }) + require.True(t, ok) + require.True(t, node.IsTagged()) + + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: node.MachineKey(), NodeKey: key.NewNode().Public(), + Hostname: node.Hostname(), + Hostinfo: &tailcfg.Hostinfo{Hostname: node.Hostname(), RequestTags: []string{"tag:ci"}}, + })) + relogged, _, err := s.HandleNodeFromAuthPath(authID, userID, nil, util.RegisterMethodOIDC) + require.NoError(t, err) + require.Equal(t, nodeID, relogged.ID()) + require.True(t, relogged.IsTagged()) + require.True(t, relogged.Expiry().Valid()) + assert.True(t, future.Equal(relogged.Expiry().Get())) + + row, err := s.DB().GetNodeByID(nodeID) + require.NoError(t, err) + require.NotNil(t, row.Expiry) + assert.True(t, future.Equal(*row.Expiry)) +} + +func TestReauthReplacesReportedTags(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + _, err := s.SetPolicy([]byte(`{"tagOwners":{"tag:a":["persist-user@"],"tag:b":["persist-user@"]}}`)) + require.NoError(t, err) + + node, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + + userID := node.TypedUserID() + + for _, tags := range [][]string{{"tag:a"}, nil, {"tag:b"}} { + want := slices.Clone(tags) + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: node.MachineKey(), NodeKey: key.NewNode().Public(), + Hostname: node.Hostname(), + Hostinfo: &tailcfg.Hostinfo{Hostname: node.Hostname(), RequestTags: tags}, + })) + relogged, _, err := s.HandleNodeFromAuthPath(authID, userID, nil, util.RegisterMethodOIDC) + require.NoError(t, err) + require.Equal(t, nodeID, relogged.ID()) + assert.Equal(t, want, relogged.Hostinfo().RequestTags().AsSlice()) + assert.Equal(t, len(want) > 0, relogged.IsTagged()) + + if len(tags) > 0 { + tags[0] = "tag:changed" + } + + stored, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + assert.Equal(t, want, stored.Hostinfo().RequestTags().AsSlice(), + "cached tags must not alias stored Hostinfo") + + row, err := s.DB().GetNodeByID(nodeID) + require.NoError(t, err) + assert.Equal(t, want, row.Hostinfo.RequestTags) + } +} + +func TestReauthRejectsOversizedMergedHostinfo(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + node, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + + hi := &tailcfg.Hostinfo{Hostname: node.Hostname(), OSVersion: "v"} + encoded, err := json.Marshal(hi) + require.NoError(t, err) + + hi.OSVersion = strings.Repeat("v", types.MaxHostinfoSize-len(encoded)+1) + require.NoError(t, types.ValidateHostinfo(hi)) + _, err = s.UpdateNodeFromMapRequest(nodeID, tailcfg.MapRequest{ + NodeKey: node.NodeKey(), DiscoKey: node.DiscoKey(), Hostinfo: hi, + }) + require.NoError(t, err) + + before, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + + rowBefore, err := s.DB().GetNodeByID(nodeID) + require.NoError(t, err) + + hostname := strings.Repeat("h", types.MaxHostnameLength) + require.NoError(t, types.ValidateHostinfo(&tailcfg.Hostinfo{Hostname: hostname})) + + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: node.MachineKey(), NodeKey: key.NewNode().Public(), + Hostname: hostname, Hostinfo: &tailcfg.Hostinfo{Hostname: hostname}, + })) + _, _, err = s.HandleNodeFromAuthPath(authID, node.TypedUserID(), nil, util.RegisterMethodOIDC) + require.ErrorIs(t, err, types.ErrHostinfoTooLarge) + + after, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + assert.Equal(t, before.AsStruct(), after.AsStruct(), "rejected merge leaves the live node unchanged") + + rowAfter, err := s.DB().GetNodeByID(nodeID) + require.NoError(t, err) + assert.Equal(t, rowBefore, rowAfter, "rejected merge leaves the stored node unchanged") +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index a94a4073a..66c8fcddb 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -51,10 +51,8 @@ const ( registerCacheExpiration = time.Minute * 15 // defaultRegisterCacheMaxEntries is the default upper bound on the number - // of pending registration entries the auth cache will hold. With a 15-minute - // TTL and a stripped-down RegistrationData payload (~200 bytes per entry), - // 1024 entries cap the worst-case cache footprint at well under 1 MiB even - // under sustained unauthenticated cache-fill attempts. + // of pending registration entries the auth cache will hold. Entries keep + // only hostname, OS and requested tags. defaultRegisterCacheMaxEntries = 1024 // defaultNodeStoreBatchSize is the default number of write operations to batch @@ -1727,8 +1725,6 @@ type authNodeUpdateParams struct { ExistingNode types.NodeView // Cached registration payload from the originating client request. RegData *types.RegistrationData - // Pre-validated hostinfo; NetInfo preserved from ExistingNode. - ValidHostinfo *tailcfg.Hostinfo // Hostname from hostinfo, or generated from keys if client omits it. Hostname string // Auth user; may differ from ExistingNode.User() on conversion. @@ -1805,19 +1801,27 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView return types.NodeView{}, false, ErrNodeKeyInUse } - // Update existing node in [NodeStore] - validation passed, safe to mutate - before, updatedNodeView, ok := s.nodeStore.UpdateNodeDiff(params.ExistingNode.ID(), func(node *types.Node) { - node.NodeKey = regData.NodeKey - node.DiscoKey = regData.DiscoKey - node.Hostname = params.Hostname + var hostinfoErr error - // Preserve NetInfo from existing node when re-registering - node.Hostinfo = params.ValidHostinfo - node.Hostinfo.NetInfo = preserveNetInfo( - params.ExistingNode, - params.ExistingNode.ID(), - params.ValidHostinfo, - ) + // Read current operational metadata while holding the NodeStore writer lock. + before, updatedNodeView, ok := s.nodeStore.UpdateNodeDiff(params.ExistingNode.ID(), func(node *types.Node) { + hostinfo := cmp.Or(node.Hostinfo.Clone(), &tailcfg.Hostinfo{}) + + hostinfo.Hostname = params.Hostname + hostinfo.RequestTags = slices.Clone(requestTags) + + hostinfoErr = types.ValidateHostinfo(hostinfo) + if hostinfoErr != nil { + return + } + + node.NodeKey = regData.NodeKey + if !regData.DiscoKey.IsZero() { + node.DiscoKey = regData.DiscoKey + } + + node.Hostname = params.Hostname + node.Hostinfo = hostinfo // Preserve the node's live endpoints when the register request carried // none. Web/OIDC relogins report endpoints via MapRequest, not register, @@ -1851,34 +1855,11 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView isTagged := node.IsTagged() switch { - case wasTagged && !isTagged: - // Tagged → Personal: set expiry from client request - if params.Expiry != nil { - node.Expiry = params.Expiry - } else { - node.Expiry = regData.Expiry - } - case !wasTagged && isTagged: - // Personal → Tagged: clear expiry; tagged nodes disable it by default. - node.Expiry = nil - case params.IsConvertFromTag && !isTagged: - // Explicit conversion from tagged to user-owned: set expiry from client request - if params.Expiry != nil { - node.Expiry = params.Expiry - } else { - node.Expiry = regData.Expiry - } case !isTagged: - // Personal → Personal: update expiry from client - if params.Expiry != nil { - node.Expiry = params.Expiry - } else { - node.Expiry = regData.Expiry - } - case isTagged && node.IsExpired(): - // Tagged → Tagged and expired: a person re-authenticated it, so clear - // the past expiry. A future administrative expiry has not taken - // effect and falls through to the no-op below. + // User-owned nodes take the expiry from this authentication. + node.Expiry = cmp.Or(params.Expiry, regData.Expiry) + case !wasTagged || node.IsExpired(): + // Becoming tagged disables expiry; reauth clears a past expiry. node.Expiry = nil } // Tagged → Tagged and not expired: keep existing expiry - no action. @@ -1899,6 +1880,10 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView return types.NodeView{}, false, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, params.ExistingNode.ID()) } + if hostinfoErr != nil { + return types.NodeView{}, false, hostinfoErr + } + keysOnly := onlyKeysChanged(before, updatedNodeView, time.Now()) // Persist to database. @@ -2336,7 +2321,6 @@ func (s *State) HandleNodeFromAuthPath( // Common params for update operations updateParams := authNodeUpdateParams{ RegData: regData, - ValidHostinfo: hostinfo, Hostname: hostname, User: user, Expiry: expiry, diff --git a/hscontrol/types/registration.go b/hscontrol/types/registration.go index 669114d97..8ade4be1a 100644 --- a/hscontrol/types/registration.go +++ b/hscontrol/types/registration.go @@ -13,11 +13,9 @@ import ( // only the fields the registration callback path actually consumes when // promoting a pending registration to a real node. // -// Combined with the bounded-LRU cache that holds these entries, this caps -// the worst-case memory footprint of unauthenticated cache-fill attempts -// at (max_entries × per_entry_size). The cache is sized so that the -// product is bounded to a few MiB even with attacker-supplied 1 MiB -// Hostinfos (the Noise body limit). +// Any client can open a pending registration. The bounded-LRU cache that +// holds these entries caps their number. Hostinfo retains only auth and +// confirmation metadata. type RegistrationData struct { // MachineKey is the cryptographic identity of the machine being // registered. Required. @@ -34,14 +32,9 @@ type RegistrationData struct { // Already validated/normalised by EnsureHostname at producer time. Hostname string - // Hostinfo is the original [tailcfg.Hostinfo] from the [tailcfg.RegisterRequest], - // stored so that the auth callback can populate the new node's - // initial [tailcfg.Hostinfo] (and so that observability/CLI consumers see - // fields like OS, OSVersion, and IPNVersion before the first - // [tailcfg.MapRequest] restores the live set). - // - // May be nil if the client did not send [tailcfg.Hostinfo] in the original - // [tailcfg.RegisterRequest]. + // Hostinfo carries hostname and requested tags for auth, and OS for the + // confirmation page. Other fields arrive in the first MapRequest. + // It may be nil when the registration did not include Hostinfo. Hostinfo *tailcfg.Hostinfo // Endpoints is the initial set of WireGuard endpoints the node