pr feedback

.github/workflows/ci.yml

@@ -129,12 +129,12 @@ jobs:
           secrets.PROTONVPN_OPENVPN_PASSWORD }}" | ./ci/runner
           protonvpn-openvpn-port-forwarding
 
-      # - name:
+      - name:
-      #     Run Gluetun container with Private Internet Access OpenVPN and port
+          Run Gluetun container with Private Internet Access OpenVPN and port
-      #     forwarding configuration
+          forwarding configuration
-      #   run: echo -e "${{ secrets.PRIVATEINTERNETACCESS_OPENVPN_USER }}\n${{
+        run: echo -e "${{ secrets.PRIVATEINTERNETACCESS_OPENVPN_USER }}\n${{
-      #     secrets.PRIVATEINTERNETACCESS_OPENVPN_PASSWORD }}" | ./ci/runner
+          secrets.PRIVATEINTERNETACCESS_OPENVPN_PASSWORD }}" | ./ci/runner
-      #     private-internet-access-openvpn-port-forwarding
+          private-internet-access-openvpn-port-forwarding
 
       - name: Run Gluetun container with AirVPN Wireguard configuration
         run: echo -e "${{ secrets.AIRVPN_WIREGUARD_PRIVATE_KEY }}\n${{

Dockerfile

@@ -276,7 +276,7 @@ ENV VPN_SERVICE_PROVIDER=pia \
     PUID=1000 \
     PGID=1000
 ENTRYPOINT ["/gluetun-entrypoint"]
-EXPOSE 8000/tcp 8888/tcp 8388/tcp 8388/udp 1080/tcp 1080/udp
+EXPOSE 8000/tcp 8888/tcp 8388/tcp 8388/udp 1080/tcp
 HEALTHCHECK --interval=5s --timeout=5s --start-period=10s --retries=3 CMD /gluetun-entrypoint healthcheck
 ARG TARGETPLATFORM
 RUN apk add --no-cache --update -l wget && \

README.md

@@ -73,7 +73,7 @@ Lightweight swiss-army-knife-like VPN client to multiple VPN service providers
 - Choose the vpn network protocol, `udp` or `tcp`
 - Built in firewall kill switch to allow traffic only with needed the VPN servers and LAN devices
 - Built in Shadowsocks proxy server (protocol based on SOCKS5 with an encryption layer, tunnels TCP+UDP)
