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

[Bug] Registration of duplicate NodeKeys leads to DoS / Spoofing #1086

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

Originally created by @mjwrona on GitHub (Aug 15, 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

This issue affects only the main branch at commit 30cec3a and has not yet been included in a release.

In pull request #2628, a change was introduced that modifies how a re-registration request is handled:
30cec3aa2b/hscontrol/auth.go (L36-L58)

Under the new registration flow, the system does not properly verify whether the RegisterRequest.NodeKey provided by the client actually belongs to the requesting user if a valid regReq.Auth.AuthKey is included.

Although there is an appropriate validation implemented later in the code, it is never executed because the logic performs an early return before reaching it:
30cec3aa2b/hscontrol/auth.go (L92-L94)

As a result, Tailscale can't handle multiple nodes with the same NodeKey. In such cases, it arbitrarily selects one of them (for example, admin1 or staging1), leading to unpredictable behavior.
Image

Expected Behavior

  • Owner verification early in the registration process
    The registration workflow should verify node ownership at an early stage to prevent unauthorized associations. This ensures that the RegisterRequest.NodeKey is explicitly tied to the correct user before any other registration logic proceeds.

  • Database-level unique constraint on NodeKey
    Enforcing a unique constraint on NodeKey at the database layer would prevent multiple records from sharing the same identifier. Tailscale relies on map[key.NodePublic] in several parts of its codebase, this would reflect that behavior.

Steps To Reproduce

  1. Build Headscale from commit 30cec3a.
  2. Register a node under the username attacker.
  3. Register another node under the username admin.
  4. Obtain the admin node’s NodeKey using: tailscale status --json
  5. Re-register as attacker, but replace the NodeKey with the one belonging to admin.
    In my test, the NodeKey was hardcoded directly into the Tailscale client to simulate the tampering.
  6. Observe that the admin node is no longer able to connect, resulting in a denial-of-service (DoS).
  7. Running tailscale status on the affected network will randomly return either the attacker or admin entry for the same node, leading to spoofing behavior.

Environment

- OS: Kali
- Headscale version: 30cec3a
- Tailscale version: 3f1851a

Runtime environment

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

Debug information

Spoofed NodeKey:
"node_key": "nodekey:b28ee415a1789cb9326ef2c7f82a096c501c053d948a4690ab56aa3058859d79"
headscale-nodes.json

Originally created by @mjwrona on GitHub (Aug 15, 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 This issue affects only the main branch at commit 30cec3a and has not yet been included in a release. In pull request #2628, a change was introduced that modifies how a re-registration request is handled: https://github.com/juanfont/headscale/blob/30cec3aa2b422a9d8184e47a747598fbe2f9f569/hscontrol/auth.go#L36-L58 Under the new registration flow, the system does not properly verify whether the `RegisterRequest.NodeKey` provided by the client actually belongs to the requesting user if a valid `regReq.Auth.AuthKey` is included. Although there is an appropriate validation implemented later in the code, it is never executed because the logic performs an early return before reaching it: https://github.com/juanfont/headscale/blob/30cec3aa2b422a9d8184e47a747598fbe2f9f569/hscontrol/auth.go#L92-L94 As a result, Tailscale can't handle multiple nodes with the same NodeKey. In such cases, it arbitrarily selects one of them (for example, `admin1` or `staging1`), leading to unpredictable behavior. <img width="575" height="571" alt="Image" src="https://github.com/user-attachments/assets/08173a39-61a6-4800-afc8-894aa32d1742" /> ### Expected Behavior - Owner verification early in the registration process The registration workflow should verify node ownership at an early stage to prevent unauthorized associations. This ensures that the `RegisterRequest.NodeKey` is explicitly tied to the correct user before any other registration logic proceeds. - Database-level unique constraint on NodeKey Enforcing a unique constraint on NodeKey at the database layer would prevent multiple records from sharing the same identifier. Tailscale relies on `map[key.NodePublic]` in several parts of its codebase, this would reflect that behavior. ### Steps To Reproduce 1. Build Headscale from commit 30cec3a. 2. Register a node under the username `attacker`. 3. Register another node under the username `admin`. 4. Obtain the admin node’s NodeKey using: `tailscale status --json` 5. Re-register as `attacker`, but replace the NodeKey with the one belonging to admin. In my test, the NodeKey was hardcoded directly into the Tailscale client to simulate the tampering. 6. Observe that the admin node is no longer able to connect, resulting in a denial-of-service (DoS). 7. Running `tailscale status` on the affected network will randomly return either the `attacker` or `admin` entry for the same node, leading to spoofing behavior. ### Environment ```markdown - OS: Kali - Headscale version: 30cec3a - Tailscale version: 3f1851a ``` ### Runtime environment - [ ] Headscale is behind a (reverse) proxy - [ ] Headscale runs in a container ### Debug information Spoofed NodeKey: `"node_key": "nodekey:b28ee415a1789cb9326ef2c7f82a096c501c053d948a4690ab56aa3058859d79"` [headscale-nodes.json](https://github.com/user-attachments/files/21795755/headscale-nodes.json)
adam added the bugwell described ❤️ labels 2025-12-29 02:28:11 +01:00
adam closed this issue 2025-12-29 02:28:11 +01:00
adam commented 2025-12-29 02:28:11 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Sep 5, 2025):

Hello! good find, I am trying to replicate this as a integration test before fixing it, but I am having trouble to do so.

Image

You can see the current implementation behind the screenshot in #2759.

I opted for implementing it with auth keys, but I am not sure if you used another auth method?

@kradalby commented on GitHub (Sep 5, 2025): Hello! good find, I am trying to replicate this as a integration test before fixing it, but I am having trouble to do so. <img width="1800" height="1009" alt="Image" src="https://github.com/user-attachments/assets/47524045-5731-4f10-bb34-ce7245bdbe34" /> You can see the current implementation behind the screenshot in #2759. I opted for implementing it with auth keys, but I am not sure if you used another auth method?
adam commented 2025-12-29 02:28:11 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Sep 9, 2025):

