Support DriveMount API in CreateVM.

This implements the proposal found in docs/drive-mounts-proposal.md, with the
exception for supporting RateLimiters and IsReadOnly in the DriveMount objects,
due to the need for further refactoring of the internal stub drive code (will
be followed up in #296).

Signed-off-by: Erik Sipsma <[email protected]>

Author: sipsma

agent/drive_handler.go

@@ -14,12 +14,20 @@
 package main
 
 import (
+	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
+	"github.com/containerd/containerd/log"
+	"github.com/containerd/containerd/mount"
 	"github.com/firecracker-microvm/firecracker-containerd/internal"
+	drivemount "github.com/firecracker-microvm/firecracker-containerd/proto/service/drivemount/ttrpc"
+	"github.com/golang/protobuf/ptypes/empty"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -28,6 +36,14 @@ const (
 	blockMajorMinor = "dev"
 )
 
+var (
+	bannedSystemDirs = []string{
+		"/proc",
+		"/sys",
+		"/dev",
+	}
+)
+
 type drive struct {
 	Name       string
 	DriveID    string
@@ -45,6 +61,8 @@ type driveHandler struct {
 	DrivePath string
 }
 
+var _ drivemount.DriveMounterService = &driveHandler{}
+
 func newDriveHandler(blockPath, drivePath string) (*driveHandler, error) {
 	d := &driveHandler{
 		drives:    map[string]drive{},
@@ -146,3 +164,107 @@ func isStubDrive(d drive) bool {
 
 	return internal.IsStubDrive(f)
 }
+
+func (dh driveHandler) MountDrive(ctx context.Context, req *drivemount.MountDriveRequest) (*empty.Empty, error) {
+	logger := log.G(ctx)
+	logger.Debugf("%+v", req.String())
+	logger = logger.WithField("drive_id", req.DriveID)
+
+	drive, ok := dh.GetDrive(req.DriveID)
+	if !ok {
+		return nil, fmt.Errorf("drive %q could not be found", req.DriveID)
+	}
+	logger = logger.WithField("drive_path", drive.Path())
+
+	// Do a basic check that we won't be mounting over any important system directories
+	resolvedDest, err := evalAnySymlinks(req.DestinationPath)
+	if err != nil {
+		return nil, errors.Wrapf(err,
+			"failed to evaluate any symlinks in drive mount destination %q", req.DestinationPath)
+	}
+
+	for _, systemDir := range bannedSystemDirs {
+		if isOrUnderDir(resolvedDest, systemDir) {
+			return nil, errors.Errorf(
+				"drive mount destination %q resolves to path %q under banned system directory %q",
+				req.DestinationPath, resolvedDest, systemDir,
+			)
+		}
+	}
+
+	err = os.MkdirAll(req.DestinationPath, 0700)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to create drive mount destination %q", req.DestinationPath)
+	}
+
+	// Retry the mount in the case of failure a fixed number of times. This works around a rare issue
+	// where we get to this mount attempt before the guest OS has realized a drive was patched:
+	// https://github.com/firecracker-microvm/firecracker-containerd/issues/214
+	const (
+		maxRetries = 100
+		retryDelay = 10 * time.Millisecond
+	)
+
+	for i := 0; i < maxRetries; i++ {
+		err := mount.All([]mount.Mount{{
+			Source:  drive.Path(),
+			Type:    req.FilesytemType,
+			Options: req.Options,
+		}}, req.DestinationPath)
+		if err == nil {
+			return &empty.Empty{}, nil
+		}
+
+		if isRetryableMountError(err) {
+			logger.WithError(err).Warnf("retryable failure mounting drive")
+			time.Sleep(retryDelay)
+			continue
+		}
+
+		return nil, errors.Wrapf(err, "non-retryable failure mounting drive from %q to %q",
+			drive.Path(), req.DestinationPath)
+	}
+
+	return nil, errors.Errorf("exhausted retries mounting drive from %q to %q",
+		drive.Path(), req.DestinationPath)
+}
+
+// evalAnySymlinks is similar to filepath.EvalSymlinks, except it will not return an error if part of the
+// provided path does not exist. It will evaluate symlinks present in the path up to a component that doesn't
+// exist, at which point it will just append the rest of the provided path to what has been resolved so far.
+// We validate earlier that input to this function is an absolute path.
+func evalAnySymlinks(path string) (string, error) {
+	curPath := "/"
+	pathSplit := strings.Split(filepath.Clean(path), "/")
+	for len(pathSplit) > 0 {
+		curPath = filepath.Join(curPath, pathSplit[0])
+		pathSplit = pathSplit[1:]
+
+		resolvedPath, err := filepath.EvalSymlinks(curPath)
+		if os.IsNotExist(err) {
+			return filepath.Join(append([]string{curPath}, pathSplit...)...), nil
+		}
+		if err != nil {
+			return "", err
+		}
+		curPath = resolvedPath
+	}
+
+	return curPath, nil
+}
+
+// returns whether the given path is the provided baseDir or is under it
+func isOrUnderDir(path, baseDir string) bool {
+	path = filepath.Clean(path)
+	baseDir = filepath.Clean(baseDir)
+
+	if baseDir == "/" {
+		return true
+	}
+
+	if path == baseDir {
+		return true
+	}
+
+	return strings.HasPrefix(path, baseDir+"/")
+}

agent/drive_handler_test.go

@@ -14,9 +14,14 @@
 package main
 
 import (
+	"os"
+	"path/filepath"
+	"strconv"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDiscoverDrives(t *testing.T) {
@@ -39,3 +44,124 @@ func TestDiscoverDrives(t *testing.T) {
 		})
 	}
 }
+
+func TestEvalAnySymlinks(t *testing.T) {
+	existingSymlink := "/proc/self/cwd" // will always exist and be a symlink to our cwd
+	nonExistentPath := filepath.Join(strconv.Itoa(int(time.Now().UnixNano())), "foo")
+	testPath := filepath.Join(existingSymlink, nonExistentPath)
+
+	resolvedPath, err := evalAnySymlinks(testPath)
+	require.NoError(t, err, "failed to evaluate symlinks in %q", testPath)
+
+	cwd, err := os.Getwd()
+	require.NoError(t, err, "failed to get current working dir")
+	assert.Equal(t, filepath.Join(cwd, nonExistentPath), resolvedPath)
+}
+
+func TestIsOrUnderDir(t *testing.T) {
+	type testcase struct {
+		baseDir      string
+		path         string
+		expectedTrue bool
+	}
+
+	for _, tc := range []testcase{
+		{
+			baseDir:      "/foo",
+			path:         "/foo/bar",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo/bar",
+			path:         "/foo/bar/baz",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/foo",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo/bar",
+			path:         "/foo/bar",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/foobar",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/bar",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo/bar",
+			path:         "/bar",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo/bar",
+			path:         "/foo",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo/bar",
+			path:         "/bar/bar",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "foo",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "bar",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/foo/../foo",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo/bar",
+			path:         "/foo/../foo/bar",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/foo/../bar",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/foo/..bar",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/foo/..bar/baz",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/",
+			path:         "/",
+			expectedTrue: true,
+		},
+		{
+			baseDir:      "/foo",
+			path:         "/",
+			expectedTrue: false,
+		},
+		{
+			baseDir:      "/",
+			path:         "/foo",
+			expectedTrue: true,
+		},
+	} {
+		assert.Equalf(t, tc.expectedTrue, isOrUnderDir(tc.path, tc.baseDir), "unexpected output for isOrUnderDir case %+v", tc)
+	}
+}

agent/main.go

@@ -34,6 +34,8 @@ import (
 	"github.com/firecracker-microvm/firecracker-containerd/eventbridge"
 	"github.com/firecracker-microvm/firecracker-containerd/internal/event"
 	"github.com/firecracker-microvm/firecracker-containerd/internal/vm"
+
+	drivemount "github.com/firecracker-microvm/firecracker-containerd/proto/service/drivemount/ttrpc"
 )
 
 const (
@@ -77,19 +79,25 @@ func main() {
 
 	log.G(shimCtx).Info("creating task service")
 
+	server, err := ttrpc.NewServer()
+	if err != nil {
+		log.G(shimCtx).WithError(err).Fatal("failed to create ttrpc server")
+	}
+
 	eventExchange := &event.ExchangeCloser{Exchange: exchange.NewExchange()}
+	eventbridge.RegisterGetterService(server, eventbridge.NewGetterService(shimCtx, eventExchange))
+
 	taskService, err := NewTaskService(shimCtx, shimCancel, eventExchange)
 	if err != nil {
 		log.G(shimCtx).WithError(err).Fatal("failed to create task service")
 	}
+	taskAPI.RegisterTaskService(server, taskService)
 
-	server, err := ttrpc.NewServer()
+	dh, err := newDriveHandler(blockPath, drivePath)
 	if err != nil {
-		log.G(shimCtx).WithError(err).Fatal("failed to create ttrpc server")
+		log.G(shimCtx).WithError(err).Fatal("failed to create drive handler")
 	}
-
-	taskAPI.RegisterTaskService(server, taskService)
-	eventbridge.RegisterGetterService(server, eventbridge.NewGetterService(shimCtx, eventExchange))
+	drivemount.RegisterDriveMounterService(server, dh)
 
 	// Run ttrpc over vsock