We have two sources of truth for configuration state: the node view
(from the netmap/policy) and prefs (the --advertise-connector option).
These come with two independent update paths: onSelfChange for node view
changes and profileStateChange for pref changes.
Centralize config on Conn25 so that onSelfChange and profileStateChange
can update their independent parts without bundling changes together.
The old bundled approach required read-modify-write, which opened the
door to potential TOCTOU bugs. The node view config is
stored as an atomic.Pointer[config] and the prefs-derived field
(advertise-connector) becomes an independent atomic.Bool. onSelfChange
creates a fresh config and stores it atomically. profileStateChange sets
the bool.
This also establishes clearer lines of responsibility:
- Configuration state lives on Conn25. Methods that need to read
config (isConnectorDomain, mapDNSResponse, the IPMapper methods)
are on Conn25, and use the atomics for synchronization.
- "Active" state (address allocations, transit IP mappings) lives on
client and connector, and use a mutex for synchronization on that
state, without conflicting with configuration synchronization.
It's fine for active state to be out of sync with config — e.g. a
transit IP allocated for an app should still be tracked, and gracefully
expired, even if the app is removed from the node view.
Removing config responsibility from client/connector makes these
cases clearer to handle.
- In cases where the client or connector does need access to
config-derived state, e.g. a client reconfiguring its IP pools from
the IPSets in the config, we can use closures for the
client or connector to get just the latest state it needs from the
config. See getIPSets() in this commit.
- As of this commit, the connector doesn't need config-derived state at
all.
Fixestailscale/corp#40872
Signed-off-by: Michael Ben-Ami <mzb@tailscale.com>
Michael Ben-Amiandmzbenami
822299642b