diff options
-rw-r--r-- | cmd/podman/images/scp.go | 19 | ||||
-rw-r--r-- | pkg/domain/infra/abi/images.go | 69 | ||||
-rw-r--r-- | test/system/120-load.bats | 36 | ||||
-rw-r--r-- | utils/utils.go | 15 |
4 files changed, 64 insertions, 75 deletions
diff --git a/cmd/podman/images/scp.go b/cmd/podman/images/scp.go index 1481e71c7..81dcda123 100644 --- a/cmd/podman/images/scp.go +++ b/cmd/podman/images/scp.go @@ -17,7 +17,6 @@ import ( "github.com/containers/podman/v4/cmd/podman/system/connection" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/pkg/domain/entities" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/utils" scpD "github.com/dtylman/scp" "github.com/pkg/errors" @@ -337,21 +336,9 @@ func GetServiceInformation(cliConnections []string, cfg *config.Config) (map[str // execPodman executes the podman save/load command given the podman binary func execPodman(podman string, command []string) error { - if rootless.IsRootless() { - cmd := exec.Command(podman) - utils.CreateSCPCommand(cmd, command[1:]) - logrus.Debug("Executing podman command") - return cmd.Run() - } - machinectl, err := exec.LookPath("machinectl") - if err != nil { - cmd := exec.Command("su", "-l", "root", "--command") - cmd = utils.CreateSCPCommand(cmd, []string{strings.Join(command, " ")}) - return cmd.Run() - } - cmd := exec.Command(machinectl, "shell", "-q", "root@.host") - cmd = utils.CreateSCPCommand(cmd, command) - logrus.Debug("Executing load command machinectl") + cmd := exec.Command(podman) + utils.CreateSCPCommand(cmd, command[1:]) + logrus.Debugf("Executing podman command: %q", cmd) return cmd.Run() } diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index b9c6d3ac7..0b1281aac 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strconv" "strings" + "syscall" "github.com/containers/common/libimage" "github.com/containers/common/pkg/config" @@ -782,7 +783,7 @@ func transferRootless(source entities.ImageScpOptions, dest entities.ImageScpOpt return cmdLoad.Run() } -// TransferRootful creates new podman processes using exec.Command and su/machinectl, transferring images between the given source and destination users +// TransferRootful creates new podman processes using exec.Command and a new uid/gid alongside a cleared environment func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOptions, podman string, parentFlags []string) error { basicCommand := []string{podman} basicCommand = append(basicCommand, parentFlags...) @@ -794,12 +795,9 @@ func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOpti } saveCommand = append(saveCommand, []string{"--output", source.File, source.Image}...) loadCommand = append(loadCommand, []string{"--input", dest.File}...) - save := []string{strings.Join(saveCommand, " ")} - load := []string{strings.Join(loadCommand, " ")} - // if executing using sudo or transferring between two users, the TransferRootless approach will not work, default to using machinectl or su as necessary. - // the approach using sudo is preferable and more straightforward. There is no reason for using sudo in these situations - // since the feature is meant to transfer from root to rootless an vice versa without explicit sudo evocaiton. + // if executing using sudo or transferring between two users, the TransferRootless approach will not work, the new process needs to be set up + // with the proper uid and gid as well as environmental variables. var uSave *user.User var uLoad *user.User var err error @@ -830,20 +828,11 @@ func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOpti return err } } - machinectl, err := exec.LookPath("machinectl") - if err != nil { - logrus.Warn("defaulting to su since machinectl is not available, su will fail if no user session is available") - err = execSu(uSave, save) - if err != nil { - return err - } - return execSu(uLoad, load) - } - err = execMachine(uSave, saveCommand, machinectl) + err = execPodman(uSave, saveCommand) if err != nil { return err } - return execMachine(uLoad, loadCommand, machinectl) + return execPodman(uLoad, loadCommand) } func lookupUser(u string) (*user.User, error) { @@ -853,21 +842,37 @@ func lookupUser(u string) (*user.User, error) { return user.Lookup(u) } -func execSu(execUser *user.User, command []string) error { - cmd := exec.Command("su", "-l", execUser.Username, "--command") - cmd = utils.CreateSCPCommand(cmd, command) - logrus.Debugf("Executing via su: %q", cmd) - return cmd.Run() -} - -func execMachine(execUser *user.User, command []string, machinectl string) error { - verb := machinectl - args := []string{"shell", "-q", execUser.Username + "@.host"} - if execUser.Uid == "0" { - args = append([]string{verb}, args...) - verb = "sudo" +func execPodman(execUser *user.User, command []string) error { + cmdLogin, err := utils.LoginUser(execUser.Username) + if err != nil { + return err + } + defer func() error { + err := cmdLogin.Process.Kill() + if err != nil { + return err + } + return cmdLogin.Wait() + }() + cmd := exec.Command(command[0], command[1:]...) + cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "TERM=" + os.Getenv("TERM")} + cmd.Stderr = os.Stderr + cmd.Stdout = os.Stdout + uid, err := strconv.ParseInt(execUser.Uid, 10, 32) + if err != nil { + return err + } + gid, err := strconv.ParseInt(execUser.Gid, 10, 32) + if err != nil { + return err + } + cmd.SysProcAttr = &syscall.SysProcAttr{ + Credential: &syscall.Credential{ + Uid: uint32(uid), + Gid: uint32(gid), + Groups: nil, + NoSetGroups: false, + }, } - cmd := utils.CreateSCPCommand(exec.Command(verb, args...), command) - logrus.Debugf("Executing via machinectl: %q", cmd) return cmd.Run() } diff --git a/test/system/120-load.bats b/test/system/120-load.bats index f5ba93d8a..8bf785081 100644 --- a/test/system/120-load.bats +++ b/test/system/120-load.bats @@ -104,10 +104,6 @@ verify_iid_and_name() { # If we can't sudo, we can't test. _sudo true || skip "cannot sudo to $notme" - # FIXME FIXME FIXME: it'd be reeeeeeally nice if we could pass --root - # to the non-self user, hence avoid vandalizing - # their storage. - # Preserve digest of original image; we will compare against it later run_podman image inspect --format '{{.Digest}}' $IMAGE src_digest=$output @@ -117,12 +113,8 @@ verify_iid_and_name() { run_podman tag $IMAGE $newname # Copy it there. - # FIXME: the first '.*' in the expect string below is unfortunate; it's - # a workaround for Ubuntu which gripes: - # "warning.*defaulting to su since machinectl is not available" - # Reexamine this once #12829 is fixed run_podman image scp $newname ${notme}@localhost:: - is "$output" ".*Copying blob .*Copying config.*Writing manifest.*Storing signatures" + is "$output" "Copying blob .*Copying config.*Writing manifest.*Storing signatures" # confirm that image was copied. FIXME: also try $PODMAN image inspect? _sudo $PODMAN image exists $newname @@ -132,13 +124,6 @@ verify_iid_and_name() { run_podman image scp -q ${notme}@localhost::$newname expect="Loaded image(s): $newname" - # FIXME FIXME FIXME: ubuntu has no machinectl, emits useless warning message instead - if ! is_rootless; then - # FIXME: root on fedora uses machinectl, which emits useless \n and \r (#12829) - NL=$'\n' - CR=$'\r' - expect="$NL$expect$CR" - fi is "$output" "$expect" "-q silences output" # Confirm that we have it, and that its digest matches our original @@ -150,17 +135,14 @@ verify_iid_and_name() { run_podman untag $IMAGE $newname # Negative test for nonexistent image. - # FIXME FIXME: cannot test on root, because it uses machinectl (#12829) - if is_rootless; then - # FIXME: error message is 2 lines, the 2nd being "exit status 125". - # FIXME: is that fixable, or do we have to live with it? - nope="nope.nope/nonesuch:notag" - run_podman 125 image scp ${notme}@localhost::$nope - is "$output" "Error: $nope: image not known.*" "Pulling nonexistent image" - - run_podman 125 image scp $nope ${notme}@localhost:: - is "$output" "Error: $nope: image not known.*" "Pushing nonexistent image" - fi + # FIXME: error message is 2 lines, the 2nd being "exit status 125". + # FIXME: is that fixable, or do we have to live with it? + nope="nope.nope/nonesuch:notag" + run_podman 125 image scp ${notme}@localhost::$nope + is "$output" "Error: $nope: image not known.*" "Pulling nonexistent image" + + run_podman 125 image scp $nope ${notme}@localhost:: + is "$output" "Error: $nope: image not known.*" "Pushing nonexistent image" # Negative test for copying to a different name run_podman 125 image scp $IMAGE ${notme}@localhost::newname:newtag diff --git a/utils/utils.go b/utils/utils.go index caf63c975..52586b937 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -238,3 +238,18 @@ func CreateSCPCommand(cmd *exec.Cmd, command []string) *exec.Cmd { cmd.Stdout = os.Stdout return cmd } + +// LoginUser starts the user process on the host so that image scp can use systemd-run +func LoginUser(user string) (*exec.Cmd, error) { + sleep, err := exec.LookPath("sleep") + if err != nil { + return nil, err + } + machinectl, err := exec.LookPath("machinectl") + if err != nil { + return nil, err + } + cmd := exec.Command(machinectl, "shell", "-q", user+"@.host", sleep, "inf") + err = cmd.Start() + return cmd, err +} |