diff --git a/CHANGELOG.md b/CHANGELOG.md index de3e52204..cc2aa6971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -157,6 +157,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`, - Require a valid pre-auth key to re-register an expired tagged node - Another user's key or login on a tagged machine now creates a new device for that user instead of re-authenticating the tagged node - Fix one node with a stored key expiry outside the years 0 to 9999 in UTC failing the map of every node that can see it; it is now left out of its peers' maps and the startup check names it with the fix command +- Refuse a key expiry outside the years 0 to 9999 in UTC on registration, on the OIDC token-expiry path and in `headscale nodes expire` / `POST /api/v1/node/{nodeId}/expire` (400); the latest accepted expiry no longer fails node listings on PostgreSQL in a zone east of UTC ## 0.29.7 (202x-xx-xx) diff --git a/hscontrol/api/v1/errors.go b/hscontrol/api/v1/errors.go index 478a0e450..87ef93de2 100644 --- a/hscontrol/api/v1/errors.go +++ b/hscontrol/api/v1/errors.go @@ -6,6 +6,7 @@ import ( "github.com/danielgtaylor/huma/v2" "github.com/juanfont/headscale/hscontrol/db" "github.com/juanfont/headscale/hscontrol/state" + "github.com/juanfont/headscale/hscontrol/types" "gorm.io/gorm" ) @@ -33,6 +34,7 @@ func mapError(msg string, err error) error { errors.Is(err, state.ErrNodeMarkedTaggedButHasNoTags), errors.Is(err, state.ErrNodeHasNeitherUserNorTags), errors.Is(err, state.ErrRequestedTagsInvalidOrNotPermitted), + errors.Is(err, types.ErrExpiryOutOfRange), errors.Is(err, db.ErrUserStillHasNodes), errors.Is(err, db.ErrCannotChangeOIDCUser), errors.Is(err, db.ErrPreAuthKeyNotTaggedOrOwned), diff --git a/hscontrol/apiv1_nodes_test.go b/hscontrol/apiv1_nodes_test.go index 3a58addcc..d6197dc01 100644 --- a/hscontrol/apiv1_nodes_test.go +++ b/hscontrol/apiv1_nodes_test.go @@ -201,6 +201,15 @@ func TestAPIV1NodeExpire(t *testing.T) { assert.NotNil(t, got.Node["expiry"]) }) + t.Run("huma refuses an unencodable expiry", func(t *testing.T) { + h := newAPIV1Harness(t) + seedNodes(newNodeSeed("alice", "node-a"))(t, h.app) + + res := h.callHuma(http.MethodPost, "/api/v1/node/1/expire", + []byte(`{"expiry":"9999-12-31T23:59:59-23:59"}`)) + assertStatus(t, res, http.StatusBadRequest) + }) + t.Run("not found parity", func(t *testing.T) { h := newAPIV1Harness(t) res := h.assertParity(t, http.MethodPost, "/api/v1/node/99999/expire", nil) diff --git a/hscontrol/auth.go b/hscontrol/auth.go index b2b0c6e8f..99b8720dd 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -40,6 +40,14 @@ func (h *Headscale) handleRegister( req tailcfg.RegisterRequest, machineKey key.MachinePublic, ) (*tailcfg.RegisterResponse, error) { + // Every path below may store the client's expiry and send it to peers. + expiry, err := types.ValidateExpiry(req.Expiry) + if err != nil { + return nil, NewHTTPError(http.StatusBadRequest, err.Error(), err) + } + + req.Expiry = expiry + // Check for logout/expiry FIRST, before checking auth key. // Tailscale clients may send logout requests with BOTH a past expiry AND an auth key. // A past expiry takes precedence - it's a logout regardless of other fields. diff --git a/hscontrol/auth_test.go b/hscontrol/auth_test.go index 380ee1f83..053471b92 100644 --- a/hscontrol/auth_test.go +++ b/hscontrol/auth_test.go @@ -4544,3 +4544,57 @@ func TestFollowupWaitPrefersCompletedAuthOverExpiredContext(t *testing.T) { timeouts, iterations) assert.Equal(t, iterations, authorized, "every completed registration must be returned as authorized") } + +// TestRegisterRejectsUnencodableExpiry proves a registration cannot store a key +// expiry that is in range in its own zone but outside the years [0,9999] in +// UTC, which every map carrying the node would fail to encode. +func TestRegisterRejectsUnencodableExpiry(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("expiry-range-user") + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + + register := func(expiry time.Time) (key.NodePublic, error) { + nodeKey := key.NewNode().Public() + + _, err := app.handleRegister(context.Background(), tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey, + Hostinfo: &tailcfg.Hostinfo{Hostname: "expiry-range-node"}, + Expiry: expiry, + }, key.NewMachine().Public()) + + return nodeKey, err + } + + for _, raw := range []string{ + "9999-12-31T23:59:59-23:59", + "0000-01-01T00:00:00+23:59", + } { + expiry, err := time.Parse(time.RFC3339, raw) + require.NoError(t, err) + + nodeKey, err := register(expiry) + + httpErr, ok := errors.AsType[HTTPError](err) + require.True(t, ok, "expiry %s: want an HTTPError, got %v", raw, err) + require.Equal(t, http.StatusBadRequest, httpErr.Code) + + _, found := app.state.GetNodeByNodeKey(nodeKey) + require.False(t, found, "expiry %s must not create a node", raw) + } + + // The latest encodable expiry still registers, and SQLite reads it back + // although it is in the year 10000 east of UTC. + latest := time.Date(9999, time.December, 31, 23, 59, 59, 0, time.UTC) + nodeKey, err := register(latest.In(time.FixedZone("", 60*60))) + require.NoError(t, err, "the latest encodable expiry must still register") + + node, found := app.state.GetNodeByNodeKey(nodeKey) + require.True(t, found) + + stored, err := app.state.DB().GetNodeByID(node.ID()) + require.NoError(t, err) + assert.True(t, latest.Equal(*stored.Expiry)) +} diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go index d8d1d9c8f..4b9d69e7b 100644 --- a/hscontrol/state/persist_test.go +++ b/hscontrol/state/persist_test.go @@ -1260,3 +1260,111 @@ func TestBackfillNodeIPsSurvivesConcurrentPersist(t *testing.T) { require.NoError(t, err) assert.Nil(t, dbNode.IPv6, "database kept the removed IPv6 address") } + +// TestSetNodeExpiryRejectsUnencodableExpiry proves an admin cannot store a key +// expiry that every map carrying the node would fail to encode. +func TestSetNodeExpiryRejectsUnencodableExpiry(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + beyond, err := time.Parse(time.RFC3339, "9999-12-31T23:59:59-23:59") + require.NoError(t, err) + + before := s.nodeStore.data.Load() + + _, _, err = s.SetNodeExpiry(nodeID, &beyond) + require.ErrorIs(t, err, types.ErrExpiryOutOfRange) + require.Same(t, before, s.nodeStore.data.Load(), + "a refused expiry must not reach the NodeStore") + + latest := time.Date(9999, time.December, 31, 23, 59, 59, 0, time.UTC) + _, _, err = s.SetNodeExpiry(nodeID, &latest) + require.NoError(t, err) +} + +// TestAuthPathRejectsUnencodableExpiry proves an expiry handed to the auth +// path, such as an OIDC token's, cannot store what SetNodeExpiry refuses. +func TestAuthPathRejectsUnencodableExpiry(t *testing.T) { + _, s, _ := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + user := s.CreateUserForTest("auth-path-user") + nodeKey := key.NewNode().Public() + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: key.NewMachine().Public(), + NodeKey: nodeKey, + Hostname: "auth-path-node", + })) + + // How go-oidc reads an exp claim in the year 10000. + beyond := time.Unix(253402300800, 0) + + _, _, err := s.HandleNodeFromAuthPath( + authID, types.UserID(user.ID), &beyond, util.RegisterMethodOIDC, + ) + require.ErrorIs(t, err, types.ErrExpiryOutOfRange) + + _, found := s.GetNodeByNodeKey(nodeKey) + require.False(t, found, "a refused expiry must not create a node") +} + +// TestAcceptedExpirySurvivesRestart proves an accepted expiry that is already in +// the year 10000 in its own zone is stored so SQLite can load it at the next +// start. go-oidc reads a token's exp in the server's zone, east of UTC here. +func TestAcceptedExpirySurvivesRestart(t *testing.T) { + latest := time.Date(9999, time.December, 31, 23, 59, 59, 0, time.UTC) + east := latest.In(time.FixedZone("", 60*60)) + + dbPath, s, adminID := persistTestSetup(t) + + _, _, err := s.SetNodeExpiry(adminID, &east) + require.NoError(t, err) + + user := s.CreateUserForTest("oidc-user") + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: key.NewMachine().Public(), + NodeKey: key.NewNode().Public(), + Hostname: "oidc-node", + })) + + oidcNode, _, err := s.HandleNodeFromAuthPath( + authID, types.UserID(user.ID), &east, util.RegisterMethodOIDC, + ) + require.NoError(t, err) + require.NoError(t, s.Close()) + + s = persistTestReopen(t, dbPath) + + for _, id := range []types.NodeID{adminID, oidcNode.ID()} { + nv, ok := s.GetNodeByID(id) + require.True(t, ok) + assert.True(t, latest.Equal(nv.Expiry().Get()), "node %d", id) + } +} + +// TestLoadedExpiryIsUTC proves an expiry the database returns in another zone is +// served in UTC. PostgreSQL returns the server's zone, where the latest accepted +// expiry is in the year 10000 and cannot be encoded. +func TestLoadedExpiryIsUTC(t *testing.T) { + dbPath, s, nodeID := persistTestSetup(t) + require.NoError(t, s.Close()) + + expiry := time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) + + database, err := db.NewHeadscaleDatabase(persistTestConfig(dbPath)) + require.NoError(t, err) + require.NoError(t, database.DB.Model(&types.Node{}).Where("id = ?", nodeID). + Update("expiry", expiry.In(time.FixedZone("", 60*60))).Error) + require.NoError(t, database.Close()) + + s = persistTestReopen(t, dbPath) + + nv, ok := s.GetNodeByID(nodeID) + require.True(t, ok) + + got := nv.Expiry().Get() + assert.True(t, expiry.Equal(got)) + assert.Equal(t, time.UTC, got.Location()) +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 3a4ebf412..cd1589d6d 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -253,6 +253,12 @@ func NewState(cfg *types.Config) (*State, error) { // This ensures we don't have stale online status from previous runs for _, node := range nodes { node.IsOnline = new(false) + + // PostgreSQL returns the server's zone, where the latest accepted + // expiry is in the year 10000 and cannot be encoded. + if node.Expiry != nil { + node.Expiry = new(node.Expiry.UTC()) + } } users, err := db.ListUsers(nil) @@ -946,6 +952,15 @@ func (s *State) ListEphemeralNodes() views.Slice[types.NodeView] { // SetNodeExpiry updates the expiration time for a node. // If expiry is nil, the node's expiry is disabled (node will never expire). func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.NodeView, change.Change, error) { + if expiry != nil { + utc, err := types.ValidateExpiry(*expiry) + if err != nil { + return types.NodeView{}, change.Change{}, err + } + + expiry = &utc + } + var onlineChanged bool genBefore := s.polMan.NodesGeneration() @@ -2462,6 +2477,16 @@ func (s *State) HandleNodeFromAuthPath( expiry *time.Time, registrationMethod string, ) (types.NodeView, change.Change, error) { + // An OIDC token's expiry arrives here without passing handleRegister. + if expiry != nil { + utc, err := types.ValidateExpiry(*expiry) + if err != nil { + return types.NodeView{}, change.Change{}, err + } + + expiry = &utc + } + // Read before any NodeStore write below; see updatePolicyManagerNodes. genBefore := s.polMan.NodesGeneration() diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 4c949e601..fb5834b02 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -560,6 +560,10 @@ func ValidateGivenName(givenName, baseDomain string) error { // or an error when it cannot be sent to clients. [time.Time.MarshalJSON] refuses // a year outside [0,9999], failing every map response that carries it. A time // that is in range in its own zone can still leave the range in UTC. +// +// Callers store the returned value: SQLite writes a time in its own zone and +// cannot read back a year outside [0,9999], which that zone can reach when UTC +// does not. func ValidateExpiry(expiry time.Time) (time.Time, error) { utc := expiry.UTC() if year := utc.Year(); year < 0 || year > 9999 {