Bot review (CodeRabbit, cubic) on PR #7098 surfaced several real defects:
- Strict() parsed the raw env value instead of the normalized one, so a
mixed-case NB_PQ_MLKEM_STRICT such as "tRuE" silently disabled fail-closed
mode. Parse the lower-cased value, matching Enabled().
- OnDataPathMessage labeled a data-path answer as "signal" in logs, mislabeling
every rotation answer. Use the data-path label.
- A failed sendAnswer aborted the whole connection setup, skipping the relay/ICE
listeners; a transient signalling failure now still brings the local transport
up (the peer retries the answer).
- processOffer left the reserved exchange slot in place when Respond failed, so
every retransmission of that offer was dropped forever. Clear the reservation
on a pre-commit error so a retry can derive again.
- Dropped a no-op time.Since that implied a convergence-latency metric that was
never recorded, and the now-unused startedAt field.
- The strict-mode status line asserted active blocking even for a peer that is
simply offline; reword it to state that no PSK is established yet.
- conn_test used t.Fatalf for conditions under test; use assert.
Start tripped SonarCloud's cognitive-complexity gate (S3776) after the ML-KEM
block landed. Pull the Rosenpass and ML-KEM manager startup into two helpers
that read as one line each from Start. Pure refactor, behavior unchanged.
* [client] Build the Android network list from a single peer snapshot
Networks() called GetFullStatus() once per network to find the peer
that serves it, copying every peer state and taking eight recorder
locks each time. With 100+ peers and the UI calling Networks() from
every peer list change, this queued hundreds of callers on the status
recorder lock. Take one snapshot per call and index it by route.
* [client] Track the active route peer by HA unique ID in the status recorder
The Android network list resolved the route owner by scanning peer state
route keys. Those keys are the handler string: a prefix for static routes
and the domain pattern for dynamic ones, so the prefix-based lookup never
matched dynamic routes, and two networks sharing a prefix resolved to the
same owner.
The route watcher now records the chosen route peer under the route's HA
unique ID in the status recorder, and the Android binding looks the owner
up by that ID. The key is unique per network and independent of the
handler string format, so both anomalies are gone. The prefix-based
routeOwners helper is removed.
* [client] Record the active route peer before notifying listeners
AddPeerStateRoute and RemovePeerStateRoute fire the peer list change
callback and wake the status subscribers. The active route peer mapping
was written after those calls, so a Networks() call landing in between
found no mapping for the network and fell back to the first connected
peer, or kept showing the previous peer on removal. Nothing re-notified
after the mapping write, so the wrong peer stayed until the next peer
list change.
Write and delete the mapping before the notifying calls so a listener
reacting to the notification always reads the current owner.
* [client] Add a debug bundle file export to the Android bridge
The Android app can only upload a debug bundle and hand the user a key.
Users who want to inspect what leaves their device before sharing it
have no way to get the zip itself. Add DebugBundleFile, which generates
the bundle into the cache directory and returns its path instead of
uploading; the app copies it wherever the user chose and removes it.
DebugBundle keeps its behavior. Both entry points share the unexported
debugBundle with an upload switch, so the body stays where it was and
merges cleanly with the MDM overlay change on main.
Because the file variant leaves the zip to the caller and the upload
variant only removes it after the upload finishes, a process killed in
between leaves a zip behind in the cache. Remove stale bundles before
generating a new one: RemoveStaleBundles deletes zips matching the
generator's pattern that are older than an hour. Remote debug jobs write
to the same directory, so younger files are treated as still in use.
* Preserve network map for debug bundle on Android
* [client] Keep exported Android debug bundles out of the stale cleanup
DebugBundleFile hands the zip to the caller, but the file kept the
netbird.debug.*.zip name that RemoveStaleBundles matches, so a later
debug run could delete it once it was older than an hour. Rename the
exported bundle to netbird.debug-file.*.zip after generation so the
cleanup only ever touches bundles no caller owns.
* [client] Warn when a stale debug bundle cannot be removed
A failed removal means bundles pile up in the cache directory, so log it
at Warn instead of Debug. A file that is already gone was removed by a
concurrent cleanup and is skipped silently.
* [client] Drop the outdated debugBundle comment
The comment still said the file variant leaves the zip in place, but it
is renamed by debug.ExportBundle since the stale-cleanup change.
* [client] Test that the network map reaches the debug bundle
Cover both halves of the path Android now relies on: the engine keeps
the latest sync response once persistence is enabled, and the bundle
generator writes it to network_map.json (anonymized or not) and omits
the file when there is no sync response.
* [client] Remove abandoned exported debug bundles after a day
An exported bundle is owned by the caller, but if the app is killed
before it copies and deletes the file, nothing ever removes it from the
cache directory. Let RemoveStaleBundles also match exported bundles,
with a 24 hour max age instead of the caller-provided one, so a bundle
that is still being saved survives while an abandoned one goes.
* [client] Discover interfaces lazily in stdnet instead of at construction
stdnet.NewNet and NewNetWithDiscover ended with
return n, n.UpdateInterfaces()
handing back a non-nil *Net together with the discovery error. Three of the
five call sites (Engine.newWgIface, ice.NewAgent, SingleSocketUDPMux) logged
the error and kept using the instance, which is only safe as long as the
instance still works after a failed discovery.
That stopped being true when Interfaces() gained a lazily refreshed cache:
updateInterfaces sets lastUpdate only on success, so after a failed
construction the 30s cache guard never holds and Interfaces() returns an
error rather than the empty list it used to return. Feeding such an instance
to pion is worse than passing nothing at all - ice.NewAgent falls back to its
own stdnet when Net is nil, and the interface blacklist is applied separately
through AgentConfig.InterfaceFilter, so the fallback loses nothing. Instead,
a transient discovery failure (the Android bridge at boot, or an interface
disappearing between net.Interfaces() and Interface.Addrs()) turned into a
hard "error getting local interfaces" from ice.NewAgent, and aborted the STUN
and TURN probes, which never even need the interface list.
Since the accessors already refresh a stale cache on demand, the eager
discovery in the constructors is redundant: drop it, make both constructors
infallible, and let the discovery error surface at the call that actually
needs the interfaces. UpdateInterfaces had no callers left and is not part of
transport.Net, so it is removed along with it.
InterfaceByIndex and InterfaceByName read the cached slice directly and never
refreshed it, so they would have kept reporting ErrInterfaceNotFound forever
on an instance whose first discovery failed. They now go through the same
refresh path as Interfaces().
* [client] Warm the stdnet interface cache at construction
Moving discovery to first use regressed the privileged suites on the three
platforms that always build an ICE bind: Darwin, FreeBSD and Windows time out
in TestWGIface_UpdateAddr, TestRecreation, TestEngine_SSH and
TestEngine_MultiplePeers, while Linux stays green because a host with the
WireGuard kernel module takes the kernel-device branch and never drives the
mux that asks for interfaces.
interfaceFilter probes with wgctrl every interface the disallow list does not
already exclude. Discovering at construction ran that probe before the caller
had an overlay interface of its own; discovering at first use runs it after,
so on a userspace WireGuard platform the probe reaches the UAPI socket of the
same process. The tests reach it because they construct with a nil disallow
list, where the client passes DefaultInterfaceBlacklist and its own interface
is excluded by prefix.
Restore the original timing with an explicit warm-up. The constructors stay
infallible and the error is still reported by the accessor that needs the
interfaces, so the contract this branch is about is unchanged.
* Revert "[client] Warm the stdnet interface cache at construction"
This reverts commit 947e25288f.
* [client] Give the privileged tests the interface blacklist the client uses
The suites that create a WireGuard interface construct stdnet with a nil
disallow list, which the client never does: Engine passes
profilemanager.DefaultInterfaceBlacklist, whose "wt" and "utun" prefixes
exclude the overlay interface before the filter reaches its wgctrl probe.
With an empty list every interface reaches that probe, the one the test has
just created included, and on a userspace WireGuard platform the probe talks
to the UAPI socket of the same process. That is why Darwin, FreeBSD and
Windows timed out here while Linux, which takes the kernel-device branch on a
host with the module loaded, stayed green.
Pass the blacklist in both suites so they exercise the configuration the
client ships. client/iface declares the prefixes locally because
profilemanager imports it.
Also cover the constructors directly: the existing tests build the struct
literal, so nothing asserted that NewNet and NewNetWithDiscover leave the
cache cold.
* [client] Pass the blacklist in the remaining tests that build an interface
Same reason as the previous commit, four call sites it missed: engine_test,
the route manager and systemops suites, and the privileged DNS server suite
all construct stdnet with a nil disallow list and then create a WireGuard
interface. TestAddVPNRoute surfaced it on FreeBSD once the earlier two files
stopped timing out first.
client/internal/dns declares the prefixes locally; profilemanager imports
that package, so it cannot import profilemanager back.
A worker that saw a new remote session ID rebuilt its agent and also
picked a new local ID. On the answer path nothing carries that ID back,
so the next offer made the remote see a changed session, rebuild, and
answer with yet another ID. Two peers kept tearing down working ICE
connections on every offer and answer; nearly every answer in the
affected logs carried a new remote session ID.
Only a local restart changes the local ID now: a failed negotiation, as
before, and an explicit Close, which previously kept the old ID and left
the remote answering from a negotiation this side had abandoned.
Following a remote restart keeps the ID the remote already knows, so the
pair settles after one rebuild, also against peers that still pick a
new ID when following a restart.
Connections to the daemon were left on gRPC's own defaults, which cap a received
message at 4 MB. A detailed status carries an entry per peer, so on a large
deployment the response outgrows that cap and the command fails outright:
netbird status -d
Error: status failed: grpc: received message larger than max (4287609 vs. 4194304)
The limit is raised where the daemon dial options are built, so every caller
inherits it: the CLI, the desktop UI, the JSON gateway, and the SSH client and
proxy. It is overridable through NB_DAEMON_GRPC_MAX_MSG_SIZE for a deployment
that outgrows the new default too, mirroring what the management client already
does with NB_MANAGEMENT_GRPC_MAX_MSG_SIZE, and reusing its 16 MB default.
Only the receive direction needs raising. Requests to the daemon are small, and
gRPC does not cap the send side by default, so the daemon could already send a
response the caller then refused to read.
* [client] Export the only-owner-writable path check from elevate
Pure refactor, no behavior change: the existing checkOnlyOwnerWritable gets a
thin exported wrapper so callers outside the elevation path can reuse it. No
call site changes here.
* [client] Validate the saved service parameters before applying them
The install reads <stateDir>/service.json and applies it to the service it then
registers: its arguments, its config path and its environment. The restricted
ACL that saveServiceParams puts on the state directory is applied when the file
is written, which is not necessarily before the file is first read, so the
install now checks the file rather than assuming it.
A file whose ownership or permissions are not the ones saveServiceParams
produces is treated as absent, and the install proceeds with its defaults. The
check covers the directories above the file as well, so what is checked is what
is read.
* [client] Restrict which environment variables the service is registered with
--service-env, and the service.json it persists to, accepted any name. A small
set of them decides how a process resolves the executables and libraries it
loads, and the daemon needs none of those: it now refuses them when they are
passed explicitly, and drops them with a warning when they come back from a
service.json written by an older version, so an upgrade does not fail over a
variable nobody needs.
* [client] Resolve netsh by absolute path
The lookup consulted PATH first and fell back to System32, in both the copy the
userspace firewall uses and the one that tears the interface down. It now asks
Windows for the system directory, so the resolution no longer depends on the
environment the service happens to be started with.
* [client] Move the System32 lookup into a package both callers share
Pure refactor, no behavior change: client/iface and client/firewall/uspfilter
carried a copy each of the same function, and neither imports the other, so the
body moves to client/internal/wincmd — alongside winregistry, which is where
the client's other Windows-only helper already lives. Both call sites now read
wincmd.System32("netsh").
* [client] Cover the System32 lookup with a test
Asserts what the previous commits changed: the lookup is absolute, and neither
PATH nor %SystemRoot% moves it.
* [client] Refuse the loader environment families by prefix
Review follow-up on the previous commit:
- LD_* and DYLD_* are now refused whole rather than name by name. Their members
differ per platform and libc and grow with new OS releases, so a list of them
is out of date as soon as it is written — DYLD_FALLBACK_LIBRARY_PATH and
DYLD_FALLBACK_FRAMEWORK_PATH were already missing from it.
- The names are folded to upper case only on Windows, where a variable is the
same one however it is spelled. Elsewhere the environment is case-sensitive,
so Path and PATH are two variables and only the exact spelling is the one that
is read; the fold refused the wrong one.
- TEMP and TMP stay in the denylist, but the rationale and the message now say
what they actually decide: where the service writes, not what it loads.
* [client,management] Skip route firewall rule computation when no firewall
A peer that runs with the firewall disabled has no ACL manager and no
firewall to program, so nothing ever reads RoutesFirewallRules: the only
consumers are acl.Manager, which is reached solely when e.acl is set, and
the legacy-management probe in updateNetworkMap, which is guarded by a
non-nil firewall.
Building those rules is the most expensive part of a sync on a peer that
routes many network resources. On a 15k-peer deployment a debug bundle
showed getPeerNetworkResourceFirewallRules accounting for 62% of the
allocations of Calculate, and Calculate for effectively all of the
allocations of handleSync, which was taking 3.2s on average and holding
the engine lock for the duration.
Let the caller ask Calculate to leave the rules out. The client passes
its existing DisableFirewall setting; the management server keeps the
default and still produces them.
RoutesFirewallRulesIsEmpty is set from the resulting empty list, so a
receiver that would otherwise infer legacy management from an empty rule
set does not misread the skip.
* [client,management] Cover the skip flag through the envelope
Review feedback on #7624.
The components test compared only the length of the peer firewall rules, so
a change to their content would have passed while the message claimed they
came out unchanged. Compare the slices.
The skip path was also only exercised by setting the field directly on the
components, which bypasses the envelope conversion where
RoutesFirewallRulesIsEmpty is derived. That bit is what keeps the client from
reading skipped rules as a legacy management server, so it gets a test that
goes through EnvelopeToNetworkMap with the flag set.
* [management] Give the router a peer ACL so the rule comparison bites
Review feedback on #7624.
peer-router-1 appears in no peer ACL in the shared fixture, so its
FirewallRules came out empty and the equality assertion compared two empty
slices — it would have passed even if the peer rules were dropped entirely.
Add a policy covering the router and require the baseline to be non-empty
before comparing.
* [relay] Signal relay disconnects through the conn context
AddCloseListener deduplicated listeners by comparing
reflect.ValueOf(callback).Pointer(). For a method value that pointer is
the address of the compiler-generated wrapper, not an identity bound to
the receiver, so every peer's w.onRelayClientDisconnected compared equal.
All peers on the home relay register under the same connectionURL key, so
only the first registration survived and the rest were silently dropped.
On a relay disconnect those peers were never notified: statusRelay stayed
connected and the reconnect guard never fired. The relayed net.Conn itself
was closed by closeAllConns, so nothing leaked, but the peer state machine
did not learn about it. Foreign relays had the same defect scoped to the
peers sharing that server.
Rather than fixing the deduplication, drop the peer-level listener registry
entirely. A relayed Conn now exposes Context(), cancelled when the
connection is torn down, with a cancellation cause naming the reason. This
is the same shape quic-go uses for its Conn and Stream types, and it
removes the whole class of problems around listener identity, lifetime and
deregistration: the signal belongs to the resource instead of a side table.
WorkerRelay watches that context in a goroutine whose lifetime matches the
connection. A watcher that wakes up for a superseded connection compares
the conn pointer against the current one and returns without touching the
state machine, so a fast relay reconnect cannot have a stale watcher tear
down the connection that replaced it.
Client.SetOnDisconnectListener stays: it is server-level and drives the
reconnect guard and foreign relay eviction, unrelated to peers.
handleRelayReady also checks the conn context, closing the race where the
relay dies between OpenConn and the readiness handoff and the peer would
otherwise build a WireGuard endpoint over a dead connection.
TestNotifierDoubleAdd covered the removed mechanism and is gone.
TestForeignAutoClose asserted nothing (both branches logged); it now waits
for the relay to leave the client map and fails if it does not.
* [relay] Fix build: return the concrete conn from Client.OpenConn
OpenConn now returns *Conn, but it still went through connContainer.netConn(),
which widens to net.Conn. The helper had one caller and only existed to produce
the interface value the signature no longer wants, so return container.conn
directly and drop it.
* [relay] Assert the local-close cancellation cause explicitly
The local-close test only rejected ErrServerDisconnected, so it would also
have passed for ErrPeerDisconnected or a bare context.Canceled. closeConn
cancels with net.ErrClosed, so assert that.
* [client] Ignore relay disconnects from superseded connections
The relayed conn watcher compared the conn pointer under relayLock, released
it, and only then tore the connection down. A new offer could install its
replacement in that window, so a watcher that validated the old pointer went
on to close the proxy of the connection that had already replaced it and
report the peer as disconnected while it was up.
Move the decision to where the teardown happens. Conn records which relayed
connection the current proxy was built from, and onRelayDisconnected takes the
connection the signal belongs to and drops it under conn.mu when it is no
longer the current one. Check and effect are now in the same critical section,
so the verdict cannot go stale before it is acted on.
This also covers the proxy read loops, whose disconnect listener took no
argument and had the same defect: it now names the connection it belongs to.
The WG timeout path keeps passing nil, since it deliberately tears down
whatever is current.
* [client] Bind the relayed conn reference to the proxy swap
relayedConnRef was set at the top of the readiness path, but wgProxyRelay only
changes at the end, in setRelayedProxy. The two failure returns in between —
newProxy and ConfigureWGEndpoint — left the reference pointing at a connection
that never became active while the old proxy was still installed. A disconnect
of that old, live relay would then be dismissed as belonging to a superseded
connection and never cleaned up.
Set the reference in setRelayedProxy, next to the proxy it belongs to. Both
success paths go through it and neither failure path does, so no failure branch
has to remember to roll anything back.
GetSessionExpiresAt took the exclusive lock for a plain field read, so
every caller queued behind writers and behind each other. The Android
SessionMonitor polls it from the main thread, and in the captured ANR
that is exactly where the main thread was blocked while hundreds of
peer-list callbacks held or waited on the same mutex.
d.mux is already an RWMutex and the other getters use RLock; this brings
the deadline read in line with them.
Read the relay instance URL and IP atomically to prevent reconnects from mixing values from different connections. Extend existing connection and offer/answer logs with relay URLs and IPs to help trace mismatched advertisements.
The per-exchange lifecycle logs were all at LevelTrace (below the default field
level) and did not distinguish a signalling re-bootstrap from a data-path rekey,
which made diagnosing exchange activity in the field guesswork (e.g. telling an
ICE-retry-driven re-bootstrap storm apart from normal KEM rotation).
Promote the two pivotal events — an offer going out and a PSK converging — to
LevelDebug and tag every lifecycle log with four dimensions:
- msg: offer vs answer (implicit in the message)
- role: initiator vs responder for this peer
- via: signal vs data-path (threaded into processOffer/processAnswer)
- kind: bootstrap (a new connection / reconnect) vs rotation (a rekey)
kind is derived from the exchange's AckID (zero = bootstrap) on the responder
side and from viaSignal on the initiator side. No behavioural change.
pqControllerReoffer returned true whenever we are the controller running the
KEM, even when ShouldSendBootstrapOffer was false — which is the steady state
for every PQ peer past its bootstrap (exchange in awaitingRekey) and for
non-capable peers. handleRemoteOffer then returned early, dropping the ICE
credentials and relay info carried in the peer's offer, so a responder-initiated
reconnect (the normal path under lazy connections) stalled: the controller only
recovered on its own guard timing.
Return true only when a re-offer was actually sent; otherwise fall through to the
normal offer handling (sendAnswer + notifyListeners) so the connection can come
up on the retained PSK. A stale PSK still self-heals via the WG watcher, which
triggers a fresh signal offer that re-bootstraps.
Two convergence bugs surfaced by the security review:
- Role guard (finding B): processOffer accepted an offer even when we are the
KEM initiator for the peer, and processAnswer accepted an answer when we are
the responder. The KEM is unidirectional (initiator offers, responder
answers), so a role-violating message is anomalous — a desync, a duplicate,
or an injected/spoofed data-path packet. Processing it derived and committed
a fresh PSK, overwriting a live one and silently dropping any in-flight
exchange (whose retry loop then exited without raising a failure or
re-bootstrapping). Reject offers when we are the initiator and answers when
we are not; this drops only anomalous traffic and leaves the normal flow
untouched.
- Re-bootstrap on signal re-negotiation (finding A): SignalOffer was idempotent
in stateAwaitingRekey too, replaying the frozen bootstrap offer. After the
responder restarted and lost its state it derived a different PSK from fresh
material, which our awaitingRekey side then rejected — a permanent desync with
no recovery (in strict mode the peer stays blocked). Make the idempotency
apply only while a bootstrap is still in flight (awaitingAnswer); once a PSK
is derived, a fresh signal offer starts a new exchange so both sides converge.
The controller-double-offer case the idempotency guarded is already covered by
ShouldSendBootstrapOffer. Reusing the cached offer also reused the same
ephemeral keys across exchanges, reducing forward secrecy.
Both paths have a failing-without-the-fix regression test.
- strict-kem vs strict-rp said "Connected + Quantum resistance: true" but it is actually blocked
- perm-kem vs perm-rp "Connected + Quantum resistance: true" but it's a classic WG link, without PQ safety
Reduce Listen's cognitive complexity (SonarCloud S3776, 30 -> under 25) by
extracting the offer and answer cases into handleRemoteOffer/handleRemoteAnswer,
with shared onSignalReceived/notifyListeners helpers and a pqControllerReoffer
helper for the controller re-offer branch. No functional change.
The responder configures WireGuard with endpoint=nil first, then a delayed
update (scheduleDelayedUpdate) applies the real endpoint after fallbackDelay.
It captured the preshared key at schedule time and re-applied it. With the
post-quantum exchange the PSK can change within that window (a fresher PSK
derived and applied via SetPresharedKey), so re-applying the captured one
reverted WireGuard to a key the remote peer no longer used — a mismatch that
stalled the handshake until the WGWatcher timeout forced a retry (~30s).
Pass a nil PSK in the delayed update so it only sets the endpoint and leaves
the current PSK in place; the latest SetPresharedKey wins.
When the controller receives the responder's (KEM-less) offer it replies with
its own KEM offer instead of answering, so the only transaction that brings the
tunnel up is the one that also carries the PSK. Guard that reply with
ShouldSendBootstrapOffer so it fires only when no exchange is in flight: without
it, every responder offer triggered another offer (an offer-per-offer runaway).
The whole behaviour is isolated to the KEM path (config.PQ != nil); non-PQ
connections answer as before.
The controller sends its KEM offer both on its own guard event and in reply
to the responder's offer. SignalOffer was idempotent only while awaiting the
answer; once the answer arrived (awaitingRekey) a repeat call started a fresh
exchange with a different PSK, desyncing the two peers (one on the old PSK,
one on the new) so WireGuard derived misaligned transport keys and dropped all
data. Treat awaitingRekey as in-flight too and return the same offer.
To ensure two peers agree on a key, we need asymmetry. one peer is
the controller ("initiator") the other is the "responder".
Otherwise imagine two offers in parallel driving two answers at the same time
A B
| <----B-OFFER----- |
| -----A-OFFER----> |
| |
| |
---------------------------------
|****** ICE + WG Handshake ****** |
---------------------------------
| |
| <----B-ANSWER---- |
| -----A-ANSWER---> |
PSK is derived on receive of offer, so A and B derive different PSKs.
When WG handshake takes place it picks misaligned PSKs.
So we impair the two nodes and only the offer of one of the two (the controller/initiator)
is allowed to progress and drive the answer (and carry the KEM material).
If a responder initiates an offer, we redo the offer towards it. This is oK
since the ICEworker don't treat offer/answers differently.
To ensure two peers agree on a key, we need asymmetry. one peer is
the controller ("initiator") the other is the "responder".
Otherwise imagine two offers in parallel driving two answers at the same time
A B
| <----B-OFFER----- |
| -----A-OFFER----> |
| |
| |
---------------------------------
|****** ICE + WG Handshake ****** |
---------------------------------
| |
| <----B-ANSWER---- |
| -----A-ANSWER---> |
PSK is derived on receive of offer, so A and B derive different PSKs.
When WG handshake takes place it picks misaligned PSKs.
So we impair the two nodes and only the offer of one of the two (the controller/initiator)
carries the KEM material.
This means that if the responder OFFER/ANSWER comes first, when the controller/initiator's one
completes (and the genuine PSK is shared between A and B, we need to force a new WG handshake with
the proper keys.
- RecoversViaResignalAfterDataPathBreak: a data-path rotation that can no longer
converge raises OnRekeyFailed, and re-bootstrapping over signalling resyncs both
peers on a fresh PSK even while the data path stays broken.
- ConcurrentRekeysNoRace: hammers the single-lock state machine with concurrent
rotation clocks from many goroutines (run with -race) and asserts no split-brain
via a final deterministic bootstrap.
OnRekeyFailed now re-runs the KEM bootstrap over Signal (conn.RequestReoffer ->
handshaker.SendOffer) instead of only logging: a fresh signalling offer starts a new
exchange that overwrites the stalled PSK on both sides, resyncing after a persistent
data-path desync. Chosen over a responder-side awaitingAck revert (which fights the
confirm-less ack timing) and a full tunnel teardown (heavier). The tunnel stays up on
the previous PSK meanwhile since Signal is independent of the broken data path.
Strict mode (NB_PQ_MLKEM_STRICT, default off) closes the initial PQ-vulnerable
window (NET-1408): when enabled, conn.presharedKey programs a per-conn random
sentinel PSK until the ML-KEM exchange derives the real one, so no session can form
on a non-PQ key (the real PSK is pushed via SetPresharedKey once it converges).
Default stays opportunistic.
Also surface PQ status: the peer 'Quantum resistance' flag (RosenpassEnabled) is now
true when an ML-KEM PSK has been derived for the peer, not only for Rosenpass.
The idle-gate reads LastActivities, which only tracks per-peer data in userspace;
in kernel mode it is empty, so the gate treated every kernel peer as idle and
disabled data-path rotation entirely. Detect the bind via IsUserspaceBind and, in
kernel mode, report zero activity age (always 'active') so rotation runs on every
rekey. Lazy back-to-idle is already limited in kernel; the eBPF WG-activity
detection will later supply a real signal that excludes handshake/pqkem traffic.
Replace the raw SHA-256 concat combiner with HKDF-SHA256 (crypto/hkdf, Go 1.24):
IKM = ML-KEM_ss || X25519_ss (draft-ietf-tls-ecdhe-mlkem order), salt = the
domain-separation label, info = full transcript (offer || answer) || canonicalised
peer identities. Keeps the transcript + identity binding while using a proper KDF.
riccardom