Compare commits

...

26 Commits

Author SHA1 Message Date
Kristoffer Dalby 5aff68b5b9 mkdocs: bump version
Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
(cherry picked from commit 565fd254d0)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby 235a57ec31 CHANGELOG: add 0.29.3
Updates #3385

(cherry picked from commit 8bb26c5967)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby bdc3e996de hscontrol: prefer completed auth over expired ctx in followup wait
waitForFollowup selected on ctx.Done() and the verdict channel with equal
priority; when both were ready, select picked at random and discarded a
successful registration as a spurious 401 timeout. Check for a completed
verdict first, race the deadline only if none is ready.

Fixes #3385

(cherry picked from commit d28a6b111a)
2026-07-29 14:35:31 +02:00
Igor Serganov 5fb514e6e0 poll: do not cancel ephemeral GC until Connect succeeds
With node.ephemeral.inactivity_timeout set, ephemeral nodes are
usually deleted after they go offline, but under reconnect churn some
departed nodes stayed in the node list as disconnected indefinitely
until removed manually or until Headscale restarted.

Ephemeral cleanup is timer-based via EphemeralGarbageCollector, not a
periodic LastSeen scan. serveLongPoll cancelled any pending GC timer
at the very start of a long-poll attempt and only rescheduled on a
clean disconnect after Connect. If a reconnect cancelled the timer and
then failed before Connect (for example an UpdateNodeFromMapRequest
error), the deferred cleanup saw connectGen == 0 and returned without
Schedule. The node remained offline with no deletion timer and no
reconciler to recover it.

Cancel the ephemeral GC timer only after a successful Connect, so a
failed reconnect leaves an already-armed inactivity timer intact.
Successful reconnects still cancel GC once the node is online, and a
later disconnect reschedules as before.

Add TestFailedReconnectDoesNotCancelEphemeralGC to lock in the
ordering, plus IsScheduled and DeleteNodeFromStoreForTest helpers for
the test.

Fixes #3382

Co-authored-by: Cursor <cursoragent@cursor.com>
(cherry picked from commit cfd845cb53)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby 4a1e77359d policy,state: authorize reauth tags against the authenticating user
Re-authenticating a tagged node with --advertise-tags checked the tag-owned
node, not the authenticating user, so every tag was rejected.

Fixes #3374

(cherry picked from commit 6275e3a356)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby 1fccdb18bd state: apply a new pre-auth key's tags on re-registration
Re-registering a tagged node with a different key discarded the new key's
tags and left a stale auth-key reference; retag on key change and persist it.

Fixes #3370

(cherry picked from commit fc16cc6905)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby d202883200 state: do not expire tagged nodes on logout
Tagged nodes never expire, but handleLogout stamped a past expiry on them,
leaving them stuck expired and unable to re-authenticate.

Fixes #3371

(cherry picked from commit 1ed5693fa4)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby 9609a0b87d hscontrol: gate /key on supported capability version
/key handed out the Noise public key for any v>=39, a floor unrelated
to the handshake's capver.MinSupportedCapabilityVersion. Reject below
the supported floor, matching /ts2021, and drop the stale constant.

Fixes #3380

(cherry picked from commit 5b6e1e17be)
2026-07-29 14:35:31 +02:00
Arpit Jain fba84ca232 auth: check machine key on the followup registration path
waitForFollowup returns nodeToRegisterResponse for a completed
registration without checking that the Noise session polling for the
result was started with the machine key that opened the registration.
That response carries the registering user's User and Login, so the auth
ID in the followup URL is the only thing protecting it.

handleRegister and handleLogout both call machineKeyMismatch before
handing back a node, so this is the one path of the three that does not.
The key is already available: HandleNodeFromAuthPath resolves the node
from the MachineKey cached in RegistrationData, so on the normal path the
node and the session agree and the check is a no-op.

The auth ID is 96 bits of randomness and is not guessable, so this is not
reachable by brute force. It is logged at info level when a registration
is created, which makes log access the realistic way to obtain one.

The existing followup_registration_success case built its node with
CreateNodeForTest, which picks a random machine key that no real
registration would produce. Set the registering machine key so the
fixture matches the production path.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
(cherry picked from commit 0ce3356b89)
2026-07-29 14:35:31 +02:00
Florian Preinstorfer 089d6c4109 Explicitly select lunr as search provider
Keep the existing search provider instead (and the separator) option.

(cherry picked from commit 7a1ee34f71)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby 12928418b8 build: bump Go toolchain to 1.26.5
Tailscale HEAD go.mod now requires go >= 1.26.5; build images pinned
1.26.4. Bump go.mod, the four Go Dockerfiles, and the nixpkgs pin
(staging-next-26.05 ships go_1_26 1.26.5; unstable still lags).

(cherry picked from commit 8eec2dbdc2)
2026-07-29 14:35:31 +02:00
Kristoffer Dalby 8eea89488c CHANGELOG: drop unreleased 0.30.0 stub
Release-branch only; the stub tracks unreleased main work.
2026-07-01 14:47:39 +00:00
Florian Preinstorfer f885d87827 Fix invalid ip syntax
The "ip" field needs to be a list.

(cherry picked from commit a066a06bef)
2026-07-01 14:33:43 +00:00
Kristoffer Dalby 3ac33cf1d5 CHANGELOG: shorten 0.29.2 invalid-name entry, set date
Updates #3346

(cherry picked from commit a84945f134)
2026-07-01 14:33:43 +00:00
Kristoffer Dalby f708c5b010 CHANGELOG: note /ts2021 WebSocket GET fix
Updates #3357

(cherry picked from commit fef80f3eb0)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby 8f4e69d2a6 integration: add TS2021 WebSocket tests to CI matrix
Generated by gh-action-integration-generator.

Updates #3357

(cherry picked from commit 01ef350d5f)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby e7851ef881 integration: test /ts2021 WebSocket GET with a real WASM client
A raw coder/websocket dial and the real tailscale.com js/wasm control client under Node, both against headscale alongside normal Tailscale clients.

Updates #3357

(cherry picked from commit f4fba32dc6)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby 1ec7b7fb72 hscontrol: register /ts2021 for WebSocket GET
The chi migration dropped GET; JS/WASM control clients open /ts2021 as a WebSocket GET and were rejected with 405 before reaching NoiseUpgradeHandler.

Fixes #3357

(cherry picked from commit fc6f216b61)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby 735742e3ee CHANGELOG: note 0.29.2 invalid-name map fix
Updates #3346

(cherry picked from commit de9db9c811)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby fd154fdb66 state: log nodes with map-breaking data at startup
Scan a node-health check registry at boot and log each node whose name
can't form a valid FQDN, with the rename fix. Log-only, no mutation.

