virtiofs: impersonate Windows users for host filesystem access by damanm24 · Pull Request #4876 · microsoft/openvmm

Daman Mulye (damanm24) · 2026-11-03T18:33:53Z

This PR adds per-user access to Windows shares through virtiofs. Host filesystem access currently uses the VM service process token, which may have different permissions from the user whose files are being shared.

Callers can provide a Windows token handle duplicated into the VM service process through the virtiofs RPC configuration. OpenVMM owns a duplicate and impersonates it when opening the share and handling each FUSE request, then reverts to the process identity afterward. The token is kept with the FUSE session. Omitting the token or passing zero preserves the existing behavior.

Invalid tokens fail device creation, and requests return an I/O error if impersonation fails rather than falling back to the process token. Windows regression tests check that successful and failing FUSE requests run under a restricted token and leave no thread token afterward.

github-actions · 2026-11-03T18:36:02Z

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

ImpersonationGuard is intended to be thread-bound, but PhantomData<*const ()> does not reliably… · New
Panicking in Drop is risky because if a panic is already in-flight, this will cause a… · New
HostFs validates root_path for empty/NUL, but SectionFs currently bypasses similar… · New

What changed in this PR

Adds Windows user-token impersonation support for virtio-fs host filesystem access, enabling per-user permissions rather than always using the VM service process token.

Changes:

Introduces VirtioFsSession wrapper to optionally impersonate a Windows user token per FUSE request.
Extends virtio-fs resource/config plumbing to carry optional Windows token handle and duplicate it into OpenVMM.
Refactors device construction to use a VirtioFsDeviceOptions struct and adds parsing/validation in the virtiofs resolver.

File	Description
vmm_tests/vmm_tests/tests/tests/ttrpc.rs	Updates virtio-fs test configs to populate the new `windows_options` field.
vm/devices/virtio/virtiofs/src/virtio.rs	Refactors device construction around options and routes requests through `VirtioFsSession` for optional impersonation.
vm/devices/virtio/virtiofs/src/session.rs	Adds a session wrapper that impersonates a Windows token during dispatch and returns EIO on impersonation failure.
vm/devices/virtio/virtiofs/src/resolver.rs	Adds structured parsing/validation and Windows-specific token duplication/impersonated open behavior.
vm/devices/virtio/virtiofs/src/lib.rs	Wires in new `impersonation` (Windows) and `session` modules.
vm/devices/virtio/virtiofs/src/integration_tests.rs	Updates integration test harness to use `VirtioFsDeviceOptions`.
vm/devices/virtio/virtiofs/src/impersonation.rs	Implements Windows token duplication and thread impersonation guard with tests.
vm/devices/virtio/virtiofs/Cargo.toml	Adds deps/features needed for impersonation + resolver cfg branching.
vm/devices/virtio/virtio_resources/src/lib.rs	Extends virtio-fs resources with Windows options payload (token handle).
petri/burette/src/tests/virtio_fs.rs	Updates test resource construction to include `windows_options`.
openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto	Adds `WindowsOptions` and token handle to virtio-fs RPC config.
openvmm/openvmm_entry/src/ttrpc/mod.rs	Plumbs `windows_options` from RPC into virtio resource handle and refactors virtio-fs builder.
openvmm/openvmm_entry/src/lib.rs	Updates CLI-based virtio-fs config to set `windows_options: None`.

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

The dispatch test does not cover impersonation failure; a Windows regression must verify -EIO, no filesystem invocation, and no residual thread token.

1 open issue

Panicking in Drop is risky because if a panic is already in-flight, this will cause a…

1 previously missed issue

In code that hasn't changed since last review

Add regression test for Windows impersonation failure

vm/devices/virtio/virtiofs/src/session.rs:51

Please add a Windows regression for impersonation failure, using an owned non-token handle, and assert that dispatch returns -EIO, never invokes the filesystem, and leaves no thread token. The added dispatch test only exercises a filesystem EACCES after impersonation succeeds, so it would still pass if this failure path regressed to dispatching with the process identity.

2 resolved since last review

_{Review effort: Balanced}

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open issues

RevertToSelfGuard is documented as thread-bound, but PhantomData<*const ()> does not make the… · New
build_virtio_fs previously validated tag/root_path and returned anyhow::Result, but now… · New

1 resolved since last review

Panicking in Drop is risky because if a panic is already in-flight, this will cause a…

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

Human assessment is needed because the Windows regression test does not exercise impersonation failure, leaving fail-closed behavior against dispatch under the process identity unverified.

No open issues 🎉

2 resolved since last review

RevertToSelfGuard is documented as thread-bound, but PhantomData<*const ()> does not make the…
build_virtio_fs previously validated tag/root_path and returned anyhow::Result, but now…

_{Review effort: Balanced}

github-actions · 2026-11-04T00:23:03Z

2 Petri tests failed (2 unstable)

Copilot

🟡 Changes recommended

Missing Windows regression tests leave impersonation-failure handling and token-scoped share opening unverified, including protection against process-identity fallback and thread-token cleanup.

