starred/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-01-12 04:10:32 +01:00
Code Issues 98 Packages Projects Releases 10 Wiki Activity

[Feature] Headscale policy set validate ACL before applying? #758

New Issue
Closed
opened 2025-12-29 02:23:29 +01:00 by adam · 12 comments
adam commented 2025-12-29 02:23:29 +01:00
Owner
Copy Link

Originally created by @dragon2611 on GitHub (Aug 5, 2024).

Use case

Is it possible to have the headscale policy set command run validation before applying the policy and only load the policy if it passed syntax checking.

Description

I accidentally put autogroup:internet instead of autogroup:internet:* and it really didn't like that.

Basically all the clients started throwing back

Could not get the create map update error="strconv.ParseUint: parsing \"internet\": invalid syntax" and I'm pretty sure they all dropped off the tailnet.

Contribution

  • I can write the design doc for this feature
  • I can contribute this feature

How can it be implemented?

No response

Originally created by @dragon2611 on GitHub (Aug 5, 2024). ### Use case Is it possible to have the headscale policy set command run validation before applying the policy and only load the policy if it passed syntax checking. ### Description I accidentally put autogroup:internet instead of autogroup:internet:* and it really didn't like that. Basically all the clients started throwing back `Could not get the create map update error="strconv.ParseUint: parsing \"internet\": invalid syntax" ` and I'm pretty sure they all dropped off the tailnet. ### Contribution - [X] I can write the design doc for this feature - [X] I can contribute this feature ### How can it be implemented? _No response_
adam added the enhancement label 2025-12-29 02:23:29 +01:00
adam closed this issue 2025-12-29 02:23:29 +01:00
adam commented 2025-12-29 02:23:29 +01:00
Author
Owner
Copy Link

@dragon2611 commented on GitHub (Aug 5, 2024):

Also see https://github.com/juanfont/headscale/issues/2045 - I'm happy to contribute but writing design docs is rather outside my area of expertise, and you probably wouldn't want any code contributions from me as I'm very much a novice.

@dragon2611 commented on GitHub (Aug 5, 2024): Also see https://github.com/juanfont/headscale/issues/2045 - I'm happy to contribute but writing design docs is rather outside my area of expertise, and you probably wouldn't want any code contributions from me as I'm very much a novice.
adam commented 2025-12-29 02:23:30 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Aug 6, 2024):

Would make sense, @pallabpain are you able to take a look?

@kradalby commented on GitHub (Aug 6, 2024): Would make sense, @pallabpain are you able to take a look?
adam commented 2025-12-29 02:23:30 +01:00
Author
Owner
Copy Link

@pallabpain commented on GitHub (Aug 6, 2024):

Would make sense, @pallabpain are you able to take a look?

Yes, I'll take a look. @dragon2611 It'd be great if you could share the policy file you were trying to apply.

Although the SetPolicy API does validate the payload 948d53f934/hscontrol/grpcv1.go (L723), maybe it's missing something.

@pallabpain commented on GitHub (Aug 6, 2024): > Would make sense, @pallabpain are you able to take a look? Yes, I'll take a look. @dragon2611 It'd be great if you could share the policy file you were trying to apply. Although the SetPolicy API does validate the payload https://github.com/juanfont/headscale/blob/948d53f934b83f0ca6d4d5007973b334a4ed306a/hscontrol/grpcv1.go#L723, maybe it's missing something.
adam commented 2025-12-29 02:23:30 +01:00
Author
Owner
Copy Link

@dragon2611 commented on GitHub (Aug 6, 2024):

Whilst I'd rather not post the entire ACL The thing that seemed to trip things up was


 { "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet"] },
 { "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet"] }

What appeared to work was however

 { "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet:*"] },
 { "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet:*"] }
@dragon2611 commented on GitHub (Aug 6, 2024): Whilst I'd rather not post the entire ACL The thing that seemed to trip things up was ``` { "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet"] }, { "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet"] } ``` What appeared to work was however ``` { "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet:*"] }, { "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet:*"] } ```
adam commented 2025-12-29 02:23:30 +01:00
Author
Owner
Copy Link

@pallabpain commented on GitHub (Aug 6, 2024):

Whilst I'd rather not post the entire ACL The thing that seemed to trip things up was

{ "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet"] },
{ "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet"] }

What appeared to work was however

{ "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet:"] },
{ "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet:"] }

Thanks. This helps. I'll use this for debugging the issue.

