From 60f0544b788905d16f2fc69999775b9af283c6b3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 16 Jun 2026 06:56:10 +0000 Subject: [PATCH] dns, change, noise, auth, capver: misc consolidation --- cmd/headscale/headscale.go | 16 +-------- hscontrol/auth.go | 26 ++++++++++----- hscontrol/dns/extrarecords.go | 30 ++++++++--------- hscontrol/handlers.go | 2 +- hscontrol/mapper/batcher_test.go | 4 +-- hscontrol/noise.go | 48 +++++++++------------------ hscontrol/state/state.go | 10 ++++-- hscontrol/types/change/change.go | 16 --------- hscontrol/types/change/change_test.go | 4 +-- tools/capver/main.go | 48 ++++++++++++--------------- 10 files changed, 84 insertions(+), 120 deletions(-) diff --git a/cmd/headscale/headscale.go b/cmd/headscale/headscale.go index 679f082e..933e78e0 100644 --- a/cmd/headscale/headscale.go +++ b/cmd/headscale/headscale.go @@ -11,21 +11,7 @@ import ( ) func main() { - var colors bool - - switch l := termcolor.SupportLevel(os.Stderr); l { - case termcolor.Level16M: - colors = true - case termcolor.Level256: - colors = true - case termcolor.LevelBasic: - colors = true - case termcolor.LevelNone: - colors = false - default: - // no color, return text as is. - colors = false - } + colors := termcolor.SupportLevel(os.Stderr) != termcolor.LevelNone // Adhere to no-color.org manifesto of allowing users to // turn off color in cli/services diff --git a/hscontrol/auth.go b/hscontrol/auth.go index a6f39666..81e620bf 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -23,6 +23,18 @@ type AuthProvider interface { AuthURL(authID types.AuthID) string } +// machineKeyMismatch fails closed when a node looked up by NodeKey was started +// in a Noise session with a different machine key. Without this anyone holding a +// target's NodeKey could open a session with a throwaway machine key and act on +// the owner's node. Returns a 401 [HTTPError] on mismatch, nil otherwise. +func machineKeyMismatch(node types.NodeView, machineKey key.MachinePublic) error { + if node.MachineKey() != machineKey { + return NewHTTPError(http.StatusUnauthorized, "node exists with a different machine key", nil) + } + + return nil +} + func (h *Headscale) handleRegister( ctx context.Context, req tailcfg.RegisterRequest, @@ -75,12 +87,9 @@ func (h *Headscale) handleRegister( // open a Noise session with a throwaway machine key and read // the owner's User/Login back through [nodeToRegisterResponse]. // [Headscale.handleLogout] enforces the same check on its own path. - if node.MachineKey() != machineKey { - return nil, NewHTTPError( - http.StatusUnauthorized, - "node exists with a different machine key", - nil, - ) + err := machineKeyMismatch(node, machineKey) + if err != nil { + return nil, err } // When tailscaled restarts, it sends [tailcfg.RegisterRequest] with Auth=nil and Expiry=zero. @@ -153,8 +162,9 @@ func (h *Headscale) handleLogout( // Fail closed if it looks like this is an attempt to modify a node where // the node key and the machine key the noise session was started with does // not align. - if node.MachineKey() != machineKey { - return nil, NewHTTPError(http.StatusUnauthorized, "node exist with different machine key", nil) + err := machineKeyMismatch(node, machineKey) + if err != nil { + return nil, err } // Note: We do NOT return early if req.Auth is set, because Tailscale clients diff --git a/hscontrol/dns/extrarecords.go b/hscontrol/dns/extrarecords.go index 5dfc223d..05f42bc9 100644 --- a/hscontrol/dns/extrarecords.go +++ b/hscontrol/dns/extrarecords.go @@ -27,7 +27,7 @@ type ExtraRecordsMan struct { updateCh chan []tailcfg.DNSRecord closeCh chan struct{} - hashes map[string][32]byte + hash [32]byte } // NewExtraRecordsManager creates a new [ExtraRecordsMan] and starts watching the file at the given path. @@ -52,12 +52,10 @@ func NewExtraRecordsManager(path string) (*ExtraRecordsMan, error) { } er := &ExtraRecordsMan{ - watcher: watcher, - path: path, - records: set.SetOf(records), - hashes: map[string][32]byte{ - path: hash, - }, + watcher: watcher, + path: path, + records: set.SetOf(records), + hash: hash, closeCh: make(chan struct{}), updateCh: make(chan []tailcfg.DNSRecord), } @@ -160,18 +158,16 @@ func (e *ExtraRecordsMan) updateRecords() { e.mu.Lock() // If there has not been any change, ignore the update. - if oldHash, ok := e.hashes[e.path]; ok { - if newHash == oldHash { - e.mu.Unlock() + if newHash == e.hash { + e.mu.Unlock() - return - } + return } oldCount := e.records.Len() e.records = set.SetOf(records) - e.hashes[e.path] = newHash + e.hash = newHash toSend := e.records.Slice() log.Trace().Caller().Interface("records", e.records).Msgf("extra records updated from path, count old: %d, new: %d", oldCount, e.records.Len()) @@ -190,22 +186,24 @@ func (e *ExtraRecordsMan) updateRecords() { // readExtraRecordsFromPath reads a JSON file of [tailcfg.DNSRecord] // and returns the records and the hash of the file. func readExtraRecordsFromPath(path string) ([]tailcfg.DNSRecord, [32]byte, error) { + var zero [32]byte + b, err := os.ReadFile(path) if err != nil { - return nil, [32]byte{}, fmt.Errorf("reading path: %s, err: %w", path, err) + return nil, zero, fmt.Errorf("reading path: %s, err: %w", path, err) } // If the read was triggered too fast, and the file is not complete, ignore the update // if the file is empty. A consecutive update will be triggered when the file is complete. if len(b) == 0 { - return nil, [32]byte{}, nil + return nil, zero, nil } var records []tailcfg.DNSRecord err = json.Unmarshal(b, &records) if err != nil { - return nil, [32]byte{}, fmt.Errorf("unmarshalling records, content: %q: %w", string(b), err) + return nil, zero, fmt.Errorf("unmarshalling records, content: %q: %w", string(b), err) } hash := sha256.Sum256(b) diff --git a/hscontrol/handlers.go b/hscontrol/handlers.go index f8d2f178..3914c6dc 100644 --- a/hscontrol/handlers.go +++ b/hscontrol/handlers.go @@ -343,7 +343,7 @@ func (a *AuthProviderWeb) AuthHandler( } func authIDFromRequest(req *http.Request) (types.AuthID, error) { - raw, err := urlParam[string](req, "auth_id") + raw, err := stringParam(req, "auth_id") if err != nil { return "", NewHTTPError(http.StatusBadRequest, "invalid auth id", fmt.Errorf("parsing auth_id from URL: %w", err)) } diff --git a/hscontrol/mapper/batcher_test.go b/hscontrol/mapper/batcher_test.go index 49aeb54d..2e36d5aa 100644 --- a/hscontrol/mapper/batcher_test.go +++ b/hscontrol/mapper/batcher_test.go @@ -66,7 +66,7 @@ func (t *testBatcherWrapper) AddNode(id types.NodeID, c chan<- *tailcfg.MapRespo return fmt.Errorf("%w: %d", errNodeNotFoundAfterAdd, id) } - t.AddWork(change.NodeOnlineFor(node)) + t.AddWork(change.NodeOnline(node.ID())) return nil } @@ -90,7 +90,7 @@ func (t *testBatcherWrapper) RemoveNode(id types.NodeID, c chan<- *tailcfg.MapRe // Do this BEFORE removing from batcher so the change can be processed node, ok := t.state.GetNodeByID(id) if ok { - t.AddWork(change.NodeOfflineFor(node)) + t.AddWork(change.NodeOffline(node.ID())) } // Finally remove from the real batcher diff --git a/hscontrol/noise.go b/hscontrol/noise.go index 8f65a1ea..3c6bb6e2 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -31,9 +31,6 @@ var ErrUnsupportedClientVersion = errors.New("unsupported client version") // ErrMissingURLParameter is returned when a required URL parameter is not provided. var ErrMissingURLParameter = errors.New("missing URL parameter") -// ErrUnsupportedURLParameterType is returned when a URL parameter has an unsupported type. -var ErrUnsupportedURLParameterType = errors.New("unsupported URL parameter type") - // ErrNoAuthSession is returned when an auth_id does not match any active auth session. var ErrNoAuthSession = errors.New("no auth session found") @@ -338,40 +335,27 @@ func (h *Headscale) PingResponseHandler( } } -func urlParam[T any](req *http.Request, key string) (T, error) { - var zero T - +func stringParam(req *http.Request, key string) (string, error) { param := chi.URLParam(req, key) if param == "" { - return zero, fmt.Errorf("%w: %s", ErrMissingURLParameter, key) + return "", fmt.Errorf("%w: %s", ErrMissingURLParameter, key) } - var value T - switch any(value).(type) { - case string: - v, ok := any(param).(T) - if !ok { - return zero, fmt.Errorf("%w: %T", ErrUnsupportedURLParameterType, value) - } + return param, nil +} - value = v - case types.NodeID: - id, err := types.ParseNodeID(param) - if err != nil { - return zero, fmt.Errorf("parsing %s: %w", key, err) - } - - v, ok := any(id).(T) - if !ok { - return zero, fmt.Errorf("%w: %T", ErrUnsupportedURLParameterType, value) - } - - value = v - default: - return zero, fmt.Errorf("%w: %T", ErrUnsupportedURLParameterType, value) +func nodeIDParam(req *http.Request, key string) (types.NodeID, error) { + param := chi.URLParam(req, key) + if param == "" { + return 0, fmt.Errorf("%w: %s", ErrMissingURLParameter, key) } - return value, nil + id, err := types.ParseNodeID(param) + if err != nil { + return 0, fmt.Errorf("parsing %s: %w", key, err) + } + + return id, nil } // SSHActionHandler handles the /ssh-action endpoint, returning a @@ -381,7 +365,7 @@ func (ns *noiseServer) SSHActionHandler( writer http.ResponseWriter, req *http.Request, ) { - srcNodeID, err := urlParam[types.NodeID](req, "src_node_id") + srcNodeID, err := nodeIDParam(req, "src_node_id") if err != nil { httpError(writer, NewHTTPError( http.StatusBadRequest, @@ -392,7 +376,7 @@ func (ns *noiseServer) SSHActionHandler( return } - dstNodeID, err := urlParam[types.NodeID](req, "dst_node_id") + dstNodeID, err := nodeIDParam(req, "dst_node_id") if err != nil { httpError(writer, NewHTTPError( http.StatusBadRequest, diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index b67d2df1..00077c8d 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -644,7 +644,10 @@ func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) { return nil, 0 } - c := []change.Change{change.NodeOnlineFor(node)} + // A node coming online sends a lightweight online peer patch. Subnet + // routers, relay targets, and via targets get their full peer recompute + // from the gated PolicyChange below, so no full update is needed here. + c := []change.Change{change.NodeOnline(node.ID())} log.Info().EmbedObject(node).Msg("node connected") @@ -727,7 +730,10 @@ func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, erro // An ordinary node going offline just sends the lightweight offline // patch; emitting a PolicyChange for it would force every peer to // rebuild its netmap on every disconnect. - cs := []change.Change{change.NodeOfflineFor(node), c} + // A node going offline sends a lightweight offline peer patch. Subnet + // routers and other recompute-forcing nodes rely on the gated + // PolicyChange below for the peer recompute, so no full update here. + cs := []change.Change{change.NodeOffline(node.ID()), c} if s.polMan.NodeNeedsPeerRecompute(node) { cs = append(cs, change.PolicyChange()) } diff --git a/hscontrol/types/change/change.go b/hscontrol/types/change/change.go index d15469d9..b6015b73 100644 --- a/hscontrol/types/change/change.go +++ b/hscontrol/types/change/change.go @@ -456,22 +456,6 @@ func NodeRemoved(id types.NodeID) Change { return PeersRemoved(id) } -// NodeOnlineFor returns the [Change] for a node coming online: a lightweight -// [NodeOnline] peer patch. Subnet routers, relay targets, and via targets get -// their full peer recompute from the gated [PolicyChange] that State.Connect -// emits, so no full update is needed here. -func NodeOnlineFor(node types.NodeView) Change { - return NodeOnline(node.ID()) -} - -// NodeOfflineFor returns the [Change] for a node going offline: a lightweight -// [NodeOffline] peer patch. As with [NodeOnlineFor], subnet routers and other -// recompute-forcing nodes rely on the gated [PolicyChange] from State.Disconnect -// for the peer recompute, so no full update is needed here. -func NodeOfflineFor(node types.NodeView) Change { - return NodeOffline(node.ID()) -} - // KeyExpiryFor returns a [Change] for when a node's key expiry changes. // The [Change.OriginNode] field enables self-update detection by the mapper. func KeyExpiryFor(id types.NodeID, expiry time.Time) Change { diff --git a/hscontrol/types/change/change_test.go b/hscontrol/types/change/change_test.go index 5d857892..e8523e17 100644 --- a/hscontrol/types/change/change_test.go +++ b/hscontrol/types/change/change_test.go @@ -632,8 +632,8 @@ func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) { got Change wantOnline bool }{ - {name: "online", got: NodeOnlineFor(view), wantOnline: true}, - {name: "offline", got: NodeOfflineFor(view), wantOnline: false}, + {name: "online", got: NodeOnline(view.ID()), wantOnline: true}, + {name: "offline", got: NodeOffline(view.ID()), wantOnline: false}, } for _, tt := range tests { diff --git a/tools/capver/main.go b/tools/capver/main.go index 1cb3c363..a46ad8ca 100644 --- a/tools/capver/main.go +++ b/tools/capver/main.go @@ -261,6 +261,24 @@ func calculateMinSupportedCapabilityVersion(versions map[string]tailcfg.Capabili return versions[oldestSupportedMinor] } +// firstTailscaleVerPerCapVer inverts versions into a map from each capability +// version to the first (lowest-sorted) Tailscale minor version reporting it. +func firstTailscaleVerPerCapVer(versions map[string]tailcfg.CapabilityVersion) map[tailcfg.CapabilityVersion]string { + sortedVersions := xmaps.Keys(versions) + sort.Strings(sortedVersions) + + capVerToTailscaleVer := make(map[tailcfg.CapabilityVersion]string) + + for _, v := range sortedVersions { + capabilityVersion := versions[v] + if _, ok := capVerToTailscaleVer[capabilityVersion]; !ok { + capVerToTailscaleVer[capabilityVersion] = v + } + } + + return capVerToTailscaleVer +} + func writeCapabilityVersionsToFile(versions map[string]tailcfg.CapabilityVersion, minSupportedCapVer tailcfg.CapabilityVersion) error { // Generate the Go code as a string var content strings.Builder @@ -282,26 +300,13 @@ func writeCapabilityVersionsToFile(versions map[string]tailcfg.CapabilityVersion content.WriteString("\n\n") content.WriteString("var capVerToTailscaleVer = map[tailcfg.CapabilityVersion]string{\n") - capVarToTailscaleVer := make(map[tailcfg.CapabilityVersion]string) + capVerToTailscaleVer := firstTailscaleVerPerCapVer(versions) - for _, v := range sortedVersions { - capabilityVersion := versions[v] - - // If it is already set, skip and continue, - // we only want the first tailscale version per - // capability version. - if _, ok := capVarToTailscaleVer[capabilityVersion]; ok { - continue - } - - capVarToTailscaleVer[capabilityVersion] = v - } - - capsSorted := xmaps.Keys(capVarToTailscaleVer) + capsSorted := xmaps.Keys(capVerToTailscaleVer) slices.Sort(capsSorted) for _, capVer := range capsSorted { - fmt.Fprintf(&content, "\t%d:\t\t\"%s\",\n", capVer, capVarToTailscaleVer[capVer]) + fmt.Fprintf(&content, "\t%d:\t\t\"%s\",\n", capVer, capVerToTailscaleVer[capVer]) } content.WriteString("}\n\n") @@ -398,16 +403,7 @@ func writeTestDataFile(versions map[string]tailcfg.CapabilityVersion, minSupport content.WriteString("}\n\n") // Build capVerToTailscaleVer for test data - capVerToTailscaleVer := make(map[tailcfg.CapabilityVersion]string) - sortedVersions := xmaps.Keys(versions) - sort.Strings(sortedVersions) - - for _, v := range sortedVersions { - capabilityVersion := versions[v] - if _, ok := capVerToTailscaleVer[capabilityVersion]; !ok { - capVerToTailscaleVer[capabilityVersion] = v - } - } + capVerToTailscaleVer := firstTailscaleVerPerCapVer(versions) // Generate complete test struct for [capver.CapVerMinimumTailscaleVersion] content.WriteString("var capVerMinimumTailscaleVersionTests = []struct {\n")