Be explicit about ssh configs suitable only for localhost

... and warn loudly against generalization.

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>

Author: mtrmac

cmd/podman/machine/cp.go

@@ -95,7 +95,7 @@ func cp(_ *cobra.Command, args []string) error {
 	cpOpts.SrcPath = srcPath
 	cpOpts.DestPath = destPath
 
-	err = secureCopy(&cpOpts)
+	err = localhostSSHCopy(&cpOpts)
 	if err != nil {
 		return fmt.Errorf("copy failed: %s", err.Error())
 	}
@@ -105,7 +105,8 @@ func cp(_ *cobra.Command, args []string) error {
 	return nil
 }
 
-func secureCopy(opts *cpOptions) error {
+// localhostSSHCopy uses scp to copy files from/to a localhost machine using ssh.
+func localhostSSHCopy(opts *cpOptions) error {
 	srcPath := opts.SrcPath
 	destPath := opts.DestPath
 	sshConfig := opts.Machine.SSH
@@ -123,7 +124,7 @@ func secureCopy(opts *cpOptions) error {
 	}
 
 	args := []string{"-r", "-i", sshConfig.IdentityPath, "-P", strconv.Itoa(sshConfig.Port)}
-	args = append(args, machine.CommonSSHArgs()...)
+	args = append(args, machine.LocalhostSSHArgs()...) // Warning: This MUST NOT be generalized to allow communication over untrusted networks.
 	args = append(args, []string{srcPath, destPath}...)
 
 	cmd := exec.Command("scp", args...)

cmd/podman/machine/ssh.go

@@ -115,6 +115,6 @@ func ssh(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	err = machine.CommonSSHShell(sshOpts.Username, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, sshOpts.Args)
+	err = machine.LocalhostSSHShell(sshOpts.Username, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, sshOpts.Args)
 	return utils.HandleOSExecError(err)
 }

pkg/machine/hyperv/volumes.go

@@ -64,7 +64,7 @@ func startShares(mc *vmconfigs.MachineConfig) error {
 		}
 		args = append(args, "machine", "client9p", fmt.Sprintf("%d", *mount.VSockNumber), strconv.Quote(mount.Target))
 
-		if err := machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil {
+		if err := machine.LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil {
 			return err
 		}
 	}

pkg/machine/os/machine_os.go

@@ -24,7 +24,7 @@ type MachineOS struct {
 func (m *MachineOS) Apply(image string, opts ApplyOptions) error {
 	args := []string{"podman", "machine", "os", "apply", image}
 
-	if err := machine.CommonSSH(m.VM.SSH.RemoteUsername, m.VM.SSH.IdentityPath, m.VMName, m.VM.SSH.Port, args); err != nil {
+	if err := machine.LocalhostSSH(m.VM.SSH.RemoteUsername, m.VM.SSH.IdentityPath, m.VMName, m.VM.SSH.Port, args); err != nil {
 		return err
 	}
 

pkg/machine/proxyenv/env.go

@@ -52,6 +52,6 @@ func getProxyScript(isWSL bool) io.Reader {
 }
 
 func ApplyProxies(mc *vmconfigs.MachineConfig) error {
-	return machine.CommonSSHWithStdin("root", mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, []string{"/usr/bin/bash"},
+	return machine.LocalhostSSHWithStdin("root", mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, []string{"/usr/bin/bash"},
 		getProxyScript(mc.WSLHypervisor != nil))
 }

pkg/machine/qemu/stubber.go

@@ -353,7 +353,7 @@ func (q *QEMUStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool)
 		if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") {
 			args = append(args, ";", "sudo", "chattr", "+i", "/", ";")
 		}
-		err := machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args)
+		err := machine.LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args)
 		if err != nil {
 			return err
 		}
@@ -368,7 +368,7 @@ func (q *QEMUStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool)
 			mountFlags += ",ro"
 		}
 		mountOptions = append(mountOptions, "-o", mountFlags)
-		err = machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, append([]string{"sudo", "mount"}, mountOptions...))
+		err = machine.LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, append([]string{"sudo", "mount"}, mountOptions...))
 		if err != nil {
 			return err
 		}

pkg/machine/shim/host.go

@@ -609,7 +609,7 @@ func Start(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDe
 	}
 
 	if mp.VMType() == machineDefine.WSLVirt && mc.Ansible != nil && mc.IsFirstBoot() {
-		if err := machine.CommonSSHSilent(mc.Ansible.User, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, []string{"ansible-playbook", mc.Ansible.PlaybookPath}); err != nil {
+		if err := machine.LocalhostSSHSilent(mc.Ansible.User, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, []string{"ansible-playbook", mc.Ansible.PlaybookPath}); err != nil {
 			logrus.Error(err)
 		}
 	}

pkg/machine/shim/networking.go

@@ -150,7 +150,7 @@ func conductVMReadinessCheck(mc *vmconfigs.MachineConfig, maxBackoffs int, backo
 		// CoreOS users have reported the same observation but
 		// the underlying source of the issue remains unknown.
 
-		if sshError = machine.CommonSSHSilent(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, []string{"true"}); sshError != nil {
+		if sshError = machine.LocalhostSSHSilent(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, []string{"true"}); sshError != nil {
 			logrus.Debugf("SSH readiness check for machine failed: %v", sshError)
 			continue
 		}

pkg/machine/ssh.go

@@ -13,27 +13,27 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-// CommonSSH is a common function for ssh'ing to a podman machine using system-connections
+// LocalhostSSH is a common function for ssh'ing to a podman machine using system-connections
 // and a port
 // TODO This should probably be taught about an machineconfig to reduce input
-func CommonSSH(username, identityPath, name string, sshPort int, inputArgs []string) error {
-	return commonBuiltinSSH(username, identityPath, name, sshPort, inputArgs, true, os.Stdin)
+func LocalhostSSH(username, identityPath, name string, sshPort int, inputArgs []string) error {
+	return localhostBuiltinSSH(username, identityPath, name, sshPort, inputArgs, true, os.Stdin)
 }
 
-func CommonSSHShell(username, identityPath, name string, sshPort int, inputArgs []string) error {
-	return commonNativeSSH(username, identityPath, name, sshPort, inputArgs, os.Stdin)
+func LocalhostSSHShell(username, identityPath, name string, sshPort int, inputArgs []string) error {
+	return localhostNativeSSH(username, identityPath, name, sshPort, inputArgs, os.Stdin)
 }
 
-func CommonSSHSilent(username, identityPath, name string, sshPort int, inputArgs []string) error {
-	return commonBuiltinSSH(username, identityPath, name, sshPort, inputArgs, false, nil)
+func LocalhostSSHSilent(username, identityPath, name string, sshPort int, inputArgs []string) error {
+	return localhostBuiltinSSH(username, identityPath, name, sshPort, inputArgs, false, nil)
 }
 
-func CommonSSHWithStdin(username, identityPath, name string, sshPort int, inputArgs []string, stdin io.Reader) error {
-	return commonBuiltinSSH(username, identityPath, name, sshPort, inputArgs, true, stdin)
+func LocalhostSSHWithStdin(username, identityPath, name string, sshPort int, inputArgs []string, stdin io.Reader) error {
+	return localhostBuiltinSSH(username, identityPath, name, sshPort, inputArgs, true, stdin)
 }
 
-func commonBuiltinSSH(username, identityPath, name string, sshPort int, inputArgs []string, passOutput bool, stdin io.Reader) error {
-	config, err := createConfig(username, identityPath)
+func localhostBuiltinSSH(username, identityPath, name string, sshPort int, inputArgs []string, passOutput bool, stdin io.Reader) error {
+	config, err := createLocalhostConfig(username, identityPath) // WARNING: This MUST NOT be generalized to allow communication over untrusted networks.
 	if err != nil {
 		return err
 	}
@@ -91,7 +91,10 @@ func runSessionWithDebug(session *ssh.Session, cmd string) error {
 	return session.Wait()
 }
 
-func createConfig(user string, identityPath string) (*ssh.ClientConfig, error) {
+// createLocalhostConfig returns a *ssh.ClientConfig for authenticating a user using a private key
+//
+// WARNING: This MUST NOT be used to communicate over untrusted networks.
+func createLocalhostConfig(user string, identityPath string) (*ssh.ClientConfig, error) {
 	key, err := os.ReadFile(identityPath)
 	if err != nil {
 		return nil, err
@@ -103,18 +106,23 @@ func createConfig(user string, identityPath string) (*ssh.ClientConfig, error) {
 	}
 
 	return &ssh.ClientConfig{
-		User:            user,
-		Auth:            []ssh.AuthMethod{ssh.PublicKeys(signer)},
+		// Not specifying ciphers / MACs seems to allow fairly weak ciphers. This config is restricted
+		// to connecting to localhost: where we rely on the kernel’s process isolation, not primarily on cryptography.
+		User: user,
+		Auth: []ssh.AuthMethod{ssh.PublicKeys(signer)},
+		// This config is restricted to connecting to localhost (and to a VM we manage),
+		// we rely on the kernel’s process isolation, not on cryptography,
+		// This would be UNACCEPTABLE for most other uses.
 		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
 	}, nil
 }
 
-func commonNativeSSH(username, identityPath, name string, sshPort int, inputArgs []string, stdin io.Reader) error {
+func localhostNativeSSH(username, identityPath, name string, sshPort int, inputArgs []string, stdin io.Reader) error {
 	sshDestination := username + "@localhost"
 	port := strconv.Itoa(sshPort)
 	interactive := true
 
-	args := append([]string{"-i", identityPath, "-p", port, sshDestination}, CommonSSHArgs()...)
+	args := append([]string{"-i", identityPath, "-p", port, sshDestination}, LocalhostSSHArgs()...) // WARNING: This MUST NOT be generalized to allow communication over untrusted networks.
 	if len(inputArgs) > 0 {
 		interactive = false
 		args = append(args, inputArgs...)
@@ -134,7 +142,13 @@ func commonNativeSSH(username, identityPath, name string, sshPort int, inputArgs
 	return cmd.Run()
 }
 
-func CommonSSHArgs() []string {
+// LocalhostSSHArgs returns OpenSSH command-line options for connecting with no host key identity checks.
+//
+// WARNING: This MUST NOT be used to communicate over untrusted networks.
+func LocalhostSSHArgs() []string {
+	// This config is restricted to connecting to localhost (and to a VM we manage),
+	// we rely on the kernel’s process isolation, not on cryptography,
+	// This would be UNACCEPTABLE for most other uses.
 	return []string{
 		"-o", "IdentitiesOnly=yes",
 		"-o", "StrictHostKeyChecking=no",

pkg/machine/update.go

@@ -14,13 +14,13 @@ func UpdatePodmanDockerSockService(mc *vmconfigs.MachineConfig) error {
 	content := ignition.GetPodmanDockerTmpConfig(mc.HostUser.UID, mc.HostUser.Rootful, false)
 	command := fmt.Sprintf("'echo %q > %s'", content, ignition.PodmanDockerTmpConfPath)
 	args := []string{"sudo", "bash", "-c", command}
-	if err := CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil {
+	if err := LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil {
 		logrus.Warnf("Could not update internal docker sock config")
 		return err
 	}
 
 	args = []string{"sudo", "systemd-tmpfiles", "--create", "--prefix=/run/docker.sock"}
-	if err := CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil {
+	if err := LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil {
 		logrus.Warnf("Could not create internal docker sock")
 		return err
 	}