securitypolicy: Add enforcement point for blockdev mounts#2762
Conversation
There was a problem hiding this comment.
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_blockdevenforcement 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
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>
3a6b55b to
28149c9
Compare
| 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 { |
There was a problem hiding this comment.
leaving as-is to match the ctx used for the enforcement point calls below this.
| 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}, |
There was a problem hiding this comment.
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}, |
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>
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>
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>
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>
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>
28149c9 to
410806e
Compare
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>
410806e to
6ad1ccb
Compare
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>
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>
6ad1ccb to
4a2d964
Compare

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 beconsumed 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_blockdevandunmount_blockdevby default. An allow all policy willstill let it through, which is useful for testing.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Tingmao Wang tingmaowang@microsoft.com