mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-01 03:04:07 +09:00
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
(cherry picked from commit b833be283c, adapted: follow-up trimmed in 0.29's RegistrationHandler)
This commit is contained in:
1 parent
1b7435c57f
commit
a4c0a097d3
5 files changed
+340
-19
No files matched your search
+23
-12
@@ -136,7 +136,7 @@ func (h *Headscale) handleRegister(
|
|||||||
// node has already started the registration process and we should wait for
|
// node has already started the registration process and we should wait for
|
||||||
// it to finish the original registration.
|
// it to finish the original registration.
|
||||||
if req.Followup != "" {
|
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
|
// Pre authenticated keys are handled slightly different than interactive
|
||||||
@@ -310,12 +310,18 @@ func nodeToRegisterResponse(node types.NodeView) *tailcfg.RegisterResponse {
|
|||||||
return resp
|
return resp
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const maxFollowupURLLength = 2048
|
||||||
|
|
||||||
func (h *Headscale) waitForFollowup(
|
func (h *Headscale) waitForFollowup(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
req tailcfg.RegisterRequest,
|
followup string,
|
||||||
machineKey key.MachinePublic,
|
regData *types.RegistrationData,
|
||||||
) (*tailcfg.RegisterResponse, error) {
|
) (*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 {
|
if err != nil {
|
||||||
return nil, NewHTTPError(http.StatusUnauthorized, "invalid followup URL", err)
|
return nil, NewHTTPError(http.StatusUnauthorized, "invalid followup URL", err)
|
||||||
}
|
}
|
||||||
@@ -334,7 +340,7 @@ func (h *Headscale) waitForFollowup(
|
|||||||
// [State.HandleNodeFromAuthPath] builds the node from this cached
|
// [State.HandleNodeFromAuthPath] builds the node from this cached
|
||||||
// [types.RegistrationData.MachineKey]. [Headscale.handleRegister]
|
// [types.RegistrationData.MachineKey]. [Headscale.handleRegister]
|
||||||
// and [Headscale.handleLogout] apply the same check.
|
// 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(
|
return nil, NewHTTPError(
|
||||||
http.StatusUnauthorized,
|
http.StatusUnauthorized,
|
||||||
"registration was opened by a different machine key",
|
"registration was opened by a different machine key",
|
||||||
@@ -342,6 +348,8 @@ func (h *Headscale) waitForFollowup(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
superseded := reg.Park()
|
||||||
|
|
||||||
var verdict types.AuthVerdict
|
var verdict types.AuthVerdict
|
||||||
select {
|
select {
|
||||||
// Prefer a completed registration even if the context has also
|
// Prefer a completed registration even if the context has also
|
||||||
@@ -353,6 +361,12 @@ func (h *Headscale) waitForFollowup(
|
|||||||
select {
|
select {
|
||||||
case <-ctx.Done():
|
case <-ctx.Done():
|
||||||
return nil, NewHTTPError(http.StatusUnauthorized, "registration timed out", ctx.Err())
|
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():
|
case verdict = <-reg.WaitForAuth():
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -360,7 +374,7 @@ func (h *Headscale) waitForFollowup(
|
|||||||
if verdict.Accept() {
|
if verdict.Accept() {
|
||||||
if !verdict.Node.Valid() {
|
if !verdict.Node.Valid() {
|
||||||
// registration is expired in the cache, instruct the client to try a new registration
|
// 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
|
return nodeToRegisterResponse(verdict.Node), nil
|
||||||
@@ -368,24 +382,21 @@ func (h *Headscale) waitForFollowup(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// if the follow-up registration isn't found anymore, instruct the client to try a new registration
|
// 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
|
// reqToNewRegisterResponse refreshes the registration flow by creating a new
|
||||||
// registration ID and returning the corresponding [tailcfg.RegisterResponse.AuthURL]
|
// registration ID and returning the corresponding [tailcfg.RegisterResponse.AuthURL]
|
||||||
// so the client can restart the authentication process.
|
// so the client can restart the authentication process.
|
||||||
func (h *Headscale) reqToNewRegisterResponse(
|
func (h *Headscale) reqToNewRegisterResponse(
|
||||||
req tailcfg.RegisterRequest,
|
regData *types.RegistrationData,
|
||||||
machineKey key.MachinePublic,
|
|
||||||
) (*tailcfg.RegisterResponse, error) {
|
) (*tailcfg.RegisterResponse, error) {
|
||||||
newAuthID, err := types.NewAuthID()
|
newAuthID, err := types.NewAuthID()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, NewHTTPError(http.StatusInternalServerError, "failed to generate registration ID", err)
|
return nil, NewHTTPError(http.StatusInternalServerError, "failed to generate registration ID", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
authRegReq := types.NewRegisterAuthRequest(
|
authRegReq := types.NewRegisterAuthRequest(regData)
|
||||||
registrationDataFromRequest(req, machineKey),
|
|
||||||
)
|
|
||||||
|
|
||||||
log.Info().Msgf("new followup node registration using auth id: %s", newAuthID)
|
log.Info().Msgf("new followup node registration using auth id: %s", newAuthID)
|
||||||
h.state.SetAuthCacheEntry(newAuthID, authRegReq)
|
h.state.SetAuthCacheEntry(newAuthID, authRegReq)
|
||||||
|
|||||||
+177
-5
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/netip"
|
"net/netip"
|
||||||
"net/url"
|
"net/url"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
@@ -4379,12 +4380,10 @@ func TestFollowupWaitPrefersCompletedAuthOverExpiredContext(t *testing.T) {
|
|||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
cancel()
|
cancel()
|
||||||
|
|
||||||
req := tailcfg.RegisterRequest{
|
regData := &types.RegistrationData{MachineKey: machineKey, NodeKey: nodeKey}
|
||||||
Followup: fmt.Sprintf("http://localhost:8080/register/%s", regID),
|
followup := fmt.Sprintf("http://localhost:8080/register/%s", regID)
|
||||||
NodeKey: nodeKey,
|
|
||||||
}
|
|
||||||
|
|
||||||
resp, err := app.waitForFollowup(ctx, req, machineKey)
|
resp, err := app.waitForFollowup(ctx, followup, regData)
|
||||||
switch {
|
switch {
|
||||||
case err != nil:
|
case err != nil:
|
||||||
timeouts++
|
timeouts++
|
||||||
@@ -4481,3 +4480,176 @@ func TestPendingRegistrationKeepsAuthMetadata(t *testing.T) {
|
|||||||
assert.Equal(t, []string{"tag:router"}, entry.RegistrationData().Hostinfo.RequestTags)
|
assert.Equal(t, []string{"tag:router"}, entry.RegistrationData().Hostinfo.RequestTags)
|
||||||
assert.Len(t, hostinfo.RoutableIPs, 1, "the request itself must be left alone")
|
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
@@ -621,8 +621,9 @@ func (ns *noiseServer) sshActionHoldAndDelegate(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// sshActionFollowUp handles follow-up requests where the client
|
// sshActionFollowUp handles follow-up requests where the client
|
||||||
// provides an auth_id. It blocks until the auth session resolves or
|
// provides an auth_id. It blocks until the auth session resolves, the
|
||||||
// the request context is cancelled (e.g. the client disconnects).
|
// request context is cancelled (e.g. the client disconnects), or a newer
|
||||||
|
// follow-up parks on the session.
|
||||||
func (ns *noiseServer) sshActionFollowUp(
|
func (ns *noiseServer) sshActionFollowUp(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
reqLog zerolog.Logger,
|
reqLog zerolog.Logger,
|
||||||
@@ -686,6 +687,8 @@ func (ns *noiseServer) sshActionFollowUp(
|
|||||||
|
|
||||||
reqLog.Trace().Caller().Msg("SSH action follow-up")
|
reqLog.Trace().Caller().Msg("SSH action follow-up")
|
||||||
|
|
||||||
|
superseded := auth.Park()
|
||||||
|
|
||||||
var (
|
var (
|
||||||
verdict types.AuthVerdict
|
verdict types.AuthVerdict
|
||||||
verdictOK bool
|
verdictOK bool
|
||||||
@@ -703,6 +706,10 @@ func (ns *noiseServer) sshActionFollowUp(
|
|||||||
"ssh action follow-up cancelled",
|
"ssh action follow-up cancelled",
|
||||||
ctx.Err(),
|
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():
|
case verdict, verdictOK = <-auth.WaitForAuth():
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -872,6 +879,20 @@ func (ns *noiseServer) RegistrationHandler(
|
|||||||
return ®Req, regErr(err)
|
return ®Req, regErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if regReq.Followup != "" {
|
||||||
|
// Keep only fields used by logout checks or a refreshed registration.
|
||||||
|
var auth *tailcfg.RegisterResponseAuth
|
||||||
|
if regReq.Auth != nil {
|
||||||
|
auth = &tailcfg.RegisterResponseAuth{}
|
||||||
|
}
|
||||||
|
|
||||||
|
regReq = tailcfg.RegisterRequest{
|
||||||
|
Version: regReq.Version, NodeKey: regReq.NodeKey,
|
||||||
|
Followup: regReq.Followup, Expiry: regReq.Expiry,
|
||||||
|
Auth: auth, Hostinfo: registrationDataFromRequest(regReq, ns.machineKey).Hostinfo,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if hi := regReq.Hostinfo; hi != nil {
|
if hi := regReq.Hostinfo; hi != nil {
|
||||||
if len(hi.RoutableIPs) > types.MaxRoutableIPs {
|
if len(hi.RoutableIPs) > types.MaxRoutableIPs {
|
||||||
log.Warn().Str(zf.MachineKey, ns.machineKey.ShortString()).
|
log.Warn().Str(zf.MachineKey, ns.machineKey.ShortString()).
|
||||||
|
|||||||
@@ -1460,6 +1460,35 @@ func TestRegistrationHandler_ValidatesClientReport(t *testing.T) {
|
|||||||
}, entry.RegistrationData().Hostinfo)
|
}, 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.Run("oversized report", func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
@@ -1703,3 +1732,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)
|
||||||
|
}
|
||||||
@@ -282,6 +282,10 @@ type AuthRequest struct {
|
|||||||
|
|
||||||
finished chan AuthVerdict
|
finished chan AuthVerdict
|
||||||
closed *atomic.Bool
|
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
|
// NewAuthRequest creates a pending auth request with no payload, suitable
|
||||||
@@ -387,6 +391,22 @@ func (rn *AuthRequest) WaitForAuth() <-chan AuthVerdict {
|
|||||||
return rn.finished
|
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 {
|
type AuthVerdict struct {
|
||||||
// Err is the error that occurred during the authentication process, if any.
|
// Err is the error that occurred during the authentication process, if any.
|
||||||
// If Err is nil, the authentication process has succeeded.
|
// If Err is nil, the authentication process has succeeded.
|
||||||
|
|||||||
Reference in new issue
Block a user