From a928125afab800b6c6d03609236fcf9078ed5b10 Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@dalby.cc>
Date: Wed, 14 Oct 2026 13:16:11 +0000
Subject: [PATCH] state: lowercase renames and refresh self DNS config on
 rename

"Foo" and "foo" could share one DNS name; collisions now ignore case.
A rename never resent the node its own DNS config, so CertDomains lagged.

Updates #2137
---
 hscontrol/state/node_store.go               |  8 +++++---
 hscontrol/state/node_store_hostname_test.go | 19 +++++++++++++++++++
 hscontrol/state/state.go                    | 14 ++++++++++++--
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/hscontrol/state/node_store.go b/hscontrol/state/node_store.go
index 67672d2c8..81b08ff7a 100644
--- a/hscontrol/state/node_store.go
+++ b/hscontrol/state/node_store.go
@@ -576,8 +576,9 @@ func (s *NodeStore) applyBatch(batch []work) {
 
 			taken := false
 
+			// DNS is case-insensitive: a legacy "Laptop" already holds "laptop".
 			for id, other := range nodes {
-				if id != w.nodeID && other.GivenName == w.name {
+				if id != w.nodeID && strings.EqualFold(other.GivenName, w.name) {
 					taken = true
 					break
 				}
@@ -676,12 +677,13 @@ func resolveGivenName(nodes map[types.NodeID]types.Node, self types.NodeID, base
 			continue
 		}
 
-		taken[n.GivenName] = struct{}{}
+		// DNS is case-insensitive: a legacy "Laptop" already holds "laptop".
+		taken[strings.ToLower(n.GivenName)] = struct{}{}
 	}
 
 	candidate := base
 	for i := 1; ; i++ {
-		if _, busy := taken[candidate]; !busy {
+		if _, busy := taken[strings.ToLower(candidate)]; !busy {
 			return candidate
 		}
 
diff --git a/hscontrol/state/node_store_hostname_test.go b/hscontrol/state/node_store_hostname_test.go
index 8ac007307..f95c546e2 100644
--- a/hscontrol/state/node_store_hostname_test.go
+++ b/hscontrol/state/node_store_hostname_test.go
@@ -172,6 +172,25 @@ func TestSetGivenNameRejectsTaken(t *testing.T) {
 	require.Equal(t, "phone", view.GivenName(), "rejected rename must not mutate state")
 }
 
+// TestGivenNameCollisionIgnoresCase covers mixed-case names from before
+// renames were lowercased: "Laptop" and "laptop" are one DNS name, so a
+// certificate for one would be valid for the other.
+func TestGivenNameCollisionIgnoresCase(t *testing.T) {
+	store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
+
+	store.Start()
+	defer store.Stop()
+
+	store.PutNode(createTestNode(1, 1, "alice", "Laptop"))
+	store.PutNode(createTestNode(2, 1, "alice", "phone"))
+
+	_, err := store.SetGivenName(2, "laptop")
+	require.ErrorIs(t, err, ErrGivenNameTaken)
+
+	got := store.PutNode(createTestNode(3, 1, "alice", "laptop"))
+	require.Equal(t, "laptop-1", got.GivenName())
+}
+
 // TestSetGivenNameRejectsInvalid returns ErrGivenNameInvalid for
 // labels that are not valid DNS labels.
 func TestSetGivenNameRejectsInvalid(t *testing.T) {
diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go
index fcf79db41..e73238486 100644
--- a/hscontrol/state/state.go
+++ b/hscontrol/state/state.go
@@ -191,8 +191,8 @@ type State struct {
 	// caller snapshot or resurrected by an update racing with deletion.
 	persistMu sync.Mutex
 
-	// selfRefresh holds nodes whose NextDNS device metadata changed, drained
-	// by [State.DrainSelfRefreshes].
+	// selfRefresh holds nodes whose own DNS config inputs changed (NextDNS
+	// device metadata, given name), drained by [State.DrainSelfRefreshes].
 	selfRefresh   []types.NodeID
 	selfRefreshMu sync.Mutex
 
@@ -1110,6 +1110,10 @@ func (s *State) SetApprovedRoutes(nodeID types.NodeID, routes []netip.Prefix) (t
 // auto-sanitisation) and collisions error out rather than silently
 // bumping a user-facing label. See HOSTNAME.md for the CLI contract.
 func (s *State) RenameNode(nodeID types.NodeID, newName string) (types.NodeView, change.Change, error) {
+	// DNS is case-insensitive but uniqueness and the client's certificate
+	// name match are not, so "Foo" and "foo" would share one name.
+	newName = strings.ToLower(newName)
+
 	// Validate the label AND that the resulting FQDN fits MaxHostnameLength:
 	// a valid 63-char label can still overflow under a long base_domain, and
 	// an unmappable name would break this node and its peers (issue #3346).
@@ -1142,6 +1146,12 @@ func (s *State) RenameNode(nodeID types.NodeID, newName string) (types.NodeView,
 		c = change.NodeAdded(nodeID)
 	}
 
+	// The node's own DNS config carries its name (CertDomains), and the
+	// change above does not resend DNS config to the renamed node itself.
+	s.selfRefreshMu.Lock()
+	s.selfRefresh = append(s.selfRefresh, nodeID)
+	s.selfRefreshMu.Unlock()
+
 	return nodeView, c, nil
 }