-- Built in Socks5 proxy server (tunnels TCP+UDP) - partial credits to @angelakis and @adjscent
+- Built in Socks5 proxy server (tunnels TCP) - partial credits to @angelakis and @adjscent
 - Built in HTTP proxy (tunnels HTTP and HTTPS through TCP)
 - [Connect other containers to it](https://github.com/qdm12/gluetun-wiki/blob/main/setup/connect-a-container-to-gluetun.md)
 - [Connect LAN devices to it](https://github.com/qdm12/gluetun-wiki/blob/main/setup/connect-a-lan-device-to-gluetun.md)

internal/provider/custom/openvpnconf.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/qdm12/gluetun/internal/configuration/settings"
-	"github.com/qdm12/gluetun/internal/constants"
 	"github.com/qdm12/gluetun/internal/constants/openvpn"
 	"github.com/qdm12/gluetun/internal/models"
 	"github.com/qdm12/gluetun/internal/provider/utils"
@@ -66,11 +65,7 @@ func modifyConfig(lines []string, connection models.Connection,
 	}
 
 	// Add values
-	protocol := connection.Protocol
+	modified = append(modified, "proto "+connection.Protocol)
-	if protocol == constants.TCP {
-		protocol = "tcp-client"
-	}
-	modified = append(modified, "proto "+protocol)
 	modified = append(modified, fmt.Sprintf("remote %s %d", connection.IP, connection.Port))
 	modified = append(modified, "dev "+settings.Interface)
 	modified = append(modified, "mute-replay-warnings")

internal/socks5/socks5.go

@@ -10,7 +10,6 @@ import (
 	"net/netip"
 	"strconv"
 	"strings"
-	"sync"
 )
 
 var (
@@ -211,13 +210,6 @@ func (c *socksConn) handleUDPAssociateRequest(ctx context.Context,
 		return fmt.Errorf("getting udp association addresses: %w", err)
 	}
 
-	association, err := c.udpRouter.registerAssociation(c.clientConn, expectedAddrPort)
-	if err != nil {
-		c.encodeFailedResponse(c.clientConn, socksVersion, generalServerFailure)
-		return fmt.Errorf("registering udp association: %w", err)
-	}
-	defer c.udpRouter.unregisterAssociation(association)
-
 	err = c.encodeSuccessResponse(c.clientConn, socksVersion, succeeded,
 		bindAddrType, bindAddress, bindPort)
 	if err != nil {
@@ -225,21 +217,28 @@ func (c *socksConn) handleUDPAssociateRequest(ctx context.Context,
 		return fmt.Errorf("writing successful %s response: %w", udpAssociate, err)
 	}
 
+	association, err := c.udpRouter.registerAssociation(c.clientConn, expectedAddrPort)
+	if err != nil {
+		c.encodeFailedResponse(c.clientConn, socksVersion, generalServerFailure)
+		return fmt.Errorf("registering udp association: %w", err)
+	}
+	defer c.udpRouter.unregisterAssociation(association)
+
 	associationCtx, associationCancel := context.WithCancel(ctx)
 	defer associationCancel()
 
-	var wg sync.WaitGroup
+	handlerDone := make(chan struct{})
-
+	go func() {
-	wg.Go(func() {
+		defer close(handlerDone)
 		c.udpRouter.runAssociationHandler(associationCtx, association)
-	})
+	}()
 
-	wg.Go(func() {
+	go func() {
 		_, _ = io.Copy(io.Discard, c.clientConn)
 		associationCancel()
-	})
+	}()
 	<-associationCtx.Done()
-	wg.Wait()
+	<-handlerDone
 	return nil
 }
 

internal/socks5/udp_router.go

@@ -48,7 +48,7 @@ func newUDPRouter(ctx context.Context, address string, logger Logger) (router *u
 		listener: listener,
 		bufferPool: sync.Pool{
 			New: func() any {
-				return bytes.NewBuffer(make([]byte, 0, pooledUDPPacketBufferCapacity))
+				return bytes.NewBuffer(make([]byte, pooledUDPPacketBufferCapacity))
 			},
 		},
 		nextAssociationID:             1,
@@ -111,6 +111,10 @@ func (r *udpRouter) unregisterAssociation(association udpAssociation) {
 		delete(r.clientAddrPortToAssociation, clientAddrPort)
 	}
 
+	if association.clientAddrPort.IsValid() {
+		delete(r.clientAddrPortToAssociation, association.clientAddrPort)
+	}
+
 	pendingAssociations := r.clientIPToPendingAssociations[association.controlConnAddr]
 	for i, pendingAssociation := range pendingAssociations {
 		if pendingAssociation.id == association.id {
@@ -159,9 +163,8 @@ func (r *udpRouter) run(ctx context.Context) error {
 
 func (r *udpRouter) routePacket(sourceAddrPort netip.AddrPort, packet *bytes.Buffer) error {
 	r.mutex.Lock()
+	defer r.mutex.Unlock()
 	association, packetFromClient := r.findClientAssociation(sourceAddrPort)
-	r.mutex.Unlock()
-
 	if !packetFromClient {
 		r.bufferPool.Put(packet)
 		return nil
@@ -331,23 +334,6 @@ func (r *udpRouter) writeClientPacketToDestination(ctx context.Context,
 		return fmt.Errorf("decoding UDP datagram: %w", err)
 	}
 
-	host, portStr, err := net.SplitHostPort(destination)
-	if err != nil {
-		return fmt.Errorf("splitting destination host and port: %w", err)
-	}
-
-	if _, err := netip.ParseAddr(host); err != nil { // domain name
-		addrs, err := net.DefaultResolver.LookupHost(ctx, host)
-		if err != nil {
-			return fmt.Errorf("resolving destination host: %w", err)
-		}
-		if len(addrs) == 0 {
-			return fmt.Errorf("resolving destination host: no addresses found for %q", host)
-		}
-
-		destination = net.JoinHostPort(addrs[0], portStr)
-	}
-
 	resolvedDestinationUDPAddress, err := net.ResolveUDPAddr("udp", destination)
 	if err != nil {
 		return fmt.Errorf("resolving destination UDP address: %w", err)