diff --git a/.golangci.yaml b/.golangci.yaml index 3c5f4d5b8..44b03d0e1 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -130,3 +130,10 @@ issues: - path: mock\.go linters: - nilnil + # Exclude specific deprecation warnings for grpc methods + - linters: + - staticcheck + text: "grpc.DialContext is deprecated" + - linters: + - staticcheck + text: "grpc.WithBlock is deprecated" diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index b4a1a61c8..7eb8e4e60 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -5,7 +5,7 @@ We as members, contributors, and leaders pledge to make participation in our community a harassment-free experience for everyone, regardless of age, body size, visible or invisible disability, ethnicity, sex characteristics, gender -identity and expression, level of experience, education, socio-economic status, +identity and expression, level of experience, education, socioeconomic status, nationality, personal appearance, race, caste, color, religion, or sexual identity and orientation. diff --git a/client/android/client.go b/client/android/client.go index d0efb47ed..d937e132e 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -57,15 +57,17 @@ type Client struct { ctxCancel context.CancelFunc ctxCancelLock *sync.Mutex deviceName string + uiVersion string networkChangeListener listener.NetworkChangeListener } // NewClient instantiate a new Client -func NewClient(cfgFile, deviceName string, tunAdapter TunAdapter, iFaceDiscover IFaceDiscover, networkChangeListener NetworkChangeListener) *Client { +func NewClient(cfgFile, deviceName string, uiVersion string, tunAdapter TunAdapter, iFaceDiscover IFaceDiscover, networkChangeListener NetworkChangeListener) *Client { net.SetAndroidProtectSocketFn(tunAdapter.ProtectSocket) return &Client{ cfgFile: cfgFile, deviceName: deviceName, + uiVersion: uiVersion, tunAdapter: tunAdapter, iFaceDiscover: iFaceDiscover, recorder: peer.NewRecorder(""), @@ -88,6 +90,9 @@ func (c *Client) Run(urlOpener URLOpener, dns *DNSList, dnsReadyListener DnsRead var ctx context.Context //nolint ctxWithValues := context.WithValue(context.Background(), system.DeviceNameCtxKey, c.deviceName) + //nolint + ctxWithValues = context.WithValue(ctxWithValues, system.UiVersionCtxKey, c.uiVersion) + c.ctxCancelLock.Lock() ctx, c.ctxCancel = context.WithCancel(ctxWithValues) defer c.ctxCancel() diff --git a/client/cmd/debug.go b/client/cmd/debug.go index 4deff11a6..da5e0945a 100644 --- a/client/cmd/debug.go +++ b/client/cmd/debug.go @@ -3,13 +3,14 @@ package cmd import ( "context" "fmt" - "strings" "time" "github.com/spf13/cobra" "google.golang.org/grpc/status" + "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/proto" + "github.com/netbirdio/netbird/client/server" ) var debugCmd = &cobra.Command{ @@ -58,7 +59,7 @@ var forCmd = &cobra.Command{ } func debugBundle(cmd *cobra.Command, _ []string) error { - conn, err := getClient(cmd.Context()) + conn, err := getClient(cmd) if err != nil { return err } @@ -79,14 +80,14 @@ func debugBundle(cmd *cobra.Command, _ []string) error { } func setLogLevel(cmd *cobra.Command, args []string) error { - conn, err := getClient(cmd.Context()) + conn, err := getClient(cmd) if err != nil { return err } defer conn.Close() client := proto.NewDaemonServiceClient(conn) - level := parseLogLevel(args[0]) + level := server.ParseLogLevel(args[0]) if level == proto.LogLevel_UNKNOWN { return fmt.Errorf("unknown log level: %s. Available levels are: panic, fatal, error, warn, info, debug, trace\n", args[0]) } @@ -102,34 +103,13 @@ func setLogLevel(cmd *cobra.Command, args []string) error { return nil } -func parseLogLevel(level string) proto.LogLevel { - switch strings.ToLower(level) { - case "panic": - return proto.LogLevel_PANIC - case "fatal": - return proto.LogLevel_FATAL - case "error": - return proto.LogLevel_ERROR - case "warn": - return proto.LogLevel_WARN - case "info": - return proto.LogLevel_INFO - case "debug": - return proto.LogLevel_DEBUG - case "trace": - return proto.LogLevel_TRACE - default: - return proto.LogLevel_UNKNOWN - } -} - func runForDuration(cmd *cobra.Command, args []string) error { duration, err := time.ParseDuration(args[0]) if err != nil { return fmt.Errorf("invalid duration format: %v", err) } - conn, err := getClient(cmd.Context()) + conn, err := getClient(cmd) if err != nil { return err } @@ -137,18 +117,33 @@ func runForDuration(cmd *cobra.Command, args []string) error { client := proto.NewDaemonServiceClient(conn) + stat, err := client.Status(cmd.Context(), &proto.StatusRequest{}) + if err != nil { + return fmt.Errorf("failed to get status: %v", status.Convert(err).Message()) + } + + restoreUp := stat.Status == string(internal.StatusConnected) || stat.Status == string(internal.StatusConnecting) + + initialLogLevel, err := client.GetLogLevel(cmd.Context(), &proto.GetLogLevelRequest{}) + if err != nil { + return fmt.Errorf("failed to get log level: %v", status.Convert(err).Message()) + } + if _, err := client.Down(cmd.Context(), &proto.DownRequest{}); err != nil { return fmt.Errorf("failed to down: %v", status.Convert(err).Message()) } cmd.Println("Netbird down") - _, err = client.SetLogLevel(cmd.Context(), &proto.SetLogLevelRequest{ - Level: proto.LogLevel_TRACE, - }) - if err != nil { - return fmt.Errorf("failed to set log level to trace: %v", status.Convert(err).Message()) + initialLevelTrace := initialLogLevel.GetLevel() >= proto.LogLevel_TRACE + if !initialLevelTrace { + _, err = client.SetLogLevel(cmd.Context(), &proto.SetLogLevelRequest{ + Level: proto.LogLevel_TRACE, + }) + if err != nil { + return fmt.Errorf("failed to set log level to TRACE: %v", status.Convert(err).Message()) + } + cmd.Println("Log level set to trace.") } - cmd.Println("Log level set to trace.") time.Sleep(1 * time.Second) @@ -175,10 +170,22 @@ func runForDuration(cmd *cobra.Command, args []string) error { } cmd.Println("Netbird down") - // TODO reset log level - time.Sleep(1 * time.Second) + if restoreUp { + if _, err := client.Up(cmd.Context(), &proto.UpRequest{}); err != nil { + return fmt.Errorf("failed to up: %v", status.Convert(err).Message()) + } + cmd.Println("Netbird up") + } + + if !initialLevelTrace { + if _, err := client.SetLogLevel(cmd.Context(), &proto.SetLogLevelRequest{Level: initialLogLevel.GetLevel()}); err != nil { + return fmt.Errorf("failed to restore log level: %v", status.Convert(err).Message()) + } + cmd.Println("Log level restored to", initialLogLevel.GetLevel()) + } + cmd.Println("Creating debug bundle...") resp, err := client.DebugBundle(cmd.Context(), &proto.DebugBundleRequest{ diff --git a/client/cmd/down.go b/client/cmd/down.go index d906059ca..1837b13da 100644 --- a/client/cmd/down.go +++ b/client/cmd/down.go @@ -2,9 +2,10 @@ package cmd import ( "context" - "github.com/netbirdio/netbird/util" "time" + "github.com/netbirdio/netbird/util" + log "github.com/sirupsen/logrus" "github.com/spf13/cobra" diff --git a/client/cmd/root.go b/client/cmd/root.go index 1eca27d8c..839380712 100644 --- a/client/cmd/root.go +++ b/client/cmd/root.go @@ -353,8 +353,11 @@ func migrateToNetbird(oldPath, newPath string) bool { return true } -func getClient(ctx context.Context) (*grpc.ClientConn, error) { - conn, err := DialClientGRPCServer(ctx, daemonAddr) +func getClient(cmd *cobra.Command) (*grpc.ClientConn, error) { + SetFlagsFromEnvVars(rootCmd) + cmd.SetOut(cmd.OutOrStdout()) + + conn, err := DialClientGRPCServer(cmd.Context(), daemonAddr) if err != nil { return nil, fmt.Errorf("failed to connect to daemon error: %v\n"+ "If the daemon is not running please run: "+ diff --git a/client/cmd/route.go b/client/cmd/route.go index 95cedb8ba..d92e079ad 100644 --- a/client/cmd/route.go +++ b/client/cmd/route.go @@ -49,7 +49,7 @@ func init() { } func routesList(cmd *cobra.Command, _ []string) error { - conn, err := getClient(cmd.Context()) + conn, err := getClient(cmd) if err != nil { return err } @@ -79,7 +79,7 @@ func routesList(cmd *cobra.Command, _ []string) error { } func routesSelect(cmd *cobra.Command, args []string) error { - conn, err := getClient(cmd.Context()) + conn, err := getClient(cmd) if err != nil { return err } @@ -106,7 +106,7 @@ func routesSelect(cmd *cobra.Command, args []string) error { } func routesDeselect(cmd *cobra.Command, args []string) error { - conn, err := getClient(cmd.Context()) + conn, err := getClient(cmd) if err != nil { return err } diff --git a/client/firewall/create_linux.go b/client/firewall/create_linux.go index a872e11c4..92deb63dc 100644 --- a/client/firewall/create_linux.go +++ b/client/firewall/create_linux.go @@ -42,20 +42,20 @@ func NewFirewall(context context.Context, iface IFaceMapper) (firewall.Manager, switch check() { case IPTABLES: - log.Debug("creating an iptables firewall manager") + log.Info("creating an iptables firewall manager") fm, errFw = nbiptables.Create(context, iface) if errFw != nil { log.Errorf("failed to create iptables manager: %s", errFw) } case NFTABLES: - log.Debug("creating an nftables firewall manager") + log.Info("creating an nftables firewall manager") fm, errFw = nbnftables.Create(context, iface) if errFw != nil { log.Errorf("failed to create nftables manager: %s", errFw) } default: errFw = fmt.Errorf("no firewall manager found") - log.Debug("no firewall manager found, try to use userspace packet filtering firewall") + log.Info("no firewall manager found, trying to use userspace packet filtering firewall") } if iface.IsUserspaceBind() { @@ -85,16 +85,58 @@ func NewFirewall(context context.Context, iface IFaceMapper) (firewall.Manager, // check returns the firewall type based on common lib checks. It returns UNKNOWN if no firewall is found. func check() FWType { - nf := nftables.Conn{} - if _, err := nf.ListChains(); err == nil && os.Getenv(SKIP_NFTABLES_ENV) != "true" { - return NFTABLES + useIPTABLES := false + var iptablesChains []string + ip, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) + if err == nil && isIptablesClientAvailable(ip) { + major, minor, _ := ip.GetIptablesVersion() + // use iptables when its version is lower than 1.8.0 which doesn't work well with our nftables manager + if major < 1 || (major == 1 && minor < 8) { + return IPTABLES + } + + useIPTABLES = true + + iptablesChains, err = ip.ListChains("filter") + if err != nil { + log.Errorf("failed to list iptables chains: %s", err) + useIPTABLES = false + } } - ip, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) - if err != nil { - return UNKNOWN + nf := nftables.Conn{} + if chains, err := nf.ListChains(); err == nil && os.Getenv(SKIP_NFTABLES_ENV) != "true" { + if !useIPTABLES { + return NFTABLES + } + + // search for chains where table is filter + // if we find one, we assume that nftables manager can be used with iptables + for _, chain := range chains { + if chain.Table.Name == "filter" { + return NFTABLES + } + } + + // check tables for the following constraints: + // 1. there is no chain in nftables for the filter table and there is at least one chain in iptables, we assume that nftables manager can not be used + // 2. there is no tables or more than one table, we assume that nftables manager can be used + // 3. there is only one table and its name is filter, we assume that nftables manager can not be used, since there was no chain in it + // 4. if we find an error we log and continue with iptables check + nbTablesList, err := nf.ListTables() + switch { + case err == nil && len(iptablesChains) > 0: + return IPTABLES + case err == nil && len(nbTablesList) != 1: + return NFTABLES + case err == nil && len(nbTablesList) == 1 && nbTablesList[0].Name == "filter": + return IPTABLES + case err != nil: + log.Errorf("failed to list nftables tables on fw manager discovery: %s", err) + } } - if isIptablesClientAvailable(ip) { + + if useIPTABLES { return IPTABLES } diff --git a/client/internal/connect.go b/client/internal/connect.go index 83909dfdd..d34d0aab0 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net" "runtime" "runtime/debug" "strings" @@ -330,6 +331,15 @@ func createEngineConfig(key wgtypes.Key, config *Config, peerConfig *mgmProto.Pe engineConf.PreSharedKey = &preSharedKey } + port, err := freePort(config.WgPort) + if err != nil { + return nil, err + } + if port != config.WgPort { + log.Infof("using %d as wireguard port: %d is in use", port, config.WgPort) + } + engineConf.WgPort = port + return engineConf, nil } @@ -379,3 +389,20 @@ func statusRecorderToSignalConnStateNotifier(statusRecorder *peer.Status) signal notifier, _ := sri.(signal.ConnStateNotifier) return notifier } + +func freePort(start int) (int, error) { + addr := net.UDPAddr{} + if start == 0 { + start = iface.DefaultWgPort + } + for x := start; x <= 65535; x++ { + addr.Port = x + conn, err := net.ListenUDP("udp", &addr) + if err != nil { + continue + } + conn.Close() + return x, nil + } + return 0, errors.New("no free ports") +} diff --git a/client/internal/connect_test.go b/client/internal/connect_test.go new file mode 100644 index 000000000..6f4a6bbb7 --- /dev/null +++ b/client/internal/connect_test.go @@ -0,0 +1,57 @@ +package internal + +import ( + "net" + "testing" +) + +func Test_freePort(t *testing.T) { + tests := []struct { + name string + port int + want int + wantErr bool + }{ + { + name: "available", + port: 51820, + want: 51820, + wantErr: false, + }, + { + name: "notavailable", + port: 51830, + want: 51831, + wantErr: false, + }, + { + name: "noports", + port: 65535, + want: 0, + wantErr: true, + }, + } + for _, tt := range tests { + + c1, err := net.ListenUDP("udp", &net.UDPAddr{Port: 51830}) + if err != nil { + t.Errorf("freePort error = %v", err) + } + c2, err := net.ListenUDP("udp", &net.UDPAddr{Port: 65535}) + if err != nil { + t.Errorf("freePort error = %v", err) + } + t.Run(tt.name, func(t *testing.T) { + got, err := freePort(tt.port) + if (err != nil) != tt.wantErr { + t.Errorf("freePort() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("freePort() = %v, want %v", got, tt.want) + } + }) + c1.Close() + c2.Close() + } +} diff --git a/client/internal/engine.go b/client/internal/engine.go index 351b21b2e..b09235714 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "maps" "math/rand" "net" "net/netip" @@ -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 @@ -150,6 +152,8 @@ type Engine struct { signalProbe *Probe relayProbe *Probe wgProbe *Probe + + wgConnWorker sync.WaitGroup } // Peer is an instance of the Connection Peer @@ -238,13 +242,16 @@ 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 time.Sleep(500 * time.Millisecond) e.close() + e.wgConnWorker.Wait() log.Infof("stopped Netbird Engine") return nil } @@ -735,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 { @@ -869,18 +878,25 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error { log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err) } + e.wgConnWorker.Add(1) go e.connWorker(conn, peerKey) } return nil } func (e *Engine) connWorker(conn *peer.Conn, peerKey string) { + defer e.wgConnWorker.Done() for { // randomize starting time a bit min := 500 max := 2000 - time.Sleep(time.Duration(rand.Intn(max-min)+min) * time.Millisecond) + duration := time.Duration(rand.Intn(max-min)+min) * time.Millisecond + select { + case <-e.ctx.Done(): + return + case <-time.After(duration): + } // if peer has been removed -> give up if !e.peerExists(peerKey) { @@ -967,7 +983,6 @@ func (e *Engine) createPeerConn(pubKey string, allowedIPs string) (*peer.Conn, e WgConfig: wgConfig, LocalWgPort: e.config.WgPort, NATExternalIPs: e.parseNATExternalIPMappings(), - UserspaceBind: e.wgInterface.IsUserspaceBind(), RosenpassPubKey: e.getRosenpassPubKey(), RosenpassAddr: e.getRosenpassAddr(), } @@ -1030,8 +1045,6 @@ func (e *Engine) receiveSignalEvents() { return err } - conn.RegisterProtoSupportMeta(msg.Body.GetFeaturesSupported()) - var rosenpassPubKey []byte rosenpassAddr := "" if msg.GetBody().GetRosenpassConfig() != nil { @@ -1054,8 +1067,6 @@ func (e *Engine) receiveSignalEvents() { return err } - conn.RegisterProtoSupportMeta(msg.GetBody().GetFeaturesSupported()) - var rosenpassPubKey []byte rosenpassAddr := "" if msg.GetBody().GetRosenpassConfig() != nil { @@ -1078,7 +1089,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: } @@ -1272,11 +1284,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 diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 0239ae58a..f5a98cb7f 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -229,6 +229,7 @@ func TestEngine_UpdateNetworkMap(t *testing.T) { t.Fatal(err) } engine.udpMux = bind.NewUniversalUDPMuxDefault(bind.UniversalUDPMuxParams{UDPConn: conn}) + engine.ctx = ctx type testCase struct { name string @@ -408,6 +409,7 @@ func TestEngine_Sync(t *testing.T) { WgPrivateKey: key, WgPort: 33100, }, MobileDependency{}, peer.NewRecorder("https://mgm")) + engine.ctx = ctx engine.dnsServer = &dns.MockServer{ UpdateDNSServerFunc: func(serial uint64, update nbdns.Config) error { return nil }, @@ -566,6 +568,7 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) { WgPrivateKey: key, WgPort: 33100, }, MobileDependency{}, peer.NewRecorder("https://mgm")) + engine.ctx = ctx newNet, err := stdnet.NewNet() if err != nil { t.Fatal(err) @@ -735,6 +738,8 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) { WgPrivateKey: key, WgPort: 33100, }, MobileDependency{}, peer.NewRecorder("https://mgm")) + engine.ctx = ctx + newNet, err := stdnet.NewNet() if err != nil { t.Fatal(err) @@ -1003,7 +1008,9 @@ func createEngine(ctx context.Context, cancel context.CancelFunc, setupKey strin WgPort: wgPort, } - return NewEngine(ctx, cancel, signalClient, mgmtClient, conf, MobileDependency{}, peer.NewRecorder("https://mgm")), nil + e, err := NewEngine(ctx, cancel, signalClient, mgmtClient, conf, MobileDependency{}, peer.NewRecorder("https://mgm")), nil + e.ctx = ctx + return e, err } func startSignal() (*grpc.Server, string, error) { diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index 3c4cfb13a..c64c074a7 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net" + "net/netip" "runtime" "strings" "sync" @@ -18,7 +19,7 @@ import ( "github.com/netbirdio/netbird/client/internal/wgproxy" "github.com/netbirdio/netbird/iface" "github.com/netbirdio/netbird/iface/bind" - signal "github.com/netbirdio/netbird/signal/client" + "github.com/netbirdio/netbird/route" sProto "github.com/netbirdio/netbird/signal/proto" nbnet "github.com/netbirdio/netbird/util/net" "github.com/netbirdio/netbird/version" @@ -68,9 +69,6 @@ type ConnConfig struct { NATExternalIPs []string - // UsesBind indicates whether the WireGuard interface is userspace and uses bind.ICEBind - UserspaceBind bool - // RosenpassPubKey is this peer's Rosenpass public key RosenpassPubKey []byte // RosenpassPubKey is this peer's RosenpassAddr server address (IP:port) @@ -133,32 +131,15 @@ type Conn struct { wgProxyFactory *wgproxy.Factory wgProxy wgproxy.Proxy - remoteModeCh chan ModeMessage - meta meta - adapter iface.TunAdapter iFaceDiscover stdnet.ExternalIFaceDiscover sentExtraSrflx bool - remoteEndpoint *net.UDPAddr - remoteConn *ice.Conn - connID nbnet.ConnectionID beforeAddPeerHooks []BeforeAddPeerHookFunc afterRemovePeerHooks []AfterRemovePeerHookFunc } -// meta holds meta information about a connection -type meta struct { - protoSupport signal.FeaturesSupport -} - -// ModeMessage represents a connection mode chosen by the peer -type ModeMessage struct { - // Direct indicates that it decided to use a direct connection - Direct bool -} - // GetConf returns the connection config func (conn *Conn) GetConf() ConnConfig { return conn.config @@ -185,7 +166,6 @@ func NewConn(config ConnConfig, statusRecorder *Status, wgProxyFactory *wgproxy. remoteOffersCh: make(chan OfferAnswer), remoteAnswerCh: make(chan OfferAnswer), statusRecorder: statusRecorder, - remoteModeCh: make(chan ModeMessage, 1), wgProxyFactory: wgProxyFactory, adapter: adapter, iFaceDiscover: iFaceDiscover, @@ -353,7 +333,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 @@ -370,14 +350,12 @@ 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 } - conn.remoteConn = remoteConn - // the ice connection has been established successfully so we are ready to start the proxy remoteAddr, err := conn.configureConnection(remoteConn, remoteWgPort, remoteOfferAnswer.RosenpassPubKey, remoteOfferAnswer.RosenpassAddr) @@ -435,7 +413,6 @@ func (conn *Conn) configureConnection(remoteConn net.Conn, remoteWgPort int, rem } endpointUdpAddr, _ := net.ResolveUDPAddr(endpoint.Network(), endpoint.String()) - conn.remoteEndpoint = endpointUdpAddr log.Debugf("Conn resolved IP for %s: %s", endpoint, endpointUdpAddr.IP) conn.connID = nbnet.GenerateConnID() @@ -621,40 +598,39 @@ func (conn *Conn) SetSendSignalMessage(handler func(message *sProto.Message) err // onICECandidate is a callback attached to an ICE Agent to receive new local connection candidates // and then signals them to the remote peer func (conn *Conn) onICECandidate(candidate ice.Candidate) { - if candidate != nil { - // TODO: reported port is incorrect for CandidateTypeHost, makes understanding ICE use via logs confusing as port is ignored - log.Debugf("discovered local candidate %s", candidate.String()) - go func() { - err := conn.signalCandidate(candidate) - if err != nil { - log.Errorf("failed signaling candidate to the remote peer %s %s", conn.config.Key, err) - } - - // sends an extra server reflexive candidate to the remote peer with our related port (usually the wireguard port) - // this is useful when network has an existing port forwarding rule for the wireguard port and this peer - if !conn.sentExtraSrflx && candidate.Type() == ice.CandidateTypeServerReflexive && candidate.Port() != candidate.RelatedAddress().Port { - relatedAdd := candidate.RelatedAddress() - extraSrflx, err := ice.NewCandidateServerReflexive(&ice.CandidateServerReflexiveConfig{ - Network: candidate.NetworkType().String(), - Address: candidate.Address(), - Port: relatedAdd.Port, - Component: candidate.Component(), - RelAddr: relatedAdd.Address, - RelPort: relatedAdd.Port, - }) - if err != nil { - log.Errorf("failed creating extra server reflexive candidate %s", err) - return - } - err = conn.signalCandidate(extraSrflx) - if err != nil { - log.Errorf("failed signaling the extra server reflexive candidate to the remote peer %s: %s", conn.config.Key, err) - return - } - conn.sentExtraSrflx = true - } - }() + // nil means candidate gathering has been ended + if candidate == nil { + return } + + // TODO: reported port is incorrect for CandidateTypeHost, makes understanding ICE use via logs confusing as port is ignored + log.Debugf("discovered local candidate %s", candidate.String()) + go func() { + err := conn.signalCandidate(candidate) + if err != nil { + log.Errorf("failed signaling candidate to the remote peer %s %s", conn.config.Key, err) + } + }() + + if !conn.shouldSendExtraSrflxCandidate(candidate) { + return + } + + // sends an extra server reflexive candidate to the remote peer with our related port (usually the wireguard port) + // this is useful when network has an existing port forwarding rule for the wireguard port and this peer + extraSrflx, err := extraSrflxCandidate(candidate) + if err != nil { + log.Errorf("failed creating extra server reflexive candidate %s", err) + return + } + conn.sentExtraSrflx = true + + go func() { + err = conn.signalCandidate(extraSrflx) + if err != nil { + log.Errorf("failed signaling the extra server reflexive candidate to the remote peer %s: %s", conn.config.Key, err) + } + }() } func (conn *Conn) onICESelectedCandidatePair(c1 ice.Candidate, c2 ice.Candidate) { @@ -779,7 +755,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() @@ -789,6 +765,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) @@ -801,8 +781,49 @@ func (conn *Conn) GetKey() string { return conn.config.Key } -// RegisterProtoSupportMeta register supported proto message in the connection metadata -func (conn *Conn) RegisterProtoSupportMeta(support []uint32) { - protoSupport := signal.ParseFeaturesSupported(support) - conn.meta.protoSupport = protoSupport +func (conn *Conn) shouldSendExtraSrflxCandidate(candidate ice.Candidate) bool { + if !conn.sentExtraSrflx && candidate.Type() == ice.CandidateTypeServerReflexive && candidate.Port() != candidate.RelatedAddress().Port { + return true + } + return false +} + +func extraSrflxCandidate(candidate ice.Candidate) (*ice.CandidateServerReflexive, error) { + relatedAdd := candidate.RelatedAddress() + return ice.NewCandidateServerReflexive(&ice.CandidateServerReflexiveConfig{ + Network: candidate.NetworkType().String(), + Address: candidate.Address(), + Port: relatedAdd.Port, + Component: candidate.Component(), + RelAddr: relatedAdd.Address, + RelPort: relatedAdd.Port, + }) +} + +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 } diff --git a/client/internal/relay/relay.go b/client/internal/relay/relay.go index 84fd72e49..4542a37fe 100644 --- a/client/internal/relay/relay.go +++ b/client/internal/relay/relay.go @@ -170,7 +170,7 @@ func ProbeAll( var wg sync.WaitGroup for i, uri := range relays { - ctx, cancel := context.WithTimeout(ctx, 1*time.Second) + ctx, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() wg.Add(1) diff --git a/client/internal/routemanager/systemops_darwin.go b/client/internal/routemanager/systemops_darwin.go index 017dc6c28..ee4196a0c 100644 --- a/client/internal/routemanager/systemops_darwin.go +++ b/client/internal/routemanager/systemops_darwin.go @@ -43,11 +43,6 @@ func routeCmd(action string, prefix netip.Prefix, nexthop netip.Addr, intf *net. } if prefix.Addr().Is6() { inet = "-inet6" - // Special case for IPv6 split default route, pointing to the wg interface fails - // TODO: Remove once we have IPv6 support on the interface - if prefix.Bits() == 1 { - intf = &net.Interface{Name: "lo0"} - } } args := []string{"-n", action, inet, network} diff --git a/client/internal/templates/pkce-auth-msg.html b/client/internal/templates/pkce-auth-msg.html index efd1e06a3..4825c48e7 100644 --- a/client/internal/templates/pkce-auth-msg.html +++ b/client/internal/templates/pkce-auth-msg.html @@ -1,7 +1,7 @@ - +