Compare commits

..

1 Commits

Author SHA1 Message Date
github-actions[bot] 74a4dd5bb1 flake.lock: Update
Flake lock file updates:

• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/9ee75f1' (2026-06-11)
  → 'github:NixOS/nixpkgs/ef27201' (2026-06-14)
2026-06-14 01:00:22 +00:00
55 changed files with 295 additions and 5899 deletions
-8
View File
@@ -340,8 +340,6 @@ jobs:
- TestTagsAuthKeyWithTagCannotAddViaCLI
- TestTagsAuthKeyWithTagCannotChangeViaCLI
- TestTagsAuthKeyWithTagAdminOverrideReauthPreserves
- TestTagsReauthDifferentKeyRetagsNode
- TestTagsReauthDifferentKeyRemovesTag
- TestTagsAuthKeyWithTagCLICannotModifyAdminTags
- TestTagsAuthKeyWithoutTagCannotRequestTags
- TestTagsAuthKeyWithoutTagRegisterNoTags
@@ -369,12 +367,6 @@ jobs:
- TestTagsAuthKeyWithoutUserInheritsTags
- TestTagsAuthKeyWithoutUserRejectsAdvertisedTags
- TestTagsAuthKeyConvertToUserViaCLIRegister
- TestTaggedNodeLogoutReloginSingleUseKeyOnline
- TestTaggedNodeLogoutReloginReusableKeyOnline
- TestTagsOIDCReauthAddOwnedTag
- TestTagsReauthEmptyTagsReturnsToUserSurvives
- TestTS2021WebSocketGET
- TestTS2021WASMClientUnderNode
- TestTailscaleRustAxum
uses: ./.github/workflows/integration-test-template.yml
secrets: inherit
+1 -33
View File
@@ -1,38 +1,6 @@
# CHANGELOG
## 0.29.3 (2026-07-29)
**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)
- Re-registering a tagged node with a different pre-auth key now applies the new key's tags instead of silently keeping the old ones [#3394](https://github.com/juanfont/headscale/pull/3394)
- Fix re-authenticating an already-tagged node with `--advertise-tags` being rejected when the authenticating user owns the tags [#3394](https://github.com/juanfont/headscale/pull/3394)
- Fix ephemeral nodes lingering as disconnected after reconnect churn [#3383](https://github.com/juanfont/headscale/pull/3383)
- Fix node registration falsely returning `401 registration timed out` when auth completes as the request context expires [#3392](https://github.com/juanfont/headscale/pull/3392)
- Check the machine key on the followup registration poll so a leaked auth ID cannot return the registering user's identity [#3393](https://github.com/juanfont/headscale/pull/3393)
- Reject `/key` requests below the supported capability version floor, matching `/ts2021` [#3391](https://github.com/juanfont/headscale/pull/3391)
## 0.29.2 (2026-07-01)
**Minimum supported Tailscale client version: v1.80.0**
### Changes
- Fix map generation serializing on the policy lock, so a mass reconnect on `autogroup:self`, via or relay policies no longer stalls clients into `unexpected EOF` retry loops [#3358](https://github.com/juanfont/headscale/pull/3358)
- Fix `/ts2021` rejecting the WebSocket `GET` upgrade with 405, which prevented Tailscale JS/WASM control clients from connecting [#3359](https://github.com/juanfont/headscale/pull/3359)
- Gracefully handle nodes with an invalid FQDN (empty or too long) instead of failing map delivery; offending names are logged at startup with the fix command [#3349](https://github.com/juanfont/headscale/pull/3349)
## 0.29.1 (2026-06-18)
**Minimum supported Tailscale client version: v1.80.0**
### Changes
- Fix nodes with `tags='null'` losing their assigned user on upgrade [#3325](https://github.com/juanfont/headscale/pull/3325)
## 0.29.0 (2026-06-17)
## 0.29.0 (202x-xx-xx)
**Minimum supported Tailscale client version: v1.80.0**
+1 -1
View File
@@ -1,6 +1,6 @@
# For testing purposes only
FROM golang:1.26.5-alpine AS build-env
FROM golang:1.26.4-alpine AS build-env
WORKDIR /go/src
+1 -1
View File
@@ -2,7 +2,7 @@
# and are in no way endorsed by Headscale's maintainers as an
# official nor supported release or distribution.
FROM docker.io/golang:1.26.5-trixie AS builder
FROM docker.io/golang:1.26.4-trixie AS builder
ARG VERSION=dev
ENV GOPATH /go
WORKDIR /go/src/headscale
+1 -1
View File
@@ -4,7 +4,7 @@
# This Dockerfile is more or less lifted from tailscale/tailscale
# to ensure a similar build process when testing the HEAD of tailscale.
FROM golang:1.26.5-alpine AS build-env
FROM golang:1.26.4-alpine AS build-env
WORKDIR /go/src
-30
View File
@@ -1,30 +0,0 @@
# For integration testing only.
#
# Builds the Tailscale control client (integration/wasmic/wasmclient) for
# GOOS=js/GOARCH=wasm and packages it with Go's wasm_exec Node runner. The
# container idles; the integration test execs
# node /app/wasm_exec_node.js /app/client.wasm <control-url>
# to drive a real browser-style WebSocket GET against headscale's /ts2021,
# guarding the regression in issue #3357.
FROM golang:1.26.5-alpine AS build
WORKDIR /src
# Only the module metadata and the wasm client package are needed to build the
# js/wasm binary; its imports (tailscale.com/control/controlhttp, ...) resolve
# from the module proxy.
COPY go.mod go.sum ./
COPY integration/wasmic/wasmclient ./integration/wasmic/wasmclient
RUN GOOS=js GOARCH=wasm go build -o /out/client.wasm ./integration/wasmic/wasmclient \
&& cp "$(go env GOROOT)/lib/wasm/wasm_exec.js" /out/wasm_exec.js \
&& cp "$(go env GOROOT)/lib/wasm/wasm_exec_node.js" /out/wasm_exec_node.js
FROM node:24-alpine
WORKDIR /app
COPY --from=build /out/ /app/
# Idle; the test execs the client on demand with the headscale control URL.
ENTRYPOINT ["tail", "-f", "/dev/null"]
+3 -3
View File
@@ -158,17 +158,17 @@ devices. Can only be used in policy destinations.
{
"src": ["boss@"],
"dst": ["boss@"],
"ip": ["*"]
"ip": "*"
},
{
"src": ["dev1@"],
"dst": ["dev1@"],
"ip": ["*"]
"ip": "*"
},
{
"src": ["intern1@"],
"dst": ["intern1@"],
"ip": ["*"]
"ip": "*"
}
]
}
Generated
+3 -25
View File
@@ -1,27 +1,5 @@
{
"nodes": {
"flake-checks": {
"inputs": {
"flake-utils": "flake-utils",
"nixpkgs": [
"nixpkgs"
],
"treefmt-nix": "treefmt-nix"
},
"locked": {
"lastModified": 1783015821,
"narHash": "sha256-vmzYTZxIAy3OYXwkRL5GQYk8fNnh8TLWcxQRIO3ywOU=",
"owner": "kradalby",
"repo": "flake-checks",
"rev": "3c821706eb0bd07f515f9a7726b650ccae927433",
"type": "github"
},
"original": {
"owner": "kradalby",
"repo": "flake-checks",
"type": "github"
}
},
"flake-utils": {
"inputs": {
"systems": "systems"
@@ -42,11 +20,11 @@
},
"nixpkgs": {
"locked": {
"lastModified": 1784421286,
"narHash": "sha256-YKn3t7FvaargtyNHbrOw+qzDM+93q5kSCREUPNxeZQs=",
"lastModified": 1781398523,
"narHash": "sha256-NC4/V9NtyzrCCPRJrt7szMW6zEP2r0XPerXOWsQiBCI=",
"owner": "NixOS",
"repo": "nixpkgs",
"rev": "62eaae30cd05c04718500ddb44e7ef85cd45ddd5",
"rev": "ef272014ec7a5577ef64ead0aef248f82cf9f3a1",
"type": "github"
},
"original": {
+7 -8
View File
@@ -2,12 +2,11 @@
description = "headscale - Open Source Tailscale Control server";
inputs = {
# Pinned to staging-next-26.05 for Go 1.26.5: the Tailscale HEAD build
# (Dockerfile.tailscale-HEAD) requires go >= 1.26.5, and nixpkgs-unstable
# still ships 1.26.4 — the bump is merged to nixpkgs staging but the
# large-rebuild staging->unstable pipeline lags. The 26.05 line is otherwise
# current (dev tools match unstable). Switch back to nixpkgs-unstable once it
# ships go_1_26 >= 1.26.5.
# Pinned to staging-next-26.05 for Go 1.26.4 (security fix GO-2026-5037/5039):
# nixpkgs-unstable still ships 1.26.3 — the bump is merged to nixpkgs staging
# but the large-rebuild staging->unstable pipeline lags. The 26.05 line is
# otherwise current (dev tools match unstable). Switch back to nixpkgs-unstable
# once it ships go_1_26 >= 1.26.4.
nixpkgs.url = "github:NixOS/nixpkgs/staging-next-26.05";
flake-utils.url = "github:numtide/flake-utils";
};
@@ -32,7 +31,7 @@
overlays.default = _: prev:
let
pkgs = nixpkgs.legacyPackages.${prev.stdenv.hostPlatform.system};
# Go 1.26 builder; resolves to Go 1.26.5 from the pinned nixpkgs.
# Go 1.26 builder; resolves to Go 1.26.4 from the pinned nixpkgs.
buildGo = pkgs.buildGo126Module;
vendorHash = (builtins.fromJSON (builtins.readFile ./flakehashes.json)).vendor.sri;
in
@@ -102,7 +101,7 @@
};
# Build golangci-lint with stock Go 1.26 (upstream uses hardcoded Go
# version); it does not build against the pinned 1.26.5.
# version); it does not build against the pinned 1.26.4.
golangci-lint = buildGo rec {
pname = "golangci-lint";
version = "2.12.2";
+1 -1
View File
@@ -1,6 +1,6 @@
{
"vendor": {
"goModSum": "sha256-GFiMC2ktrQnpSVzJHabVP1TYwJvn5+a/BDLhvmGTUWc=",
"goModSum": "sha256-csVm5v6HZ49PBp/FCX+yK1sjV8/nuUQz3GKN21Ne1mg=",
"sri": "sha256-fzKyXNMw/2yAEhaTZu0n1NXatPO2IP0HFA2ey1vZIYM="
}
}
+1 -1
View File
@@ -1,6 +1,6 @@
module github.com/juanfont/headscale
go 1.26.5
go 1.26.4
require (
github.com/arl/statsviz v0.8.0
-5
View File
@@ -544,11 +544,6 @@ func (h *Headscale) createRouter(grpcMux *grpcRuntime.ServeMux) *chi.Mux {
r.Use(middleware.Recoverer)
r.Use(securityHeaders)
// TS2021 accepts both the native client's HTTP POST upgrade and the
// browser/WASM client's WebSocket GET upgrade; NoiseUpgradeHandler
// dispatches on the Upgrade header, not the method. Registering GET as
// well keeps the router from rejecting the WebSocket handshake with 405.
r.Get(ts2021UpgradePath, h.NoiseUpgradeHandler)
r.Post(ts2021UpgradePath, h.NoiseUpgradeHandler)
r.Get("/robots.txt", h.RobotsHandler)
+9 -57
View File
@@ -23,18 +23,6 @@ type AuthProvider interface {
AuthURL(authID types.AuthID) string
}
// machineKeyMismatch fails closed when a node looked up by NodeKey was started
// in a Noise session with a different machine key. Without this anyone holding a
// target's NodeKey could open a session with a throwaway machine key and act on
// the owner's node. Returns a 401 [HTTPError] on mismatch, nil otherwise.
func machineKeyMismatch(node types.NodeView, machineKey key.MachinePublic) error {
if node.MachineKey() != machineKey {
return NewHTTPError(http.StatusUnauthorized, "node exists with a different machine key", nil)
}
return nil
}
func (h *Headscale) handleRegister(
ctx context.Context,
req tailcfg.RegisterRequest,
@@ -235,20 +223,6 @@ 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.
//
@@ -317,40 +291,18 @@ func (h *Headscale) waitForFollowup(
}
if reg, ok := h.state.GetAuthCacheEntry(followupReg); ok {
var verdict types.AuthVerdict
select {
// Prefer a completed registration even if the context has also
// expired. When both are ready, a plain select picks at random and
// would discard a successful registration as a spurious timeout
// (issue #3385).
case verdict = <-reg.WaitForAuth():
default:
select {
case <-ctx.Done():
return nil, NewHTTPError(http.StatusUnauthorized, "registration timed out", ctx.Err())
case verdict = <-reg.WaitForAuth():
}
}
case <-ctx.Done():
return nil, NewHTTPError(http.StatusUnauthorized, "registration timed out", err)
case verdict := <-reg.WaitForAuth():
if verdict.Accept() {
if !verdict.Node.Valid() {
// registration is expired in the cache, instruct the client to try a new registration
return h.reqToNewRegisterResponse(req, machineKey)
}
if verdict.Accept() {
if !verdict.Node.Valid() {
// registration is expired in the cache, instruct the client to try a new registration
return h.reqToNewRegisterResponse(req, machineKey)
return nodeToRegisterResponse(verdict.Node), nil
}
// The followup poll is only authenticated by the auth ID in the
// URL, so fail closed unless the Noise session asking for the
// result was started with the same machine key that opened the
// registration. [State.HandleNodeFromAuthPath] resolves the node
// from the cached [types.RegistrationData.MachineKey], so the two
// match on the normal path. [Headscale.handleRegister] and
// [Headscale.handleLogout] apply the same check.
err := machineKeyMismatch(verdict.Node, machineKey)
if err != nil {
return nil, err
}
return nodeToRegisterResponse(verdict.Node), nil
}
}
-389
View File
@@ -1338,392 +1338,3 @@ 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")
}
+13 -166
View File
@@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
"strings"
"testing"
@@ -693,11 +692,6 @@ func TestAuthenticationFlows(t *testing.T) {
user := app.state.CreateUserForTest("followup-user")
node := app.state.CreateNodeForTest(user, "followup-success-node")
// [State.HandleNodeFromAuthPath] resolves the node from the
// machine key cached when the registration was opened, so on
// the real path the node carries the polling session's
// machine key. CreateNodeForTest picks a random one.
node.MachineKey = machineKey1.Public()
nodeToRegister.FinishAuth(types.AuthVerdict{Node: node.View()})
}()
@@ -1705,7 +1699,7 @@ func TestAuthenticationFlows(t *testing.T) {
assert.False(t, resp.NodeKeyExpired)
// Verify NEW node was created for user2
node2, found := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(2)]
node2, found := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(2))
require.True(t, found, "new node should exist for user2")
assert.Equal(t, uint(2), node2.UserID().Get(), "new node should belong to user2")
@@ -1713,7 +1707,7 @@ func TestAuthenticationFlows(t *testing.T) {
assert.Equal(t, "user2-context", user.Name(), "new node should show user2 username")
// Verify original node still exists for user1
node1, found := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(1)]
node1, found := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(1))
require.True(t, found, "original node should still exist for user1")
assert.Equal(t, uint(1), node1.UserID().Get(), "original node should still belong to user1")
@@ -1781,13 +1775,13 @@ func TestAuthenticationFlows(t *testing.T) {
validateCompleteResponse: true,
validate: func(t *testing.T, resp *tailcfg.RegisterResponse, app *Headscale) { //nolint:thelper
// User1's original node should STILL exist (not transferred)
node1, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(1)]
node1, found1 := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(1))
require.True(t, found1, "user1's original node should still exist")
assert.Equal(t, uint(1), node1.UserID().Get(), "user1's node should still belong to user1")
assert.Equal(t, nodeKey1.Public(), node1.NodeKey(), "user1's node should have original node key")
// User2 should have a NEW node created
node2, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(2)]
node2, found2 := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(2))
require.True(t, found2, "user2 should have new node created")
assert.Equal(t, uint(2), node2.UserID().Get(), "user2's node should belong to user2")
@@ -2920,7 +2914,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
for i := range 2 {
node := nodes[i]
// User1's original nodes should still be owned by user1
registeredNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user1.ID)]
registeredNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user1.ID))
require.True(t, found, "User1's original node %s should still exist", node.hostname)
require.Equal(t, user1.ID, registeredNode.UserID().Get(), "Node %s should still belong to user1", node.hostname)
t.Logf("✓ User1's original node %s (ID=%d) still owned by user1", node.hostname, registeredNode.ID().Uint64())
@@ -2929,7 +2923,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
for i := 2; i < 4; i++ {
node := nodes[i]
// User2's original nodes should still be owned by user2
registeredNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user2.ID)]
registeredNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user2.ID))
require.True(t, found, "User2's original node %s should still exist", node.hostname)
require.Equal(t, user2.ID, registeredNode.UserID().Get(), "Node %s should still belong to user2", node.hostname)
t.Logf("✓ User2's original node %s (ID=%d) still owned by user2", node.hostname, registeredNode.ID().Uint64())
@@ -2941,7 +2935,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
for i := 2; i < 4; i++ {
node := nodes[i]
// Should be able to find a node with user1 and this machine key (the new one)
newNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user1.ID)]
newNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user1.ID))
require.True(t, found, "Should have created new node for user1 with machine key from %s", node.hostname)
require.Equal(t, user1.ID, newNode.UserID().Get(), "New node should belong to user1")
t.Logf("✓ New node created for user1 with machine key from %s (ID=%d)", node.hostname, newNode.ID().Uint64())
@@ -2990,7 +2984,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
require.True(t, resp1.MachineAuthorized, "Should be authorized via pre-auth key")
// Verify node exists for user1
user1Node, found := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
user1Node, found := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
require.True(t, found, "Node should exist for user1")
require.Equal(t, user1.ID, user1Node.UserID().Get(), "Node should belong to user1")
user1NodeID := user1Node.ID()
@@ -3006,7 +3000,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
require.NoError(t, err)
// Verify node is expired
user1Node, found = app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
user1Node, found = app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
require.True(t, found, "Node should still exist after logout")
require.True(t, user1Node.IsExpired(), "Node should be expired after logout")
t.Logf("✓ User1 node expired (logged out)")
@@ -3047,7 +3041,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
t.Run("user1_original_node_still_exists", func(t *testing.T) {
// User1's original node should STILL exist (not transferred to user2)
user1NodeAfter, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
user1NodeAfter, found1 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
assert.True(t, found1, "User1's original node should still exist (not transferred)")
if !found1 {
@@ -3062,7 +3056,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
t.Run("user2_has_new_node_created", func(t *testing.T) {
// User2 should have a NEW node created (not transfer from user1)
user2Node, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user2.ID)]
user2Node, found2 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user2.ID))
assert.True(t, found2, "User2 should have a new node created")
if !found2 {
@@ -3086,8 +3080,8 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
t.Run("both_nodes_share_machine_key", func(t *testing.T) {
// Both nodes should have the same machine key (same physical device)
user1NodeFinal, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
user2NodeFinal, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user2.ID)]
user1NodeFinal, found1 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
user2NodeFinal, found2 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user2.ID))
require.True(t, found1, "User1 node should exist")
require.True(t, found2, "User2 node should exist")
@@ -4107,150 +4101,3 @@ func TestHandleNodeFromAuthPath_OldUserNil_NoPanic(t *testing.T) {
assert.NotEqual(t, types.NodeID(99002), node.ID(), "new node, not orphan")
assert.Equal(t, userB.ID, node.UserID().Get(), "new node belongs to userB")
}
// TestWaitForFollowupMachineKeyMismatch covers the followup poll in
// [Headscale.waitForFollowup]. That poll is authenticated only by the auth ID
// embedded in the followup URL, so without a machine-key check anyone who
// learns an ID gets the registering user's User/Login back in the
// [tailcfg.RegisterResponse].
//
// [Headscale.handleRegister] and [Headscale.handleLogout] already fail closed
// here; see the "existing_node_machine_key_mismatch" case in
// [TestAuthenticationFlows] for the equivalent assertion on that path.
//
// The nodes are given the registering session's machine key because that is
// what production produces: [State.HandleNodeFromAuthPath] resolves the node
// from the machine key cached in [types.RegistrationData] when the
// registration was opened.
func TestWaitForFollowupMachineKeyMismatch(t *testing.T) {
app := createTestApp(t)
victimMachineKey := key.NewMachine()
attackerMachineKey := key.NewMachine()
// Park a completed registration in the auth cache, as a node that is
// already polling for its verdict would see it.
newPendingFollowup := func(hostname string) string {
authID := types.MustAuthID()
regEntry := types.NewRegisterAuthRequest(&types.RegistrationData{
MachineKey: victimMachineKey.Public(),
NodeKey: key.NewNode().Public(),
Hostname: hostname,
})
app.state.SetAuthCacheEntry(authID, regEntry)
user := app.state.CreateUserForTest(hostname + "-user")
node := app.state.CreateNodeForTest(user, hostname)
node.MachineKey = victimMachineKey.Public()
// CreateNodeForTest only sets UserID, but nodeToRegisterResponse reads
// the owner, and the owner's identity is exactly what must not leak.
node.User = user
regEntry.FinishAuth(types.AuthVerdict{Node: node.View()})
return fmt.Sprintf("http://localhost:8080/register/%s", authID)
}
followup := func(url string, machineKey key.MachinePublic) (*tailcfg.RegisterResponse, error) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
return app.handleRegister(ctx, tailcfg.RegisterRequest{
Followup: url,
NodeKey: key.NewNode().Public(),
}, machineKey)
}
t.Run("mismatched machine key is rejected", func(t *testing.T) {
resp, err := followup(newPendingFollowup("followup-mismatch"), attackerMachineKey.Public())
require.Error(t, err, "followup with a foreign machine key must not succeed")
assert.Nil(t, resp, "no registration details should be returned")
var httpErr HTTPError
require.ErrorAs(t, err, &httpErr)
assert.Equal(t, http.StatusUnauthorized, httpErr.Code)
})
// Positive control. Without it a regression that stops the poll from
// finding the cache entry at all would still pass the case above, because
// waitForFollowup falls back to handing out a fresh AuthURL.
t.Run("matching machine key still completes", func(t *testing.T) {
resp, err := followup(newPendingFollowup("followup-match"), victimMachineKey.Public())
require.NoError(t, err)
require.NotNil(t, resp)
assert.True(t, resp.MachineAuthorized)
assert.NotEmpty(t, resp.User.DisplayName, "the owner's identity is returned on the legitimate path")
})
}
// TestFollowupWaitPrefersCompletedAuthOverExpiredContext reproduces
// https://github.com/juanfont/headscale/issues/3385.
//
// Root cause: [Headscale.waitForFollowup] selects on ctx.Done() and the auth
// verdict channel with equal priority. When the registration has ALREADY
// completed (verdict buffered) but the request context has ALSO expired, Go's
// select picks a ready case at random, so roughly half the time it returns
// "registration timed out" and discards a successful registration.
//
// The v0.28.0 hscontrol test suite hit this because the followup context
// timeout was only 100ms while the setup goroutine (create user + node in
// SQLite) frequently took longer on slower/constrained builders (ppc64le,
// Alpine CI). Both channels ended up ready at once and the flake surfaced as
// TestAuthenticationFlows/followup_registration_success failing with
// "http error[401]: registration timed out".
//
// The fix must give the completed-auth case priority over context
// cancellation. This test forces both cases ready on every iteration; it must
// never report a timeout.
func TestFollowupWaitPrefersCompletedAuthOverExpiredContext(t *testing.T) {
app := createTestApp(t)
machineKey := key.NewMachine().Public()
nodeKey := key.NewNode().Public()
const iterations = 300
timeouts, authorized := 0, 0
for i := range iterations {
regID, err := types.NewAuthID()
require.NoError(t, err)
authReq := types.NewRegisterAuthRequest(&types.RegistrationData{
Hostname: "followup-race-node",
})
app.state.SetAuthCacheEntry(regID, authReq)
// Registration completes BEFORE we wait: verdict is buffered.
user := app.state.CreateUserForTest(fmt.Sprintf("followup-race-user-%d", i))
node := app.state.CreateNodeForTest(user, "followup-race-node")
// waitForFollowup fails closed unless the node carries the polling
// session's machine key; production sets this via the cached
// RegistrationData. CreateNodeForTest picks a random one.
node.MachineKey = machineKey
authReq.FinishAuth(types.AuthVerdict{Node: node.View()})
// Context is expired BEFORE we wait: both select cases are ready.
ctx, cancel := context.WithCancel(context.Background())
cancel()
req := tailcfg.RegisterRequest{
Followup: fmt.Sprintf("http://localhost:8080/register/%s", regID),
NodeKey: nodeKey,
}
resp, err := app.waitForFollowup(ctx, req, machineKey)
switch {
case err != nil:
timeouts++
case resp != nil && resp.MachineAuthorized:
authorized++
}
}
assert.Zero(t, timeouts,
"waitForFollowup must never report a timeout when auth has already completed; got %d/%d timeouts",
timeouts, iterations)
assert.Equal(t, iterations, authorized, "every completed registration must be returned as authorized")
}
+1 -65
View File
@@ -706,20 +706,13 @@ AND auth_key_id NOT IN (
// but this prevents deleting users whose nodes have been
// tagged, and the ON DELETE CASCADE FK would destroy the
// tagged nodes if the user were deleted.
//
// A nil tags slice marshals to the JSON literal 'null', so
// untagged nodes can carry tags='null'. That spelling must be
// excluded alongside '[]' and '' or untagged nodes lose their
// user. Nodes already detached by the earlier version of this
// migration are repaired by the recovery migration below.
// Fixes: https://github.com/juanfont/headscale/issues/3077
// Fixes: https://github.com/juanfont/headscale/issues/3323
ID: "202602201200-clear-tagged-node-user-id",
Migrate: func(tx *gorm.DB) error {
err := tx.Exec(`
UPDATE nodes
SET user_id = NULL
WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null';
WHERE tags IS NOT NULL AND tags != '[]' AND tags != '';
`).Error
if err != nil {
return fmt.Errorf("clearing user_id on tagged nodes: %w", err)
@@ -751,63 +744,6 @@ WHERE expiry IS NOT NULL AND expiry < '1900-01-01';
},
Rollback: func(db *gorm.DB) error { return nil },
},
{
// Recover user_id on untagged nodes detached by the earlier
// version of 202602201200-clear-tagged-node-user-id, which
// treated tags='null' as tagged and cleared the user. This
// repairs databases that already upgraded to 0.29.0; fresh
// upgrades are protected by the fixed migration above and find
// nothing to repair. Recovery is best-effort: the owner is
// re-derived from the node's pre-auth key, so nodes registered
// via CLI/OIDC (no pre-auth key) cannot be recovered and must
// be reassigned manually.
// Fixes: https://github.com/juanfont/headscale/issues/3323
ID: "202606181200-recover-null-tags-node-user-id",
Migrate: func(tx *gorm.DB) error {
err := tx.Exec(`
UPDATE nodes
SET user_id = (
SELECT pak.user_id FROM pre_auth_keys pak WHERE pak.id = nodes.auth_key_id
)
WHERE user_id IS NULL
AND auth_key_id IS NOT NULL
AND (tags IS NULL OR tags = '' OR tags = '[]' OR tags = 'null');
`).Error
if err != nil {
return fmt.Errorf("recovering user_id on untagged nodes: %w", err)
}
return nil
},
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 },
},
},
)
-155
View File
@@ -198,161 +198,6 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) {
assert.False(t, node5.IsExpired(), "node5 should not be reported as expired")
},
},
// Test for the clear-tagged-node-user-id migration
// (202602201200-clear-tagged-node-user-id). A nil tags slice
// marshals to the JSON literal 'null', so untagged nodes can carry
// tags='null' in the database. The migration must only clear
// user_id on genuinely tagged nodes, not on these untagged ones.
// Fixes: https://github.com/juanfont/headscale/issues/3323
{
dbPath: "testdata/sqlite/null_tags_user_id_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, 4, "should have all 4 nodes")
byHostname := make(map[string]*types.Node, len(nodes))
for _, n := range nodes {
byHostname[n.Hostname] = n
}
// Node 1 had tags='null' (untagged) and belonged to user2.
// The migration must NOT clear its user_id.
node1 := byHostname["node1"]
require.NotNil(t, node1, "node1 should exist")
assert.False(t, node1.IsTagged(), "node1 with tags='null' should be untagged")
require.NotNil(t, node1.UserID, "node1 should keep its user assigned")
assert.Equal(t, uint(2), *node1.UserID, "node1 should still belong to user2")
// Node 2 is genuinely tagged; user_id must be cleared.
node2 := byHostname["node2"]
require.NotNil(t, node2, "node2 should exist")
assert.True(t, node2.IsTagged(), "node2 should be tagged")
assert.Nil(t, node2.UserID, "node2 (tagged) should have user_id cleared")
// Node 3 had tags='[]' (untagged); user_id preserved.
node3 := byHostname["node3"]
require.NotNil(t, node3, "node3 should exist")
assert.False(t, node3.IsTagged(), "node3 with tags='[]' should be untagged")
require.NotNil(t, node3.UserID, "node3 should keep its user assigned")
assert.Equal(t, uint(1), *node3.UserID, "node3 should still belong to user1")
// Node 4 had tags='' (untagged); user_id preserved.
node4 := byHostname["node4"]
require.NotNil(t, node4, "node4 should exist")
assert.False(t, node4.IsTagged(), "node4 with tags='' should be untagged")
require.NotNil(t, node4.UserID, "node4 should keep its user assigned")
assert.Equal(t, uint(1), *node4.UserID, "node4 should still belong to user1")
},
},
// Test for the null-tags user_id recovery migration. Databases that
// already upgraded to 0.29.0 had user_id wrongly cleared on untagged
// nodes with tags='null'. The recovery migration re-derives user_id
// from the node's pre-auth key where one exists.
// Fixes: https://github.com/juanfont/headscale/issues/3323
{
dbPath: "testdata/sqlite/recover_null_tags_user_id_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, 4, "should have all 4 nodes")
byHostname := make(map[string]*types.Node, len(nodes))
for _, n := range nodes {
byHostname[n.Hostname] = n
}
// Node 1: authkey-registered, orphaned by the bug. The recovery
// migration restores user_id from its pre-auth key (user2).
node1 := byHostname["node1"]
require.NotNil(t, node1, "node1 should exist")
require.NotNil(t, node1.UserID, "node1 user_id should be recovered")
assert.Equal(t, uint(2), *node1.UserID, "node1 should be recovered to user2")
// Node 2: genuinely tagged, correctly cleared. Must stay cleared.
node2 := byHostname["node2"]
require.NotNil(t, node2, "node2 should exist")
assert.True(t, node2.IsTagged(), "node2 should be tagged")
assert.Nil(t, node2.UserID, "node2 (tagged) must remain cleared")
// Node 3: CLI-registered, no pre-auth key. Unrecoverable.
node3 := byHostname["node3"]
require.NotNil(t, node3, "node3 should exist")
assert.Nil(t, node3.UserID, "node3 has no pre-auth key to recover from")
// Node 4: never orphaned; user_id must be untouched.
node4 := byHostname["node4"]
require.NotNil(t, node4, "node4 should exist")
require.NotNil(t, node4.UserID, "node4 user_id should be untouched")
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 {
+27 -20
View File
@@ -143,6 +143,27 @@ func GetNodeByID(tx *gorm.DB, id types.NodeID) (*types.Node, error) {
return &mach, nil
}
func (hsdb *HSDatabase) GetNodeByMachineKey(machineKey key.MachinePublic) (*types.Node, error) {
return GetNodeByMachineKey(hsdb.DB, machineKey)
}
// GetNodeByMachineKey finds a [types.Node] by its [key.MachinePublic] and returns the [types.Node] struct.
func GetNodeByMachineKey(
tx *gorm.DB,
machineKey key.MachinePublic,
) (*types.Node, error) {
mach := types.Node{}
if result := tx.
Preload("AuthKey").
Preload("AuthKey.User").
Preload("User").
First(&mach, "machine_key = ?", machineKey.String()); result.Error != nil {
return nil, result.Error
}
return &mach, nil
}
func (hsdb *HSDatabase) GetNodeByNodeKey(nodeKey key.NodePublic) (*types.Node, error) {
return GetNodeByNodeKey(hsdb.DB, nodeKey)
}
@@ -276,16 +297,12 @@ func RegisterNodeForTest(tx *gorm.DB, node types.Node, ipv4 *netip.Addr, ipv6 *n
logEvent.Msg("registering test node")
// Reuse the existing node's identity only when the same machine
// re-registers for the same user; a different user is a new node. Match on
// (machine_key, user_id) precisely - a machine key can map to several nodes
// (one per user), so a machine-key-only lookup would be ambiguous.
var oldNode types.Node
err := tx.
Where("machine_key = ? AND user_id = ?", node.MachineKey.String(), node.UserID).
First(&oldNode).Error
if err == nil {
// If the a new node is registered with the same machine key, to the same user,
// update the existing node.
// If the same node is registered again, but to a new user, then that is considered
// a new node.
oldNode, _ := GetNodeByMachineKey(tx, node.MachineKey)
if oldNode != nil && oldNode.UserID == node.UserID {
node.ID = oldNode.ID
node.GivenName = oldNode.GivenName
node.ApprovedRoutes = oldNode.ApprovedRoutes
@@ -496,16 +513,6 @@ func (e *EphemeralGarbageCollector) Cancel(nodeID types.NodeID) {
}
}
// IsScheduled reports whether a deletion timer is currently armed for nodeID.
func (e *EphemeralGarbageCollector) IsScheduled(nodeID types.NodeID) bool {
e.mu.Lock()
defer e.mu.Unlock()
_, ok := e.toBeDeleted[nodeID]
return ok
}
// Start starts the garbage collector.
func (e *EphemeralGarbageCollector) Start() {
for {
+1 -4
View File
@@ -15,10 +15,7 @@ import (
)
var (
// ErrPreAuthKeyNotFound wraps gorm.ErrRecordNotFound so an unknown or
// deleted key is treated as a missing record by callers, which the
// registration handler maps to a 401 rather than a raw server error.
ErrPreAuthKeyNotFound = fmt.Errorf("auth-key not found: %w", gorm.ErrRecordNotFound)
ErrPreAuthKeyNotFound = errors.New("auth-key not found")
ErrPreAuthKeyExpired = errors.New("auth-key expired")
ErrSingleUseAuthKeyHasBeenUsed = errors.New("auth-key has already been used")
ErrUserMismatch = errors.New("user mismatch")
-13
View File
@@ -487,16 +487,3 @@ func TestUsePreAuthKeyAtomicCAS(t *testing.T) {
"second UsePreAuthKey error must be a PAKError, got: %v", err)
assert.Equal(t, "authkey already used", pakErr.Error())
}
// TestGetPreAuthKeyUnknownMapsToRecordNotFound ensures an unknown (or deleted)
// pre-auth key resolves to a record-not-found error, which the registration
// handler maps to a 401 rather than a raw server error.
func TestGetPreAuthKeyUnknownMapsToRecordNotFound(t *testing.T) {
db, err := newSQLiteTestDB()
require.NoError(t, err)
_, err = db.GetPreAuthKey("nonexistent-key")
require.Error(t, err)
require.ErrorIs(t, err, gorm.ErrRecordNotFound,
"unknown pre-auth key must map to record-not-found (handled as 401)")
}
@@ -1,85 +0,0 @@
-- 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;
@@ -1,85 +0,0 @@
-- Test SQL dump for the clear-tagged-node-user-id migration
-- (202602201200-clear-tagged-node-user-id) against nodes whose tags
-- column holds the JSON literal 'null'.
--
-- A nil Strings slice marshals to the JSON literal `null`, so pre-0.29
-- databases contain untagged nodes with tags='null'. The migration's
-- WHERE clause (tags IS NOT NULL AND tags != '[]' AND tags != '') treats
-- the 4-character string 'null' as "tagged" and wrongly clears user_id,
-- detaching the node from its owning user on upgrade.
-- Fixes: https://github.com/juanfont/headscale/issues/3323
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
-- Migrations table: every entry BEFORE clear-tagged-node-user-id has been
-- applied. That migration is intentionally absent so it runs against this dump.
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');
-- 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);
INSERT INTO users VALUES(2,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user2','User Two','user2@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: tags='null' (untagged, nil slice marshalled to JSON null), owned by user2.
-- After migration: user_id MUST be preserved (this is the bug).
INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',2,'cli','null',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 2: genuinely tagged, owned by user1.
-- After migration: user_id MUST be cleared to NULL (tagged nodes are owned by tags).
INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',1,'cli','["tag:server"]',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 3: empty-array tags (untagged), owned by user1.
-- After migration: user_id MUST be preserved.
INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',1,'cli','[]',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: empty-string tags (untagged), owned by user1.
-- After migration: user_id MUST be preserved.
INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'cli','',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);
-- 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',2);
INSERT INTO sqlite_sequence VALUES('nodes',4);
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;
@@ -1,87 +0,0 @@
-- Test SQL dump for the null-tags user_id RECOVERY migration.
--
-- Represents a database that already upgraded to 0.29.0, where the buggy
-- clear-tagged-node-user-id migration (202602201200) already cleared
-- user_id on untagged nodes whose tags column held 'null'. The recovery
-- migration runs against this state and re-derives user_id from the node's
-- pre-auth key where possible.
-- Fixes: https://github.com/juanfont/headscale/issues/3323
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
-- Migrations table: everything through the current last migration has been
-- applied (this DB already ran the buggy clear-tagged migration). The new
-- recovery migration is intentionally absent so it runs against this dump.
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');
INSERT INTO migrations VALUES('202605221435-clear-zero-time-node-expiry');
-- 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);
INSERT INTO users VALUES(2,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user2','User Two','user2@example.com',NULL,NULL,NULL);
-- Pre-auth keys table. Key 1 belongs to user2, key 2 to user1.
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);
INSERT INTO pre_auth_keys VALUES(1,NULL,2,1,false,true,NULL,'2024-01-01 00:00:00+00:00',NULL,'pak1',NULL);
INSERT INTO pre_auth_keys VALUES(2,NULL,1,1,false,true,NULL,'2024-01-01 00:00:00+00:00',NULL,'pak2',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
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: authkey-registered, tags='null', already orphaned (user_id NULL) by
-- the buggy migration. auth_key_id=1 (user2). Recovery: user_id -> 2.
INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',NULL,'authkey','null',1,'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 2: genuinely tagged, user_id correctly cleared. Must stay NULL.
INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',NULL,'authkey','["tag:server"]',2,'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 3: CLI-registered, tags='null', orphaned, no auth_key_id.
-- Unrecoverable: must stay NULL.
INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',NULL,'cli','null',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: authkey-registered, untouched (user_id still set). Must stay user1.
INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'authkey','null',2,'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);
-- 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',2);
INSERT INTO sqlite_sequence VALUES('pre_auth_keys',2);
INSERT INTO sqlite_sequence VALUES('nodes',4);
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;
+24 -20
View File
@@ -19,6 +19,17 @@ import (
)
const (
// NoiseCapabilityVersion is used by Tailscale clients to indicate
// their codebase version. Tailscale clients can communicate over TS2021
// from CapabilityVersion 28, but we only have good support for it
// since https://github.com/tailscale/tailscale/pull/4323 (Noise in any HTTPS port).
//
// Related to this change, there is https://github.com/tailscale/tailscale/pull/5379,
// where CapabilityVersion 39 is introduced to indicate #4323 was merged.
//
// See also https://github.com/tailscale/tailscale/blob/main/tailcfg/tailcfg.go
NoiseCapabilityVersion = 39
reservedResponseHeaderSize = 4
)
@@ -191,28 +202,21 @@ func (h *Headscale) KeyHandler(
return
}
// Only disclose the Noise public key to clients this server can
// actually complete a handshake with. Gating on the same floor the
// Noise handshake enforces (capver.MinSupportedCapabilityVersion, see
// isSupportedVersion in noise.go) keeps /key consistent with /ts2021:
// versions the handshake would reject get a clear rejection here
// instead of a key that only serves as a version-boundary oracle.
// See https://github.com/juanfont/headscale/issues/3380.
if !isSupportedVersion(capVer) {
httpError(writer, NewHTTPError(http.StatusBadRequest, "unsupported client version", unsupportedClientError(capVer)))
// TS2021 (Tailscale v2 protocol) requires to have a different key
if capVer >= NoiseCapabilityVersion {
resp := tailcfg.OverTLSPublicKeyResponse{
PublicKey: h.noisePrivateKey.Public(),
}
writer.Header().Set("Content-Type", "application/json")
err := json.NewEncoder(writer).Encode(resp)
if err != nil {
log.Error().Err(err).Msg("failed to encode public key response")
}
return
}
resp := tailcfg.OverTLSPublicKeyResponse{
PublicKey: h.noisePrivateKey.Public(),
}
writer.Header().Set("Content-Type", "application/json")
err = json.NewEncoder(writer).Encode(resp)
if err != nil {
log.Error().Err(err).Msg("failed to encode public key response")
}
}
func (h *Headscale) HealthHandler(
-49
View File
@@ -4,15 +4,12 @@ import (
"bytes"
"context"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/juanfont/headscale/hscontrol/capver"
"github.com/stretchr/testify/assert"
"tailscale.com/types/key"
)
var errTestUnexpected = errors.New("unexpected failure")
@@ -50,52 +47,6 @@ func TestHandleVerifyRequest_OversizedBodyRejected(t *testing.T) {
"oversized body must surface 413")
}
// TestKeyHandler_UnsupportedCapVerDoesNotLeakKey reproduces
// https://github.com/juanfont/headscale/issues/3380. The /key handler
// must gate key disclosure on the same floor the Noise handshake
// enforces (capver.MinSupportedCapabilityVersion). A capability version
// below that floor can never complete a handshake, so it must be
// rejected rather than handed the server's Noise public key, which would
// otherwise serve only as a fingerprint / version-boundary oracle.
func TestKeyHandler_UnsupportedCapVerDoesNotLeakKey(t *testing.T) {
t.Parallel()
noise := key.NewMachine()
h := &Headscale{noisePrivateKey: &noise}
unsupported := capver.MinSupportedCapabilityVersion - 1
rec := httptest.NewRecorder()
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodGet,
fmt.Sprintf("/key?v=%d", unsupported),
nil,
)
h.KeyHandler(rec, req)
assert.Equal(t, http.StatusBadRequest, rec.Code,
"a client below the supported floor must be rejected")
assert.NotContains(t, rec.Body.String(), noise.Public().String(),
"must not disclose Noise public key to a client below the supported floor")
// A supported client still receives the key.
recOK := httptest.NewRecorder()
reqOK := httptest.NewRequestWithContext(
context.Background(),
http.MethodGet,
fmt.Sprintf("/key?v=%d", capver.MinSupportedCapabilityVersion),
nil,
)
h.KeyHandler(recOK, reqOK)
assert.Equal(t, http.StatusOK, recOK.Code)
assert.Contains(t, recOK.Body.String(), noise.Public().String(),
"a supported client must receive the Noise public key")
}
// errorAsHTTPError is a small local helper that unwraps an [HTTPError]
// from an error chain.
func errorAsHTTPError(err error) (HTTPError, bool) {
+2 -24
View File
@@ -9,8 +9,6 @@ import (
"github.com/juanfont/headscale/hscontrol/policy"
policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util/zlog/zf"
"github.com/rs/zerolog/log"
"tailscale.com/tailcfg"
"tailscale.com/types/views"
"tailscale.com/util/multierr"
@@ -146,15 +144,7 @@ func (b *MapResponseBuilder) WithSSHPolicy() *MapResponseBuilder {
sshPolicy, err := b.mapper.state.SSHPolicy(node)
if err != nil {
// SSH policy is optional for a node to function. Rather than fail the
// whole map (leaving the node unable to connect), log and continue
// without it; the node still receives a usable netmap.
log.Warn().Caller().
Err(err).
Uint64(zf.NodeID, node.ID().Uint64()).
Str(zf.NodeHostname, node.Hostname()).
Msg("building map response: skipping SSH policy for node; node will receive a map without SSH rules")
b.addError(err)
return b
}
@@ -289,19 +279,7 @@ func (b *MapResponseBuilder) buildTailPeers(peers views.Slice[types.NodeView]) (
return b.mapper.state.RoutesForPeer(node, peer, matchers)
}, b.mapper.cfg, allCapMaps[peer.ID()])
if err != nil {
// One peer with invalid data (e.g. an empty or over-long
// GivenName that fails GetFQDN) must not blank out the map for
// every node that can see it. Drop the offending peer, log it
// with the identity an operator needs to fix it, and keep
// building from the remaining valid peers.
log.Warn().Caller().
Err(err).
Uint64(zf.NodeID, peer.ID().Uint64()).
Str(zf.NodeHostname, peer.Hostname()).
Uint64("map.viewer.node.id", b.nodeID.Uint64()).
Msgf("dropping peer %d from map response: invalid node data; fix with `headscale nodes rename %d <name>`", peer.ID(), peer.ID())
continue
return nil, err
}
// [tailcfg.Node.CapMap] on a peer carries the small set of
-189
View File
@@ -1,189 +0,0 @@
//go:build !race
// This is a timing-sensitive performance regression test; the race detector's
// ~10x slowdown makes its wall-clock assertion meaningless, so it is excluded
// from -race builds. The concurrency correctness of the policy lock change it
// guards is covered under -race by TestPolicyManagerConcurrentReads in
// hscontrol/policy/v2.
package mapper
import (
"net/netip"
"runtime"
"slices"
"sync"
"testing"
"time"
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/derp"
"github.com/juanfont/headscale/hscontrol/state"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"tailscale.com/tailcfg"
)
// setupStormBatcher builds a real state+batcher with production-default
// NodeStore batching so the reconnect-storm contention is realistic. It mirrors
// setupBatcherWithTestData but lets the test control BatcherWorkers and the
// policy.
func setupStormBatcher(tb testing.TB, nodeCount, workers int, policy string) (*TestData, func()) {
tb.Helper()
tmpDir := tb.TempDir()
prefixV4 := netip.MustParsePrefix("100.64.0.0/10")
prefixV6 := netip.MustParsePrefix("fd7a:115c:a1e0::/48")
cfg := &types.Config{
Database: types.DatabaseConfig{
Type: types.DatabaseSqlite,
Sqlite: types.SqliteConfig{Path: tmpDir + "/headscale_test.db"},
},
PrefixV4: &prefixV4,
PrefixV6: &prefixV6,
IPAllocation: types.IPAllocationStrategySequential,
BaseDomain: "headscale.test",
Policy: types.PolicyConfig{Mode: types.PolicyModeDB},
DERP: types.DERPConfig{
ServerEnabled: false,
DERPMap: &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{999: {RegionID: 999}},
},
},
Tuning: types.Tuning{
BatchChangeDelay: 10 * time.Millisecond,
BatcherWorkers: workers,
// Production defaults: coalesce writes so the storm is not
// exaggerated by an unrealistically small NodeStore batch.
NodeStoreBatchSize: 100,
NodeStoreBatchTimeout: 500 * time.Millisecond,
},
}
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(tb, err)
users := database.CreateUsersForTest(1, "testuser")
dbNodes := database.CreateRegisteredNodesForTest(users[0], nodeCount, "node")
allNodes := make([]node, 0, nodeCount)
for i := range dbNodes {
allNodes = append(allNodes, node{
n: dbNodes[i],
ch: make(chan *tailcfg.MapResponse, normalBufferSize),
})
}
st, err := state.NewState(cfg)
require.NoError(tb, err)
derpMap, err := derp.GetDERPMap(cfg.DERP)
require.NoError(tb, err)
st.SetDERPMap(derpMap)
_, err = st.SetPolicy([]byte(policy))
require.NoError(tb, err)
batcher := wrapBatcherForTest(NewBatcherAndMapper(cfg, st), st)
batcher.Start()
td := &TestData{
Database: database,
Users: users,
Nodes: allNodes,
State: st,
Config: cfg,
Batcher: batcher,
}
return td, func() {
batcher.Close()
st.Close()
database.Close()
}
}
// TestInitialMapNotStarvedByReconnectStorm reproduces juanfont/headscale#3346.
//
// When every node redials at once (e.g. after a server upgrade restart), each
// connection writes the NodeStore (UpdateNodeFromMapRequest + Connect) and the
// batcher generates its initial map. All of that reads the policy through the
// PolicyManager. Before the fix the PolicyManager guarded every read with a
// single exclusive mutex, so the NodeStore writer's O(n^2) BuildPeerMap and
// every node's FilterForNode serialised against each other. On a per-node
// filter policy (autogroup:self, via, relay grants) each hold is expensive, so
// under the storm time-to-initial-map grew without bound.
//
// On the production server in #3346 this drove the batcher's per-node
// total.duration from ~4s to ~76s; tailscale clients aborted the map POST
// first and reported
//
// PollNetMap: Post ".../machine/map": unexpected EOF
//
// then redialled, feeding the storm so it never converged. An allow-all policy
// does NOT reproduce this — BuildPeerMap is cheap there; the per-node filter
// path is what makes it expensive, matching a real deployment's ACLs.
//
// The fix makes PolicyManager reads take a shared RLock so map generation runs
// concurrently. AddNode blocks until the initial map is generated and handed to
// the node channel, so its wall-clock duration is the time-to-initial-map the
// client experiences. Without the fix this test's slowest node takes ~10s+ at
// this scale (lock-bound, and more workers do not help); with it, generation
// parallelises across workers and stays well within a client's patience.
func TestInitialMapNotStarvedByReconnectStorm(t *testing.T) {
if testing.Short() {
t.Skip("timing-sensitive storm regression; skipped in -short")
}
const (
nodeCount = 300
// A per-node-filter policy: forces BuildPeerMap and FilterForNode onto
// the slow path that recompiles filter rules per node, the same shape
// as a real ACL using autogroup:self / via / relay grants.
perNodeFilterPolicy = `{"acls":[{"action":"accept","src":["autogroup:member"],"dst":["autogroup:self:*"]}]}`
// Deliberately roomy so it passes on CI's few-core runners, where the
// single-writer BuildPeerMap sets the floor (~10s) whatever the reads
// do. It still trips on a hang or a return to the ~76s serialised
// behaviour; the fine-grained concurrency is verified separately by
// TestPolicyManagerConcurrentReads under -race.
maxAcceptableLatency = 30 * time.Second
)
// Use the real available parallelism, as production does.
workers := runtime.NumCPU()
td, cleanup := setupStormBatcher(t, nodeCount, workers, perNodeFilterPolicy)
defer cleanup()
latencies := make([]time.Duration, nodeCount)
var wg sync.WaitGroup
for i := range td.Nodes {
wg.Go(func() {
n := &td.Nodes[i]
start := time.Now()
err := td.Batcher.AddNode(n.n.ID, n.ch, tailcfg.CapabilityVersion(100), nil)
latencies[i] = time.Since(start)
assert.NoError(t, err) //nolint:testifylint // assert (not require) is correct off the test goroutine
})
}
wg.Wait()
slices.Sort(latencies)
p50 := latencies[len(latencies)/2]
p95 := latencies[len(latencies)*95/100]
maxLatency := latencies[len(latencies)-1]
t.Logf("initial-map latency over %d nodes (workers=%d): p50=%s p95=%s max=%s",
nodeCount, workers, p50, p95, maxLatency)
require.Less(t, maxLatency, maxAcceptableLatency,
"slowest initial map took %s: policy reads are serialising instead of running concurrently (issue #3346)", maxLatency)
}
-89
View File
@@ -3,7 +3,6 @@ package mapper
import (
"fmt"
"net/netip"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
@@ -537,94 +536,6 @@ func TestBuildFromChangeVisibilityMatchesFullMap(t *testing.T) {
}
}
// TestFullMapResponseSurvivesPeerWithInvalidName proves a single node with an
// FQDN-invalid GivenName must not break map generation for its peers.
//
// A node whose stored GivenName is empty (ErrNodeHasNoGivenName) or yields an
// FQDN longer than MaxHostnameLength (ErrHostnameTooLong) makes GetFQDN, and
// therefore TailNode, return an error. buildTailPeers used to abort the entire
// peer list on the first such error, so MapResponseBuilder.Build() failed for
// every node that could see the bad peer; on the initial-connection path that
// surfaced as "PollNetMap: ... unexpected EOF" and the "Unable to connect to
// the Tailscale coordination server" health warning. A legacy DB row loads
// verbatim (NewNodeStore reads db.ListNodes() without re-sanitising names), so
// the bad peer persists across restart. The build for an unaffected viewer
// must succeed: the bad peer is dropped, valid peers and self survive.
func TestFullMapResponseSurvivesPeerWithInvalidName(t *testing.T) {
for _, tt := range []struct {
name string
badName string
}{
{"empty given name", ""},
{"over-long fqdn", strings.Repeat("a", types.MaxHostnameLength+1)},
} {
t.Run(tt.name, func(t *testing.T) {
tmp := t.TempDir()
p4 := netip.MustParsePrefix("100.64.0.0/10")
p6 := netip.MustParsePrefix("fd7a:115c:a1e0::/48")
cfg := &types.Config{
Database: types.DatabaseConfig{
Type: types.DatabaseSqlite,
Sqlite: types.SqliteConfig{Path: tmp + "/h.db"},
},
PrefixV4: &p4,
PrefixV6: &p6,
IPAllocation: types.IPAllocationStrategySequential,
BaseDomain: "headscale.test",
Policy: types.PolicyConfig{Mode: types.PolicyModeDB},
DERP: types.DERPConfig{
DERPMap: &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{999: {RegionID: 999}},
},
},
Tuning: types.Tuning{
NodeStoreBatchSize: state.TestBatchSize,
NodeStoreBatchTimeout: state.TestBatchTimeout,
},
}
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(t, err)
user := database.CreateUserForTest("u1")
n1 := database.CreateRegisteredNodeForTest(user, "n1") // viewer, valid
bad := database.CreateRegisteredNodeForTest(user, "bad") // peer, name corrupted below
good := database.CreateRegisteredNodeForTest(user, "good") // peer, valid control
// Simulate a legacy/corrupt row that v29 loads verbatim.
require.NoError(t, database.DB.
Model(&types.Node{}).
Where("id = ?", bad.ID).
Update("given_name", tt.badName).Error)
require.NoError(t, database.Close())
s, err := state.NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
// Allow-all so n1 sees both peers; the bad one must still be dropped.
_, err = s.SetPolicy([]byte(`{"acls":[{"action":"accept","src":["*"],"dst":["*:*"]}]}`))
require.NoError(t, err)
m := &mapper{state: s, cfg: cfg}
capVer := tailcfg.CurrentCapabilityVersion
resp, err := m.fullMapResponse(n1.ID, capVer)
require.NoError(t, err, "n1's map must build despite a peer with an invalid name")
require.NotNil(t, resp)
require.NotNil(t, resp.Node, "n1 must receive its own self node")
peers := map[tailcfg.NodeID]bool{}
for _, p := range resp.Peers {
peers[p.ID] = true
}
assert.False(t, peers[bad.ID.NodeID()], "the peer with an invalid name must be dropped")
assert.True(t, peers[good.ID.NodeID()], "valid peers must remain in the map")
})
}
}
// TestGenerateDNSConfigNilHostinfoNoPanic proves generateDNSConfig does not
// panic when a node's Hostinfo is nil (e.g. a legacy DB row with a NULL
// host_info column). addNextDNSMetadata dereferenced node.Hostinfo().OS()
-65
View File
@@ -459,71 +459,6 @@ func TestSSHActionHandler_RejectsMissingSessionWithoutCheck(t *testing.T) {
"a bogus auth_id with no active check must be rejected, body=%s", rec.Body.String())
}
// TestTS2021Route_AcceptsGETAndPOST reproduces a regression where the
// browser/WASM control client could not connect. Tailscale's JS/WASM control
// client opens /ts2021 as a WebSocket, which is an HTTP GET upgrade; the native
// Go client uses an HTTP POST upgrade. The gorilla->chi router migration
// registered /ts2021 for POST only, so the GET WebSocket handshake was rejected
// with 405 Method Not Allowed by the router before it could reach
// NoiseUpgradeHandler. Both methods must route to the handler.
//
// NoiseUpgradeHandler dispatches on the Upgrade header, not the HTTP method, so
// once the route is reachable the handler handles both upgrade styles. The
// httptest recorder is not an http.Hijacker, so the upgrade itself fails past
// the router (501 for the WebSocket path, 400 for the native path) — the point
// is only that neither is 405, i.e. the router no longer rejects GET early.
func TestTS2021Route_AcceptsGETAndPOST(t *testing.T) {
t.Parallel()
handler := createTestApp(t).HTTPHandler()
tests := []struct {
name string
method string
headers map[string]string
}{
{
name: "websocket_get_from_wasm_client",
method: http.MethodGet,
headers: map[string]string{
"Connection": "Upgrade",
"Upgrade": "websocket",
"Sec-WebSocket-Version": "13",
"Sec-WebSocket-Key": "dGhlIHNhbXBsZSBub25jZQ==",
"Sec-WebSocket-Protocol": "tailscale-control-protocol",
},
},
{
name: "native_post_upgrade",
method: http.MethodPost,
headers: map[string]string{
"Connection": "upgrade",
"Upgrade": "tailscale-control-protocol",
"X-Tailscale-Handshake": "AAAA",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
req := httptest.NewRequestWithContext(context.Background(), tt.method,
"/ts2021?X-Tailscale-Handshake=AAAA", nil)
for k, v := range tt.headers {
req.Header.Set(k, v)
}
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
assert.NotEqual(t, http.StatusMethodNotAllowed, rec.Code,
"%s /ts2021 must reach NoiseUpgradeHandler, not be rejected by the router with 405",
tt.method)
})
}
}
// newSSHActionFollowUpRequest is like newSSHActionRequest but carries the
// auth_id query parameter that marks a follow-up poll.
func newSSHActionFollowUpRequest(t *testing.T, src, dst types.NodeID, authID types.AuthID) *http.Request {
-6
View File
@@ -30,12 +30,6 @@ type PolicyManager interface {
// NodeCanHaveTag reports whether the given node can have the given tag.
NodeCanHaveTag(node types.NodeView, tag string) bool
// UserCanHaveTag reports whether the given user owns the given tag, i.e.
// is listed (directly or via a group) in the tag's tagOwners. This is the
// user half of NodeCanHaveTag, used to authorise re-auth tag changes
// against the authenticating user rather than the node's stale ownership.
UserCanHaveTag(user types.UserView, tag string) bool
// TagExists reports whether the given tag is defined in the policy.
TagExists(tag string) bool
+63 -104
View File
@@ -15,7 +15,6 @@ import (
"github.com/juanfont/headscale/hscontrol/policy/matcher"
"github.com/juanfont/headscale/hscontrol/policy/policyutil"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/puzpuzpuz/xsync/v4"
"github.com/rs/zerolog/log"
"go4.org/netipx"
"tailscale.com/net/tsaddr"
@@ -29,10 +28,7 @@ import (
var ErrInvalidTagOwner = errors.New("tag owner is not an Alias")
type PolicyManager struct {
// RWMutex, not Mutex, so concurrent map generation does not serialise on
// reads. The per-node caches are xsync.Maps so a read can fill them without
// taking the write lock.
mu sync.RWMutex
mu sync.Mutex
pol *Policy
users []types.User
nodes views.Slice[types.NodeView]
@@ -59,7 +55,7 @@ type PolicyManager struct {
viaTargetTags map[Tag]struct{}
// Lazy map of SSH policies
sshPolicyMap *xsync.Map[types.NodeID, *tailcfg.SSHPolicy]
sshPolicyMap map[types.NodeID]*tailcfg.SSHPolicy
// compiledGrants are the grants with sources pre-resolved.
// The single source of truth for filter compilation. Both
@@ -68,12 +64,12 @@ type PolicyManager struct {
userNodeIdx userNodeIndex
// Lazy map of per-node filter rules (reduced, for packet filters)
filterRulesMap *xsync.Map[types.NodeID, []tailcfg.FilterRule]
filterRulesMap map[types.NodeID][]tailcfg.FilterRule
// Lazy map of per-node matchers derived from UNREDUCED filter
// rules. Only populated on the slow path when needsPerNodeFilter
// is true; the fast path returns pm.matchers directly.
matchersForNodeMap *xsync.Map[types.NodeID, []matcher.Match]
matchersForNodeMap map[types.NodeID][]matcher.Match
// needsPerNodeFilter is true when any compiled grant requires
// per-node work (autogroup:self or via grants).
@@ -201,9 +197,9 @@ func NewPolicyManager(b []byte, users []types.User, nodes views.Slice[types.Node
pol: policy,
users: users,
nodes: nodes,
sshPolicyMap: xsync.NewMap[types.NodeID, *tailcfg.SSHPolicy](),
filterRulesMap: xsync.NewMap[types.NodeID, []tailcfg.FilterRule](),
matchersForNodeMap: xsync.NewMap[types.NodeID, []matcher.Match](),
sshPolicyMap: make(map[types.NodeID]*tailcfg.SSHPolicy, nodes.Len()),
filterRulesMap: make(map[types.NodeID][]tailcfg.FilterRule, nodes.Len()),
matchersForNodeMap: make(map[types.NodeID][]matcher.Match, nodes.Len()),
}
_, err = pm.updateLocked()
@@ -358,9 +354,9 @@ func (pm *PolicyManager) updateLocked() (bool, error) {
// TODO(kradalby): This could potentially be optimized by only clearing the
// policies for nodes that have changed. Particularly if the only difference is
// that nodes has been added or removed.
pm.sshPolicyMap.Clear()
pm.filterRulesMap.Clear()
pm.matchersForNodeMap.Clear()
clear(pm.sshPolicyMap)
clear(pm.filterRulesMap)
clear(pm.matchersForNodeMap)
}
// If nothing changed, no need to update nodes
@@ -404,8 +400,8 @@ func (pm *PolicyManager) NodeNeedsPeerRecompute(node types.NodeView) bool {
return true
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
if pm.relayTargetIPs != nil && node.InIPSet(pm.relayTargetIPs) {
return true
@@ -426,10 +422,10 @@ func (pm *PolicyManager) NodeNeedsPeerRecompute(node types.NodeView) bool {
// /machine/ssh/action/{src}/to/{dst}?local_user={local_user} per the
// SaaS wire format. Cache is invalidated on policy reload.
func (pm *PolicyManager) SSHPolicy(baseURL string, node types.NodeView) (*tailcfg.SSHPolicy, error) {
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
if sshPol, ok := pm.sshPolicyMap.Load(node.ID()); ok {
if sshPol, ok := pm.sshPolicyMap[node.ID()]; ok {
return sshPol, nil
}
@@ -438,7 +434,7 @@ func (pm *PolicyManager) SSHPolicy(baseURL string, node types.NodeView) (*tailcf
return nil, fmt.Errorf("compiling SSH policy: %w", err)
}
pm.sshPolicyMap.Store(node.ID(), sshPol)
pm.sshPolicyMap[node.ID()] = sshPol
return sshPol, nil
}
@@ -454,8 +450,8 @@ func (pm *PolicyManager) SSHPolicy(baseURL string, node types.NodeView) (*tailcf
func (pm *PolicyManager) SSHCheckParams(
srcNodeID, dstNodeID types.NodeID,
) (time.Duration, bool) {
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
if pm.pol == nil || len(pm.pol.SSHs) == 0 {
return 0, false
@@ -588,8 +584,8 @@ func (pm *PolicyManager) Filter() ([]tailcfg.FilterRule, []matcher.Match) {
return nil, nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
return pm.filter, pm.matchers
}
@@ -608,8 +604,8 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ
return nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
// Precompute each node's subnet routes and exit-node status once; the
// O(n^2) pair scans below would otherwise recompute them for every pair.
@@ -730,7 +726,7 @@ func (pm *PolicyManager) filterForNodeLocked(
return nil
}
if rules, ok := pm.filterRulesMap.Load(node.ID()); ok {
if rules, ok := pm.filterRulesMap[node.ID()]; ok {
return rules
}
@@ -742,7 +738,7 @@ func (pm *PolicyManager) filterForNodeLocked(
}
reduced := policyutil.ReduceFilterRules(node, unreduced)
pm.filterRulesMap.Store(node.ID(), reduced)
pm.filterRulesMap[node.ID()] = reduced
return reduced
}
@@ -759,8 +755,8 @@ func (pm *PolicyManager) FilterForNode(node types.NodeView) ([]tailcfg.FilterRul
return nil, nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
return pm.filterForNodeLocked(node), nil
}
@@ -780,8 +776,8 @@ func (pm *PolicyManager) MatchersForNode(node types.NodeView) ([]matcher.Match,
return nil, nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
// For global policies, return the shared global matchers.
// Via grants require per-node matchers because the global matchers
@@ -790,7 +786,7 @@ func (pm *PolicyManager) MatchersForNode(node types.NodeView) ([]matcher.Match,
return pm.matchers, nil
}
if cached, ok := pm.matchersForNodeMap.Load(node.ID()); ok {
if cached, ok := pm.matchersForNodeMap[node.ID()]; ok {
return cached, nil
}
@@ -798,7 +794,7 @@ func (pm *PolicyManager) MatchersForNode(node types.NodeView) ([]matcher.Match,
// the stored compiled grants for this specific node.
unreduced := pm.filterRulesForNodeLocked(node)
matchers := matcher.MatchesFromFilterRules(unreduced)
pm.matchersForNodeMap.Store(node.ID(), matchers)
pm.matchersForNodeMap[node.ID()] = matchers
return matchers, nil
}
@@ -817,7 +813,7 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, error) {
// Clear SSH policy map when users change to force SSH policy recomputation
// This ensures that if SSH policy compilation previously failed due to missing users,
// it will be retried with the new user list
pm.sshPolicyMap.Clear()
clear(pm.sshPolicyMap)
changed, err := pm.updateLocked()
if err != nil {
@@ -870,9 +866,9 @@ func (pm *PolicyManager) SetNodes(nodes views.Slice[types.NodeView]) (bool, erro
if !needsUpdate {
// This ensures fresh filter rules are generated for all nodes
pm.sshPolicyMap.Clear()
pm.filterRulesMap.Clear()
pm.matchersForNodeMap.Clear()
clear(pm.sshPolicyMap)
clear(pm.filterRulesMap)
clear(pm.matchersForNodeMap)
}
// Always return true when nodes changed, even if filter hash didn't change
// (can happen with autogroup:self or when nodes are added but don't affect rules)
@@ -925,8 +921,8 @@ func (pm *PolicyManager) NodeCanHaveTag(node types.NodeView, tag string) bool {
return false
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
// pm.pol is written by SetPolicy under pm.mu; reading it before the
// lock races with concurrent policy reloads.
@@ -964,37 +960,6 @@ func (pm *PolicyManager) NodeCanHaveTag(node types.NodeView, tag string) bool {
return false
}
// UserCanHaveTag reports whether the given user is one of the tag's owners
// (directly or via a group). It is the user half of [PolicyManager.NodeCanHaveTag]:
// re-authentication authorises requested tags against the authenticating user,
// because a tag-owned node carries no user and its IP is not in any owner set,
// so only the user presenting the credential can prove ownership.
func (pm *PolicyManager) UserCanHaveTag(user types.UserView, tag string) bool {
if pm == nil || !user.Valid() {
return false
}
pm.mu.RLock()
defer pm.mu.RUnlock()
if pm.pol == nil {
return false
}
owners, exists := pm.pol.TagOwners[Tag(tag)]
if !exists {
return false
}
for _, owner := range owners {
if pm.userMatchesOwner(user, owner) {
return true
}
}
return false
}
// userMatchesOwner checks if a user matches a tag owner entry.
// This is used as a fallback when the node's IP is not in the [PolicyManager.tagOwnerMap].
func (pm *PolicyManager) userMatchesOwner(user types.UserView, owner Owner) bool {
@@ -1045,8 +1010,8 @@ func (pm *PolicyManager) TagExists(tag string) bool {
return false
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
// pm.pol is written by SetPolicy under pm.mu; reading it before the
// lock races with concurrent policy reloads.
@@ -1064,8 +1029,8 @@ func (pm *PolicyManager) NodeCanApproveRoute(node types.NodeView, route netip.Pr
return false
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
// If the route to-be-approved is an exit route, then we need to check
// if the node is in allowed to approve it. This is treated differently
@@ -1127,8 +1092,8 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
return result
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
// pm.pol is written by SetPolicy under pm.mu; reading it before the
// lock races with concurrent policy reloads.
@@ -1346,8 +1311,8 @@ func (pm *PolicyManager) DebugString() string {
// pm.pol, filter, matchers, and the derived maps are all written
// under pm.mu by SetPolicy/SetUsers/SetNodes.
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
var sb strings.Builder
@@ -1510,7 +1475,7 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
// Clear cache entries for affected users only.
// For autogroup:self, we need to clear all nodes belonging to affected users
// because autogroup:self rules depend on the entire user's device set.
pm.filterRulesMap.Range(func(nodeID types.NodeID, _ []tailcfg.FilterRule) bool {
for nodeID := range pm.filterRulesMap {
// Find the user for this cached node
var nodeUserID types.UserID
@@ -1553,22 +1518,20 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
// If we found the user and they're affected, clear this cache entry
if found {
if _, affected := affectedUsers[nodeUserID]; affected {
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
delete(pm.filterRulesMap, nodeID)
delete(pm.matchersForNodeMap, nodeID)
}
} else {
// Node not found in either old or new list, clear it
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
delete(pm.filterRulesMap, nodeID)
delete(pm.matchersForNodeMap, nodeID)
}
return true
})
}
if len(affectedUsers) > 0 {
log.Debug().
Int("affected_users", len(affectedUsers)).
Int("remaining_cache_entries", pm.filterRulesMap.Size()).
Int("remaining_cache_entries", len(pm.filterRulesMap)).
Msg("Selectively cleared autogroup:self cache for affected users")
}
}
@@ -1609,27 +1572,23 @@ func (pm *PolicyManager) invalidateGlobalPolicyCache(newNodes views.Slice[types.
}
if newNode.HasNetworkChanges(oldNode) {
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
delete(pm.filterRulesMap, nodeID)
delete(pm.matchersForNodeMap, nodeID)
}
}
// Remove deleted nodes from cache
pm.filterRulesMap.Range(func(nodeID types.NodeID, _ []tailcfg.FilterRule) bool {
for nodeID := range pm.filterRulesMap {
if _, exists := newNodeMap[nodeID]; !exists {
pm.filterRulesMap.Delete(nodeID)
delete(pm.filterRulesMap, nodeID)
}
}
return true
})
pm.matchersForNodeMap.Range(func(nodeID types.NodeID, _ []matcher.Match) bool {
for nodeID := range pm.matchersForNodeMap {
if _, exists := newNodeMap[nodeID]; !exists {
pm.matchersForNodeMap.Delete(nodeID)
delete(pm.matchersForNodeMap, nodeID)
}
return true
})
}
}
// flattenTags resolves nested tag-owner references. Cycles
@@ -1811,8 +1770,8 @@ func (pm *PolicyManager) NodeCapMap(id types.NodeID) tailcfg.NodeCapMap {
return nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
src := pm.nodeAttrsMap[id]
if len(src) == 0 {
@@ -1835,8 +1794,8 @@ func (pm *PolicyManager) NodeCapMaps() map[types.NodeID]tailcfg.NodeCapMap {
return nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
out := make(map[types.NodeID]tailcfg.NodeCapMap, len(pm.nodeAttrsMap))
maps.Copy(out, pm.nodeAttrsMap)
@@ -1,103 +0,0 @@
package v2
import (
"fmt"
"sync"
"testing"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
)
// TestPolicyManagerConcurrentReads is the correctness guard for the #3346 fix:
// PolicyManager read methods take a shared RLock and populate their per-node
// caches (filterRulesMap, matchersForNodeMap) concurrently. This test hammers
// those reads from many goroutines while a writer mutates the node set, so the
// race detector catches any unsafe access to the shared caches or policy state.
//
// It uses an autogroup:self policy so reads take the per-node filter slow path
// — the same path that made #3346's reconnect storm expensive — which is where
// the lazy caches are written.
func TestPolicyManagerConcurrentReads(t *testing.T) {
users := types.Users{
{Model: gorm.Model{ID: 1}, Name: "user1", Email: "user1@headscale.net"},
{Model: gorm.Model{ID: 2}, Name: "user2", Email: "user2@headscale.net"},
{Model: gorm.Model{ID: 3}, Name: "user3", Email: "user3@headscale.net"},
}
policy := `{
"acls": [
{
"action": "accept",
"src": ["autogroup:member"],
"dst": ["autogroup:self:*"]
}
]
}`
const nodeCount = 60
nodes := make(types.Nodes, 0, nodeCount)
for i := range nodeCount {
n := node(
fmt.Sprintf("node%d", i),
fmt.Sprintf("100.64.0.%d", i+1),
fmt.Sprintf("fd7a:115c:a1e0::%d", i+1),
users[i%len(users)],
)
n.ID = types.NodeID(i + 1) //nolint:gosec // safe in test
nodes = append(nodes, n)
}
pm, err := NewPolicyManager([]byte(policy), users, nodes.ViewSlice())
require.NoError(t, err)
const (
readers = 16
iterations = 60
mutatorReloads = 30
)
var wg sync.WaitGroup
// Concurrent readers exercise every converted RLock read path, including
// the two lazily populated per-node caches. Assertions inside the
// goroutines use assert (not require) so a failure does not call
// t.FailNow from a non-test goroutine.
for r := range readers {
wg.Go(func() {
for i := range iterations {
nv := nodes[(r+i)%len(nodes)].View()
rules, err := pm.FilterForNode(nv)
assert.NoError(t, err) //nolint:testifylint // assert (not require) is correct off the test goroutine
assert.NotNil(t, rules)
_, err = pm.MatchersForNode(nv)
assert.NoError(t, err) //nolint:testifylint // assert (not require) is correct off the test goroutine
pm.Filter()
pm.NodeCapMap(nv.ID())
// BuildPeerMap is the O(n^2) writer-side read; exercise it
// under RLock too, but not every iteration.
if i%8 == 0 {
assert.NotNil(t, pm.BuildPeerMap(nodes.ViewSlice()))
}
}
})
}
// A writer repeatedly re-sets the node set, invalidating and racing the
// caches the readers are populating.
wg.Go(func() {
for range mutatorReloads {
_, err := pm.SetNodes(nodes.ViewSlice())
assert.NoError(t, err) //nolint:testifylint // assert (not require) is correct off the test goroutine
}
})
wg.Wait()
}
+7 -13
View File
@@ -8,7 +8,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/juanfont/headscale/hscontrol/policy/matcher"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/puzpuzpuz/xsync/v4"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"tailscale.com/net/tsaddr"
@@ -123,7 +122,7 @@ func TestInvalidateAutogroupSelfCache(t *testing.T) {
require.NoError(t, err)
}
require.Equal(t, len(initialNodes), pm.filterRulesMap.Size())
require.Len(t, pm.filterRulesMap, len(initialNodes))
tests := []struct {
name string
@@ -208,20 +207,19 @@ func TestInvalidateAutogroupSelfCache(t *testing.T) {
}
}
pm.filterRulesMap.Clear()
pm.filterRulesMap = make(map[types.NodeID][]tailcfg.FilterRule)
for _, n := range initialNodes {
_, err := pm.FilterForNode(n.View())
require.NoError(t, err)
}
initialCacheSize := pm.filterRulesMap.Size()
initialCacheSize := len(pm.filterRulesMap)
require.Equal(t, len(initialNodes), initialCacheSize)
pm.invalidateAutogroupSelfCache(initialNodes.ViewSlice(), tt.newNodes.ViewSlice())
// Verify the expected number of cache entries were cleared
finalCacheSize := pm.filterRulesMap.Size()
finalCacheSize := len(pm.filterRulesMap)
clearedEntries := initialCacheSize - finalCacheSize
require.Equal(t, tt.expectedCleared, clearedEntries, tt.description)
})
@@ -500,19 +498,15 @@ func TestInvalidateGlobalPolicyCache(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pm := &PolicyManager{
nodes: tt.oldNodes.ViewSlice(),
filterRulesMap: xsync.NewMap[types.NodeID, []tailcfg.FilterRule](),
matchersForNodeMap: xsync.NewMap[types.NodeID, []matcher.Match](),
}
for id, rules := range tt.initialCache {
pm.filterRulesMap.Store(id, rules)
nodes: tt.oldNodes.ViewSlice(),
filterRulesMap: tt.initialCache,
}
pm.invalidateGlobalPolicyCache(tt.newNodes.ViewSlice())
// Verify cache state
for nodeID, shouldExist := range tt.expectedCacheAfter {
_, exists := pm.filterRulesMap.Load(nodeID)
_, exists := pm.filterRulesMap[nodeID]
require.Equal(t, shouldExist, exists, "node %d cache existence mismatch", nodeID)
}
})
+2 -2
View File
@@ -128,8 +128,8 @@ func (pm *PolicyManager) RunSSHTests() error {
return nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
cache := make(map[types.NodeID]*tailcfg.SSHPolicy)
results := runSSHPolicyTests(pm.pol, pm.users, pm.nodes, cache)
+2 -2
View File
@@ -204,8 +204,8 @@ func (pm *PolicyManager) RunTests() error {
return nil
}
pm.mu.RLock()
defer pm.mu.RUnlock()
pm.mu.Lock()
defer pm.mu.Unlock()
results := runPolicyTests(pm.pol, pm.filter, pm.users, pm.nodes)
if results.AllPassed {
+8 -17
View File
@@ -96,6 +96,12 @@ func (m *mapSession) stopFromBatcher() {
}
}
func (m *mapSession) beforeServeLongPoll() {
if m.node.IsEphemeral() {
m.h.ephemeralGC.Cancel(m.node.ID)
}
}
// afterServeLongPoll is called when a long-polling session ends and the node
// is disconnected.
func (m *mapSession) afterServeLongPoll() {
@@ -138,6 +144,8 @@ func (m *mapSession) serve() {
//
//nolint:gocyclo
func (m *mapSession) serveLongPoll() {
m.beforeServeLongPoll()
m.log.Trace().Caller().Msg("long poll session started")
// connectGen is set by [state.State.Connect] below and captured by the deferred cleanup closure.
@@ -222,11 +230,6 @@ func (m *mapSession) serveLongPoll() {
mapReqChange, err := m.h.state.UpdateNodeFromMapRequest(m.node.ID, m.req)
if err != nil {
m.log.Error().Caller().Err(err).Msg("failed to update node from initial MapRequest")
// Write an explicit error rather than returning silently: a bare
// return leaves net/http to send an empty 200, which the client
// reads as "unexpected EOF" and retries forever (issue #3346).
httpError(m.w, err)
return
}
@@ -240,13 +243,6 @@ func (m *mapSession) serveLongPoll() {
connectChanges, connectGen = m.h.state.Connect(m.node.ID)
// Cancel ephemeral GC only after Connect succeeds. Cancelling at the start
// of serveLongPoll left departed nodes without a deletion timer when a
// reconnect attempt failed before Connect (issue #3382).
if m.node.IsEphemeral() {
m.h.ephemeralGC.Cancel(m.node.ID)
}
m.log.Info().Caller().Str(zf.Chan, fmt.Sprintf("%p", m.ch)).Msg("node has connected")
// TODO(kradalby): Redo the comments here
@@ -256,11 +252,6 @@ func (m *mapSession) serveLongPoll() {
// time between the node connecting and the batcher being ready.
if err := m.h.mapBatcher.AddNode(m.node.ID, m.ch, m.capVer, m.stopFromBatcher); err != nil { //nolint:noinlineerr
m.log.Error().Caller().Err(err).Msg("failed to add node to batcher")
// Write an explicit error rather than returning silently: a bare
// return leaves net/http to send an empty 200, which the client
// reads as "unexpected EOF" and retries forever (issue #3346).
httpError(m.w, err)
return
}
-189
View File
@@ -7,15 +7,12 @@ import (
"testing"
"time"
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/mapper"
"github.com/juanfont/headscale/hscontrol/state"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/types/change"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
)
type delayedSuccessResponseWriter struct {
@@ -92,192 +89,6 @@ func (w *delayedSuccessResponseWriter) WriteCount() int {
return w.writeCount
}
// recordingResponseWriter records the status code and whether anything was
// written, so a test can tell an explicit error response apart from a handler
// that returned without writing (which net/http turns into an empty 200 the
// client reads as "unexpected EOF").
type recordingResponseWriter struct {
mu sync.Mutex
header http.Header
status int
writes int
}
func (w *recordingResponseWriter) Header() http.Header {
if w.header == nil {
w.header = make(http.Header)
}
return w.header
}
func (w *recordingResponseWriter) WriteHeader(code int) {
w.mu.Lock()
defer w.mu.Unlock()
if w.status == 0 {
w.status = code
}
}
func (w *recordingResponseWriter) Write(data []byte) (int, error) {
w.mu.Lock()
defer w.mu.Unlock()
if w.status == 0 {
w.status = http.StatusOK
}
w.writes++
return len(data), nil
}
func (w *recordingResponseWriter) Flush() {}
func (w *recordingResponseWriter) statusCode() int {
w.mu.Lock()
defer w.mu.Unlock()
return w.status
}
// TestServeLongPollWritesErrorWhenInitialMapFails proves that when the initial
// map cannot be generated (here: the node's own GivenName is invalid, so
// WithSelfNode fails and AddNode errors), serveLongPoll writes an explicit HTTP
// error instead of returning with no body. Returning empty leaves net/http to
// send an empty 200, which the Tailscale client reports as
// "PollNetMap: ... unexpected EOF" and retries forever (issue #3346).
func TestServeLongPollWritesErrorWhenInitialMapFails(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("self-bad-name-user")
createdNode := app.state.CreateRegisteredNodeForTest(user, "self-bad-name-node")
// Corrupt the node's stored name to empty so GetFQDN fails for itself,
// then reload state so the bad row enters the NodeStore verbatim.
app.mapBatcher.Close()
require.NoError(t, app.state.Close())
database, err := db.NewHeadscaleDatabase(app.cfg)
require.NoError(t, err)
require.NoError(t, database.DB.
Model(&types.Node{}).
Where("id = ?", createdNode.ID).
Update("given_name", "").Error)
require.NoError(t, database.Close())
app.state, err = state.NewState(app.cfg)
require.NoError(t, err)
app.mapBatcher = mapper.NewBatcherAndMapper(app.cfg, app.state)
app.mapBatcher.Start()
t.Cleanup(func() {
app.mapBatcher.Close()
require.NoError(t, app.state.Close())
})
nodeView, ok := app.state.GetNodeByID(createdNode.ID)
require.True(t, ok)
node := nodeView.AsStruct()
ctx, cancel := context.WithCancel(context.Background())
writer := &recordingResponseWriter{}
session := app.newMapSession(ctx, tailcfg.MapRequest{
Stream: true,
Version: tailcfg.CapabilityVersion(100),
}, writer, node)
serveDone := make(chan struct{})
go func() {
session.serveLongPoll()
close(serveDone)
}()
t.Cleanup(func() {
// Break the post-disconnect reconnect wait so the goroutine exits.
dummyCh := make(chan *tailcfg.MapResponse, 1)
_ = app.mapBatcher.AddNode(node.ID, dummyCh, tailcfg.CapabilityVersion(100), nil)
cancel()
select {
case <-serveDone:
case <-time.After(2 * time.Second):
}
_ = app.mapBatcher.RemoveNode(node.ID, dummyCh)
})
assert.Eventually(t, func() bool {
return writer.statusCode() >= http.StatusInternalServerError
}, 2*time.Second, 10*time.Millisecond,
"serveLongPoll must write an HTTP error response when the initial map cannot be built, not an empty 200")
}
// TestFailedReconnectDoesNotCancelEphemeralGC proves that a
// long-poll reconnect attempt which fails before [state.State.Connect] must
// not cancel a previously armed ephemeral GC timer. Cancelling at the start of
// [mapSession.serveLongPoll] left departed ephemeral nodes stuck offline with
// no deletion scheduled (https://github.com/juanfont/headscale/issues/3382).
func TestFailedReconnectDoesNotCancelEphemeralGC(t *testing.T) {
t.Parallel()
app := createTestApp(t)
app.StartEphemeralGCForTest(t)
user := app.state.CreateUserForTest("eph-gc-cancel-user")
pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, true, nil, nil)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
_, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{
AuthKey: pak.Key,
},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{
Hostname: "eph-gc-cancel-node",
},
Expiry: time.Now().Add(24 * time.Hour),
}, machineKey.Public())
require.NoError(t, err)
nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, ok)
require.True(t, nodeView.IsEphemeral(), "node must be ephemeral so Cancel would arm on long-poll")
node := nodeView.AsStruct()
// Arm a long-lived deletion timer — the state after a normal disconnect
// has called afterServeLongPoll. A long expiry avoids racing the
// fail-before-Connect path below.
app.ephemeralGC.Schedule(node.ID, time.Hour)
require.True(t, app.ephemeralGC.IsScheduled(node.ID), "test sanity: GC timer must be armed")
// Drop the node from the NodeStore so UpdateNodeFromMapRequest fails before
// Connect, while the session still carries an ephemeral AuthKey (so the
// old Cancel-on-entry path would clear the timer).
app.state.DeleteNodeFromStoreForTest(node.ID)
writer := &recordingResponseWriter{}
session := app.newMapSession(context.Background(), tailcfg.MapRequest{
Stream: true,
Version: tailcfg.CapabilityVersion(100),
}, writer, node)
session.serveLongPoll()
assert.GreaterOrEqual(t, writer.statusCode(), http.StatusInternalServerError,
"failed reconnect must write an HTTP error before Connect")
assert.True(t, app.ephemeralGC.IsScheduled(node.ID),
"failed reconnect must not cancel the ephemeral GC timer (issue #3382)")
}
// TestGitHubIssue3129_TransientlyBlockedWriteDoesNotLeaveLiveStaleSession
// tests the scenario reported in
// https://github.com/juanfont/headscale/issues/3129.
File diff suppressed because it is too large Load Diff
-85
View File
@@ -111,88 +111,3 @@ func TestEndpointStorageInNodeStore(t *testing.T) {
}
}
}
// TestEndpointBroadcastWorthy verifies the gate that decides whether an
// endpoint-only delta is worth fanning out to peers as an incremental
// PeersChangedPatch. A delta that only adds STUN-derived endpoints (or only
// removes endpoints) is suppressed: it is churny and unlikely to be useful,
// and disco's callMeMaybe re-derives STUN paths anyway. Only deltas that
// introduce a genuinely useful (non-STUN) endpoint are broadcast-worthy.
func TestEndpointBroadcastWorthy(t *testing.T) {
local := netip.MustParseAddrPort("192.168.1.5:41641")
local2 := netip.MustParseAddrPort("192.168.1.6:41641")
stun := netip.MustParseAddrPort("203.0.113.7:41641")
stun2 := netip.MustParseAddrPort("203.0.113.8:41641")
portmap := netip.MustParseAddrPort("198.51.100.9:41641")
tests := []struct {
name string
stored []netip.AddrPort
newEPs []netip.AddrPort
newType []tailcfg.EndpointType
want bool
}{
{
name: "adds only a STUN endpoint - suppress",
stored: []netip.AddrPort{local},
newEPs: []netip.AddrPort{local, stun},
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
want: false,
},
{
name: "adds only STUN4LocalPort - suppress",
stored: []netip.AddrPort{local},
newEPs: []netip.AddrPort{local, stun},
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN4LocalPort},
want: false,
},
{
name: "adds a useful local endpoint - broadcast",
stored: []netip.AddrPort{stun},
newEPs: []netip.AddrPort{stun, local},
newType: []tailcfg.EndpointType{tailcfg.EndpointSTUN, tailcfg.EndpointLocal},
want: true,
},
{
name: "adds a useful portmapped endpoint - broadcast",
stored: []netip.AddrPort{local},
newEPs: []netip.AddrPort{local, portmap},
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointPortmapped},
want: true,
},
{
name: "pure shrink, no additions - suppress",
stored: []netip.AddrPort{local, local2},
newEPs: []netip.AddrPort{local},
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal},
want: false,
},
{
name: "nil types (older client) adding endpoint - broadcast",
stored: []netip.AddrPort{local},
newEPs: []netip.AddrPort{local, local2},
want: true,
},
{
name: "only STUN endpoints churn (replace one STUN with another) - suppress",
stored: []netip.AddrPort{local, stun},
newEPs: []netip.AddrPort{local, stun2},
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
want: false,
},
{
name: "mixed add: one STUN and one useful - broadcast",
stored: []netip.AddrPort{local},
newEPs: []netip.AddrPort{local, stun, local2},
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN, tailcfg.EndpointLocal},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := endpointBroadcastWorthy(tt.stored, tt.newEPs, tt.newType)
assert.Equal(t, tt.want, got)
})
}
}
-92
View File
@@ -1,92 +0,0 @@
package state
import (
"fmt"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util/zlog/zf"
"github.com/rs/zerolog/log"
)
// nodeHealthCheck names a class of stored-node-data defect that breaks normal
// operation and explains how to fix it. ok == true means the node passes the
// check. This is the extension point for node-data validation: add a check
// here as new corrupt-data classes surface (nil hostinfo, invalid IPs,
// tags-XOR-user violations, ...) and both the boot scan and any future caller
// run the whole set.
type nodeHealthCheck struct {
name string
check func(nv types.NodeView, cfg *types.Config) (problem, fixHint string, ok bool)
}
// nodeHealthChecks is the registry of node-data health checks. Today it carries
// the one issue #3346 needs; append to it rather than reshaping callers.
var nodeHealthChecks = []nodeHealthCheck{givenNameMapsToValidFQDN}
// givenNameMapsToValidFQDN flags a node whose stored GivenName cannot produce a
// valid FQDN (empty, or longer than MaxHostnameLength once base_domain is
// applied). Such a node cannot be rendered into a netmap — neither its own nor
// any peer's — so it must be renamed to recover.
var givenNameMapsToValidFQDN = nodeHealthCheck{
name: "given-name-maps-to-valid-fqdn",
check: func(nv types.NodeView, cfg *types.Config) (string, string, bool) {
err := types.ValidateGivenName(nv.GivenName(), cfg.BaseDomain)
if err != nil {
return err.Error(), fmt.Sprintf("headscale nodes rename %d <name>", nv.ID()), false
}
return "", "", true
},
}
// nodeHealthFinding is a single failed check for a single node.
type nodeHealthFinding struct {
nodeID types.NodeID
hostname string
check string
problem string
fixHint string
}
// scanNodeHealth runs every registered check against every node in the store
// and returns one finding per failure. It only reports — it never mutates a
// node — so an operator can repair the underlying data without the server
// silently rewriting a user-visible name.
func (s *State) scanNodeHealth() []nodeHealthFinding {
var findings []nodeHealthFinding
for _, nv := range s.nodeStore.ListNodes().All() {
for _, c := range nodeHealthChecks {
problem, fixHint, ok := c.check(nv, s.cfg)
if ok {
continue
}
findings = append(findings, nodeHealthFinding{
nodeID: nv.ID(),
hostname: nv.Hostname(),
check: c.name,
problem: problem,
fixHint: fixHint,
})
}
}
return findings
}
// logNodeHealth scans the store once and logs an actionable warning per
// finding. Called at startup so an operator learns — by node id and fix
// command — about stored data that will break map generation, without the
// server changing anything itself.
func (s *State) logNodeHealth() {
for _, f := range s.scanNodeHealth() {
log.Warn().
Uint64(zf.NodeID, f.nodeID.Uint64()).
Str(zf.NodeHostname, f.hostname).
Str("check", f.check).
Str("problem", f.problem).
Str("fix", f.fixHint).
Msg("node has invalid data that breaks map generation; rename it to restore connectivity")
}
}
-67
View File
@@ -1,67 +0,0 @@
package state
import (
"testing"
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/stretchr/testify/require"
)
func TestGivenNameMapsToValidFQDNCheck(t *testing.T) {
cfg := &types.Config{BaseDomain: "example.com"}
_, _, ok := givenNameMapsToValidFQDN.check((&types.Node{ID: 1, GivenName: "valid"}).View(), cfg)
require.True(t, ok, "a valid given name must pass the check")
problem, fixHint, ok := givenNameMapsToValidFQDN.check((&types.Node{ID: 7, GivenName: ""}).View(), cfg)
require.False(t, ok, "an empty given name must fail the check")
require.NotEmpty(t, problem)
require.Contains(t, fixHint, "rename 7", "fix hint must name the offending node")
}
// TestScanNodeHealthReportsInvalidNameWithoutMutating proves the boot scan
// reports a node whose stored name would break map generation (issue #3346)
// with an actionable fix, and that it never rewrites the stored name — the
// maintainer's decision is log-only, no silent mutation.
func TestScanNodeHealthReportsInvalidNameWithoutMutating(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(t, err)
user := database.CreateUserForTest("scan-user")
bad := database.CreateRegisteredNodeForTest(user, "scan-bad")
good := database.CreateRegisteredNodeForTest(user, "scan-good")
require.NoError(t, database.DB.
Model(&types.Node{}).
Where("id = ?", bad.ID).
Update("given_name", "").Error)
require.NoError(t, database.Close())
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
findings := s.scanNodeHealth()
var badFinding *nodeHealthFinding
for i := range findings {
require.NotEqual(t, good.ID, findings[i].nodeID, "a valid node must not be reported")
if findings[i].nodeID == bad.ID {
badFinding = &findings[i]
}
}
require.NotNil(t, badFinding, "a node with an invalid name must be reported")
require.Contains(t, badFinding.fixHint, "rename", "finding must carry an actionable fix")
// Log-only: neither the scan nor boot may rewrite the stored name.
nv, ok := s.GetNodeByID(bad.ID)
require.True(t, ok)
require.Empty(t, nv.GivenName(), "boot scan must not mutate the stored name")
}
+33 -16
View File
@@ -787,27 +787,44 @@ func (s *NodeStore) GetNodeByNodeKey(nodeKey key.NodePublic) (types.NodeView, bo
return nodeView, exists
}
// GetNodesByMachineKeyAllUsers returns every node sharing machineKey, keyed by
// owning UserID. Tagged nodes are indexed under UserID(0) (the tagged sentinel);
// user-owned nodes under their owning UserID. Returns an empty map if none.
//
// One machine key can map to several nodes (the same device registered by
// different users via the "create new, do not transfer" path). Exposing the
// whole set lets callers decide with full context — index [userID] for an exact
// match, [0] for a tagged node, or reject when the set is ambiguous — rather
// than guessing from a single arbitrary pick.
func (s *NodeStore) GetNodesByMachineKeyAllUsers(machineKey key.MachinePublic) map[types.UserID]types.NodeView {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_nodes_by_machine_key_all_users"))
// GetNodeByMachineKey returns a node by its machine key and user ID. The bool indicates if the node exists.
func (s *NodeStore) GetNodeByMachineKey(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key"))
defer timer.ObserveDuration()
nodeStoreOperations.WithLabelValues("get_nodes_by_machine_key_all_users").Inc()
nodeStoreOperations.WithLabelValues("get_by_machine_key").Inc()
userMap := s.data.Load().nodesByMachineKey[machineKey]
snapshot := s.data.Load()
if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
if node, exists := userMap[userID]; exists {
return node, true
}
}
out := make(map[types.UserID]types.NodeView, len(userMap))
maps.Copy(out, userMap)
return types.NodeView{}, false
}
return out
// GetNodeByMachineKeyAnyUser returns the first node with the given machine key,
// regardless of which user it belongs to. This is useful for scenarios like
// transferring a node to a different user when re-authenticating with a
// different user's auth key.
// If multiple nodes exist with the same machine key (different users), the
// first one found is returned (order is not guaranteed).
func (s *NodeStore) GetNodeByMachineKeyAnyUser(machineKey key.MachinePublic) (types.NodeView, bool) {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key_any_user"))
defer timer.ObserveDuration()
nodeStoreOperations.WithLabelValues("get_by_machine_key_any_user").Inc()
snapshot := s.data.Load()
if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
// Return the first node found (order not guaranteed due to map iteration)
for _, node := range userMap {
return node, true
}
}
return types.NodeView{}, false
}
// DebugString returns debug information about the [NodeStore].
-60
View File
@@ -1321,63 +1321,3 @@ func TestRebuildPeerMapsWithChangedPeersFunc(t *testing.T) {
assert.Equal(t, 1, peers1.Len(), "ListPeers for node1 should return 1")
assert.Equal(t, 1, peers2.Len(), "ListPeers for node2 should return 1")
}
// TestGetNodesByMachineKeyAllUsers ensures the lookup returns every node sharing
// a machine key keyed by owning UserID (tagged nodes under UserID(0)), so callers
// see the full set instead of a single arbitrary pick.
func TestGetNodesByMachineKeyAllUsers(t *testing.T) {
mk := key.NewMachine().Public()
t.Run("empty when absent", func(t *testing.T) {
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
store.Start()
defer store.Stop()
require.Empty(t, store.GetNodesByMachineKeyAllUsers(mk))
})
t.Run("returns all user-owned nodes keyed by user", func(t *testing.T) {
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
store.Start()
defer store.Stop()
n1 := createTestNode(1, 1, "user1", "node1")
n1.MachineKey = mk
n2 := createTestNode(2, 2, "user2", "node2")
n2.MachineKey = mk
store.PutNode(n1)
store.PutNode(n2)
all := store.GetNodesByMachineKeyAllUsers(mk)
require.Len(t, all, 2)
require.Equal(t, types.NodeID(1), all[types.UserID(1)].ID())
require.Equal(t, types.NodeID(2), all[types.UserID(2)].ID())
})
t.Run("tagged node indexed under UserID(0)", func(t *testing.T) {
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
store.Start()
defer store.Stop()
owned := createTestNode(1, 1, "user1", "node1")
owned.MachineKey = mk
tagged := createTestNode(3, 3, "user3", "node3")
tagged.MachineKey = mk
tagged.UserID = nil
tagged.User = nil
tagged.Tags = []string{"tag:foo"}
store.PutNode(owned)
store.PutNode(tagged)
all := store.GetNodesByMachineKeyAllUsers(mk)
require.Len(t, all, 2)
require.Equal(t, types.NodeID(1), all[types.UserID(1)].ID())
require.True(t, all[types.UserID(0)].IsTagged())
require.Equal(t, types.NodeID(3), all[types.UserID(0)].ID())
})
}
-257
View File
@@ -1,18 +1,14 @@
package state
import (
"errors"
"net/netip"
"sync"
"testing"
"time"
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
)
@@ -329,256 +325,3 @@ func TestReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
require.Error(t, err,
"re-auth claiming a NodeKey bound to another machine must be rejected")
}
// TestReauthPreservesEndpointsWhenClientOmitsThem proves the re-auth/update
// path keeps a node's live WireGuard endpoints when the originating
// RegisterRequest carried none. Web/OIDC relogins report endpoints via
// MapRequest, not register, so RegData.Endpoints is empty; wiping the stored
// endpoints would advertise the re-keyed node to peers endpoint-less, which
// drives head/unstable tailscale clients into one-way disco-deafness.
func TestReauthPreservesEndpointsWhenClientOmitsThem(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(t, err)
user := database.CreateUserForTest("user")
require.NoError(t, database.Close())
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
machine := key.NewMachine()
endpoints := []netip.AddrPort{
netip.MustParseAddrPort("192.168.1.5:41641"),
netip.MustParseAddrPort("10.0.0.5:41641"),
}
// Node is registered and has reported live endpoints (as after its first
// MapRequest).
node, err := s.createAndSaveNewNode(newNodeParams{
User: *user,
MachineKey: machine.Public(),
NodeKey: key.NewNode().Public(),
DiscoKey: key.NewDisco().Public(),
Hostname: "node",
Endpoints: endpoints,
RegisterMethod: util.RegisterMethodCLI,
})
require.NoError(t, err)
require.Equal(t, endpoints, node.Endpoints().AsSlice(),
"precondition: node has live endpoints")
// Node re-authenticates, rotating its NodeKey. The RegisterRequest carries
// no endpoints.
updated, err := s.applyAuthNodeUpdate(authNodeUpdateParams{
ExistingNode: node,
RegData: &types.RegistrationData{
MachineKey: machine.Public(),
NodeKey: key.NewNode().Public(),
Hostname: "node",
Hostinfo: &tailcfg.Hostinfo{},
Endpoints: nil,
},
ValidHostinfo: &tailcfg.Hostinfo{},
Hostname: "node",
User: user,
RegisterMethod: util.RegisterMethodCLI,
})
require.NoError(t, err)
assert.Equal(t, endpoints, updated.Endpoints().AsSlice(),
"re-auth without reported endpoints must preserve the node's live endpoints")
}
// TestReauthChange covers the decision both re-auth paths share: a same-user
// relogin must be an incremental peer patch (so the tailscale client takes its
// fast patch path), never a whole-node add (which strands a re-keyed,
// momentarily-endpoint-less peer disco-deaf); a policy change forces a full
// recompute; a new node is a whole-node add.
func TestReauthChange(t *testing.T) {
n := types.Node{
ID: 7,
NodeKey: key.NewNode().Public(),
DiscoKey: key.NewDisco().Public(),
}
node := n.View()
relogin := reauthChange(node, true, false)
assert.Len(t, relogin.PeerPatches, 1, "relogin must be a peer patch")
assert.Empty(t, relogin.PeersChanged, "relogin must not be a whole-node add")
added := reauthChange(node, false, false)
assert.Empty(t, added.PeerPatches)
assert.Len(t, added.PeersChanged, 1, "a new node must be a whole-node add")
pol := reauthChange(node, true, true)
assert.Empty(t, pol.PeerPatches, "a policy change must not be a peer patch")
assert.Empty(t, pol.PeersChanged)
assert.False(t, pol.IsEmpty(), "a policy change must be non-empty")
}
// TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine is the pre-auth-key
// analogue of TestReauthRejectsNodeKeyClaimedByAnotherMachine: re-registering
// via a pre-auth key must enforce the same 1:1 NodeKey<->MachineKey binding the
// auth path and poll-time validation enforce, so a node cannot rotate its key
// to a victim's and poison the NodeStore NodeKey index.
func TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
attacker := s.CreateUserForTest("attacker")
victim := s.CreateUserForTest("victim")
victimMachine := key.NewMachine()
victimNodeKey := key.NewNode()
_, err = s.createAndSaveNewNode(newNodeParams{
User: *victim,
MachineKey: victimMachine.Public(),
NodeKey: victimNodeKey.Public(),
DiscoKey: key.NewDisco().Public(),
Hostname: "victim",
RegisterMethod: util.RegisterMethodCLI,
})
require.NoError(t, err)
// Attacker registers its own node with a reusable pre-auth key.
pak, err := s.CreatePreAuthKey(attacker.TypedID(), true, false, nil, nil)
require.NoError(t, err)
attackerMachine := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "attacker"},
Expiry: time.Now().Add(24 * time.Hour),
}
_, _, err = s.HandleNodeFromPreAuthKey(regReq, attackerMachine.Public())
require.NoError(t, err)
// Attacker re-registers its own node but supplies the victim's NodeKey.
attack := regReq
attack.NodeKey = victimNodeKey.Public()
_, _, err = s.HandleNodeFromPreAuthKey(attack, attackerMachine.Public())
require.ErrorIs(t, err, ErrNodeKeyInUse,
"pre-auth-key re-registration claiming another machine's NodeKey must be rejected")
// The victim still owns its NodeKey.
owner, ok := s.GetNodeByNodeKey(victimNodeKey.Public())
require.True(t, ok)
require.Equal(t, victimMachine.Public(), owner.MachineKey(),
"victim's NodeKey index entry must be untouched")
}
var errInjectedNodeUpdate = errors.New("injected node update failure")
// TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure ensures a failed database
// write during pre-auth-key re-registration does not leave the NodeStore
// holding a node key that was never persisted: a restart would reload the old
// row and the client's current key would no longer resolve, locking it out.
func TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
user := s.CreateUserForTest("reauth-user")
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "reauth-node"},
Expiry: time.Now().Add(24 * time.Hour),
}
node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
origNodeKey := node.NodeKey()
// Fail the node row update so the re-registration's database write errors
// after the NodeStore has already been mutated.
require.NoError(t, s.db.DB.Callback().Update().Before("gorm:update").
Register("fail_node_update", func(tx *gorm.DB) {
if tx.Statement.Table == "nodes" {
_ = tx.AddError(errInjectedNodeUpdate)
}
}))
reReg := regReq
reReg.NodeKey = key.NewNode().Public() // rotate -> NodeStore mutation, then DB write fails
_, _, err = s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, s.db.DB.Callback().Update().Remove("fail_node_update"))
require.Error(t, err, "re-registration must fail when the database write fails")
got, ok := s.nodeStore.GetNode(node.ID())
require.True(t, ok)
require.Equal(t, origNodeKey, got.NodeKey(),
"NodeStore must revert to the persisted node key when the write fails")
}
// TestConcurrentPreAuthKeyRegistrationSameMachineKey ensures concurrent
// registrations of the same machine key resolve to a single node. Without
// serialising the find-then-create section, each request sees "no existing
// node" and creates its own, leaving duplicate nodes and IP allocations for
// one machine.
func TestConcurrentPreAuthKeyRegistrationSameMachineKey(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
user := s.CreateUserForTest("concurrent-user")
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
require.NoError(t, err)
machineKey := key.NewMachine()
const n = 12
var wg sync.WaitGroup
start := make(chan struct{})
errs := make(chan error, n)
for range n {
wg.Go(func() {
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "concurrent-node"},
Expiry: time.Now().Add(24 * time.Hour),
}
<-start
_, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
errs <- err
})
}
close(start)
wg.Wait()
close(errs)
for err := range errs {
require.NoError(t, err)
}
require.Equal(t, 1, s.ListNodes().Len(),
"concurrent registrations of one machine key must yield a single node")
}
-40
View File
@@ -1,40 +0,0 @@
package state
import (
"strings"
"testing"
"github.com/juanfont/headscale/hscontrol/db"
"github.com/stretchr/testify/require"
)
// TestRenameNodeRejectsNameExceedingFQDNLimit proves RenameNode rejects a name
// that is a valid DNS label but whose FQDN, under the configured base_domain,
// exceeds MaxHostnameLength. Without the FQDN-length gate such a name persists
// and then breaks map generation for the node and its peers (issue #3346):
// admin-facing writes must not be able to introduce an unmappable name.
func TestRenameNodeRejectsNameExceedingFQDNLimit(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
// A long base domain so a 63-char label overflows the 255-char FQDN bound.
cfg.BaseDomain = strings.Repeat("b", 200) + ".example.com"
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(t, err)
user := database.CreateUserForTest("rename-user")
node := database.CreateRegisteredNodeForTest(user, "rename-node")
require.NoError(t, database.Close())
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
// Valid 63-char DNS label, but the resulting FQDN exceeds 255 chars.
_, _, err = s.RenameNode(node.ID, strings.Repeat("a", 63))
require.Error(t, err, "rename to a name whose FQDN exceeds the limit must be rejected")
// A short, valid name is still accepted.
_, _, err = s.RenameNode(node.ID, "short")
require.NoError(t, err)
}
+82 -449
View File
@@ -32,7 +32,6 @@ import (
"github.com/juanfont/headscale/hscontrol/types/change"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/juanfont/headscale/hscontrol/util/zlog/zf"
"github.com/puzpuzpuz/xsync/v4"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"gorm.io/gorm"
@@ -118,13 +117,6 @@ var ErrRegistrationExpired = errors.New("registration expired")
// binding.
var ErrNodeKeyInUse = errors.New("node key already in use by another machine")
// ErrAmbiguousNodeOwnership is returned when a machine key maps to a set of
// nodes from which the correct one to update or convert cannot be determined:
// multiple user-owned candidates for a tagged conversion, or a tagged node and
// a user-owned node coexisting (impossible per validateNodeOwnership). The
// registration is rejected rather than mutating an arbitrarily-picked node.
var ErrAmbiguousNodeOwnership = errors.New("machine key maps to ambiguous node ownership")
// sshCheckPair identifies a (source, destination) node pair for
// SSH check auth tracking.
type sshCheckPair struct {
@@ -187,22 +179,6 @@ type State struct {
// persistNodeToDB so the database row always converges on [NodeStore]
// rather than being clobbered by a stale caller snapshot.
persistMu sync.Mutex
// registerLocks serialises registration per machine key so concurrent
// registrations of the same machine resolve to a single node instead of
// racing the find-then-create section and each creating their own.
// ponytail: entries are never pruned; bounded by distinct machine keys
// seen, add cleanup on node delete only if it ever matters.
registerLocks *xsync.Map[key.MachinePublic, *sync.Mutex]
}
// lockRegistration serialises registration for a single machine key and
// returns the unlock function.
func (s *State) lockRegistration(machineKey key.MachinePublic) func() {
mu, _ := s.registerLocks.LoadOrStore(machineKey, &sync.Mutex{})
mu.Lock()
return mu.Unlock
}
// NewState creates and initializes a new [State] instance, setting up the database,
@@ -286,7 +262,7 @@ func NewState(cfg *types.Config) (*State, error) {
)
nodeStore.Start()
s := &State{
return &State{
cfg: cfg,
db: db,
@@ -296,16 +272,8 @@ func NewState(cfg *types.Config) (*State, error) {
nodeStore: nodeStore,
pings: newPingTracker(),
sshCheckAuth: make(map[sshCheckPair]time.Time),
registerLocks: xsync.NewMap[key.MachinePublic, *sync.Mutex](),
}
// Surface nodes whose stored data would break map generation (e.g. an
// invalid given name from a legacy row) so an operator can fix them. This
// only logs; it never mutates a node's stored name at boot.
s.logNodeHealth()
return s, nil
sshCheckAuth: make(map[sshCheckPair]time.Time),
}, nil
}
// Close gracefully shuts down the [State] instance and releases all resources.
@@ -759,11 +727,12 @@ func (s *State) GetNodeByNodeKey(nodeKey key.NodePublic) (types.NodeView, bool)
return s.nodeStore.GetNodeByNodeKey(nodeKey)
}
// GetNodesByMachineKeyAllUsers returns every node sharing the machine key,
// keyed by owning UserID (tagged nodes under UserID(0)). See
// [NodeStore.GetNodesByMachineKeyAllUsers].
func (s *State) GetNodesByMachineKeyAllUsers(machineKey key.MachinePublic) map[types.UserID]types.NodeView {
return s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
// GetNodeByMachineKey retrieves a node by its machine key and user ID.
// The bool indicates if the node exists or is available (like "err not found").
// The NodeView might be invalid, so it must be checked with .Valid(), which must be used to ensure
// it isn't an invalid node (this is more of a node error or node is broken).
func (s *State) GetNodeByMachineKey(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
return s.nodeStore.GetNodeByMachineKey(machineKey, userID)
}
// ResolveNode looks up a node by numeric ID, IPv4/IPv6 address, given
@@ -1038,10 +1007,7 @@ func (s *State) SetApprovedRoutes(nodeID types.NodeID, routes []netip.Prefix) (t
// auto-sanitisation) and collisions error out rather than silently
// bumping a user-facing label. See HOSTNAME.md for the CLI contract.
func (s *State) RenameNode(nodeID types.NodeID, newName string) (types.NodeView, change.Change, error) {
// Validate the label AND that the resulting FQDN fits MaxHostnameLength:
// a valid 63-char label can still overflow under a long base_domain, and
// an unmappable name would break this node and its peers (issue #3346).
err := types.ValidateGivenName(newName, s.cfg.BaseDomain)
err := dnsname.ValidLabel(newName)
if err != nil {
return types.NodeView{}, change.Change{}, fmt.Errorf("renaming node: %w", err)
}
@@ -1446,13 +1412,6 @@ func (s *State) PutNodeInStoreForTest(node types.Node) types.NodeView {
return s.nodeStore.PutNode(node)
}
// DeleteNodeFromStoreForTest removes a node from the in-memory [NodeStore]
// without touching the database. Used to force [State.UpdateNodeFromMapRequest]
// failures in poll-session tests while keeping the DB row intact for later restore.
func (s *State) DeleteNodeFromStoreForTest(id types.NodeID) {
s.nodeStore.DeleteNode(id)
}
// CreateRegisteredNodeForTest creates a test node with allocated IPs. This is a convenience wrapper around the database layer.
func (s *State) CreateRegisteredNodeForTest(user *types.User, hostname ...string) *types.Node {
return s.db.CreateRegisteredNodeForTest(user, hostname...)
@@ -1622,17 +1581,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
// Validate tags BEFORE calling [NodeStore.UpdateNode] to ensure we don't modify
// [NodeStore] if validation fails. This maintains consistency between [NodeStore]
// and database.
//
// A tag-owned node carries no user and its IP is not in any tag owner's set,
// so checking the node alone rejects every tag on re-auth (#3374). Authorise
// against the authenticating user too: they are the one presenting the
// credential and may own the requested tags.
var authUser types.UserView
if params.User != nil {
authUser = params.User.View()
}
rejectedTags := s.validateRequestTagsForReauth(params.ExistingNode, authUser, requestTags)
rejectedTags := s.validateRequestTags(params.ExistingNode, requestTags)
if len(rejectedTags) > 0 {
return types.NodeView{}, fmt.Errorf(
"%w %v are invalid or not permitted",
@@ -1666,14 +1615,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
params.ValidHostinfo,
)
// Preserve the node's live endpoints when the register request carried
// none. Web/OIDC relogins report endpoints via MapRequest, not register,
// so RegData.Endpoints is empty; clearing the stored set would advertise
// the re-keyed node with no way for peers to reach it. The first
// MapRequest restores the live set.
if len(regData.Endpoints) > 0 {
node.Endpoints = regData.Endpoints
}
node.Endpoints = regData.Endpoints
// Do NOT reset IsOnline here. Online status is managed exclusively by
// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
// Resetting it during re-registration causes a false offline blip: the
@@ -1726,14 +1668,8 @@ 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 with no stale expiry: keep existing expiry - no action.
// Tagged → Tagged: keep existing expiry (nil) - no action needed
// Apply default node expiry for non-tagged nodes when the
// resolved expiry is still nil or zero (e.g., CLI registration
@@ -1754,21 +1690,8 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
// Persist to database.
// Explicitly select all node columns so GORM includes nil/zero-value fields
// (see nodeUpdateColumns comment).
//
// AuthKeyID is excluded from nodeUpdateColumns (#2862: never persist a
// possibly-deleted key's stale reference on the shared update path). But
// when a re-auth untags a node it clears AuthKeyID to nil (above), and that
// must persist or the node reloads as tagged/ephemeral after a restart and
// is garbage-collected. Writing NULL can never cause an FK error, so include
// the column only in that clearing case; the other transitions keep the
// #2862-safe column set untouched.
updateColumns := nodeUpdateColumns
if !updatedNodeView.AuthKeyID().Valid() {
updateColumns = append(slices.Clone(nodeUpdateColumns), "AuthKeyID")
}
_, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
err := tx.Select(updateColumns).Updates(updatedNodeView.AsStruct()).Error
err := tx.Select(nodeUpdateColumns).Updates(updatedNodeView.AsStruct()).Error
if err != nil {
return nil, fmt.Errorf("saving node: %w", err)
}
@@ -1972,34 +1895,6 @@ func (s *State) validateRequestTags(node types.NodeView, requestTags []string) [
return rejectedTags
}
// validateRequestTagsForReauth authorises re-auth request tags against the
// existing node OR the authenticating user. A tag-owned node (#3374) has no
// user and its IP is in no owner set, so NodeCanHaveTag alone rejects every
// tag; the authenticating user who owns the tags must also be consulted.
// Tags neither the node nor the user owns are still rejected, so this
// authorises the user, it does not skip authorisation.
func (s *State) validateRequestTagsForReauth(node types.NodeView, authUser types.UserView, requestTags []string) []string {
if len(requestTags) == 0 {
return nil
}
var rejectedTags []string
for _, tag := range requestTags {
if s.polMan.NodeCanHaveTag(node, tag) {
continue
}
if authUser.Valid() && s.polMan.UserCanHaveTag(authUser, tag) {
continue
}
rejectedTags = append(rejectedTags, tag)
}
return rejectedTags
}
// processReauthTags handles tag changes during node re-authentication.
// It processes RequestTags from the client and updates node tags accordingly.
// Returns rejected tags (if any) for post-validation error handling.
@@ -2034,34 +1929,16 @@ func (s *State) processReauthTags(
node.Tags = []string{}
node.UserID = &user.ID
node.User = user
// The node is no longer tagged, so it must not keep a reference to
// the tagged auth key. Leaving AuthKey set means a node created by a
// tagged+ephemeral key stays IsEphemeral() after converting to
// user-owned and is garbage-collected on its next disconnect,
// silently deleting the user's just-claimed device. Clearing the
// reference is persisted via the AuthKeyID column added to this
// path's write below.
node.AuthKey = nil
node.AuthKeyID = nil
}
return nil
}
// Non-empty RequestTags: validate and apply. Authorise each tag against the
// node OR the authenticating user, matching the pre-check in
// validateRequestTagsForReauth. Without the user half, a tag-owned node
// (#3374) has every tag rejected here even after the pre-check passed, so
// this returns the tags as rejected and the re-advertised tag is silently
// dropped despite a success response.
authUser := user.View()
// Non-empty RequestTags: validate and apply
var approvedTags, rejectedTags []string
for _, tag := range requestTags {
if s.polMan.NodeCanHaveTag(node.View(), tag) ||
(authUser.Valid() && s.polMan.UserCanHaveTag(authUser, tag)) {
if s.polMan.NodeCanHaveTag(node.View(), tag) {
approvedTags = append(approvedTags, tag)
} else {
rejectedTags = append(rejectedTags, tag)
@@ -2144,37 +2021,16 @@ func (s *State) HandleNodeFromAuthPath(
// Lookup existing nodes
machineKey := regData.MachineKey
existingNodeSameUser, _ := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(user.ID))
existingNodeAnyUser, _ := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
// Serialise registration for this machine so concurrent auth callbacks
// resolve to a single node rather than racing the find-then-create section.
defer s.lockRegistration(machineKey)()
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
// Named conditions - describe WHAT we found, not HOW we check it.
existingNodeSameUser, nodeExistsForSameUser := all[types.UserID(user.ID)]
taggedNode, hasTagged := all[0]
existingNodeIsTagged := hasTagged && taggedNode.IsTagged()
var existingNodeOtherUser types.NodeView
existingNodeOwnedByOtherUser := false
for uid, n := range all {
if uid != 0 && uid != types.UserID(user.ID) && !n.IsTagged() {
existingNodeOtherUser = n
existingNodeOwnedByOtherUser = true
}
}
// A tagged node and a user-owned node cannot legitimately share a machine
// key (validateNodeOwnership enforces tags XOR user ownership). If both are
// present the machine key is in a corrupt/ambiguous state; reject rather
// than converting an arbitrary node and orphaning the other.
if existingNodeIsTagged && (nodeExistsForSameUser || existingNodeOwnedByOtherUser) {
return types.NodeView{}, change.Change{}, ErrAmbiguousNodeOwnership
}
// Named conditions - describe WHAT we found, not HOW we check it
nodeExistsForSameUser := existingNodeSameUser.Valid()
nodeExistsForAnyUser := existingNodeAnyUser.Valid()
existingNodeIsTagged := nodeExistsForAnyUser && existingNodeAnyUser.IsTagged()
existingNodeOwnedByOtherUser := nodeExistsForAnyUser &&
!existingNodeIsTagged &&
existingNodeAnyUser.UserID().Get() != user.ID
// Create logger with common fields for all auth operations
logger := log.With().
@@ -2204,7 +2060,7 @@ func (s *State) HandleNodeFromAuthPath(
return types.NodeView{}, change.Change{}, err
}
} else if existingNodeIsTagged {
updateParams.ExistingNode = taggedNode
updateParams.ExistingNode = existingNodeAnyUser
updateParams.IsConvertFromTag = true
finalNode, err = s.applyAuthNodeUpdate(updateParams)
@@ -2212,7 +2068,7 @@ func (s *State) HandleNodeFromAuthPath(
return types.NodeView{}, change.Change{}, err
}
} else if existingNodeOwnedByOtherUser {
oldUser := existingNodeOtherUser.User()
oldUser := existingNodeAnyUser.User()
oldUserName := ""
if oldUser.Valid() {
@@ -2220,14 +2076,14 @@ func (s *State) HandleNodeFromAuthPath(
}
logger.Info().
Str(zf.ExistingNodeName, existingNodeOtherUser.Hostname()).
Uint64(zf.ExistingNodeID, existingNodeOtherUser.ID().Uint64()).
Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
Str(zf.OldUser, oldUserName).
Msg("Creating new node for different user (same machine key exists for another user)")
finalNode, err = s.createNewNodeFromAuth(
logger, user, regData, hostname, hostinfo,
expiry, registrationMethod, existingNodeOtherUser,
expiry, registrationMethod, existingNodeAnyUser,
)
if err != nil {
return types.NodeView{}, change.Change{}, err
@@ -2259,12 +2115,14 @@ func (s *State) HandleNodeFromAuthPath(
return finalNode, change.NodeAdded(finalNode.ID()), fmt.Errorf("updating policy manager nodes: %w", err)
}
policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
var c change.Change
if !usersChange.IsEmpty() || !nodesChange.IsEmpty() {
c = change.PolicyChange()
} else {
c = change.NodeAdded(finalNode.ID())
}
// nodeExistsForSameUser is true only for a same-user relogin; a tag->user
// conversion is excluded, as it changes the peer's User — a structural
// change peers must see in full, not a key-rotation patch.
return finalNode, reauthChange(finalNode, nodeExistsForSameUser, policyChanged), nil
return finalNode, c, nil
}
// createNewNodeFromAuth creates a new node during auth callback.
@@ -2306,60 +2164,21 @@ func (s *State) createNewNodeFromAuth(
func (s *State) findExistingNodeForPAK(
machineKey key.MachinePublic,
pak *types.PreAuthKey,
) (types.NodeView, bool, error) {
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
) (types.NodeView, bool) {
if pak.User != nil {
if node, ok := all[types.UserID(pak.User.ID)]; ok {
return node, true, nil
node, exists := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(pak.User.ID))
if exists {
return node, true
}
// The node may have been converted to a tagged node since it first
// registered (SetNodeTags clears UserID, re-indexing it under UserID(0)).
// It is still the same machine, proven by the machine key, so recognise
// it for re-registration instead of re-validating the spent key or
// creating a duplicate node. Re-registration preserves the node's tagged
// ownership. See https://github.com/juanfont/headscale/issues/3312.
if node, ok := all[0]; ok && node.IsTagged() {
return node, true, nil
}
return types.NodeView{}, false, nil
}
// A tagged key re-registers the same machine regardless of how it is
// currently owned. An existing tagged node is a plain re-registration. A
// single user-owned node is converted to tagged in place (handled by the
// caller). More than one user-owned node is ambiguous - we cannot know
// which to convert - so reject rather than convert an arbitrary one and
// orphan the rest.
// Tagged nodes have nil UserID, so they are indexed under UserID(0)
// in nodesByMachineKey. Check there for tagged PAK re-registration.
if pak.IsTagged() {
if node, ok := all[0]; ok && node.IsTagged() {
return node, true, nil
}
var userOwned types.NodeView
count := 0
for uid, node := range all {
if uid != 0 && !node.IsTagged() {
userOwned = node
count++
}
}
switch count {
case 0:
return types.NodeView{}, false, nil
case 1:
return userOwned, true, nil
default:
return types.NodeView{}, false, ErrAmbiguousNodeOwnership
}
return s.nodeStore.GetNodeByMachineKey(machineKey, 0)
}
return types.NodeView{}, false, nil
return types.NodeView{}, false
}
//nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
@@ -2367,10 +2186,6 @@ func (s *State) HandleNodeFromPreAuthKey(
regReq tailcfg.RegisterRequest,
machineKey key.MachinePublic,
) (types.NodeView, change.Change, error) {
// Serialise registration for this machine so concurrent restarts resolve
// to a single node rather than racing the find-then-create section.
defer s.lockRegistration(machineKey)()
pak, err := s.GetPreAuthKey(regReq.Auth.AuthKey)
if err != nil {
return types.NodeView{}, change.Change{}, err
@@ -2385,10 +2200,7 @@ func (s *State) HandleNodeFromPreAuthKey(
return types.TaggedDevices.Name
}
existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak)
if err != nil {
return types.NodeView{}, change.Change{}, err
}
existingNodeSameUser, existsSameUser := s.findExistingNodeForPAK(machineKey, pak)
// For existing nodes, skip validation if:
// 1. MachineKey matches (cryptographic proof of machine identity)
@@ -2407,41 +2219,10 @@ func (s *State) HandleNodeFromPreAuthKey(
isNodeKeyRotation := existsSameUser && existingNodeSameUser.Valid() &&
existingNodeSameUser.NodeKey() != regReq.NodeKey
// An expired node is genuinely re-authenticating, not just waking up, so it
// 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
// to tagged. That is an ownership change, not a plain refresh, so it must
// present a valid key rather than ride the skip-validation fast-path.
isOwnershipConversion := existsSameUser && existingNodeSameUser.Valid() &&
pak.IsTagged() && !existingNodeSameUser.IsTagged()
// A tagged key that differs from the one the node last authed with retags
// the node (see the in-place update below). Applying a key's tags is an
// authorisation decision, so the key must be validated rather than ride the
// skip-validation fast-path; otherwise a spent, revoked or expired tagged
// key could still retag a node that reuses its node key. Like isExpired,
// this boundary must not depend on the client rotating its key.
isRetag := existsSameUser && existingNodeSameUser.Valid() &&
pak.IsTagged() && existingNodeSameUser.IsTagged() &&
(!existingNodeSameUser.AuthKeyID().Valid() || existingNodeSameUser.AuthKeyID().Get() != pak.ID)
if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion && !isRetag {
// Existing, still-valid node re-registering with same NodeKey: skip
// validation. Pre-auth keys are only needed for initial authentication.
// Critical for containers that run "tailscale up --authkey=KEY" on every
// restart.
if isExistingNodeReregistering && !isNodeKeyRotation {
// Existing node re-registering with same NodeKey: skip validation.
// Pre-auth keys are only needed for initial authentication. Critical for
// containers that run "tailscale up --authkey=KEY" on every restart.
log.Debug().
Caller().
Uint64(zf.NodeID, existingNodeSameUser.ID().Uint64()).
@@ -2485,10 +2266,8 @@ func (s *State) HandleNodeFromPreAuthKey(
var finalNode types.NodeView
// If this node exists for this user, update the node in place. For a
// tags-only key (pak.User == nil) this is true when the machine already has
// a tagged node (findExistingNodeForPAK matches it under UserID 0); for a
// user-owned key it is true when the same user already has the node.
// If this node exists for this user, update the node in place.
// Note: For tags-only keys (pak.User == nil), existsSameUser is always false.
if existsSameUser && existingNodeSameUser.Valid() {
log.Trace().
Caller().
@@ -2499,23 +2278,6 @@ func (s *State) HandleNodeFromPreAuthKey(
Str(zf.UserName, pakUsername()).
Msg("Node re-registering with existing machine key and user, updating in place")
// Re-registration rotates the NodeKey to the client-supplied value.
// Enforce the same 1:1 NodeKey<->MachineKey binding the auth path
// (applyAuthNodeUpdate) and poll-time validation enforce: a NodeKey
// already bound to a different machine must not be claimed here, or a
// re-registering node could rotate its key to a victim's and poison the
// NodeStore NodeKey index, denying the victim service.
if existing, ok := s.nodeStore.GetNodeByNodeKey(regReq.NodeKey); ok &&
existing.MachineKey() != machineKey {
return types.NodeView{}, change.Change{}, ErrNodeKeyInUse
}
// Snapshot the pre-update node so the NodeStore can be rolled back if
// the database write below fails. The view points at the immutable
// pre-update snapshot (UpdateNode swaps in a new one), so this stays
// valid after the mutation.
priorNode := existingNodeSameUser.AsStruct()
// Update existing node - NodeStore first, then database
updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
node.NodeKey = regReq.NodeKey
@@ -2530,38 +2292,9 @@ func (s *State) HandleNodeFromPreAuthKey(
node.RegisterMethod = util.RegisterMethodAuthKey
// Tags from a PreAuthKey are applied on initial registration and
// re-applied whenever a *different* key is presented on
// re-registration: re-keying is Tailscale's documented way to change
// an auth-key device's tags (KB 1068 - "generate a new auth key with
// the new set of tags ... doing so replaces the device's existing
// tags"). Presenting the SAME key again (container restart,
// #2830/#3312) preserves the node's current tags and any admin
// override. A tagged key presented for a user-owned node also converts
// it, dropping user ownership.
//
// node.AuthKeyID still holds the prior key's ID here (it is reassigned
// below), and SetNodeTags leaves AuthKeyID intact, so an admin retag
// cannot masquerade as a new key.
keyChanged := node.AuthKeyID == nil || *node.AuthKeyID != pak.ID
if pak.IsTagged() && (!node.IsTagged() || keyChanged) {
wasUserOwned := !node.IsTagged()
node.Tags = pak.Tags
node.UserID = nil
node.User = nil
// Converting a user-owned node to tagged drops the user's key
// expiry (tagged nodes never expire). But retagging an
// already-tagged node must preserve a deliberate FUTURE expiry
// set via `headscale nodes expire` - that is a node property, not
// tied to the auth key - and only clear a stale PAST expiry. This
// keeps the retag path symmetric with the same-key relogin path
// (#3371) rather than silently overriding an admin decision.
if wasUserOwned || node.IsExpired() {
node.Expiry = nil
}
}
// Tags from PreAuthKey are only applied during initial registration.
// On re-registration the node keeps its existing tags and ownership.
// Only update AuthKey reference.
node.AuthKey = pak
node.AuthKeyID = &pak.ID
// Do NOT reset IsOnline here. Online status is managed exclusively by
@@ -2585,13 +2318,6 @@ 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
}
})
@@ -2601,14 +2327,8 @@ func (s *State) HandleNodeFromPreAuthKey(
_, err = hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
// Explicitly select all node columns so GORM includes nil/zero-value fields
// (see nodeUpdateColumns comment). AuthKeyID is normally excluded to
// avoid persisting a deleted key's stale reference on MapRequest
// (#2862), but re-registration presents a freshly-validated key, so
// its ID must be persisted here — otherwise a restart reloads the old
// key and any key-scoped property (e.g. Ephemeral) silently reverts.
reregColumns := append(slices.Clone(nodeUpdateColumns), "AuthKeyID")
err := tx.Select(reregColumns).Updates(updatedNodeView.AsStruct()).Error
// (see nodeUpdateColumns comment).
err := tx.Select(nodeUpdateColumns).Updates(updatedNodeView.AsStruct()).Error
if err != nil {
return nil, fmt.Errorf("saving node: %w", err)
}
@@ -2629,13 +2349,6 @@ func (s *State) HandleNodeFromPreAuthKey(
return nil, nil //nolint:nilnil // intentional: transaction success
})
if err != nil {
// The NodeStore was updated before the database write. Roll it back
// so it does not advertise a registration the database rejected
// (e.g. a node key that a restart would not reload).
if priorNode != nil {
s.nodeStore.PutNode(*priorNode)
}
return types.NodeView{}, change.Change{}, fmt.Errorf("writing node to database: %w", err)
}
@@ -2650,29 +2363,24 @@ func (s *State) HandleNodeFromPreAuthKey(
finalNode = updatedNodeView
} else {
// Node does not exist for this user with this machine key.
// For a user-owned key, check whether the machine key is already held
// by a node belonging to a different user (tags-only keys skip this;
// tagged nodes have no owning user). Any such node yields the same
// outcome - create a new node for the new user, do not transfer - so a
// single representative is enough.
var differentUserNode types.NodeView
// Node does not exist for this user with this machine key
// Check if node exists with this machine key for a different user
existingNodeAnyUser, existsAnyUser := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
belongsToDifferentUser := false
if pak.User != nil {
for uid, node := range s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey) {
if uid != 0 && !node.IsTagged() && uid != types.UserID(pak.User.ID) {
differentUserNode = node
belongsToDifferentUser = true
}
}
}
// For user-owned keys, check if node exists for a different user.
// Tags-only keys (pak.User == nil) skip this check.
// Tagged nodes are also skipped since they have no owning user.
existingIsUserOwned := existsAnyUser &&
existingNodeAnyUser.Valid() &&
!existingNodeAnyUser.IsTagged()
belongsToDifferentUser := pak.User != nil &&
existingIsUserOwned &&
existingNodeAnyUser.UserID().Get() != pak.User.ID
if belongsToDifferentUser {
// Node exists but belongs to a different user.
// Create a new node for the new user (do not transfer).
oldUser := differentUserNode.User()
oldUser := existingNodeAnyUser.User()
oldUserName := ""
if oldUser.Valid() {
@@ -2681,8 +2389,8 @@ func (s *State) HandleNodeFromPreAuthKey(
log.Info().
Caller().
Str(zf.ExistingNodeName, differentUserNode.Hostname()).
Uint64(zf.ExistingNodeID, differentUserNode.ID().Uint64()).
Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
Str(zf.MachineKey, machineKey.ShortString()).
Str(zf.OldUser, oldUserName).
Str(zf.NewUser, pakUsername()).
@@ -2722,7 +2430,7 @@ func (s *State) HandleNodeFromPreAuthKey(
Expiry: reqExpiry,
RegisterMethod: util.RegisterMethodAuthKey,
PreAuthKey: pak,
ExistingNodeForNetinfo: cmp.Or(differentUserNode, types.NodeView{}),
ExistingNodeForNetinfo: cmp.Or(existingNodeAnyUser, types.NodeView{}),
})
if err != nil {
return types.NodeView{}, change.Change{}, fmt.Errorf("creating new node: %w", err)
@@ -2740,27 +2448,14 @@ func (s *State) HandleNodeFromPreAuthKey(
return finalNode, change.NodeAdded(finalNode.ID()), fmt.Errorf("updating policy manager nodes: %w", err)
}
policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
return finalNode, reauthChange(finalNode, existsSameUser, policyChanged), nil
}
// reauthChange returns the [change.Change] to broadcast after an authentication
// that updated or created a node.
//
// A pure relogin (isRelogin: an existing node, same user, with only its NodeKey
// rotated) is sent as a minimal incremental peer patch via [change.NodeKeyRotated]
// rather than re-advertising the whole node. A policy change forces a full
// recompute; any other (new) node is a whole-node add.
func reauthChange(node types.NodeView, isRelogin, policyChanged bool) change.Change {
switch {
case policyChanged:
return change.PolicyChange()
case isRelogin:
return change.NodeKeyRotated(node)
default:
return change.NodeAdded(node.ID())
var c change.Change
if !usersChange.IsEmpty() || !nodesChange.IsEmpty() {
c = change.PolicyChange()
} else {
c = change.NodeAdded(finalNode.ID())
}
return finalNode, c, nil
}
// updatePolicyManagerUsers updates the policy manager with current users.
@@ -2961,13 +2656,8 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
updatedNode, ok := s.nodeStore.UpdateNode(id, func(currentNode *types.Node) {
peerChange := currentNode.PeerChangeFromMapRequest(req)
// Track what specifically changed. An endpoint delta is only
// broadcast-worthy when it adds a useful (non-STUN) endpoint;
// STUN-only churn and pure shrinks are suppressed to reduce peer
// churn (see endpointBroadcastWorthy). The new set is still stored
// via ApplyPeerChange below regardless of this decision.
endpointChanged = peerChange.Endpoints != nil &&
endpointBroadcastWorthy(currentNode.Endpoints, req.Endpoints, req.EndpointTypes)
// Track what specifically changed
endpointChanged = peerChange.Endpoints != nil
derpChanged = peerChange.DERPRegion != 0
hostinfoChanged = !hostinfoEqual(currentNode.View(), req.Hostinfo)
@@ -3151,63 +2841,6 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
return buildMapRequestChangeResponse(id, updatedNode, hostinfoChanged, endpointChanged, derpChanged)
}
// endpointBroadcastWorthy reports whether an endpoint-only delta is worth
// fanning out to peers as an incremental PeersChangedPatch. A delta that only
// adds STUN-derived endpoints — or only removes endpoints — is suppressed:
// bare STUN endpoints are unlikely to be open and churn a lot (the client
// re-derives those paths over disco anyway), and a pure shrink is not worth
// telling peers about. Suppressing this churn keeps peers' views stable.
//
// The decision is intentionally conservative: it gates the broadcast only,
// not storage. The node's full endpoint set (STUN included) is still stored
// and rides along the next substantive change or full MapResponse, so no
// reachable path is permanently hidden from peers.
//
// Limitation: headscale stores bare []netip.AddrPort with no per-endpoint
// type, so we can only classify the *new* request's endpoints (via the
// parallel newTypes slice). We therefore gate on whether any newly-added
// endpoint (present in new, absent from stored) is useful (non-STUN). When
// newTypes is absent or shorter than newEPs (older clients), the unknown
// endpoints are treated as useful, preserving the pre-existing always-broadcast
// behaviour and never hiding a genuinely new endpoint.
func endpointBroadcastWorthy(
stored, newEPs []netip.AddrPort,
newTypes []tailcfg.EndpointType,
) bool {
storedSet := make(map[netip.AddrPort]struct{}, len(stored))
for _, ep := range stored {
storedSet[ep] = struct{}{}
}
for i, ep := range newEPs {
if _, ok := storedSet[ep]; ok {
// Already known to peers; not a newly-added endpoint.
continue
}
// A newly-added endpoint with no type information (older client)
// is treated as useful so we never hide a genuinely new endpoint.
t := tailcfg.EndpointUnknownType
if i < len(newTypes) {
t = newTypes[i]
}
if isUsefulEndpointType(t) {
return true
}
}
return false
}
// isUsefulEndpointType reports whether an endpoint type is worth eagerly
// broadcasting to peers. STUN-derived endpoints are excluded because they are
// churny and unlikely to be directly reachable; magicsock's disco handles
// establishing those paths.
func isUsefulEndpointType(t tailcfg.EndpointType) bool {
return t != tailcfg.EndpointSTUN && t != tailcfg.EndpointSTUN4LocalPort
}
// buildMapRequestChangeResponse determines the appropriate response type for a [tailcfg.MapRequest] update.
// Hostinfo changes require a full update, while endpoint/DERP changes can use lightweight patches.
func buildMapRequestChangeResponse(
-29
View File
@@ -490,35 +490,6 @@ func EndpointOrDERPUpdate(id types.NodeID, patch *tailcfg.PeerChange) Change {
return c
}
// NodeKeyRotated returns a [Change] for a node re-logging in: its NodeKey (and
// possibly DiscoKey, key expiry, or endpoints) changed, but nothing structural
// did. Peers only need those changed fields, so it is sent as the minimal
// incremental [tailcfg.PeerChange] patch rather than re-advertising the whole
// node — the smallest update that conveys the rotation, and the least
// disruptive for peers reconciling it.
func NodeKeyRotated(node types.NodeView) Change {
nk := node.NodeKey()
dk := node.DiscoKey()
// KeyExpiry is always set: the zero value clears any prior expiry on the
// peer (un-expire), and a non-zero value carries the new expiry.
var expiry time.Time
if e, ok := node.Expiry().GetOk(); ok {
expiry = e
}
c := PeerPatched("node key rotated (relogin)", &tailcfg.PeerChange{
NodeID: tailcfg.NodeID(node.ID()), //nolint:gosec // NodeID is bounded
Key: &nk,
DiscoKey: &dk,
KeyExpiry: &expiry,
Endpoints: node.Endpoints().AsSlice(),
})
c.OriginNode = node.ID()
return c
}
// UserAdded returns a [Change] for when a user is added or updated.
// A full update is sent to refresh user profiles on all nodes.
func UserAdded() Change {
-33
View File
@@ -4,13 +4,11 @@ import (
"net/netip"
"reflect"
"testing"
"time"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
)
func TestChange_FieldSync(t *testing.T) {
@@ -655,34 +653,3 @@ func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) {
})
}
}
// TestNodeKeyRotatedEmitsPatchNotWholeNode proves a relogin is delivered to
// peers as an incremental peer patch, not a whole-node add. A whole-node add is
// non-patchifiable on the tailscale client whenever Hostinfo changed (which it
// does on relogin), forcing the broken NodeMutationAdd path that strands a
// re-keyed, momentarily-endpoint-less peer.
func TestNodeKeyRotatedEmitsPatchNotWholeNode(t *testing.T) {
expiry := time.Now().Add(24 * time.Hour).UTC()
node := types.Node{
ID: 7,
NodeKey: key.NewNode().Public(),
DiscoKey: key.NewDisco().Public(),
Endpoints: []netip.AddrPort{netip.MustParseAddrPort("192.168.1.9:41641")},
Expiry: &expiry,
}
view := node.View()
c := NodeKeyRotated(view)
assert.False(t, c.IsFull(), "relogin must be a peer patch, not a full update")
assert.Empty(t, c.PeersChanged, "relogin must not emit a whole-node PeersChanged")
require.Len(t, c.PeerPatches, 1, "relogin must emit exactly one peer patch")
patch := c.PeerPatches[0]
assert.Equal(t, view.ID().NodeID(), patch.NodeID)
require.NotNil(t, patch.Key, "patch must carry the rotated NodeKey")
assert.Equal(t, node.NodeKey, *patch.Key)
require.NotNil(t, patch.KeyExpiry, "patch must carry KeyExpiry to (un)expire the peer")
assert.Equal(t, expiry, *patch.KeyExpiry)
assert.Equal(t, []netip.AddrPort(node.Endpoints), patch.Endpoints, "patch must carry endpoints")
}
-23
View File
@@ -21,7 +21,6 @@ import (
"tailscale.com/tailcfg"
"tailscale.com/types/key"
"tailscale.com/types/views"
"tailscale.com/util/dnsname"
)
var (
@@ -569,28 +568,6 @@ func (node *Node) GetFQDN(baseDomain string) (string, error) {
return hostname, nil
}
// ValidateGivenName reports whether givenName is usable as a node's DNS label:
// a valid DNS label that, combined with baseDomain, yields an FQDN within
// MaxHostnameLength. Admin-facing write paths (e.g. node rename) reject names
// that fail this, since the mapper cannot build a map for a node — or any of
// its peers — whose GetFQDN fails. Derived paths sanitise/coerce instead.
func ValidateGivenName(givenName, baseDomain string) error {
err := dnsname.ValidLabel(givenName)
if err != nil {
return fmt.Errorf("%q is not a valid DNS label: %w", givenName, err)
}
// Reuse GetFQDN so the length bound stays identical to what the mapper
// enforces; a valid 63-char label can still overflow under a long
// base_domain.
_, err = (&Node{GivenName: givenName}).GetFQDN(baseDomain)
if err != nil {
return err
}
return nil
}
// AnnouncedRoutes returns the list of routes the node announces, as
// reported by the client in [tailcfg.Hostinfo.RoutableIPs]. Announcement alone
// does not grant visibility — see [Node.SubnetRoutes] for approval-gated
-27
View File
@@ -418,33 +418,6 @@ func TestNodeFQDN(t *testing.T) {
}
}
func TestValidateGivenName(t *testing.T) {
tests := []struct {
name string
givenName string
baseDomain string
wantErr bool
}{
{"valid", "test", "example.com", false},
{"empty", "", "example.com", true},
{"invalid label chars", "not valid", "example.com", true},
{"label too long", strings.Repeat("a", 64), "example.com", true},
// A valid 63-char label whose FQDN overflows only because the base
// domain is long: ValidLabel passes, the FQDN-length bound rejects it.
{"fqdn too long under long base domain", strings.Repeat("a", 63), strings.Repeat("b", 200) + ".example.com", true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := ValidateGivenName(tc.givenName, tc.baseDomain)
if (err != nil) != tc.wantErr {
t.Errorf("ValidateGivenName(%q, %q) error = %v, wantErr %v",
tc.givenName, tc.baseDomain, err, tc.wantErr)
}
})
}
}
func TestPeerChangeFromMapRequest(t *testing.T) {
nKeys := []key.NodePublic{
key.NewNode().Public(),
-654
View File
@@ -11,7 +11,6 @@ import (
"github.com/juanfont/headscale/integration/hsic"
"github.com/juanfont/headscale/integration/integrationutil"
"github.com/juanfont/headscale/integration/tsic"
"github.com/oauth2-proxy/mockoidc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"tailscale.com/tailcfg"
@@ -580,213 +579,6 @@ func TestTagsAuthKeyWithTagAdminOverrideReauthPreserves(t *testing.T) {
t.Logf("Test 2.5 PASS: Admin tags preserved through reauth (admin decisions are authoritative)")
}
// TestTagsReauthDifferentKeyRetagsNode reproduces issue #3370 end-to-end with a
// real tailscale client: a node registered with a single-use tag:valid-owned
// key is re-authenticated via `tailscale up --force-reauth` with a *fresh*
// single-use tag:second key. Tailscale's documented behaviour (KB 1068) is that
// re-keying replaces the device's tags, verified by the reporter against SaaS on
// the same node/IP. Before the fix headscale consumes the new key but keeps the
// old tag; after the fix the node retags in place.
//
// Unlike Test 2.5 (same reusable key + admin override -> tags preserved), this
// presents a *different* key, so it exercises the opposite arm of the retag
// discriminator. It also asserts what a state-unit test cannot: the new tag
// propagates to the node's own Self view and netmap, and the node ID and IPs are
// unchanged.
//
// https://github.com/juanfont/headscale/issues/3370
func TestTagsReauthDifferentKeyRetagsNode(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-rekey-retag"),
)
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
userMap, err := headscale.MapUsers()
require.NoError(t, err)
userID := userMap[tagTestUser].GetId()
// KEY1: single-use tag:valid-owned.
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.GetKey())
require.NoError(t, err)
var (
initialNodeID uint64
initialIPs []string
)
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 = nodes[0].GetId()
initialIPs = nodes[0].GetIpAddresses()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
t.Logf("Step 1: node %d registered with tag:valid-owned, IPs %v", initialNodeID, initialIPs)
// KEY2: fresh single-use tag:second. Re-key via --force-reauth.
key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:second"})
require.NoError(t, err)
//nolint:errcheck // result is verified via EventuallyWithT below
client.Execute([]string{
"tailscale", "up",
"--login-server=" + headscale.GetEndpoint(),
"--hostname=" + client.Hostname(),
"--authkey=" + key2.GetKey(),
"--force-reauth",
})
// Server-side: node retagged in place, same node ID and IPs, no duplicate.
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 {
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:second"})
assert.Equal(c, initialNodeID, nodes[0].GetId(), "node ID must be unchanged")
assert.ElementsMatch(c, initialIPs, nodes[0].GetIpAddresses(), "IPs must be preserved")
}
}, integrationutil.ScaledTimeout(20*time.Second), integrationutil.SlowPoll, "server must reflect the retag")
// Node self view: the new tag propagates to the client (issue #2978 surface).
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertNodeSelfHasTagsWithCollect(c, client, []string{"tag:second"})
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "node self view must reflect the retag")
// Netmap: independent serialization surface.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertNetmapSelfHasTagsWithCollect(c, client, []string{"tag:second"})
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "netmap self must reflect the retag")
t.Logf("Test #3370 PASS: re-keying with a different tagged key retagged the node in place")
}
// TestTagsReauthDifferentKeyRemovesTag is the sharpest proof of the KB 1068
// "replaces, not merges" rule at the integration level: a node registered with
// a two-tag key is re-keyed with a single-tag key, and the second tag must be
// *removed*, not retained. Tag removal is the highest-risk propagation path
// (peers must stop seeing the removed tag), so it is worth a real-client test.
//
// https://github.com/juanfont/headscale/issues/3370
func TestTagsReauthDifferentKeyRemovesTag(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-rekey-remove"),
)
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
userMap, err := headscale.MapUsers()
require.NoError(t, err)
userID := userMap[tagTestUser].GetId()
// KEY1: single-use with BOTH tags.
key1, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned", "tag:second"})
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.GetKey())
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 = nodes[0].GetId()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:second", "tag:valid-owned"})
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
// KEY2: fresh single-use with ONLY tag:valid-owned. Re-key.
key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"})
require.NoError(t, err)
//nolint:errcheck // result is verified via EventuallyWithT below
client.Execute([]string{
"tailscale", "up",
"--login-server=" + headscale.GetEndpoint(),
"--hostname=" + client.Hostname(),
"--authkey=" + key2.GetKey(),
"--force-reauth",
})
// tag:second must be gone on every surface.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1)
if len(nodes) == 1 {
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
assert.Equal(c, initialNodeID, nodes[0].GetId())
}
}, integrationutil.ScaledTimeout(20*time.Second), integrationutil.SlowPoll, "re-keying must replace (remove tag:second), not merge")
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertNodeSelfHasTagsWithCollect(c, client, []string{"tag:valid-owned"})
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "removed tag must clear from node self view")
t.Logf("Test #3370 PASS: re-keying replaced the tag set (tag:second removed)")
}
// TestTagsAuthKeyWithTagCLICannotModifyAdminTags tests that the client CLI
// cannot modify admin-assigned tags.
//
@@ -3411,449 +3203,3 @@ 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 := userMap[tagTestUser].GetId()
// 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.GetKey())
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 = nodes[0].GetId()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
assert.Nil(c, nodes[0].GetExpiry(), "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].GetExpiry(), "#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.GetKey())
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, nodes[0].GetId(), "node ID must be unchanged")
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
assert.Nil(c, nodes[0].GetExpiry(), "#3371: tagged node must have no expiry after relogin")
assert.True(c, nodes[0].GetOnline(), "#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 := userMap[tagTestUser].GetId()
// 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.GetKey())
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 = nodes[0].GetId()
assert.Nil(c, nodes[0].GetExpiry(), "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.GetKey())
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, nodes[0].GetId(), "node ID must be unchanged")
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
assert.Nil(c, nodes[0].GetExpiry(), "#3371: tagged node must have no expiry after reusable-key relogin")
assert.True(c, nodes[0].GetOnline(), "#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")
}
// TestTagsOIDCReauthAddOwnedTag reproduces issue #3374 through the interactive
// OIDC path with a real client. A node is registered tag-owned (no user) via
// --advertise-tags, then re-authenticates via OIDC advertising an ADDITIONAL
// owned tag. Because a tag-owned node has no user and its IP is in no owner
// set, the pre-fix authorization (which asked only "can this NODE have the
// tag") rejected the whole set, leaving the node logged out. The fix also
// authorises against the authenticating user, so the added owned tag is
// accepted.
//
// This is the OIDC/interactive twin of the #3370 PAK-path retag tests, and it
// hard-asserts the resulting tag SET (the pre-existing web-auth add-tag test
// only logs), so it catches the silent-drop where the pre-check passes but the
// apply-time re-check in processReauthTags still rejects.
//
// https://github.com/juanfont/headscale/issues/3374
func TestTagsOIDCReauthAddOwnedTag(t *testing.T) {
IntegrationSkip(t)
oidcUser := "oidcuser"
// Each OIDC login consumes one mock user, so the same identity must be
// listed twice: once for the initial login and once for the reauth.
spec := ScenarioSpec{
NodesPerUser: 0,
OIDCUsers: []mockoidc.MockUser{
oidcMockUser(oidcUser, true),
oidcMockUser(oidcUser, true),
},
}
scenario, err := NewScenario(spec)
require.NoError(t, err)
defer scenario.ShutdownAssertNoPanics(t)
oidcMap := map[string]string{
"HEADSCALE_OIDC_ISSUER": scenario.mockOIDC.Issuer(),
"HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(),
"CREDENTIALS_DIRECTORY_TEST": "/tmp",
"HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret",
}
// The OIDC user owns both tags. Ownership is what authorises the reauth tag
// change once the node is tag-owned. Reference the user by email; it already
// contains an "@", so no trailing "@" is added (that suffix is only for
// non-email usernames).
owner := new(policyv2.Username(oidcUser + "@headscale.net"))
policy := &policyv2.Policy{
TagOwners: policyv2.TagOwners{
"tag:valid-owned": policyv2.Owners{owner},
"tag:second": policyv2.Owners{owner},
},
ACLs: []policyv2.ACL{
{
Action: "accept",
Sources: []policyv2.Alias{policyv2.Wildcard},
Destinations: []policyv2.AliasWithPorts{{Alias: policyv2.Wildcard, Ports: []tailcfg.PortRange{tailcfg.PortRangeAny}}},
},
},
}
err = scenario.CreateHeadscaleEnvWithLoginURL(
[]tsic.Option{
tsic.WithExtraLoginArgs([]string{"--advertise-tags=tag:valid-owned"}),
},
hsic.WithTestName("tags-oidc-addtag"),
hsic.WithConfigEnv(oidcMap),
hsic.WithFileInContainer("/tmp/hs_client_oidc_secret", []byte(scenario.mockOIDC.ClientSecret())),
hsic.WithACLPolicy(policy),
)
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
client, err := scenario.CreateTailscaleNode(
"unstable",
tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]),
tsic.WithExtraLoginArgs([]string{"--advertise-tags=tag:valid-owned"}),
)
require.NoError(t, err)
// Initial OIDC login advertising tag:valid-owned.
u, err := client.LoginWithURL(headscale.GetEndpoint())
require.NoError(t, err)
_, err = doLoginURL(client.Hostname(), u)
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 = nodes[0].GetId()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial tag-owned registration")
// Re-authenticate advertising an ADDITIONAL owned tag via --force-reauth,
// which drives the register/auth path (HandleNodeFromAuthPath), not a poll.
// Parse the login URL from this command's own output; issuing a separate
// `tailscale up` while a reauth is pending errors server-side.
command := []string{
"tailscale", "up",
"--login-server=" + headscale.GetEndpoint(),
"--hostname=" + client.Hostname(),
"--advertise-tags=tag:valid-owned,tag:second",
"--force-reauth",
}
stdout, stderr, _ := client.Execute(command)
t.Logf("reauth command output: stdout=%s stderr=%s", stdout, stderr)
loginURL, err := util.ParseLoginURLFromCLILogin(stdout + stderr)
require.NoError(t, err, "failed to parse login URL from reauth command")
_, err = doLoginURL(client.Hostname(), loginURL)
require.NoError(t, err)
// Both tags must be present — not rejected, not silently dropped.
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, nodes[0].GetId(), "node ID must be unchanged")
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned", "tag:second"})
}
}, integrationutil.ScaledTimeout(30*time.Second), integrationutil.SlowPoll, "#3374: added owned tag must be accepted on OIDC reauth")
t.Logf("Test #3374 PASS: OIDC reauth added an owned tag to a tag-owned node")
}
// TestTagsReauthEmptyTagsReturnsToUserSurvives covers the #3374 untag path with
// a real client: a tag-owned node created by a tagged+ephemeral key
// re-authenticates via user login with an EMPTY tag set. It must return to the
// user, and — crucially — survive: clearing the tags must also clear the
// node's reference to the ephemeral auth key, or the node stays IsEphemeral()
// and is garbage-collected on its next disconnect, silently deleting the
// user's just-claimed device.
//
// https://github.com/juanfont/headscale/issues/3374
func TestTagsReauthEmptyTagsReturnsToUserSurvives(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.CreateHeadscaleEnvWithLoginURL(
[]tsic.Option{},
hsic.WithACLPolicy(tagsTestPolicy()),
hsic.WithTestName("tags-untag-survive"),
)
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
userMap, err := headscale.MapUsers()
require.NoError(t, err)
userID := userMap[tagTestUser].GetId()
// A tagged + EPHEMERAL key: the node is tag-owned and ephemeral.
key, err := scenario.CreatePreAuthKeyWithTags(userID, false, true, []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.GetKey())
require.NoError(t, err)
err = client.WaitForRunning(integrationutil.PeerSyncTimeout())
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 = nodes[0].GetId()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial tag-owned ephemeral registration")
// Re-authenticate with an EMPTY tag set via --force-reauth. An
// already-authenticated node only emits a fresh login URL when forced, so
// parse the URL from this command's own output rather than issuing a
// second `tailscale up` (which would error with "no URL found").
command := []string{
"tailscale", "up",
"--login-server=" + headscale.GetEndpoint(),
"--hostname=" + client.Hostname(),
"--advertise-tags=",
"--force-reauth",
}
stdout, stderr, _ := client.Execute(command)
t.Logf("reauth command output: stdout=%s stderr=%s", stdout, stderr)
loginURL, err := util.ParseLoginURLFromCLILogin(stdout + stderr)
require.NoError(t, err, "failed to parse login URL from reauth command")
body, err := doLoginURL(client.Hostname(), loginURL)
require.NoError(t, err)
// CLI user-login registration untags the node and returns it to the user.
err = scenario.runHeadscaleRegister(tagTestUser, body)
require.NoError(t, err)
// Node returns to the user, keeps its ID, and does NOT vanish.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1, "#3374: untagged node must survive, not be GC'd as ephemeral")
if len(nodes) == 1 {
assert.Equal(c, initialNodeID, nodes[0].GetId(), "node ID must be unchanged")
assert.Empty(c, nodes[0].GetTags(), "#3374: node must have no tags after untag")
// A user-owned node reports its real user; a tagged node would
// report the special "tagged-devices" user instead.
assert.Equal(c, tagTestUser, nodes[0].GetUser().GetName(), "#3374: untagged node must return to the authenticating user")
}
}, integrationutil.ScaledTimeout(30*time.Second), integrationutil.SlowPoll, "#3374: empty-tags reauth returns node to user and it survives")
t.Logf("Test #3374 PASS: empty-tags reauth returned the ephemeral tag-owned node to its user and it survived")
}
-315
View File
@@ -1,315 +0,0 @@
package integration
import (
"context"
"crypto/tls"
"crypto/x509"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/netip"
"net/url"
"testing"
"time"
"github.com/coder/websocket"
"github.com/juanfont/headscale/integration/dockertestutil"
"github.com/juanfont/headscale/integration/hsic"
"github.com/juanfont/headscale/integration/tsic"
"github.com/ory/dockertest/v3"
"github.com/samber/lo"
"github.com/stretchr/testify/require"
"tailscale.com/control/controlbase"
"tailscale.com/control/controlhttp/controlhttpcommon"
"tailscale.com/net/wsconn"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
"tailscale.com/util/rands"
)
// Tailscale's JS/WASM control client opens /ts2021 as a browser WebSocket — an
// HTTP GET upgrade — rather than the native client's HTTP POST upgrade. A router
// that registers /ts2021 for POST only rejects that GET with 405 before the
// Noise handshake starts, which breaks every WASM client (issue #3357).
//
// These two tests guard that path against real headscale:
//
// - TestTS2021WebSocketGET dials the WebSocket GET directly from the test
// process using the same coder/websocket + controlbase primitives the WASM
// client uses. It is fast and always on.
// - TestTS2021WASMClientUnderNode runs the *actual* tailscale.com js/wasm
// control dial (integration/wasmic/wasmclient, built for GOOS=js) inside a
// Node container, alongside normal Tailscale clients, and asserts it
// completes the Noise handshake with headscale over the WebSocket.
//
// The server cannot tell the two apart: both send GET /ts2021 with
// Sec-WebSocket-Protocol: tailscale-control-protocol. Before the fix both fail
// with 405; after it, both complete the handshake.
// TestTS2021WebSocketGET connects to /ts2021 over a WebSocket GET from the test
// process and completes the Noise handshake, exactly as a browser/WASM client
// would.
func TestTS2021WebSocketGET(t *testing.T) {
IntegrationSkip(t)
t.Parallel()
spec := ScenarioSpec{
NodesPerUser: 1,
Users: []string{"user1"},
}
scenario, err := NewScenario(spec)
require.NoErrorf(t, err, "failed to create scenario: %s", err)
defer scenario.ShutdownAssertNoPanics(t)
err = scenario.CreateHeadscaleEnv([]tsic.Option{}, hsic.WithTestName("ts2021ws"))
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
conn, err := dialTS2021WebSocket(t, headscale.GetEndpoint(), headscale.GetCert())
require.NoError(t, err,
"WebSocket GET to /ts2021 must reach NoiseUpgradeHandler, not be rejected by the router with 405")
require.NotNil(t, conn)
t.Cleanup(func() { _ = conn.Close() })
t.Logf("noise established over websocket, protocol version %d", conn.ProtocolVersion())
}
// TestTS2021WASMClientUnderNode runs the real tailscale.com js/wasm control dial
// inside a Node container against real headscale, next to normal Tailscale
// clients, and asserts the WASM client completes the /ts2021 handshake.
func TestTS2021WASMClientUnderNode(t *testing.T) {
IntegrationSkip(t)
t.Parallel()
spec := ScenarioSpec{
NodesPerUser: 2,
Users: []string{"user1"},
Networks: map[string]NetworkSpec{
"wasmnet": {Users: []string{"user1"}},
},
ExtraService: map[string][]extraServiceFunc{
"wasmnet": {wasmClientService},
},
// The wasm client image builds from this module; pair it with the
// head Tailscale clients so the whole environment is current.
Versions: []string{"head"},
}
scenario, err := NewScenario(spec)
require.NoErrorf(t, err, "failed to create scenario: %s", err)
defer scenario.ShutdownAssertNoPanics(t)
// The Tailscale JS/WASM client dials the control server as a WebSocket.
// client_js.go only honours a custom port for ws:// (plain HTTP); over
// wss:// it always targets :443, so it cannot reach a TLS control server on
// :8080. Run headscale without TLS, matching the http:// setup in the issue.
err = scenario.CreateHeadscaleEnv(
[]tsic.Option{},
hsic.WithTestName("ts2021wasm"),
hsic.WithoutTLS(),
)
requireNoErrHeadscaleEnv(t, err)
allClients, err := scenario.ListTailscaleClients()
requireNoErrListClients(t, err)
allIPs, err := scenario.ListTailscaleClientsIPs()
requireNoErrListClientIPs(t, err)
// Normal Tailscale clients come up and form a working tailnet alongside the
// WASM control client.
err = scenario.WaitForTailscaleSync()
requireNoErrSync(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
// Sanity-check the tailnet the WASM client is joining: the normal clients
// must be able to reach each other.
allAddrs := lo.Map(allIPs, func(x netip.Addr, _ int) string { return x.String() })
assertPingAll(t, allClients, allAddrs)
services, err := scenario.Services("wasmnet")
require.NoError(t, err)
require.Len(t, services, 1, "expected the wasm client container")
wasm := services[0]
controlURL := headscale.GetEndpoint()
// Fetch the server's Noise key here and pass it to the WASM client: Go's
// net/http DNS resolver is unavailable under GOOS=js, so the client can only
// use the JS WebSocket transport, not an HTTP GET to /key.
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
defer cancel()
controlKey, err := fetchServerNoiseKey(ctx, &http.Client{Timeout: 15 * time.Second}, controlURL)
require.NoError(t, err)
controlKeyText, err := controlKey.MarshalText()
require.NoError(t, err)
// Run the real js/wasm control client under Node: it dials /ts2021 as a
// WebSocket GET; success means the Noise handshake completed.
stdout, stderr, err := dockertestutil.ExecuteCommand(
wasm,
[]string{"node", "/app/wasm_exec_node.js", "/app/client.wasm", controlURL, string(controlKeyText)},
[]string{},
dockertestutil.ExecuteCommandTimeout(60*time.Second),
)
t.Logf("wasm client stdout:\n%s", stdout)
t.Logf("wasm client stderr:\n%s", stderr)
require.NoError(t, err,
"wasm control client must connect to /ts2021 over websocket (405 means the router rejected the GET)")
require.Contains(t, stdout, "WASM_TS2021_OK",
"wasm control client should report a completed Noise handshake")
}
// wasmClientService builds and starts the Node + js/wasm control-client
// container (Dockerfile.wasmclient) on the given network so it can reach
// headscale by hostname. It idles; the test execs the client on demand.
func wasmClientService(s *Scenario, networkName string) (*dockertest.Resource, error) {
hash := rands.HexString(hsicOIDCMockHashLength)
hostname := "hs-wasmclient-" + hash
network, ok := s.networks[s.prefixedNetworkName(networkName)]
if !ok {
return nil, fmt.Errorf("network does not exist: %s", networkName) //nolint:err113
}
runOpts := &dockertest.RunOptions{
Name: hostname,
Networks: []*dockertest.Network{network},
Env: []string{},
}
dockertestutil.DockerAddIntegrationLabels(runOpts, "wasmclient")
buildOpts := &dockertest.BuildOptions{
Dockerfile: "Dockerfile.wasmclient",
ContextDir: dockerContextPath,
}
resource, err := s.pool.BuildAndRunWithBuildOptions(
buildOpts,
runOpts,
dockertestutil.DockerRestartPolicy,
)
if err != nil {
return nil, fmt.Errorf("building wasm client container: %w", err)
}
return resource, nil
}
// dialTS2021WebSocket opens /ts2021 as a WebSocket GET (subprotocol
// tailscale-control-protocol) and completes the Noise handshake, mirroring what
// tailscale.com/control/controlhttp/client_js.go does in a browser. It returns
// the established Noise connection, or an error (a router that only allows POST
// returns 405 here).
func dialTS2021WebSocket(t *testing.T, endpoint string, caCert []byte) (*controlbase.Conn, error) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
t.Cleanup(cancel)
u, err := url.Parse(endpoint)
require.NoError(t, err)
httpClient := &http.Client{Timeout: 15 * time.Second}
if u.Scheme == "https" {
pool := x509.NewCertPool()
pool.AppendCertsFromPEM(caCert)
httpClient.Transport = &http.Transport{
TLSClientConfig: &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12},
}
}
controlKey, err := fetchServerNoiseKey(ctx, httpClient, endpoint)
require.NoError(t, err)
init, cont, err := controlbase.ClientDeferred(
key.NewMachine(),
controlKey,
uint16(tailcfg.CurrentCapabilityVersion),
)
require.NoError(t, err)
wsScheme := "ws"
if u.Scheme == "https" {
wsScheme = "wss"
}
wsURL := &url.URL{
Scheme: wsScheme,
Host: u.Host,
Path: "/ts2021",
RawQuery: url.Values{
controlhttpcommon.HandshakeHeaderName: []string{base64.StdEncoding.EncodeToString(init)},
}.Encode(),
}
wsConn, resp, err := websocket.Dial(ctx, wsURL.String(), &websocket.DialOptions{
Subprotocols: []string{controlhttpcommon.UpgradeHeaderValue},
HTTPClient: httpClient,
})
if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
}
if err != nil {
return nil, err
}
netConn := wsconn.NetConn(ctx, wsConn, websocket.MessageBinary, wsURL.String())
cbConn, err := cont(ctx, netConn)
if err != nil {
_ = netConn.Close()
return nil, fmt.Errorf("noise handshake over websocket: %w", err)
}
return cbConn, nil
}
// fetchServerNoiseKey retrieves headscale's Noise public key from /key, the same
// endpoint a real client consults before dialing /ts2021.
func fetchServerNoiseKey(
ctx context.Context,
client *http.Client,
endpoint string,
) (key.MachinePublic, error) {
var zero key.MachinePublic
keyURL := fmt.Sprintf("%s/key?v=%d", endpoint, tailcfg.CurrentCapabilityVersion)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, keyURL, nil)
if err != nil {
return zero, err
}
resp, err := client.Do(req)
if err != nil {
return zero, err
}
defer resp.Body.Close()
var k tailcfg.OverTLSPublicKeyResponse
err = json.NewDecoder(resp.Body).Decode(&k)
if err != nil {
return zero, fmt.Errorf("decoding /key response: %w", err)
}
if k.PublicKey.IsZero() {
return zero, errors.New("server returned zero Noise public key") //nolint:err113
}
return k.PublicKey, nil
}
-103
View File
@@ -1,103 +0,0 @@
//go:build js
// Command wasmclient is a minimal Tailscale control client compiled to
// GOOS=js/GOARCH=wasm and run under Node. It exercises the real
// tailscale.com/control/controlhttp js/wasm dial path
// (control/controlhttp/client_js.go), which opens /ts2021 as a browser-style
// WebSocket GET — the exact transport a Tailscale JS/WASM client uses.
//
// It is the container-side half of the integration test guarding issue #3357:
// headscale must register /ts2021 for GET, not POST only, or the WebSocket
// upgrade is rejected with 405 before the Noise handshake can start.
//
// It is intentionally not the full tsconnect IPN — the regression is entirely
// in the control-connection upgrade, and this drives the real upgrade code with
// the smallest possible harness. On success it prints wasmSuccessMarker and
// exits 0; on any failure it prints wasmFailureMarker and exits non-zero.
package main
import (
"context"
"fmt"
"net/url"
"os"
"time"
"tailscale.com/control/controlhttp"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
)
// These markers are matched by the integration test on the client's stdout.
const (
wasmSuccessMarker = "WASM_TS2021_OK"
wasmFailureMarker = "WASM_TS2021_FAIL"
)
func main() {
if len(os.Args) < 3 {
fmt.Printf("%s: usage: wasmclient <control-url> <noise-key>\n", wasmFailureMarker)
os.Exit(2)
}
if err := run(os.Args[1], os.Args[2]); err != nil {
fmt.Printf("%s: %v\n", wasmFailureMarker, err)
os.Exit(1)
}
}
// run dials /ts2021 exactly as tailscale.com/control/controlhttp/client_js.go
// does in a browser: a WebSocket GET via the JS/undici WebSocket. The server's
// Noise key is passed in (the test fetches /key) rather than fetched here,
// because Go's net/http DNS resolver is unavailable under GOOS=js — only the
// WebSocket transport, which runs through the JS host, works.
func run(controlURL, noiseKeyText string) error {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
u, err := url.Parse(controlURL)
if err != nil {
return fmt.Errorf("parse control url %q: %w", controlURL, err)
}
var controlKey key.MachinePublic
if err := controlKey.UnmarshalText([]byte(noiseKeyText)); err != nil {
return fmt.Errorf("parse noise key %q: %w", noiseKeyText, err)
}
port := u.Port()
if port == "" {
if u.Scheme == "https" {
port = "443"
} else {
port = "80"
}
}
// client_js.go selects ws:// (and appends the port) only when HTTPPort is a
// custom non-80 port and HTTPS is 443 or disabled; otherwise it dials wss://
// on the default port. Set the fields to match the server's actual scheme.
d := &controlhttp.Dialer{
Hostname: u.Hostname(),
MachineKey: key.NewMachine(),
ControlKey: controlKey,
ProtocolVersion: uint16(tailcfg.CurrentCapabilityVersion),
}
if u.Scheme == "https" {
d.HTTPSPort = port
} else {
d.HTTPPort = port
d.HTTPSPort = controlhttp.NoPort
}
conn, err := d.Dial(ctx)
if err != nil {
return fmt.Errorf("ts2021 websocket dial: %w", err)
}
defer conn.Close()
fmt.Printf("%s: noise established over websocket, protocol version %d\n",
wasmSuccessMarker, conn.ProtocolVersion())
return nil
}
-8
View File
@@ -1,8 +0,0 @@
//go:build !js
// This package only does something when built for GOOS=js (see main.go). The
// stub exists so `go build ./...` and `go vet ./...` on the host don't fail with
// "build constraints exclude all Go files" for this directory.
package main
func main() {}
+2 -4
View File
@@ -68,9 +68,7 @@ exclude_docs: |
# Plugins
plugins:
- search:
provider: lunr
lunr:
separator: '[\s\-,:!=\[\]()"`/]+|\.(?!\d)|&[lg]t;|(?!\b)(?=[A-Z][a-z])'
separator: '[\s\-,:!=\[\]()"`/]+|\.(?!\d)|&[lg]t;|(?!\b)(?=[A-Z][a-z])'
- macros:
- include-markdown:
- minify:
@@ -113,7 +111,7 @@ extra:
- icon: fontawesome/brands/discord
link: https://discord.gg/c84AZQhmpx
headscale:
version: 0.29.3
version: 0.28.0
# Extensions
markdown_extensions: