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

[Bug] Subnet routes are pushed to clients when not in allowed ACL #918

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

Originally created by @Nathanael-Mtd on GitHub (Jan 22, 2025).

Is this a support request?

  • This is not a support request

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

When we allow only one route from subnet router node for clients, all routes from that subnet router nodes are pushed to clients.

Expected Behavior

Only push routes allowed by ACLs, not every subnet node router routes.

Steps To Reproduce

  1. Add a node and announce multiple subnet (example : 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24)
  2. Add ACL to only allow one subnet 10.10.12.0/24 for a client
  3. Check routing table from client (ip route show table 52 on linux) and all routes should appear

Environment

- OS: Ubuntu 24.04 for Headscale server and clients (but same issue for Windows clients)
- Headscale version: 0.24.0
- Tailscale version: 1.78.1

Runtime environment

  • Headscale is behind a (reverse) proxy
  • Headscale runs in a container

Anything else?

Netmap dump from the client with tag:headscale (cleaned and redacted for some parts) : netmap-hs.json

ACL config (important parts) : acl-redacted.json

Routes output :

user@ks-headscale:~$ ip route show table 52
10.108.0.0/24 dev tailscale0
10.108.1.0/24 dev tailscale0
10.108.2.0/24 dev tailscale0
REDACTED.66 dev tailscale0
REDACTED.67 dev tailscale0
100.64.0.1 dev tailscale0
100.64.0.4 dev tailscale0
100.64.0.9 dev tailscale0
100.64.0.11 dev tailscale0
100.64.0.16 dev tailscale0
100.64.0.27 dev tailscale0
100.64.0.68 dev tailscale0
100.100.100.100 dev tailscale0

The 5 first routes you can see is the ones announced from subnet router node, ACL allow only trafic to REDACTED.67 but other routes are present.

Originally created by @Nathanael-Mtd on GitHub (Jan 22, 2025). ### Is this a support request? - [x] This is not a support request ### Is there an existing issue for this? - [x] I have searched the existing issues ### Current Behavior When we allow only one route from subnet router node for clients, all routes from that subnet router nodes are pushed to clients. ### Expected Behavior Only push routes allowed by ACLs, not every subnet node router routes. ### Steps To Reproduce 1. Add a node and announce multiple subnet (example : 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24) 2. Add ACL to only allow one subnet 10.10.12.0/24 for a client 3. Check routing table from client (`ip route show table 52` on linux) and all routes should appear ### Environment ```markdown - OS: Ubuntu 24.04 for Headscale server and clients (but same issue for Windows clients) - Headscale version: 0.24.0 - Tailscale version: 1.78.1 ``` ### Runtime environment - [x] Headscale is behind a (reverse) proxy - [x] Headscale runs in a container ### Anything else? Netmap dump from the client with tag:headscale (cleaned and redacted for some parts) : [netmap-hs.json](https://github.com/user-attachments/files/18505341/netmap-hs.json) ACL config (important parts) : [acl-redacted.json](https://github.com/user-attachments/files/18505346/acl-redacted.json) Routes output : ``` user@ks-headscale:~$ ip route show table 52 10.108.0.0/24 dev tailscale0 10.108.1.0/24 dev tailscale0 10.108.2.0/24 dev tailscale0 REDACTED.66 dev tailscale0 REDACTED.67 dev tailscale0 100.64.0.1 dev tailscale0 100.64.0.4 dev tailscale0 100.64.0.9 dev tailscale0 100.64.0.11 dev tailscale0 100.64.0.16 dev tailscale0 100.64.0.27 dev tailscale0 100.64.0.68 dev tailscale0 100.100.100.100 dev tailscale0 ``` The 5 first routes you can see is the ones announced from subnet router node, ACL allow only trafic to REDACTED.67 but other routes are present.
adam added the bugno-stale-bot labels 2025-12-29 02:25:56 +01:00
adam closed this issue 2025-12-29 02:25:56 +01:00
adam commented 2025-12-29 02:25:56 +01:00
Author
Owner
Copy Link

