mirror of
https://git.vectorsigma.ru/public/headscale.git
synced 2026-08-04 04:48:06 +00:00
state: do not expire tagged nodes on logout
Tagged nodes never expire, but handleLogout stamped a past expiry on them, leaving them stuck expired and unable to re-authenticate. Fixes #3371
This commit is contained in:
Kristoffer Dalby
committed by
Kristoffer Dalby
Kristoffer Dalby
parent
5b6e1e17be
commit
1ed5693fa4
2
.github/workflows/test-integration.yaml
vendored
2
.github/workflows/test-integration.yaml
vendored
@@ -379,6 +379,8 @@ jobs:
|
||||
- TestTagsAuthKeyWithoutUserInheritsTags
|
||||
- TestTagsAuthKeyWithoutUserRejectsAdvertisedTags
|
||||
- TestTagsAuthKeyConvertToUserViaCLIRegister
|
||||
- TestTaggedNodeLogoutReloginSingleUseKeyOnline
|
||||
- TestTaggedNodeLogoutReloginReusableKeyOnline
|
||||
- TestTS2021WebSocketGET
|
||||
- TestTS2021WASMClientUnderNode
|
||||
- TestTailscaleRustAxum
|
||||
|
||||
@@ -45,6 +45,14 @@ keys remain all-access.
|
||||
- Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324)
|
||||
- Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341)
|
||||
|
||||
## 0.29.3 (202x-xx-xx)
|
||||
|
||||
**Minimum supported Tailscale client version: v1.80.0**
|
||||
|
||||
### Changes
|
||||
|
||||
- Fix tagged node stuck expired after `tailscale logout`, unable to re-authenticate [#3394](https://github.com/juanfont/headscale/pull/3394)
|
||||
|
||||
## 0.29.2 (2026-07-01)
|
||||
|
||||
**Minimum supported Tailscale client version: v1.80.0**
|
||||
|
||||
@@ -233,6 +233,20 @@ func (h *Headscale) handleLogout(
|
||||
Msg("Node is not ephemeral, setting expiry instead of deleting")
|
||||
}
|
||||
|
||||
// Tagged nodes have key expiry permanently disabled (they are owned by
|
||||
// their tags, not a user, and never expire - KB 1068). Logging one out has
|
||||
// no expiry semantics, so do not stamp an expiry on it: doing so leaves the
|
||||
// node IsExpired() forever and it can never re-authenticate (#3371). The
|
||||
// admin path `headscale nodes expire` remains free to set a deliberate
|
||||
// expiry via SetNodeExpiry; only the logout path is guarded here.
|
||||
if node.IsTagged() {
|
||||
log.Debug().
|
||||
EmbedObject(node).
|
||||
Msg("Tagged node logout: not stamping expiry (tagged nodes never expire)")
|
||||
|
||||
return nodeToRegisterResponse(node), nil
|
||||
}
|
||||
|
||||
// Update the internal state with the nodes new expiry, meaning it is
|
||||
// logged out.
|
||||
//
|
||||
|
||||
@@ -1337,3 +1337,392 @@ func TestReregistrationZeroExpiryStaysNil(t *testing.T) {
|
||||
assert.False(t, node2.Expiry().Valid(),
|
||||
"re-registration with zero client expiry and no default should leave expiry nil, not pointer to zero time")
|
||||
}
|
||||
|
||||
// tsLogoutSentinelExpiry is the past expiry a real tailscale client sends on
|
||||
// `tailscale logout`: time.Unix(123, 0) (controlclient/direct.go). The issue
|
||||
// #3371 trace shows it verbatim as `expiry=123`. Using it here (rather than a
|
||||
// generic time.Now().Add(-1h)) keeps the reproduction faithful to the wire
|
||||
// behaviour: handleRegister must classify this as a logout, and handleLogout
|
||||
// must clamp it to now.
|
||||
func tsLogoutSentinelExpiry() time.Time {
|
||||
return time.Unix(123, 0)
|
||||
}
|
||||
|
||||
// TestIssue3371_TaggedNodeLogoutReloginSingleUseKey reproduces
|
||||
// https://github.com/juanfont/headscale/issues/3371 through the real
|
||||
// register/logout HTTP-handler path (handleRegister -> handleLogout ->
|
||||
// handleRegister), not by poking SetNodeExpiry directly.
|
||||
//
|
||||
// Root cause (a): `tailscale logout` sends a past expiry; handleLogout stamps
|
||||
// it on the node via SetNodeExpiry with no IsTagged guard, so a tagged node —
|
||||
// which must have key-expiry disabled — becomes Expired.
|
||||
//
|
||||
// Root cause (b): on the next `tailscale up --auth-key <fresh key>`,
|
||||
// HandleNodeFromPreAuthKey sees an expired node, takes the expired-node
|
||||
// validation path, consumes the fresh single-use key on the in-place
|
||||
// re-registration, yet leaves the node expired (the expiry-refresh block is
|
||||
// gated `!node.IsTagged()`). The response carries NodeKeyExpired=true, so the
|
||||
// client rotates its node key and retries with the now-spent key, which is
|
||||
// rejected with "authkey already used" forever.
|
||||
//
|
||||
// Faithful to the artifacts: the client rotates its NodeKey on relogin, the
|
||||
// logout carries BOTH a past expiry AND an auth key, and a BRAND NEW key is
|
||||
// presented for the relogin (the trace shows tag:tag2 keys burned while the
|
||||
// node kept tag:tag1).
|
||||
func TestIssue3371_TaggedNodeLogoutReloginSingleUseKey(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("tag-logout-user")
|
||||
tags := []string{"tag:tag1"}
|
||||
|
||||
// `headscale preauthkeys create --tags tag:tag1` (single-use).
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, tags)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
// `tailscale up --auth-key $KEY1`: initial join.
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "headscale-debug"},
|
||||
}
|
||||
|
||||
resp, err := app.handleRegister(context.Background(), regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, resp.MachineAuthorized)
|
||||
require.False(t, resp.NodeKeyExpired)
|
||||
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, node.IsTagged(), "precondition: node is tagged")
|
||||
require.False(t, node.Expiry().Valid(), "precondition: tagged node has expiry disabled")
|
||||
|
||||
// `tailscale logout`: client sends a past-expiry register with the auth key
|
||||
// still attached (handleRegister must treat past expiry as logout regardless
|
||||
// of Auth). Reuse the same node key: logout does not rotate it.
|
||||
logoutReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Expiry: tsLogoutSentinelExpiry(),
|
||||
}
|
||||
|
||||
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
// A tagged node must NOT be expired by logout — tagged nodes never expire.
|
||||
// This is root cause (a); it fails before the fix.
|
||||
nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
assert.False(t, nodeAfterLogout.IsExpired(),
|
||||
"issue #3371 root cause (a): logout must not expire a tagged node")
|
||||
assert.False(t, nodeAfterLogout.Expiry().Valid(),
|
||||
"issue #3371 root cause (a): tagged node must keep key-expiry disabled after logout")
|
||||
|
||||
// `tailscale up --auth-key $KEY2`: a BRAND NEW single-use key, and the client
|
||||
// rotates its node key (as the real client does on relogin).
|
||||
pak2, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, tags)
|
||||
require.NoError(t, err)
|
||||
|
||||
nodeKey2 := key.NewNode()
|
||||
reloginReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak2.Key},
|
||||
NodeKey: nodeKey2.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "headscale-debug"},
|
||||
}
|
||||
|
||||
reloginResp, err := app.handleRegister(context.Background(), reloginReq, machineKey.Public())
|
||||
require.NoError(t, err,
|
||||
"issue #3371: a fresh valid key must re-authenticate the tagged node after logout")
|
||||
require.NotNil(t, reloginResp)
|
||||
|
||||
// The whole point: the node comes back online, not stuck expired.
|
||||
assert.False(t, reloginResp.NodeKeyExpired,
|
||||
"issue #3371: relogin response must not report the node key as expired")
|
||||
assert.True(t, reloginResp.MachineAuthorized)
|
||||
|
||||
relogged, found := app.state.GetNodeByNodeKey(nodeKey2.Public())
|
||||
require.True(t, found)
|
||||
assert.True(t, relogged.IsTagged(), "node stays tagged after relogin")
|
||||
assert.False(t, relogged.IsExpired(),
|
||||
"issue #3371: tagged node must be online (not expired) after relogin")
|
||||
assert.False(t, relogged.Expiry().Valid(),
|
||||
"issue #3371: tagged node must have key-expiry disabled after relogin")
|
||||
assert.Equal(t, node.ID(), relogged.ID(), "must re-use the same node, not duplicate")
|
||||
assert.Equal(t, 1, app.state.ListNodes().Len(), "machine maps to exactly one node")
|
||||
}
|
||||
|
||||
// TestIssue3371_TaggedNodeLogoutReloginReusableKey is the reusable-key variant
|
||||
// from the issue ("tailscale up then hangs indefinitely instead of erroring").
|
||||
// With a reusable key the relogin does not hit "authkey already used", but the
|
||||
// node still stays expired without the fix — so the client never observes a
|
||||
// non-expired node and hangs. The observable failure here is the persisted
|
||||
// expired state after relogin.
|
||||
func TestIssue3371_TaggedNodeLogoutReloginReusableKey(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("tag-logout-reusable")
|
||||
tags := []string{"tag:tag1"}
|
||||
|
||||
// `headscale preauthkeys create --reusable --tags tag:tag1`.
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-tagged"},
|
||||
}
|
||||
|
||||
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, node.IsTagged())
|
||||
require.False(t, node.Expiry().Valid())
|
||||
|
||||
// Logout.
|
||||
logoutReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Expiry: tsLogoutSentinelExpiry(),
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
// Relogin with the same reusable key, rotating the node key.
|
||||
nodeKey2 := key.NewNode()
|
||||
reloginReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey2.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-tagged"},
|
||||
}
|
||||
reloginResp, err := app.handleRegister(context.Background(), reloginReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, reloginResp)
|
||||
|
||||
assert.False(t, reloginResp.NodeKeyExpired,
|
||||
"issue #3371: reusable-key relogin must not report node key expired")
|
||||
|
||||
relogged, found := app.state.GetNodeByNodeKey(nodeKey2.Public())
|
||||
require.True(t, found)
|
||||
assert.True(t, relogged.IsTagged())
|
||||
assert.False(t, relogged.IsExpired(),
|
||||
"issue #3371: tagged node must be online (not expired) after reusable-key relogin")
|
||||
assert.False(t, relogged.Expiry().Valid(),
|
||||
"issue #3371: tagged node must have key-expiry disabled after reusable-key relogin")
|
||||
}
|
||||
|
||||
// TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry pins the deepest root cause
|
||||
// (a) in isolation: `tailscale logout` on a tagged node must not stamp an
|
||||
// expiry at all. This is the assertion PR #3372 does not make — it leaves
|
||||
// handleLogout expiring tagged nodes and only unwinds the damage on the next
|
||||
// registration. Keeping this separate from the relogin tests means a
|
||||
// regression that re-introduces logout-sets-expiry is caught even if the
|
||||
// re-registration cleanup masks it.
|
||||
func TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("tag-logout-noexpiry")
|
||||
tags := []string{"tag:tag1"}
|
||||
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "noexpiry-tagged"},
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
logoutReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Expiry: tsLogoutSentinelExpiry(),
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
assert.True(t, nodeAfterLogout.IsTagged(), "node stays tagged through logout")
|
||||
assert.False(t, nodeAfterLogout.Expiry().Valid(),
|
||||
"issue #3371 root cause (a): logout must not set an expiry on a tagged node")
|
||||
assert.False(t, nodeAfterLogout.IsExpired(),
|
||||
"issue #3371 root cause (a): tagged node must not be expired by logout")
|
||||
|
||||
// The database column must be NULL, not a clamped 'now' timestamp — a
|
||||
// persisted expiry survives restart and re-triggers the lockout.
|
||||
var dbNode types.Node
|
||||
require.NoError(t,
|
||||
app.state.DB().DB.First(&dbNode, nodeAfterLogout.ID().Uint64()).Error)
|
||||
assert.Nil(t, dbNode.Expiry,
|
||||
"issue #3371 root cause (a): tagged node's DB expiry must remain NULL after logout")
|
||||
}
|
||||
|
||||
// TestIssue3371_UserOwnedNodeLogoutStillExpires is the guard rail: the fix for
|
||||
// tagged nodes must not change logout for ordinary user-owned nodes. A
|
||||
// user-owned node that logs out MUST still be expired (that is what logout
|
||||
// means for it).
|
||||
func TestIssue3371_UserOwnedNodeLogoutStillExpires(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("user-logout")
|
||||
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "user-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.False(t, node.IsTagged(), "precondition: user-owned node")
|
||||
|
||||
logoutReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Expiry: tsLogoutSentinelExpiry(),
|
||||
}
|
||||
logoutResp, err := app.handleRegister(context.Background(), logoutReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, logoutResp)
|
||||
|
||||
nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
assert.True(t, nodeAfterLogout.IsExpired(),
|
||||
"user-owned node must still be expired by logout (fix must not regress this)")
|
||||
assert.True(t, logoutResp.NodeKeyExpired,
|
||||
"logout response for a user-owned node must report the key expired")
|
||||
}
|
||||
|
||||
// TestIssue3371_TaggedNodeFutureExpirySurvivesRelogin is the discriminator
|
||||
// guard rail for the fix. A tagged node may carry a DELIBERATE future expiry
|
||||
// set by an admin (`headscale nodes expire`); TestTaggedNodeCanHaveKeyExpiry
|
||||
// establishes that is legal. The #3371 fix clears only a STALE PAST expiry (the
|
||||
// logout stamp) on re-registration — it must NOT wipe a future expiry. This
|
||||
// test locks that boundary: without care, a "tagged => clear expiry" fix would
|
||||
// silently destroy the admin's setting.
|
||||
//
|
||||
// Passes before the fix (re-registration currently never touches a tagged
|
||||
// node's expiry) and must keep passing after.
|
||||
func TestIssue3371_TaggedNodeFutureExpirySurvivesRelogin(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("tag-future-expiry")
|
||||
tags := []string{"tag:tag1"}
|
||||
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "future-expiry-tagged"},
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, node.IsTagged())
|
||||
|
||||
// Admin sets a deliberate future expiry (`headscale nodes expire`).
|
||||
future := time.Now().Add(24 * time.Hour)
|
||||
_, _, err = app.state.SetNodeExpiry(node.ID(), &future)
|
||||
require.NoError(t, err)
|
||||
|
||||
withFuture, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, withFuture.Expiry().Valid(), "precondition: future expiry set")
|
||||
require.False(t, withFuture.IsExpired(), "precondition: future expiry is not expired")
|
||||
|
||||
// Node re-registers (rotating its node key). The future expiry must survive.
|
||||
nodeKey2 := key.NewNode()
|
||||
reregReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey2.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "future-expiry-tagged"},
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), reregReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
after, found := app.state.GetNodeByNodeKey(nodeKey2.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, after.IsTagged(), "node stays tagged")
|
||||
assert.True(t, after.Expiry().Valid(),
|
||||
"deliberate future expiry must survive re-registration (not cleared by #3371 fix)")
|
||||
assert.WithinDuration(t, future, after.Expiry().Get(), 5*time.Second,
|
||||
"the surviving expiry must be the admin-set future value, unchanged")
|
||||
}
|
||||
|
||||
// TestIssue3371_EphemeralTaggedNodeLogoutDeletes is a regression guard for the
|
||||
// ephemeral+tagged combination. A tagged pre-auth key can also be ephemeral.
|
||||
// handleLogout deletes ephemeral nodes (before any expiry stamp), so the #3371
|
||||
// fix (which suppresses the expiry stamp for tagged nodes) must not divert an
|
||||
// ephemeral tagged node away from deletion.
|
||||
//
|
||||
// Passes before the fix and must keep passing after.
|
||||
func TestIssue3371_EphemeralTaggedNodeLogoutDeletes(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("tag-ephemeral")
|
||||
tags := []string{"tag:tag1"}
|
||||
|
||||
// Ephemeral + tagged key.
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, true, nil, tags)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "ephemeral-tagged"},
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, node.IsTagged(), "precondition: node is tagged")
|
||||
require.True(t, node.IsEphemeral(), "precondition: node is ephemeral")
|
||||
|
||||
// Logout: an ephemeral node is deleted, not expired.
|
||||
logoutReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Expiry: tsLogoutSentinelExpiry(),
|
||||
}
|
||||
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
require.EventuallyWithT(t, func(c *assert.CollectT) {
|
||||
_, stillThere := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
assert.False(c, stillThere,
|
||||
"ephemeral tagged node must be deleted on logout, not expired")
|
||||
}, 2*time.Second, 50*time.Millisecond, "waiting for ephemeral node deletion")
|
||||
}
|
||||
|
||||
@@ -900,6 +900,33 @@ WHERE user_id IS NULL
|
||||
},
|
||||
Rollback: func(db *gorm.DB) error { return nil },
|
||||
},
|
||||
{
|
||||
// Clear stale key expiry on tagged nodes. A tagged node is
|
||||
// owned by its tags and never expires (KB 1068), but a buggy
|
||||
// handleLogout stamped a past expiry on it, leaving it
|
||||
// permanently Expired and unable to re-authenticate. The
|
||||
// buggy writer is fixed, so this only repairs rows written
|
||||
// before the upgrade; a fixed server cannot recreate them.
|
||||
// Match the tagged-node predicate the earlier
|
||||
// clear-tagged-node-user-id migration uses (a nil tags slice
|
||||
// marshals to 'null', so exclude it).
|
||||
// Fixes: https://github.com/juanfont/headscale/issues/3371
|
||||
ID: "202607241200-clear-tagged-node-expiry",
|
||||
Migrate: func(tx *gorm.DB) error {
|
||||
err := tx.Exec(`
|
||||
UPDATE nodes
|
||||
SET expiry = NULL
|
||||
WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null'
|
||||
AND expiry IS NOT NULL;
|
||||
`).Error
|
||||
if err != nil {
|
||||
return fmt.Errorf("clearing expiry on tagged nodes: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
},
|
||||
Rollback: func(db *gorm.DB) error { return nil },
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@@ -295,6 +295,64 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) {
|
||||
assert.Equal(t, uint(1), *node4.UserID, "node4 should still belong to user1")
|
||||
},
|
||||
},
|
||||
// Test for the clear-tagged-node-expiry migration
|
||||
// (202607241200-clear-tagged-node-expiry). A buggy handleLogout stamped
|
||||
// a key expiry on tagged nodes, which never expire (KB 1068), leaving
|
||||
// them permanently Expired. The migration clears expiry on tagged rows
|
||||
// only, preserving user-owned nodes' expiry.
|
||||
// Fixes: https://github.com/juanfont/headscale/issues/3371
|
||||
{
|
||||
dbPath: "testdata/sqlite/clear_tagged_node_expiry_migration_test.sql",
|
||||
wantFunc: func(t *testing.T, hsdb *HSDatabase) {
|
||||
t.Helper()
|
||||
|
||||
nodes, err := Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) {
|
||||
return ListNodes(rx)
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, nodes, 5, "should have all 5 nodes")
|
||||
|
||||
byHostname := make(map[string]*types.Node, len(nodes))
|
||||
for _, n := range nodes {
|
||||
byHostname[n.Hostname] = n
|
||||
}
|
||||
|
||||
// Node 1: tagged with a stale PAST expiry (the bug). Cleared.
|
||||
node1 := byHostname["node1"]
|
||||
require.NotNil(t, node1, "node1 should exist")
|
||||
assert.True(t, node1.IsTagged(), "node1 should be tagged")
|
||||
assert.Nil(t, node1.Expiry, "node1 (tagged) stale expiry should be cleared")
|
||||
assert.False(t, node1.IsExpired(), "node1 must not be reported expired")
|
||||
|
||||
// Node 2: tagged with a FUTURE expiry. Tagged nodes never expire,
|
||||
// so this is cleared too.
|
||||
node2 := byHostname["node2"]
|
||||
require.NotNil(t, node2, "node2 should exist")
|
||||
assert.True(t, node2.IsTagged(), "node2 should be tagged")
|
||||
assert.Nil(t, node2.Expiry, "node2 (tagged) expiry should be cleared")
|
||||
|
||||
// Node 3: tagged, expiry already NULL. Stays NULL.
|
||||
node3 := byHostname["node3"]
|
||||
require.NotNil(t, node3, "node3 should exist")
|
||||
assert.True(t, node3.IsTagged(), "node3 should be tagged")
|
||||
assert.Nil(t, node3.Expiry, "node3 (tagged) NULL expiry should be preserved")
|
||||
|
||||
// Node 4: untagged (tags='null') with a PAST expiry. PRESERVED —
|
||||
// the migration must not touch user-owned nodes.
|
||||
node4 := byHostname["node4"]
|
||||
require.NotNil(t, node4, "node4 should exist")
|
||||
assert.False(t, node4.IsTagged(), "node4 (tags='null') should be untagged")
|
||||
require.NotNil(t, node4.Expiry, "node4 (user-owned) expiry must be preserved")
|
||||
assert.Equal(t, 2020, node4.Expiry.UTC().Year(), "node4 past expiry preserved")
|
||||
|
||||
// Node 5: untagged (tags='[]') with a FUTURE expiry. PRESERVED.
|
||||
node5 := byHostname["node5"]
|
||||
require.NotNil(t, node5, "node5 should exist")
|
||||
assert.False(t, node5.IsTagged(), "node5 (tags='[]') should be untagged")
|
||||
require.NotNil(t, node5.Expiry, "node5 (user-owned) expiry must be preserved")
|
||||
assert.Equal(t, 2099, node5.Expiry.UTC().Year(), "node5 future expiry preserved")
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
85
hscontrol/db/testdata/sqlite/clear_tagged_node_expiry_migration_test.sql
vendored
Normal file
85
hscontrol/db/testdata/sqlite/clear_tagged_node_expiry_migration_test.sql
vendored
Normal file
@@ -0,0 +1,85 @@
|
||||
-- Test SQL dump for the clear-tagged-node-expiry migration
|
||||
-- (202607241200-clear-tagged-node-expiry)
|
||||
--
|
||||
-- A buggy handleLogout stamped a past key expiry on tagged nodes. Tagged
|
||||
-- nodes are owned by their tags and never expire (KB 1068), so such a row is
|
||||
-- reported as permanently Expired and can never re-authenticate. The migration
|
||||
-- clears expiry on tagged rows while leaving user-owned nodes untouched.
|
||||
-- Fixes: https://github.com/juanfont/headscale/issues/3371
|
||||
|
||||
PRAGMA foreign_keys=OFF;
|
||||
BEGIN TRANSACTION;
|
||||
|
||||
-- Migrations table: entries applied up to (but not including) the fix. The
|
||||
-- intervening expiry migrations (clear-zero-time) also run against this dump;
|
||||
-- their predicates (expiry < 1900) do not match the post-2000 dates below, so
|
||||
-- they leave these rows for the new migration to handle.
|
||||
CREATE TABLE `migrations` (`id` text,PRIMARY KEY (`id`));
|
||||
INSERT INTO migrations VALUES('202312101416');
|
||||
INSERT INTO migrations VALUES('202312101430');
|
||||
INSERT INTO migrations VALUES('202402151347');
|
||||
INSERT INTO migrations VALUES('2024041121742');
|
||||
INSERT INTO migrations VALUES('202406021630');
|
||||
INSERT INTO migrations VALUES('202409271400');
|
||||
INSERT INTO migrations VALUES('202407191627');
|
||||
INSERT INTO migrations VALUES('202408181235');
|
||||
INSERT INTO migrations VALUES('202501221827');
|
||||
INSERT INTO migrations VALUES('202501311657');
|
||||
INSERT INTO migrations VALUES('202502070949');
|
||||
INSERT INTO migrations VALUES('202502131714');
|
||||
INSERT INTO migrations VALUES('202502171819');
|
||||
INSERT INTO migrations VALUES('202505091439');
|
||||
INSERT INTO migrations VALUES('202505141324');
|
||||
INSERT INTO migrations VALUES('202507021200');
|
||||
INSERT INTO migrations VALUES('202510311551');
|
||||
INSERT INTO migrations VALUES('202511101554-drop-old-idx');
|
||||
INSERT INTO migrations VALUES('202511011637-preauthkey-bcrypt');
|
||||
INSERT INTO migrations VALUES('202511122344-remove-newline-index');
|
||||
INSERT INTO migrations VALUES('202511131445-node-forced-tags-to-tags');
|
||||
INSERT INTO migrations VALUES('202601121700-migrate-hostinfo-request-tags');
|
||||
INSERT INTO migrations VALUES('202602201200-clear-tagged-node-user-id');
|
||||
|
||||
-- Users table
|
||||
CREATE TABLE `users` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`name` text,`display_name` text,`email` text,`provider_identifier` text,`provider` text,`profile_pic_url` text);
|
||||
INSERT INTO users VALUES(1,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user1','User One','user1@example.com',NULL,NULL,NULL);
|
||||
|
||||
-- Pre-auth keys table
|
||||
CREATE TABLE `pre_auth_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`user_id` integer,`reusable` numeric,`ephemeral` numeric DEFAULT false,`used` numeric DEFAULT false,`tags` text,`created_at` datetime,`expiration` datetime,`prefix` text,`hash` blob,CONSTRAINT `fk_pre_auth_keys_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE SET NULL);
|
||||
|
||||
-- API keys table
|
||||
CREATE TABLE `api_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`prefix` text,`hash` blob,`created_at` datetime,`expiration` datetime,`last_seen` datetime);
|
||||
|
||||
-- Nodes table - current schema (after the tags rename + last_seen/expiry reordering)
|
||||
CREATE TABLE IF NOT EXISTS "nodes" (`id` integer PRIMARY KEY AUTOINCREMENT,`machine_key` text,`node_key` text,`disco_key` text,`endpoints` text,`host_info` text,`ipv4` text,`ipv6` text,`hostname` text,`given_name` varchar(63),`user_id` integer,`register_method` text,`tags` text,`auth_key_id` integer,`last_seen` datetime,`expiry` datetime,`approved_routes` text,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,CONSTRAINT `fk_nodes_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_nodes_auth_key` FOREIGN KEY (`auth_key_id`) REFERENCES `pre_auth_keys`(`id`));
|
||||
|
||||
-- Node 1: TAGGED, user_id NULL, stale PAST expiry (the #3371 bug). After migration: expiry NULL.
|
||||
INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00','2020-06-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
|
||||
|
||||
-- Node 2: TAGGED, user_id NULL, FUTURE expiry. Tagged nodes never expire, so cleared to NULL too.
|
||||
INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00','2099-01-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
|
||||
|
||||
-- Node 3: TAGGED, user_id NULL, expiry already NULL. After migration: still NULL.
|
||||
INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
|
||||
|
||||
-- Node 4: UNTAGGED user-owned (tags='null'), PAST expiry. Must be PRESERVED (guard against
|
||||
-- the #3323-class over-match: a nil tags slice marshals to the literal 'null').
|
||||
INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'cli','null',NULL,'2024-01-01 00:00:00+00:00','2020-06-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
|
||||
|
||||
-- Node 5: UNTAGGED user-owned (tags='[]'), FUTURE expiry. Must be PRESERVED.
|
||||
INSERT INTO nodes VALUES(5,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e05','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605505','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57705','[]','{}','100.64.0.5','fd7a:115c:a1e0::5','node5','node5',1,'cli','[]',NULL,'2024-01-01 00:00:00+00:00','2099-01-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
|
||||
|
||||
-- Policies table (empty)
|
||||
CREATE TABLE `policies` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`data` text);
|
||||
|
||||
DELETE FROM sqlite_sequence;
|
||||
INSERT INTO sqlite_sequence VALUES('users',1);
|
||||
INSERT INTO sqlite_sequence VALUES('nodes',5);
|
||||
CREATE INDEX idx_users_deleted_at ON users(deleted_at);
|
||||
CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix);
|
||||
CREATE INDEX idx_policies_deleted_at ON policies(deleted_at);
|
||||
CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE provider_identifier IS NOT NULL;
|
||||
CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier);
|
||||
CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL;
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != '';
|
||||
|
||||
COMMIT;
|
||||
@@ -408,6 +408,151 @@ func TestAuthPathRejectsTaggedAndUserCoexistence(t *testing.T) {
|
||||
require.ErrorIs(t, err, ErrAmbiguousNodeOwnership)
|
||||
}
|
||||
|
||||
// TestIssue3371_TaggedNodeInteractiveReloginAfterLogout reproduces the
|
||||
// interactive/OIDC arm of https://github.com/juanfont/headscale/issues/3371
|
||||
// ("With no key (interactive): the register URL is printed and the login never
|
||||
// completes").
|
||||
//
|
||||
// A logout stamps a stale PAST expiry on a tagged node. When the node
|
||||
// re-authenticates through the auth path (HandleNodeFromAuthPath ->
|
||||
// applyAuthNodeUpdate), the tagged->tagged branch keeps the existing expiry
|
||||
// ("Tagged → Tagged: keep existing expiry (nil) - no action needed",
|
||||
// state.go). That comment assumes the existing expiry is nil; after a logout it
|
||||
// is a past timestamp, so the node stays expired. The fix must clear a stale
|
||||
// past expiry on a node that remains tagged (scoped to IsExpired(), so a
|
||||
// deliberate future expiry is preserved).
|
||||
func TestIssue3371_TaggedNodeInteractiveReloginAfterLogout(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
database, err := db.NewHeadscaleDatabase(cfg)
|
||||
require.NoError(t, err)
|
||||
|
||||
user := database.CreateUserForTest("interactive-user")
|
||||
node := database.CreateRegisteredNodeForTest(user, "interactive-tagged")
|
||||
machineKey := node.MachineKey
|
||||
nodeID := node.ID
|
||||
discoKey := node.DiscoKey
|
||||
|
||||
require.NoError(t, database.Close())
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
// Make the node tagged with a stale PAST expiry (the state a `tailscale
|
||||
// logout` leaves behind). Leaving UserID nil but retaining User routes the
|
||||
// re-auth through the convert-from-tag branch and lets the tag
|
||||
// re-advertisement be permitted (mirrors TestTaggedReauthKeepsNilExpiry,
|
||||
// which seeds Expiry=nil; here the only change is a past expiry).
|
||||
past := time.Now().Add(-1 * time.Hour)
|
||||
seeded, ok := s.nodeStore.UpdateNode(nodeID, func(n *types.Node) {
|
||||
n.Tags = []string{"tag:foo"}
|
||||
n.UserID = nil
|
||||
n.User = user
|
||||
n.Expiry = &past
|
||||
})
|
||||
require.True(t, ok)
|
||||
require.True(t, seeded.IsTagged(), "precondition: node is tagged")
|
||||
require.True(t, seeded.IsExpired(), "precondition: logout left the tagged node expired")
|
||||
|
||||
policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name)
|
||||
_, err = s.SetPolicy([]byte(policy))
|
||||
require.NoError(t, err)
|
||||
require.True(t, s.NodeCanHaveTag(seeded, "tag:foo"),
|
||||
"precondition: tagged node is permitted to re-advertise tag:foo")
|
||||
|
||||
// Interactive/OIDC relogin: the client re-advertises the same tag (rotating
|
||||
// its node key, as a real client does). The node must come back not-expired.
|
||||
regData := &types.RegistrationData{
|
||||
MachineKey: machineKey,
|
||||
NodeKey: key.NewNode().Public(),
|
||||
DiscoKey: discoKey,
|
||||
Hostname: "interactive-tagged",
|
||||
Hostinfo: &tailcfg.Hostinfo{
|
||||
Hostname: "interactive-tagged",
|
||||
RequestTags: []string{"tag:foo"},
|
||||
},
|
||||
}
|
||||
authID := types.MustAuthID()
|
||||
s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(regData))
|
||||
|
||||
relogged, _, err := s.HandleNodeFromAuthPath(
|
||||
authID, types.UserID(user.ID), nil, util.RegisterMethodOIDC,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.True(t, relogged.Valid())
|
||||
|
||||
require.True(t, relogged.IsTagged(), "node stays tagged after interactive relogin")
|
||||
require.False(t, relogged.IsExpired(),
|
||||
"issue #3371: interactive relogin must clear the stale logout expiry")
|
||||
require.Nil(t, relogged.AsStruct().Expiry,
|
||||
"issue #3371: tagged node must have key-expiry disabled after interactive relogin")
|
||||
}
|
||||
|
||||
// TestIssue3371_TaggedNodePastExpirySelfHealsOnReregister covers the 0.29.x
|
||||
// upgrade path: a tagged node broken by an OLDER headscale carries a past
|
||||
// expiry persisted in its DB row. After a restart (State reloads the row) it
|
||||
// comes back expired, and its next auth-key re-registration must self-heal it
|
||||
// by clearing the stale past expiry. This is the "part b" defensive clear.
|
||||
func TestIssue3371_TaggedNodePastExpirySelfHealsOnReregister(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:foo":["tagger@"]}}`))
|
||||
require.NoError(t, err)
|
||||
|
||||
pak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "broken-tagged"},
|
||||
}
|
||||
node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
nodeID := node.ID()
|
||||
|
||||
// A prior (buggy) version persisted a past expiry on this tagged node.
|
||||
past := time.Now().Add(-1 * time.Hour)
|
||||
err = s.DB().NodeSetExpiry(nodeID, &past)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Restart: reload State from the same database file.
|
||||
require.NoError(t, s.Close())
|
||||
|
||||
s2, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s2.Close() })
|
||||
|
||||
reloaded, ok := s2.GetNodeByID(nodeID)
|
||||
require.True(t, ok)
|
||||
require.True(t, reloaded.IsExpired(),
|
||||
"precondition: a persisted past expiry survives restart and re-triggers the lockout")
|
||||
|
||||
// Re-register (rotating the node key). The stale past expiry must be cleared.
|
||||
reregReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "broken-tagged"},
|
||||
}
|
||||
healed, _, err := s2.HandleNodeFromPreAuthKey(reregReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, healed.IsTagged(), "node stays tagged")
|
||||
require.False(t, healed.IsExpired(),
|
||||
"issue #3371: re-registration must self-heal a tagged node broken by an older version")
|
||||
require.Nil(t, healed.AsStruct().Expiry,
|
||||
"issue #3371: self-healed tagged node must have key-expiry disabled (DB NULL)")
|
||||
require.Equal(t, nodeID, healed.ID(), "must be the same node")
|
||||
}
|
||||
|
||||
// TestTaggedNodeCanHaveKeyExpiry matches Tailscale: a tagged node has key
|
||||
// expiry disabled by default, but it can still be set explicitly (e.g. via
|
||||
// `headscale nodes expire`).
|
||||
|
||||
@@ -1801,8 +1801,14 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
|
||||
} else {
|
||||
node.Expiry = regData.Expiry
|
||||
}
|
||||
case isTagged && node.IsExpired():
|
||||
// Tagged → Tagged, but carrying a stale PAST expiry from an older
|
||||
// headscale's logout stamp (#3371). Tagged nodes never expire, so
|
||||
// clear it; a deliberate future expiry has IsExpired() == false and
|
||||
// falls through to the no-op below.
|
||||
node.Expiry = nil
|
||||
}
|
||||
// Tagged → Tagged: keep existing expiry (nil) - no action needed
|
||||
// Tagged → Tagged with no stale expiry: keep existing expiry - no action.
|
||||
|
||||
// Apply default node expiry for non-tagged nodes when the
|
||||
// resolved expiry is still nil or zero (e.g., CLI registration
|
||||
@@ -2421,7 +2427,14 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
// must present a valid key. Without this a node that re-uses its NodeKey
|
||||
// after expiry would skip validation and be re-authorised with a spent or
|
||||
// expired key; the boundary must not depend on the client rotating its key.
|
||||
//
|
||||
// Tagged nodes are excluded: they never expire (KB 1068), so an
|
||||
// IsExpired() tagged node only reflects a stale logout stamp left by an
|
||||
// older headscale (#3371). Forcing it down the re-validation path burns its
|
||||
// fresh key and blocks re-auth forever; treat it as a plain re-registration
|
||||
// and clear the stale expiry in the update below.
|
||||
isExpired := existsSameUser && existingNodeSameUser.Valid() &&
|
||||
!existingNodeSameUser.IsTagged() &&
|
||||
existingNodeSameUser.IsExpired()
|
||||
|
||||
// A tagged key presented for a currently user-owned node converts that node
|
||||
@@ -2557,6 +2570,13 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
} else {
|
||||
node.Expiry = nil
|
||||
}
|
||||
} else if node.IsExpired() {
|
||||
// #3371: a tagged node must never carry key expiry. Clear a
|
||||
// stale PAST expiry left by a logout (older headscale) so
|
||||
// re-auth is not permanently blocked. A deliberate future
|
||||
// expiry (headscale nodes expire) has IsExpired() == false and
|
||||
// is left untouched.
|
||||
node.Expiry = nil
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -3203,3 +3203,202 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
|
||||
}
|
||||
}, integrationutil.HAConvergeTimeout, 1*time.Second, "node should be user-owned after conversion via CLI register")
|
||||
}
|
||||
|
||||
// TestTaggedNodeLogoutReloginSingleUseKeyOnline reproduces issue #3371
|
||||
// end-to-end with a real tailscale client: a tagged node registered with a
|
||||
// single-use key logs out (`tailscale logout`) and re-authenticates with a
|
||||
// FRESH single-use tagged key. Tagged nodes never expire (KB 1068), so logout
|
||||
// must not stamp an expiry; before the fix the node was left permanently
|
||||
// expired and the fresh key was consumed on a re-registration that still
|
||||
// reported NodeKeyExpired, locking the node out forever.
|
||||
//
|
||||
// The observable proof at the integration level is that after relogin the node
|
||||
// is back online with a NULL expiry and the same node ID — not stuck expired.
|
||||
//
|
||||
// https://github.com/juanfont/headscale/issues/3371
|
||||
func TestTaggedNodeLogoutReloginSingleUseKeyOnline(t *testing.T) {
|
||||
IntegrationSkip(t)
|
||||
|
||||
spec := ScenarioSpec{
|
||||
NodesPerUser: 0,
|
||||
Users: []string{tagTestUser},
|
||||
}
|
||||
|
||||
scenario, err := NewScenario(spec)
|
||||
|
||||
require.NoError(t, err)
|
||||
defer scenario.ShutdownAssertNoPanics(t)
|
||||
|
||||
err = scenario.CreateHeadscaleEnv(
|
||||
[]tsic.Option{},
|
||||
hsic.WithACLPolicy(tagsTestPolicy()),
|
||||
hsic.WithTestName("tags-logout-single"),
|
||||
)
|
||||
requireNoErrHeadscaleEnv(t, err)
|
||||
|
||||
headscale, err := scenario.Headscale()
|
||||
requireNoErrGetHeadscale(t, err)
|
||||
|
||||
userMap, err := headscale.MapUsers()
|
||||
require.NoError(t, err)
|
||||
|
||||
userID := mustParseID(userMap[tagTestUser].Id)
|
||||
|
||||
// KEY1: single-use tag:valid-owned. Initial join.
|
||||
key1, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"})
|
||||
require.NoError(t, err)
|
||||
|
||||
client, err := scenario.CreateTailscaleNode(
|
||||
"head",
|
||||
tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.Login(headscale.GetEndpoint(), key1.Key)
|
||||
require.NoError(t, err)
|
||||
|
||||
var initialNodeID uint64
|
||||
|
||||
assert.EventuallyWithT(t, func(c *assert.CollectT) {
|
||||
nodes, err := headscale.ListNodes()
|
||||
assert.NoError(c, err)
|
||||
assert.Len(c, nodes, 1)
|
||||
|
||||
if len(nodes) == 1 {
|
||||
initialNodeID = mustParseID(nodes[0].Id)
|
||||
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
|
||||
assert.Nil(c, nodes[0].Expiry, "tagged node must have no expiry")
|
||||
}
|
||||
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
|
||||
|
||||
// `tailscale logout`. A tagged node must not be expired by this.
|
||||
err = client.Logout()
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.WaitForNeedsLogin(integrationutil.ScaledTimeout(60 * time.Second))
|
||||
require.NoError(t, err)
|
||||
|
||||
// The node must remain in the DB, tagged, and crucially NOT carry a
|
||||
// stale expiry. This is the #3371 root cause (a) surface.
|
||||
assert.EventuallyWithT(t, func(c *assert.CollectT) {
|
||||
nodes, err := headscale.ListNodes()
|
||||
assert.NoError(c, err)
|
||||
assert.Len(c, nodes, 1, "node must persist through logout")
|
||||
|
||||
if len(nodes) == 1 {
|
||||
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
|
||||
assert.Nil(c, nodes[0].Expiry, "#3371: logout must not stamp expiry on a tagged node")
|
||||
}
|
||||
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "tagged node must survive logout without expiry")
|
||||
|
||||
// KEY2: a FRESH single-use tagged key. Relogin.
|
||||
key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"})
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.Login(headscale.GetEndpoint(), key2.Key)
|
||||
require.NoError(t, err,
|
||||
"#3371: a fresh key must re-authenticate the tagged node after logout")
|
||||
|
||||
// Back online, same node, still tagged, still no expiry.
|
||||
assert.EventuallyWithT(t, func(c *assert.CollectT) {
|
||||
nodes, err := headscale.ListNodes()
|
||||
assert.NoError(c, err)
|
||||
assert.Len(c, nodes, 1, "must not duplicate the node")
|
||||
|
||||
if len(nodes) == 1 {
|
||||
assert.Equal(c, initialNodeID, mustParseID(nodes[0].Id), "node ID must be unchanged")
|
||||
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
|
||||
assert.Nil(c, nodes[0].Expiry, "#3371: tagged node must have no expiry after relogin")
|
||||
assert.True(c, nodes[0].Online, "#3371: tagged node must be online after relogin, not stuck expired")
|
||||
}
|
||||
}, integrationutil.ScaledTimeout(60*time.Second), integrationutil.SlowPoll, "tagged node must come back online after relogin")
|
||||
|
||||
t.Logf("Test #3371 PASS: tagged node logged out and re-authenticated online with a fresh single-use key")
|
||||
}
|
||||
|
||||
// TestTaggedNodeLogoutReloginReusableKeyOnline is the reusable-key variant of
|
||||
// issue #3371 (the "tailscale up hangs indefinitely" report). With a reusable
|
||||
// key the relogin does not hit "authkey already used", but before the fix the
|
||||
// node still stayed expired, so the client never observed a non-expired node.
|
||||
// The observable proof is the same: online with NULL expiry after relogin.
|
||||
//
|
||||
// https://github.com/juanfont/headscale/issues/3371
|
||||
func TestTaggedNodeLogoutReloginReusableKeyOnline(t *testing.T) {
|
||||
IntegrationSkip(t)
|
||||
|
||||
spec := ScenarioSpec{
|
||||
NodesPerUser: 0,
|
||||
Users: []string{tagTestUser},
|
||||
}
|
||||
|
||||
scenario, err := NewScenario(spec)
|
||||
|
||||
require.NoError(t, err)
|
||||
defer scenario.ShutdownAssertNoPanics(t)
|
||||
|
||||
err = scenario.CreateHeadscaleEnv(
|
||||
[]tsic.Option{},
|
||||
hsic.WithACLPolicy(tagsTestPolicy()),
|
||||
hsic.WithTestName("tags-logout-reuse"),
|
||||
)
|
||||
requireNoErrHeadscaleEnv(t, err)
|
||||
|
||||
headscale, err := scenario.Headscale()
|
||||
requireNoErrGetHeadscale(t, err)
|
||||
|
||||
userMap, err := headscale.MapUsers()
|
||||
require.NoError(t, err)
|
||||
|
||||
userID := mustParseID(userMap[tagTestUser].Id)
|
||||
|
||||
// A single REUSABLE tag:valid-owned key used for both login and relogin.
|
||||
key, err := scenario.CreatePreAuthKeyWithTags(userID, true, false, []string{"tag:valid-owned"})
|
||||
require.NoError(t, err)
|
||||
|
||||
client, err := scenario.CreateTailscaleNode(
|
||||
"head",
|
||||
tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.Login(headscale.GetEndpoint(), key.Key)
|
||||
require.NoError(t, err)
|
||||
|
||||
var initialNodeID uint64
|
||||
|
||||
assert.EventuallyWithT(t, func(c *assert.CollectT) {
|
||||
nodes, err := headscale.ListNodes()
|
||||
assert.NoError(c, err)
|
||||
assert.Len(c, nodes, 1)
|
||||
|
||||
if len(nodes) == 1 {
|
||||
initialNodeID = mustParseID(nodes[0].Id)
|
||||
assert.Nil(c, nodes[0].Expiry, "tagged node must have no expiry")
|
||||
}
|
||||
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
|
||||
|
||||
err = client.Logout()
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.WaitForNeedsLogin(integrationutil.ScaledTimeout(60 * time.Second))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Relogin with the SAME reusable key.
|
||||
err = client.Login(headscale.GetEndpoint(), key.Key)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.EventuallyWithT(t, func(c *assert.CollectT) {
|
||||
nodes, err := headscale.ListNodes()
|
||||
assert.NoError(c, err)
|
||||
assert.Len(c, nodes, 1, "must not duplicate the node")
|
||||
|
||||
if len(nodes) == 1 {
|
||||
assert.Equal(c, initialNodeID, mustParseID(nodes[0].Id), "node ID must be unchanged")
|
||||
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
|
||||
assert.Nil(c, nodes[0].Expiry, "#3371: tagged node must have no expiry after reusable-key relogin")
|
||||
assert.True(c, nodes[0].Online, "#3371: tagged node must be online after reusable-key relogin")
|
||||
}
|
||||
}, integrationutil.ScaledTimeout(60*time.Second), integrationutil.SlowPoll, "tagged node must come back online after reusable-key relogin")
|
||||
|
||||
t.Logf("Test #3371 PASS: tagged node logged out and re-authenticated online with a reusable key")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user