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
This commit is contained in:
Kristoffer Dalby committed 2026-10-18 20:53:18 +02:00
1 parent 0efe4b7798
commit a928125afa
3 files changed
+36 -5

No files matched your search

+5 -3
View File
@@ -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
}
@@ -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) {
+12 -2
View File
@@ -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
}