Updates #3346

(cherry picked from commit 4946d1c88d)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby 9c9206a92b state: reject renames whose FQDN exceeds the hostname limit
A valid label can still overflow 255 chars under a long base_domain; gate
RenameNode with the new types.ValidateGivenName.

Updates #3346

(cherry picked from commit c497612c99)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby 5fb76eb231 poll: return an HTTP error on long-poll setup failure
A bare return sent an empty 200 the client read as "unexpected EOF" and
retried forever; emit a real error instead.

Updates #3346

(cherry picked from commit 4e4512c4b7)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby ec67197368 mapper: skip peers with invalid names instead of failing the map
A peer whose GivenName fails GetFQDN aborted the whole map for every node
that could see it. Drop and log it; SSH policy errors degrade too.

Fixes #3346

(cherry picked from commit 08956d51a4)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby 9d1327458f mapper,policy: add reconnect-storm and lock-concurrency regression tests
TestInitialMapNotStarvedByReconnectStorm reproduces the #3346 stall;
TestPolicyManagerConcurrentReads guards the RLock cache access under -race.

Updates #3346

(cherry picked from commit d528686f14)
2026-07-01 14:33:03 +00:00
Kristoffer Dalby d4f2acf3ab policy: take RLock for reads so map generation runs concurrently
One exclusive mutex serialized every policy read, so a mass reconnect on
autogroup:self/via/relay policies stalled clients into "unexpected EOF"
retries. Per-node caches become xsync.Maps for lazy population under RLock.

Fixes #3346

(cherry picked from commit 6f317c7576)
2026-07-01 14:32:49 +00:00
Kristoffer Dalby 636f660caf db: preserve user_id on untagged nodes with tags='null'
A nil tags slice marshals to JSON `null`; the clear-tagged migration
read that as tagged and cleared user_id. Exclude it, and recover
already-detached nodes from their pre-auth key.

