Merge 62279d469d into 048308511c

CHANGELOG.md

@@ -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)
 

hscontrol/policy/pm.go

@@ -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
 

hscontrol/policy/v2/policy.go

@@ -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

hscontrol/state/issue_3374_test.go

@@ -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")
+}

hscontrol/state/state.go

@@ -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.