* [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.
* Name the account owner in the pending approval error
A user refused because their account is pending approval had no way to
learn who could approve them. The refusal now carries the account
owner's address, masked, so a caller can name someone to contact without
being handed the address itself.
Resolving the owner is best effort: a lookup failure, or an account
predating the stored email, falls back to the refusal as it was.
* Name only the caller's own owner in the pending approval error
The refusal is raised before ValidateAccountAccess has established that
the caller belongs to the account the request asked about, and the user
is loaded by ID alone. Resolving the owner of the requested account
therefore disclosed that owner's address to a pending user with no claim
to it, reachable through any handler that takes an account ID from the
caller — DELETE /api/accounts/{accountId} passes one straight through.
The owner who can approve a pending user is the owner of their own
account, so resolve that one. The requested account is never read.
* Mask short local parts whole in MaskedEmail
Keeping the first two characters and the last hides nothing until the
local part is four long: at three or fewer they are the whole of it, so
"abc@example.com" masked to "ab****c@example.com" and a pending user
could recover the owner's address in full from what is meant to conceal
it. Short local parts are now replaced entirely.
* Name the owner from GetCurrentUserInfo instead of the permission gate
The gate could only read the stored user row, which carries no address
when an external IdP owns the identities — the usual case — so it named
no one in practice. It also had no way to reach the IdP without being
handed the account manager, which meant restoring bootstrap wiring that
a refactor had dropped.
GetCurrentUserInfo already holds that account manager, so it answers for
a pending user itself and reuses GetOwnerInfo, the same lookup /msp uses
to resolve an owner's address. The gate returns to exactly what it was,
and with it goes the risk of naming the owner of an account the caller
only asked about.
MaskedEmail becomes MaskEmail: with a UserInfo in hand there is no stored
row to hang it off.
* [management] Cover the pending approval refusal in GetCurrentUserInfo
The branch that names the owner had no coverage at the manager level, so
neither the named refusal nor the fallback for an owner without a resolvable
address was pinned down.
* [management] Cover the failed owner lookup in the pending approval refusal
The generic fallback has two ways in: no address on the resolved owner, and no
owner to resolve at all. Only the first was pinned down.
* [management] Pin the owner lookup to the caller's own account
A mismatched account claim must not steer which owner the refusal names, and
a blocked user is still answered before the claim is validated. Both are load
bearing and neither was covered.
A client that hits a private service before its peer joins the overlay gets a 403 from the tunnel-peer check. After it connects to NetBird, the browser reuses the warm socket to the public listener, so the request never traverses the tunnel and keeps failing until the 120s idle timeout closes it.
Private service denials now set Connection: close and Cache-Control: no-store before the 403, both at the tunnel-peer check and at IP restriction denials on a private domain. Go's HTTP/1.1 server closes after the response; its HTTP/2 server turns the exact lowercase close token into a GOAWAY, which retires the stale connection for h2 clients. Public services and allowed private traffic keep their keep-alive behaviour.
Tests cover HTTP/1.1 and HTTP/2 denials over a real listener (retry lands on a new connection), public denials and allowed private requests (connection reused), and both IP restriction paths.
Declare the runtime dependencies, generate the changelog from git tags with
chglog at release time, and ship LICENSE, README.md and an example
/etc/sysconfig/netbird as %license, %doc and %config(noreplace). The unit
generated by "netbird service install" already reads that path via
EnvironmentFile, so post_install.sh is unchanged.
AGENTS.md forbids attribution trailers, but a rule an agent has to go and read loses to the instruction it is handed every turn. CLAUDE.md now imports AGENTS.md so it is always in context; a commit-msg hook (via make setup-hooks) refuses the trailers at commit time; a CodeRabbit pre-merge check flags a PR whose description or commits carry them. The check reports rather than blocks, since the repository keeps CodeRabbit's request-changes workflow off; turning that on is a separate, repository-wide decision.
Every curl invocation in the install script that follows redirects now passes
`--proto` and `--proto-redir` set to https only, so neither the initial request
nor any hop in the redirect chain can drop to plaintext. This matters most for
the macOS .pkg download, whose URL is itself the result of a redirect
resolution, and for the release tarballs that get moved into the install dir as
root.
The protocol set lives in a single `PROTO_HTTPS` variable rather than being
repeated at each call site, and every expansion is quoted — the variable holds
one option value, not a list of flags.
The two call sites without `-L` (the release metadata lookups) are left alone:
they do not follow redirects and their URLs are https literals.
Verified against every URL the script fetches on curl 7.29.0 (CentOS 7),
7.68.0, 7.76.1, 7.88.1 and 8.14.1; both options have existed since curl 7.20.0.
Plaintext http:// is refused on all of them.
Viktor Liu