Validate the interface index and release endpoint addresses on the error paths

client/firewall/iptables/manager_linux.go

@@ -528,7 +528,11 @@ const (
 // not always available, so a lookup failure is not an error here.
 func (m *Manager) cleanupNoTrackChain() error {
 	exists, err := m.ipv4Client.ChainExists(tableRaw, chainNameRaw)
-	if err != nil || !exists {
+	if err != nil {
+		log.Debugf("look up %s chain: %v", chainNameRaw, err)
+		return nil
+	}
+	if !exists {
 		return nil
 	}
 

client/iface/wgproxy/loopback/proxy.go

@@ -38,6 +38,7 @@ type Proxy struct {
 
 	conn        *net.UDPConn
 	packetConn  *ipv4.PacketConn
+	loIndex     int
 	rawConnIPv4 net.PacketConn
 	rawConnIPv6 net.PacketConn
 
@@ -73,6 +74,12 @@ func (p *Proxy) Listen() error {
 		log.Warnf("failed to prepare IPv6 raw socket, continuing with IPv4 only: %v", err)
 	}
 
+	loopback, err := net.InterfaceByName(loopbackDevice)
+	if err != nil {
+		return fmt.Errorf("look up %s: %w", loopbackDevice, err)
+	}
+	p.loIndex = loopback.Index
+
 	if err := p.listen(); err != nil {
 		if freeErr := p.Free(); freeErr != nil {
 			log.Errorf("failed to free the wgproxy: %s", freeErr)
@@ -149,11 +156,12 @@ func (p *Proxy) listenOn(proxyPort int) error {
 }
 
 // AddRelayedConn assigns an endpoint address to the relayed connection and
-// returns the address WireGuard should send to.
-func (p *Proxy) AddRelayedConn(relayedConn net.Conn) (*net.UDPAddr, error) {
+// returns the address WireGuard should send to, along with the key the
+// connection is stored under.
+func (p *Proxy) AddRelayedConn(relayedConn net.Conn) (*net.UDPAddr, netip.Addr, error) {
 	addr, err := p.storeRelayedConn(relayedConn)
 	if err != nil {
-		return nil, err
+		return nil, netip.Addr{}, err
 	}
 
 	log.Infof("relayed conn added to wg proxy store: %s, endpoint address: %s", relayedConn.RemoteAddr(), addr)
@@ -161,7 +169,7 @@ func (p *Proxy) AddRelayedConn(relayedConn net.Conn) (*net.UDPAddr, error) {
 	return &net.UDPAddr{
 		IP:   addr.AsSlice(),
 		Port: p.proxyPort,
-	}, nil
+	}, addr, nil
 }
 
 // Free releases the proxy resources. The relayed connections are left open.
@@ -197,7 +205,6 @@ func (p *Proxy) Free() error {
 	return nberrors.FormatErrorOrNil(result)
 }
 
-
 // proxyToRemote reads packets from the local WireGuard instance and forwards
 // them to the relayed connection the destination address belongs to.
 func (p *Proxy) proxyToRemote() {
@@ -222,6 +229,11 @@ func (p *Proxy) readAndForwardPacket(buf []byte) error {
 		return fmt.Errorf("no control message on packet")
 	}
 
+	if cm.IfIndex != p.loIndex {
+		log.Tracef("dropping packet received on interface %d instead of %s", cm.IfIndex, loopbackDevice)
+		return nil
+	}
+
 	dst, ok := netip.AddrFromSlice(cm.Dst.To4())
 	if !ok || !inRange(dst) {
 		log.Tracef("dropping packet for unexpected destination %s", cm.Dst)
@@ -260,12 +272,17 @@ func (p *Proxy) storeRelayedConn(relayedConn net.Conn) (netip.Addr, error) {
 	return addr, nil
 }
 
-func (p *Proxy) removeRelayedConn(addr netip.Addr) {
+// removeRelayedConn releases an endpoint address. It only removes the entry
+// while it still belongs to relayedConn, so a late release cannot take an
+// address away from the peer it was handed to next.
+func (p *Proxy) removeRelayedConn(addr netip.Addr, relayedConn net.Conn) {
 	p.relayedConnMutex.Lock()
 	defer p.relayedConnMutex.Unlock()
 
-	if _, ok := p.relayedConnStore[addr]; ok {
-		log.Debugf("remove relayed conn from store by address: %s", addr)
+	if stored, ok := p.relayedConnStore[addr]; !ok || stored != relayedConn {
+		return
 	}
+
+	log.Debugf("remove relayed conn from store by address: %s", addr)
 	delete(p.relayedConnStore, addr)
 }

client/iface/wgproxy/loopback/proxy_privileged_test.go

@@ -3,6 +3,7 @@
 package loopback
 
 import (
+	"context"
 	"net"
 	"strconv"
 	"testing"
@@ -58,7 +59,7 @@ func TestProxyDemuxesByDestinationAddress(t *testing.T) {
 	readers := make([]*net.UDPConn, 0, peers)
 	for i := 0; i < peers; i++ {
 		proxySide, testSide := relayEnd(t)
-		endpoint, err := proxy.AddRelayedConn(proxySide)
+		endpoint, _, err := proxy.AddRelayedConn(proxySide)
 		if err != nil {
 			t.Fatalf("add relayed conn %d: %v", i, err)
 		}
@@ -135,7 +136,7 @@ func TestProxyDropsPacketsOutsideTheRange(t *testing.T) {
 	}()
 
 	proxySide, testSide := relayEnd(t)
-	if _, err := proxy.AddRelayedConn(proxySide); err != nil {
+	if _, _, err := proxy.AddRelayedConn(proxySide); err != nil {
 		t.Fatalf("add relayed conn: %v", err)
 	}
 
@@ -161,3 +162,35 @@ func TestProxyDropsPacketsOutsideTheRange(t *testing.T) {
 		t.Error("packet addressed to 127.0.0.1 was forwarded to a relayed peer")
 	}
 }
+
+// A wrapper that is closed before it starts forwarding still has to give its
+// endpoint address back, otherwise the range leaks an address per attempt.
+func TestClosingBeforeWorkReleasesTheAddress(t *testing.T) {
+	proxy := NewProxy(testWGPort+2, 1280)
+	if err := proxy.Listen(); err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer func() {
+		if err := proxy.Free(); err != nil {
+			t.Errorf("free proxy: %v", err)
+		}
+	}()
+
+	proxySide, _ := relayEnd(t)
+	wrapper := NewProxyWrapper(proxy)
+	if err := wrapper.AddRelayedConn(context.Background(), nil, proxySide); err != nil {
+		t.Fatalf("add relayed conn: %v", err)
+	}
+
+	if got := len(proxy.relayedConnStore); got != 1 {
+		t.Fatalf("store holds %d entries after adding one conn, want 1", got)
+	}
+
+	if err := wrapper.CloseConn(); err != nil {
+		t.Fatalf("close conn: %v", err)
+	}
+
+	if got := len(proxy.relayedConnStore); got != 0 {
+		t.Errorf("store holds %d entries after close, want 0", got)
+	}
+}

client/iface/wgproxy/loopback/wrapper.go

@@ -124,26 +124,28 @@ func NewProxyWrapper(proxy *Proxy) *ProxyWrapper {
 }
 
 func (p *ProxyWrapper) AddRelayedConn(ctx context.Context, _ *net.UDPAddr, remoteConn net.Conn) error {
-	addr, err := p.proxy.AddRelayedConn(remoteConn)
+	addr, peerAddr, err := p.proxy.AddRelayedConn(remoteConn)
 	if err != nil {
 		return fmt.Errorf("add relayed conn: %w", err)
 	}
 
-	peerAddr, ok := netip.AddrFromSlice(addr.IP.To4())
-	if !ok {
-		return fmt.Errorf("unexpected endpoint address %s", addr.IP)
-	}
+	// the endpoint address is otherwise only released by the forwarding
+	// goroutine, which never starts when the setup below fails
+	release := func() { p.proxy.removeRelayedConn(peerAddr, remoteConn) }
 
 	headers, err := NewPacketHeaders(p.proxy.localWGListenPort, addr)
 	if err != nil {
+		release()
 		return fmt.Errorf("create packet sender: %w", err)
 	}
 
 	// Check if required raw connection is available
 	if !headers.isIPv4 && p.proxy.rawConnIPv6 == nil {
+		release()
 		return errIPv6ConnNotAvailable
 	}
 	if headers.isIPv4 && p.proxy.rawConnIPv4 == nil {
+		release()
 		return errIPv4ConnNotAvailable
 	}
 
@@ -248,6 +250,10 @@ func (p *ProxyWrapper) CloseConn() error {
 
 	p.closeListener.SetCloseListener(nil)
 
+	// releases the endpoint address for a wrapper that was never started, and
+	// is a no-op once the forwarding goroutine has released it
+	p.proxy.removeRelayedConn(p.peerAddr, p.remoteConn)
+
 	p.pausedCond.L.Lock()
 	p.paused = false
 	p.pausedCond.Signal()
@@ -260,7 +266,7 @@ func (p *ProxyWrapper) CloseConn() error {
 }
 
 func (p *ProxyWrapper) proxyToLocal(ctx context.Context) {
-	defer p.proxy.removeRelayedConn(p.peerAddr)
+	defer p.proxy.removeRelayedConn(p.peerAddr, p.remoteConn)
 
 	buf := make([]byte, p.proxy.mtu+bufsize.WGBufferOverhead)
 	for {