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 firecracker-microvm#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