From b8b400c8accd9d5fb20cd86af43559160be0de62 Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Thu, 7 Nov 2024 17:58:46 -0800 Subject: [PATCH] Make sure err isn't nil when returning failure Signed-off-by: Jan Dubois --- .github/workflows/test.yml | 8 +++++--- hack/test-templates.sh | 10 +++++++++- pkg/hostagent/port_darwin.go | 8 +++++--- pkg/portfwd/listener.go | 10 ++++++++-- pkg/portfwd/listener_darwin.go | 15 +++++++++++---- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 72b8757434f..eb93d02af01 100644 --- a/.github/workflows/test.yml +++ b/.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} # SSH port forwarder is currently still the default + - {template: fedora.yaml, ssh-port-forwarder: false} # gRPC port forwarder will become default in the future 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: diff --git a/hack/test-templates.sh b/hack/test-templates.sh index 57ff37a450b..b4c90f3c3e1 100755 --- a/hack/test-templates.sh +++ b/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 diff --git a/pkg/hostagent/port_darwin.go b/pkg/hostagent/port_darwin.go index 615a2bd03f9..a99897b6f3e 100644 --- a/pkg/hostagent/port_darwin.go +++ b/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 diff --git a/pkg/portfwd/listener.go b/pkg/portfwd/listener.go index b5649ddaeaa..9911edcbe08 100644 --- a/pkg/portfwd/listener.go +++ b/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) diff --git a/pkg/portfwd/listener_darwin.go b/pkg/portfwd/listener_darwin.go index 3c1d080e747..d5f83a7c4bd 100644 --- a/pkg/portfwd/listener_darwin.go +++ b/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 net.ParseIP(addr).IsLoopback() +}