@github-actions[bot] commented on GitHub (Apr 23, 2025):

This issue is stale because it has been open for 90 days with no activity.

@github-actions[bot] commented on GitHub (Apr 23, 2025): This issue is stale because it has been open for 90 days with no activity.
adam commented 2025-12-29 02:25:57 +01:00
Author
Owner
Copy Link

@cv-prod-github commented on GitHub (Apr 25, 2025):

Hello,

i think here should also be considered, that only subnet routes, that are not local to the node should be pushed.
When i use my local notebook, that has tailscale installed, it always prioritize the routes of tailscale, no matter the metric. So if im in the local network it still doesn't communicate directly and hops over the tailscale link.

@cv-prod-github commented on GitHub (Apr 25, 2025): Hello, i think here should also be considered, that only subnet routes, that are not local to the node should be pushed. When i use my local notebook, that has tailscale installed, it always prioritize the routes of tailscale, no matter the metric. So if im in the local network it still doesn't communicate directly and hops over the tailscale link.
adam commented 2025-12-29 02:25:57 +01:00
Author
Owner
Copy Link

@Nathanael-Mtd commented on GitHub (Apr 25, 2025):

Hello,

i think here should also be considered, that only subnet routes, that are not local to the node should be pushed.
When i use my local notebook, that has tailscale installed, it always prioritize the routes of tailscale, no matter the metric. So if im in the local network it still doesn't communicate directly and hops over the tailscale link.

I think it's out of Headscale scope, because Headscale don't know (and don't need to know) about your local attached node networks.
You should search if there are an issue on Tailscale client project about that.

@Nathanael-Mtd commented on GitHub (Apr 25, 2025): > Hello, > > i think here should also be considered, that only subnet routes, that are not local to the node should be pushed. > When i use my local notebook, that has tailscale installed, it always prioritize the routes of tailscale, no matter the metric. So if im in the local network it still doesn't communicate directly and hops over the tailscale link. I think it's out of Headscale scope, because Headscale don't know (and don't need to know) about your local attached node networks. You should search if there are an issue on Tailscale client project about that.
adam commented 2025-12-29 02:25:58 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Apr 29, 2025):

Just so I understand correctly, the two concern for why this would be desired would be:

  • If a node does not have access to a subnet, why should it know about it (akin to peer trimming)
    • There is no security concern per say as they policy should drop it and they wont work, just confusing.
  • If a node gets the route from Headscale and it does not have access to it and it also has the local range available.

I imagine this can be implemented in a similar way to the peer trimming, it might be somewhat computational expensive, but that is traditionally not something we think about until later.

@kradalby commented on GitHub (Apr 29, 2025): Just so I understand correctly, the two concern for why this would be desired would be: - If a node does not have access to a subnet, why should it know about it (akin to peer trimming) - There is no security concern per say as they policy should drop it and they wont work, just confusing. - If a node gets the route from Headscale _and_ it does not have access to it _and_ it also has the local range available. I imagine this can be implemented in a similar way to the peer trimming, it might be somewhat computational expensive, but that is traditionally not something we think about until later.
adam commented 2025-12-29 02:25:58 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Apr 29, 2025):

I would be interested to hear what upstream does here, if they dont trim this, I do not think we will implement it. If they do, then it seems worthwhile to continue our goal of parity.

@kradalby commented on GitHub (Apr 29, 2025): I would be interested to hear what upstream does here, if they dont trim this, I do not think we will implement it. If they do, then it seems worthwhile to continue our goal of parity.
adam commented 2025-12-29 02:25:58 +01:00
Author
Owner
Copy Link

@Nathanael-Mtd commented on GitHub (Apr 29, 2025):

Upstream Tailscale only push allowed routes, I just tested it.

