mirror of
https://github.com/juanfont/headscale.git
synced 2026-10-30 18:24:06 +09:00
hscontrol: refuse a key expiry that cannot be encoded
handleRegister, the OIDC auth path and SetNodeExpiry refuse it. Expiries are stored in UTC, which SQLite can read back, and loaded in UTC, as PostgreSQL returns the server's zone. The v1 API answers 400. Fixes ANT-2026-QMCAFJ1Q
This commit is contained in:
1 parent
e3c1bfd68e
commit
15a8c7584e
8 files changed
+211
No files matched your search
@@ -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)
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
@@ -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())
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in new issue
Block a user