Fixes #3323
2026-06-18 10:22:27 +00:00
47 changed files with 4708 additions and 163 deletions
+8
View File
@@ -340,6 +340,8 @@ jobs:
- TestTagsAuthKeyWithTagCannotAddViaCLI
- TestTagsAuthKeyWithTagCannotChangeViaCLI
- TestTagsAuthKeyWithTagAdminOverrideReauthPreserves
- TestTagsReauthDifferentKeyRetagsNode
- TestTagsReauthDifferentKeyRemovesTag
- TestTagsAuthKeyWithTagCLICannotModifyAdminTags
- TestTagsAuthKeyWithoutTagCannotRequestTags
- TestTagsAuthKeyWithoutTagRegisterNoTags
@@ -367,6 +369,12 @@ jobs:
- TestTagsAuthKeyWithoutUserInheritsTags
- TestTagsAuthKeyWithoutUserRejectsAdvertisedTags
- TestTagsAuthKeyConvertToUserViaCLIRegister
- TestTaggedNodeLogoutReloginSingleUseKeyOnline
- TestTaggedNodeLogoutReloginReusableKeyOnline
- TestTagsOIDCReauthAddOwnedTag
- TestTagsReauthEmptyTagsReturnsToUserSurvives
- TestTS2021WebSocketGET
- TestTS2021WASMClientUnderNode
- TestTailscaleRustAxum
uses: ./.github/workflows/integration-test-template.yml
secrets: inherit
+30 -2
View File
@@ -1,8 +1,36 @@
# CHANGELOG
## 0.30.0 (202x-xx-xx)
## 0.29.3 (2026-07-29)
**Minimum supported Tailscale client version: v1.xx.0**
**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)
+1 -1
View File
@@ -1,6 +1,6 @@
# For testing purposes only
FROM golang:1.26.4-alpine AS build-env
FROM golang:1.26.5-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.4-trixie AS builder
FROM docker.io/golang:1.26.5-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.4-alpine AS build-env
FROM golang:1.26.5-alpine AS build-env
WORKDIR /go/src
+30
View File
@@ -0,0 +1,30 @@
# 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
+25 -3
View File
@@ -1,5 +1,27 @@
{
"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"
@@ -20,11 +42,11 @@
},
"nixpkgs": {
"locked": {
"lastModified": 1781153106,
"narHash": "sha256-yzsroLCcuRG4KdGMxWt0eXKOrRSgQT8/xjYngeq9ujU=",
"lastModified": 1784421286,
"narHash": "sha256-YKn3t7FvaargtyNHbrOw+qzDM+93q5kSCREUPNxeZQs=",
"owner": "NixOS",
"repo": "nixpkgs",
"rev": "9ee75f111a06d7ab2b2f729698a8eff53d54e070",
"rev": "62eaae30cd05c04718500ddb44e7ef85cd45ddd5",
"type": "github"
},
"original": {
+8 -7
View File
@@ -2,11 +2,12 @@
description = "headscale - Open Source Tailscale Control server";
inputs = {
# 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.
# 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.
nixpkgs.url = "github:NixOS/nixpkgs/staging-next-26.05";
flake-utils.url = "github:numtide/flake-utils";
};
@@ -31,7 +32,7 @@
overlays.default = _: prev:
let
pkgs = nixpkgs.legacyPackages.${prev.stdenv.hostPlatform.system};
# Go 1.26 builder; resolves to Go 1.26.4 from the pinned nixpkgs.
# Go 1.26 builder; resolves to Go 1.26.5 from the pinned nixpkgs.
buildGo = pkgs.buildGo126Module;
vendorHash = (builtins.fromJSON (builtins.readFile ./flakehashes.json)).vendor.sri;
in
@@ -101,7 +102,7 @@
};
# Build golangci-lint with stock Go 1.26 (upstream uses hardcoded Go
# version); it does not build against the pinned 1.26.4.
# version); it does not build against the pinned 1.26.5.
golangci-lint = buildGo rec {
pname = "golangci-lint";
version = "2.12.2";
+1 -1
View File
@@ -1,6 +1,6 @@
{
"vendor": {
"goModSum": "sha256-csVm5v6HZ49PBp/FCX+yK1sjV8/nuUQz3GKN21Ne1mg=",
"goModSum": "sha256-GFiMC2ktrQnpSVzJHabVP1TYwJvn5+a/BDLhvmGTUWc=",
"sri": "sha256-fzKyXNMw/2yAEhaTZu0n1NXatPO2IP0HFA2ey1vZIYM="
}
}
+1 -1
View File
@@ -1,6 +1,6 @@
module github.com/juanfont/headscale
go 1.26.4
go 1.26.5
require (
github.com/arl/statsviz v0.8.0
+5
View File
@@ -544,6 +544,11 @@ 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)
+58 -10
View File
@@ -23,6 +23,18 @@ 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,
@@ -223,6 +235,20 @@ func (h *Headscale) handleLogout(
Msg("Node is not ephemeral, setting expiry instead of deleting")
}
// Tagged nodes have key expiry permanently disabled (they are owned by
// their tags, not a user, and never expire - KB 1068). Logging one out has
// no expiry semantics, so do not stamp an expiry on it: doing so leaves the
// node IsExpired() forever and it can never re-authenticate (#3371). The
// admin path `headscale nodes expire` remains free to set a deliberate
// expiry via SetNodeExpiry; only the logout path is guarded here.
if node.IsTagged() {
log.Debug().
EmbedObject(node).
Msg("Tagged node logout: not stamping expiry (tagged nodes never expire)")
return nodeToRegisterResponse(node), nil
}
// Update the internal state with the nodes new expiry, meaning it is
// logged out.
//
@@ -291,19 +317,41 @@ func (h *Headscale) waitForFollowup(
}
if reg, ok := h.state.GetAuthCacheEntry(followupReg); ok {
var verdict types.AuthVerdict
select {
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)
}
return nodeToRegisterResponse(verdict.Node), nil
// 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():
}
}
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)
}
// 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
}
}
// if the follow-up registration isn't found anymore, instruct the client to try a new registration
+389
View File
@@ -1338,3 +1338,392 @@ func TestReregistrationZeroExpiryStaysNil(t *testing.T) {
assert.False(t, node2.Expiry().Valid(),
"re-registration with zero client expiry and no default should leave expiry nil, not pointer to zero time")
}
// tsLogoutSentinelExpiry is the past expiry a real tailscale client sends on
// `tailscale logout`: time.Unix(123, 0) (controlclient/direct.go). The issue
// #3371 trace shows it verbatim as `expiry=123`. Using it here (rather than a
// generic time.Now().Add(-1h)) keeps the reproduction faithful to the wire
// behaviour: handleRegister must classify this as a logout, and handleLogout
// must clamp it to now.
func tsLogoutSentinelExpiry() time.Time {
return time.Unix(123, 0)
}
// TestIssue3371_TaggedNodeLogoutReloginSingleUseKey reproduces
// https://github.com/juanfont/headscale/issues/3371 through the real
// register/logout HTTP-handler path (handleRegister -> handleLogout ->
// handleRegister), not by poking SetNodeExpiry directly.
//
// Root cause (a): `tailscale logout` sends a past expiry; handleLogout stamps
// it on the node via SetNodeExpiry with no IsTagged guard, so a tagged node —
// which must have key-expiry disabled — becomes Expired.
//
// Root cause (b): on the next `tailscale up --auth-key <fresh key>`,
// HandleNodeFromPreAuthKey sees an expired node, takes the expired-node
// validation path, consumes the fresh single-use key on the in-place
// re-registration, yet leaves the node expired (the expiry-refresh block is
// gated `!node.IsTagged()`). The response carries NodeKeyExpired=true, so the
// client rotates its node key and retries with the now-spent key, which is
// rejected with "authkey already used" forever.
//
// Faithful to the artifacts: the client rotates its NodeKey on relogin, the
// logout carries BOTH a past expiry AND an auth key, and a BRAND NEW key is
// presented for the relogin (the trace shows tag:tag2 keys burned while the
// node kept tag:tag1).
func TestIssue3371_TaggedNodeLogoutReloginSingleUseKey(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("tag-logout-user")
tags := []string{"tag:tag1"}
// `headscale preauthkeys create --tags tag:tag1` (single-use).
pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, tags)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
// `tailscale up --auth-key $KEY1`: initial join.
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "headscale-debug"},
}
resp, err := app.handleRegister(context.Background(), regReq, machineKey.Public())
require.NoError(t, err)
require.True(t, resp.MachineAuthorized)
require.False(t, resp.NodeKeyExpired)
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
require.True(t, node.IsTagged(), "precondition: node is tagged")
require.False(t, node.Expiry().Valid(), "precondition: tagged node has expiry disabled")
// `tailscale logout`: client sends a past-expiry register with the auth key
// still attached (handleRegister must treat past expiry as logout regardless
// of Auth). Reuse the same node key: logout does not rotate it.
logoutReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Expiry: tsLogoutSentinelExpiry(),
}
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
require.NoError(t, err)
// A tagged node must NOT be expired by logout — tagged nodes never expire.
// This is root cause (a); it fails before the fix.
nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
assert.False(t, nodeAfterLogout.IsExpired(),
"issue #3371 root cause (a): logout must not expire a tagged node")
assert.False(t, nodeAfterLogout.Expiry().Valid(),
"issue #3371 root cause (a): tagged node must keep key-expiry disabled after logout")
// `tailscale up --auth-key $KEY2`: a BRAND NEW single-use key, and the client
// rotates its node key (as the real client does on relogin).
pak2, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, tags)
require.NoError(t, err)
nodeKey2 := key.NewNode()
reloginReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak2.Key},
NodeKey: nodeKey2.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "headscale-debug"},
}
reloginResp, err := app.handleRegister(context.Background(), reloginReq, machineKey.Public())
require.NoError(t, err,
"issue #3371: a fresh valid key must re-authenticate the tagged node after logout")
require.NotNil(t, reloginResp)
// The whole point: the node comes back online, not stuck expired.
assert.False(t, reloginResp.NodeKeyExpired,
"issue #3371: relogin response must not report the node key as expired")
assert.True(t, reloginResp.MachineAuthorized)
relogged, found := app.state.GetNodeByNodeKey(nodeKey2.Public())
require.True(t, found)
assert.True(t, relogged.IsTagged(), "node stays tagged after relogin")
assert.False(t, relogged.IsExpired(),
"issue #3371: tagged node must be online (not expired) after relogin")
assert.False(t, relogged.Expiry().Valid(),
"issue #3371: tagged node must have key-expiry disabled after relogin")
assert.Equal(t, node.ID(), relogged.ID(), "must re-use the same node, not duplicate")
assert.Equal(t, 1, app.state.ListNodes().Len(), "machine maps to exactly one node")
}
// TestIssue3371_TaggedNodeLogoutReloginReusableKey is the reusable-key variant
// from the issue ("tailscale up then hangs indefinitely instead of erroring").
// With a reusable key the relogin does not hit "authkey already used", but the
// node still stays expired without the fix — so the client never observes a
// non-expired node and hangs. The observable failure here is the persisted
// expired state after relogin.
func TestIssue3371_TaggedNodeLogoutReloginReusableKey(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("tag-logout-reusable")
tags := []string{"tag:tag1"}
// `headscale preauthkeys create --reusable --tags tag:tag1`.
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-tagged"},
}
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
require.NoError(t, err)
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
require.True(t, node.IsTagged())
require.False(t, node.Expiry().Valid())
// Logout.
logoutReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Expiry: tsLogoutSentinelExpiry(),
}
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
require.NoError(t, err)
// Relogin with the same reusable key, rotating the node key.
nodeKey2 := key.NewNode()
reloginReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey2.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-tagged"},
}
reloginResp, err := app.handleRegister(context.Background(), reloginReq, machineKey.Public())
require.NoError(t, err)
require.NotNil(t, reloginResp)
assert.False(t, reloginResp.NodeKeyExpired,
"issue #3371: reusable-key relogin must not report node key expired")
relogged, found := app.state.GetNodeByNodeKey(nodeKey2.Public())
require.True(t, found)
assert.True(t, relogged.IsTagged())
assert.False(t, relogged.IsExpired(),
"issue #3371: tagged node must be online (not expired) after reusable-key relogin")
assert.False(t, relogged.Expiry().Valid(),
"issue #3371: tagged node must have key-expiry disabled after reusable-key relogin")
}
// TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry pins the deepest root cause
// (a) in isolation: `tailscale logout` on a tagged node must not stamp an
// expiry at all. This is the assertion PR #3372 does not make — it leaves
// handleLogout expiring tagged nodes and only unwinds the damage on the next
// registration. Keeping this separate from the relogin tests means a
// regression that re-introduces logout-sets-expiry is caught even if the
// re-registration cleanup masks it.
func TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("tag-logout-noexpiry")
tags := []string{"tag:tag1"}
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "noexpiry-tagged"},
}
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
require.NoError(t, err)
logoutReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Expiry: tsLogoutSentinelExpiry(),
}
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
require.NoError(t, err)
nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
assert.True(t, nodeAfterLogout.IsTagged(), "node stays tagged through logout")
assert.False(t, nodeAfterLogout.Expiry().Valid(),
"issue #3371 root cause (a): logout must not set an expiry on a tagged node")
assert.False(t, nodeAfterLogout.IsExpired(),
"issue #3371 root cause (a): tagged node must not be expired by logout")
// The database column must be NULL, not a clamped 'now' timestamp — a
// persisted expiry survives restart and re-triggers the lockout.
var dbNode types.Node
require.NoError(t,
app.state.DB().DB.First(&dbNode, nodeAfterLogout.ID().Uint64()).Error)
assert.Nil(t, dbNode.Expiry,
"issue #3371 root cause (a): tagged node's DB expiry must remain NULL after logout")
}
// TestIssue3371_UserOwnedNodeLogoutStillExpires is the guard rail: the fix for
// tagged nodes must not change logout for ordinary user-owned nodes. A
// user-owned node that logs out MUST still be expired (that is what logout
// means for it).
func TestIssue3371_UserOwnedNodeLogoutStillExpires(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("user-logout")
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "user-node"},
Expiry: time.Now().Add(24 * time.Hour),
}
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
require.NoError(t, err)
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
require.False(t, node.IsTagged(), "precondition: user-owned node")
logoutReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Expiry: tsLogoutSentinelExpiry(),
}
logoutResp, err := app.handleRegister(context.Background(), logoutReq, machineKey.Public())
require.NoError(t, err)
require.NotNil(t, logoutResp)
nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
assert.True(t, nodeAfterLogout.IsExpired(),
"user-owned node must still be expired by logout (fix must not regress this)")
assert.True(t, logoutResp.NodeKeyExpired,
"logout response for a user-owned node must report the key expired")
}
// TestIssue3371_TaggedNodeFutureExpirySurvivesRelogin is the discriminator
// guard rail for the fix. A tagged node may carry a DELIBERATE future expiry
// set by an admin (`headscale nodes expire`); TestTaggedNodeCanHaveKeyExpiry
// establishes that is legal. The #3371 fix clears only a STALE PAST expiry (the
// logout stamp) on re-registration — it must NOT wipe a future expiry. This
// test locks that boundary: without care, a "tagged => clear expiry" fix would
// silently destroy the admin's setting.
//
// Passes before the fix (re-registration currently never touches a tagged
// node's expiry) and must keep passing after.
func TestIssue3371_TaggedNodeFutureExpirySurvivesRelogin(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("tag-future-expiry")
tags := []string{"tag:tag1"}
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "future-expiry-tagged"},
}
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
require.NoError(t, err)
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
require.True(t, node.IsTagged())
// Admin sets a deliberate future expiry (`headscale nodes expire`).
future := time.Now().Add(24 * time.Hour)
_, _, err = app.state.SetNodeExpiry(node.ID(), &future)
require.NoError(t, err)
withFuture, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
require.True(t, withFuture.Expiry().Valid(), "precondition: future expiry set")
require.False(t, withFuture.IsExpired(), "precondition: future expiry is not expired")
// Node re-registers (rotating its node key). The future expiry must survive.
nodeKey2 := key.NewNode()
reregReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey2.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "future-expiry-tagged"},
}
_, err = app.handleRegister(context.Background(), reregReq, machineKey.Public())
require.NoError(t, err)
after, found := app.state.GetNodeByNodeKey(nodeKey2.Public())
require.True(t, found)
require.True(t, after.IsTagged(), "node stays tagged")
assert.True(t, after.Expiry().Valid(),
"deliberate future expiry must survive re-registration (not cleared by #3371 fix)")
assert.WithinDuration(t, future, after.Expiry().Get(), 5*time.Second,
"the surviving expiry must be the admin-set future value, unchanged")
}
// TestIssue3371_EphemeralTaggedNodeLogoutDeletes is a regression guard for the
// ephemeral+tagged combination. A tagged pre-auth key can also be ephemeral.
// handleLogout deletes ephemeral nodes (before any expiry stamp), so the #3371
// fix (which suppresses the expiry stamp for tagged nodes) must not divert an
// ephemeral tagged node away from deletion.
//
// Passes before the fix and must keep passing after.
func TestIssue3371_EphemeralTaggedNodeLogoutDeletes(t *testing.T) {
app := createTestApp(t)
user := app.state.CreateUserForTest("tag-ephemeral")
tags := []string{"tag:tag1"}
// Ephemeral + tagged key.
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, true, nil, tags)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "ephemeral-tagged"},
}
_, err = app.handleRegister(context.Background(), regReq, machineKey.Public())
require.NoError(t, err)
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
require.True(t, found)
require.True(t, node.IsTagged(), "precondition: node is tagged")
require.True(t, node.IsEphemeral(), "precondition: node is ephemeral")
// Logout: an ephemeral node is deleted, not expired.
logoutReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nodeKey.Public(),
Expiry: tsLogoutSentinelExpiry(),
}
_, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public())
require.NoError(t, err)
require.EventuallyWithT(t, func(c *assert.CollectT) {
_, stillThere := app.state.GetNodeByNodeKey(nodeKey.Public())
assert.False(c, stillThere,
"ephemeral tagged node must be deleted on logout, not expired")
}, 2*time.Second, 50*time.Millisecond, "waiting for ephemeral node deletion")
}
+153
View File
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
"strings"
"testing"
@@ -692,6 +693,11 @@ 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()})
}()
@@ -4101,3 +4107,150 @@ 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")
}
+65 -1
View File
@@ -706,13 +706,20 @@ 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 != '';
WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null';
`).Error
if err != nil {
return fmt.Errorf("clearing user_id on tagged nodes: %w", err)
@@ -744,6 +751,63 @@ 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,6 +198,161 @@ 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 {
+10
View File
@@ -496,6 +496,16 @@ 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 {
@@ -0,0 +1,85 @@
-- Test SQL dump for the clear-tagged-node-expiry migration
-- (202607241200-clear-tagged-node-expiry)
--
-- A buggy handleLogout stamped a past key expiry on tagged nodes. Tagged
-- nodes are owned by their tags and never expire (KB 1068), so such a row is
-- reported as permanently Expired and can never re-authenticate. The migration
-- clears expiry on tagged rows while leaving user-owned nodes untouched.
-- Fixes: https://github.com/juanfont/headscale/issues/3371
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
-- Migrations table: entries applied up to (but not including) the fix. The
-- intervening expiry migrations (clear-zero-time) also run against this dump;
-- their predicates (expiry < 1900) do not match the post-2000 dates below, so
-- they leave these rows for the new migration to handle.
CREATE TABLE `migrations` (`id` text,PRIMARY KEY (`id`));
INSERT INTO migrations VALUES('202312101416');
INSERT INTO migrations VALUES('202312101430');
INSERT INTO migrations VALUES('202402151347');
INSERT INTO migrations VALUES('2024041121742');
INSERT INTO migrations VALUES('202406021630');
INSERT INTO migrations VALUES('202409271400');
INSERT INTO migrations VALUES('202407191627');
INSERT INTO migrations VALUES('202408181235');
INSERT INTO migrations VALUES('202501221827');
INSERT INTO migrations VALUES('202501311657');
INSERT INTO migrations VALUES('202502070949');
INSERT INTO migrations VALUES('202502131714');
INSERT INTO migrations VALUES('202502171819');
INSERT INTO migrations VALUES('202505091439');
INSERT INTO migrations VALUES('202505141324');
INSERT INTO migrations VALUES('202507021200');
INSERT INTO migrations VALUES('202510311551');
INSERT INTO migrations VALUES('202511101554-drop-old-idx');
INSERT INTO migrations VALUES('202511011637-preauthkey-bcrypt');
INSERT INTO migrations VALUES('202511122344-remove-newline-index');
INSERT INTO migrations VALUES('202511131445-node-forced-tags-to-tags');
INSERT INTO migrations VALUES('202601121700-migrate-hostinfo-request-tags');
INSERT INTO migrations VALUES('202602201200-clear-tagged-node-user-id');
-- Users table
CREATE TABLE `users` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`name` text,`display_name` text,`email` text,`provider_identifier` text,`provider` text,`profile_pic_url` text);
INSERT INTO users VALUES(1,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user1','User One','user1@example.com',NULL,NULL,NULL);
-- Pre-auth keys table
CREATE TABLE `pre_auth_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`user_id` integer,`reusable` numeric,`ephemeral` numeric DEFAULT false,`used` numeric DEFAULT false,`tags` text,`created_at` datetime,`expiration` datetime,`prefix` text,`hash` blob,CONSTRAINT `fk_pre_auth_keys_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE SET NULL);
-- API keys table
CREATE TABLE `api_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`prefix` text,`hash` blob,`created_at` datetime,`expiration` datetime,`last_seen` datetime);
-- Nodes table - current schema (after the tags rename + last_seen/expiry reordering)
CREATE TABLE IF NOT EXISTS "nodes" (`id` integer PRIMARY KEY AUTOINCREMENT,`machine_key` text,`node_key` text,`disco_key` text,`endpoints` text,`host_info` text,`ipv4` text,`ipv6` text,`hostname` text,`given_name` varchar(63),`user_id` integer,`register_method` text,`tags` text,`auth_key_id` integer,`last_seen` datetime,`expiry` datetime,`approved_routes` text,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,CONSTRAINT `fk_nodes_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_nodes_auth_key` FOREIGN KEY (`auth_key_id`) REFERENCES `pre_auth_keys`(`id`));
-- Node 1: TAGGED, user_id NULL, stale PAST expiry (the #3371 bug). After migration: expiry NULL.
INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00','2020-06-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
-- Node 2: TAGGED, user_id NULL, FUTURE expiry. Tagged nodes never expire, so cleared to NULL too.
INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00','2099-01-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
-- Node 3: TAGGED, user_id NULL, expiry already NULL. After migration: still NULL.
INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
-- Node 4: UNTAGGED user-owned (tags='null'), PAST expiry. Must be PRESERVED (guard against
-- the #3323-class over-match: a nil tags slice marshals to the literal 'null').
INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'cli','null',NULL,'2024-01-01 00:00:00+00:00','2020-06-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
-- Node 5: UNTAGGED user-owned (tags='[]'), FUTURE expiry. Must be PRESERVED.
INSERT INTO nodes VALUES(5,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e05','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605505','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57705','[]','{}','100.64.0.5','fd7a:115c:a1e0::5','node5','node5',1,'cli','[]',NULL,'2024-01-01 00:00:00+00:00','2099-01-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
-- Policies table (empty)
CREATE TABLE `policies` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`data` text);
DELETE FROM sqlite_sequence;
INSERT INTO sqlite_sequence VALUES('users',1);
INSERT INTO sqlite_sequence VALUES('nodes',5);
CREATE INDEX idx_users_deleted_at ON users(deleted_at);
CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix);
CREATE INDEX idx_policies_deleted_at ON policies(deleted_at);
CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE provider_identifier IS NOT NULL;
CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier);
CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL;
CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != '';
COMMIT;
@@ -0,0 +1,85 @@
-- 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;
@@ -0,0 +1,87 @@
-- 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;
+20 -24
View File
@@ -19,17 +19,6 @@ 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
)
@@ -202,21 +191,28 @@ func (h *Headscale) KeyHandler(
return
}
// 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")
}
// 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)))
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,12 +4,15 @@ 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")
@@ -47,6 +50,52 @@ 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) {
+24 -2
View File
@@ -9,6 +9,8 @@ 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"
@@ -144,7 +146,15 @@ func (b *MapResponseBuilder) WithSSHPolicy() *MapResponseBuilder {
sshPolicy, err := b.mapper.state.SSHPolicy(node)
if err != nil {
b.addError(err)
// 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")
return b
}
@@ -279,7 +289,19 @@ 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 {
return nil, err
// 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
}
// [tailcfg.Node.CapMap] on a peer carries the small set of
+189
View File
@@ -0,0 +1,189 @@
//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,6 +3,7 @@ package mapper
import (
"fmt"
"net/netip"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
@@ -536,6 +537,94 @@ 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,6 +459,71 @@ 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,6 +30,12 @@ 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
+104 -63
View File
@@ -15,6 +15,7 @@ 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"
@@ -28,7 +29,10 @@ import (
var ErrInvalidTagOwner = errors.New("tag owner is not an Alias")
type PolicyManager struct {
mu sync.Mutex
// 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
pol *Policy
users []types.User
nodes views.Slice[types.NodeView]
@@ -55,7 +59,7 @@ type PolicyManager struct {
viaTargetTags map[Tag]struct{}
// Lazy map of SSH policies
sshPolicyMap map[types.NodeID]*tailcfg.SSHPolicy
sshPolicyMap *xsync.Map[types.NodeID, *tailcfg.SSHPolicy]
// compiledGrants are the grants with sources pre-resolved.
// The single source of truth for filter compilation. Both
@@ -64,12 +68,12 @@ type PolicyManager struct {
userNodeIdx userNodeIndex
// Lazy map of per-node filter rules (reduced, for packet filters)
filterRulesMap map[types.NodeID][]tailcfg.FilterRule
filterRulesMap *xsync.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 map[types.NodeID][]matcher.Match
matchersForNodeMap *xsync.Map[types.NodeID, []matcher.Match]
// needsPerNodeFilter is true when any compiled grant requires
// per-node work (autogroup:self or via grants).
@@ -197,9 +201,9 @@ func NewPolicyManager(b []byte, users []types.User, nodes views.Slice[types.Node
pol: policy,
users: users,
nodes: nodes,
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()),
sshPolicyMap: xsync.NewMap[types.NodeID, *tailcfg.SSHPolicy](),
filterRulesMap: xsync.NewMap[types.NodeID, []tailcfg.FilterRule](),
matchersForNodeMap: xsync.NewMap[types.NodeID, []matcher.Match](),
}
_, err = pm.updateLocked()
@@ -354,9 +358,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.
clear(pm.sshPolicyMap)
clear(pm.filterRulesMap)
clear(pm.matchersForNodeMap)
pm.sshPolicyMap.Clear()
pm.filterRulesMap.Clear()
pm.matchersForNodeMap.Clear()
}
// If nothing changed, no need to update nodes
@@ -400,8 +404,8 @@ func (pm *PolicyManager) NodeNeedsPeerRecompute(node types.NodeView) bool {
return true
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
if pm.relayTargetIPs != nil && node.InIPSet(pm.relayTargetIPs) {
return true
@@ -422,10 +426,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.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
if sshPol, ok := pm.sshPolicyMap[node.ID()]; ok {
if sshPol, ok := pm.sshPolicyMap.Load(node.ID()); ok {
return sshPol, nil
}
@@ -434,7 +438,7 @@ func (pm *PolicyManager) SSHPolicy(baseURL string, node types.NodeView) (*tailcf
return nil, fmt.Errorf("compiling SSH policy: %w", err)
}
pm.sshPolicyMap[node.ID()] = sshPol
pm.sshPolicyMap.Store(node.ID(), sshPol)
return sshPol, nil
}
@@ -450,8 +454,8 @@ func (pm *PolicyManager) SSHPolicy(baseURL string, node types.NodeView) (*tailcf
func (pm *PolicyManager) SSHCheckParams(
srcNodeID, dstNodeID types.NodeID,
) (time.Duration, bool) {
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
if pm.pol == nil || len(pm.pol.SSHs) == 0 {
return 0, false
@@ -584,8 +588,8 @@ func (pm *PolicyManager) Filter() ([]tailcfg.FilterRule, []matcher.Match) {
return nil, nil
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
return pm.filter, pm.matchers
}
@@ -604,8 +608,8 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ
return nil
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
// 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.
@@ -726,7 +730,7 @@ func (pm *PolicyManager) filterForNodeLocked(
return nil
}
if rules, ok := pm.filterRulesMap[node.ID()]; ok {
if rules, ok := pm.filterRulesMap.Load(node.ID()); ok {
return rules
}
@@ -738,7 +742,7 @@ func (pm *PolicyManager) filterForNodeLocked(
}
reduced := policyutil.ReduceFilterRules(node, unreduced)
pm.filterRulesMap[node.ID()] = reduced
pm.filterRulesMap.Store(node.ID(), reduced)
return reduced
}
@@ -755,8 +759,8 @@ func (pm *PolicyManager) FilterForNode(node types.NodeView) ([]tailcfg.FilterRul
return nil, nil
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
return pm.filterForNodeLocked(node), nil
}
@@ -776,8 +780,8 @@ func (pm *PolicyManager) MatchersForNode(node types.NodeView) ([]matcher.Match,
return nil, nil
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
// For global policies, return the shared global matchers.
// Via grants require per-node matchers because the global matchers
@@ -786,7 +790,7 @@ func (pm *PolicyManager) MatchersForNode(node types.NodeView) ([]matcher.Match,
return pm.matchers, nil
}
if cached, ok := pm.matchersForNodeMap[node.ID()]; ok {
if cached, ok := pm.matchersForNodeMap.Load(node.ID()); ok {
return cached, nil
}
@@ -794,7 +798,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[node.ID()] = matchers
pm.matchersForNodeMap.Store(node.ID(), matchers)
return matchers, nil
}
@@ -813,7 +817,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
clear(pm.sshPolicyMap)
pm.sshPolicyMap.Clear()
changed, err := pm.updateLocked()
if err != nil {
@@ -866,9 +870,9 @@ func (pm *PolicyManager) SetNodes(nodes views.Slice[types.NodeView]) (bool, erro
if !needsUpdate {
// This ensures fresh filter rules are generated for all nodes
clear(pm.sshPolicyMap)
clear(pm.filterRulesMap)
clear(pm.matchersForNodeMap)
pm.sshPolicyMap.Clear()
pm.filterRulesMap.Clear()
pm.matchersForNodeMap.Clear()
}
// 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)
@@ -921,8 +925,8 @@ func (pm *PolicyManager) NodeCanHaveTag(node types.NodeView, tag string) bool {
return false
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
// pm.pol is written by SetPolicy under pm.mu; reading it before the
// lock races with concurrent policy reloads.
@@ -960,6 +964,37 @@ 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 {
@@ -1010,8 +1045,8 @@ func (pm *PolicyManager) TagExists(tag string) bool {
return false
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
// pm.pol is written by SetPolicy under pm.mu; reading it before the
// lock races with concurrent policy reloads.
@@ -1029,8 +1064,8 @@ func (pm *PolicyManager) NodeCanApproveRoute(node types.NodeView, route netip.Pr
return false
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
// 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
@@ -1092,8 +1127,8 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
return result
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
// pm.pol is written by SetPolicy under pm.mu; reading it before the
// lock races with concurrent policy reloads.
@@ -1311,8 +1346,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.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
var sb strings.Builder
@@ -1475,7 +1510,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.
for nodeID := range pm.filterRulesMap {
pm.filterRulesMap.Range(func(nodeID types.NodeID, _ []tailcfg.FilterRule) bool {
// Find the user for this cached node
var nodeUserID types.UserID
@@ -1518,20 +1553,22 @@ 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 {
delete(pm.filterRulesMap, nodeID)
delete(pm.matchersForNodeMap, nodeID)
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
}
} else {
// Node not found in either old or new list, clear it
delete(pm.filterRulesMap, nodeID)
delete(pm.matchersForNodeMap, nodeID)
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
}
}
return true
})
if len(affectedUsers) > 0 {
log.Debug().
Int("affected_users", len(affectedUsers)).
Int("remaining_cache_entries", len(pm.filterRulesMap)).
Int("remaining_cache_entries", pm.filterRulesMap.Size()).
Msg("Selectively cleared autogroup:self cache for affected users")
}
}
@@ -1572,23 +1609,27 @@ func (pm *PolicyManager) invalidateGlobalPolicyCache(newNodes views.Slice[types.
}
if newNode.HasNetworkChanges(oldNode) {
delete(pm.filterRulesMap, nodeID)
delete(pm.matchersForNodeMap, nodeID)
pm.filterRulesMap.Delete(nodeID)
pm.matchersForNodeMap.Delete(nodeID)
}
}
// Remove deleted nodes from cache
for nodeID := range pm.filterRulesMap {
pm.filterRulesMap.Range(func(nodeID types.NodeID, _ []tailcfg.FilterRule) bool {
if _, exists := newNodeMap[nodeID]; !exists {
delete(pm.filterRulesMap, nodeID)
pm.filterRulesMap.Delete(nodeID)
}
}
for nodeID := range pm.matchersForNodeMap {
return true
})
pm.matchersForNodeMap.Range(func(nodeID types.NodeID, _ []matcher.Match) bool {
if _, exists := newNodeMap[nodeID]; !exists {
delete(pm.matchersForNodeMap, nodeID)
pm.matchersForNodeMap.Delete(nodeID)
}
}
return true
})
}
// flattenTags resolves nested tag-owner references. Cycles
@@ -1770,8 +1811,8 @@ func (pm *PolicyManager) NodeCapMap(id types.NodeID) tailcfg.NodeCapMap {
return nil
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
src := pm.nodeAttrsMap[id]
if len(src) == 0 {
@@ -1794,8 +1835,8 @@ func (pm *PolicyManager) NodeCapMaps() map[types.NodeID]tailcfg.NodeCapMap {
return nil
}
pm.mu.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
out := make(map[types.NodeID]tailcfg.NodeCapMap, len(pm.nodeAttrsMap))
maps.Copy(out, pm.nodeAttrsMap)
@@ -0,0 +1,103 @@
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()
}
+13 -7
View File
@@ -8,6 +8,7 @@ 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"
@@ -122,7 +123,7 @@ func TestInvalidateAutogroupSelfCache(t *testing.T) {
require.NoError(t, err)
}
require.Len(t, pm.filterRulesMap, len(initialNodes))
require.Equal(t, len(initialNodes), pm.filterRulesMap.Size())
tests := []struct {
name string
@@ -207,19 +208,20 @@ func TestInvalidateAutogroupSelfCache(t *testing.T) {
}
}
pm.filterRulesMap = make(map[types.NodeID][]tailcfg.FilterRule)
pm.filterRulesMap.Clear()
for _, n := range initialNodes {
_, err := pm.FilterForNode(n.View())
require.NoError(t, err)
}
initialCacheSize := len(pm.filterRulesMap)
initialCacheSize := pm.filterRulesMap.Size()
require.Equal(t, len(initialNodes), initialCacheSize)
pm.invalidateAutogroupSelfCache(initialNodes.ViewSlice(), tt.newNodes.ViewSlice())
// Verify the expected number of cache entries were cleared
finalCacheSize := len(pm.filterRulesMap)
finalCacheSize := pm.filterRulesMap.Size()
clearedEntries := initialCacheSize - finalCacheSize
require.Equal(t, tt.expectedCleared, clearedEntries, tt.description)
})
@@ -498,15 +500,19 @@ func TestInvalidateGlobalPolicyCache(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pm := &PolicyManager{
nodes: tt.oldNodes.ViewSlice(),
filterRulesMap: tt.initialCache,
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)
}
pm.invalidateGlobalPolicyCache(tt.newNodes.ViewSlice())
// Verify cache state
for nodeID, shouldExist := range tt.expectedCacheAfter {
_, exists := pm.filterRulesMap[nodeID]
_, exists := pm.filterRulesMap.Load(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.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
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.Lock()
defer pm.mu.Unlock()
pm.mu.RLock()
defer pm.mu.RUnlock()
results := runPolicyTests(pm.pol, pm.filter, pm.users, pm.nodes)
if results.AllPassed {
+17 -8
View File
@@ -96,12 +96,6 @@ 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() {
@@ -144,8 +138,6 @@ 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.
@@ -230,6 +222,11 @@ 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
}
@@ -243,6 +240,13 @@ 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
@@ -252,6 +256,11 @@ 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,12 +7,15 @@ 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 {
@@ -89,6 +92,192 @@ 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
+92
View File
@@ -0,0 +1,92 @@
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
@@ -0,0 +1,67 @@
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")
}
+40
View File
@@ -0,0 +1,40 @@
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)
}
+166 -22
View File
@@ -286,7 +286,7 @@ func NewState(cfg *types.Config) (*State, error) {
)
nodeStore.Start()
return &State{
s := &State{
cfg: cfg,
db: db,
@@ -298,7 +298,14 @@ func NewState(cfg *types.Config) (*State, error) {
sshCheckAuth: make(map[sshCheckPair]time.Time),
registerLocks: xsync.NewMap[key.MachinePublic, *sync.Mutex](),
}, nil
}
// 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
}
// Close gracefully shuts down the [State] instance and releases all resources.
@@ -1031,7 +1038,10 @@ 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) {
err := dnsname.ValidLabel(newName)
// 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)
if err != nil {
return types.NodeView{}, change.Change{}, fmt.Errorf("renaming node: %w", err)
}
@@ -1436,6 +1446,13 @@ 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...)
@@ -1605,7 +1622,17 @@ 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.
rejectedTags := s.validateRequestTags(params.ExistingNode, requestTags)
//
// 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)
if len(rejectedTags) > 0 {
return types.NodeView{}, fmt.Errorf(
"%w %v are invalid or not permitted",
@@ -1699,8 +1726,14 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
} else {
node.Expiry = regData.Expiry
}
case isTagged && node.IsExpired():
// Tagged → Tagged, but carrying a stale PAST expiry from an older
// headscale's logout stamp (#3371). Tagged nodes never expire, so
// clear it; a deliberate future expiry has IsExpired() == false and
// falls through to the no-op below.
node.Expiry = nil
}
// Tagged → Tagged: keep existing expiry (nil) - no action needed
// Tagged → Tagged with no stale expiry: keep existing expiry - no action.
// Apply default node expiry for non-tagged nodes when the
// resolved expiry is still nil or zero (e.g., CLI registration
@@ -1721,8 +1754,21 @@ 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(nodeUpdateColumns).Updates(updatedNodeView.AsStruct()).Error
err := tx.Select(updateColumns).Updates(updatedNodeView.AsStruct()).Error
if err != nil {
return nil, fmt.Errorf("saving node: %w", err)
}
@@ -1926,6 +1972,34 @@ 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.
@@ -1960,16 +2034,34 @@ 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
// 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()
var approvedTags, rejectedTags []string
for _, tag := range requestTags {
if s.polMan.NodeCanHaveTag(node.View(), tag) {
if s.polMan.NodeCanHaveTag(node.View(), tag) ||
(authUser.Valid() && s.polMan.UserCanHaveTag(authUser, tag)) {
approvedTags = append(approvedTags, tag)
} else {
rejectedTags = append(rejectedTags, tag)
@@ -2319,7 +2411,14 @@ func (s *State) HandleNodeFromPreAuthKey(
// must present a valid key. Without this a node that re-uses its NodeKey
// after expiry would skip validation and be re-authorised with a spent or
// expired key; the boundary must not depend on the client rotating its key.
//
// Tagged nodes are excluded: they never expire (KB 1068), so an
// IsExpired() tagged node only reflects a stale logout stamp left by an
// older headscale (#3371). Forcing it down the re-validation path burns its
// fresh key and blocks re-auth forever; treat it as a plain re-registration
// and clear the stale expiry in the update below.
isExpired := existsSameUser && existingNodeSameUser.Valid() &&
!existingNodeSameUser.IsTagged() &&
existingNodeSameUser.IsExpired()
// A tagged key presented for a currently user-owned node converts that node
@@ -2328,7 +2427,17 @@ func (s *State) HandleNodeFromPreAuthKey(
isOwnershipConversion := existsSameUser && existingNodeSameUser.Valid() &&
pak.IsTagged() && !existingNodeSameUser.IsTagged()
if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion {
// 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
@@ -2376,8 +2485,10 @@ func (s *State) HandleNodeFromPreAuthKey(
var finalNode types.NodeView
// 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 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 existsSameUser && existingNodeSameUser.Valid() {
log.Trace().
Caller().
@@ -2419,17 +2530,37 @@ func (s *State) HandleNodeFromPreAuthKey(
node.RegisterMethod = util.RegisterMethodAuthKey
// Tags from PreAuthKey are only applied during initial registration.
// On re-registration the node keeps its existing tags and ownership,
// except when a tagged key converts a user-owned node: that adopts
// the key's tags and drops user ownership (tagged nodes are
// user-less and never expire). Only update AuthKey reference
// otherwise.
if pak.IsTagged() && !node.IsTagged() {
node.Tags = pak.Proto().GetAclTags()
// 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
node.Expiry = 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
}
}
node.AuthKey = pak
node.AuthKeyID = &pak.ID
@@ -2454,6 +2585,13 @@ func (s *State) HandleNodeFromPreAuthKey(
} else {
node.Expiry = nil
}
} else if node.IsExpired() {
// #3371: a tagged node must never carry key expiry. Clear a
// stale PAST expiry left by a logout (older headscale) so
// re-auth is not permanently blocked. A deliberate future
// expiry (headscale nodes expire) has IsExpired() == false and
// is left untouched.
node.Expiry = nil
}
})
@@ -2463,8 +2601,14 @@ 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).
err := tx.Select(nodeUpdateColumns).Updates(updatedNodeView.AsStruct()).Error
// (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
if err != nil {
return nil, fmt.Errorf("saving node: %w", err)
}
+23
View File
@@ -21,6 +21,7 @@ import (
"tailscale.com/tailcfg"
"tailscale.com/types/key"
"tailscale.com/types/views"
"tailscale.com/util/dnsname"
)
var (
@@ -568,6 +569,28 @@ 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,6 +418,33 @@ 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,6 +11,7 @@ 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"
@@ -579,6 +580,213 @@ 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.
//
@@ -3203,3 +3411,449 @@ 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
@@ -0,0 +1,315 @@
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
@@ -0,0 +1,103 @@
//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
@@ -0,0 +1,8 @@
//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() {}
+4 -2
View File
@@ -68,7 +68,9 @@ exclude_docs: |
# Plugins
plugins:
- search:
separator: '[\s\-,:!=\[\]()"`/]+|\.(?!\d)|&[lg]t;|(?!\b)(?=[A-Z][a-z])'
provider: lunr
lunr:
separator: '[\s\-,:!=\[\]()"`/]+|\.(?!\d)|&[lg]t;|(?!\b)(?=[A-Z][a-z])'
- macros:
- include-markdown:
- minify:
@@ -111,7 +113,7 @@ extra:
- icon: fontawesome/brands/discord
link: https://discord.gg/c84AZQhmpx
headscale:
version: 0.29.0
version: 0.29.3
# Extensions
markdown_extensions: