podman ssh work, using new c/common interface

implement new ssh interface into podman

this completely redesigns the entire functionality of podman image scp,
podman system connection add, and podman --remote. All references to golang.org/x/crypto/ssh
have been moved to common as have native ssh/scp execs and the new usage of the sftp package.

this PR adds a global flag, --ssh to podman which has two valid inputs `golang` and `native` where golang is the default.
Users should not notice any difference in their everyday workflows if they continue using the golang option. UNLESS they have been using an improperly verified ssh key, this will now fail. This is because podman was incorrectly using the
ssh callback method to IGNORE the ssh known hosts file which is very insecure and golang tells you not yo use this in production.

The native paths allows for immense flexibility, with a new containers.conf field `SSH_CONFIG` that specifies a specific ssh config file to be used in all operations. Else the users ~/.ssh/config file will be used.
podman --remote currently only uses the golang path, given its deep interconnection with dialing multiple clients and urls.

My goal after this PR is to go back and abstract the idea of podman --remote from golang's dialed clients, as it should not be so intrinsically connected. Overall, this is a v1 of a long process of offering native ssh, and one that covers some good ground with podman system connection add and podman image scp.

Signed-off-by: Charlie Doern <cdoern@redhat.com>

1 files changed, 93 insertions, 215 deletions

diff --git a/pkg/domain/utils/scp.go b/pkg/domain/utils/scp.go
index 3c73cddd1..44a0d94d7 100644
--- a/pkg/domain/utils/scp.go
+++ b/pkg/domain/utils/scp.go
@@ -1,31 +1,24 @@
 package utils
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
-	"net"
 	"net/url"
 	"os"
 	"os/exec"
 	"os/user"
 	"strconv"
 	"strings"
-	"time"
-
-	scpD "github.com/dtylman/scp"
 
 	"github.com/containers/common/pkg/config"
+	"github.com/containers/common/pkg/ssh"
+	"github.com/containers/image/v5/transports/alltransports"
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/pkg/domain/entities"
-	"github.com/containers/podman/v4/pkg/terminal"
-	"github.com/docker/distribution/reference"
 	"github.com/sirupsen/logrus"
-	"golang.org/x/crypto/ssh"
-	"golang.org/x/crypto/ssh/agent"
 )
 
-func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool) (*entities.ImageLoadReport, *entities.ImageScpOptions, *entities.ImageScpOptions, []string, error) {
+func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool, sshMode ssh.EngineMode) (*entities.ImageLoadReport, *entities.ImageScpOptions, *entities.ImageScpOptions, []string, error) {
 	source := entities.ImageScpOptions{}
 	dest := entities.ImageScpOptions{}
 	sshInfo := entities.ImageScpConnections{}
@@ -46,10 +39,6 @@ func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool) (*entiti
 		return nil, nil, nil, nil, fmt.Errorf("could not make config: %w", err)
 	}
 
-	cfg, err := config.ReadCustomConfig() // get ready to set ssh destination if necessary
-	if err != nil {
-		return nil, nil, nil, nil, err
-	}
 	locations := []*entities.ImageScpOptions{}
 	cliConnections := []string{}
 	args := []string{src}