Before (with dst *) :
image

After (only one subnet allowed) :
image

@Nathanael-Mtd commented on GitHub (Apr 29, 2025): Upstream Tailscale only push allowed routes, I just tested it. Before (with dst *) : ![image](https://github.com/user-attachments/assets/5e2b4ca9-7b8a-4d03-953f-29c72ddc1161) After (only one subnet allowed) : ![image](https://github.com/user-attachments/assets/a1c61bdf-3f71-4084-8d6d-192fe9f71514)
adam commented 2025-12-29 02:25:58 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Apr 29, 2025):

So we need something like what we are doing here for routes: https://github.com/juanfont/headscale/blob/main/hscontrol/policy/policy.go#L27

@kradalby commented on GitHub (Apr 29, 2025): So we need something like what we are doing here for routes: https://github.com/juanfont/headscale/blob/main/hscontrol/policy/policy.go#L27
adam commented 2025-12-29 02:25:58 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (May 3, 2025):

Can you give https://github.com/juanfont/headscale/pull/2561 a go?

@kradalby commented on GitHub (May 3, 2025): Can you give https://github.com/juanfont/headscale/pull/2561 a go?
adam commented 2025-12-29 02:25:58 +01:00
Author
Owner
Copy Link

@nblock commented on GitHub (May 4, 2025):

Can you give #2561 a go?

