Enforce security policy at unmount

internal/guest/runtime/hcsv2/uvm.go

@@ -437,7 +437,7 @@ func modifyMappedVirtualDisk(ctx context.Context, rt prot.ModifyRequestType, mvd
 		return nil
 	case prot.MreqtRemove:
 		if mvd.MountPath != "" {
-			if err := scsi.Unmount(ctx, mvd.Controller, mvd.Lun, mvd.MountPath, mvd.Encrypted); err != nil {
+			if err := scsi.Unmount(ctx, mvd.Controller, mvd.Lun, mvd.MountPath, mvd.Encrypted, mvd.VerityInfo, securityPolicy); err != nil {
 				return err
 			}
 		}

internal/guest/storage/pmem/pmem.go

@@ -128,6 +128,10 @@ func Unmount(ctx context.Context, devNumber uint32, target string, mappingInfo *
 		trace.Int64Attribute("device", int64(devNumber)),
 		trace.StringAttribute("target", target))
 
+	if err := securityPolicy.EnforceDeviceUnmountPolicy(target); err != nil {
+		return errors.Wrapf(err, "unmounting pmem device from %s denied by policy", target)
+	}
+
 	if err := storage.UnmountPath(ctx, target, true); err != nil {
 		return errors.Wrapf(err, "failed to unmount target: %s", target)
 	}

internal/guest/storage/pmem/pmem_test.go

@@ -227,7 +227,7 @@ func Test_Mount_Valid_Data(t *testing.T) {
 	}
 }
 
-func Test_Security_Policy_Enforcement(t *testing.T) {
+func Test_Security_Policy_Enforcement_Mount_Calls(t *testing.T) {
 	clearTestDependencies()
 
 	osMkdirAll = func(path string, perm os.FileMode) error {
@@ -249,6 +249,49 @@ func Test_Security_Policy_Enforcement(t *testing.T) {
 		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMountCalls, enforcer.DeviceMountCalls)
 	}
 
+	expectedDeviceUnmountCalls := 0
+	if enforcer.DeviceUnmountCalls != expectedDeviceUnmountCalls {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceUnmountCalls, enforcer.DeviceUnmountCalls)
+	}
+
+	expectedOverlay := 0
+	if enforcer.OverlayMountCalls != expectedOverlay {
+		t.Fatalf("expected %d attempts at overlay mount enforcement, got %d", expectedOverlay, enforcer.OverlayMountCalls)
+	}
+}
+
+func Test_Security_Policy_Enforcement_Unmount_Calls(t *testing.T) {
+	clearTestDependencies()
+
+	osMkdirAll = func(path string, perm os.FileMode) error {
+		return nil
+	}
+
+	unixMount = func(source string, target string, fstype string, flags uintptr, data string) error {
+		return nil
+	}
+
+	enforcer := mountMonitoringSecurityPolicyEnforcer()
+	err := Mount(context.Background(), 0, "/fake/path", nil, nil, enforcer)
+	if err != nil {
+		t.Fatalf("expected nil err, got: %v", err)
+	}
+
+	err = Unmount(context.Background(), 0, "/fake/path", nil, nil, enforcer)
+	if err != nil {
+		t.Fatalf("expected nil err, got: %v", err)
+	}
+
+	expectedDeviceMountCalls := 1
+	if enforcer.DeviceMountCalls != expectedDeviceMountCalls {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMountCalls, enforcer.DeviceMountCalls)
+	}
+
+	expectedDeviceUnmountCalls := 1
+	if enforcer.DeviceUnmountCalls != expectedDeviceUnmountCalls {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceUnmountCalls, enforcer.DeviceUnmountCalls)
+	}
+
 	expectedOverlay := 0
 	if enforcer.OverlayMountCalls != expectedOverlay {
 		t.Fatalf("expected %d attempts at overlay mount enforcement, got %d", expectedOverlay, enforcer.OverlayMountCalls)

internal/guest/storage/scsi/scsi.go

@@ -128,7 +128,7 @@ func Mount(ctx context.Context, controller, lun uint8, target string, readonly b
 // Unmount unmounts a SCSI device mounted at `target`.
 //
 // If `encrypted` is true, it removes all its associated dm-crypto state.
-func Unmount(ctx context.Context, controller, lun uint8, target string, encrypted bool) (err error) {
+func Unmount(ctx context.Context, controller, lun uint8, target string, encrypted bool, verityInfo *prot.DeviceVerityInfo, securityPolicy securitypolicy.SecurityPolicyEnforcer) (err error) {
 	ctx, span := trace.StartSpan(ctx, "scsi::Unmount")
 	defer span.End()
 	defer func() { oc.SetSpanStatus(span, err) }()
@@ -138,6 +138,10 @@ func Unmount(ctx context.Context, controller, lun uint8, target string, encrypte
 		trace.Int64Attribute("lun", int64(lun)),
 		trace.StringAttribute("target", target))
 
+	if err = securityPolicy.EnforceDeviceUnmountPolicy(target); err != nil {
+		return errors.Wrapf(err, "unmounting scsi controller %d lun %d from  %s denied by policy", controller, lun, target)
+	}
+
 	// Unmount unencrypted device
 	if err := storage.UnmountPath(ctx, target, true); err != nil {
 		return errors.Wrapf(err, "unmount failed: "+target)

internal/guest/storage/scsi/scsi_test.go

@@ -388,7 +388,7 @@ func Test_Mount_Readonly_Valid_Data(t *testing.T) {
 	}
 }
 
-func Test_Read_Only_Security_Policy_Enforcement(t *testing.T) {
+func Test_Read_Only_Security_Policy_Enforcement_Mount_Calls(t *testing.T) {
 	clearTestDependencies()
 
 	target := "/fake/path"
@@ -420,13 +420,18 @@ func Test_Read_Only_Security_Policy_Enforcement(t *testing.T) {
 		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMounts, enforcer.DeviceMountCalls)
 	}
 
+	expectedDeviceUnmounts := 0
+	if enforcer.DeviceUnmountCalls != expectedDeviceUnmounts {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceUnmounts, enforcer.DeviceUnmountCalls)
+	}
+
 	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) {
+func Test_Read_Write_Security_Policy_Enforcement_Mount_Calls(t *testing.T) {
 	clearTestDependencies()
 
 	target := "/fake/path"
@@ -458,6 +463,59 @@ func Test_Read_Write_Security_Policy_Enforcement(t *testing.T) {
 		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMounts, enforcer.DeviceMountCalls)
 	}
 
+	expectedDeviceUnmounts := 0
+	if enforcer.DeviceUnmountCalls != expectedDeviceUnmounts {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceUnmounts, enforcer.DeviceUnmountCalls)
+	}
+
+	expectedOverlay := 0
+	if enforcer.OverlayMountCalls != expectedOverlay {
+		t.Fatalf("expected %d attempts at overlay mount enforcement, got %d", expectedOverlay, enforcer.OverlayMountCalls)
+	}
+}
+
+func Test_Security_Policy_Enforcement_Unmount_Calls(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)
+	}
+
+	err = Unmount(context.Background(), 0, 0, target, false, 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)
+	}
+
+	expectedDeviceUnmounts := 1
+	if enforcer.DeviceUnmountCalls != expectedDeviceUnmounts {
+		t.Fatalf("expected %d attempt at pmem mount enforcement, got %d", expectedDeviceMounts, enforcer.DeviceUnmountCalls)
+	}
+
 	expectedOverlay := 0
 	if enforcer.OverlayMountCalls != expectedOverlay {
 		t.Fatalf("expected %d attempts at overlay mount enforcement, got %d", expectedOverlay, enforcer.OverlayMountCalls)

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

@@ -7,8 +7,9 @@ import (
 // For testing. Records the number of calls to each method so we can verify
 // the expected interactions took place.
 type MountMonitoringSecurityPolicyEnforcer struct {
-	DeviceMountCalls  int
-	OverlayMountCalls int
+	DeviceMountCalls   int
+	DeviceUnmountCalls int
+	OverlayMountCalls  int
 }
 
 var _ securitypolicy.SecurityPolicyEnforcer = (*MountMonitoringSecurityPolicyEnforcer)(nil)
@@ -18,6 +19,11 @@ func (p *MountMonitoringSecurityPolicyEnforcer) EnforceDeviceMountPolicy(target
 	return nil
 }
 
+func (p *MountMonitoringSecurityPolicyEnforcer) EnforceDeviceUnmountPolicy(target string) (err error) {
+	p.DeviceUnmountCalls++
+	return nil
+}
+
 func (p *MountMonitoringSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error) {
 	p.OverlayMountCalls++
 	return nil

pkg/securitypolicy/securitypolicy_test.go

@@ -122,9 +122,9 @@ func Test_StandardSecurityPolicyEnforcer_Devices_Initialization(t *testing.T) {
 	}
 }
 
-// Verify that StandardSecurityPolicyEnforcer.EnforcePmemMountPolicy will return
-// an error when there's no matching root hash in the policy
-func Test_EnforcePmemMountPolicy_No_Matches(t *testing.T) {
+// Verify that StandardSecurityPolicyEnforcer.EnforceDeviceMountPolicy will
+// return an error when there's no matching root hash in the policy
+func Test_EnforceDeviceMountPolicy_No_Matches(t *testing.T) {
 	f := func(p *generatedContainers) bool {
 		policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString)
 
@@ -165,6 +165,53 @@ func Test_EnforceDeviceMountPolicy_Matches(t *testing.T) {
 	}
 }
 
+func Test_EnforceDeviceUmountPolicy_Removes_Device_Entries(t *testing.T) {
+	f := func(p *generatedContainers) bool {
+		policy := NewStandardSecurityPolicyEnforcer(p.containers, ignoredEncodedPolicyString)
+
+		r := rand.New(rand.NewSource(time.Now().UnixNano()))
+		target := generateMountTarget(r)
+		rootHash := selectRootHashFromContainers(p, r)
+
+		err := policy.EnforceDeviceMountPolicy(target, rootHash)
+		if err != nil {
+			return false
+		}
+
+		// we set up an expected new data structure shape were
+		// the target has been removed, but everything else is
+		// the same
+		setupCorrectlyDone := false
+		expectedDevices := make([][]string, len(policy.Devices))
+		for i, container := range policy.Devices {
+			expectedDevices[i] = make([]string, len(container))
+			for j, storedTarget := range container {
+				if target == storedTarget {
+					setupCorrectlyDone = true
+				} else {
+					expectedDevices[i][j] = storedTarget
+				}
+			}
+		}
+		if !setupCorrectlyDone {
+			// somehow, setup failed. this should never happen without another test
+			// also failing
+			return false
+		}
+
+		err = policy.EnforceDeviceUnmountPolicy(target)
+		if err != nil {
+			return false
+		}
+
+		return cmp.Equal(policy.Devices, expectedDevices)
+	}
+
+	if err := quick.Check(f, &quick.Config{MaxCount: 1000}); err != nil {
+		t.Errorf("Test_EnforceDeviceUmountPolicy_Removes_Device_Entries failed: %v", err)
+	}
+}
+
 // Verify that StandardSecurityPolicyEnforcer.EnforceOverlayMountPolicy will
 // return an error when there's no matching overlay targets.
 func Test_EnforceOverlayMountPolicy_No_Matches(t *testing.T) {

pkg/securitypolicy/securitypolicyenforcer.go

@@ -12,6 +12,7 @@ import (
 
 type SecurityPolicyEnforcer interface {
 	EnforceDeviceMountPolicy(target string, deviceHash string) (err error)
+	EnforceDeviceUnmountPolicy(unmountTarget string) (err error)
 	EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error)
 	EnforceStartContainerPolicy(containerID string, argList []string, envList []string) (err error)
 }
@@ -38,7 +39,7 @@ type StandardSecurityPolicyEnforcer struct {
 	// map them back to a container definition from the user supplied
 	// SecurityPolicy
 	//
-	// Devices is a listing of dm-verity root hashes seen when mounting a device
+	// Devices is a listing of targets seen when mounting a device
 	// stored in a "per-container basis". As the UVM goes through its process of
 	// bringing up containers, we have to piece together information about what
 	// is going on.
@@ -50,7 +51,7 @@ type StandardSecurityPolicyEnforcer struct {
 	// in the supplied SecurityPolicy. Each "seen" layer is recorded in devices
 	// as it is mounted. So for example, if a root hash mount is found for the
 	// device being mounted and the first layer of the first container then we
-	// record the root hash in Devices[0][0].
+	// record the device target in Devices[0][0].
 	//
 	// Later, when overlay filesystems  created, we verify that the ordered layers
 	// for said overlay filesystem match one of the device orderings in Devices.
@@ -214,6 +215,21 @@ func (policyState *StandardSecurityPolicyEnforcer) EnforceDeviceMountPolicy(targ
 	return nil
 }
 
+func (policyState *StandardSecurityPolicyEnforcer) EnforceDeviceUnmountPolicy(unmountTarget string) (err error) {
+	policyState.mutex.Lock()
+	defer policyState.mutex.Unlock()
+
+	for _, container := range policyState.Devices {
+		for j, storedTarget := range container {
+			if unmountTarget == storedTarget {
+				container[j] = ""
+			}
+		}
+	}
+
+	return nil
+}
+
 func (policyState *StandardSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error) {
 	policyState.mutex.Lock()
 	defer policyState.mutex.Unlock()
@@ -409,6 +425,10 @@ func (p *OpenDoorSecurityPolicyEnforcer) EnforceDeviceMountPolicy(target string,
 	return nil
 }
 
+func (p *OpenDoorSecurityPolicyEnforcer) EnforceDeviceUnmountPolicy(target string) (err error) {
+	return nil
+}
+
 func (p *OpenDoorSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error) {
 	return nil
 }
@@ -425,6 +445,10 @@ func (p *ClosedDoorSecurityPolicyEnforcer) EnforceDeviceMountPolicy(target strin
 	return errors.New("mounting is denied by policy")
 }
 
+func (p *ClosedDoorSecurityPolicyEnforcer) EnforceDeviceUnmountPolicy(target string) (err error) {
+	return errors.New("unmounting is denied by policy")
+}
+
 func (p *ClosedDoorSecurityPolicyEnforcer) EnforceOverlayMountPolicy(containerID string, layerPaths []string) (err error) {
 	return errors.New("creating an overlay fs is denied by policy")
 }