Files
netbird/client/internal/peer/worker_relay.go
T
Zoltan Papp bc0671fd21 [client] Fix peers not being notified when the relay connection drops (#7490)
* [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.
2026-09-21 17:00:37 +02:00

138 lines
3.3 KiB
Go

package peer
import (
"context"
"errors"
"net/netip"
"sync"
"sync/atomic"
log "github.com/sirupsen/logrus"
relayClient "github.com/netbirdio/netbird/shared/relay/client"
)
type RelayConnInfo struct {
relayedConn *relayClient.Conn
rosenpassPubKey []byte
rosenpassAddr string
}
type WorkerRelay struct {
peerCtx context.Context
log *log.Entry
isController bool
config ConnConfig
conn *Conn
relayManager *relayClient.Manager
relayedConn *relayClient.Conn
relayLock sync.Mutex
relaySupportedOnRemotePeer atomic.Bool
}
func NewWorkerRelay(ctx context.Context, log *log.Entry, ctrl bool, config ConnConfig, conn *Conn, relayManager *relayClient.Manager) *WorkerRelay {
r := &WorkerRelay{
peerCtx: ctx,
log: log,
isController: ctrl,
config: config,
conn: conn,
relayManager: relayManager,
}
return r
}
func (w *WorkerRelay) OnNewOffer(remoteOfferAnswer *OfferAnswer) {
if !w.isRelaySupported(remoteOfferAnswer) {
w.log.Infof("Relay is not supported by remote peer")
w.relaySupportedOnRemotePeer.Store(false)
return
}
w.relaySupportedOnRemotePeer.Store(true)
// the relayManager will return with error in case if the connection has lost with relay server
currentRelayAddress, _, err := w.relayManager.RelayInstanceAddress()
if err != nil {
w.log.Errorf("failed to handle new offer: %s", err)
return
}
srv := w.preferredRelayServer(currentRelayAddress, remoteOfferAnswer.RelaySrvAddress)
var serverIP netip.Addr
if srv == remoteOfferAnswer.RelaySrvAddress {
serverIP = remoteOfferAnswer.RelaySrvIP
}
relayedConn, err := w.relayManager.OpenConn(w.peerCtx, srv, w.config.Key, serverIP)
if err != nil {
if errors.Is(err, relayClient.ErrConnAlreadyExists) {
w.log.Debugf("handled offer by reusing existing relay connection")
return
}
w.log.Errorf("failed to open connection via Relay: %s", err)
return
}
w.relayLock.Lock()
w.relayedConn = relayedConn
w.relayLock.Unlock()
go w.watchRelayedConn(relayedConn)
w.log.Debugf("peer conn opened via Relay: %s", srv)
go w.conn.onRelayConnectionIsReady(RelayConnInfo{
relayedConn: relayedConn,
rosenpassPubKey: remoteOfferAnswer.RosenpassPubKey,
rosenpassAddr: remoteOfferAnswer.RosenpassAddr,
})
}
func (w *WorkerRelay) RelayInstanceAddress() (string, netip.Addr, error) {
return w.relayManager.RelayInstanceAddress()
}
func (w *WorkerRelay) IsRelayConnectionSupportedWithPeer() bool {
return w.relaySupportedOnRemotePeer.Load() && w.RelayIsSupportedLocally()
}
func (w *WorkerRelay) RelayIsSupportedLocally() bool {
return w.relayManager.HasRelayAddress()
}
func (w *WorkerRelay) CloseConn() {
w.relayLock.Lock()
conn := w.relayedConn
w.relayedConn = nil
w.relayLock.Unlock()
if conn == nil {
return
}
if err := conn.Close(); err != nil {
w.log.Warnf("failed to close relay connection: %v", err)
}
}
func (w *WorkerRelay) isRelaySupported(answer *OfferAnswer) bool {
if !w.relayManager.HasRelayAddress() {
return false
}
return answer.RelaySrvAddress != ""
}
func (w *WorkerRelay) preferredRelayServer(myRelayAddress, remoteRelayAddress string) string {
if w.isController {
return myRelayAddress
}
return remoteRelayAddress
}
func (w *WorkerRelay) watchRelayedConn(relayedConn *relayClient.Conn) {
<-relayedConn.Context().Done()
w.conn.onRelayDisconnected(relayedConn)
}