fix(policy): validate reauth tags against the authenticating user

A tagged node re-authenticating with --advertise-tags was always rejected,
even when the authenticating user owned every requested tag. applyAuthNodeUpdate
validated the tags against the existing tag-owned node, which has no user of its
own and whose IP is not in any tag owner's set, so NodeCanHaveTag could never
approve them; the authenticating user who actually owns the tags was never
consulted. Fresh registration works because it assigns that user to the node
before validating.

Add PolicyManager.UserCanHaveTag (the user half of NodeCanHaveTag) and validate
reauth tags against the authenticating user as well as the node, so an owned tag
is permitted while unowned tags are still rejected.

Fixes #3374
This commit is contained in:
maximilize 2026-07-13 21:30:54 +00:00
parent 048308511c
commit 62279d469d
5 changed files with 175 additions and 1 deletions

View file

@ -44,6 +44,7 @@ keys remain all-access.
- Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324)
- Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341)
- Re-authenticating a tagged node with `--advertise-tags` now authorises the requested tags against the authenticating user, instead of rejecting every tag [#3374](https://github.com/juanfont/headscale/issues/3374)
## 0.29.2 (2026-07-01)

View file

@ -30,6 +30,10 @@ 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 and may
// therefore apply it, e.g. when re-authenticating a tag-owned node.
UserCanHaveTag(user types.UserView, tag string) bool
// TagExists reports whether the given tag is defined in the policy.
TagExists(tag string) bool

View file

@ -972,6 +972,37 @@ func (pm *PolicyManager) NodeCanHaveTag(node types.NodeView, tag string) bool {
return false
}
// UserCanHaveTag reports whether user is authorised to apply tag, i.e. the user
// is one of the tag's owners in the policy (directly or through a group). This
// is the user half of [PolicyManager.NodeCanHaveTag]: it is used to authorise
// tags supplied by an authenticating user (e.g. re-auth with --advertise-tags)
// when the target node is tag-owned and thus carries no user of its own.
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
}
// TagOwnedByTags reports whether a credential holding ownerTags is authorised to
// apply tag. It is true when tag is one of ownerTags, or when tag's tagOwners
// chain (tag-to-tag ownership) transitively includes one of ownerTags. This is

View file

@ -0,0 +1,102 @@
package state
import (
"fmt"
"testing"
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/stretchr/testify/require"
"tailscale.com/tailcfg"
)
// TestTaggedReauthValidatesAgainstAuthUser reproduces issue #3374: an already
// tagged node (tag-owned, so neither UserID nor User is set and its IP is not
// in any tag owner's node set) cannot re-advertise a tag through the auth path
// — even a tag it already holds — when the authenticating user owns that tag.
//
// applyAuthNodeUpdate validates RequestTags against the existing tagged node
// instead of the authenticating user, so both NodeCanHaveTag paths (node IP in
// the tag-owner set, node's own user owns the tag) dead-end. The
// fresh-registration path validates against the authenticating user and works,
// so tagged->tagged re-auth is the only rejected case. The fix must still
// reject tags the authenticating user does not own.
//
// https://github.com/juanfont/headscale/issues/3374
func TestTaggedReauthValidatesAgainstAuthUser(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(t, err)
// tagger owns tag:foo and performs the re-auth. The node is created under a
// different user and then tag-owned, so its IP is not in tagger's node set —
// the realistic tag-owned state, unlike a node that is still user-owned.
tagger := database.CreateUserForTest("tagger")
other := database.CreateUserForTest("other")
node := database.CreateRegisteredNodeForTest(other, "tagged-node")
machineKey := node.MachineKey
nodeID := node.ID
require.NoError(t, database.Close())
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
// Make it genuinely tag-owned: tagged, neither UserID nor User set, as a node
// registered with a tagged pre-auth key ends up. UserID nil routes the re-auth
// through the convert-from-tag lookup.
seeded, ok := s.nodeStore.UpdateNode(nodeID, func(n *types.Node) {
n.Tags = []string{"tag:foo"}
n.UserID = nil
n.User = nil
n.Expiry = nil
})
require.True(t, ok)
require.True(t, seeded.IsTagged(), "precondition: node must be tagged")
// tagger owns tag:foo but not tag:bar.
policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"],"tag:bar":["%s@"]}}`, tagger.Name, other.Name)
_, err = s.SetPolicy([]byte(policy))
require.NoError(t, err)
reauth := func(t *testing.T, tags []string) (types.NodeView, error) {
t.Helper()
regData := &types.RegistrationData{
MachineKey: machineKey,
NodeKey: node.NodeKey,
DiscoKey: node.DiscoKey,
Hostname: "tagged-node",
Hostinfo: &tailcfg.Hostinfo{
Hostname: "tagged-node",
RequestTags: tags,
},
}
authID := types.MustAuthID()
s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(regData))
n, _, err := s.HandleNodeFromAuthPath(authID, types.UserID(tagger.ID), nil, util.RegisterMethodOIDC)
return n, err
}
// tagger owns tag:foo, so re-advertising it must be permitted.
finalNode, err := reauth(t, []string{"tag:foo"})
require.NoError(t, err,
"tag owner must be allowed to re-auth a tagged node re-advertising a tag they own")
require.True(t, finalNode.Valid())
require.True(t, finalNode.IsTagged(),
"node should remain tagged after re-advertising a permitted tag")
require.Contains(t, finalNode.Tags().AsSlice(), "tag:foo")
// tag:bar is owned by other, not tagger, so it must still be rejected — the
// fix authorises the authenticating user, it does not skip authorisation.
_, err = reauth(t, []string{"tag:bar"})
require.Error(t, err,
"tags the authenticating user does not own must still be rejected")
}

View file

@ -1707,7 +1707,15 @@ 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)
//
// #3374: a tag-owned node carries no user of its own, so authorise the
// requested tags against the authenticating user as well (mirroring the
// fresh-registration path, which validates a node already assigned that user).
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",
@ -2028,6 +2036,34 @@ func (s *State) validateRequestTags(node types.NodeView, requestTags []string) [
return rejectedTags
}
// validateRequestTagsForReauth is like validateRequestTags but also authorises
// tags owned by the authenticating user, not just by the node being
// re-authenticated. A tag-owned node has no user of its own, so on re-auth the
// node alone can never authorise a tag; the user completing the interactive
// login is the one who owns it (#3374). A tag is permitted when the existing
// node may hold it OR the authenticating user owns it; unowned tags are still
// rejected.
func (s *State) validateRequestTagsForReauth(node types.NodeView, authUser types.UserView, requestTags []string) []string {
// Empty tags = clear tags, always permitted
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.