From 1716d497a420f0cd4e53a99535704c6d215e38c7 Mon Sep 17 00:00:00 2001 From: Dan Walsh Date: Wed, 28 Oct 2015 09:19:51 -0400 Subject: [PATCH] Relabel BTRFS Content on container Creation This change will allow us to run SELinux in a container with BTRFS back end. We continue to work on fixing the kernel/BTRFS but this change will allow SELinux Security separation on BTRFS. It basically relabels the content on container creation. Just relabling -init directory in BTRFS use case. Everything looks like it works. I don't believe tar/achive stores the SELinux labels, so we are good as far as docker commit. Tested Speed on startup with BTRFS on top of loopback directory. BTRFS not on loopback should get even better perfomance on startup time. The more inodes inside of the container image will increase the relabel time. This patch will give people who care more about security the option of runnin BTRFS with SELinux. Those who don't want to take the slow down can disable SELinux either in individual containers or for all containers by continuing to disable SELinux in the daemon. Without relabel: > time docker run --security-opt label:disable fedora echo test test real 0m0.918s user 0m0.009s sys 0m0.026s With Relabel test real 0m1.942s user 0m0.007s sys 0m0.030s Signed-off-by: Dan Walsh Signed-off-by: Dan Walsh --- daemon/create.go | 6 ++ daemon/daemon.go | 11 +--- daemon/daemon_unix.go | 4 +- daemon/graphdriver/aufs/aufs.go | 2 +- daemon/graphdriver/aufs/aufs_test.go | 60 +++++++++---------- daemon/graphdriver/aufs/migrate.go | 6 +- daemon/graphdriver/btrfs/btrfs.go | 6 +- daemon/graphdriver/devmapper/driver.go | 2 +- daemon/graphdriver/driver.go | 4 +- .../graphdriver/graphtest/graphtest_unix.go | 6 +- daemon/graphdriver/overlay/overlay.go | 2 +- daemon/graphdriver/proxy.go | 7 ++- daemon/graphdriver/vfs/driver.go | 2 +- daemon/graphdriver/windows/windows.go | 2 +- daemon/graphdriver/zfs/zfs.go | 2 +- daemon/start.go | 6 ++ graph/graph.go | 2 +- ...cker_cli_external_graphdriver_unix_test.go | 2 +- man/docker-daemon.8.md | 2 +- 19 files changed, 72 insertions(+), 62 deletions(-) diff --git a/daemon/create.go b/daemon/create.go index fb20d213ca..2c26931733 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -87,6 +87,12 @@ func (daemon *Daemon) create(params *ContainerCreateConfig) (retC *Container, re if err := daemon.Register(container); err != nil { return nil, err } + container.Lock() + if err := parseSecurityOpt(container, params.HostConfig); err != nil { + container.Unlock() + return nil, err + } + container.Unlock() if err := daemon.createRootfs(container); err != nil { return nil, err } diff --git a/daemon/daemon.go b/daemon/daemon.go index 6336faf55a..fc962ff0d3 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -991,7 +991,8 @@ func (daemon *Daemon) createRootfs(container *Container) error { return err } initID := fmt.Sprintf("%s-init", container.ID) - if err := daemon.driver.Create(initID, container.ImageID); err != nil { + + if err := daemon.driver.Create(initID, container.ImageID, container.getMountLabel()); err != nil { return err } initPath, err := daemon.driver.Get(initID, "") @@ -1012,7 +1013,7 @@ func (daemon *Daemon) createRootfs(container *Container) error { return err } - if err := daemon.driver.Create(container.ID, initID); err != nil { + if err := daemon.driver.Create(container.ID, initID, ""); err != nil { return err } return nil @@ -1187,12 +1188,6 @@ func tempDir(rootDir string, rootUID, rootGID int) (string, error) { } func (daemon *Daemon) setHostConfig(container *Container, hostConfig *runconfig.HostConfig) error { - container.Lock() - if err := parseSecurityOpt(container, hostConfig); err != nil { - container.Unlock() - return err - } - container.Unlock() // Do not lock while creating volumes since this could be calling out to external plugins // Don't want to block other actions, like `docker ps` because we're waiting on an external plugin diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index cca5691a64..ea7a0c20c3 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -259,8 +259,8 @@ func checkSystem() error { func configureKernelSecuritySupport(config *Config, driverName string) error { if config.EnableSelinuxSupport { if selinuxEnabled() { - // As Docker on either btrfs or overlayFS and SELinux are incompatible at present, error on both being enabled - if driverName == "btrfs" || driverName == "overlay" { + // As Docker on overlayFS and SELinux are incompatible at present, error on overlayfs being enabled + if driverName == "overlay" { return fmt.Errorf("SELinux is not supported with the %s graph driver", driverName) } logrus.Debug("SELinux enabled successfully") diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index dc66f06c27..0cb5a053d6 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -201,7 +201,7 @@ func (a *Driver) Exists(id string) bool { // Create three folders for each id // mnt, layers, and diff -func (a *Driver) Create(id, parent string) error { +func (a *Driver) Create(id, parent, mountLabel string) error { if err := a.createDirsFor(id); err != nil { return err } diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index bad7513ea7..ee70a8b72c 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -99,7 +99,7 @@ func TestCreateNewDir(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } } @@ -108,7 +108,7 @@ func TestCreateNewDirStructure(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -129,7 +129,7 @@ func TestRemoveImage(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -154,7 +154,7 @@ func TestGetWithoutParent(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -181,7 +181,7 @@ func TestCleanupWithDir(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -194,7 +194,7 @@ func TestMountedFalseResponse(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -213,10 +213,10 @@ func TestMountedTrueReponse(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } - if err := d.Create("2", "1"); err != nil { + if err := d.Create("2", "1", ""); err != nil { t.Fatal(err) } @@ -239,10 +239,10 @@ func TestMountWithParent(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } - if err := d.Create("2", "1"); err != nil { + if err := d.Create("2", "1", ""); err != nil { t.Fatal(err) } @@ -270,10 +270,10 @@ func TestRemoveMountedDir(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } - if err := d.Create("2", "1"); err != nil { + if err := d.Create("2", "1", ""); err != nil { t.Fatal(err) } @@ -309,7 +309,7 @@ func TestCreateWithInvalidParent(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", "docker"); err == nil { + if err := d.Create("1", "docker", ""); err == nil { t.Fatalf("Error should not be nil with parent does not exist") } } @@ -318,7 +318,7 @@ func TestGetDiff(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -352,10 +352,10 @@ func TestChanges(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } - if err := d.Create("2", "1"); err != nil { + if err := d.Create("2", "1", ""); err != nil { t.Fatal(err) } @@ -401,7 +401,7 @@ func TestChanges(t *testing.T) { t.Fatalf("Change kind should be ChangeAdd got %s", change.Kind) } - if err := d.Create("3", "2"); err != nil { + if err := d.Create("3", "2", ""); err != nil { t.Fatal(err) } mntPoint, err = d.Get("3", "") @@ -446,7 +446,7 @@ func TestDiffSize(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -488,7 +488,7 @@ func TestChildDiffSize(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -524,7 +524,7 @@ func TestChildDiffSize(t *testing.T) { t.Fatalf("Expected size to be %d got %d", size, diffSize) } - if err := d.Create("2", "1"); err != nil { + if err := d.Create("2", "1", ""); err != nil { t.Fatal(err) } @@ -543,7 +543,7 @@ func TestExists(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -561,7 +561,7 @@ func TestStatus(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -590,7 +590,7 @@ func TestApplyDiff(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", ""); err != nil { + if err := d.Create("1", "", ""); err != nil { t.Fatal(err) } @@ -616,10 +616,10 @@ func TestApplyDiff(t *testing.T) { t.Fatal(err) } - if err := d.Create("2", ""); err != nil { + if err := d.Create("2", "", ""); err != nil { t.Fatal(err) } - if err := d.Create("3", "2"); err != nil { + if err := d.Create("3", "2", ""); err != nil { t.Fatal(err) } @@ -647,7 +647,7 @@ func TestHardlinks(t *testing.T) { origFile := "test_file" linkedFile := "linked_file" - if err := d.Create("source-1", ""); err != nil { + if err := d.Create("source-1", "", ""); err != nil { t.Fatal(err) } @@ -667,7 +667,7 @@ func TestHardlinks(t *testing.T) { t.Fatal(err) } - if err := d.Create("source-2", "source-1"); err != nil { + if err := d.Create("source-2", "source-1", ""); err != nil { t.Fatal(err) } @@ -685,7 +685,7 @@ func TestHardlinks(t *testing.T) { t.Fatal(err) } - if err := d.Create("target-1", ""); err != nil { + if err := d.Create("target-1", "", ""); err != nil { t.Fatal(err) } @@ -693,7 +693,7 @@ func TestHardlinks(t *testing.T) { t.Fatal(err) } - if err := d.Create("target-2", "target-1"); err != nil { + if err := d.Create("target-2", "target-1", ""); err != nil { t.Fatal(err) } @@ -751,7 +751,7 @@ func testMountMoreThan42Layers(t *testing.T, mountPath string) { } current = hash(current) - if err := d.Create(current, parent); err != nil { + if err := d.Create(current, parent, ""); err != nil { t.Logf("Current layer %d", i) t.Error(err) } diff --git a/daemon/graphdriver/aufs/migrate.go b/daemon/graphdriver/aufs/migrate.go index 2da3c61e5b..b195bf3f88 100644 --- a/daemon/graphdriver/aufs/migrate.go +++ b/daemon/graphdriver/aufs/migrate.go @@ -86,7 +86,7 @@ func (a *Driver) migrateContainers(pth string, setupInit func(p string, rootUID, } initID := fmt.Sprintf("%s-init", id) - if err := a.Create(initID, metadata.Image); err != nil { + if err := a.Create(initID, metadata.Image, ""); err != nil { return err } @@ -99,7 +99,7 @@ func (a *Driver) migrateContainers(pth string, setupInit func(p string, rootUID, return err } - if err := a.Create(id, initID); err != nil { + if err := a.Create(id, initID, ""); err != nil { return err } } @@ -153,7 +153,7 @@ func (a *Driver) migrateImage(m *metadata, pth string, migrated map[string]bool) return err } if !a.Exists(m.ID) { - if err := a.Create(m.ID, m.ParentID); err != nil { + if err := a.Create(m.ID, m.ParentID, ""); err != nil { return err } } diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index 59f98f9886..eaf5498407 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/daemon/graphdriver/btrfs/btrfs.go @@ -21,6 +21,7 @@ import ( "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/mount" + "github.com/opencontainers/runc/libcontainer/label" ) func init() { @@ -233,7 +234,7 @@ func (d *Driver) subvolumesDirID(id string) string { } // Create the filesystem with given id. -func (d *Driver) Create(id string, parent string) error { +func (d *Driver) Create(id, parent, mountLabel string) error { subvolumes := path.Join(d.home, "subvolumes") rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { @@ -255,7 +256,8 @@ func (d *Driver) Create(id string, parent string) error { return err } } - return nil + + return label.Relabel(path.Join(subvolumes, id), mountLabel, false) } // Remove the filesystem with given id. diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go index 5e3d9bac7c..b548dad4b5 100644 --- a/daemon/graphdriver/devmapper/driver.go +++ b/daemon/graphdriver/devmapper/driver.go @@ -133,7 +133,7 @@ func (d *Driver) Cleanup() error { } // Create adds a device with a given id and the parent. -func (d *Driver) Create(id, parent string) error { +func (d *Driver) Create(id, parent, mountLabel string) error { if err := d.DeviceSet.AddDevice(id, parent); err != nil { return err } diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index a24961d7d4..4a6e67c9e0 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -48,8 +48,8 @@ type ProtoDriver interface { // String returns a string representation of this driver. String() string // Create creates a new, empty, filesystem layer with the - // specified id and parent. Parent may be "". - Create(id, parent string) error + // specified id and parent and mountLabel. Parent and mountLabel may be "". + Create(id, parent, mountLabel string) error // Remove attempts to remove the filesystem layer with this id. Remove(id string) error // Get returns the mountpoint for the layered filesystem referred diff --git a/daemon/graphdriver/graphtest/graphtest_unix.go b/daemon/graphdriver/graphtest/graphtest_unix.go index 9ad8b4c395..534f2e586b 100644 --- a/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/daemon/graphdriver/graphtest/graphtest_unix.go @@ -177,7 +177,7 @@ func DriverTestCreateEmpty(t *testing.T, drivername string) { driver := GetDriver(t, drivername) defer PutDriver(t) - if err := driver.Create("empty", ""); err != nil { + if err := driver.Create("empty", "", ""); err != nil { t.Fatal(err) } @@ -215,7 +215,7 @@ func createBase(t *testing.T, driver graphdriver.Driver, name string) { oldmask := syscall.Umask(0) defer syscall.Umask(oldmask) - if err := driver.Create(name, ""); err != nil { + if err := driver.Create(name, "", ""); err != nil { t.Fatal(err) } @@ -283,7 +283,7 @@ func DriverTestCreateSnap(t *testing.T, drivername string) { createBase(t, driver, "Base") - if err := driver.Create("Snap", "Base"); err != nil { + if err := driver.Create("Snap", "Base", ""); err != nil { t.Fatal(err) } diff --git a/daemon/graphdriver/overlay/overlay.go b/daemon/graphdriver/overlay/overlay.go index df3ff13cc8..885d55a268 100644 --- a/daemon/graphdriver/overlay/overlay.go +++ b/daemon/graphdriver/overlay/overlay.go @@ -230,7 +230,7 @@ func (d *Driver) Cleanup() error { // Create is used to create the upper, lower, and merge directories required for overlay fs for a given id. // The parent filesystem is used to configure these directories for the overlay. -func (d *Driver) Create(id string, parent string) (retErr error) { +func (d *Driver) Create(id, parent, mountLabel string) (retErr error) { dir := d.dir(id) rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) diff --git a/daemon/graphdriver/proxy.go b/daemon/graphdriver/proxy.go index ff19dcdfa8..35a06da415 100644 --- a/daemon/graphdriver/proxy.go +++ b/daemon/graphdriver/proxy.go @@ -55,10 +55,11 @@ func (d *graphDriverProxy) String() string { return d.name } -func (d *graphDriverProxy) Create(id, parent string) error { +func (d *graphDriverProxy) Create(id, parent, mountLabel string) error { args := &graphDriverRequest{ - ID: id, - Parent: parent, + ID: id, + Parent: parent, + MountLabel: mountLabel, } var ret graphDriverResponse if err := d.client.Call("GraphDriver.Create", args, &ret); err != nil { diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 5633de483f..d2b807abc1 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -66,7 +66,7 @@ func (d *Driver) Cleanup() error { } // Create prepares the filesystem for the VFS driver and copies the directory for the given id under the parent. -func (d *Driver) Create(id, parent string) error { +func (d *Driver) Create(id, parent, mountLabel string) error { dir := d.dir(id) rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { diff --git a/daemon/graphdriver/windows/windows.go b/daemon/graphdriver/windows/windows.go index de5e48ff60..007c6f907f 100644 --- a/daemon/graphdriver/windows/windows.go +++ b/daemon/graphdriver/windows/windows.go @@ -129,7 +129,7 @@ func (d *Driver) Exists(id string) bool { } // Create creates a new layer with the given id. -func (d *Driver) Create(id, parent string) error { +func (d *Driver) Create(id, parent, mountLabel string) error { rPId, err := d.resolveID(parent) if err != nil { return err diff --git a/daemon/graphdriver/zfs/zfs.go b/daemon/graphdriver/zfs/zfs.go index ac10680e6d..156f4cfa15 100644 --- a/daemon/graphdriver/zfs/zfs.go +++ b/daemon/graphdriver/zfs/zfs.go @@ -241,7 +241,7 @@ func (d *Driver) mountPath(id string) string { } // Create prepares the dataset and filesystem for the ZFS driver for the given id under the parent. -func (d *Driver) Create(id string, parent string) error { +func (d *Driver) Create(id string, parent string, mountLabel string) error { err := d.create(id, parent) if err == nil { return nil diff --git a/daemon/start.go b/daemon/start.go index de4516c7b6..5f6d5b8dbb 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -29,6 +29,12 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *runconfig.HostConf // This is kept for backward compatibility - hostconfig should be passed when // creating a container, not during start. if hostConfig != nil { + container.Lock() + if err := parseSecurityOpt(container, hostConfig); err != nil { + container.Unlock() + return err + } + container.Unlock() if err := daemon.setHostConfig(container, hostConfig); err != nil { return err } diff --git a/graph/graph.go b/graph/graph.go index fed2ee0216..2de3e56959 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -359,7 +359,7 @@ func (graph *Graph) register(im image.Descriptor, layerData io.Reader) (err erro } func createRootFilesystemInDriver(graph *Graph, id, parent string) error { - if err := graph.driver.Create(id, parent); err != nil { + if err := graph.driver.Create(id, parent, ""); err != nil { return fmt.Errorf("Driver %s failed to create image rootfs %s: %s", graph.driver, id, err) } return nil diff --git a/integration-cli/docker_cli_external_graphdriver_unix_test.go b/integration-cli/docker_cli_external_graphdriver_unix_test.go index 06fc5d9668..4f87f60f08 100644 --- a/integration-cli/docker_cli_external_graphdriver_unix_test.go +++ b/integration-cli/docker_cli_external_graphdriver_unix_test.go @@ -122,7 +122,7 @@ func (s *DockerExternalGraphdriverSuite) SetUpSuite(c *check.C) { if err := decReq(r.Body, &req, w); err != nil { return } - if err := driver.Create(req.ID, req.Parent); err != nil { + if err := driver.Create(req.ID, req.Parent, ""); err != nil { respond(w, err) return } diff --git a/man/docker-daemon.8.md b/man/docker-daemon.8.md index 31e20c7c67..de16f6849e 100644 --- a/man/docker-daemon.8.md +++ b/man/docker-daemon.8.md @@ -191,7 +191,7 @@ unix://[/path/to/socket] to use. Force the Docker runtime to use a specific storage driver. **--selinux-enabled**=*true*|*false* - Enable selinux support. Default is false. SELinux does not presently support the BTRFS storage driver. + Enable selinux support. Default is false. SELinux does not presently support the overlay storage driver. **--storage-opt**=[] Set storage driver options. See STORAGE DRIVER OPTIONS.