Proposal for code cleaning

This reverts commit 189321f09d.

client/internal/engine.go

@@ -18,7 +18,6 @@ import (
 
 	"github.com/netbirdio/netbird/client/internal/dns"
 	"github.com/netbirdio/netbird/client/internal/peer"
-	"github.com/netbirdio/netbird/client/internal/proxy"
 	"github.com/netbirdio/netbird/client/internal/routemanager"
 	"github.com/netbirdio/netbird/client/internal/stdnet"
 	nbssh "github.com/netbirdio/netbird/client/ssh"
@@ -179,7 +178,7 @@ func (e *Engine) Start() error {
 	var err error
 	transportNet, err := e.newStdNet()
 	if err != nil {
-		log.Errorf("failed to create pion's stdnet: %s", err)
+		log.Warnf("failed to create pion's stdnet: %s", err)
 	}
 	e.wgInterface, err = iface.NewWGIFace(wgIFaceName, wgAddr, iface.DefaultMTU, e.config.TunAdapter, transportNet)
 	if err != nil {
@@ -809,14 +808,6 @@ func (e Engine) createPeerConn(pubKey string, allowedIPs string) (*peer.Conn, er
 	stunTurn = append(stunTurn, e.STUNs...)
 	stunTurn = append(stunTurn, e.TURNs...)
 
-	proxyConfig := proxy.Config{
-		RemoteKey:    pubKey,
-		WgListenAddr: fmt.Sprintf("127.0.0.1:%d", e.config.WgPort),
-		WgInterface:  e.wgInterface,
-		AllowedIps:   allowedIPs,
-		PreSharedKey: e.config.PreSharedKey,
-	}
-
 	// randomize connection timeout
 	timeout := time.Duration(rand.Intn(PeerConnectionTimeoutMax-PeerConnectionTimeoutMin)+PeerConnectionTimeoutMin) * time.Millisecond
 	config := peer.ConnConfig{
@@ -828,13 +819,12 @@ func (e Engine) createPeerConn(pubKey string, allowedIPs string) (*peer.Conn, er
 		Timeout:              timeout,
 		UDPMux:               e.udpMux,
 		UDPMuxSrflx:          e.udpMuxSrflx,
-		ProxyConfig:          proxyConfig,
 		LocalWgPort:          e.config.WgPort,
 		NATExternalIPs:       e.parseNATExternalIPMappings(),
-		UserspaceBind:        e.wgInterface.IsUserspaceBind(),
+		AllowedIPs:           allowedIPs,
 	}
 
-	peerConn, err := peer.NewConn(config, e.statusRecorder, e.config.TunAdapter, e.config.IFaceDiscover)
+	peerConn, err := peer.NewConn(config, e.wgInterface, e.statusRecorder, e.config.TunAdapter, e.config.IFaceDiscover)
 	if err != nil {
 		return nil, err
 	}

client/internal/peer/conn.go

@@ -37,8 +37,6 @@ type ConnConfig struct {
 
 	Timeout time.Duration
 
-	ProxyConfig proxy.Config
-
 	UDPMux      ice.UDPMux
 	UDPMuxSrflx ice.UniversalUDPMux
 
@@ -46,8 +44,7 @@ type ConnConfig struct {
 
 	NATExternalIPs []string
 
-	// UsesBind indicates whether the WireGuard interface is userspace and uses bind.ICEBind
-	UserspaceBind bool
+	AllowedIPs string
 }
 
 // OfferAnswer represents a session establishment offer or answer
@@ -96,6 +93,7 @@ type Conn struct {
 	remoteModeCh chan ModeMessage
 	meta         meta
 
+	wgIface       *iface.WGIface
 	adapter       iface.TunAdapter
 	iFaceDiscover stdnet.ExternalIFaceDiscover
 }
@@ -123,7 +121,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.ExternalIFaceDiscover) (*Conn, error) {
+func NewConn(config ConnConfig, wgIface *iface.WGIface, statusRecorder *Status, adapter iface.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover) (*Conn, error) {
 	return &Conn{
 		config:         config,
 		mu:             sync.Mutex{},
@@ -135,6 +133,7 @@ func NewConn(config ConnConfig, statusRecorder *Status, adapter iface.TunAdapter
 		remoteModeCh:   make(chan ModeMessage, 1),
 		adapter:        adapter,
 		iFaceDiscover:  iFaceDiscover,
+		wgIface:        wgIface,
 	}, nil
 }
 
@@ -147,7 +146,7 @@ func (conn *Conn) reCreateAgent() error {
 	var err error
 	transportNet, err := conn.newStdNet()
 	if err != nil {
-		log.Errorf("failed to create pion's stdnet: %s", err)
+		log.Warnf("failed to create pion's stdnet: %s", err)
 	}
 	agentConfig := &ice.AgentConfig{
 		MulticastDNSMode: ice.MulticastDNSModeDisabled,
@@ -198,7 +197,7 @@ func (conn *Conn) Open() error {
 
 	peerState := State{PubKey: conn.config.Key}
 
-	peerState.IP = strings.Split(conn.config.ProxyConfig.AllowedIps, "/")[0]
+	peerState.IP = strings.Split(conn.config.AllowedIPs, "/")[0]
 	peerState.ConnStatusUpdate = time.Now()
 	peerState.ConnStatus = conn.status
 
@@ -329,28 +328,18 @@ func (conn *Conn) Open() error {
 func shouldUseProxy(pair *ice.CandidatePair, userspaceBind bool) bool {
 
 	if !isRelayCandidate(pair.Local) && userspaceBind {
-		log.Debugf("shouldn't use proxy because using Bind and the connection is not relayed")
 		return false
 	}
 
 	if !isHardNATCandidate(pair.Local) && isHostCandidateWithPublicIP(pair.Remote) {
-		log.Debugf("shouldn't use proxy because the local peer is not behind a hard NAT and the remote one has a public IP")
 		return false
 	}
 
 	if !isHardNATCandidate(pair.Remote) && isHostCandidateWithPublicIP(pair.Local) {
-		log.Debugf("shouldn't use proxy because the remote peer is not behind a hard NAT and the local one has a public IP")
 		return false
 	}
 
 	if isHostCandidateWithPrivateIP(pair.Local) && isHostCandidateWithPrivateIP(pair.Remote) && isSameNetworkPrefix(pair) {
-		log.Debugf("shouldn't use proxy because peers are in the same private /16 network")
-		return false
-	}
-
-	if (isPeerReflexiveCandidateWithPrivateIP(pair.Local) && isHostCandidateWithPrivateIP(pair.Remote) ||
-		isHostCandidateWithPrivateIP(pair.Local) && isPeerReflexiveCandidateWithPrivateIP(pair.Remote)) && isSameNetworkPrefix(pair) {
-		log.Debugf("shouldn't use proxy because peers are in the same private /16 network and one peer is peer reflexive")
 		return false
 	}
 
@@ -359,8 +348,16 @@ func shouldUseProxy(pair *ice.CandidatePair, userspaceBind bool) bool {
 
 func isSameNetworkPrefix(pair *ice.CandidatePair) bool {
 
-	localIP := net.ParseIP(pair.Local.Address())
-	remoteIP := net.ParseIP(pair.Remote.Address())
+	localIPStr, _, err := net.SplitHostPort(pair.Local.Address())
+	if err != nil {
+		return false
+	}
+	remoteIPStr, _, err := net.SplitHostPort(pair.Remote.Address())
+	if err != nil {
+		return false
+	}
+	localIP := net.ParseIP(localIPStr)
+	remoteIP := net.ParseIP(remoteIPStr)
 	if localIP == nil || remoteIP == nil {
 		return false
 	}
@@ -385,13 +382,9 @@ func isHostCandidateWithPrivateIP(candidate ice.Candidate) bool {
 	return candidate.Type() == ice.CandidateTypeHost && !isPublicIP(candidate.Address())
 }
 
-func isPeerReflexiveCandidateWithPrivateIP(candidate ice.Candidate) bool {
-	return candidate.Type() == ice.CandidateTypePeerReflexive && !isPublicIP(candidate.Address())
-}
-
 func isPublicIP(address string) bool {
 	ip := net.ParseIP(address)
-	if ip == nil || ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() {
+	if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() {
 		return false
 	}
 	return true
@@ -425,7 +418,7 @@ func (conn *Conn) startProxy(remoteConn net.Conn, remoteWgPort int) error {
 	if pair.Local.Type() == ice.CandidateTypeRelay || pair.Remote.Type() == ice.CandidateTypeRelay {
 		peerState.Relayed = true
 	}
-	peerState.Direct = p.Type() == proxy.TypeDirectNoProxy || p.Type() == proxy.TypeNoProxy
+	peerState.Direct = p.Type() == proxy.TypeDirectNoProxy
 
 	err = conn.statusRecorder.UpdatePeerState(peerState)
 	if err != nil {
@@ -436,7 +429,7 @@ func (conn *Conn) startProxy(remoteConn net.Conn, remoteWgPort int) error {
 }
 
 func (conn *Conn) getProxyWithMessageExchange(pair *ice.CandidatePair, remoteWgPort int) proxy.Proxy {
-	useProxy := shouldUseProxy(pair, conn.config.UserspaceBind)
+	useProxy := shouldUseProxy(pair, conn.wgIface.IsUserspaceBind())
 	localDirectMode := !useProxy
 	remoteDirectMode := localDirectMode
 
@@ -446,12 +439,13 @@ func (conn *Conn) getProxyWithMessageExchange(pair *ice.CandidatePair, remoteWgP
 		remoteDirectMode = conn.receiveRemoteDirectMode()
 	}
 
-	if conn.config.UserspaceBind && localDirectMode {
+	if conn.wgIface.IsUserspaceBind() && localDirectMode {
 		return proxy.NewNoProxy(conn.config.ProxyConfig)
 	}
 
 	if localDirectMode && remoteDirectMode {
-		return proxy.NewDirectNoProxy(conn.config.ProxyConfig, remoteWgPort)
+		//wgInterface *iface.WGIface, remoteKey string, allowedIps string, preSharedKey *wgtypes.Key, remoteWgPort int)
+		return proxy.NewDirectNoProxy(conn.wgIface, conn.config.Key, conn.config.AllowedIPs, remoteWgPort)
 	}
 
 	log.Debugf("falling back to local proxy mode with peer %s", conn.config.Key)

client/internal/peer/conn_test.go

@@ -1,12 +1,11 @@
 package peer
 
 import (
+	"github.com/netbirdio/netbird/client/internal/stdnet"
 	"sync"
 	"testing"
 	"time"
 
-	"github.com/netbirdio/netbird/client/internal/stdnet"
-
 	"github.com/magiconair/properties/assert"
 	"github.com/pion/ice/v2"
 	"golang.org/x/sync/errgroup"
@@ -204,7 +203,7 @@ func TestConn_ShouldUseProxy(t *testing.T) {
 	}
 	privateHostCandidate := &mockICECandidate{
 		AddressFunc: func() string {
-			return "10.0.0.1"
+			return "10.0.0.1:44576"
 		},
 		TypeFunc: func() ice.CandidateType {
 			return ice.CandidateTypeHost
@@ -323,42 +322,6 @@ func TestConn_ShouldUseProxy(t *testing.T) {
 			},
 			expected: false,
 		},
-		{
-			name: "Don't Use Proxy When Both Candidates are in private network and one is peer reflexive",
-			candatePair: &ice.CandidatePair{
-				Local: &mockICECandidate{AddressFunc: func() string {
-					return "10.16.102.168"
-				},
-					TypeFunc: func() ice.CandidateType {
-						return ice.CandidateTypeHost
-					}},
-				Remote: &mockICECandidate{AddressFunc: func() string {
-					return "10.16.101.96"
-				},
-					TypeFunc: func() ice.CandidateType {
-						return ice.CandidateTypePeerReflexive
-					}},
-			},
-			expected: false,
-		},
-		{
-			name: "Should Use Proxy When Both Candidates are in private network and both are peer reflexive",
-			candatePair: &ice.CandidatePair{
-				Local: &mockICECandidate{AddressFunc: func() string {
-					return "10.16.102.168"
-				},
-					TypeFunc: func() ice.CandidateType {
-						return ice.CandidateTypePeerReflexive
-					}},
-				Remote: &mockICECandidate{AddressFunc: func() string {
-					return "10.16.101.96"
-				},
-					TypeFunc: func() ice.CandidateType {
-						return ice.CandidateTypePeerReflexive
-					}},
-			},
-			expected: true,
-		},
 	}
 
 	for _, testCase := range testCases {

client/internal/proxy/direct.go

@@ -1,8 +1,11 @@
 package proxy
 
 import (
-	log "github.com/sirupsen/logrus"
 	"net"
+
+	log "github.com/sirupsen/logrus"
+
+	"github.com/netbirdio/netbird/iface"
 )
 
 // DirectNoProxy is used when there is no need for a proxy between ICE and WireGuard.
@@ -12,20 +15,28 @@ import (
 // DirectNoProxy will just update remote peer with a remote host and fixed WireGuard port (r.g. 51820).
 // In order DirectNoProxy to work, WireGuard port has to be fixed for the time being.
 type DirectNoProxy struct {
-	config Config
+	wgInterface *iface.WGIface
+
+	remoteKey  string
+	allowedIps string
+
 	// RemoteWgListenPort is a WireGuard port of a remote peer.
 	// It is used instead of the hardcoded 51820 port.
-	RemoteWgListenPort int
+	remoteWgListenPort int
 }
 
 // NewDirectNoProxy creates a new DirectNoProxy with a provided config and remote peer's WireGuard listen port
-func NewDirectNoProxy(config Config, remoteWgPort int) *DirectNoProxy {
-	return &DirectNoProxy{config: config, RemoteWgListenPort: remoteWgPort}
+func NewDirectNoProxy(wgInterface *iface.WGIface, remoteKey string, allowedIps string, remoteWgPort int) *DirectNoProxy {
+	return &DirectNoProxy{
+		wgInterface:        wgInterface,
+		remoteKey:          remoteKey,
+		allowedIps:         allowedIps,
+		remoteWgListenPort: remoteWgPort}
 }
 
 // Close removes peer from the WireGuard interface
 func (p *DirectNoProxy) Close() error {
-	err := p.config.WgInterface.RemovePeer(p.config.RemoteKey)
+	err := p.wgInterface.RemovePeer(p.remoteKey)
 	if err != nil {
 		return err
 	}
@@ -35,14 +46,13 @@ func (p *DirectNoProxy) Close() error {
 // Start just updates WireGuard peer with the remote IP and default WireGuard port
 func (p *DirectNoProxy) Start(remoteConn net.Conn) error {
 
-	log.Debugf("using DirectNoProxy while connecting to peer %s", p.config.RemoteKey)
+	log.Debugf("using DirectNoProxy while connecting to peer %s", p.remoteKey)
 	addr, err := net.ResolveUDPAddr("udp", remoteConn.RemoteAddr().String())
 	if err != nil {
 		return err
 	}
-	addr.Port = p.RemoteWgListenPort
-	err = p.config.WgInterface.UpdatePeer(p.config.RemoteKey, p.config.AllowedIps, DefaultWgKeepAlive,
-		addr, p.config.PreSharedKey)
+	addr.Port = p.remoteWgListenPort
+	err = p.wgInterface.UpdatePeer(p.remoteKey, p.allowedIps, addr)
 
 	if err != nil {
 		return err

client/internal/proxy/proxy.go

@@ -1,15 +1,10 @@
 package proxy
 
 import (
-	"github.com/netbirdio/netbird/iface"
-	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 	"io"
 	"net"
-	"time"
 )
 
-const DefaultWgKeepAlive = 25 * time.Second
-
 type Type string
 
 const (
@@ -19,14 +14,6 @@ const (
 	TypeNoProxy       Type = "NoProxy"
 )
 
-type Config struct {
-	WgListenAddr string
-	RemoteKey    string
-	WgInterface  *iface.WGIface
-	AllowedIps   string
-	PreSharedKey *wgtypes.Key
-}
-
 type Proxy interface {
 	io.Closer
 	// Start creates a local remoteConn and starts proxying data from/to remoteConn

client/internal/stdnet/discover_mobile.go

@@ -78,9 +78,6 @@ func (m *mobileIFaceDiscover) parseInterfacesString(interfaces string) []*transp
 		addrs := strings.Trim(fields[1], " \n")
 		foundAddress := false
 		for _, addr := range strings.Split(addrs, " ") {
-			if strings.Contains(addr, "%") {
-				continue
-			}
 			ip, ipNet, err := net.ParseCIDR(addr)
 			if err != nil {
 				log.Warnf("%s", err)

client/internal/stdnet/discover_mobile_test.go

@@ -3,8 +3,6 @@ package stdnet
 import (
 	"fmt"
 	"testing"
-
-	log "github.com/sirupsen/logrus"
 )
 
 func Test_parseInterfacesString(t *testing.T) {
@@ -22,7 +20,6 @@ func Test_parseInterfacesString(t *testing.T) {
 		{"wlan0", 30, 1500, true, true, false, false, true, "10.1.10.131/24"},
 		{"rmnet0", 30, 1500, true, true, false, false, true, "192.168.0.56/24"},
 		{"rmnet_data1", 30, 1500, true, true, false, false, true, "fec0::118c:faf7:8d97:3cb2/64"},
-		{"rmnet_data2", 30, 1500, true, true, false, false, true, "fec0::118c:faf7:8d97:3cb2%rmnet2/64"},
 	}
 
 	var exampleString string
@@ -38,13 +35,13 @@ func Test_parseInterfacesString(t *testing.T) {
 			d.multicast,
 			d.addr)
 	}
+
 	d := mobileIFaceDiscover{}
 	nets := d.parseInterfacesString(exampleString)
 	if len(nets) == 0 {
 		t.Fatalf("failed to parse interfaces")
 	}
 
-	log.Printf("%d", len(nets))
 	for i, net := range nets {
 		if net.MTU != testData[i].mtu {
 			t.Errorf("invalid mtu: %d, expected: %d", net.MTU, testData[0].mtu)
@@ -63,7 +60,7 @@ func Test_parseInterfacesString(t *testing.T) {
 		if len(addr) == 0 {
 			t.Errorf("invalid address parsing")
 		}
-		log.Printf("%v", addr)
+
 		if addr[0].String() != testData[i].addr {
 			t.Errorf("invalid address: %s, expected: %s", addr[0].String(), testData[i].addr)
 		}

iface/bind/bind.go

@@ -3,29 +3,29 @@ package bind
 import (
 	"errors"
 	"fmt"
-	"net"
-	"net/netip"
-	"sync"
-	"syscall"
-
 	"github.com/pion/stun"
 	"github.com/pion/transport/v2"
 	log "github.com/sirupsen/logrus"
 	"golang.zx2c4.com/wireguard/conn"
+	"net"
+	"net/netip"
+	"sync"
+	"syscall"
 )
 
 // ICEBind is the userspace implementation of WireGuard's conn.Bind interface using ice.UDPMux of the pion/ice library
 type ICEBind struct {
 	// below fields, initialized on open
-	ipv4   net.PacketConn
-	udpMux *UniversalUDPMuxDefault
+	sharedConn net.PacketConn
+	udpMux     *UniversalUDPMuxDefault
 
 	// below are fields initialized on creation
 	transportNet transport.Net
 	mu           sync.Mutex
 }
 
-// NewICEBind create a new instance of ICEBind with a given transportNet function.
+// NewICEBind create a new instance of ICEBind with a given transportNet and an interfaceFilter function.
+// The interfaceFilter function is used to exclude interfaces from hole punching (the IPs of that interfaces won't be used as connection candidates)
 // The transportNet can be nil.
 func NewICEBind(transportNet transport.Net) *ICEBind {
 	return &ICEBind{
@@ -50,51 +50,51 @@ func (b *ICEBind) Open(uport uint16) ([]conn.ReceiveFunc, uint16, error) {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
-	if b.ipv4 != nil {
+	if b.sharedConn != nil {
 		return nil, 0, conn.ErrBindAlreadyOpen
 	}
 
-	var err error
-	b.ipv4, _, err = listenNet("udp4", int(uport))
+	ipv4Conn, _, err := listenNet("udp4", int(uport))
 	if err != nil && !errors.Is(err, syscall.EAFNOSUPPORT) {
 		return nil, 0, err
 	}
+	b.sharedConn = ipv4Conn
+	b.udpMux = NewUniversalUDPMuxDefault(UniversalUDPMuxParams{UDPConn: b.sharedConn, Net: b.transportNet})
 
-	b.udpMux = NewUniversalUDPMuxDefault(UniversalUDPMuxParams{UDPConn: b.ipv4, Net: b.transportNet})
-
-	portAddr, err := netip.ParseAddrPort(b.ipv4.LocalAddr().String())
+	portAddr1, err := netip.ParseAddrPort(ipv4Conn.LocalAddr().String())
 	if err != nil {
 		return nil, 0, err
 	}
 
-	log.Infof("opened ICEBind on %s", b.ipv4.LocalAddr().String())
+	log.Infof("opened ICEBind on %s", ipv4Conn.LocalAddr().String())
 
 	return []conn.ReceiveFunc{
-			b.makeReceiveIPv4(b.ipv4),
+			b.makeReceiveIPv4(b.sharedConn),
 		},
-		portAddr.Port(), nil
+		portAddr1.Port(), nil
 }
 
-func listenNet(network string, port int) (net.PacketConn, int, error) {
-	c, err := net.ListenUDP(network, &net.UDPAddr{Port: port})
+func listenNet(network string, port int) (*net.UDPConn, int, error) {
+	conn, err := net.ListenUDP(network, &net.UDPAddr{Port: port})
 	if err != nil {
 		return nil, 0, err
 	}
 
-	lAddr := c.LocalAddr()
-	uAddr, err := net.ResolveUDPAddr(
-		lAddr.Network(),
-		lAddr.String(),
+	// Retrieve port.
+	laddr := conn.LocalAddr()
+	uaddr, err := net.ResolveUDPAddr(
+		laddr.Network(),
+		laddr.String(),
 	)
 	if err != nil {
 		return nil, 0, err
 	}
-	return c, uAddr.Port, nil
+	return conn, uaddr.Port, nil
 }
 
 func parseSTUNMessage(raw []byte) (*stun.Message, error) {
 	msg := &stun.Message{
-		Raw: raw,
+		Raw: append([]byte{}, raw...),
 	}
 	if err := msg.Decode(); err != nil {
 		return nil, err
@@ -113,7 +113,7 @@ func (b *ICEBind) makeReceiveIPv4(c net.PacketConn) conn.ReceiveFunc {
 		if err != nil {
 			return 0, nil, err
 		}
-		if !stun.IsMessage(buff) {
+		if !stun.IsMessage(buff[:20]) {
 			// WireGuard traffic
 			return n, (conn.StdNetEndpoint)(netip.AddrPortFrom(e.Addr(), e.Port())), nil
 		}
@@ -139,9 +139,9 @@ func (b *ICEBind) Close() error {
 	defer b.mu.Unlock()
 
 	var err1, err2 error
-	if b.ipv4 != nil {
-		c := b.ipv4
-		b.ipv4 = nil
+	if b.sharedConn != nil {
+		c := b.sharedConn
+		b.sharedConn = nil
 		err1 = c.Close()
 	}
 
@@ -172,7 +172,7 @@ func (b *ICEBind) Send(buff []byte, endpoint conn.Endpoint) error {
 		return conn.ErrWrongEndpointType
 	}
 	addrPort := netip.AddrPort(nend)
-	_, err := b.ipv4.WriteTo(buff, &net.UDPAddr{
+	_, err := b.sharedConn.WriteTo(buff, &net.UDPAddr{
 		IP:   addrPort.Addr().AsSlice(),
 		Port: int(addrPort.Port()),
 		Zone: addrPort.Addr().Zone(),

iface/bind/udp_mux.go

@@ -2,15 +2,14 @@ package bind
 
 import (
 	"fmt"
-	"io"
-	"net"
-	"strings"
-	"sync"
-
 	"github.com/pion/ice/v2"
 	"github.com/pion/stun"
 	"github.com/pion/transport/v2/stdnet"
 	log "github.com/sirupsen/logrus"
+	"io"
+	"net"
+	"strings"
+	"sync"
 
 	"github.com/pion/logging"
 	"github.com/pion/transport/v2"

iface/bind/udp_mux_universal.go

@@ -6,11 +6,10 @@ package bind
 
 import (
 	"fmt"
+	log "github.com/sirupsen/logrus"
 	"net"
 	"time"
 
-	log "github.com/sirupsen/logrus"
-
 	"github.com/pion/logging"
 	"github.com/pion/stun"
 	"github.com/pion/transport/v2"

iface/iface.go

@@ -8,12 +8,13 @@ import (
 	"github.com/netbirdio/netbird/iface/bind"
 
 	log "github.com/sirupsen/logrus"
-	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 )
 
 const (
 	DefaultMTU    = 1280
 	DefaultWgPort = 51820
+
+	defaultWgKeepAlive = 25 * time.Second
 )
 
 // WGIface represents a interface instance
@@ -77,12 +78,12 @@ func (w *WGIface) UpdateAddr(newAddr string) error {
 
 // UpdatePeer updates existing Wireguard Peer or creates a new one if doesn't exist
 // Endpoint is optional
-func (w *WGIface) UpdatePeer(peerKey string, allowedIps string, keepAlive time.Duration, endpoint *net.UDPAddr, preSharedKey *wgtypes.Key) error {
+func (w *WGIface) UpdatePeer(peerKey string, allowedIps string, endpoint *net.UDPAddr) error {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 
 	log.Debugf("updating interface %s peer %s: endpoint %s ", w.tun.DeviceName(), peerKey, endpoint)
-	return w.configurer.updatePeer(peerKey, allowedIps, keepAlive, endpoint, preSharedKey)
+	return w.configurer.updatePeer(peerKey, allowedIps, endpoint)
 }
 
 // RemovePeer removes a Wireguard Peer from the interface iface

iface/iface_android.go

@@ -7,7 +7,7 @@ import (
 )
 
 // NewWGIFace Creates a new WireGuard interface instance
-func NewWGIFace(iFaceName string, address string, mtu int, tunAdapter TunAdapter, transportNet transport.Net) (*WGIface, error) {
+func NewWGIFace(iFaceName string, address string, mtu int, preSharedKey *wgtypes.Key, tunAdapter TunAdapter, transportNet transport.Net) (*WGIface, error) {
 	wgIFace := &WGIface{
 		mu: sync.Mutex{},
 	}
@@ -20,7 +20,7 @@ func NewWGIFace(iFaceName string, address string, mtu int, tunAdapter TunAdapter
 	tun := newTunDevice(wgAddress, mtu, tunAdapter, transportNet)
 	wgIFace.tun = tun
 
-	wgIFace.configurer = newWGConfigurer(tun)
+	wgIFace.configurer = newWGConfigurer(tun, preSharedKey)
 
 	wgIFace.userspaceBind = !WireGuardModuleIsLoaded()
 

iface/iface_nonandroid.go

@@ -3,13 +3,14 @@
 package iface
 
 import (
-	"sync"
-
 	"github.com/pion/transport/v2"
+	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
+
+	"sync"
 )
 
 // NewWGIFace Creates a new WireGuard interface instance
-func NewWGIFace(iFaceName string, address string, mtu int, tunAdapter TunAdapter, transportNet transport.Net) (*WGIface, error) {
+func NewWGIFace(iFaceName string, address string, mtu int, preSharedKey *wgtypes.Key, tunAdapter TunAdapter, transportNet transport.Net) (*WGIface, error) {
 	wgIFace := &WGIface{
 		mu: sync.Mutex{},
 	}
@@ -21,7 +22,7 @@ func NewWGIFace(iFaceName string, address string, mtu int, tunAdapter TunAdapter
 
 	wgIFace.tun = newTunDevice(iFaceName, wgAddress, mtu, transportNet)
 
-	wgIFace.configurer = newWGConfigurer(iFaceName)
+	wgIFace.configurer = newWGConfigurer(iFaceName, preSharedKey)
 	wgIFace.userspaceBind = !WireGuardModuleIsLoaded()
 	return wgIFace, nil
 }

iface/iface_test.go

@@ -2,15 +2,14 @@ package iface
 
 import (
 	"fmt"
-	"net"
-	"testing"
-	"time"
-
 	"github.com/pion/transport/v2/stdnet"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"golang.zx2c4.com/wireguard/wgctrl"
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
+	"net"
+	"testing"
+	"time"
 )
 
 // keep darwin compability

iface/module_linux.go

@@ -7,6 +7,9 @@ import (
 	"bufio"
 	"errors"
 	"fmt"
+	log "github.com/sirupsen/logrus"
+	"github.com/vishvananda/netlink"
+	"golang.org/x/sys/unix"
 	"io"
 	"io/fs"
 	"math"
@@ -14,10 +17,6 @@ import (
 	"path/filepath"
 	"strings"
 	"syscall"
-
-	log "github.com/sirupsen/logrus"
-	"github.com/vishvananda/netlink"
-	"golang.org/x/sys/unix"
 )
 
 // Holds logic to check existence of kernel modules used by wireguard interfaces

iface/module_linux_test.go

@@ -3,14 +3,13 @@ package iface
 import (
 	"bufio"
 	"bytes"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 	"io"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
-
-	"github.com/stretchr/testify/require"
-	"golang.org/x/sys/unix"
 )
 
 func TestGetModuleDependencies(t *testing.T) {

iface/tun_android.go

@@ -1,11 +1,9 @@
 package iface
 
 import (
-	"net"
-
-	"github.com/pion/transport/v2"
-
 	"github.com/netbirdio/netbird/iface/bind"
+	"github.com/pion/transport/v2"
+	"net"
 
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"

iface/tun_unix.go

@@ -3,16 +3,14 @@
 package iface
 
 import (
+	"github.com/netbirdio/netbird/iface/bind"
+	"github.com/pion/transport/v2"
 	"net"
 	"os"
 
-	"github.com/pion/transport/v2"
-	"golang.zx2c4.com/wireguard/ipc"
-
-	"github.com/netbirdio/netbird/iface/bind"
-
 	log "github.com/sirupsen/logrus"
 	"golang.zx2c4.com/wireguard/device"
+	"golang.zx2c4.com/wireguard/ipc"
 	"golang.zx2c4.com/wireguard/tun"
 )
 
@@ -22,8 +20,6 @@ type tunDevice struct {
 	mtu          int
 	netInterface NetInterface
 	iceBind      *bind.ICEBind
-	uapi         net.Listener
-	close        chan struct{}
 }
 
 func newTunDevice(name string, address WGAddress, mtu int, transportNet transport.Net) *tunDevice {
@@ -32,7 +28,6 @@ func newTunDevice(name string, address WGAddress, mtu int, transportNet transpor
 		address: address,
 		mtu:     mtu,
 		iceBind: bind.NewICEBind(transportNet),
-		close:   make(chan struct{}),
 	}
 }
 
@@ -50,38 +45,23 @@ func (c *tunDevice) DeviceName() string {
 }
 
 func (c *tunDevice) Close() error {
-
-	select {
-	case c.close <- struct{}{}:
-	default:
+	if c.netInterface == nil {
+		return nil
 	}
-
-	var err1, err2, err3 error
-	if c.netInterface != nil {
-		err1 = c.netInterface.Close()
-	}
-
-	if c.uapi != nil {
-		err2 = c.uapi.Close()
+	err := c.netInterface.Close()
+	if err != nil {
+		return err
 	}
 
 	sockPath := "/var/run/wireguard/" + c.name + ".sock"
 	if _, statErr := os.Stat(sockPath); statErr == nil {
 		statErr = os.Remove(sockPath)
 		if statErr != nil {
-			err3 = statErr
+			return statErr
 		}
 	}
 
-	if err1 != nil {
-		return err1
-	}
-
-	if err2 != nil {
-		return err2
-	}
-
-	return err3
+	return nil
 }
 
 // createWithUserspace Creates a new Wireguard interface, using wireguard-go userspace implementation
@@ -92,36 +72,26 @@ func (c *tunDevice) createWithUserspace() (NetInterface, error) {
 	}
 
 	// We need to create a wireguard-go device and listen to configuration requests
-	tunDev := device.NewDevice(tunIface, c.iceBind, device.NewLogger(device.LogLevelSilent, "[netbird] "))
-	err = tunDev.Up()
+	tunDevice := device.NewDevice(tunIface, c.iceBind, device.NewLogger(device.LogLevelSilent, "[wiretrustee] "))
+	err = tunDevice.Up()
 	if err != nil {
-		_ = tunIface.Close()
-		return nil, err
+		return tunIface, err
 	}
 
-	c.uapi, err = c.getUAPI(c.name)
+	// todo: after this line in case of error close the tunSock
+	uapi, err := c.getUAPI(c.name)
 	if err != nil {
-		_ = tunIface.Close()
-		return nil, err
+		return tunIface, err
 	}
 
 	go func() {
 		for {
-			select {
-			case <-c.close:
-				log.Debugf("exit uapi.Accept()")
-				return
-			default:
-			}
-			uapiConn, uapiErr := c.uapi.Accept()
+			uapiConn, uapiErr := uapi.Accept()
 			if uapiErr != nil {
 				log.Traceln("uapi Accept failed with error: ", uapiErr)
 				continue
 			}
-			go func() {
-				tunDev.IpcHandle(uapiConn)
-				log.Debugf("exit tunDevice.IpcHandle")
-			}()
+			go tunDevice.IpcHandle(uapiConn)
 		}
 	}()
 

iface/tun_windows.go

@@ -2,36 +2,27 @@ package iface
 
 import (
 	"fmt"
-	"net"
-
+	"github.com/netbirdio/netbird/iface/bind"
 	"github.com/pion/transport/v2"
 	log "github.com/sirupsen/logrus"
 	"golang.zx2c4.com/wireguard/device"
 	"golang.zx2c4.com/wireguard/ipc"
 	"golang.zx2c4.com/wireguard/tun"
 	"golang.zx2c4.com/wireguard/windows/tunnel/winipcfg"
-
-	"github.com/netbirdio/netbird/iface/bind"
+	"net"
 )
 
 type tunDevice struct {
 	name         string
 	address      WGAddress
 	netInterface NetInterface
+	uapi         net.Listener
 	iceBind      *bind.ICEBind
 	mtu          int
-	uapi         net.Listener
-	close        chan struct{}
 }
 
 func newTunDevice(name string, address WGAddress, mtu int, transportNet transport.Net) *tunDevice {
-	return &tunDevice{
-		name:    name,
-		address: address,
-		mtu:     mtu,
-		iceBind: bind.NewICEBind(transportNet),
-		close:   make(chan struct{}),
-	}
+	return &tunDevice{name: name, address: address, iceBind: bind.NewICEBind(transportNet), mtu: mtu}
 }
 
 func (c *tunDevice) Create() error {
@@ -44,44 +35,34 @@ func (c *tunDevice) Create() error {
 	return c.assignAddr()
 }
 
-// createWithUserspace Creates a new Wireguard interface, using wireguard-go userspace implementation
+// createWithUserspace Creates a new WireGuard interface, using wireguard-go userspace implementation
 func (c *tunDevice) createWithUserspace() (NetInterface, error) {
+
 	tunIface, err := tun.CreateTUN(c.name, c.mtu)
 	if err != nil {
 		return nil, err
 	}
-
 	// We need to create a wireguard-go device and listen to configuration requests
-	tunDev := device.NewDevice(tunIface, c.iceBind, device.NewLogger(device.LogLevelSilent, "[netbird] "))
-	err = tunDev.Up()
+	tunDevice := device.NewDevice(tunIface, c.iceBind, device.NewLogger(device.LogLevelSilent, "[netbird] "))
+	err = tunDevice.Up()
 	if err != nil {
-		_ = tunIface.Close()
-		return nil, err
+		return tunIface, err
 	}
 
-	c.uapi, err = c.getUAPI(c.name)
+	uapi, err := c.getUAPI(c.name)
 	if err != nil {
-		_ = tunIface.Close()
 		return nil, err
 	}
+	c.uapi = uapi
 
 	go func() {
 		for {
-			select {
-			case <-c.close:
-				log.Debugf("exit uapi.Accept()")
-				return
-			default:
-			}
-			uapiConn, uapiErr := c.uapi.Accept()
+			uapiConn, uapiErr := uapi.Accept()
 			if uapiErr != nil {
-				log.Traceln("uapi Accept failed with error: ", uapiErr)
+				log.Traceln("uapi accept failed with error: ", uapiErr)
 				continue
 			}
-			go func() {
-				tunDev.IpcHandle(uapiConn)
-				log.Debugf("exit tunDevice.IpcHandle")
-			}()
+			go tunDevice.IpcHandle(uapiConn)
 		}
 	}()
 
@@ -103,11 +84,6 @@ func (c *tunDevice) DeviceName() string {
 }
 
 func (c *tunDevice) Close() error {
-	select {
-	case c.close <- struct{}{}:
-	default:
-	}
-
 	var err1, err2 error
 	if c.netInterface != nil {
 		err1 = c.netInterface.Close()

iface/wg_configurer_android.go

@@ -3,7 +3,6 @@ package iface
 import (
 	"errors"
 	"net"
-	"time"
 
 	log "github.com/sirupsen/logrus"
 
@@ -15,12 +14,14 @@ var (
 )
 
 type wGConfigurer struct {
-	tunDevice *tunDevice
+	tunDevice    *tunDevice
+	preSharedKey *wgtypes.Key
 }
 
-func newWGConfigurer(tunDevice *tunDevice) wGConfigurer {
+func newWGConfigurer(tunDevice *tunDevice, preSharedKey *wgtypes.Key) wGConfigurer {
 	return wGConfigurer{
-		tunDevice: tunDevice,
+		tunDevice:    tunDevice,
+		preSharedKey: preSharedKey,
 	}
 }
 
@@ -41,7 +42,7 @@ func (c *wGConfigurer) configureInterface(privateKey string, port int) error {
 	return c.tunDevice.Device().IpcSet(toWgUserspaceString(config))
 }
 
-func (c *wGConfigurer) updatePeer(peerKey string, allowedIps string, keepAlive time.Duration, endpoint *net.UDPAddr, preSharedKey *wgtypes.Key) error {
+func (c *wGConfigurer) updatePeer(peerKey string, allowedIps string, endpoint *net.UDPAddr) error {
 	//parse allowed ips
 	_, ipNet, err := net.ParseCIDR(allowedIps)
 	if err != nil {
@@ -52,12 +53,13 @@ func (c *wGConfigurer) updatePeer(peerKey string, allowedIps string, keepAlive t
 	if err != nil {
 		return err
 	}
+	keepalive := defaultWgKeepAlive
 	peer := wgtypes.PeerConfig{
 		PublicKey:                   peerKeyParsed,
 		ReplaceAllowedIPs:           true,
 		AllowedIPs:                  []net.IPNet{*ipNet},
 		PersistentKeepaliveInterval: &keepAlive,
-		PresharedKey:                preSharedKey,
+		PresharedKey:                c.preSharedKey,
 		Endpoint:                    endpoint,
 	}
 

iface/wg_configurer_nonandroid.go

@@ -5,7 +5,6 @@ package iface
 import (
 	"fmt"
 	"net"
-	"time"
 
 	log "github.com/sirupsen/logrus"
 	"golang.zx2c4.com/wireguard/wgctrl"
@@ -13,12 +12,14 @@ import (
 )
 
 type wGConfigurer struct {
-	deviceName string
+	deviceName   string
+	preSharedKey *wgtypes.Key
 }
 
-func newWGConfigurer(deviceName string) wGConfigurer {
+func newWGConfigurer(deviceName string, preSharedKey *wgtypes.Key) wGConfigurer {
 	return wGConfigurer{
-		deviceName: deviceName,
+		deviceName:   deviceName,
+		preSharedKey: preSharedKey,
 	}
 }
 
@@ -43,7 +44,7 @@ func (c *wGConfigurer) configureInterface(privateKey string, port int) error {
 	return nil
 }
 
-func (c *wGConfigurer) updatePeer(peerKey string, allowedIps string, keepAlive time.Duration, endpoint *net.UDPAddr, preSharedKey *wgtypes.Key) error {
+func (c *wGConfigurer) updatePeer(peerKey string, allowedIps string, endpoint *net.UDPAddr) error {
 	//parse allowed ips
 	_, ipNet, err := net.ParseCIDR(allowedIps)
 	if err != nil {
@@ -54,12 +55,13 @@ func (c *wGConfigurer) updatePeer(peerKey string, allowedIps string, keepAlive t
 	if err != nil {
 		return err
 	}
+	keepalive := defaultWgKeepAlive
 	peer := wgtypes.PeerConfig{
 		PublicKey:                   peerKeyParsed,
 		ReplaceAllowedIPs:           true,
 		AllowedIPs:                  []net.IPNet{*ipNet},
-		PersistentKeepaliveInterval: &keepAlive,
-		PresharedKey:                preSharedKey,
+		PersistentKeepaliveInterval: &keepalive,
+		PresharedKey:                c.preSharedKey,
 		Endpoint:                    endpoint,
 	}
 

management/server/account.go

@@ -283,7 +283,7 @@ func (a *Account) GetGroup(groupID string) *Group {
 
 // GetPeerNetworkMap returns a group by ID if exists, nil otherwise
 func (a *Account) GetPeerNetworkMap(peerID, dnsDomain string) *NetworkMap {
-	aclPeers := a.getPeersByACL(peerID)
+	aclPeers, _ := a.getPeersByPolicy(peerID)
 	// exclude expired peers
 	var peersToConnect []*Peer
 	var expiredPeers []*Peer

management/server/file_store.go

@@ -286,7 +286,9 @@ func (s *FileStore) SaveAccount(account *Account) error {
 		s.PrivateDomain2AccountID[accountCopy.Domain] = accountCopy.Id
 	}
 
-	accountCopy.Rules = make(map[string]*Rule)
+	if accountCopy.Rules == nil {
+		accountCopy.Rules = make(map[string]*Rule)
+	}
 	for _, policy := range accountCopy.Policies {
 		for _, rule := range policy.Rules {
 			accountCopy.Rules[rule.ID] = rule.ToRule()

management/server/peer.go

@@ -209,7 +209,7 @@ func (am *DefaultAccountManager) GetPeers(accountID, userID string) ([]*Peer, er
 	// fetch all the peers that have access to the user's peers
 	for _, peer := range peers {
 		// TODO: use firewall rules
-		aclPeers := account.getPeersByACL(peer.ID)
+		aclPeers, _ := account.getPeersByPolicy(peer.ID)
 		for _, p := range aclPeers {
 			peersMap[p.ID] = p
 		}
@@ -816,7 +816,7 @@ func (am *DefaultAccountManager) GetPeer(accountID, peerID, userID string) (*Pee
 	}
 
 	for _, p := range userPeers {
-		aclPeers := account.getPeersByACL(p.ID)
+		aclPeers, _ := account.getPeersByPolicy(p.ID)
 		for _, aclPeer := range aclPeers {
 			if aclPeer.ID == peerID {
 				return peer, nil
@@ -833,98 +833,6 @@ func updatePeerMeta(peer *Peer, meta PeerSystemMeta, account *Account) *Peer {
 	return peer
 }
 
-// GetPeerRules returns a list of source or destination rules of a given peer.
-func (a *Account) GetPeerRules(peerID string) (srcRules []*Rule, dstRules []*Rule) {
-	// Rules are group based so there is no direct access to peers.
-	// First, find all groups that the given peer belongs to
-	peerGroups := make(map[string]struct{})
-
-	for s, group := range a.Groups {
-		for _, peer := range group.Peers {
-			if peerID == peer {
-				peerGroups[s] = struct{}{}
-				break
-			}
-		}
-	}
-
-	// Second, find all rules that have discovered source and destination groups
-	srcRulesMap := make(map[string]*Rule)
-	dstRulesMap := make(map[string]*Rule)
-	for _, rule := range a.Rules {
-		for _, g := range rule.Source {
-			if _, ok := peerGroups[g]; ok && srcRulesMap[rule.ID] == nil {
-				srcRules = append(srcRules, rule)
-				srcRulesMap[rule.ID] = rule
-			}
-		}
-		for _, g := range rule.Destination {
-			if _, ok := peerGroups[g]; ok && dstRulesMap[rule.ID] == nil {
-				dstRules = append(dstRules, rule)
-				dstRulesMap[rule.ID] = rule
-			}
-		}
-	}
-
-	return srcRules, dstRules
-}
-
-// getPeersByACL returns all peers that given peer has access to.
-func (a *Account) getPeersByACL(peerID string) []*Peer {
-	var peers []*Peer
-	srcRules, dstRules := a.GetPeerRules(peerID)
-
-	groups := map[string]*Group{}
-	for _, r := range srcRules {
-		if r.Disabled {
-			continue
-		}
-		if r.Flow == TrafficFlowBidirect {
-			for _, gid := range r.Destination {
-				if group, ok := a.Groups[gid]; ok {
-					groups[gid] = group
-				}
-			}
-		}
-	}
-
-	for _, r := range dstRules {
-		if r.Disabled {
-			continue
-		}
-		if r.Flow == TrafficFlowBidirect {
-			for _, gid := range r.Source {
-				if group, ok := a.Groups[gid]; ok {
-					groups[gid] = group
-				}
-			}
-		}
-	}
-
-	peersSet := make(map[string]struct{})
-	for _, g := range groups {
-		for _, pid := range g.Peers {
-			peer, ok := a.Peers[pid]
-			if !ok {
-				log.Warnf(
-					"peer %s found in group %s but doesn't belong to account %s",
-					pid,
-					g.ID,
-					a.Id,
-				)
-				continue
-			}
-			// exclude original peer
-			if _, ok := peersSet[peer.ID]; peer.ID != peerID && !ok {
-				peersSet[peer.ID] = struct{}{}
-				peers = append(peers, peer.Copy())
-			}
-		}
-	}
-
-	return peers
-}
-
 // updateAccountPeers updates all peers that belong to an account.
 // Should be called when changes have to be synced to peers.
 func (am *DefaultAccountManager) updateAccountPeers(account *Account) error {

management/server/peer_test.go

@@ -136,8 +136,6 @@ func TestAccountManager_GetNetworkMap(t *testing.T) {
 }
 
 func TestAccountManager_GetNetworkMapWithPolicy(t *testing.T) {
-	// TODO: disable until we start use policy again
-	t.Skip()
 	manager, err := createManager(t)
 	if err != nil {
 		t.Fatal(err)