Tested 1929942bcf2362ae7704b273cdc7c25b6c7112f6 (HEAD of #2561) with the following behavior:

Setup

$ headscale nodes list
ID | Hostname | Name   | MachineKey | NodeKey | User | IP addresses                  | Ephemeral | Last seen | Expiration | Connected | Expired
1  | router   | router | [mSe6U]    | [0/7M8] | user | 100.64.0.1, fd7a:115c:a1e0::1 | false     |           | N/A        | online    | no
2  | node     | client | [UswUo]    | [x8J3n] | user | 100.64.0.2, fd7a:115c:a1e0::2 | false     |           | N/A        | online    | no

$ headscale nodes list-routes
ID | Hostname | Approved                                    | Available                                   | Serving (Primary)
1  | router   | 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24 | 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24 | 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24

The router announces three routes:

  • 10.10.10.0/24 (reachable for the client per policy)
  • 10.10.11.0/24 (unreachable)
  • 10.10.12.0/24 (unreachable)
Setup dummy service for access tests 
$ ip link add dummy0 type dummy
$ ip addr add 10.10.10.1/24 dev dummy0
$ ip link set dummy0 up

$ ip link add dummy1 type dummy
$ ip addr add 10.10.11.1/24 dev dummy1
$ ip link set dummy1 up

$ ip link add dummy2 type dummy
$ ip addr add 10.10.12.1/24 dev dummy2
$ ip link set dummy2 up

$ python3 -m http.server

Policy without any router related rules

  • The policy allows access to a single network 10.10.10.0/24
  • No router related rules
  • Result: OK, only the allowed route is pushed to the client.

Policy:

{
  "acls": [
    {
      "action": "accept",
      "src": [
        "*"
      ],
      "dst": [
        "10.10.10.0/24:*"
      ]
    }
  ]
}

Routing table on the client:

$ ip route show table 52
10.10.10.0/24 dev tailscale0
100.64.0.1 dev tailscale0
100.100.100.100 dev tailscale0

Access checks:

URL Accessible Result
http://100.64.0.1:8000 No OK
http://10.10.10.1:8000 Yes OK
http://10.10.11.1:8000 No OK
http://10.10.12.1:8000 No OK

Policy with some router related rules

  • The policy allows access to a single network 10.10.10.0/24
  • Either ping access to the router or regular router access rule
  • Result: Not OK, no routes are pushed to the client

Policy:

{
  "hosts": {
    "router": "100.64.0.1/32",
    "client": "100.64.0.2/32",
  },
  "acls": [
    {
      "action": "accept",
      "src": [
        "*"
      ],
      "dst": [
        "router:8000"
        // or:   "router:0"
      ]
    },
    {
      "action": "accept",
      "src": [
        "client"
      ],
      "dst": [
        "10.10.10.0/24:*"
      ]
    }
  ]
}

Routing table on the client:

$ ip route show table 52
100.64.0.1 dev tailscale0
100.100.100.100 dev tailscale0

Access checks:

URL Accessible Result
http://100.64.0.1:8000 Yes OK
http://10.10.10.1:8000 No NOT OK
http://10.10.11.1:8000 No OK
http://10.10.12.1:8000 No OK
@nblock commented on GitHub (May 4, 2025): > Can you give [#2561](https://github.com/juanfont/headscale/pull/2561) a go? Tested 1929942bcf2362ae7704b273cdc7c25b6c7112f6 (HEAD of #2561) with the following behavior: ## Setup ```console $ headscale nodes list ID | Hostname | Name | MachineKey | NodeKey | User | IP addresses | Ephemeral | Last seen | Expiration | Connected | Expired 1 | router | router | [mSe6U] | [0/7M8] | user | 100.64.0.1, fd7a:115c:a1e0::1 | false | | N/A | online | no 2 | node | client | [UswUo] | [x8J3n] | user | 100.64.0.2, fd7a:115c:a1e0::2 | false | | N/A | online | no ``` ```console $ headscale nodes list-routes ID | Hostname | Approved | Available | Serving (Primary) 1 | router | 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24 | 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24 | 10.10.10.0/24, 10.10.11.0/24, 10.10.12.0/24 ``` The router announces three routes: - 10.10.10.0/24 (reachable for the client per policy) - 10.10.11.0/24 (unreachable) - 10.10.12.0/24 (unreachable) <details> <summary>Setup dummy service for access tests</summary> ```console $ ip link add dummy0 type dummy $ ip addr add 10.10.10.1/24 dev dummy0 $ ip link set dummy0 up $ ip link add dummy1 type dummy $ ip addr add 10.10.11.1/24 dev dummy1 $ ip link set dummy1 up $ ip link add dummy2 type dummy $ ip addr add 10.10.12.1/24 dev dummy2 $ ip link set dummy2 up $ python3 -m http.server ``` </details> ## Policy without any router related rules * The policy allows access to a single network `10.10.10.0/24` * No router related rules * Result: OK, only the allowed route is pushed to the client. Policy: ```json5 { "acls": [ { "action": "accept", "src": [ "*" ], "dst": [ "10.10.10.0/24:*" ] } ] } ``` Routing table on the client: ```console $ ip route show table 52 10.10.10.0/24 dev tailscale0 100.64.0.1 dev tailscale0 100.100.100.100 dev tailscale0 ``` Access checks: | URL | Accessible | Result | | --- | ---------- | ------ | | http://100.64.0.1:8000 | No | OK | | http://10.10.10.1:8000 | Yes | OK | | http://10.10.11.1:8000 | No | OK | | http://10.10.12.1:8000 | No | OK | ## Policy with some router related rules * The policy allows access to a single network `10.10.10.0/24` * Either ping access to the router or regular router access rule * Result: Not OK, no routes are pushed to the client Policy: ```json5 { "hosts": { "router": "100.64.0.1/32", "client": "100.64.0.2/32", }, "acls": [ { "action": "accept", "src": [ "*" ], "dst": [ "router:8000" // or: "router:0" ] }, { "action": "accept", "src": [ "client" ], "dst": [ "10.10.10.0/24:*" ] } ] } ``` Routing table on the client: ```console $ ip route show table 52 100.64.0.1 dev tailscale0 100.100.100.100 dev tailscale0 ``` Access checks: | URL | Accessible | Result | | --- | ---------- | ------ | | http://100.64.0.1:8000 | Yes | OK | | http://10.10.10.1:8000 | No | NOT OK | | http://10.10.11.1:8000 | No | OK | | http://10.10.12.1:8000 | No | OK |
adam commented 2025-12-29 02:25:59 +01:00
Author
Owner
Copy Link

@Nathanael-Mtd commented on GitHub (May 4, 2025):

@nblock Thank you for the test, but I don't understand the "Policy with some router related rules" test, and the difference with the first one (and why it don't work).

@Nathanael-Mtd commented on GitHub (May 4, 2025): @nblock Thank you for the test, but I don't understand the "Policy with some router related rules" test, and the difference with the first one (and why it don't work).
adam commented 2025-12-29 02:25:59 +01:00
Author
Owner
Copy Link

