Skip to content

Add various agent instructions for working with confidential containers.#2757

Closed
micromaomao wants to merge 3 commits into
microsoft:mainfrom
micromaomao:confidential-copilot-instructions
Closed

Add various agent instructions for working with confidential containers.#2757
micromaomao wants to merge 3 commits into
microsoft:mainfrom
micromaomao:confidential-copilot-instructions

Conversation

@micromaomao

@micromaomao micromaomao commented May 28, 2026

Copy link
Copy Markdown
Member

This was generated via a long prompt plus ingesting a bunch of previous PRs doing confidential fixes, then manually curated by me.

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

Test run:
Me:

Review a past PR 53f2486325624a81d2797b26294021c8647a6811 ("feature: block-device mounts (#2168)") for confidential security

Copilot:

Details

Confidential-security review of 53f2486 ("feature: block-device mounts")

The change touches confidential trust boundaries: uvm.go, workload_container.go, scsi.go, and the host‑facing LCOWMappedVirtualDisk.BlockDev field in resources.go. All inputs (mvd.BlockDev, OCI Mount.Destination, Mount.Source) come from the host. None of the new behaviour is added to the Rego policy / framework, and none of it is gated in Go either. Multiple host bypasses result.

Finding 1 — BlockDev bypasses dm‑crypt set by policy (high)

internal/guest/storage/scsi/scsi.go#L166-L178 (post‑patch):

// create and symlink block device mount target
if config.BlockDev {
    parent := filepath.Dir(target)
    if err := osMkdirAll(parent, 0700); err != nil { return err }
    ...
    return osSymlink(source, target)
}
...
if config.Encrypted { encryptedSource, err := encryptDevice(...) ... }

The BlockDev short‑circuit returns before the config.Encrypted branch. The Rego enforcement points mount_device / rw_mount_device (securitypolicyenforcer_rego.go ~L515/L532) take only target (and verity), and Config.Encrypted is decided on the GCS side from the policy's allow_unencrypted_scratch flag — they don't see BlockDev at all.

Attack: malicious host sends LCOWMappedVirtualDisk{ ..., BlockDev: true } for any SCSI mount the policy was willing to approve as encrypted scratch. The disk that the workload writes to (and that the policy author believes is dm‑crypt protected) is just a symlink to the raw /dev/sdX. The host trivially reads plaintext from the backing VHD.

(Verity is incidentally preserved only because the verity wrap is set up before the BlockDev check; encryption isn't.)

Finding 2 — blockdev:// destination prefix is invisible to the create‑container policy (high)

internal/guest/runtime/hcsv2/workload_container.go#L87-L124 runs inside setupWorkloadContainerSpec, which is invoked at uvm.go — before EnforceCreateContainerPolicy at uvm.go. The handler:

  1. Iterates spec.Mounts, matches Destination starting with blockdev://.
  2. os.Readlink(m.Source) + devices.DeviceFromPath(...) → appends an Allow:true LinuxDeviceCgroup with rwm (or r if ro) for that major/minor.
  3. spec.Mounts[i].Destination = strings.TrimPrefix(m.Destination, "blockdev://").

By the time policy runs it sees:

  • a plain bind‑mount with the prefix stripped (so destinations the policy already allows match), and
  • new entries in spec.Linux.Resources.Devices, which the Rego framework does not inspect.

Attack: for any container/mount destination the policy permits (say /data), the host sends OCI Mount{ Source: "/run/mounts/scsi/m<N>", Destination: "blockdev:///data" } plus a BlockDev=true SCSI MVD to create that symlink. After updateBlockDeviceMounts, the spec looks identical to a normal /data bind mount, the create‑container policy approves it, and the container is launched with rwm cgroup access to the raw block device. The policy author has no blockdev knob to refuse this.

Finding 3 — Device cgroup widening is not part of any enforcement point (medium, enabling Finding 2)

spec.Linux.Resources.Devices is mutated host‑drivenly before policy check, but no enforcement point covers it. Even if Finding 2's mount masquerade is fixed, the device cgroup pathway must be reasoned about because future policies that assume Linux.Resources.Devices reflects the manifest will be wrong here. The default LinuxDeviceCgroup permission rwm is also chosen on the GCS side from a host‑supplied OCI option string ("ro" vs anything else), not from policy.

Recommended fixes

...

Finding 1 is a bit overstated, but 2 and 3 are valid.

Hopefully this lets me 1 (or "few") shot creating an enforcement point for blockdev

@micromaomao
micromaomao requested a review from a team as a code owner May 28, 2026 23:38
@micromaomao
micromaomao force-pushed the confidential-copilot-instructions branch from cd3031e to e5d1fcf Compare May 28, 2026 23:41

@KenGordon KenGordon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated via a long prompt plus ingesting a bunch of previous PRs
doing confidential fixes, then manually curated by me.

Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
…t point or extend an existing one, and more enhancements

Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
@micromaomao
micromaomao force-pushed the confidential-copilot-instructions branch from ffcd391 to ab630d2 Compare May 29, 2026 13:49
@micromaomao
micromaomao marked this pull request as draft May 29, 2026 14:03
@micromaomao micromaomao closed this Jun 1, 2026
@micromaomao
micromaomao deleted the confidential-copilot-instructions branch June 1, 2026 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants