Make sure err isn't nil when returning failure

Signed-off-by: Jan Dubois <[email protected]>

.github/workflows/test.yml

@@ -443,9 +443,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        template:
-        - default.yaml
-        - fedora.yaml
+        include:
+        - {template: default.yaml, ssh-port-forwarder: true}
+        - {template: fedora.yaml, ssh-port-forwarder: false}
     steps:
     - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2
       with:
@@ -467,6 +467,8 @@ jobs:
       run: brew uninstall --ignore-dependencies --force qemu
     - name: Test
       run: ./hack/test-templates.sh templates/${{ matrix.template }}
+      env:
+        LIMA_SSH_PORT_FORWARDER: ${{ matrix.ssh-port-forwarder }}
     - if: failure()
       uses: ./.github/actions/upload_failure_logs_if_exists
       with:

hack/test-templates.sh

@@ -307,9 +307,17 @@ if [[ -n ${CHECKS["port-forwards"]} ]]; then
 			fi
 			limactl shell "$NAME" $sudo $CONTAINER_ENGINE info
 			limactl shell "$NAME" $sudo $CONTAINER_ENGINE pull --quiet ${nginx_image}
-			limactl shell "$NAME" $sudo $CONTAINER_ENGINE run -d --name nginx -p 8888:80 ${nginx_image}
 
+			limactl shell "$NAME" $sudo $CONTAINER_ENGINE run -d --name nginx -p 8888:80 ${nginx_image}
 			timeout 3m bash -euxc "until curl -f --retry 30 --retry-connrefused http://${hostip}:8888; do sleep 3; done"
+			limactl shell "$NAME" $sudo $CONTAINER_ENGINE rm -f nginx
+
+			if [ "$(uname)" = "Darwin" ]; then
+				# Only macOS can bind to port 80 without root
+				limactl shell "$NAME" $sudo $CONTAINER_ENGINE run -d --name nginx -p 127.0.0.1:80:80 ${nginx_image}
+				timeout 3m bash -euxc "until curl -f --retry 30 --retry-connrefused http://localhost:80; do sleep 3; done"
+				limactl shell "$NAME" $sudo $CONTAINER_ENGINE rm -f nginx
+			fi
 		fi
 	fi
 	set +x

pkg/hostagent/port_darwin.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/lima-vm/lima/pkg/bicopy"
+	"github.com/lima-vm/lima/pkg/portfwd"
 	"github.com/lima-vm/sshocker/pkg/ssh"
 	"github.com/sirupsen/logrus"
 )
@@ -96,11 +97,12 @@ func newPseudoLoopbackForwarder(localPort int, unixSock string) (*pseudoLoopback
 		return nil, err
 	}
 
-	lnAddr, err := net.ResolveTCPAddr("tcp4", fmt.Sprintf("0.0.0.0:%d", localPort))
+	// use "tcp" network to listen on both "tcp4" and "tcp6"
+	lnAddr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("0.0.0.0:%d", localPort))
 	if err != nil {
 		return nil, err
 	}
-	ln, err := net.ListenTCP("tcp4", lnAddr)
+	ln, err := net.ListenTCP("tcp", lnAddr)
 	if err != nil {
 		return nil, err
 	}
@@ -127,7 +129,7 @@ func (plf *pseudoLoopbackForwarder) Serve() error {
 			ac.Close()
 			continue
 		}
