LCOW: Create layer folders with correct ACL

Signed-off-by: John Howard <jhoward@microsoft.com>
moby · Jun 21, 2017 · ed10ac6 · ed10ac6
1 parent f154588
commit ed10ac6
Show file tree

Hide file tree

Showing 15 changed files with 49 additions and 42 deletions.
diff --git a/api/common.go b/api/common.go
@@ -126,7 +126,7 @@ func MatchesContentType(contentType, expectedType string) bool {
 // LoadOrCreateTrustKey attempts to load the libtrust key at the given path,
 // otherwise generates a new one
 func LoadOrCreateTrustKey(trustKeyPath string) (libtrust.PrivateKey, error) {
-	err := system.MkdirAll(filepath.Dir(trustKeyPath), 0700)
+	err := system.MkdirAll(filepath.Dir(trustKeyPath), 0700, "")
 	if err != nil {
 		return nil, err
 	}

diff --git a/container/container_windows.go b/container/container_windows.go
@@ -43,7 +43,7 @@ func (container *Container) CreateSecretSymlinks() error {
 		if err != nil {
 			return err
 		}
-		if err := system.MkdirAll(filepath.Dir(resolvedPath), 0); err != nil {
+		if err := system.MkdirAll(filepath.Dir(resolvedPath), 0, ""); err != nil {
 			return err
 		}
 		if err := os.Symlink(filepath.Join(containerInternalSecretMountPath, r.SecretID), resolvedPath); err != nil {
@@ -85,7 +85,7 @@ func (container *Container) CreateConfigSymlinks() error {
 		if err != nil {
 			return err
 		}
-		if err := system.MkdirAll(filepath.Dir(resolvedPath), 0); err != nil {
+		if err := system.MkdirAll(filepath.Dir(resolvedPath), 0, ""); err != nil {
 			return err
 		}
 		if err := os.Symlink(filepath.Join(containerInternalConfigsDirPath, configRef.ConfigID), resolvedPath); err != nil {

diff --git a/daemon/container_operations_windows.go b/daemon/container_operations_windows.go
@@ -25,7 +25,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
 	logrus.Debugf("configs: setting up config dir: %s", localPath)
 
 	// create local config root
-	if err := system.MkdirAllWithACL(localPath, 0); err != nil {
+	if err := system.MkdirAllWithACL(localPath, 0, system.SddlAdministratorsLocalSystem); err != nil {
 		return errors.Wrap(err, "error creating config dir")
 	}
 
@@ -98,7 +98,7 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
 	logrus.Debugf("secrets: setting up secret dir: %s", localMountPath)
 
 	// create local secret root
-	if err := system.MkdirAllWithACL(localMountPath, 0); err != nil {
+	if err := system.MkdirAllWithACL(localMountPath, 0, system.SddlAdministratorsLocalSystem); err != nil {
 		return errors.Wrap(err, "error creating secret local directory")
 	}
 

diff --git a/daemon/daemon.go b/daemon/daemon.go
@@ -594,7 +594,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
 	}
 
 	if runtime.GOOS == "windows" {
-		if err := system.MkdirAll(filepath.Join(config.Root, "credentialspecs"), 0); err != nil && !os.IsExist(err) {
+		if err := system.MkdirAll(filepath.Join(config.Root, "credentialspecs"), 0, ""); err != nil && !os.IsExist(err) {
 			return nil, err
 		}
 	}
@@ -709,7 +709,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
 
 	trustDir := filepath.Join(config.Root, "trust")
 
-	if err := system.MkdirAll(trustDir, 0700); err != nil {
+	if err := system.MkdirAll(trustDir, 0700, ""); err != nil {
 		return nil, err
 	}
 

diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go
@@ -465,7 +465,7 @@ func setupRemappedRoot(config *config.Config) (*idtools.IDMappings, error) {
 func setupDaemonRoot(config *config.Config, rootDir string, rootIDs idtools.IDPair) error {
 	config.Root = rootDir
 	// Create the root directory if it doesn't exists
-	if err := system.MkdirAllWithACL(config.Root, 0); err != nil && !os.IsExist(err) {
+	if err := system.MkdirAllWithACL(config.Root, 0, system.SddlAdministratorsLocalSystem); err != nil && !os.IsExist(err) {
 		return err
 	}
 	return nil

diff --git a/daemon/graphdriver/lcow/lcow.go b/daemon/graphdriver/lcow/lcow.go
@@ -190,6 +190,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error {
 	// Make sure layers are created with the correct ACL so that VMs can access them.
 	layerPath := d.dir(id)
 	logrus.Debugf("lcowdriver: create: id %s: creating layerPath %s", id, layerPath)
+	// Make sure the layers are created with the correct ACL so that VMs can access them.
 	if err := system.MkdirAllWithACL(layerPath, 755, system.SddlNtvmAdministratorsLocalSystem); err != nil {
 		return err
 	}

diff --git a/libcontainerd/remote_unix.go b/libcontainerd/remote_unix.go
@@ -80,7 +80,7 @@ func New(stateDir string, options ...RemoteOption) (_ Remote, err error) {
 		}
 	}
 
-	if err := system.MkdirAll(stateDir, 0700); err != nil {
+	if err := system.MkdirAll(stateDir, 0700, ""); err != nil {
 		return nil, err
 	}
 

diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
@@ -1035,7 +1035,7 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
 		dst = filepath.Join(dst, filepath.Base(src))
 	}
 	// Create the holding directory if necessary
-	if err := system.MkdirAll(filepath.Dir(dst), 0700); err != nil {
+	if err := system.MkdirAll(filepath.Dir(dst), 0700, ""); err != nil {
 		return err
 	}
 

diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go
@@ -84,7 +84,7 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64,
 			parentPath := filepath.Join(dest, parent)
 
 			if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) {
-				err = system.MkdirAll(parentPath, 0600)
+				err = system.MkdirAll(parentPath, 0600, "")
 				if err != nil {
 					return 0, err
 				}

diff --git a/pkg/chrootarchive/archive_test.go b/pkg/chrootarchive/archive_test.go
@@ -47,7 +47,7 @@ func TestChrootTarUntar(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if err := ioutil.WriteFile(filepath.Join(src, "toto"), []byte("hello toto"), 0644); err != nil {
@@ -61,7 +61,7 @@ func TestChrootTarUntar(t *testing.T) {
 		t.Fatal(err)
 	}
 	dest := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := system.MkdirAll(dest, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if err := Untar(stream, dest, &archive.TarOptions{ExcludePatterns: []string{"lolo"}}); err != nil {
@@ -78,7 +78,7 @@ func TestChrootUntarWithHugeExcludesList(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if err := ioutil.WriteFile(filepath.Join(src, "toto"), []byte("hello toto"), 0644); err != nil {
@@ -89,7 +89,7 @@ func TestChrootUntarWithHugeExcludesList(t *testing.T) {
 		t.Fatal(err)
 	}
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := system.MkdirAll(dest, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	options := &archive.TarOptions{}
@@ -180,7 +180,7 @@ func TestChrootTarUntarWithSymlink(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, false); err != nil {
@@ -206,7 +206,7 @@ func TestChrootCopyWithTar(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, true); err != nil {
@@ -252,7 +252,7 @@ func TestChrootCopyFileWithTar(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, true); err != nil {
@@ -299,7 +299,7 @@ func TestChrootUntarPath(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, false); err != nil {
@@ -360,7 +360,7 @@ func TestChrootUntarEmptyArchiveFromSlowReader(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := system.MkdirAll(dest, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	stream := &slowEmptyTarReader{size: 10240, chunkSize: 1024}
@@ -376,7 +376,7 @@ func TestChrootApplyEmptyArchiveFromSlowReader(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := system.MkdirAll(dest, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	stream := &slowEmptyTarReader{size: 10240, chunkSize: 1024}
@@ -392,7 +392,7 @@ func TestChrootApplyDotDotFile(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := system.MkdirAll(src, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if err := ioutil.WriteFile(filepath.Join(src, "..gitme"), []byte(""), 0644); err != nil {
@@ -403,7 +403,7 @@ func TestChrootApplyDotDotFile(t *testing.T) {
 		t.Fatal(err)
 	}
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := system.MkdirAll(dest, 0700, ""); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := ApplyLayer(dest, stream); err != nil {

diff --git a/pkg/idtools/idtools_unix.go b/pkg/idtools/idtools_unix.go
@@ -49,7 +49,7 @@ func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chown
 				paths = append(paths, dirPath)
 			}
 		}
-		if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
+		if err := system.MkdirAll(path, mode, ""); err != nil && !os.IsExist(err) {
 			return err
 		}
 	} else {

diff --git a/pkg/idtools/idtools_windows.go b/pkg/idtools/idtools_windows.go
@@ -11,7 +11,7 @@ import (
 // Platforms such as Windows do not support the UID/GID concept. So make this
 // just a wrapper around system.MkdirAll.
 func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chownExisting bool) error {
-	if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
+	if err := system.MkdirAll(path, mode, ""); err != nil && !os.IsExist(err) {
 		return err
 	}
 	return nil

diff --git a/pkg/pidfile/pidfile.go b/pkg/pidfile/pidfile.go
@@ -37,7 +37,7 @@ func New(path string) (*PIDFile, error) {
 		return nil, err
 	}
 	// Note MkdirAll returns nil if a directory already exists
-	if err := system.MkdirAll(filepath.Dir(path), os.FileMode(0755)); err != nil {
+	if err := system.MkdirAll(filepath.Dir(path), os.FileMode(0755), ""); err != nil {
 		return nil, err
 	}
 	if err := ioutil.WriteFile(path, []byte(fmt.Sprintf("%d", os.Getpid())), 0644); err != nil {

diff --git a/pkg/system/filesys.go b/pkg/system/filesys.go
@@ -8,15 +8,14 @@ import (
 	"path/filepath"
 )
 
-// MkdirAllWithACL is a wrapper for MkdirAll that creates a directory
-// ACL'd for Builtin Administrators and Local System.
-func MkdirAllWithACL(path string, perm os.FileMode) error {
-	return MkdirAll(path, perm)
+// MkdirAllWithACL is a wrapper for MkdirAll on unix systems.
+func MkdirAllWithACL(path string, perm os.FileMode, sddl string) error {
+	return MkdirAll(path, perm, sddl)
 }
 
 // MkdirAll creates a directory named path along with any necessary parents,
 // with permission specified by attribute perm for all dir created.
-func MkdirAll(path string, perm os.FileMode) error {
+func MkdirAll(path string, perm os.FileMode, sddl string) error {
 	return os.MkdirAll(path, perm)
 }
 

diff --git a/pkg/system/filesys_windows.go b/pkg/system/filesys_windows.go
@@ -16,21 +16,28 @@ import (
 	winio "github.com/Microsoft/go-winio"
 )
 
+const (
+	// SddlAdministratorsLocalSystem is local administrators plus NT AUTHORITY\System
+	SddlAdministratorsLocalSystem = "D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)"
+	// SddlNtvmAdministratorsLocalSystem is NT VIRTUAL MACHINE\Virtual Machines plus local administrators plus NT AUTHORITY\System
+	SddlNtvmAdministratorsLocalSystem = "D:P(A;OICI;GA;;;S-1-5-83-0)(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)"
+)
+
 // MkdirAllWithACL is a wrapper for MkdirAll that creates a directory
-// ACL'd for Builtin Administrators and Local System.
-func MkdirAllWithACL(path string, perm os.FileMode) error {
-	return mkdirall(path, true)
+// with an appropriate SDDL defined ACL.
+func MkdirAllWithACL(path string, perm os.FileMode, sddl string) error {
+	return mkdirall(path, true, sddl)
 }
 
 // MkdirAll implementation that is volume path aware for Windows.
-func MkdirAll(path string, _ os.FileMode) error {
-	return mkdirall(path, false)
+func MkdirAll(path string, _ os.FileMode, sddl string) error {
+	return mkdirall(path, false, sddl)
 }
 
 // mkdirall is a custom version of os.MkdirAll modified for use on Windows
 // so that it is both volume path aware, and can create a directory with
 // a DACL.
-func mkdirall(path string, adminAndLocalSystem bool) error {
+func mkdirall(path string, applyACL bool, sddl string) error {
 	if re := regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}$`); re.MatchString(path) {
 		return nil
 	}
@@ -64,15 +71,15 @@ func mkdirall(path string, adminAndLocalSystem bool) error {
 
 	if j > 1 {
 		// Create parent
-		err = mkdirall(path[0:j-1], false)
+		err = mkdirall(path[0:j-1], false, sddl)
 		if err != nil {
 			return err
 		}
 	}
 
 	// Parent now exists; invoke os.Mkdir or mkdirWithACL and use its result.
-	if adminAndLocalSystem {
-		err = mkdirWithACL(path)
+	if applyACL {
+		err = mkdirWithACL(path, sddl)
 	} else {
 		err = os.Mkdir(path, 0)
 	}
@@ -96,9 +103,9 @@ func mkdirall(path string, adminAndLocalSystem bool) error {
 // in golang to cater for creating a directory am ACL permitting full
 // access, with inheritance, to any subfolder/file for Built-in Administrators
 // and Local System.
-func mkdirWithACL(name string) error {
+func mkdirWithACL(name string, sddl string) error {
 	sa := syscall.SecurityAttributes{Length: 0}
-	sddl := "D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)"
+
 	sd, err := winio.SddlToSecurityDescriptor(sddl)
 	if err != nil {
 		return &os.PathError{Op: "mkdir", Path: name, Err: err}