hscontrol: admit DERP clients via NodeKey index

/verify scanned every node per DERP connect; use GetNodeByNodeKey.
This commit is contained in:
Kristoffer Dalby
2026-09-30 15:30:03 +00:00
committed by Kristoffer Dalby
parent bc7fe6b8eb
commit 7ffdba7175
3 changed files with 103 additions and 6 deletions
+1
View File
@@ -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) - 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 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) - 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) ## 0.29.5 (202x-xx-xx)
+7 -6
View File
@@ -147,19 +147,20 @@ func (h *Headscale) handleVerifyRequest(
return NewHTTPError(http.StatusBadRequest, "Bad Request: invalid JSON", fmt.Errorf("parsing DERP client request: %w", err)) 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 { // Every DERP connect lands here, unauthenticated, so use the NodeKey
return n.NodeKey() == derpAdmitClientRequest.NodePublic // index rather than scanning every node.
}) nv, ok := h.state.GetNodeByNodeKey(derpAdmitClientRequest.NodePublic)
resp := &tailcfg.DERPAdmitClientResponse{ resp := &tailcfg.DERPAdmitClientResponse{
Allow: allow, Allow: ok && nv.Valid(),
} }
return json.NewEncoder(writer).Encode(resp) return json.NewEncoder(writer).Encode(resp)
} }
// VerifyHandler see https://github.com/tailscale/tailscale/blob/964282d34f06ecc06ce644769c66b0b31d118340/derp/derp_server.go#L1159 // VerifyHandler answers a DERP server's client-verification POST
// DERP use verifyClientsURL to verify whether a client is allowed to connect to the DERP server. // ([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( func (h *Headscale) VerifyHandler(
writer http.ResponseWriter, writer http.ResponseWriter,
req *http.Request, req *http.Request,
+95
View File
@@ -11,6 +11,7 @@ import (
"net/netip" "net/netip"
"strings" "strings"
"testing" "testing"
"time"
"github.com/juanfont/headscale/hscontrol/capver" "github.com/juanfont/headscale/hscontrol/capver"
"github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/types"
@@ -118,6 +119,100 @@ func TestVerifyHandler_SuccessSetsJSONContentType(t *testing.T) {
"successful /verify response must advertise application/json") "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 // TestKeyHandler_UnsupportedCapVerDoesNotLeakKey reproduces
// https://github.com/juanfont/headscale/issues/3380. The /key handler // https://github.com/juanfont/headscale/issues/3380. The /key handler
// must gate key disclosure on the same floor the Noise handshake // must gate key disclosure on the same floor the Noise handshake