@@ -83,9 +72,7 @@ func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool) (*entiti
 	source.Quiet = quiet
 	source.File = f.Name() // after parsing the arguments, set the file for the save/load
 	dest.File = source.File
-	if err = os.Remove(source.File); err != nil { // remove the file and simply use its name so podman creates the file upon save. avoids umask errors
-		return nil, nil, nil, nil, err
-	}
+	defer os.Remove(source.File)
 
 	allLocal := true // if we are all localhost, do not validate connections but if we are using one localhost and one non we need to use sshd
 	for _, val := range cliConnections {
@@ -98,6 +85,10 @@ func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool) (*entiti
 		cliConnections = []string{}
 	}
 
+	cfg, err := config.ReadCustomConfig() // get ready to set ssh destination if necessary
+	if err != nil {
+		return nil, nil, nil, nil, err
+	}
 	var serv map[string]config.Destination
 	serv, err = GetServiceInformation(&sshInfo, cliConnections, cfg)
 	if err != nil {
@@ -109,12 +100,12 @@ func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool) (*entiti
 
 	switch {
 	case source.Remote: // if we want to load FROM the remote, dest can either be local or remote in this case
-		err = SaveToRemote(source.Image, source.File, "", sshInfo.URI[0], sshInfo.Identities[0])
+		err = SaveToRemote(source.Image, source.File, "", sshInfo.URI[0], sshInfo.Identities[0], sshMode)
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
 		if dest.Remote { // we want to load remote -> remote, both source and dest are remote
-			rep, id, err := LoadToRemote(dest, dest.File, "", sshInfo.URI[1], sshInfo.Identities[1])
+			rep, id, err := LoadToRemote(dest, dest.File, "", sshInfo.URI[1], sshInfo.Identities[1], sshMode)
 			if err != nil {
 				return nil, nil, nil, nil, err
 			}
@@ -138,7 +129,8 @@ func ExecuteTransfer(src, dst string, parentFlags []string, quiet bool) (*entiti
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
-		rep, id, err := LoadToRemote(dest, source.File, "", sshInfo.URI[0], sshInfo.Identities[0])
+
+		rep, id, err := LoadToRemote(dest, source.File, "", sshInfo.URI[0], sshInfo.Identities[0], sshMode)
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
@@ -220,34 +212,37 @@ func LoginUser(user string) (*exec.Cmd, error) {
 // loadToRemote takes image and remote connection information. it connects to the specified client
 // and copies the saved image dir over to the remote host and then loads it onto the machine
 // returns a string containing output or an error
-func LoadToRemote(dest entities.ImageScpOptions, localFile string, tag string, url *url.URL, iden string) (string, string, error) {
-	dial, remoteFile, err := CreateConnection(url, iden)
+func LoadToRemote(dest entities.ImageScpOptions, localFile string, tag string, url *url.URL, iden string, sshEngine ssh.EngineMode) (string, string, error) {
+	port, err := strconv.Atoi(url.Port())
 	if err != nil {
 		return "", "", err
 	}
-	defer dial.Close()
 
-	n, err := scpD.CopyTo(dial, localFile, remoteFile)
+	remoteFile, err := ssh.Exec(&ssh.ConnectionExecOptions{Host: url.String(), Port: port, User: url.User, Args: []string{"mktemp"}}, sshEngine)
 	if err != nil {
-		errOut := strconv.Itoa(int(n)) + " Bytes copied before error"
-		return " ", "", fmt.Errorf("%v: %w", errOut, err)
+		return "", "", err
 	}
-	var run string
-	if tag != "" {
-		return "", "", fmt.Errorf("renaming of an image is currently not supported: %w", define.ErrInvalidArg)
+
+	opts := ssh.ConnectionScpOptions{User: url.User, Identity: iden, Port: port, Source: localFile, Destination: "ssh://" + url.User.String() + "@" + url.Hostname() + ":" + remoteFile}
+	scpRep, err := ssh.Scp(&opts, sshEngine)
+	if err != nil {
+		return "", "", err
 	}
-	podman := os.Args[0]
-	run = podman + " image load --input=" + remoteFile + ";rm " + remoteFile // run ssh image load of the file copied via scp
-	out, err := ExecRemoteCommand(dial, run)
+	out, err := ssh.Exec(&ssh.ConnectionExecOptions{Host: url.String(), Port: port, User: url.User, Args: []string{"podman", "image", "load", "--input=" + scpRep + ";", "rm", scpRep}}, sshEngine)
 	if err != nil {
 		return "", "", err
 	}
-	rep := strings.TrimSuffix(string(out), "\n")
+	if tag != "" {
+		return "", "", fmt.Errorf("renaming of an image is currently not supported: %w", define.ErrInvalidArg)
+	}
+	rep := strings.TrimSuffix(out, "\n")
 	outArr := strings.Split(rep, " ")
 	id := outArr[len(outArr)-1]
 	if len(dest.Tag) > 0 { // tag the remote image using the output ID
-		run = podman + " tag " + id + " " + dest.Tag
-		_, err = ExecRemoteCommand(dial, run)
+		_, err := ssh.Exec(&ssh.ConnectionExecOptions{Host: url.Hostname(), Port: port, User: url.User, Args: []string{"podman", "image", "tag", id, dest.Tag}}, sshEngine)
+		if err != nil {
+			return "", "", err
+		}
 		if err != nil {
 			return "", "", err
 		}
@@ -258,94 +253,37 @@ func LoadToRemote(dest entities.ImageScpOptions, localFile string, tag string, u
 // saveToRemote takes image information and remote connection information. it connects to the specified client
 // and saves the specified image on the remote machine and then copies it to the specified local location
 // returns an error if one occurs.
-func SaveToRemote(image, localFile string, tag string, uri *url.URL, iden string) error {
-	dial, remoteFile, err := CreateConnection(uri, iden)
-
-	if err != nil {
-		return err
-	}
-	defer dial.Close()
-
+func SaveToRemote(image, localFile string, tag string, uri *url.URL, iden string, sshEngine ssh.EngineMode) error {
 	if tag != "" {
 		return fmt.Errorf("renaming of an image is currently not supported: %w", define.ErrInvalidArg)
 	}
-	podman := os.Args[0]
-	run := podman + " image save " + image + " --format=oci-archive --output=" + remoteFile // run ssh image load of the file copied via scp. Files are reverse in this case...
-	_, err = ExecRemoteCommand(dial, run)
+
+	port, err := strconv.Atoi(uri.Port())
 	if err != nil {
 		return err
 	}
-	n, err := scpD.CopyFrom(dial, remoteFile, localFile)
-	if _, conErr := ExecRemoteCommand(dial, "rm "+remoteFile); conErr != nil {
-		logrus.Errorf("Removing file on endpoint: %v", conErr)
-	}
-	if err != nil {
-		errOut := strconv.Itoa(int(n)) + " Bytes copied before error"
-		return fmt.Errorf("%v: %w", errOut, err)
-	}
-	return nil
-}
 
-// makeRemoteFile creates the necessary remote file on the host to
-// save or load the image to. returns a string with the file name or an error
-func MakeRemoteFile(dial *ssh.Client) (string, error) {
-	run := "mktemp"
-	remoteFile, err := ExecRemoteCommand(dial, run)
+	remoteFile, err := ssh.Exec(&ssh.ConnectionExecOptions{Host: uri.String(), Port: port, User: uri.User, Args: []string{"mktemp"}}, sshEngine)
 	if err != nil {
-		return "", err
+		return err
 	}
-	return strings.TrimSuffix(string(remoteFile), "\n"), nil
-}
 
-// createConnections takes a boolean determining which ssh client to dial
-// and returns the dials client, its newly opened remote file, and an error if applicable.
-func CreateConnection(url *url.URL, iden string) (*ssh.Client, string, error) {
-	cfg, err := ValidateAndConfigure(url, iden)
+	_, err = ssh.Exec(&ssh.ConnectionExecOptions{Host: uri.String(), Port: port, User: uri.User, Args: []string{"podman", "image", "save", image, "--format", "oci-archive", "--output", remoteFile}}, sshEngine)
 	if err != nil {
-		return nil, "", err
+		return err
 	}
-	dialAdd, err := ssh.Dial("tcp", url.Host, cfg) // dial the client
+
+	opts := ssh.ConnectionScpOptions{User: uri.User, Identity: iden, Port: port, Source: "ssh://" + uri.User.String() + "@" + uri.Hostname() + ":" + remoteFile, Destination: localFile}
+	scpRep, err := ssh.Scp(&opts, sshEngine)
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to connect: %w", err)
+		return err
 	}
-	file, err := MakeRemoteFile(dialAdd)
+	_, err = ssh.Exec(&ssh.ConnectionExecOptions{Host: uri.String(), Port: port, User: uri.User, Args: []string{"rm", scpRep}}, sshEngine)
 	if err != nil {
-		return nil, "", err
+		logrus.Errorf("Removing file on endpoint: %v", err)
 	}
 
-	return dialAdd, file, nil
-}
-
-// GetSerivceInformation takes the parsed list of hosts to connect to and validates the information
-func GetServiceInformation(sshInfo *entities.ImageScpConnections, cliConnections []string, cfg *config.Config) (map[string]config.Destination, error) {
-	var serv map[string]config.Destination
-	var urlS string
-	var iden string
-	for i, val := range cliConnections {
-		splitEnv := strings.SplitN(val, "::", 2)
-		sshInfo.Connections = append(sshInfo.Connections, splitEnv[0])
-		conn, found := cfg.Engine.ServiceDestinations[sshInfo.Connections[i]]
-		if found {
-			urlS = conn.URI
-			iden = conn.Identity
-		} else { // no match, warn user and do a manual connection.
-			urlS = "ssh://" + sshInfo.Connections[i]
-			iden = ""
-			logrus.Warnf("Unknown connection name given. Please use system connection add to specify the default remote socket location")
-		}
-		urlFinal, err := url.Parse(urlS) // create an actual url to pass to exec command
-		if err != nil {
-			return nil, err
-		}
-		if urlFinal.User.Username() == "" {
-			if urlFinal.User, err = GetUserInfo(urlFinal); err != nil {
-				return nil, err
-			}
-		}
-		sshInfo.URI = append(sshInfo.URI, urlFinal)
-		sshInfo.Identities = append(sshInfo.Identities, iden)
-	}
-	return serv, nil
+	return nil
 }
 
 // execPodman executes the podman save/load command given the podman binary
@@ -413,18 +351,32 @@ func ParseImageSCPArg(arg string) (*entities.ImageScpOptions, []string, error) {
 	return &location, cliConnections, nil
 }
 
-// validateImagePortion is a helper function to validate the image name in an SCP argument
 func ValidateImagePortion(location entities.ImageScpOptions, arg string) (entities.ImageScpOptions, error) {
 	if RemoteArgLength(arg, 1) > 0 {
-		err := ValidateImageName(strings.Split(arg, "::")[1])
-		if err != nil {
-			return location, err
-		}
-		location.Image = strings.Split(arg, "::")[1] // this will get checked/set again once we validate connections
+		before := strings.Split(arg, "::")[1]
+		name := ValidateImageName(before)
+		if before != name {
+			location.Image = name
+		} else {
+			location.Image = before
+		} // this will get checked/set again once we validate connections
 	}
 	return location, nil
 }
 
+// validateImageName makes sure that the image given is valid and no injections are occurring
+// we simply use this for error checking, bot setting the image
+func ValidateImageName(input string) string {
+	// ParseNormalizedNamed transforms a shortname image into its
+	// full name reference so busybox => docker.io/library/busybox
+	// we want to keep our shortnames, so only return an error if
+	// we cannot parse what the user has given us
+	if ref, err := alltransports.ParseImageName(input); err == nil {
+		return ref.Transport().Name()
+	}
+	return input
+}
+
 // validateSCPArgs takes the array of source and destination options and checks for common errors
 func ValidateSCPArgs(locations []*entities.ImageScpOptions) error {
 	if len(locations) > 2 {
@@ -440,17 +392,6 @@ func ValidateSCPArgs(locations []*entities.ImageScpOptions) error {
 	return nil
 }
 
-// validateImageName makes sure that the image given is valid and no injections are occurring
-// we simply use this for error checking, bot setting the image
-func ValidateImageName(input string) error {
-	// ParseNormalizedNamed transforms a shortname image into its
-	// full name reference so busybox => docker.io/library/busybox
-	// we want to keep our shortnames, so only return an error if
-	// we cannot parse what the user has given us
-	_, err := reference.ParseNormalizedNamed(input)
-	return err
-}
-
 // remoteArgLength is a helper function to simplify the extracting of host argument data
 // returns an int which contains the length of a specified index in a host::image string
 func RemoteArgLength(input string, side int) int {
@@ -460,23 +401,36 @@ func RemoteArgLength(input string, side int) int {
 	return -1
 }
 
-// ExecRemoteCommand takes a ssh client connection and a command to run and executes the
-// command on the specified client. The function returns the Stdout from the client or the Stderr
-func ExecRemoteCommand(dial *ssh.Client, run string) ([]byte, error) {
-	sess, err := dial.NewSession() // new ssh client session
-	if err != nil {
-		return nil, err
-	}
-	defer sess.Close()
-
-	var buffer bytes.Buffer
-	var bufferErr bytes.Buffer
-	sess.Stdout = &buffer                 // output from client funneled into buffer
-	sess.Stderr = &bufferErr              // err form client funneled into buffer
-	if err := sess.Run(run); err != nil { // run the command on the ssh client
-		return nil, fmt.Errorf("%v: %w", bufferErr.String(), err)
+// GetSerivceInformation takes the parsed list of hosts to connect to and validates the information
+func GetServiceInformation(sshInfo *entities.ImageScpConnections, cliConnections []string, cfg *config.Config) (map[string]config.Destination, error) {
+	var serv map[string]config.Destination
+	var urlS string
+	var iden string
+	for i, val := range cliConnections {
+		splitEnv := strings.SplitN(val, "::", 2)
+		sshInfo.Connections = append(sshInfo.Connections, splitEnv[0])
+		conn, found := cfg.Engine.ServiceDestinations[sshInfo.Connections[i]]
+		if found {
+			urlS = conn.URI
+			iden = conn.Identity
+		} else { // no match, warn user and do a manual connection.
+			urlS = "ssh://" + sshInfo.Connections[i]
+			iden = ""
+			logrus.Warnf("Unknown connection name given. Please use system connection add to specify the default remote socket location")
+		}
+		urlFinal, err := url.Parse(urlS) // create an actual url to pass to exec command
+		if err != nil {
+			return nil, err
+		}
+		if urlFinal.User.Username() == "" {
+			if urlFinal.User, err = GetUserInfo(urlFinal); err != nil {
+				return nil, err
+			}
+		}
+		sshInfo.URI = append(sshInfo.URI, urlFinal)
+		sshInfo.Identities = append(sshInfo.Identities, iden)
 	}
-	return buffer.Bytes(), nil
+	return serv, nil
 }
 
 func GetUserInfo(uri *url.URL) (*url.Userinfo, error) {
@@ -502,79 +456,3 @@ func GetUserInfo(uri *url.URL) (*url.Userinfo, error) {
 	}
 	return url.User(usr.Username), nil
 }
-
-// ValidateAndConfigure will take a ssh url and an identity key (rsa and the like) and ensure the information given is valid
-// iden iden can be blank to mean no identity key
-// once the function validates the information it creates and returns an ssh.ClientConfig.
-func ValidateAndConfigure(uri *url.URL, iden string) (*ssh.ClientConfig, error) {
-	var signers []ssh.Signer
-	passwd, passwdSet := uri.User.Password()
-	if iden != "" { // iden might be blank if coming from image scp or if no validation is needed
-		value := iden
-		s, err := terminal.PublicKey(value, []byte(passwd))
-		if err != nil {
-			return nil, fmt.Errorf("failed to read identity %q: %w", value, err)
-		}
-		signers = append(signers, s)
-		logrus.Debugf("SSH Ident Key %q %s %s", value, ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
-	}
-	if sock, found := os.LookupEnv("SSH_AUTH_SOCK"); found { // validate ssh information, specifically the unix file socket used by the ssh agent.
-		logrus.Debugf("Found SSH_AUTH_SOCK %q, ssh-agent signer enabled", sock)
-
-		c, err := net.Dial("unix", sock)
-		if err != nil {
-			return nil, err
-		}
-		agentSigners, err := agent.NewClient(c).Signers()
-		if err != nil {
-			return nil, err
-		}
-
-		signers = append(signers, agentSigners...)
-
-		if logrus.IsLevelEnabled(logrus.DebugLevel) {
-			for _, s := range agentSigners {
-				logrus.Debugf("SSH Agent Key %s %s", ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
-			}
-		}
-	}
-	var authMethods []ssh.AuthMethod // now we validate and check for the authorization methods, most notaibly public key authorization
-	if len(signers) > 0 {
-		var dedup = make(map[string]ssh.Signer)
-		for _, s := range signers {
-			fp := ssh.FingerprintSHA256(s.PublicKey())
-			if _, found := dedup[fp]; found {
-				logrus.Debugf("Dedup SSH Key %s %s", ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
-			}
-			dedup[fp] = s
-		}
-
-		var uniq []ssh.Signer
-		for _, s := range dedup {
-			uniq = append(uniq, s)
-		}
-		authMethods = append(authMethods, ssh.PublicKeysCallback(func() ([]ssh.Signer, error) {
-			return uniq, nil
-		}))
-	}
-	if passwdSet { // if password authentication is given and valid, add to the list
-		authMethods = append(authMethods, ssh.Password(passwd))
-	}
-	if len(authMethods) == 0 {
-		authMethods = append(authMethods, ssh.PasswordCallback(func() (string, error) {
-			pass, err := terminal.ReadPassword(fmt.Sprintf("%s's login password:", uri.User.Username()))
-			return string(pass), err
-		}))
-	}
-	tick, err := time.ParseDuration("40s")
-	if err != nil {
-		return nil, err
-	}
-	cfg := &ssh.ClientConfig{
-		User:            uri.User.Username(),
-		Auth:            authMethods,
-		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-		Timeout:         tick,
-	}
-	return cfg, nil
-}