From 7ffdba7175d8b9e726a75e58a20ee13a5f6aa7c4 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 30 Sep 2026 15:30:03 +0000 Subject: [PATCH] hscontrol: admit DERP clients via NodeKey index /verify scanned every node per DERP connect; use GetNodeByNodeKey. --- CHANGELOG.md | 1 + hscontrol/handlers.go | 13 +++--- hscontrol/handlers_test.go | 95 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e56d47599..02bbc7505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ clients, and how to run the same setup without Nix. - A node re-registering with a spent, expired or revoked pre-auth key is now rejected if it expired or changed node key while the re-registration was in flight [#3525](https://github.com/juanfont/headscale/pull/3525) - Fix a node ping being lost when a full map update is queued at the same time [#3523](https://github.com/juanfont/headscale/pull/3523) - Fix clients uploading logs to Tailscale Inc. while `logtail.enabled` is `false`; clients also granted the `data-plane-audit-logs` node attribute now go down until `tailscale up` [#3522](https://github.com/juanfont/headscale/pull/3522) +- Embedded DERP client verification (`/verify`) looks up the node key directly instead of scanning every node on each connection ## 0.29.5 (202x-xx-xx) diff --git a/hscontrol/handlers.go b/hscontrol/handlers.go index 2a0157cd6..717934bf0 100644 --- a/hscontrol/handlers.go +++ b/hscontrol/handlers.go @@ -147,19 +147,20 @@ func (h *Headscale) handleVerifyRequest( return NewHTTPError(http.StatusBadRequest, "Bad Request: invalid JSON", fmt.Errorf("parsing DERP client request: %w", err)) } - allow := h.state.ListNodes().ContainsFunc(func(n types.NodeView) bool { - return n.NodeKey() == derpAdmitClientRequest.NodePublic - }) + // Every DERP connect lands here, unauthenticated, so use the NodeKey + // index rather than scanning every node. + nv, ok := h.state.GetNodeByNodeKey(derpAdmitClientRequest.NodePublic) resp := &tailcfg.DERPAdmitClientResponse{ - Allow: allow, + Allow: ok && nv.Valid(), } return json.NewEncoder(writer).Encode(resp) } -// VerifyHandler see https://github.com/tailscale/tailscale/blob/964282d34f06ecc06ce644769c66b0b31d118340/derp/derp_server.go#L1159 -// DERP use verifyClientsURL to verify whether a client is allowed to connect to the DERP server. +// VerifyHandler answers a DERP server's client-verification POST +// ([tailcfg.DERPAdmitClientRequest]). A client is admitted while its NodeKey +// belongs to a registered node; expiry, tags and ephemerality do not gate it. func (h *Headscale) VerifyHandler( writer http.ResponseWriter, req *http.Request, diff --git a/hscontrol/handlers_test.go b/hscontrol/handlers_test.go index f1ed71a09..4433f16ed 100644 --- a/hscontrol/handlers_test.go +++ b/hscontrol/handlers_test.go @@ -11,6 +11,7 @@ import ( "net/netip" "strings" "testing" + "time" "github.com/juanfont/headscale/hscontrol/capver" "github.com/juanfont/headscale/hscontrol/types" @@ -118,6 +119,100 @@ func TestVerifyHandler_SuccessSetsJSONContentType(t *testing.T) { "successful /verify response must advertise application/json") } +// TestHandleVerifyRequest_AdmitsByNodeKey pins DERP admission to NodeKey +// membership. The oracle is the full-list scan the handler used to run, so +// the indexed lookup must agree with it on every row: expiry, tags and +// ephemerality never gate admission, and a rotated-away or deleted key is +// refused. +func TestHandleVerifyRequest_AdmitsByNodeKey(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + user := app.state.CreateUserForTest("derp-admit") + + owned := putTestNodeInStore(t, app, user, "owned") + + tagged := app.state.CreateNodeForTest(user, "tagged") + tagged.Tags = []string{"tag:derp"} + app.state.PutNodeInStoreForTest(*tagged) + + ephemeral := app.state.CreateNodeForTest(user, "ephemeral") + ephemeral.AuthKey = &types.Credential{Ephemeral: true} + app.state.PutNodeInStoreForTest(*ephemeral) + + expired := app.state.CreateNodeForTest(user, "expired") + expired.Expiry = new(time.Now().Add(-time.Hour)) + app.state.PutNodeInStoreForTest(*expired) + + rotated := putTestNodeInStore(t, app, user, "rotated") + rotatedFrom := rotated.NodeKey + rotated.NodeKey = key.NewNode().Public() + app.state.PutNodeInStoreForTest(*rotated) + + deleted := putTestNodeInStore(t, app, user, "deleted") + deletedView, ok := app.state.GetNodeByID(deleted.ID) + require.True(t, ok) + + _, err := app.state.DeleteNode(deletedView) + require.NoError(t, err) + + nv, ok := app.state.GetNodeByID(tagged.ID) + require.True(t, ok) + require.True(t, nv.IsTagged(), "test sanity: tagged row must be tagged") + nv, ok = app.state.GetNodeByID(ephemeral.ID) + require.True(t, ok) + require.True(t, nv.IsEphemeral(), "test sanity: ephemeral row must be ephemeral") + nv, ok = app.state.GetNodeByID(expired.ID) + require.True(t, ok) + require.True(t, nv.IsExpired(), "test sanity: expired row must be expired") + + tests := []struct { + name string + key key.NodePublic + want bool + }{ + {name: "user", key: owned.NodeKey, want: true}, + {name: "tagged", key: tagged.NodeKey, want: true}, + {name: "ephemeral", key: ephemeral.NodeKey, want: true}, + {name: "rotated/old", key: rotatedFrom, want: false}, + {name: "rotated/new", key: rotated.NodeKey, want: true}, + {name: "expired", key: expired.NodeKey, want: true}, + {name: "deleted", key: deleted.NodeKey, want: false}, + {name: "unknown", key: key.NewNode().Public(), want: false}, + {name: "zero", key: key.NodePublic{}, want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + body, err := json.Marshal(tailcfg.DERPAdmitClientRequest{NodePublic: tt.key}) + require.NoError(t, err) + + req := httptest.NewRequestWithContext( + context.Background(), + http.MethodPost, + "/verify", + bytes.NewReader(body), + ) + + var out bytes.Buffer + require.NoError(t, app.handleVerifyRequest(req, &out)) + + var resp tailcfg.DERPAdmitClientResponse + require.NoError(t, json.Unmarshal(out.Bytes(), &resp)) + + oracle := app.state.ListNodes().ContainsFunc(func(n types.NodeView) bool { + return n.NodeKey() == tt.key + }) + + assert.Equal(t, tt.want, resp.Allow) + assert.Equal(t, oracle, resp.Allow, + "indexed admission must match the full-list membership scan") + }) + } +} + // TestKeyHandler_UnsupportedCapVerDoesNotLeakKey reproduces // https://github.com/juanfont/headscale/issues/3380. The /key handler // must gate key disclosure on the same floor the Noise handshake