When I have time, I'm going to rebase the test branch (#2759) on top of the new (large) changes to main and try again. It would be really useful to have a test failing so we know what to fix, let me know if you have some time to figure out how I can reproduce it.

Unfortunately compiling and hardcoding the node key to the client is unfeasible in our tests, and if we cant add it to the tests it will be hard to ensure we do not break this in the future.

@kradalby commented on GitHub (Sep 9, 2025): When I have time, I'm going to rebase the test branch (#2759) on top of the new (large) changes to main and try again. It would be really useful to have a test failing so we know what to fix, let me know if you have some time to figure out how I can reproduce it. Unfortunately compiling and hardcoding the node key to the client is unfeasible in our tests, and if we cant add it to the tests it will be hard to ensure we do not break this in the future.
adam commented 2025-12-29 02:28:11 +01:00
Author
Owner
Copy Link

@mjwrona commented on GitHub (Sep 9, 2025):

Hey @kradalby
I quickly reviewed your code. From what i can see you're registering the admin node first and after that the attacker node.
Try to switch the order - first register the attacker and after that the admin otherwise the admin won't be locked out.
For the integration test it's fine to use the correct key pair instead of hardcoding the public key as the result should be the same.

Let me know if that works, if not i can check it more in detail

@mjwrona commented on GitHub (Sep 9, 2025): Hey @kradalby I quickly reviewed your code. From what i can see you're registering the admin node first and after that the attacker node. Try to switch the order - first register the attacker and after that the admin otherwise the admin won't be locked out. For the integration test it's fine to use the correct key pair instead of hardcoding the public key as the result should be the same. Let me know if that works, if not i can check it more in detail
adam commented 2025-12-29 02:28:11 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Sep 17, 2025):

I'm having a look at this today again, I've rebased the testing branch off the current main, I'm still having some trouble. Could you test if you find it present on the current main too?

I'm not 100% sure if we are going to be able to produce this in the integration tests, but will try a bit more.

@kradalby commented on GitHub (Sep 17, 2025): I'm having a look at this today again, I've rebased the testing branch off the current main, I'm still having some trouble. Could you test if you find it present on the current main too? I'm not 100% sure if we are going to be able to produce this in the integration tests, but will try a bit more.
adam commented 2025-12-29 02:28:11 +01:00
Author
Owner
Copy Link

@mjwrona commented on GitHub (Sep 18, 2025):

In the current main branch this is already fixed.
The code that i have reported:
30cec3aa2b/hscontrol/auth.go (L31-L58)
VS the current main:
bd35fcf338/hscontrol/auth.go (L31-L59)

As you can see the function handleExistingNode is now called before reaching handleRegisterWithAuthKey

@mjwrona commented on GitHub (Sep 18, 2025): In the current main branch this is already fixed. The code that i have reported: https://github.com/juanfont/headscale/blob/30cec3aa2b422a9d8184e47a747598fbe2f9f569/hscontrol/auth.go#L31-L58 VS the current main: https://github.com/juanfont/headscale/blob/bd35fcf338d678314fc5ef674d318f441fdf6fb6/hscontrol/auth.go#L31-L59 As you can see the function `handleExistingNode` is now called before reaching `handleRegisterWithAuthKey`
adam commented 2025-12-29 02:28:11 +01:00
Author
Owner
Copy Link

@kradalby commented on GitHub (Sep 18, 2025):

Great, I’ll closed this as fixed then.

@kradalby commented on GitHub (Sep 18, 2025): Great, I’ll closed this as fixed then.
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 well described ❤️
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#1086
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?