Add security policy enforcement for SCSI devices

internal/guest/prot/protocol.go

@@ -769,11 +769,12 @@ type MappedVirtualDisk struct {
 // MappedVirtualDiskV2 represents a disk on the host which is mapped into a
 // directory in the guest in the V2 schema.
 type MappedVirtualDiskV2 struct {
-	MountPath  string   `json:",omitempty"`
-	Lun        uint8    `json:",omitempty"`
-	Controller uint8    `json:",omitempty"`
-	ReadOnly   bool     `json:",omitempty"`
-	Options    []string `json:",omitempty"`
+	MountPath  string            `json:",omitempty"`
+	Lun        uint8             `json:",omitempty"`
+	Controller uint8             `json:",omitempty"`
+	ReadOnly   bool              `json:",omitempty"`
+	Options    []string          `json:",omitempty"`
+	VerityInfo *DeviceVerityInfo `json:",omitempty"`
 }
 
 // MappedDirectory represents a directory on the host which is mapped to a

internal/guest/runtime/hcsv2/uvm.go

@@ -266,7 +266,7 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 func (h *Host) modifyHostSettings(ctx context.Context, containerID string, settings *prot.ModifySettingRequest) error {
 	switch settings.ResourceType {
 	case prot.MrtMappedVirtualDisk:
-		return modifyMappedVirtualDisk(ctx, settings.RequestType, settings.Settings.(*prot.MappedVirtualDiskV2))
+		return modifyMappedVirtualDisk(ctx, settings.RequestType, settings.Settings.(*prot.MappedVirtualDiskV2), h.securityPolicyEnforcer)
 	case prot.MrtMappedDirectory:
 		return modifyMappedDirectory(ctx, h.vsock, settings.RequestType, settings.Settings.(*prot.MappedDirectoryV2))
 	case prot.MrtVPMemDevice:
@@ -426,13 +426,13 @@ func newInvalidRequestTypeError(rt prot.ModifyRequestType) error {
 	return errors.Errorf("the RequestType \"%s\" is not supported", rt)
 }
 
-func modifyMappedVirtualDisk(ctx context.Context, rt prot.ModifyRequestType, mvd *prot.MappedVirtualDiskV2) (err error) {
+func modifyMappedVirtualDisk(ctx context.Context, rt prot.ModifyRequestType, mvd *prot.MappedVirtualDiskV2, securityPolicy securitypolicy.SecurityPolicyEnforcer) (err error) {
 	switch rt {
 	case prot.MreqtAdd:
 		mountCtx, cancel := context.WithTimeout(ctx, time.Second*5)
 		defer cancel()
 		if mvd.MountPath != "" {
-			return scsi.Mount(mountCtx, mvd.Controller, mvd.Lun, mvd.MountPath, mvd.ReadOnly, false, mvd.Options)
+			return scsi.Mount(mountCtx, mvd.Controller, mvd.Lun, mvd.MountPath, mvd.ReadOnly, false, mvd.Options, mvd.VerityInfo, securityPolicy)
 		}
 		return nil
 	case prot.MreqtRemove:

internal/guest/storage/overlay/overlay_test.go

@@ -186,9 +186,9 @@ func Test_Security_Policy_Enforcement(t *testing.T) {
 		t.Fatalf("expected all upper: %v, work: %v, root: %v to be created", upperCreated, workCreated, rootCreated)
 	}
 
-	expectedPmem := 0
-	if enforcer.PmemMountCalls != expectedPmem {
-		t.Errorf("expected %d attempt at pmem mount enforcement, got %d", expectedPmem, enforcer.PmemMountCalls)
+	expectedDeviceMountCalls := 0
+	if enforcer.DeviceMountCalls != expectedDeviceMountCalls {
+		t.Errorf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMountCalls, enforcer.DeviceMountCalls)
 	}
 
 	expectedOverlay := 1

internal/guest/storage/pmem/pmem.go

@@ -80,7 +80,7 @@ func Mount(ctx context.Context, device uint32, target string, mappingInfo *prot.
 	if verityInfo != nil {
 		deviceHash = verityInfo.RootDigest
 	}
-	err = securityPolicy.EnforcePmemMountPolicy(target, deviceHash)
+	err = securityPolicy.EnforceDeviceMountPolicy(target, deviceHash)
 	if err != nil {
 		return errors.Wrapf(err, "won't mount pmem device %d onto %s", device, target)
 	}

internal/guest/storage/pmem/pmem_test.go

@@ -244,9 +244,9 @@ func Test_Security_Policy_Enforcement(t *testing.T) {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
 
-	expectedPmem := 1
-	if enforcer.PmemMountCalls != expectedPmem {
-		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedPmem, enforcer.PmemMountCalls)
+	expectedDeviceMountCalls := 1
+	if enforcer.DeviceMountCalls != expectedDeviceMountCalls {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMountCalls, enforcer.DeviceMountCalls)
 	}
 
 	expectedOverlay := 0

internal/guest/storage/scsi/scsi.go

@@ -10,10 +10,12 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/Microsoft/hcsshim/internal/guest/prot"
 	"github.com/Microsoft/hcsshim/internal/guest/storage"
 	"github.com/Microsoft/hcsshim/internal/guest/storage/crypt"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/oc"
+	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
 	"github.com/pkg/errors"
 	"go.opencensus.io/trace"
 	"golang.org/x/sys/unix"
@@ -41,7 +43,7 @@ const (
 //
 // If `encrypted` is set to true, the SCSI device will be encrypted using
 // dm-crypt.
-func Mount(ctx context.Context, controller, lun uint8, target string, readonly bool, encrypted bool, options []string) (err error) {
+func Mount(ctx context.Context, controller, lun uint8, target string, readonly bool, encrypted bool, options []string, verityInfo *prot.DeviceVerityInfo, securityPolicy securitypolicy.SecurityPolicyEnforcer) (err error) {
 	spnCtx, span := trace.StartSpan(ctx, "scsi::Mount")
 	defer span.End()
 	defer func() { oc.SetSpanStatus(span, err) }()
@@ -50,6 +52,18 @@ func Mount(ctx context.Context, controller, lun uint8, target string, readonly b
 		trace.Int64Attribute("controller", int64(controller)),
 		trace.Int64Attribute("lun", int64(lun)))
 
+	if readonly {
+		// containers only have read-only layers so only enforce for them
+		var deviceHash string
+		if verityInfo != nil {
+			deviceHash = verityInfo.RootDigest
+		}
+		err = securityPolicy.EnforceDeviceMountPolicy(target, deviceHash)
+		if err != nil {
+			return errors.Wrapf(err, "won't mount scsi controller %d lun %d onto %s", controller, lun, target)
+		}
+	}
+
 	if err := osMkdirAll(target, 0700); err != nil {
 		return err
 	}

internal/guest/storage/scsi/scsi_test.go

@@ -8,6 +8,8 @@ import (
 	"os"
 	"testing"
 
+	"github.com/Microsoft/hcsshim/internal/guest/storage/test/policy"
+	"github.com/Microsoft/hcsshim/pkg/securitypolicy"
 	"golang.org/x/sys/unix"
 )
 
@@ -25,7 +27,7 @@ func Test_Mount_Mkdir_Fails_Error(t *testing.T) {
 	osMkdirAll = func(path string, perm os.FileMode) error {
 		return expectedErr
 	}
-	err := Mount(context.Background(), 0, 0, "", false, false, nil)
+	err := Mount(context.Background(), 0, 0, "", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != expectedErr {
 		t.Fatalf("expected err: %v, got: %v", expectedErr, err)
 	}
@@ -52,7 +54,7 @@ func Test_Mount_Mkdir_ExpectedPath(t *testing.T) {
 		// Fake the mount success
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, target, false, false, nil)
+	err := Mount(context.Background(), 0, 0, target, false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil error got: %v", err)
 	}
@@ -79,7 +81,7 @@ func Test_Mount_Mkdir_ExpectedPerm(t *testing.T) {
 		// Fake the mount success
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, target, false, false, nil)
+	err := Mount(context.Background(), 0, 0, target, false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil error got: %v", err)
 	}
@@ -106,7 +108,7 @@ func Test_Mount_ControllerLunToName_Valid_Controller(t *testing.T) {
 		// Fake the mount success
 		return nil
 	}
-	err := Mount(context.Background(), expectedController, 0, "/fake/path", false, false, nil)
+	err := Mount(context.Background(), expectedController, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil error got: %v", err)
 	}
@@ -133,7 +135,7 @@ func Test_Mount_ControllerLunToName_Valid_Lun(t *testing.T) {
 		// Fake the mount success
 		return nil
 	}
-	err := Mount(context.Background(), 0, expectedLun, "/fake/path", false, false, nil)
+	err := Mount(context.Background(), 0, expectedLun, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil error got: %v", err)
 	}
@@ -163,7 +165,7 @@ func Test_Mount_Calls_RemoveAll_OnControllerToLunFailure(t *testing.T) {
 	// NOTE: Do NOT set unixMount because the controller to lun fails. Expect it
 	// not to be called.
 
-	err := Mount(context.Background(), 0, 0, target, false, false, nil)
+	err := Mount(context.Background(), 0, 0, target, false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != expectedErr {
 		t.Fatalf("expected err: %v, got: %v", expectedErr, err)
 	}
@@ -196,7 +198,7 @@ func Test_Mount_Calls_RemoveAll_OnMountFailure(t *testing.T) {
 		// Fake the mount failure to test remove is called
 		return expectedErr
 	}
-	err := Mount(context.Background(), 0, 0, target, false, false, nil)
+	err := Mount(context.Background(), 0, 0, target, false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != expectedErr {
 		t.Fatalf("expected err: %v, got: %v", expectedErr, err)
 	}
@@ -225,7 +227,7 @@ func Test_Mount_Valid_Source(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil)
+	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
@@ -251,7 +253,7 @@ func Test_Mount_Valid_Target(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, expectedTarget, false, false, nil)
+	err := Mount(context.Background(), 0, 0, expectedTarget, false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
@@ -277,7 +279,7 @@ func Test_Mount_Valid_FSType(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil)
+	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
@@ -303,7 +305,7 @@ func Test_Mount_Valid_Flags(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil)
+	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
@@ -329,7 +331,7 @@ func Test_Mount_Readonly_Valid_Flags(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, "/fake/path", true, false, nil)
+	err := Mount(context.Background(), 0, 0, "/fake/path", true, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
@@ -354,7 +356,7 @@ func Test_Mount_Valid_Data(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil)
+	err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
@@ -380,8 +382,92 @@ func Test_Mount_Readonly_Valid_Data(t *testing.T) {
 		}
 		return nil
 	}
-	err := Mount(context.Background(), 0, 0, "/fake/path", true, false, nil)
+	err := Mount(context.Background(), 0, 0, "/fake/path", true, false, nil, nil, openDoorSecurityPolicyEnforcer())
 	if err != nil {
 		t.Fatalf("expected nil err, got: %v", err)
 	}
 }
+
+func Test_Read_Only_Security_Policy_Enforcement(t *testing.T) {
+	clearTestDependencies()
+
+	target := "/fake/path"
+	osMkdirAll = func(path string, perm os.FileMode) error {
+		if path != target {
+			t.Errorf("expected path: %v, got: %v", target, path)
+			return errors.New("unexpected path")
+		}
+		return nil
+	}
+
+	controllerLunToName = func(ctx context.Context, controller, lun uint8) (string, error) {
+		return "", nil
+	}
+
+	unixMount = func(source string, target string, fstype string, flags uintptr, data string) error {
+		// Fake the mount success
+		return nil
+	}
+
+	enforcer := mountMonitoringSecurityPolicyEnforcer()
+	err := Mount(context.Background(), 0, 0, target, true, false, nil, nil, enforcer)
+	if err != nil {
+		t.Fatalf("expected nil err, got: %v", err)
+	}
+
+	expectedDeviceMounts := 1
+	if enforcer.DeviceMountCalls != expectedDeviceMounts {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMounts, enforcer.DeviceMountCalls)
+	}
+
+	expectedOverlay := 0
+	if enforcer.OverlayMountCalls != expectedOverlay {
+		t.Fatalf("expected %d attempts at overlay mount enforcement, got %d", expectedOverlay, enforcer.OverlayMountCalls)
+	}
+}
+
+func Test_Read_Write_Security_Policy_Enforcement(t *testing.T) {
+	clearTestDependencies()
+
+	target := "/fake/path"
+	osMkdirAll = func(path string, perm os.FileMode) error {
+		if path != target {
+			t.Errorf("expected path: %v, got: %v", target, path)
+			return errors.New("unexpected path")
+		}
+		return nil
+	}
+
+	controllerLunToName = func(ctx context.Context, controller, lun uint8) (string, error) {
+		return "", nil
+	}
+
+	unixMount = func(source string, target string, fstype string, flags uintptr, data string) error {
+		// Fake the mount success
+		return nil
+	}
+
+	enforcer := mountMonitoringSecurityPolicyEnforcer()
+	err := Mount(context.Background(), 0, 0, target, false, false, nil, nil, enforcer)
+	if err != nil {
+		t.Fatalf("expected nil err, got: %v", err)
+	}
+
+	expectedDeviceMounts := 0
+	if enforcer.DeviceMountCalls != expectedDeviceMounts {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMounts, enforcer.DeviceMountCalls)
+	}
+
+	expectedOverlay := 0
+	if enforcer.OverlayMountCalls != expectedOverlay {
+		t.Fatalf("expected %d attempts at overlay mount enforcement, got %d", expectedOverlay, enforcer.OverlayMountCalls)
+	}
+}
+
+func openDoorSecurityPolicyEnforcer() securitypolicy.SecurityPolicyEnforcer {
+	return &securitypolicy.OpenDoorSecurityPolicyEnforcer{}
+}
+
+func mountMonitoringSecurityPolicyEnforcer() *policy.MountMonitoringSecurityPolicyEnforcer {
+	return &policy.MountMonitoringSecurityPolicyEnforcer{}
+}

internal/guest/storage/test/policy/mountmonitoringsecuritypolicyenforcer.go

@@ -7,14 +7,14 @@ import (
 // For testing. Records the number of calls to each method so we can verify
 // the expected interactions took place.
 type MountMonitoringSecurityPolicyEnforcer struct {
-	PmemMountCalls    int
+	DeviceMountCalls  int
 	OverlayMountCalls int
 }
 
 var _ securitypolicy.SecurityPolicyEnforcer = (*MountMonitoringSecurityPolicyEnforcer)(nil)
 
-func (p *MountMonitoringSecurityPolicyEnforcer) EnforcePmemMountPolicy(target string, deviceHash string) (err error) {
-	p.PmemMountCalls++
+func (p *MountMonitoringSecurityPolicyEnforcer) EnforceDeviceMountPolicy(target string, deviceHash string) (err error) {
+	p.DeviceMountCalls++
 	return nil
 }
 

internal/guestrequest/types.go

@@ -33,11 +33,12 @@ type WCOWCombinedLayers struct {
 
 // SCSI. Scratch space for remote file-system commands, or R/W layer for containers
 type LCOWMappedVirtualDisk struct {
-	MountPath  string   `json:"MountPath,omitempty"`
-	Lun        uint8    `json:"Lun,omitempty"`
-	Controller uint8    `json:"Controller,omitempty"`
-	ReadOnly   bool     `json:"ReadOnly,omitempty"`
-	Options    []string `json:"Options,omitempty"`
+	MountPath  string            `json:"MountPath,omitempty"`
+	Lun        uint8             `json:"Lun,omitempty"`
+	Controller uint8             `json:"Controller,omitempty"`
+	ReadOnly   bool              `json:"ReadOnly,omitempty"`
+	Options    []string          `json:"Options,omitempty"`
+	VerityInfo *DeviceVerityInfo `json:"VerityInfo,omitempty"`
 }
 
 type WCOWMappedVirtualDisk struct {