From ba72021274e1c555d444c0d866ae6d9548fc7129 Mon Sep 17 00:00:00 2001 From: localghost Date: Mon, 2 Oct 2017 22:03:42 +0200 Subject: [PATCH] Fix owner of files uploaded to docker container run as non-root. --- builder/docker/communicator.go | 88 ++++++++++++++++++++-- builder/docker/communicator_test.go | 113 ++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 6 deletions(-) diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index 13684b2f8..bbfdb551f 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -18,12 +18,13 @@ import ( ) type Communicator struct { - ContainerID string - HostDir string - ContainerDir string - Version *version.Version - Config *Config - lock sync.Mutex + ContainerID string + HostDir string + ContainerDir string + Version *version.Version + Config *Config + containerUser *string + lock sync.Mutex } func (c *Communicator) Start(remote *packer.RemoteCmd) error { @@ -154,6 +155,10 @@ func (c *Communicator) uploadFile(dst string, src io.Reader, fi *os.FileInfo) er return fmt.Errorf("Failed to upload to '%s' in container: %s. %s.", dst, stderrOut, err) } + if err := c.fixDestinationOwner(dst); err != nil { + return err + } + return nil } @@ -207,6 +212,10 @@ func (c *Communicator) UploadDir(dst string, src string, exclude []string) error return fmt.Errorf("Failed to upload to '%s' in container: %s. %s.", dst, stderrOut, err) } + if err := c.fixDestinationOwner(dst); err != nil { + return err + } + return nil } @@ -310,3 +319,70 @@ func (c *Communicator) run(cmd *exec.Cmd, remote *packer.RemoteCmd, stdin io.Wri // Set the exit status which triggers waiters remote.SetExited(exitStatus) } + +// TODO Workaround for #5307. Remove once #5409 is fixed. +func (c *Communicator) fixDestinationOwner(destination string) error { + if c.containerUser == nil { + containerUser, err := c.discoverContainerUser() + if err != nil { + return err + } + c.containerUser = &containerUser + } + + if *c.containerUser != "" { + chownArgs := []string{ + "docker", "exec", "--user", "root", c.ContainerID, "/bin/sh", "-c", + fmt.Sprintf("chown -R %s %s", *c.containerUser, destination), + } + if _, err := c.runLocalCommand(chownArgs[0], chownArgs[1:]...); err != nil { + return fmt.Errorf("Failed to set owner of the uploaded file: %s", err) + } + } + + return nil +} + +func (c *Communicator) discoverContainerUser() (string, error) { + var err error + var stdout []byte + inspectArgs := []string{"docker", "inspect", "--format", "{{.Config.User}}", c.ContainerID} + if stdout, err = c.runLocalCommand(inspectArgs[0], inspectArgs[1:]...); err != nil { + return "", fmt.Errorf("Failed to inspect the container: %s", err) + } + return strings.TrimSpace(string(stdout)), nil +} + +func (c *Communicator) runLocalCommand(name string, arg ...string) (stdout []byte, err error) { + localCmd := exec.Command(name, arg...) + + stdoutP, err := localCmd.StdoutPipe() + if err != nil { + return nil, fmt.Errorf("failed to open stdout pipe, %s", err) + } + + stderrP, err := localCmd.StderrPipe() + if err != nil { + return nil, fmt.Errorf("failed to open stderr pipe, %s", err) + } + + if err = localCmd.Start(); err != nil { + return nil, fmt.Errorf("failed to start command, %s", err) + } + + stdout, err = ioutil.ReadAll(stdoutP) + if err != nil { + return nil, fmt.Errorf("failed to read from stdout pipe, %s", err) + } + + stderr, err := ioutil.ReadAll(stderrP) + if err != nil { + return nil, fmt.Errorf("failed to read from stderr pipe, %s", err) + } + + if err := localCmd.Wait(); err != nil { + return nil, fmt.Errorf("%s, %s", stderr, err) + } + + return stdout, nil +} diff --git a/builder/docker/communicator_test.go b/builder/docker/communicator_test.go index 0c98dbbb2..b5b5b1583 100644 --- a/builder/docker/communicator_test.go +++ b/builder/docker/communicator_test.go @@ -209,6 +209,82 @@ func TestLargeDownload(t *testing.T) { } +// TestUploadOwner verifies that owner of uploaded files is the user the container is running as. +func TestUploadOwner(t *testing.T) { + ui := packer.TestUi(t) + cache := &packer.FileCache{CacheDir: os.TempDir()} + + tpl, err := template.Parse(strings.NewReader(testUploadOwnerTemplate)) + if err != nil { + t.Fatalf("Unable to parse config: %s", err) + } + + if os.Getenv("PACKER_ACC") == "" { + t.Skip("This test is only run with PACKER_ACC=1") + } + cmd := exec.Command("docker", "-v") + cmd.Run() + if !cmd.ProcessState.Success() { + t.Error("docker command not found; please make sure docker is installed") + } + + // Setup the builder + builder := &Builder{} + warnings, err := builder.Prepare(tpl.Builders["docker"].Config) + if err != nil { + t.Fatalf("Error preparing configuration %s", err) + } + if len(warnings) > 0 { + t.Fatal("Encountered configuration warnings; aborting") + } + + // Setup the provisioners + fileProvisioner := &file.Provisioner{} + err = fileProvisioner.Prepare(tpl.Provisioners[0].Config) + if err != nil { + t.Fatalf("Error preparing single file upload provisioner: %s", err) + } + + dirProvisioner := &file.Provisioner{} + err = dirProvisioner.Prepare(tpl.Provisioners[1].Config) + if err != nil { + t.Fatalf("Error preparing directory upload provisioner: %s", err) + } + + shellProvisioner := &shell.Provisioner{} + err = shellProvisioner.Prepare(tpl.Provisioners[2].Config) + if err != nil { + t.Fatalf("Error preparing shell provisioner: %s", err) + } + + verifyProvisioner := &shell.Provisioner{} + err = verifyProvisioner.Prepare(tpl.Provisioners[3].Config) + if err != nil { + t.Fatalf("Error preparing verification provisioner: %s", err) + } + + // Add hooks so the provisioners run during the build + hooks := map[string][]packer.Hook{} + hooks[packer.HookProvision] = []packer.Hook{ + &packer.ProvisionHook{ + Provisioners: []packer.Provisioner{ + fileProvisioner, + dirProvisioner, + shellProvisioner, + verifyProvisioner, + }, + ProvisionerTypes: []string{"", "", "", ""}, + }, + } + hook := &packer.DispatchHook{Mapping: hooks} + + artifact, err := builder.Run(ui, hook, cache) + if err != nil { + t.Fatalf("Error running build %s", err) + } + defer artifact.Destroy() +} + const dockerBuilderConfig = ` { "builders": [ @@ -269,3 +345,40 @@ const dockerLargeBuilderConfig = ` ] } ` + +const testUploadOwnerTemplate = ` +{ + "builders": [ + { + "type": "docker", + "image": "ubuntu", + "discard": true, + "run_command": ["-d", "-i", "-t", "-u", "42", "{{.Image}}", "/bin/sh"] + } + ], + "provisioners": [ + { + "type": "file", + "source": "test-fixtures/onecakes/strawberry", + "destination": "/tmp/strawberry-cake" + }, + { + "type": "file", + "source": "test-fixtures/manycakes", + "destination": "/tmp/" + }, + { + "type": "shell", + "inline": "touch /tmp/testUploadOwner" + }, + { + "type": "shell", + "inline": [ + "[ $(stat -c %u /tmp/strawberry-cake) -eq 42 ] || (echo 'Invalid owner of /tmp/strawberry-cake' && exit 1)", + "[ $(stat -c %u /tmp/testUploadOwner) -eq 42 ] || (echo 'Invalid owner of /tmp/testUploadOwner' && exit 1)", + "find /tmp/manycakes | xargs -n1 -IFILE /bin/sh -c '[ $(stat -c %u FILE) -eq 42 ] || (echo \"Invalid owner of FILE\" && exit 1)'" + ] + } + ] +} +`