types, auth, noise: let one request wait on each auth session

One verdict per session, so one waiter: the newest. A displaced one
returns at once (register error, SSH re-delegate), not at session end.
A waiter keeps only the follow-up URL and the cached registration.

Fixes ANT-2026-NM50ZP4N
This commit is contained in:
Kristoffer Dalby committed 2026-10-29 14:00:02 +01:00
1 parent 7892a8462c
commit b833be283c
6 files changed
+342 -20

No files matched your search

+2 -1
View File
@@ -160,10 +160,11 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`,
- Refuse a key expiry outside the years 0 to 9999 in UTC on registration, on the OIDC token-expiry path and in `headscale nodes expire` / `POST /api/v1/node/{nodeId}/expire` (400); the latest accepted expiry no longer fails node listings on PostgreSQL in a zone east of UTC
- Keep the first 256 endpoints and 4096 announced routes from client reports; oversized hostnames, requested tags or other metadata return a registration error or HTTP 413 on map updates. Zoned IPv6 endpoints are refused
- At startup, trim stored hostnames, endpoints and routes to the client-report limits and remove zoned endpoints. Hostinfo exceeding its size or tag limit is dropped until the next valid report
- Pending interactive registrations keep only the hostname, OS and requested tags; other device details arrive with the first map request. Reauthentication preserves routes, services and endpoints
- Pending interactive registrations and follow-up waits keep only the hostname, OS and requested tags; other device details arrive with the first map request. Reauthentication preserves routes, services and endpoints
- Web reauthentication preserves the discovery key so peers can still reach the device
- A stalled request body, or a Noise tunnel with no open request, is closed after 30 seconds
- A registration follow-up from a machine other than the one that started the registration is refused before it waits
- Only the newest follow-up waits on a pending registration or SSH check; older ones are answered at once
## 0.29.7 (202x-xx-xx)
+23 -12
View File
@@ -133,7 +133,7 @@ func (h *Headscale) handleRegister(
// node has already started the registration process and we should wait for
// it to finish the original registration.
if req.Followup != "" {
return h.waitForFollowup(ctx, req, machineKey)
return h.waitForFollowup(ctx, req.Followup, registrationDataFromRequest(req, machineKey))
}
// Pre authenticated keys are handled slightly different than interactive
@@ -308,12 +308,18 @@ func nodeToRegisterResponse(node types.NodeView) *tailcfg.RegisterResponse {
return resp
}
const maxFollowupURLLength = 2048
func (h *Headscale) waitForFollowup(
ctx context.Context,
req tailcfg.RegisterRequest,
machineKey key.MachinePublic,
followup string,
regData *types.RegistrationData,
) (*tailcfg.RegisterResponse, error) {
fu, err := url.Parse(req.Followup)
if len(followup) > maxFollowupURLLength {
return nil, NewHTTPError(http.StatusUnauthorized, "followup URL too long", nil)
}
fu, err := url.Parse(followup)
if err != nil {
return nil, NewHTTPError(http.StatusUnauthorized, "invalid followup URL", err)
}
@@ -332,7 +338,7 @@ func (h *Headscale) waitForFollowup(
// [State.HandleNodeFromAuthPath] builds the node from this cached
// [types.RegistrationData.MachineKey]. [Headscale.handleRegister]
// and [Headscale.handleLogout] apply the same check.
if !reg.IsRegistration() || reg.RegistrationData().MachineKey != machineKey {
if !reg.IsRegistration() || reg.RegistrationData().MachineKey != regData.MachineKey {
return nil, NewHTTPError(
http.StatusUnauthorized,
"registration was opened by a different machine key",
@@ -340,6 +346,8 @@ func (h *Headscale) waitForFollowup(
)
}
superseded := reg.Park()
var verdict types.AuthVerdict
select {
// Prefer a completed registration even if the context has also
@@ -351,6 +359,12 @@ func (h *Headscale) waitForFollowup(
select {
case <-ctx.Done():
return nil, NewHTTPError(http.StatusUnauthorized, "registration timed out", ctx.Err())
case <-superseded:
// tailscaled keeps one follow-up in flight, so only a request
// it already abandoned is displaced.
return nil, NewHTTPError(
http.StatusConflict, "superseded by a newer followup request", nil,
)
case verdict = <-reg.WaitForAuth():
}
}
@@ -358,7 +372,7 @@ func (h *Headscale) waitForFollowup(
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 h.reqToNewRegisterResponse(regData)
}
return nodeToRegisterResponse(verdict.Node), nil
@@ -366,24 +380,21 @@ func (h *Headscale) waitForFollowup(
}
// if the follow-up registration isn't found anymore, instruct the client to try a new registration
return h.reqToNewRegisterResponse(req, machineKey)
return h.reqToNewRegisterResponse(regData)
}
// reqToNewRegisterResponse refreshes the registration flow by creating a new
// registration ID and returning the corresponding [tailcfg.RegisterResponse.AuthURL]
// so the client can restart the authentication process.
func (h *Headscale) reqToNewRegisterResponse(
req tailcfg.RegisterRequest,
machineKey key.MachinePublic,
regData *types.RegistrationData,
) (*tailcfg.RegisterResponse, error) {
newAuthID, err := types.NewAuthID()
if err != nil {
return nil, NewHTTPError(http.StatusInternalServerError, "failed to generate registration ID", err)
}
authRegReq := types.NewRegisterAuthRequest(
registrationDataFromRequest(req, machineKey),
)
authRegReq := types.NewRegisterAuthRequest(regData)
log.Info().Msgf("new followup node registration using auth id: %s", newAuthID)
h.state.SetAuthCacheEntry(newAuthID, authRegReq)
+177 -5
View File
@@ -7,6 +7,7 @@ import (
"net/http"
"net/netip"
"net/url"
"strconv"
"strings"
"testing"
"time"
@@ -4596,12 +4597,10 @@ func TestFollowupWaitPrefersCompletedAuthOverExpiredContext(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
req := tailcfg.RegisterRequest{
Followup: fmt.Sprintf("http://localhost:8080/register/%s", regID),
NodeKey: nodeKey,
}
regData := &types.RegistrationData{MachineKey: machineKey, NodeKey: nodeKey}
followup := fmt.Sprintf("http://localhost:8080/register/%s", regID)
resp, err := app.waitForFollowup(ctx, req, machineKey)
resp, err := app.waitForFollowup(ctx, followup, regData)
switch {
case err != nil:
timeouts++
@@ -4698,3 +4697,176 @@ func TestPendingRegistrationKeepsAuthMetadata(t *testing.T) {
assert.Equal(t, []string{"tag:router"}, entry.RegistrationData().Hostinfo.RequestTags)
assert.Len(t, hostinfo.RoutableIPs, 1, "the request itself must be left alone")
}
// TestFollowupParksOneRequestPerRegistration pins that a pending registration
// keeps one waiting follow-up, the newest. The session hands out one verdict,
// so older waiters are answered at once instead of staying parked until the
// session ends and then each opening a new one.
func TestFollowupParksOneRequestPerRegistration(t *testing.T) {
app := createTestApp(t)
machineKey := key.NewMachine().Public()
authID := types.MustAuthID()
reg := types.NewRegisterAuthRequest(&types.RegistrationData{
MachineKey: machineKey,
Hostname: "followup-park-node",
})
app.state.SetAuthCacheEntry(authID, reg)
regData := &types.RegistrationData{
MachineKey: machineKey,
NodeKey: key.NewNode().Public(),
}
followup := fmt.Sprintf("http://localhost:8080/register/%s", authID)
const followups = 50
type answer struct {
resp *tailcfg.RegisterResponse
err error
}
answered := make(chan answer, followups)
for range followups {
go func() {
resp, err := app.waitForFollowup(t.Context(), followup, regData)
answered <- answer{resp, err}
}()
}
next := func(msg string) answer {
t.Helper()
select {
case a := <-answered:
return a
case <-time.After(5 * time.Second):
require.FailNow(t, msg)
return answer{}
}
}
for range followups - 1 {
var httpErr HTTPError
require.ErrorAs(t,
next("more than one follow-up is parked on the registration").err,
&httpErr)
assert.Equal(t, http.StatusConflict, httpErr.Code)
}
select {
case a := <-answered:
require.FailNowf(t, "the newest follow-up must still be waiting", "got %+v", a)
case <-time.After(50 * time.Millisecond):
}
cacheLen := func() int {
n, _ := app.state.DebugRegistrationCache()["current_len"].(int)
return n
}
before := cacheLen()
user := app.state.CreateUserForTest("followup-park-user")
node := app.state.CreateNodeForTest(user, "followup-park-node")
node.MachineKey = machineKey
node.User = user
reg.FinishAuth(types.AuthVerdict{Node: node.View()})
last := next("the parked follow-up did not collect the verdict")
require.NoError(t, last.err)
assert.True(t, last.resp.MachineAuthorized)
assert.LessOrEqual(t, cacheLen(), before+1,
"the verdict must not open a session per waiter")
}
func TestFollowupURLLength(t *testing.T) {
t.Parallel()
for _, length := range []int{maxFollowupURLLength, maxFollowupURLLength + 1} {
t.Run(strconv.Itoa(length), func(t *testing.T) {
t.Parallel()
app := createTestApp(t)
followup := fmt.Sprintf("http://localhost:8080/register/%s?padding=", types.MustAuthID())
followup += strings.Repeat("x", length-len(followup))
regData := &types.RegistrationData{
MachineKey: key.NewMachine().Public(),
NodeKey: key.NewNode().Public(),
Hostname: "followup-length-node",
}
resp, err := app.waitForFollowup(t.Context(), followup, regData)
if length > maxFollowupURLLength {
var httpErr HTTPError
require.ErrorAs(t, err, &httpErr)
assert.Equal(t, http.StatusUnauthorized, httpErr.Code)
assert.Equal(t, "followup URL too long", httpErr.Msg)
assert.Nil(t, resp)
assert.Equal(t, 0, app.state.DebugRegistrationCache()["current_len"])
return
}
require.NoError(t, err)
require.NotNil(t, resp)
assert.NotEmpty(t, resp.AuthURL)
})
}
}
func TestFollowupRefreshKeepsAuthMetadata(t *testing.T) {
t.Parallel()
for _, completed := range []bool{false, true} {
t.Run(fmt.Sprintf("completed=%t", completed), func(t *testing.T) {
t.Parallel()
app := createTestApp(t)
machineKey := key.NewMachine().Public()
authID := types.MustAuthID()
if completed {
reg := types.NewRegisterAuthRequest(&types.RegistrationData{
MachineKey: machineKey, Hostname: "previous-request",
})
app.state.SetAuthCacheEntry(authID, reg)
reg.FinishAuth(types.AuthVerdict{})
}
expiry := time.Now().UTC().Add(time.Hour)
req := tailcfg.RegisterRequest{
Followup: fmt.Sprintf("http://localhost:8080/register/%s", authID),
NodeKey: key.NewNode().Public(),
Expiry: expiry,
Hostinfo: &tailcfg.Hostinfo{
Hostname: "refreshed-node", OS: "linux", OSVersion: "6.12",
RequestTags: []string{"tag:router"},
Services: []tailcfg.Service{{Proto: tailcfg.PeerAPI4, Port: 41641}},
},
}
resp, err := app.handleRegister(t.Context(), req, machineKey)
require.NoError(t, err)
require.NotNil(t, resp)
newAuthID, err := extractRegistrationIDFromAuthURL(resp.AuthURL)
require.NoError(t, err)
assert.NotEqual(t, authID, newAuthID)
entry, ok := app.state.GetAuthCacheEntry(newAuthID)
require.True(t, ok)
assert.Equal(t, &types.RegistrationData{
MachineKey: machineKey,
NodeKey: req.NodeKey,
Hostname: "refreshed-node",
Expiry: &expiry,
Hostinfo: &tailcfg.Hostinfo{
Hostname: "refreshed-node", OS: "linux", RequestTags: []string{"tag:router"},
},
}, entry.RegistrationData())
req.Hostinfo.RequestTags[0] = "tag:changed"
assert.Equal(t, []string{"tag:router"}, entry.RegistrationData().Hostinfo.RequestTags)
})
}
}
+23 -2
View File
@@ -599,8 +599,9 @@ func (ns *noiseServer) sshActionHoldAndDelegate(
}
// sshActionFollowUp handles follow-up requests where the client
// provides an auth_id. It blocks until the auth session resolves or
// the request context is cancelled (e.g. the client disconnects).
// provides an auth_id. It blocks until the auth session resolves, the
// request context is cancelled (e.g. the client disconnects), or a newer
// follow-up parks on the session.
func (ns *noiseServer) sshActionFollowUp(
ctx context.Context,
reqLog zerolog.Logger,
@@ -664,6 +665,8 @@ func (ns *noiseServer) sshActionFollowUp(
reqLog.Trace().Caller().Msg("SSH action follow-up")
superseded := auth.Park()
var (
verdict types.AuthVerdict
verdictOK bool
@@ -681,6 +684,10 @@ func (ns *noiseServer) sshActionFollowUp(
"ssh action follow-up cancelled",
ctx.Err(),
)
case <-superseded:
// The newer request collects the verdict; this one re-delegates, as
// it would on a consumed verdict, without waiting for it.
return sessionGone("SSH check follow-up superseded; re-delegating")
case verdict, verdictOK = <-auth.WaitForAuth():
}
@@ -957,6 +964,20 @@ func (ns *noiseServer) RegistrationHandler(
decodeErr := json.NewDecoder(req.Body).Decode(&registerRequest)
if decodeErr == nil {
if registerRequest.Followup != "" {
// Keep only fields used by logout checks or a refreshed registration.
var auth *tailcfg.RegisterResponseAuth
if registerRequest.Auth != nil {
auth = &tailcfg.RegisterResponseAuth{}
}
registerRequest = tailcfg.RegisterRequest{
Version: registerRequest.Version, NodeKey: registerRequest.NodeKey,
Followup: registerRequest.Followup, Expiry: registerRequest.Expiry,
Auth: auth, Hostinfo: registrationDataFromRequest(registerRequest, ns.machineKey).Hostinfo,
}
}
if hi := registerRequest.Hostinfo; hi != nil {
if len(hi.RoutableIPs) > types.MaxRoutableIPs {
log.Warn().Str(zf.MachineKey, ns.machineKey.ShortString()).
+97
View File
@@ -1975,6 +1975,35 @@ func TestRegistrationHandler_ValidatesClientReport(t *testing.T) {
}, entry.RegistrationData().Hostinfo)
})
t.Run("followup discards unused request data", func(t *testing.T) {
t.Parallel()
app := createTestApp(t)
ns := &noiseServer{headscale: app, machineKey: key.NewMachine().Public()}
rec := serveRegister(t, ns, tailcfg.RegisterRequest{
Version: tailcfg.CurrentCapabilityVersion, NodeKey: key.NewNode().Public(),
Followup: "http://localhost:8080/register/" + types.MustAuthID().String(),
Auth: &tailcfg.RegisterResponseAuth{AuthKey: "unused-followup-key"},
Hostinfo: &tailcfg.Hostinfo{
Hostname: "followup-node", OS: "linux", RequestTags: []string{"tag:test"},
OSVersion: strings.Repeat("v", types.MaxHostinfoSize),
},
DeviceCert: []byte("unused"), Signature: []byte("unused"), Tailnet: "unused",
})
require.Equal(t, http.StatusOK, rec.Code)
var resp tailcfg.RegisterResponse
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp))
require.Empty(t, resp.Error)
authID, err := extractRegistrationIDFromAuthURL(resp.AuthURL)
require.NoError(t, err)
entry, ok := app.state.GetAuthCacheEntry(authID)
require.True(t, ok)
assert.Equal(t, &tailcfg.Hostinfo{
Hostname: "followup-node", OS: "linux", RequestTags: []string{"tag:test"},
}, entry.RegistrationData().Hostinfo)
})
t.Run("oversized report", func(t *testing.T) {
t.Parallel()
@@ -2219,3 +2248,71 @@ func TestNoiseHTTPServerTimeouts(t *testing.T) {
}
})
}
// TestSSHActionFollowUp_ParksOneRequestPerSession pins that a pending SSH
// check keeps one waiting follow-up, the newest. Older waiters re-delegate at
// once instead of staying parked until the session ends.
func TestSSHActionFollowUp_ParksOneRequestPerSession(t *testing.T) {
t.Parallel()
app := createTestApp(t)
nodes := putSSHCheckNodes(t, app, "ssh-park-user", "src-node", "dst-node")
src, dst := nodes[0], nodes[1]
authID := types.MustAuthID()
auth := types.NewSSHCheckAuthRequest(src.ID, dst.ID)
app.state.SetAuthCacheEntry(authID, auth)
ns := &noiseServer{headscale: app, machineKey: dst.MachineKey}
const followups = 50
type answer struct {
action *tailcfg.SSHAction
err error
}
answered := make(chan answer, followups)
for range followups {
go func() {
action, err := ns.sshActionFollowUp(
t.Context(), zerolog.Nop(), &tailcfg.SSHAction{},
authID.String(), src.ID, dst.ID, sshTestLocalUser,
)
answered <- answer{action, err}
}()
}
next := func(msg string) answer {
t.Helper()
select {
case a := <-answered:
return a
case <-time.After(5 * time.Second):
require.FailNow(t, msg)
return answer{}
}
}
for range followups - 1 {
a := next("more than one follow-up is parked on the session")
require.NoError(t, a.err)
assert.NotEqual(t, authID, requireSSHHold(t, *a.action),
"re-delegation must mint a new session")
}
select {
case a := <-answered:
require.FailNowf(t, "the newest follow-up must still be waiting", "got %+v", a)
case <-time.After(50 * time.Millisecond):
}
auth.FinishAuth(types.AuthVerdict{})
last := next("the parked follow-up did not collect the verdict")
require.NoError(t, last.err)
assert.True(t, last.action.Accept, "accept, got %+v", last.action)
}
+20
View File
@@ -143,6 +143,10 @@ type AuthRequest struct {
finished chan AuthVerdict
closed *atomic.Bool
// waiter is closed once a newer request parks on this session; see
// [AuthRequest.Park].
waiter atomic.Pointer[chan struct{}]
}
// NewAuthRequest creates a pending auth request with no payload, suitable
@@ -245,6 +249,22 @@ func (rn *AuthRequest) WaitForAuth() <-chan AuthVerdict {
return rn.finished
}
// Park makes the caller the one request waiting on [AuthRequest.WaitForAuth]
// and returns a channel that is closed once a newer caller takes its place.
// [AuthRequest.FinishAuth] hands out one verdict, so a session needs one
// waiter. The newest wins, so a client that reconnected is not kept out by
// its own dead request. Parked requests are then bounded by the sessions in
// the auth cache, not by how many requests clients send.
func (rn *AuthRequest) Park() <-chan struct{} {
waiter := make(chan struct{})
if prev := rn.waiter.Swap(&waiter); prev != nil {
close(*prev)
}
return waiter
}
type AuthVerdict struct {
// Err is the error that occurred during the authentication process, if any.
// If Err is nil, the authentication process has succeeded.