@pallabpain commented on GitHub (Aug 6, 2024): > Whilst I'd rather not post the entire ACL The thing that seemed to trip things up was > > > { "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet"] }, > > { "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet"] } > > What appeared to work was however > > > { "action": "accept", "src": ["tag:mobiles"], "dst": ["autogroup:internet:_"] }, > > { "action": "accept", "src": ["tag:pcs"], "dst": ["autogroup:internet:_"] } Thanks. This helps. I'll use this for debugging the issue.
adam commented 2025-12-29 02:23:30 +01:00
Author
Owner
Copy Link

@dragon2611 commented on GitHub (Aug 6, 2024):

No problem, just be wary github initially parsed it I think as the * wasn't showing.

Was having a bit of problem with the formatting.

@dragon2611 commented on GitHub (Aug 6, 2024): No problem, just be wary github initially parsed it I think as the * wasn't showing. Was having a bit of problem with the formatting.
adam commented 2025-12-29 02:23:31 +01:00
Author
Owner
Copy Link

@pallabpain commented on GitHub (Aug 6, 2024):

No problem, just be wary github initially parsed it I think as the * wasn't showing.

Was having a bit of problem with the formatting.

So the issue is that while the policy may be a valid HuJSON/JSON that can be marshaled into the policy struct, it may not have the right values expected in the policy. The current code only ensures format.

@pallabpain commented on GitHub (Aug 6, 2024): > No problem, just be wary github initially parsed it I think as the * wasn't showing. > > Was having a bit of problem with the formatting. So the issue is that while the policy may be a valid HuJSON/JSON that can be marshaled into the policy struct, it may not have the right values expected in the policy. The current code only ensures format.
adam commented 2025-12-29 02:23:31 +01:00
Author
Owner
Copy Link

@dragon2611 commented on GitHub (Aug 6, 2024):

I'm guessing the tailscale client didn't like the lack of a port definition, which is weird as for no TCP/UDP traffic there wouldn't be a port anyways.

@dragon2611 commented on GitHub (Aug 6, 2024): I'm guessing the tailscale client didn't like the lack of a port definition, which is weird as for no TCP/UDP traffic there wouldn't be a port anyways.
adam commented 2025-12-29 02:23:31 +01:00
Author
Owner
Copy Link

@pallabpain commented on GitHub (Aug 6, 2024):

I'm guessing the tailscale client didn't like the lack of a port definition, which is weird as for no TCP/UDP traffic there wouldn't be a port anyways.

Per Tailscale docs, the port is expected.
https://tailscale.com/kb/1337/acl-syntax#dst

@pallabpain commented on GitHub (Aug 6, 2024): > I'm guessing the tailscale client didn't like the lack of a port definition, which is weird as for no TCP/UDP traffic there wouldn't be a port anyways. Per Tailscale docs, the port is expected. https://tailscale.com/kb/1337/acl-syntax#dst
adam commented 2025-12-29 02:23:31 +01:00
Author
Owner
Copy Link

@pallabpain commented on GitHub (Aug 6, 2024):

Would make sense, @pallabpain are you able to take a look?

@kradalby I couldn't find anything in the tailscale client that I can use to validate the policy. Looks like it has to be implemented in headscale.

@pallabpain commented on GitHub (Aug 6, 2024): > Would make sense, @pallabpain are you able to take a look? @kradalby I couldn't find anything in the tailscale client that I can use to validate the policy. Looks like it has to be implemented in headscale.
adam commented 2025-12-29 02:23:31 +01:00
Author
Owner
Copy Link

@oneingan commented on GitHub (Aug 6, 2024):

Slightly offtopic but in our company we have integration tests for ACL changes using NixOS VMs based testing framework. The misconfiguration were a coomon thing before that. So I can imagine how proper validation tests could help a lot with ACLs.

@oneingan commented on GitHub (Aug 6, 2024): Slightly offtopic but in our company we have integration tests for ACL changes using NixOS VMs based testing framework. The misconfiguration were a coomon thing before that. So I can imagine how proper validation tests could help a lot with ACLs.
adam commented 2025-12-29 02:23:31 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Aug 30, 2024):

I think this is being solved, or at least improved in #2089

@kradalby commented on GitHub (Aug 30, 2024): I think this is being solved, or at least improved in #2089
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
CLI
DERP
DNS
Nix
OIDC
SSH
bug
database
documentation
duplicate
enhancement
faq
good first issue
grants
help wanted
might-come
needs design doc
needs investigation
no-stale-bot
out of scope
performance
policy 📝
pull-request
Mirrored from GitHub Pull Request
 question
regression
routes
stale
tags
tailscale-feature-gap
well described ❤️
wontfix
No Label enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#758
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?