-		if remoteAddrIP != "127.0.0.1" {
+		if !portfwd.IsLoopback(remoteAddrIP) {
 			logrus.WithError(err).Debugf("pseudoloopback forwarder: rejecting non-loopback remoteAddr %q", remoteAddr)
 			ac.Close()
 			continue

pkg/portfwd/listener.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"strings"
 	"sync"
 
 	guestagentclient "github.com/lima-vm/lima/pkg/guestagent/api/client"
@@ -42,6 +43,7 @@ func (p *ClosableListeners) Forward(ctx context.Context, client *guestagentclien
 }
 
 func (p *ClosableListeners) Remove(_ context.Context, protocol, hostAddress, guestAddress string) {
+	logrus.Debugf("removing listener for hostAddress: %s, guestAddress: %s", hostAddress, guestAddress)
 	key := key(protocol, hostAddress, guestAddress)
 	switch protocol {
 	case "tcp", "tcp6":
@@ -65,7 +67,6 @@ func (p *ClosableListeners) Remove(_ context.Context, protocol, hostAddress, gue
 
 func (p *ClosableListeners) forwardTCP(ctx context.Context, client *guestagentclient.GuestAgentClient, hostAddress, guestAddress string) {
 	key := key("tcp", hostAddress, guestAddress)
-	defer p.Remove(ctx, "tcp", hostAddress, guestAddress)
 
 	p.listenersRW.Lock()
 	_, ok := p.listeners[key]
@@ -75,16 +76,21 @@ func (p *ClosableListeners) forwardTCP(ctx context.Context, client *guestagentcl
 	}
 	tcpLis, err := Listen(ctx, p.listenConfig, hostAddress)
 	if err != nil {
-		logrus.Errorf("failed to accept TCP connection: %v", err)
+		logrus.Errorf("failed to listen to TCP connection: %v", err)
 		p.listenersRW.Unlock()
 		return
 	}
+	defer p.Remove(ctx, "tcp", hostAddress, guestAddress)
 	p.listeners[key] = tcpLis
 	p.listenersRW.Unlock()
 	for {
 		conn, err := tcpLis.Accept()
 		if err != nil {
 			logrus.Errorf("failed to accept TCP connection: %v", err)
+			if strings.Contains(err.Error(), "pseudoloopback") {
+				// don't stop forwarding because the forwarder has rejected a non-local address
+				continue
+			}
 			return
 		}
 		go HandleTCPConnection(ctx, client, conn, guestAddress)

pkg/portfwd/listener_darwin.go

@@ -68,10 +68,13 @@ func (p pseudoLoopbackListener) Accept() (net.Conn, error) {
 		conn.Close()
 		return nil, err
 	}
-	if remoteAddrIP != "127.0.0.1" {
-		logrus.WithError(err).Debugf("pseudoloopback forwarder: rejecting non-loopback remoteAddr %q", remoteAddr)
+	if !IsLoopback(remoteAddrIP) {
+		err := fmt.Errorf("pseudoloopback forwarder: rejecting non-loopback remoteAddr %q", remoteAddr)
+		logrus.Debug(err)
+		conn.Close()
 		return nil, err
 	}
+	logrus.Infof("pseudoloopback forwarder: accepting connection from %q", remoteAddr)
 	return conn, nil
 }
 
@@ -89,7 +92,7 @@ func (pk *pseudoLoopbackPacketConn) ReadFrom(bytes []byte) (n int, addr net.Addr
 	if err != nil {
 		return 0, nil, err
 	}
-	if remoteAddrIP != "127.0.0.1" {
+	if !IsLoopback(remoteAddrIP) {
 		return 0, nil, fmt.Errorf("pseudoloopback forwarder: rejecting non-loopback remoteAddr %q", remoteAddr)
 	}
 	return n, remoteAddr, nil
@@ -100,8 +103,12 @@ func (pk *pseudoLoopbackPacketConn) WriteTo(bytes []byte, remoteAddr net.Addr) (
 	if err != nil {
 		return 0, err
 	}
-	if remoteAddrIP != "127.0.0.1" {
+	if !IsLoopback(remoteAddrIP) {
 		return 0, fmt.Errorf("pseudoloopback forwarder: rejecting non-loopback remoteAddr %q", remoteAddr)
 	}
 	return pk.PacketConn.WriteTo(bytes, remoteAddr)
 }
+
+func IsLoopback(addr string) bool {
+	return addr == "127.0.0.1" || addr == "::1"
+}