securitypolicy: Add enforcement point for blockdev mounts by micromaomao · Pull Request #2762 · microsoft/hcsshim

micromaomao · 2026-06-01T09:31:18Z

A "BlockDev" mount (as introduced by
#2168) is a special type of
MappedVirtualDisk mount request, in which the GCS just creates a symlink at the
requested target path pointing at the underlying /dev/sdX, and can later be
consumed by a container by bind-mounting that symlink in the OCI spec.

C-ACI does not currently use this feature, so the framework rejects both
mount_blockdev and unmount_blockdev by default. An allow all policy will
still let it through, which is useful for testing.

Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Tingmao Wang tingmaowang@microsoft.com

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR extends the security policy + GCS hardening model for confidential containers by adding new enforcement points (blockdev mount/unmount), introducing a revertable policy-metadata mechanism, and tightening runtime behavior around mounts/unmounts and request concurrency.

Changes:

Add mount_blockdev / unmount_blockdev enforcement points across API/framework rego, Go enforcer interfaces, and host-side mount flows.
Introduce “revertable sections” for policy metadata (Save/Restore) and use them to keep policy state consistent across mount/unmount failure paths.
Add host mount tracking + “mountsBroken” kill-switch, sequential bridge mode for SNP, and hostname / Plan9 share-name validation.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pkg/securitypolicy/version_framework	Bump embedded framework version.
pkg/securitypolicy/version_api	Bump embedded API version.
pkg/securitypolicy/securitypolicyenforcer_rego.go	Add blockdev enforcement, device input plumbing, and revertable section implementation for rego enforcer.
pkg/securitypolicy/securitypolicyenforcer.go	Extend enforcer interface/options for blockdev and revertable sections; add no-op handles for open/closed door.
pkg/securitypolicy/securitypolicy.go	Make embedded version parsing robust to trailing whitespace/newlines.
pkg/securitypolicy/regopolicy_linux_test.go	Add regression/property tests for revertable sections, overlay retry behavior, devices unsupported, and blockdev policy.
pkg/securitypolicy/rego_utils_test.go	Refactor overlay mount helper; add revertable-section test utilities; factor out container-spec creation helper.
pkg/securitypolicy/policy.rego	Wire new framework rules for blockdev enforcement points.
pkg/securitypolicy/open_door.rego	Allow blockdev mount/unmount in open-door policy.
pkg/securitypolicy/framework.rego	Default-deny blockdev; add `devices_ok` and reject linux devices; add related error strings.
pkg/securitypolicy/api.rego	Register new enforcement points and introduced versions for blockdev.
internal/regopolicyinterpreter/regopolicyinterpreter_test.go	Add tests for copying rego metadata and Save/Restore behavior.
internal/regopolicyinterpreter/regopolicyinterpreter.go	Add SavedMetadata, deep-copy support, Save/Restore APIs, and constants for metadata keys.
internal/guest/storage/scsi/scsi_test.go	Extend test to validate dm-crypt cleanup on mount failure.
internal/guest/storage/scsi/scsi.go	Ensure dm-crypt device is cleaned up on mount failure.
internal/guest/storage/plan9/plan9.go	Add Plan9 share-name validation helper.
internal/guest/storage/overlay/overlay.go	Update MountLayer comment to match behavior.
internal/guest/storage/mount.go	Log warning when unmount is requested for a non-existent path.
internal/guest/runtime/hcsv2/uvm_state_test.go	Update tests for new hostMounts API + add coverage for RO devices and overlay usage tracking.
internal/guest/runtime/hcsv2/uvm_state.go	Rework hostMounts tracking to include RO devices + overlays and usage counts; add explicit Lock/Unlock contract.
internal/guest/runtime/hcsv2/uvm.go	Add revertable policy sections around mount ops; introduce mountsBroken; add LinuxDevices to policy input; add overlay-in-use checks; centralize delete container state.
internal/guest/runtime/hcsv2/standalone_container.go	Validate hostname before writing to /etc/hosts.
internal/guest/runtime/hcsv2/sandbox_container.go	Validate hostname before writing to /etc/hosts.
internal/guest/runtime/hcsv2/process.go	Mark container terminated when init process exits.
internal/guest/runtime/hcsv2/container.go	Track container init termination state.
internal/guest/network/network_test.go	Add unit tests for hostname validation.
internal/guest/network/network.go	Add hostname validation (regex-based) to prevent injection via /etc/hosts generation.
internal/guest/bridge/bridge_v2.go	Route delete-container-state through Host API to centralize policy checks.
internal/guest/bridge/bridge.go	Add optional sequential request processing mode (with async exceptions) plus blockage warning.
internal/gcs/unrecoverable_error.go	Introduce UnrecoverableError handler (panic on non-SNP; infinite loop on SNP).
cmd/gcs/main.go	Enable sequential bridge processing automatically on SNP (confidential).

Comments suppressed due to low confidence (1)

internal/guest/storage/mount.go:134

Logging a warning for every unmount of a non-existent path can be noisy in normal flows where “remove if present” semantics are expected (the function already treats this as success). Consider lowering this to Debug/Trace, or gating it behind conditions that indicate a real anomaly, to avoid log spam in steady state.

	if _, err := osStat(target); err != nil {
		if os.IsNotExist(err) {
			log.G(ctx).WithField("target", target).Warnf("UnmountPath called for non-existent path")
			return nil
		}
		return errors.Wrapf(err, "failed to determine if path '%s' exists", target)
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Addresses the following comments: microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) Assisted-by: GitHub Copilot:claude-opus-4.7 copilot-review Signed-off-by: Tingmao Wang <m@maowtm.org>

A "BlockDev" mount (as introduced by microsoft#2168) is a special type of MappedVirtualDisk mount request, in which the GCS just creates a symlink at the requested target path pointing at the underlying `/dev/sdX`, and can later be consumed by a container by bind-mounting that symlink in the OCI spec. C-ACI does not currently use this feature, so the framework rejects both `mount_blockdev` and `unmount_blockdev` by default. An allow all policy will still let it through, which is useful for testing. Assisted-by: GitHub Copilot:claude-opus-4.7 Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

micromaomao · 2026-06-04T01:05:05Z

 		mountCtx, cancel := context.WithTimeout(ctx, time.Second*5)
 		defer cancel()
 		if mvd.MountPath != "" {
-			if mvd.ReadOnly {
+			if mvd.BlockDev {
+				if err = securityPolicy.EnforceMountBlockDevicePolicy(ctx, mvd.MountPath); err != nil {
+					return errors.Wrapf(err, "creating blockdev symlink at %s (-> scsi controller %d lun %d) denied by policy", mvd.MountPath, mvd.Controller, mvd.Lun)
+				}
+			} else if mvd.ReadOnly {


leaving as-is to match the ctx used for the enforcement point calls below this.

micromaomao · 2026-06-04T01:03:25Z

 enforcement_points := {
    "mount_device": {"introducedVersion": "0.1.0", "default_results": {"allowed": false}, "use_framework": false},
    "rw_mount_device": {"introducedVersion": "0.11.0", "default_results": {}, "use_framework": true},
+    "mount_blockdev": {"introducedVersion": "0.11.1", "default_results": {"allowed": false}, "use_framework": false},


I think the denial reason is separate from this and we should still get the framework-provided reason (untested)

    "create_container": {"introducedVersion": "0.1.0", "default_results": {"allowed": false, "env_list": null, "allow_stdio_access": false}, "use_framework": false},
    "unmount_device": {"introducedVersion": "0.2.0", "default_results": {"allowed": true}, "use_framework": false},
    "rw_unmount_device": {"introducedVersion": "0.11.0", "default_results": {}, "use_framework": true},
+    "unmount_blockdev": {"introducedVersion": "0.11.1", "default_results": {"allowed": false}, "use_framework": false},


micromaomao · 2026-06-04T01:05:54Z

@anmaxvl I've decoupled this from the MSRC PR - feel free to merge in either order

Addresses the following comments: microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) microsoft#2762 (comment) Assisted-by: GitHub Copilot:claude-opus-4.7 copilot-review Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>

micromaomao requested a review from a team as a code owner June 1, 2026 09:31

micromaomao requested review from KenGordon and Copilot June 1, 2026 09:32

micromaomao marked this pull request as draft June 1, 2026 09:32

Copilot AI reviewed Jun 1, 2026

View reviewed changes

KenGordon reviewed Jun 1, 2026

View reviewed changes

Comment thread cmd/gcs/main.go Outdated

msscotb assigned KenGordon and anmaxvl Jun 2, 2026

micromaomao force-pushed the tingmao_github/blockdev-enforcement branch from a8e3216 to 3a6b55b Compare June 4, 2026 00:44

micromaomao force-pushed the tingmao_github/blockdev-enforcement branch from 3a6b55b to 28149c9 Compare June 4, 2026 00:57

micromaomao requested a review from Copilot June 4, 2026 00:57

Copilot AI reviewed Jun 4, 2026

View reviewed changes

micromaomao marked this pull request as ready for review June 4, 2026 01:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

securitypolicy: Add enforcement point for blockdev mounts#2762

securitypolicy: Add enforcement point for blockdev mounts#2762
micromaomao wants to merge 1 commit into
microsoft:mainfrom
micromaomao:tingmao_github/blockdev-enforcement

micromaomao commented Jun 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

micromaomao Jun 4, 2026

Uh oh!

micromaomao Jun 4, 2026

Uh oh!

micromaomao commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

micromaomao commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

micromaomao Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

micromaomao Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

micromaomao commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

micromaomao commented Jun 1, 2026 •

edited

Loading