Separate discover methods (#787)

client/android/client.go

@@ -26,7 +26,7 @@ type TunAdapter interface {
 
 // IFaceDiscover export internal IFaceDiscover for mobile
 type IFaceDiscover interface {
-	stdnet.IFaceDiscover
+	stdnet.ExternalIFaceDiscover
 }
 
 func init() {

client/internal/connect.go

@@ -23,7 +23,7 @@ import (
 )
 
 // RunClient with main logic.
-func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status, tunAdapter iface.TunAdapter, iFaceDiscover stdnet.IFaceDiscover) error {
+func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status, tunAdapter iface.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover) error {
 	backOff := &backoff.ExponentialBackOff{
 		InitialInterval:     time.Second,
 		RandomizationFactor: 1,
@@ -195,7 +195,7 @@ func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status,
 }
 
 // createEngineConfig converts configuration received from Management Service to EngineConfig
-func createEngineConfig(key wgtypes.Key, config *Config, peerConfig *mgmProto.PeerConfig, tunAdapter iface.TunAdapter, iFaceDiscover stdnet.IFaceDiscover) (*EngineConfig, error) {
+func createEngineConfig(key wgtypes.Key, config *Config, peerConfig *mgmProto.PeerConfig, tunAdapter iface.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover) (*EngineConfig, error) {
 
 	engineConf := &EngineConfig{
 		WgIfaceName:          config.WgIface,

client/internal/dns/server_test.go

@@ -3,14 +3,16 @@ package dns
 import (
 	"context"
 	"fmt"
-	"github.com/netbirdio/netbird/client/internal/stdnet"
 	"net"
 	"net/netip"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/netbirdio/netbird/client/internal/stdnet"
+
 	"github.com/miekg/dns"
+
 	nbdns "github.com/netbirdio/netbird/dns"
 	"github.com/netbirdio/netbird/iface"
 )
@@ -200,7 +202,7 @@ func TestUpdateDNSServer(t *testing.T) {
 
 	for n, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			newNet, err := stdnet.NewNet(nil, nil)
+			newNet, err := stdnet.NewNet(nil)
 			if err != nil {
 				t.Fatal(err)
 			}

client/internal/engine.go

@@ -50,7 +50,7 @@ type EngineConfig struct {
 	// TunAdapter is option. It is necessary for mobile version.
 	TunAdapter iface.TunAdapter
 
-	IFaceDiscover stdnet.IFaceDiscover
+	IFaceDiscover stdnet.ExternalIFaceDiscover
 
 	// WgAddr is a Wireguard local address (Netbird Network IP)
 	WgAddr string

client/internal/engine_stdnet.go

@@ -7,5 +7,5 @@ import (
 )
 
 func (e *Engine) newStdNet() (*stdnet.Net, error) {
-	return stdnet.NewNet(nil, e.config.IFaceBlackList)
+	return stdnet.NewNet(e.config.IFaceBlackList)
 }

client/internal/engine_stdnet_android.go

@@ -3,5 +3,5 @@ package internal
 import "github.com/netbirdio/netbird/client/internal/stdnet"
 
 func (e *Engine) newStdNet() (*stdnet.Net, error) {
-	return stdnet.NewNet(e.config.IFaceDiscover, e.config.IFaceBlackList)
+	return stdnet.NewNetWithDiscover(e.config.IFaceDiscover, e.config.IFaceBlackList)
 }

client/internal/peer/conn.go

@@ -8,14 +8,15 @@ import (
 	"sync"
 	"time"
 
+	"github.com/pion/ice/v2"
+	log "github.com/sirupsen/logrus"
+
 	"github.com/netbirdio/netbird/client/internal/proxy"
 	"github.com/netbirdio/netbird/client/internal/stdnet"
 	"github.com/netbirdio/netbird/iface"
 	signal "github.com/netbirdio/netbird/signal/client"
 	sProto "github.com/netbirdio/netbird/signal/proto"
 	"github.com/netbirdio/netbird/version"
-	"github.com/pion/ice/v2"
-	log "github.com/sirupsen/logrus"
 )
 
 // ConnConfig is a peer Connection configuration
@@ -96,7 +97,7 @@ type Conn struct {
 	meta         meta
 
 	adapter       iface.TunAdapter
-	iFaceDiscover stdnet.IFaceDiscover
+	iFaceDiscover stdnet.ExternalIFaceDiscover
 }
 
 // meta holds meta information about a connection
@@ -122,7 +123,7 @@ func (conn *Conn) UpdateConf(conf ConnConfig) {
 
 // NewConn creates a new not opened Conn to the remote peer.
 // To establish a connection run Conn.Open
-func NewConn(config ConnConfig, statusRecorder *Status, adapter iface.TunAdapter, iFaceDiscover stdnet.IFaceDiscover) (*Conn, error) {
+func NewConn(config ConnConfig, statusRecorder *Status, adapter iface.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover) (*Conn, error) {
 	return &Conn{
 		config:         config,
 		mu:             sync.Mutex{},

client/internal/peer/stdnet.go

@@ -7,5 +7,5 @@ import (
 )
 
 func (conn *Conn) newStdNet() (*stdnet.Net, error) {
-	return stdnet.NewNet(nil, conn.config.InterfaceBlackList)
+	return stdnet.NewNet(conn.config.InterfaceBlackList)
 }

client/internal/peer/stdnet_android.go

@@ -3,5 +3,5 @@ package peer
 import "github.com/netbirdio/netbird/client/internal/stdnet"
 
 func (conn *Conn) newStdNet() (*stdnet.Net, error) {
-	return stdnet.NewNet(conn.iFaceDiscover, conn.config.InterfaceBlackList)
+	return stdnet.NewNetWithDiscover(conn.iFaceDiscover, conn.config.InterfaceBlackList)
 }

client/internal/stdnet/discover.go

@@ -0,0 +1,14 @@
+package stdnet
+
+import "github.com/pion/transport/v2"
+
+// ExternalIFaceDiscover provide an option for external services (mobile)
+// to collect network interface information
+type ExternalIFaceDiscover interface {
+	// IFaces return with the description of the interfaces
+	IFaces() (string, error)
+}
+
+type iFaceDiscover interface {
+	iFaces() ([]*transport.Interface, error)
+}

client/internal/stdnet/discover_mobile.go

@@ -0,0 +1,95 @@
+package stdnet
+
+import (
+	"fmt"
+	"net"
+	"strings"
+
+	"github.com/pion/transport/v2"
+	log "github.com/sirupsen/logrus"
+)
+
+type mobileIFaceDiscover struct {
+	externalDiscover ExternalIFaceDiscover
+}
+
+func newMobileIFaceDiscover(externalDiscover ExternalIFaceDiscover) *mobileIFaceDiscover {
+	return &mobileIFaceDiscover{
+		externalDiscover: externalDiscover,
+	}
+}
+
+func (m *mobileIFaceDiscover) iFaces() ([]*transport.Interface, error) {
+	ifacesString, err := m.externalDiscover.IFaces()
+	if err != nil {
+		return nil, err
+	}
+	interfaces := m.parseInterfacesString(ifacesString)
+	return interfaces, nil
+}
+
+func (m *mobileIFaceDiscover) parseInterfacesString(interfaces string) []*transport.Interface {
+	ifs := []*transport.Interface{}
+
+	for _, iface := range strings.Split(interfaces, "\n") {
+		if strings.TrimSpace(iface) == "" {
+			continue
+		}
+
+		fields := strings.Split(iface, "|")
+		if len(fields) != 2 {
+			log.Warnf("parseInterfacesString: unable to split %q", iface)
+			continue
+		}
+
+		var name string
+		var index, mtu int
+		var up, broadcast, loopback, pointToPoint, multicast bool
+		_, err := fmt.Sscanf(fields[0], "%s %d %d %t %t %t %t %t",
+			&name, &index, &mtu, &up, &broadcast, &loopback, &pointToPoint, &multicast)
+		if err != nil {
+			log.Warnf("parseInterfacesString: unable to parse %q: %v", iface, err)
+			continue
+		}
+
+		newIf := net.Interface{
+			Name:  name,
+			Index: index,
+			MTU:   mtu,
+		}
+		if up {
+			newIf.Flags |= net.FlagUp
+		}
+		if broadcast {
+			newIf.Flags |= net.FlagBroadcast
+		}
+		if loopback {
+			newIf.Flags |= net.FlagLoopback
+		}
+		if pointToPoint {
+			newIf.Flags |= net.FlagPointToPoint
+		}
+		if multicast {
+			newIf.Flags |= net.FlagMulticast
+		}
+
+		ifc := transport.NewInterface(newIf)
+
+		addrs := strings.Trim(fields[1], " \n")
+		foundAddress := false
+		for _, addr := range strings.Split(addrs, " ") {
+			ip, ipNet, err := net.ParseCIDR(addr)
+			if err != nil {
+				log.Warnf("%s", err)
+				continue
+			}
+			ipNet.IP = ip
+			ifc.AddAddress(ipNet)
+			foundAddress = true
+		}
+		if foundAddress {
+			ifs = append(ifs, ifc)
+		}
+	}
+	return ifs
+}

client/internal/stdnet/discover_mobile_test.go

@@ -35,7 +35,9 @@ func Test_parseInterfacesString(t *testing.T) {
 			d.multicast,
 			d.addr)
 	}
-	nets := parseInterfacesString(exampleString)
+
+	d := mobileIFaceDiscover{}
+	nets := d.parseInterfacesString(exampleString)
 	if len(nets) == 0 {
 		t.Fatalf("failed to parse interfaces")
 	}

client/internal/stdnet/discover_pion.go

@@ -0,0 +1,36 @@
+package stdnet
+
+import (
+	"net"
+
+	"github.com/pion/transport/v2"
+)
+
+type pionDiscover struct {
+}
+
+func (d pionDiscover) iFaces() ([]*transport.Interface, error) {
+	ifs := []*transport.Interface{}
+
+	oifs, err := net.Interfaces()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, oif := range oifs {
+		ifc := transport.NewInterface(oif)
+
+		addrs, err := oif.Addrs()
+		if err != nil {
+			return nil, err
+		}
+
+		for _, addr := range addrs {
+			ifc.AddAddress(addr)
+		}
+
+		ifs = append(ifs, ifc)
+	}
+
+	return ifs, nil
+}

client/internal/stdnet/filter.go

@@ -0,0 +1,40 @@
+package stdnet
+
+import (
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+	"golang.zx2c4.com/wireguard/wgctrl"
+)
+
+// InterfaceFilter is a function passed to ICE Agent to filter out not allowed interfaces
+// to avoid building tunnel over them.
+func InterfaceFilter(disallowList []string) func(string) bool {
+
+	return func(iFace string) bool {
+
+		if strings.HasPrefix(iFace, "lo") {
+			// hardcoded loopback check to support already installed agents
+			return false
+		}
+
+		for _, s := range disallowList {
+			if strings.HasPrefix(iFace, s) {
+				log.Debugf("ignoring interface %s - it is not allowed", iFace)
+				return false
+			}
+		}
+		// look for unlisted WireGuard interfaces
+		wg, err := wgctrl.New()
+		if err != nil {
+			log.Debugf("trying to create a wgctrl client failed with: %v", err)
+			return true
+		}
+		defer func() {
+			_ = wg.Close()
+		}()
+
+		_, err = wg.Device(iFace)
+		return err != nil
+	}
+}

client/internal/stdnet/iface_discover.go

@@ -1,9 +0,0 @@
-package stdnet
-
-// IFaceDiscover provide an option for external services (mobile)
-// to collect network interface information
-type IFaceDiscover interface {
-	// IFaces return with the description of the interfaces
-	// todo refactor this to return []*transport.Interface instead to have it generic and independent from the platform
-	IFaces() (string, error)
-}

client/internal/stdnet/stdnet.go

@@ -5,94 +5,52 @@ package stdnet
 
 import (
 	"fmt"
-	"golang.zx2c4.com/wireguard/wgctrl"
-	"net"
-	"strings"
 
 	"github.com/pion/transport/v2"
 	"github.com/pion/transport/v2/stdnet"
-	log "github.com/sirupsen/logrus"
 )
 
 // Net is an implementation of the net.Net interface
 // based on functions of the standard net package.
 type Net struct {
 	stdnet.Net
-	interfaces []*transport.Interface
+	interfaces    []*transport.Interface
+	iFaceDiscover iFaceDiscover
 	// interfaceFilter should return true if the given interfaceName is allowed
 	interfaceFilter func(interfaceName string) bool
 }
 
-// NewNet creates a new StdNet instance.
-// iFaceDiscover and disallowList can be nil.
-// iFaceDiscover
-func NewNet(iFaceDiscover IFaceDiscover, disallowList []string) (*Net, error) {
-	n := &Net{interfaceFilter: InterfaceFilter(disallowList)}
-	return n, n.UpdateInterfaces(iFaceDiscover)
+// NewNetWithDiscover creates a new StdNet instance.
+func NewNetWithDiscover(iFaceDiscover ExternalIFaceDiscover, disallowList []string) (*Net, error) {
+	n := &Net{
+		iFaceDiscover:   newMobileIFaceDiscover(iFaceDiscover),
+		interfaceFilter: InterfaceFilter(disallowList),
+	}
+	return n, n.UpdateInterfaces()
 }
 
-func (n *Net) filterInterfaces(interfaces []*transport.Interface) []*transport.Interface {
-	if n.interfaceFilter == nil {
-		return interfaces
+// NewNet creates a new StdNet instance.
+func NewNet(disallowList []string) (*Net, error) {
+	n := &Net{
+		iFaceDiscover:   pionDiscover{},
+		interfaceFilter: InterfaceFilter(disallowList),
 	}
-	result := []*transport.Interface{}
-	for _, iface := range interfaces {
-		if n.interfaceFilter(iface.Name) {
-			result = append(result, iface)
-		}
-	}
-	return result
+	return n, n.UpdateInterfaces()
 }
 
 // UpdateInterfaces updates the internal list of network interfaces
 // and associated addresses filtering them by name.
 // The interfaces are discovered by an external iFaceDiscover function or by a default discoverer if the external one
 // wasn't specified.
-func (n *Net) UpdateInterfaces(iFaceDiscover IFaceDiscover) error {
-	discoveredInterfaces := []*transport.Interface{} //nolint
-	var err error
-	if iFaceDiscover != nil {
-		interfacesString := ""
-		interfacesString, err = iFaceDiscover.IFaces()
-		discoveredInterfaces = parseInterfacesString(interfacesString)
-	} else {
-		// fallback to the default discovering if custom IFaceDiscover wasn't provided
-		discoveredInterfaces, err = discoverInterfaces()
-	}
+func (n *Net) UpdateInterfaces() (err error) {
+	allIfaces, err := n.iFaceDiscover.iFaces()
 	if err != nil {
 		return err
 	}
-
-	n.interfaces = n.filterInterfaces(discoveredInterfaces)
+	n.interfaces = n.filterInterfaces(allIfaces)
 	return nil
 }
 
-func discoverInterfaces() ([]*transport.Interface, error) {
-	ifs := []*transport.Interface{}
-
-	oifs, err := net.Interfaces()
-	if err != nil {
-		return nil, err
-	}
-
-	for _, oif := range oifs {
-		ifc := transport.NewInterface(oif)
-
-		addrs, err := oif.Addrs()
-		if err != nil {
-			return nil, err
-		}
-
-		for _, addr := range addrs {
-			ifc.AddAddress(addr)
-		}
-
-		ifs = append(ifs, ifc)
-	}
-
-	return ifs, nil
-}
-
 // Interfaces returns a slice of interfaces which are available on the
 // system
 func (n *Net) Interfaces() ([]*transport.Interface, error) {
@@ -125,100 +83,15 @@ func (n *Net) InterfaceByName(name string) (*transport.Interface, error) {
 	return nil, fmt.Errorf("%w: %s", transport.ErrInterfaceNotFound, name)
 }
 
-func parseInterfacesString(interfaces string) []*transport.Interface {
-	ifs := []*transport.Interface{}
-
-	for _, iface := range strings.Split(interfaces, "\n") {
-		if strings.TrimSpace(iface) == "" {
-			continue
-		}
-
-		fields := strings.Split(iface, "|")
-		if len(fields) != 2 {
-			log.Warnf("parseInterfacesString: unable to split %q", iface)
-			continue
-		}
-
-		var name string
-		var index, mtu int
-		var up, broadcast, loopback, pointToPoint, multicast bool
-		_, err := fmt.Sscanf(fields[0], "%s %d %d %t %t %t %t %t",
-			&name, &index, &mtu, &up, &broadcast, &loopback, &pointToPoint, &multicast)
-		if err != nil {
-			log.Warnf("parseInterfacesString: unable to parse %q: %v", iface, err)
-			continue
-		}
-
-		newIf := net.Interface{
-			Name:  name,
-			Index: index,
-			MTU:   mtu,
-		}
-		if up {
-			newIf.Flags |= net.FlagUp
-		}
-		if broadcast {
-			newIf.Flags |= net.FlagBroadcast
-		}
-		if loopback {
-			newIf.Flags |= net.FlagLoopback
-		}
-		if pointToPoint {
-			newIf.Flags |= net.FlagPointToPoint
-		}
-		if multicast {
-			newIf.Flags |= net.FlagMulticast
-		}
-
-		ifc := transport.NewInterface(newIf)
-
-		addrs := strings.Trim(fields[1], " \n")
-		foundAddress := false
-		for _, addr := range strings.Split(addrs, " ") {
-			ip, ipNet, err := net.ParseCIDR(addr)
-			if err != nil {
-				log.Warnf("%s", err)
-				continue
-			}
-			ipNet.IP = ip
-			ifc.AddAddress(ipNet)
-			foundAddress = true
-		}
-		if foundAddress {
-			ifs = append(ifs, ifc)
+func (n *Net) filterInterfaces(interfaces []*transport.Interface) []*transport.Interface {
+	if n.interfaceFilter == nil {
+		return interfaces
+	}
+	result := []*transport.Interface{}
+	for _, iface := range interfaces {
+		if n.interfaceFilter(iface.Name) {
+			result = append(result, iface)
 		}
 	}
-	return ifs
-}
-
-// InterfaceFilter is a function passed to ICE Agent to filter out not allowed interfaces
-// to avoid building tunnel over them.
-func InterfaceFilter(disallowList []string) func(string) bool {
-
-	return func(iFace string) bool {
-
-		if strings.HasPrefix(iFace, "lo") {
-			// hardcoded loopback check to support already installed agents
-			return false
-		}
-
-		for _, s := range disallowList {
-			if strings.HasPrefix(iFace, s) {
-				log.Debugf("ignoring interface %s - it is not allowed", iFace)
-				return false
-			}
-		}
-		// look for unlisted WireGuard interfaces
-		wg, err := wgctrl.New()
-		if err != nil {
-			log.Debugf("trying to create a wgctrl client failed with: %v", err)
-			return true
-		}
-		defer func() {
-			_ = wg.Close()
-		}()
-
-		_, err = wg.Device(iFace)
-		return err != nil
-	}
+	return result
 }