Only chown network files within container metadata

If the user specifies a mountpath from the host, we should not be
attempting to chown files outside the daemon's metadata directory
(represented by `daemon.repository` at init time).

This forces users who want to use user namespaces to handle the
ownership needs of any external files mounted as network files
(/etc/resolv.conf, /etc/hosts, /etc/hostname) separately from the
daemon. In all other volume/bind mount situations we have taken this
same line--we don't chown host file content.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
This commit is contained in:
Phil Estes 2017-07-22 22:11:09 -04:00
parent 3ab20a87fa
commit 42716dcf5c
No known key found for this signature in database
GPG Key ID: 0F386284C03A1162
3 changed files with 99 additions and 2 deletions

View File

@ -86,8 +86,13 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er
// remapped root (user namespaces)
rootIDs := daemon.idMappings.RootPair()
for _, mount := range netMounts {
if err := os.Chown(mount.Source, rootIDs.UID, rootIDs.GID); err != nil {
return nil, err
// we should only modify ownership of network files within our own container
// metadata repository. If the user specifies a mount path external, it is
// up to the user to make sure the file has proper ownership for userns
if strings.Index(mount.Source, daemon.repository) == 0 {
if err := os.Chown(mount.Source, rootIDs.UID, rootIDs.GID); err != nil {
return nil, err
}
}
}
return append(mounts, netMounts...), nil

View File

@ -0,0 +1,77 @@
// +build !windows
package main
import (
"io/ioutil"
"os"
"path/filepath"
"github.com/docker/docker/api/types"
containertypes "github.com/docker/docker/api/types/container"
mounttypes "github.com/docker/docker/api/types/mount"
networktypes "github.com/docker/docker/api/types/network"
"github.com/docker/docker/client"
"github.com/docker/docker/integration-cli/checker"
"github.com/docker/docker/pkg/ioutils"
"github.com/docker/docker/pkg/system"
"github.com/go-check/check"
"github.com/stretchr/testify/assert"
"golang.org/x/net/context"
)
func (s *DockerSuite) TestContainersAPINetworkMountsNoChown(c *check.C) {
// chown only applies to Linux bind mounted volumes; must be same host to verify
testRequires(c, DaemonIsLinux, SameHostDaemon)
tmpDir, err := ioutils.TempDir("", "test-network-mounts")
c.Assert(err, checker.IsNil)
defer os.RemoveAll(tmpDir)
// make tmp dir readable by anyone to allow userns process to mount from
err = os.Chmod(tmpDir, 0755)
c.Assert(err, checker.IsNil)
// create temp files to use as network mounts
tmpNWFileMount := filepath.Join(tmpDir, "nwfile")
err = ioutil.WriteFile(tmpNWFileMount, []byte("network file bind mount"), 0644)
c.Assert(err, checker.IsNil)
config := containertypes.Config{
Image: "busybox",
}
hostConfig := containertypes.HostConfig{
Mounts: []mounttypes.Mount{
{
Type: "bind",
Source: tmpNWFileMount,
Target: "/etc/resolv.conf",
},
{
Type: "bind",
Source: tmpNWFileMount,
Target: "/etc/hostname",
},
{
Type: "bind",
Source: tmpNWFileMount,
Target: "/etc/hosts",
},
},
}
cli, err := client.NewEnvClient()
c.Assert(err, checker.IsNil)
defer cli.Close()
ctrCreate, err := cli.ContainerCreate(context.Background(), &config, &hostConfig, &networktypes.NetworkingConfig{}, "")
c.Assert(err, checker.IsNil)
// container will exit immediately because of no tty, but we only need the start sequence to test the condition
err = cli.ContainerStart(context.Background(), ctrCreate.ID, types.ContainerStartOptions{})
c.Assert(err, checker.IsNil)
// check that host-located bind mount network file did not change ownership when the container was started
statT, err := system.Stat(tmpNWFileMount)
c.Assert(err, checker.IsNil)
assert.Equal(c, uint32(0), statT.UID(), "bind mounted network file should not change ownership from root")
}

View File

@ -3115,6 +3115,11 @@ func (s *DockerSuite) TestRunNetworkFilesBindMount(c *check.C) {
filename := createTmpFile(c, expected)
defer os.Remove(filename)
// for user namespaced test runs, the temp file must be accessible to unprivileged root
if err := os.Chmod(filename, 0646); err != nil {
c.Fatalf("error modifying permissions of %s: %v", filename, err)
}
nwfiles := []string{"/etc/resolv.conf", "/etc/hosts", "/etc/hostname"}
for i := range nwfiles {
@ -3132,6 +3137,11 @@ func (s *DockerSuite) TestRunNetworkFilesBindMountRO(c *check.C) {
filename := createTmpFile(c, "test123")
defer os.Remove(filename)
// for user namespaced test runs, the temp file must be accessible to unprivileged root
if err := os.Chmod(filename, 0646); err != nil {
c.Fatalf("error modifying permissions of %s: %v", filename, err)
}
nwfiles := []string{"/etc/resolv.conf", "/etc/hosts", "/etc/hostname"}
for i := range nwfiles {
@ -3149,6 +3159,11 @@ func (s *DockerSuite) TestRunNetworkFilesBindMountROFilesystem(c *check.C) {
filename := createTmpFile(c, "test123")
defer os.Remove(filename)
// for user namespaced test runs, the temp file must be accessible to unprivileged root
if err := os.Chmod(filename, 0646); err != nil {
c.Fatalf("error modifying permissions of %s: %v", filename, err)
}
nwfiles := []string{"/etc/resolv.conf", "/etc/hosts", "/etc/hostname"}
for i := range nwfiles {