mirror of
https://github.com/juanfont/headscale.git
synced 2026-01-13 04:40:29 +01:00
Add "x-tailnet" ACL src/dst type #666
Closed
opened 2025-12-29 02:21:46 +01:00 by adam
·
10 comments
No Branch/Tag Specified
main
update_flake_lock_action
gh-pages
kradalby/release-v0.27.2
dependabot/go_modules/golang.org/x/crypto-0.45.0
dependabot/go_modules/github.com/opencontainers/runc-1.3.3
copilot/investigate-headscale-issue-2788
copilot/investigate-visibility-issue-2788
copilot/investigate-issue-2833
copilot/debug-issue-2846
copilot/fix-issue-2847
dependabot/go_modules/github.com/go-viper/mapstructure/v2-2.4.0
dependabot/go_modules/github.com/docker/docker-28.3.3incompatible
kradalby/cli-experiement3
doc/0.26.1
doc/0.25.1
doc/0.25.0
doc/0.24.3
doc/0.24.2
doc/0.24.1
doc/0.24.0
kradalby/build-docker-on-pr
topic/docu-versioning
topic/docker-kos
juanfont/fix-crash-node-id
juanfont/better-disclaimer
update-contributors
topic/prettier
revert-1893-add-test-stage-to-docs
add-test-stage-to-docs
remove-node-check-interval
fix-empty-prefix
fix-ephemeral-reusable
bug_report-debuginfo
autogroups
logs-to-stderr
revert-1414-topic/fix_unix_socket
rename-machine-node
port-embedded-derp-tests-v2
port-derp-tests
duplicate-word-linter
update-tailscale-1.36
warn-against-apache
ko-fi-link
more-acl-tests
fix-typo-standalone
parallel-nolint
tparallel-fix
rerouting
ssh-changelog-docs
oidc-cleanup
web-auth-flow-tests
kradalby-gh-runner
fix-proto-lint
remove-funding-links
go-1.19
enable-1.30-in-tests
0.16.x
cosmetic-changes-integration
tmp-fix-integration-docker
fix-integration-docker
configurable-update-interval
show-nodes-online
hs2021
acl-syntax-fixes
ts2021-implementation
fix-spurious-updates
unstable-integration-tests
mandatory-stun
embedded-derp
prtemplate-fix
v0.28.0-beta.1
v0.27.2-rc.1
v0.27.1
v0.27.0
v0.27.0-beta.2
v0.27.0-beta.1
v0.26.1
v0.26.0
v0.26.0-beta.2
v0.26.0-beta.1
v0.25.1
v0.25.0
v0.25.0-beta.2
v0.24.3
v0.25.0-beta.1
v0.24.2
v0.24.1
v0.24.0
v0.24.0-beta.2
v0.24.0-beta.1
v0.23.0
v0.23.0-rc.1
v0.23.0-beta.5
v0.23.0-beta.4
v0.23.0-beta3
v0.23.0-beta2
v0.23.0-beta1
v0.23.0-alpha12
v0.23.0-alpha11
v0.23.0-alpha10
v0.23.0-alpha9
v0.23.0-alpha8
v0.23.0-alpha7
v0.23.0-alpha6
v0.23.0-alpha5
v0.23.0-alpha4
v0.23.0-alpha4-docker-ko-test9
v0.23.0-alpha4-docker-ko-test8
v0.23.0-alpha4-docker-ko-test7
v0.23.0-alpha4-docker-ko-test6
v0.23.0-alpha4-docker-ko-test5
v0.23.0-alpha-docker-release-test-debug2
v0.23.0-alpha-docker-release-test-debug
v0.23.0-alpha4-docker-ko-test4
v0.23.0-alpha4-docker-ko-test3
v0.23.0-alpha4-docker-ko-test2
v0.23.0-alpha4-docker-ko-test
v0.23.0-alpha3
v0.23.0-alpha2
v0.23.0-alpha1
v0.22.3
v0.22.2
v0.23.0-alpha-docker-release-test
v0.22.1
v0.22.0
v0.22.0-alpha3
v0.22.0-alpha2
v0.22.0-alpha1
v0.22.0-nfpmtest
v0.21.0
v0.20.0
v0.19.0
v0.19.0-beta2
v0.19.0-beta1
v0.18.0
v0.18.0-beta4
v0.18.0-beta3
v0.18.0-beta2
v0.18.0-beta1
v0.17.1
v0.17.0
v0.17.0-beta5
v0.17.0-beta4
v0.17.0-beta3
v0.17.0-beta2
v0.17.0-beta1
v0.17.0-alpha4
v0.17.0-alpha3
v0.17.0-alpha2
v0.17.0-alpha1
v0.16.4
v0.16.3
v0.16.2
v0.16.1
v0.16.0
v0.16.0-beta7
v0.16.0-beta6
v0.16.0-beta5
v0.16.0-beta4
v0.16.0-beta3
v0.16.0-beta2
v0.16.0-beta1
v0.15.0
v0.15.0-beta6
v0.15.0-beta5
v0.15.0-beta4
v0.15.0-beta3
v0.15.0-beta2
v0.15.0-beta1
v0.14.0
v0.14.0-beta2
v0.14.0-beta1
v0.13.0
v0.13.0-beta3
v0.13.0-beta2
v0.13.0-beta1
upstream/v0.12.4
v0.12.4
v0.12.3
v0.12.2
v0.12.2-beta1
v0.12.1
v0.12.0-beta2
v0.12.0-beta1
v0.11.0
v0.10.8
v0.10.7
v0.10.6
v0.10.5
v0.10.4
v0.10.3
v0.10.2
v0.10.1
v0.10.0
v0.9.3
v0.9.2
v0.9.1
v0.9.0
v0.8.1
v0.8.0
v0.7.1
v0.7.0
v0.6.1
v0.6.0
v0.5.2
v0.5.1
v0.5.0
v0.4.0
v0.3.6
v0.3.5
v0.3.4
v0.3.3
v0.3.2
v0.3.1
v0.3.0
v0.2.2
v0.2.1
v0.2.0
v0.1.1
v0.1.0
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
question
regression
routes
stale
tags
tailscale-feature-gap
well described ❤️
wontfix
Mirrored from GitHub Pull Request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: starred/headscale#666
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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?
Originally created by @cdhowie on GitHub (Mar 11, 2024).
There's a bit to unpack before I get to the proposal. This is all written from the perspective of 0.22.3.
The problem
Headscale currently implements a concept called "users" which are related to the Tailscale concepts of "tailnet" and "user" but aren't exactly equivalent to either of them. Because they aren't exactly equivalent, headscale treats users as both tailnets and users. However, it does so differently in some contexts than it does in others, and this creates confusion regarding what exactly should happen in ACLs.
Essentially, the concept "user" is overloaded in headscale, which leads to conflation of the two concepts in confusing and surprising ways. This manifests in how headscale understands by-user ACL src/dst rules: headscale considers a rule matching a user to match any node with that user unless the node has (correct or forced) tags in which case the rule does not match. Presumably, this is because Tailscale does not allow a node to have both a user and tags. So this is not a valid way to say "all nodes in a specific tailnet" in headscale ACLs, because tagged nodes get excluded.
In Tailscale, we can express "a tailnet" as
*because ACLs do not span tailnets. In headscale, we cannot use*for this purpose --*means "all nodes in all tailnets" because headscale ACLs span tailnets! So this also isn't a valid way to say "all nodes in a specific tailnet" in headscale ACLs.The consequence of both of these things together is that the src/dst rule "all nodes in a specific tailnet" is impossible to express in headscale ACLs! Since headscale ACLs span tailnets, I think this is a very important rule type to have available.
Possible solutions
How do we fix this? There are a few options each with their own advantages and disadvantages.
Do nothing and require workarounds
One possible workaround is to simply tag all nodes in a tailnet with the same tag. This works, but has significant management overhead and is prone to errors (typos in tag names, forgetting to tag a node, etc.).
👍 No code changes needed.
👎 Very bad user experience for those needing this kind of ACL rule.
Accept that headscale doesn't have Tailscale users
Simply, remove the check for whether a node has correct/forced tags when evaluating a user rule. Headscale doesn't really have Tailscale users at all, so we simply repurpose the user-based ACLs to apply to tailnets.
👍 This treats users in headscale more in line with what they actually are (tailnets).
👍 The code changes are very simple.
👎 This is not backwards-compatible; existing ACLs will change their meaning.
👎 This uses an existing Tailscale rule syntax for something different. (Counter-argument: The rule type in Tailscale does not really even make sense in headscale.)
Same, but opt in to this behavior
Add a new configuration switch either in the headscale config file or in the ACL file itself that turns on or off the behavior of excluding tagged nodes from user-based ACL rules.
👍 Backwards-compatible; can preserve the existing behavior by default.
👎 All-or-nothing: you can't mix both behaviors in the same ACL.
👎 Code changes are more complicated, and both behaviors need to be supported.
Introduce a new src/dst rule type
Add a new prefixed rule type that matches nodes by their user without regard to their tags. I propose
x-tailnet:footo mean "all nodes assigned to userfoo.The
x-prefix is unlikely to be used by Tailscale, and so shouldn't conflict with new types Tailscale may add later. It also is a well-known way to express nonstandard extensions to an existing standard (for example, in HTTP headers).👍 Backwards-compatible.
👍 Allows specifying both kinds of rules in the same ACL.
👎 Code changes are more complicated, and both behaviors need to be supported.
Proposal
I propose adding the new src/dst
x-tailnet:rule type. This seems to be the most ergonomic solution while also being both backwards-compatible and reasonably future-proof.@apollo13 commented on GitHub (Mar 11, 2024):
But isn't the goal of headscale to solely provide a single tailnet?
Can you elaborate on that?
This is exactly what tailscale itself does as well, no? Ie once you accept that headscale users are to match tailscale users then this behavior seems natural?
@cdhowie commented on GitHub (Mar 11, 2024):
Possibly, but this is not how headscale's implementation of Magic DNS behaves. In Tailscale, each user does not get their own Magic DNS domain, but they do in headscale. This implies that headscale users behave at least somewhat like tailnets.
That is to say, if the Magic DNS tailnet root is
example.comand we have a nodefoounder a userbar, in Tailscale we would havefoo.example.combut in headscale we havefoo.bar.example.com.Especially since a node should have tags or a user, it seems very odd to me that once we've tagged a node, the node not only still has a user, but the user is used in the Magic DNS path. This is certainly confusing, to say the least.
IMO headscale users should act like Tailscale users or Tailscale tailnets. Right now it's a mixture of both. I don't care much which way headscale wants to go, but I do think it needs to pick one and not try to mix both concepts together.
This would be easier to accept without the Magic DNS property above, which (to me) implies something different.
@Hacksawfred3232 commented on GitHub (Mar 17, 2024):
IMO, Headscale at it's current state treats users as just users. As to this, the MagicDNS names that are generated serve nothing more than an indicator of who owns which node.
As far as I can tell, adding a "x-tailnet" src/dst type will be fundmentally adding bloat, as the functionality is already implemented. I'm also failing to see where this "user unless tagged" issue is occuring, as in my setup, my exit node is currently tagged with a corrosponding name, and that is to allow auto-approval of exit-nodes. I still have connectivity to that node, even if using another user or not. So I am unable to replicate this issue you speak of, and can not vouch in good faith for such an enhancement.
@cdhowie commented on GitHub (Mar 17, 2024):
@Hacksawfred3232 It sounds like you might fundamentally misunderstand the issue I'm raising. I'm not exactly sure that adding a new ACL rule type is the solution (it was the best solution I could come up with at the time I opened the issue), but there are some pretty major differences between Tailscale users and Headscale users. Here is my understanding:
fooand has no tags, then an ACL rule for userfoomatches the node. If the node has any tags then an ACL rule for usingfoowill no longer match the node. This is explicitly handled in the code; it is not a bug, but intended behavior -- however, this behavior does not appear to be documented anywhere. It appears to be an attempt to match the Tailscale behavior that a node cannot both have an owning user and also have tags. This would be fine -- except the node's owning username still appears in the Magic DNS path!The fact that the node's owning username appears in the Magic DNS path is the biggest source of confusion. Tailscale doesn't do this except in the case where nodes are on different tailnets. Ergo, it seems like Headscale users behave kind of like tailnets. Because Headscale users don't actually allow direct authentication by the person adding a new device, they also don't seem to really behave like Tailscale users in any meaningful way.
The solution we've settled on is to simply put all nodes under one user, because we cannot find any use for Headscale users at all. They only seem to add confusion due to their numerous behavioral differences from Tailscale users.
@Hacksawfred3232 commented on GitHub (Mar 17, 2024):
This is not the case. If you perform a login through OIDC (Open ID Connect), and the user in question does not exist yet, the user is automatically generated in Headscale. If using a managed backend for users (such as AD + Keycloak with registration disabled), then ACL rules can be setup for "non-existing" users that will exist upon login, and Headscale will still load the ACL. Any user that then logs in for the first time will automatically have their ACL rules applied.
As far as I can tell, this difference in behaviour makes up for the fact that Headscale is not a multi-tailnet system, in contrast to Tailscale's own system. At the end of the day, this bares no difference on what appears to be a ACL aplication issue. Speaking of which...
The function that it references is: https://github.com/juanfont/headscale/blob/v0.23.0-alpha5/hscontrol/policy/acls.go#L598
However, like I said in my comment before, it appears this behaviour isn't triggering for me. And I can see why. In my ACL file, I have declared users to be part of groups, and then I explicitly have rules targeting those groups. This triggers this function here: https://github.com/juanfont/headscale/blob/v0.23.0-alpha5/hscontrol/policy/acls.go#L756
Which doesn't trigger the function at L598 in the version I'm using (v0.23.0-alpha5). Thus, tag-filtering does not apply. And hence why adding such a function is fudmentally redundant. If you explicitly need such an identifier, then consider using the group alias.
Adding more bloat to ACL handling logic risks introducing software/security breaking bugs. As such, I'm against the proposed solution, and rather in favour of "Do nothing and require workarounds", since the workaround is nothing more than a new set of groups within the ACL, barring any current issues with exit-nodes and expansion of IP routes, which the Headscale dev team are aware of.
This conclusion you make relies on false assumptions. Instead of not experimenting and just clinging to one line of code - when it was clear that an alternative solution exists - trial and error would have let you discovered that actually it is possible to work around the issue in a trivial way. Yes, It will be more maintence, but that is the cost of security.
@cdhowie commented on GitHub (Mar 17, 2024):
This entire paragraph is irrelevant to those of us not using OIDC. Is the takeaway supposed to be "users don't have a purpose outside of OIDC?"
So you agree then that Headscale users behave kind of like tailnets. This is exactly my point!
I found the same workaround during my tests, but I do not believe this is a viable workaround. I very strongly suspect that this is actually a bug! If adding a tag to a node causes the effect that it doesn't match user rules, it seems logical to me that it shouldn't belong to ACL groups that user is a part of, either. Therefore, I don't think this workaround will exist once this defect is corrected.
And if it's not actually considered to be a bug, this makes the current situation even more confused.
(We even discussed patching our build of Headscale to remove the exclusion of tagged nodes from users -- which IMO is a better solution than the groups hack -- but decided that it's far simpler to just put everything under one user, since users don't provide the ability to unilaterally add nodes without administrator involvement anyway.)
Honestly, this is a rather pointed mischaracterization of my opinion. I'm not "clinging to one line of code," I'm providing evidence for my opinion that Headscale users behave both like Tailscale users and Tailscale tailnets, an opinion you actually appear to agree with based on the section of your reply I quoted above.
I've already conceded that adding a new ACL type might not be the best solution, which appears to be the thing you're "clinging to." So let's agree that might not be the best solution, and move on from that idea.
My point is simply this: there needs to be some resolution to the fact that Headscale users are confused in their identity, behaving like users in one context and like tailnets in another.
To be absolutely clear, I'm not invested in any specific solution, but there definitely needs to be a solution because the current user concept is too overloaded in its meaning.
@Hacksawfred3232 commented on GitHub (Mar 17, 2024):
Okay... @cdhowie since you're so inclined in trying to twist other peoples words... Please see: https://en.wikipedia.org/wiki/Straw_man
What you just did is called making a straw-man argument. You're claiming that I "agreed" with you on the fact that "Headscale users" are the same as "Tailscale tailnets". I did not make such an agreement. I actually disagreed with your statement that "Headscale users" are like "Tailscale tailnets", and rather asserted that Headscale users are just users. When I declared that Headscale is "not a multi-tailnet" system, this explicitly stated that "Such a concept can not exist in Headscale, and declaring that users are Tailnets is a fallacy". And yet you claimed otherwise, despite my explicitly asserting the previous two times.
You also appeared to try and re-enforce your previous argument based on this false claim you persue. You claim that the situation is confusing, yet it appears you either do not understand, or are trying to intetionally waste people's time.
I will not continue this conversation, as you refuse to actually acknowledge what is said.
@cdhowie commented on GitHub (Mar 17, 2024):
I likewise think continued discussion between us on this topic will not be productive. I'm not terribly fond of the combative tone you've taken from your very first comment and simply think that we won't see eye to eye on this issue. That's fine, but claiming that I might be intentionally trying to waste people's time is entirely unnecessary and does absolutely nothing to contribute to the discussion.
In summary, I do not think nor have I ever claimed that Headscale users are tailnets, only that they appear to be an amalgam of both users and tailnets in different ways, and this results in confusion with regard to how they act in ACLs.
The inconsistentcy between how user rules and group rules work when a node is tagged, in particular, seems like it could be a bug.
@github-actions[bot] commented on GitHub (Jun 16, 2024):
This issue is stale because it has been open for 90 days with no activity.
@github-actions[bot] commented on GitHub (Jun 24, 2024):
This issue was closed because it has been inactive for 14 days since being marked as stale.