Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore candidates whose IP falls into a routed network #2084

Merged
merged 1 commit into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions client/internal/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"maps"
"math/rand"
"net"
"net/netip"
Expand Down Expand Up @@ -117,7 +118,8 @@ type Engine struct {
TURNs []*stun.URI

// clientRoutes is the most recent list of clientRoutes received from the Management Service
clientRoutes route.HAMap
clientRoutes route.HAMap
clientRoutesMu sync.RWMutex

clientCtx context.Context
clientCancel context.CancelFunc
Expand Down Expand Up @@ -240,7 +242,9 @@ func (e *Engine) Stop() error {
return err
}

e.clientRoutesMu.Lock()
e.clientRoutes = nil
e.clientRoutesMu.Unlock()

// very ugly but we want to remove peers from the WireGuard interface first before removing interface.
// Removing peers happens in the conn.Close() asynchronously
Expand Down Expand Up @@ -738,7 +742,9 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error {
log.Errorf("failed to update clientRoutes, err: %v", err)
}

e.clientRoutesMu.Lock()
e.clientRoutes = clientRoutes
e.clientRoutesMu.Unlock()

protoDNSConfig := networkMap.GetDNSConfig()
if protoDNSConfig == nil {
Expand Down Expand Up @@ -1088,7 +1094,8 @@ func (e *Engine) receiveSignalEvents() {
log.Errorf("failed on parsing remote candidate %s -> %s", candidate, err)
return err
}
conn.OnRemoteCandidate(candidate)

conn.OnRemoteCandidate(candidate, e.GetClientRoutes())
case sProto.Body_MODE:
}

Expand Down Expand Up @@ -1282,11 +1289,17 @@ func (e *Engine) newDnsServer() ([]*route.Route, dns.Server, error) {

// GetClientRoutes returns the current routes from the route map
func (e *Engine) GetClientRoutes() route.HAMap {
return e.clientRoutes
e.clientRoutesMu.RLock()
defer e.clientRoutesMu.RUnlock()

return maps.Clone(e.clientRoutes)
}

// GetClientRoutesWithNetID returns the current routes from the route map, but the keys consist of the network ID only
func (e *Engine) GetClientRoutesWithNetID() map[route.NetID][]*route.Route {
e.clientRoutesMu.RLock()
defer e.clientRoutesMu.RUnlock()

routes := make(map[route.NetID][]*route.Route, len(e.clientRoutes))
for id, v := range e.clientRoutes {
routes[id.NetID()] = v
Expand Down
40 changes: 37 additions & 3 deletions client/internal/peer/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net"
"net/netip"
"runtime"
"strings"
"sync"
Expand All @@ -18,6 +19,7 @@ import (
"github.com/netbirdio/netbird/client/internal/wgproxy"
"github.com/netbirdio/netbird/iface"
"github.com/netbirdio/netbird/iface/bind"
"github.com/netbirdio/netbird/route"
signal "github.com/netbirdio/netbird/signal/client"
sProto "github.com/netbirdio/netbird/signal/proto"
nbnet "github.com/netbirdio/netbird/util/net"
Expand Down Expand Up @@ -353,7 +355,7 @@ func (conn *Conn) Open(ctx context.Context) error {

err = conn.agent.GatherCandidates()
if err != nil {
return err
return fmt.Errorf("gather candidates: %v", err)
}

// will block until connection succeeded
Expand All @@ -370,7 +372,7 @@ func (conn *Conn) Open(ctx context.Context) error {
return err
}

// dynamically set remote WireGuard port is other side specified a different one from the default one
// dynamically set remote WireGuard port if other side specified a different one from the default one
remoteWgPort := iface.DefaultWgPort
if remoteOfferAnswer.WgListenPort != 0 {
remoteWgPort = remoteOfferAnswer.WgListenPort
Expand Down Expand Up @@ -779,7 +781,7 @@ func (conn *Conn) OnRemoteAnswer(answer OfferAnswer) bool {
}

// OnRemoteCandidate Handles ICE connection Candidate provided by the remote peer.
func (conn *Conn) OnRemoteCandidate(candidate ice.Candidate) {
func (conn *Conn) OnRemoteCandidate(candidate ice.Candidate, haRoutes route.HAMap) {
log.Debugf("OnRemoteCandidate from peer %s -> %s", conn.config.Key, candidate.String())
go func() {
conn.mu.Lock()
Expand All @@ -789,6 +791,10 @@ func (conn *Conn) OnRemoteCandidate(candidate ice.Candidate) {
return
}

if candidateViaRoutes(candidate, haRoutes) {
return
}

err := conn.agent.AddRemoteCandidate(candidate)
if err != nil {
log.Errorf("error while handling remote candidate from peer %s", conn.config.Key)
Expand All @@ -806,3 +812,31 @@ func (conn *Conn) RegisterProtoSupportMeta(support []uint32) {
protoSupport := signal.ParseFeaturesSupported(support)
conn.meta.protoSupport = protoSupport
}

func candidateViaRoutes(candidate ice.Candidate, clientRoutes route.HAMap) bool {
var routePrefixes []netip.Prefix
for _, routes := range clientRoutes {
if len(routes) > 0 && routes[0] != nil {
routePrefixes = append(routePrefixes, routes[0].Network)
}
}

addr, err := netip.ParseAddr(candidate.Address())
if err != nil {
log.Errorf("Failed to parse IP address %s: %v", candidate.Address(), err)
return false
}

for _, prefix := range routePrefixes {
// default route is
if prefix.Bits() == 0 {
continue
}

if prefix.Contains(addr) {
log.Debugf("Ignoring candidate [%s], its address is part of routed network %s", candidate.String(), prefix)
return true
}
}
return false
}
Loading