@nblock commented on GitHub (May 4, 2025):

@nblock Thank you for the test, but I don't understand the "Policy with some router related rules" test, and the difference with the first one (and why it don't work).

I started my tests with with the router:0 rule in and was irritated that no routes got pushed to the client. It started to work once the router:0 or router:8000 rule was removed.

@nblock commented on GitHub (May 4, 2025): > [@nblock](https://github.com/nblock) Thank you for the test, but I don't understand the "Policy with some router related rules" test, and the difference with the first one (and why it don't work). I started my tests with with the `router:0` rule in and was irritated that no routes got pushed to the client. It started to work once the `router:0` or `router:8000` rule was removed.
adam commented 2025-12-29 02:25:59 +01:00
Author
Owner
Copy Link

@Nathanael-Mtd commented on GitHub (May 4, 2025):

I started my tests with with the router:0 rule in and was irritated that no routes got pushed to the client. It started to work once the router:0 or router:8000 rule was removed.

Oh I see, but that's weird because I saw some integration tests in the PR and it's exactly your test with router rule, but it should work in tests. 😕

@Nathanael-Mtd commented on GitHub (May 4, 2025): > I started my tests with with the `router:0` rule in and was irritated that no routes got pushed to the client. It started to work once the `router:0` or `router:8000` rule was removed. Oh I see, but that's weird because I saw some integration tests in the PR and it's exactly your test with router rule, but it should work in tests. 😕
adam commented 2025-12-29 02:25:59 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (May 4, 2025):

Oh I see, but that's weird because I saw some integration tests in the PR and it's exactly your test with router rule, but it should work in tests. 😕

Tests were broken, I first fixed the tests (so that they failed 😆 ), then I fixed the issue. I think the PR should be in the correct shape now.

@kradalby commented on GitHub (May 4, 2025): > Oh I see, but that's weird because I saw some integration tests in the PR and it's exactly your test with router rule, but it should work in tests. 😕 Tests were broken, I first fixed the tests (so that they failed 😆 ), then I fixed the issue. I think the PR should be in the correct shape now.
adam commented 2025-12-29 02:25:59 +01:00
Author
Owner
Copy Link

@Nathanael-Mtd commented on GitHub (May 4, 2025):

Ok ! If nblock have time to check if everything is fine after your fixes, otherwise I can make some tests in the next days.

@Nathanael-Mtd commented on GitHub (May 4, 2025): Ok ! If nblock have time to check if everything is fine after your fixes, otherwise I can make some tests in the next days.
adam commented 2025-12-29 02:25:59 +01:00
Author
Owner
Copy Link

@nblock commented on GitHub (May 4, 2025):

Tests were broken, I first fixed the tests (so that they failed 😆 ), then I fixed the issue. I think the PR should be in the correct shape now.

Tested both policies with e93cd8bd64, works!

@nblock commented on GitHub (May 4, 2025): > Tests were broken, I first fixed the tests (so that they failed 😆 ), then I fixed the issue. I think the PR should be in the correct shape now. Tested both policies with e93cd8bd643d2c259a5d8984e5de3ada2c4d84d8, works!
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 bug no-stale-bot
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#918
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?