2 open issues

_{Review effort: Balanced}

Duplicate the caller's token while resolving HostFs, retain it with the FUSE session, and impersonate it for each dispatch. Revert afterward and cover successful and failing dispatches with a restricted-token regression test. Split user impersonation from PR microsoft#4706 without DAX remapping or backend-option changes. Keep resource and protobuf field numbers compatible with the independent DAX branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Keep common HostFs parsing and resource resolution together, with token parsing and impersonation in one platform-specific implementation block. Preserve existing validation and token ownership, and cover rejection of Windows-only options on other hosts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Extract RevertToSelfGuard into pal::windows::security and reuse it for virtiofs requests and sparse-mmap privilege adjustment. Keep impersonation thread-bound and preserve fail-fast cleanup behavior. Share root-path validation between HostFs and SectionFs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

main replaced the cfg-if crate with the standard cfg_select! macro (microsoft#4883). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

Windows regression tests do not cover fail-closed dispatch when impersonation fails or verify the identity used to open the share, leaving security-sensitive behavior unprotected against regressions.

2 open issues

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The Windows filesystem identity boundary and cross-process handle transfer require human security review and Windows runtime testing to validate ACL enforcement, token lifetime, and impersonation cleanup.

1 open issue

Add regression coverage for impersonation failure

1 resolved since last review

Test share opening under the supplied token

_{Review effort: Balanced}

github-actions · 2026-11-05T03:24:24Z

31 Petri tests failed (30 unstable)

Copilot

🔵 Needs a closer look

Windows impersonation and client-supplied handle registration alter security and cross-process ownership boundaries, requiring security-owner review and native Windows validation beyond the available static inspection.

No open issues 🎉

1 resolved since last review

Add regression coverage for impersonation failure

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Windows impersonation and cross-process token transfer alter an access-control boundary, requiring human security review and native Windows validation that was unavailable.

No open issues 🎉

_{Review effort: Balanced}

Copilot AI balanced review requested due to automatic review settings November 3, 2026 18:33

github-actions Bot added the unsafe Related to unsafe code label Nov 3, 2026

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 3, 2026 18:47

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread vm/devices/virtio/virtiofs/src/impersonation.rs Resolved

Comment thread vm/devices/virtio/virtiofs/src/impersonation.rs Resolved

Comment thread vm/devices/virtio/virtiofs/src/resolver.rs Resolved

Copilot AI balanced review requested due to automatic review settings November 3, 2026 18:52

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 3, 2026 19:00

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings November 3, 2026 19:09

Daman Mulye (damanm24) marked this pull request as ready for review November 3, 2026 19:11

Daman Mulye (damanm24) requested review from a team and Anatol Belski (weltling) as code owners November 3, 2026 19:11

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 3, 2026 19:21

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread support/pal/src/windows/security.rs Resolved

Comment thread openvmm/openvmm_entry/src/ttrpc/mod.rs Resolved

Copilot AI balanced review requested due to automatic review settings November 3, 2026 20:50

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 3, 2026 20:50

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

John Starks (jstarks) reviewed Nov 4, 2026

View reviewed changes

Comment thread openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto Resolved

Copilot AI balanced review requested due to automatic review settings November 5, 2026 00:26

github-actions Bot added the Guide label Nov 5, 2026

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 5, 2026 00:27

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Comment thread vm/devices/virtio/virtiofs/src/resolver.rs Resolved

Comment thread vm/devices/virtio/virtiofs/src/session.rs Resolved

Daman Mulye and others added 7 commits November 4, 2026 16:39

virtiofs: fix safety comment placement in impersonation test

4edb883

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

align handle and fd passing

c529c19

virtiofs: replace cfg_if with cfg_select

a8f4d1b

main replaced the cfg-if crate with the standard cfg_select! macro (microsoft#4883). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add test for handle passing

5e20a63

Daman Mulye (damanm24) force-pushed the virtiofs-windows-impersonation branch from beababd to 5e20a63 Compare November 5, 2026 00:55

Copilot AI balanced review requested due to automatic review settings November 5, 2026 00:55

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 5, 2026 00:56

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

add more tests

44efd34

Copilot AI balanced review requested due to automatic review settings November 5, 2026 01:44

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 5, 2026 01:45

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Daman Mulye (damanm24) assigned John Starks (jstarks) Nov 5, 2026

Daman Mulye (damanm24) added the wsl label Nov 7, 2026

Daman Mulye (damanm24) requested a balanced review from Copilot November 12, 2026 23:33

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 12, 2026 23:36

View session

Copilot AI reviewed Nov 12, 2026

View reviewed changes

virtiofs: resolve merge conflicts with main

9202289

Copilot AI balanced review requested due to automatic review settings November 13, 2026 00:05

Copilot started reviewing on behalf of Daman Mulye (damanm24) November 13, 2026 00:06

View session

Copilot AI reviewed Nov 13, 2026

View reviewed changes

Conversation

Daman Mulye (damanm24) commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants