diff --git a/hscontrol/auth.go b/hscontrol/auth.go index 7f50f8074..cbb499e75 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 c7efc162b..c98912e88 100644 --- a/hscontrol/auth_test.go +++ b/hscontrol/auth_test.go @@ -4327,3 +4327,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/grpcv1.go b/hscontrol/grpcv1.go index 0ce578b6d..b84eef62e 100644 --- a/hscontrol/grpcv1.go +++ b/hscontrol/grpcv1.go @@ -486,6 +486,10 @@ func (api headscaleV1APIServer) ExpireNode( node, nodeChange, err := api.h.state.SetNodeExpiry( types.NodeID(request.GetNodeId()), &expiry, ) + if errors.Is(err, types.ErrExpiryOutOfRange) { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + if err != nil { return nil, err } diff --git a/hscontrol/grpcv1_test.go b/hscontrol/grpcv1_test.go index 1bb4cbe2d..cd8998951 100644 --- a/hscontrol/grpcv1_test.go +++ b/hscontrol/grpcv1_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/timestamppb" "tailscale.com/tailcfg" "tailscale.com/types/key" ) @@ -817,3 +818,36 @@ func TestDeleteApiKey_BothIdentifiers(t *testing.T) { assert.Equal(t, codes.InvalidArgument, st.Code()) assert.Contains(t, st.Message(), "provide either id or prefix, not both") } + +// TestExpireNode_RefusesUnencodableExpiry proves ExpireNode refuses a key +// expiry outside the years [0,9999] in UTC as invalid input, which +// grpc-gateway answers with 400. +func TestExpireNode_RefusesUnencodableExpiry(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + apiServer := newHeadscaleV1APIServer(app) + + user := app.state.CreateUserForTest("expire-range-user") + node := app.state.CreateRegisteredNodeForTest(user, "expire-range-node") + app.state.PutNodeInStoreForTest(*node) + + beyond, err := time.Parse(time.RFC3339, "9999-12-31T23:59:59-23:59") + require.NoError(t, err) + + _, err = apiServer.ExpireNode(context.Background(), &v1.ExpireNodeRequest{ + NodeId: uint64(node.ID), + Expiry: timestamppb.New(beyond), + }) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok, "error should be a gRPC status error") + assert.Equal(t, codes.InvalidArgument, st.Code()) + + latest := time.Date(9999, time.December, 31, 23, 59, 59, 0, time.UTC) + _, err = apiServer.ExpireNode(context.Background(), &v1.ExpireNodeRequest{ + NodeId: uint64(node.ID), + Expiry: timestamppb.New(latest), + }) + require.NoError(t, err) +} diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go index c76baccf3..321de564c 100644 --- a/hscontrol/state/persist_test.go +++ b/hscontrol/state/persist_test.go @@ -710,3 +710,111 @@ func TestUpdatePolicyManagerUsersUnchangedKeepsSnapshot(t *testing.T) { require.NotSame(t, before, s.nodeStore.data.Load(), "a user change must rebuild the peer map") } + +// 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 882edb8ff..b4946bf56 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() @@ -912,6 +918,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 // Update [NodeStore] before database to ensure consistency. The [NodeStore] update @@ -2216,6 +2231,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 + } + // Get the registration entry from cache regEntry, ok := s.GetAuthCacheEntry(authID) if !ok { diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 35a3ede0f..ac4fb849e 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -619,6 +619,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 {