Repository: firecracker-microvm/firecracker-containerd (go)

Issue:
A jailed firecracker cannot mount DM-backed devices such as /dev/dm-X
Our [master build is failing](https://buildkite.com/firecracker-microvm/firecracker-containerd/builds/990#80128eb3-5fa0-46b0-86c9-df2e99fefbb4), probably because a jailed firecracker cannot access DM-backed devices such as /dev/dm-X.

Device Mapper snapshotter creates a snapshot like below;

```
% ls -l /dev/mapper/fcci--vg-build_1-snap-21
lrwxrwxrwx 1 root root 8 Oct 25 18:00 /dev/mapper/fcci--vg-build_1-snap-21 -> ../dm-29
```

And the actual file (/dev/dm-29 in this case) is readable from `root` or users who belong to `disk` group.

```
% ls -l /dev/dm-29
brw-rw---- 1 root disk 253, 29 Oct 25 18:00 /dev/dm-29
```

Pull request:
Chown newly created directories in jailer's ExposeDevice method.
Before this change, if exposing a device to a jail resulted in newly created
directories under the jail root, the directories were created with 0700 perms
but uid/gid owner set to root. This change results in the same 0700 perms but
chowns the new directories to the jail's uid/gid.

This is important as devices created by the devmapper snapshotter are multiple
levels underneath /dev, which means the jailer code now creates new directories
(as opposed to the naive snapshotter which didn't result in new directories
being created).

Signed-off-by: Erik Sipsma <sipsma@amazon.com>

Fixes #307, I just happened to run into this exact same bug for different reasons when making an update to the drive mount PR.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


Patch:
diff --git a/runtime/runc_jailer.go b/runtime/runc_jailer.go
index e22fe1e..096d9a7 100644
--- a/runtime/runc_jailer.go
+++ b/runtime/runc_jailer.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"syscall"
 
 	"github.com/firecracker-microvm/firecracker-go-sdk"
@@ -270,7 +271,7 @@ func (j runcJailer) ExposeDeviceToJail(srcDevicePath string) error {
 	// we will manually call mknod and create that device.
 	if (stat.Mode & syscall.S_IFMT) == syscall.S_IFBLK {
 		path := filepath.Join(j.RootPath(), filepath.Dir(srcDevicePath))
-		if err := os.MkdirAll(path, 0700); err != nil {
+		if err := mkdirAllWithPermissions(path, 0700, uid, gid); err != nil {
 			return err
 		}
 
@@ -403,3 +404,25 @@ func mkdirAndChown(path string, mode os.FileMode, uid, gid uint32) error {
 
 	return nil
 }
+
+// mkdirAllWithPermissions will create any directories in the provided path that
+// don't exist (similar to os.MkdirAll) and will chmod/chown newly created
+// directories using the provided mode, uid and gid. If a directory in the path
+// already exists, its mode and ownership are left unmodified.
+func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid uint32) error {
+	var workingPath string
+	if strings.HasPrefix(path, "/") {
+		workingPath = "/"
+	}
+
+	for _, pathPart := range strings.Split(filepath.Clean(path), "/") {
+		workingPath = filepath.Join(workingPath, pathPart)
+
+		err := mkdirAndChown(workingPath, mode, uid, gid)
+		if err != nil && !os.IsExist(err) {
+			return err
+		}
+	}
+
+	return nil
+}


Test patch:
diff --git a/runtime/runc_jailer_test.go b/runtime/runc_jailer_test.go
index 7760493..587ccdb 100644
--- a/runtime/runc_jailer_test.go
+++ b/runtime/runc_jailer_test.go
@@ -18,6 +18,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
 
 	"github.com/firecracker-microvm/firecracker-go-sdk"
@@ -91,3 +92,35 @@ func TestBuildJailedRootHandler_Isolated(t *testing.T) {
 	_, err = os.Stat(filepath.Join(dir, "rootfs", filepath.Base(rootDrivePath)))
 	assert.NoError(t, err, "failed to create root drive")
 }
+
+func TestMkdirAllWithPermissions_Isolated(t *testing.T) {
+	// requires isolation so we can change uid/gid of files
+	internal.RequiresIsolation(t)
+
+	tmpdir, err := ioutil.TempDir("./", "TestMkdirAllWithPermissions")
+	require.NoError(t, err, "failed to create temporary directory")
+
+	existingPath := filepath.Join(tmpdir, "exists")
+	existingMode := os.FileMode(0700)
+	err = os.Mkdir(existingPath, existingMode)
+	require.NoError(t, err, "failed to create existing part of test directory")
+
+	nonExistingPath := filepath.Join(existingPath, "nonexistent")
+	newMode := os.FileMode(0755)
+	newuid := uint32(123)
+	newgid := uint32(456)
+	err = mkdirAllWithPermissions(nonExistingPath, newMode, newuid, newgid)
+	require.NoError(t, err, "failed to mkdirAllWithPermissions")
+
+	existingPathStat, err := os.Stat(existingPath)
+	require.NoError(t, err, "failed to stat pre-existing path")
+	assert.Equal(t, existingMode, existingPathStat.Mode().Perm())
+	assert.Equal(t, uint32(os.Getuid()), existingPathStat.Sys().(*syscall.Stat_t).Uid)
+	assert.Equal(t, uint32(os.Getgid()), existingPathStat.Sys().(*syscall.Stat_t).Gid)
+
+	newlyCreatedPathStat, err := os.Stat(nonExistingPath)
+	require.NoError(t, err, "failed to stat newly created path")
+	assert.Equal(t, newMode, newlyCreatedPathStat.Mode().Perm())
+	assert.Equal(t, newuid, newlyCreatedPathStat.Sys().(*syscall.Stat_t).Uid)
+	assert.Equal(t, newgid, newlyCreatedPathStat.Sys().(*syscall.Stat_t).Gid)
+}