Mirrors/juanfont.headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-02-07 20:04:00 +01:00
Code Issues Projects Releases Wiki Activity

state: validate tags before UpdateNode to ensure consistency

Browse Source 
Move tag validation before the UpdateNode callback in applyAuthNodeUpdate.
Previously, tag validation happened inside the callback, and the error
check occurred after UpdateNode had already committed changes to the
NodeStore. This left the NodeStore in an inconsistent state when tags
were rejected.

Now validation happens first, and UpdateNode is only called when we know
the operation will succeed. This follows the principle that UpdateNode
should only be called when we have all information and are ready to commit.

Also extract validateRequestTags as a reusable function and use it in
createAndSaveNewNode to deduplicate the tag validation logic.

Updates #3038
Updates #3048
This commit is contained in:
Kristoffer Dalby 2026-01-28 15:09:27 +00:00
parent df184e5276
commit d7f7f2c85e
1 changed files with 37 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
hscontrol/state/state.go
View File

@ -1146,9 +1146,18 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
oldTags := params.ExistingNode.Tags().AsSlice()
var rejectedTags []string
// Validate tags BEFORE calling UpdateNode to ensure we don't modify NodeStore
// if validation fails. This maintains consistency between NodeStore and database.
rejectedTags := s.validateRequestTags(params.ExistingNode, requestTags)
if len(rejectedTags) > 0 {
return types.NodeView{}, fmt.Errorf(
"%w %v are invalid or not permitted",
ErrRequestedTagsInvalidOrNotPermitted,
rejectedTags,
)
}
// Update existing node in NodeStore
// Update existing node in NodeStore - validation passed, safe to mutate
updatedNodeView, ok := s.nodeStore.UpdateNode(params.ExistingNode.ID(), func(node *types.Node) {
node.NodeKey = params.RegEntry.Node.NodeKey
node.DiscoKey = params.RegEntry.Node.DiscoKey
@ -1178,7 +1187,8 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
wasTagged := node.IsTagged()
// Process tags - may change node.Tags and node.UserID
rejectedTags = s.processReauthTags(node, requestTags, params.User, oldTags)
// Tags were pre-validated, so this will always succeed (no rejected tags)
_ = s.processReauthTags(node, requestTags, params.User, oldTags)
// Handle expiry AFTER tag processing, based on transition
// This ensures expiry is correctly set/cleared based on the NEW tagged status
@ -1217,14 +1227,6 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
return types.NodeView{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, params.ExistingNode.ID())
}
if len(rejectedTags) > 0 {
return types.NodeView{}, fmt.Errorf(
"%w %v are invalid or not permitted",
ErrRequestedTagsInvalidOrNotPermitted,
rejectedTags,
)
}
// Persist to database
// Omit AuthKeyID/AuthKey to prevent stale PreAuthKey references from causing FK errors.
_, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
@ -1357,21 +1359,14 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
// Process RequestTags (from tailscale up --advertise-tags) ONLY for non-PreAuthKey registrations.
// Validate early before IP allocation to avoid resource leaks on failure.
if params.PreAuthKey == nil && params.Hostinfo != nil && len(params.Hostinfo.RequestTags) > 0 {
var approvedTags, rejectedTags []string
for _, tag := range params.Hostinfo.RequestTags {
if s.polMan.NodeCanHaveTag(nodeToRegister.View(), tag) {
approvedTags = append(approvedTags, tag)
} else {
rejectedTags = append(rejectedTags, tag)
}
}
// Reject registration if any requested tags are unauthorized
// Validate all tags before applying - reject if any tag is not permitted
rejectedTags := s.validateRequestTags(nodeToRegister.View(), params.Hostinfo.RequestTags)
if len(rejectedTags) > 0 {
return types.NodeView{}, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, rejectedTags)
}
// All tags are approved - apply them
approvedTags := params.Hostinfo.RequestTags
if len(approvedTags) > 0 {
nodeToRegister.Tags = approvedTags
slices.Sort(nodeToRegister.Tags)
@ -1436,6 +1431,26 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
return s.nodeStore.PutNode(*savedNode), nil
}
// validateRequestTags validates that the requested tags are permitted for the node.
// This should be called BEFORE UpdateNode to ensure we don't modify NodeStore
// if validation fails. Returns the list of rejected tags (empty if all valid).
func (s *State) validateRequestTags(node types.NodeView, 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) {
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.