diff --git a/cmd/topo/deploy.go b/cmd/topo/deploy.go index 237502dd..6ebbd834 100644 --- a/cmd/topo/deploy.go +++ b/cmd/topo/deploy.go @@ -10,6 +10,7 @@ import ( "github.com/arm/topo/internal/deploy" "github.com/arm/topo/internal/deploy/operation" checks "github.com/arm/topo/internal/deploy/project_checks" + "github.com/arm/topo/internal/env" goperation "github.com/arm/topo/internal/operation" "github.com/arm/topo/internal/output/logger" "github.com/arm/topo/internal/ssh" @@ -18,11 +19,12 @@ import ( ) var ( - noRegistry bool - registryPort string - skipProjectChecks bool - forceRecreate bool - noRecreate bool + noRegistry bool + registryPort string + skipRemotePortCheck bool + skipProjectChecks bool + forceRecreate bool + noRecreate bool ) var deployOpts deploy.DeployOptions @@ -78,8 +80,9 @@ The compose file (compose.yaml) must be in the current working directory, as thi goos := runtime.GOOS if deploy.SupportsRegistry(noRegistry, deployOpts.TargetHost) { deployOpts.Registry = &deploy.RegistryConfig{ - Port: resolvedPort, - UseControlSockets: deploy.SupportsSSHControlSockets(goos), + Port: resolvedPort, + SkipRemotePortCheck: resolveSkipRemotePortCheck(cmd), + UseControlSockets: deploy.SupportsSSHControlSockets(goos), } } switch { @@ -127,7 +130,10 @@ func validatePort(port string) error { return nil } -const portEnvVar = "TOPO_PORT" +const ( + portEnvVar = "TOPO_PORT" + skipRemotePortCheckEnvVar = "TOPO_SKIP_REMOTE_PORT_CHECK" +) func resolvePort(cmd *cobra.Command, flagValue string) (string, error) { if cmd.Flags().Changed("registry-port") { @@ -139,10 +145,20 @@ func resolvePort(cmd *cobra.Command, flagValue string) (string, error) { return flagValue, nil } +func resolveSkipRemotePortCheck(cmd *cobra.Command) bool { + flagValue, _ := cmd.Flags().GetBool("skip-remote-port-check") + if cmd.Flags().Changed("skip-remote-port-check") { + return flagValue + } + + return env.IsVarTruthy(skipRemotePortCheckEnvVar) +} + func init() { addTargetFlag(deployCmd) deployCmd.Flags().StringVarP(®istryPort, "registry-port", "p", operation.DefaultRegistryPort, fmt.Sprintf("registry and SSH tunnel port (can also be set via %s env var)", portEnvVar)) deployCmd.Flags().BoolVar(&noRegistry, "no-registry", false, "disable private registry flow; use direct save/load transfer") + deployCmd.Flags().BoolVar(&skipRemotePortCheck, "skip-remote-port-check", false, fmt.Sprintf("skip checking whether the SSH tunnel port is exposed on the remote network (can also be set via %s env var)", skipRemotePortCheckEnvVar)) deployCmd.Flags().BoolVar(&forceRecreate, "force-recreate", false, "force recreation of containers even if their configuration and image haven't changed") deployCmd.Flags().BoolVar(&noRecreate, "no-recreate", false, "prevent recreation of containers even if their configuration and image have changed") deployCmd.Flags().BoolVar(&skipProjectChecks, "skip-project-checks", false, "skip project compatibility checks for the target platform") diff --git a/internal/deploy/deploy.go b/internal/deploy/deploy.go index ad10c5fa..022e70bd 100644 --- a/internal/deploy/deploy.go +++ b/internal/deploy/deploy.go @@ -9,8 +9,9 @@ import ( ) type RegistryConfig struct { - Port string - UseControlSockets bool + Port string + SkipRemotePortCheck bool + UseControlSockets bool } type DeployOptions struct { @@ -46,7 +47,9 @@ func NewDeployment(composeFile string, opts DeployOptions) (goperation.Sequence, cleanup = stop ops = append(ops, operation.NewRunRegistry(opts.Registry.Port)...) ops = append(ops, start) - ops = append(ops, securityCheck) + if !opts.Registry.SkipRemotePortCheck { + ops = append(ops, securityCheck) + } ops = append(ops, operation.NewRegistryTransfer(composeFile, sourceHost, targetHost, opts.Registry.Port)) ops = append(ops, stop) } else { diff --git a/internal/deploy/deploy_test.go b/internal/deploy/deploy_test.go index 72617e1f..84bac7ba 100644 --- a/internal/deploy/deploy_test.go +++ b/internal/deploy/deploy_test.go @@ -148,6 +148,21 @@ func TestNewDeployment(t *testing.T) { ) assert.Equal(t, want, got) }) + + t.Run("excludes the remote port check when skipped", func(t *testing.T) { + remoteDest := ssh.NewDestination("user@remote") + opts := deploy.DeployOptions{ + TargetHost: remoteDest, + Registry: &deploy.RegistryConfig{ + SkipRemotePortCheck: true, + }, + } + + got, _ := deploy.NewDeployment(composeFile, opts) + + _, remotePortCheck, _ := ssh.NewSSHTunnel(remoteDest, opts.Registry.Port, opts.Registry.UseControlSockets) + assert.NotContains(t, got, remotePortCheck) + }) } func TestDeployment(t *testing.T) { diff --git a/internal/ssh/config.go b/internal/ssh/config.go index b689f8b9..f3a018fa 100644 --- a/internal/ssh/config.go +++ b/internal/ssh/config.go @@ -22,6 +22,19 @@ func NewConfig(dest Destination) Config { return NewConfigFromBytes(output) } +func resolveHostName(dest Destination) (string, error) { + output, err := readConfig(dest) + if err != nil { + return "", fmt.Errorf("could not resolve SSH configuration for %q: %w", dest.String(), err) + } + + hostName := NewConfigFromBytes(output).HostName + if hostName == "" { + return "", fmt.Errorf("could not resolve SSH hostname for %q: SSH configuration did not provide a hostname", dest.String()) + } + return hostName, nil +} + func NewConfigFromBytes(data []byte) Config { var config Config scanner := bufio.NewScanner(bytes.NewReader(data)) diff --git a/internal/ssh/ssh_tunnel.go b/internal/ssh/ssh_tunnel.go index 9416dfec..325df4b1 100644 --- a/internal/ssh/ssh_tunnel.go +++ b/internal/ssh/ssh_tunnel.go @@ -2,12 +2,15 @@ package ssh import ( "crypto/sha256" + "errors" "fmt" "io" + "net" "os" "os/exec" "path/filepath" "runtime" + "strings" "github.com/arm/topo/internal/command" "github.com/arm/topo/internal/operation" @@ -92,7 +95,7 @@ type CheckRemoteForwardNotExposed struct { // Checks whether the RemoteForward port exposes the registry to the target's // network, rather than being limited to target loopback. This can happen when -// sshd permits non-loopback remote forwards, such as GatewayPorts. +// the SSH server permits non-loopback remote forwards. func NewCheckRemoteForwardNotExposed(targetDest Destination, port string) *CheckRemoteForwardNotExposed { return &CheckRemoteForwardNotExposed{TargetDest: targetDest, Port: port} } @@ -106,27 +109,53 @@ func (ct *CheckRemoteForwardNotExposed) Run(w io.Writer) error { return nil } - host := NewConfig(ct.TargetDest).HostName - if RemotePortRefusedConnection(host, ct.Port) { - _, _ = fmt.Fprintf(w, "Port %s is bound to remote loopback only\n", ct.Port) - return nil + host, err := resolveHostName(ct.TargetDest) + if err != nil { + return remotePortCheckErrorWithSuggestion(err, ct.Port) + } + listening, err := isRemotePortListening(host, ct.Port) + if err != nil { + return remotePortCheckErrorWithSuggestion(err, ct.Port) } - return fmt.Errorf("remote sshd might be exposing the forwarded port %s on its network (likely GatewayPorts=yes); the local registry may be reachable without SSH auth", ct.Port) + if listening { + return fmt.Errorf("the remote SSH server is exposing forwarded registry port %s beyond remote loopback; configure the SSH server to bind remote forwards to loopback only, or use `--skip-remote-port-check` if you understand that the registry may be reachable without SSH authentication", ct.Port) + } + _, _ = fmt.Fprintf(w, "Port %s is bound to remote loopback only\n", ct.Port) + return nil } -func RemotePortRefusedConnection(host, port string) bool { - cmd := exec.Command("curl", fmt.Sprintf("%s:%s", host, port), "--max-time", "5") - cmd.Stdout = io.Discard +func isRemotePortListening(host, port string) (bool, error) { + address := net.JoinHostPort(host, port) + var remoteIP strings.Builder + // Release binaries use Go's resolver because CGO is disabled. Use curl so + // host resolution matches OpenSSH for mDNS, NSS, and split-DNS configurations. + // remote_ip detects a connection even when HTTP fails, noproxy ensures the + // connection is direct, and silent suppresses curl's progress and errors. + cmd := exec.Command("curl", "http://"+address, "--max-time", "5", "--noproxy", "*", "--output", os.DevNull, "--silent", "--write-out", "%{remote_ip}") + cmd.Stdout = &remoteIP cmd.Stderr = io.Discard - _ = cmd.Run() + err := cmd.Run() + if err == nil || remoteIP.Len() > 0 { + return true, nil + } - // Only curl exit 7 ("Failed to connect to host") proves no TCP listener - // answered. Anything else — connection succeeded, DNS failure, timeout, - // curl missing — leaves us unable to certify the port is not not listening. - if cmd.ProcessState != nil && cmd.ProcessState.ExitCode() == 7 { - return true + exitError, ok := errors.AsType[*exec.ExitError](err) + if ok { + switch exitError.ExitCode() { + case 7: + return false, nil + case 28: + return false, fmt.Errorf("timed out while checking whether remote port %s is exposed: %w", address, err) + case 6: + return false, fmt.Errorf("could not resolve remote host %q while checking tunnel exposure: %w", host, err) + } } - return false + + return false, fmt.Errorf("could not verify whether remote port %s is exposed: %w", address, err) +} + +func remotePortCheckErrorWithSuggestion(err error, port string) error { + return fmt.Errorf("cannot conclusively rule out network access to registry port %s because the exposure check did not complete: %w; retry after resolving the connectivity issue, or use `--skip-remote-port-check` if you understand the security risk", port, err) } type SSHTunnelStop struct { diff --git a/internal/ssh/ssh_tunnel_test.go b/internal/ssh/ssh_tunnel_test.go index f79a672e..f2dba956 100644 --- a/internal/ssh/ssh_tunnel_test.go +++ b/internal/ssh/ssh_tunnel_test.go @@ -2,6 +2,7 @@ package ssh_test import ( "fmt" + "io" "net" "os" "strings" @@ -106,36 +107,69 @@ func TestCheckRemoteForwardNotExposed(t *testing.T) { assert.Equal(t, "Check tunnel port is not exposed on remote network", got) }) }) -} -func TestRemotePortRefusedConnection(t *testing.T) { - t.Run("it returns true when nothing answers on the remote port", func(t *testing.T) { - port := reserveFreePort(t) + t.Run("Run", func(t *testing.T) { + t.Run("it skips the check for localhost", func(t *testing.T) { + check := ssh.NewCheckRemoteForwardNotExposed(ssh.PlainLocalhost, "invalid") + var output strings.Builder - assert.True(t, ssh.RemotePortRefusedConnection("127.0.0.1", port)) - }) + err := check.Run(&output) + + assert.NoError(t, err) + assert.Empty(t, output.String()) + }) - t.Run("it returns false when a TCP listener is bound on the remote port", func(t *testing.T) { - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - t.Cleanup(func() { _ = listener.Close() }) - _, port, err := net.SplitHostPort(listener.Addr().String()) - require.NoError(t, err) + t.Run("it fails when the SSH hostname cannot be resolved", func(t *testing.T) { + check := ssh.NewCheckRemoteForwardNotExposed(ssh.Destination{}, "12345") - assert.False(t, ssh.RemotePortRefusedConnection("127.0.0.1", port)) - }) + err := check.Run(io.Discard) + + assert.ErrorContains(t, err, "cannot conclusively rule out network access to registry port 12345") + assert.ErrorContains(t, err, `could not resolve SSH configuration for "ssh://"`) + assert.ErrorContains(t, err, "use `--skip-remote-port-check` if you understand the security risk") + }) + + t.Run("it succeeds when the remote port refuses the connection", func(t *testing.T) { + port := reserveFreePort(t, "0.0.0.0") + check := ssh.NewCheckRemoteForwardNotExposed(ssh.NewDestination("0.0.0.0"), port) + var output strings.Builder + + err := check.Run(&output) - t.Run("it returns false when curl exits with any other non-7 error", func(t *testing.T) { - // `.invalid` is reserved by RFC 6761 to never resolve, so curl - // exits 6 ("Couldn't resolve host") rather than 7. We can't - // certify the tunnel is safe, so we must fail. - assert.False(t, ssh.RemotePortRefusedConnection("nonexistent.invalid", "12345")) + assert.NoError(t, err) + assert.Equal(t, fmt.Sprintf("Port %s is bound to remote loopback only\n", port), output.String()) + }) + + t.Run("it fails when the remote port accepts a connection", func(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + connectionClosed := make(chan error, 1) + go func() { + connection, acceptErr := listener.Accept() + if acceptErr != nil { + connectionClosed <- acceptErr + return + } + connectionClosed <- connection.Close() + }() + _, port, err := net.SplitHostPort(listener.Addr().String()) + require.NoError(t, err) + check := ssh.NewCheckRemoteForwardNotExposed(ssh.NewDestination("localhost."), port) + + err = check.Run(io.Discard) + listenerCloseErr := listener.Close() + + assert.EqualError(t, err, fmt.Sprintf("the remote SSH server is exposing forwarded registry port %s beyond remote loopback; configure the SSH server to bind remote forwards to loopback only, or use `--skip-remote-port-check` if you understand that the registry may be reachable without SSH authentication", port)) + assert.NoError(t, listenerCloseErr) + assert.NoError(t, <-connectionClosed) + }) }) } -func reserveFreePort(t *testing.T) string { +func reserveFreePort(t *testing.T, host string) string { t.Helper() - listener, err := net.Listen("tcp", "127.0.0.1:0") + listener, err := net.Listen("tcp", net.JoinHostPort(host, "0")) require.NoError(t, err) _, port, err := net.SplitHostPort(listener.Addr().String()) require.NoError(t, err)