[PR #1058] [MERGED] Fix duplicate nodes due to incorrect implementation of the protocol #1847

Closed
opened 2025-12-29 02:31:57 +01:00 by adam · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/1058
Author: @juanfont
Created: 12/9/2022
Status: Merged
Merged: 12/21/2022
Merged by: @juanfont

Base: mainHead: machinekey-in-noise


📝 Commits (2)

  • ea5babc Run the Noise handlers under a new struct so we can access the noiseConn from the handlers
  • 623f058 Add integration tests that check logout and relogin

📊 Changes

14 files changed (+389 additions, -119 deletions)

View changed files

📝 CHANGELOG.md (+1 -0)
📝 app.go (+0 -18)
📝 cmd/headscale/cli/nodes.go (+11 -1)
📝 integration/auth_web_flow_test.go (+161 -0)
📝 integration/tailscale.go (+2 -1)
📝 integration/tsic/tsic.go (+16 -0)
📝 machine.go (+35 -11)
📝 machine_test.go (+6 -3)
📝 noise.go (+24 -1)
📝 protocol_common.go (+118 -70)
📝 protocol_common_utils.go (+9 -7)
📝 protocol_legacy.go (+1 -1)
📝 protocol_noise.go (+2 -3)
📝 protocol_noise_poll.go (+3 -3)

📄 Description

Under certain conditions we could get duplicated nodes (e.g., after logout/login, reported in #1054) - due to a misunderstanding from my side on the role of MachineKey in TS2021.

MachineKey in TS2021 (the permanent-ish key associated to a client) is no longer sent in the body of the protocol requests, but obtained from the method Peer() in the controlbase.Conn struct. I was not aware of this (cheers @awsong for the heads up!). Our TS2021 was completely dropping MachineKey

Fixes #1054


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/juanfont/headscale/pull/1058 **Author:** [@juanfont](https://github.com/juanfont) **Created:** 12/9/2022 **Status:** ✅ Merged **Merged:** 12/21/2022 **Merged by:** [@juanfont](https://github.com/juanfont) **Base:** `main` ← **Head:** `machinekey-in-noise` --- ### 📝 Commits (2) - [`ea5babc`](https://github.com/juanfont/headscale/commit/ea5babccd12534570116bf5f0a1dfb187e9c9176) Run the Noise handlers under a new struct so we can access the noiseConn from the handlers - [`623f058`](https://github.com/juanfont/headscale/commit/623f05855945ac1a3e7e1fb86f1af466e0c7ffc9) Add integration tests that check logout and relogin ### 📊 Changes **14 files changed** (+389 additions, -119 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+1 -0) 📝 `app.go` (+0 -18) 📝 `cmd/headscale/cli/nodes.go` (+11 -1) 📝 `integration/auth_web_flow_test.go` (+161 -0) 📝 `integration/tailscale.go` (+2 -1) 📝 `integration/tsic/tsic.go` (+16 -0) 📝 `machine.go` (+35 -11) 📝 `machine_test.go` (+6 -3) 📝 `noise.go` (+24 -1) 📝 `protocol_common.go` (+118 -70) 📝 `protocol_common_utils.go` (+9 -7) 📝 `protocol_legacy.go` (+1 -1) 📝 `protocol_noise.go` (+2 -3) 📝 `protocol_noise_poll.go` (+3 -3) </details> ### 📄 Description Under certain conditions we could get duplicated nodes (e.g., after logout/login, reported in #1054) - due to a misunderstanding from my side on the role of MachineKey in TS2021. `MachineKey` in TS2021 (the permanent-ish key associated to a client) is no longer sent in the body of the protocol requests, but obtained from the method `Peer()` in the `controlbase.Conn` struct. I was not aware of this (cheers @awsong for the heads up!). Our TS2021 was completely dropping `MachineKey` Fixes #1054 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-29 02:31:57 +01:00
adam closed this issue 2025-12-29 02:31:57 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#1847