From 772de2d956535dc21b5303d4fa5ce1b02d1232f1 Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Tue, 3 Nov 2026 09:25:29 -0800 Subject: [PATCH 1/8] virtiofs: impersonate Windows users for host filesystem access 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 #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> --- Cargo.lock | 2 + openvmm/openvmm_entry/src/lib.rs | 1 + openvmm/openvmm_entry/src/ttrpc/mod.rs | 36 +-- .../src/vmservice.proto | 10 + petri/burette/src/tests/virtio_fs.rs | 1 + vm/devices/virtio/virtio_resources/src/lib.rs | 7 + vm/devices/virtio/virtiofs/Cargo.toml | 4 + .../virtio/virtiofs/src/impersonation.rs | 300 ++++++++++++++++++ .../virtio/virtiofs/src/integration_tests.rs | 8 +- vm/devices/virtio/virtiofs/src/lib.rs | 3 + vm/devices/virtio/virtiofs/src/resolver.rs | 263 +++++++++++++-- vm/devices/virtio/virtiofs/src/session.rs | 67 ++++ vm/devices/virtio/virtiofs/src/virtio.rs | 90 +++--- vmm_tests/vmm_tests/tests/tests/ttrpc.rs | 3 + 14 files changed, 698 insertions(+), 97 deletions(-) create mode 100644 vm/devices/virtio/virtiofs/src/impersonation.rs create mode 100644 vm/devices/virtio/virtiofs/src/session.rs diff --git a/Cargo.lock b/Cargo.lock index d90c0daf797..2880922d7dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10620,6 +10620,7 @@ name = "virtiofs" version = "0.0.0" dependencies = [ "anyhow", + "cfg-if", "fuse", "futures", "guestmem", @@ -10635,6 +10636,7 @@ dependencies = [ "tempfile", "test_with_tracing", "thiserror 2.0.16", + "tracelimit", "tracing", "virtio", "virtio_resources", diff --git a/openvmm/openvmm_entry/src/lib.rs b/openvmm/openvmm_entry/src/lib.rs index 612dda575cc..713c00a8d57 100644 --- a/openvmm/openvmm_entry/src/lib.rs +++ b/openvmm/openvmm_entry/src/lib.rs @@ -1920,6 +1920,7 @@ async fn vm_config_from_command_line( fs: virtio_resources::fs::VirtioFsBackend::HostFs { root_path: args.path.clone(), mount_options: args.options.clone(), + windows_options: None, }, } .into_resource(); diff --git a/openvmm/openvmm_entry/src/ttrpc/mod.rs b/openvmm/openvmm_entry/src/ttrpc/mod.rs index 8224e1d8a70..c106fc4e2ed 100644 --- a/openvmm/openvmm_entry/src/ttrpc/mod.rs +++ b/openvmm/openvmm_entry/src/ttrpc/mod.rs @@ -1325,7 +1325,7 @@ impl VmService { } for virtiofs in devices_config.virtiofs_config { - let resource = build_virtio_fs(virtiofs)?.into_resource(); + let resource = build_virtio_fs(virtiofs).into_resource(); // Use VPCI when possible (currently only on Windows and macOS due // to KVM backend limitations). if cfg!(windows) || cfg!(target_os = "macos") { @@ -2804,39 +2804,20 @@ async fn build_virtio_device( virtio_resources::console::VirtioConsoleHandle { backend }.into_resource() } Kind::VhostUser(vhost_user) => build_vhost_user_device(vhost_user)?, - Kind::Fs(config) => build_virtio_fs(config)?.into_resource(), + Kind::Fs(config) => build_virtio_fs(config).into_resource(), }) } const OPENVMM_DEFAULT_VSOCK_GUEST_CID: u64 = 3; -fn build_virtio_fs( - config: vmservice::VirtioFs, -) -> anyhow::Result { +fn build_virtio_fs(config: vmservice::VirtioFs) -> virtio_resources::fs::VirtioFsHandle { let vmservice::VirtioFs { tag, root_path, read_only, + windows_options, } = config; - const VIRTIO_FS_TAG_LEN: usize = 36; - anyhow::ensure!(!tag.is_empty(), "virtio-fs tag must not be empty"); - anyhow::ensure!( - !tag.contains('\0'), - "virtio-fs tag must not contain NUL bytes" - ); - anyhow::ensure!( - tag.len() <= VIRTIO_FS_TAG_LEN, - "virtio-fs tag exceeds the {VIRTIO_FS_TAG_LEN}-byte protocol limit" - ); - anyhow::ensure!( - !root_path.is_empty(), - "virtio-fs root path must not be empty" - ); - anyhow::ensure!( - !root_path.contains('\0'), - "virtio-fs root path must not contain NUL bytes" - ); - Ok(virtio_resources::fs::VirtioFsHandle { + virtio_resources::fs::VirtioFsHandle { tag, fs: virtio_resources::fs::VirtioFsBackend::HostFs { root_path, @@ -2845,8 +2826,13 @@ fn build_virtio_fs( } else { String::new() }, + windows_options: windows_options.map(|options| { + virtio_resources::fs::VirtioFsWindowsOptions { + user_token_handle: options.user_token_handle, + } + }), }, - }) + } } /// Builds a disk backend resource from the proto `DiskBackend`. diff --git a/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto b/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto index 2e5ccc2d14a..088e68fec71 100644 --- a/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto +++ b/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto @@ -1083,6 +1083,16 @@ message VirtioFs { string tag = 1; string root_path = 2; bool read_only = 3; + // Host-specific virtio-fs options. Omit on non-Windows hosts. + WindowsOptions windows_options = 5; + + message WindowsOptions { + // A token handle duplicated into the VM service process. OpenVMM + // duplicates the token for host filesystem access. The caller may + // close the handle after this RPC succeeds. Zero uses the VM service + // process token. + uint64 user_token_handle = 1; + } } message VirtioConsoleConfig { diff --git a/petri/burette/src/tests/virtio_fs.rs b/petri/burette/src/tests/virtio_fs.rs index 3ed7b8cbc57..bd556d21b53 100644 --- a/petri/burette/src/tests/virtio_fs.rs +++ b/petri/burette/src/tests/virtio_fs.rs @@ -183,6 +183,7 @@ impl crate::harness::WarmPerfTest for VirtioFsTest { fs: virtio_resources::fs::VirtioFsBackend::HostFs { root_path: vfs_root_path, mount_options: String::new(), + windows_options: None, }, } .into_resource(), diff --git a/vm/devices/virtio/virtio_resources/src/lib.rs b/vm/devices/virtio/virtio_resources/src/lib.rs index 0b500dce4b3..0d4cbd09b12 100644 --- a/vm/devices/virtio/virtio_resources/src/lib.rs +++ b/vm/devices/virtio/virtio_resources/src/lib.rs @@ -51,11 +51,18 @@ pub mod fs { pub fs: VirtioFsBackend, } + #[derive(MeshPayload, Default)] + pub struct VirtioFsWindowsOptions { + pub user_token_handle: u64, + } + #[derive(MeshPayload)] pub enum VirtioFsBackend { HostFs { root_path: String, mount_options: String, + #[mesh(4)] + windows_options: Option, }, SectionFs { root_path: String, diff --git a/vm/devices/virtio/virtiofs/Cargo.toml b/vm/devices/virtio/virtiofs/Cargo.toml index b5dc84ed106..a493026f5af 100644 --- a/vm/devices/virtio/virtiofs/Cargo.toml +++ b/vm/devices/virtio/virtiofs/Cargo.toml @@ -19,8 +19,10 @@ inspect.workspace = true lx.workspace = true lxutil.workspace = true task_control.workspace = true +tracelimit.workspace = true anyhow.workspace = true +cfg-if.workspace = true futures.workspace = true pal_async.workspace = true parking_lot.workspace = true @@ -37,6 +39,8 @@ windows-sys = { workspace = true, features = [ "Wdk_System_SystemServices", "Win32_Foundation", "Win32_System_Memory", + "Win32_Security", + "Win32_System_Threading", ] } [lints] diff --git a/vm/devices/virtio/virtiofs/src/impersonation.rs b/vm/devices/virtio/virtiofs/src/impersonation.rs new file mode 100644 index 00000000000..4ebb3a27ef6 --- /dev/null +++ b/vm/devices/virtio/virtiofs/src/impersonation.rs @@ -0,0 +1,300 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Serving a file system under a user token on Windows. + +// UNSAFETY: Calling Win32 APIs and constructing owned handles from their outputs. +#![expect(unsafe_code)] + +use std::io; +use std::marker::PhantomData; +use std::os::windows::io::AsRawHandle; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::os::windows::io::RawHandle; +use std::ptr; +use windows_sys::Win32::Security; +use windows_sys::Win32::System::Threading::SetThreadToken; + +/// Creates an impersonation token, owned by this process, from a token handle +/// value duplicated into this process by the RPC caller. +pub fn duplicate_user_token(token: u64) -> io::Result { + let token = usize::try_from(token).map_err(|_| io::Error::from(io::ErrorKind::InvalidInput))?; + duplicate_impersonation_token(token as RawHandle) +} + +fn duplicate_impersonation_token(token: RawHandle) -> io::Result { + let mut duplicate = ptr::null_mut(); + // SAFETY: DuplicateTokenEx does not take ownership of `token`. On success, + // `duplicate` is a new handle owned by this process. + unsafe { + if Security::DuplicateTokenEx( + token, + Security::TOKEN_IMPERSONATE | Security::TOKEN_QUERY | Security::TOKEN_DUPLICATE, + ptr::null(), + Security::SecurityImpersonation, + Security::TokenImpersonation, + &mut duplicate, + ) == 0 + { + return Err(io::Error::last_os_error()); + } + + Ok(OwnedHandle::from_raw_handle(duplicate)) + } +} + +/// Impersonates a token on the current thread until dropped. +pub(crate) struct ImpersonationGuard { + _not_send: PhantomData<*const ()>, +} + +impl ImpersonationGuard { + pub fn new(token: &OwnedHandle) -> io::Result { + // SAFETY: A null thread selects the current thread, and `token` is a + // valid handle for the duration of the call. + if unsafe { SetThreadToken(ptr::null(), token.as_raw_handle()) } == 0 { + return Err(io::Error::last_os_error()); + } + + Ok(Self { + _not_send: PhantomData, + }) + } +} + +impl Drop for ImpersonationGuard { + fn drop(&mut self) { + // SAFETY: Calling Win32 API as documented. + if unsafe { Security::RevertToSelf() } == 0 { + panic!( + "failed to revert virtio-fs impersonation: {}", + io::Error::last_os_error() + ); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::session::VirtioFsSession; + use fuse::Fuse; + use fuse::ReplySender; + use fuse::Request; + use fuse::SessionInfo; + use fuse::protocol::*; + use std::sync::Arc; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + use test_with_tracing::test; + use windows_sys::Win32::Foundation::ERROR_NO_TOKEN; + use windows_sys::Win32::System::Threading::GetCurrentProcess; + use windows_sys::Win32::System::Threading::GetCurrentThread; + use windows_sys::Win32::System::Threading::OpenProcessToken; + use windows_sys::Win32::System::Threading::OpenThreadToken; + use zerocopy::FromBytes; + use zerocopy::FromZeros; + use zerocopy::IntoBytes; + + fn process_token() -> OwnedHandle { + let mut process_token = ptr::null_mut(); + let mut token = ptr::null_mut(); + // SAFETY: Calling Win32 APIs as documented. + unsafe { + assert_ne!( + OpenProcessToken( + GetCurrentProcess(), + Security::TOKEN_DUPLICATE, + &mut process_token + ), + 0 + ); + let process_token = OwnedHandle::from_raw_handle(process_token); + assert_ne!( + Security::DuplicateTokenEx( + process_token.as_raw_handle(), + Security::TOKEN_IMPERSONATE | Security::TOKEN_QUERY | Security::TOKEN_DUPLICATE, + ptr::null(), + Security::SecurityImpersonation, + Security::TokenImpersonation, + &mut token, + ), + 0 + ); + OwnedHandle::from_raw_handle(token) + } + } + + fn thread_token() -> io::Result { + let mut token = ptr::null_mut(); + // SAFETY: Calling Win32 API as documented. + unsafe { + if OpenThreadToken(GetCurrentThread(), Security::TOKEN_QUERY, 1, &mut token) == 0 { + return Err(io::Error::last_os_error()); + } + Ok(OwnedHandle::from_raw_handle(token)) + } + } + + #[test] + fn rejects_non_token_handle() { + duplicate_user_token(0).unwrap_err(); + } + + #[test] + fn impersonates_until_dropped() { + let process_token = process_token(); + let token_handle = process_token.as_raw_handle() as usize as u64; + let token = duplicate_user_token(token_handle).unwrap(); + { + let _guard = ImpersonationGuard::new(&token).unwrap(); + thread_token().unwrap(); + } + + let err = thread_token().unwrap_err(); + assert_eq!(err.raw_os_error(), Some(ERROR_NO_TOKEN as i32)); + } + + fn restricted_token() -> OwnedHandle { + let token = process_token(); + let mut world_sid = [0u32; Security::SECURITY_MAX_SID_SIZE as usize / size_of::()]; + let mut sid_size = size_of_val(&world_sid) as u32; + let mut restricted = ptr::null_mut(); + // SAFETY: The SID buffer is aligned and large enough for a well-known + // SID. All inputs remain valid until the new token handle is returned. + unsafe { + assert_ne!( + Security::CreateWellKnownSid( + Security::WinWorldSid, + ptr::null_mut(), + world_sid.as_mut_ptr().cast(), + &mut sid_size, + ), + 0 + ); + let sid = Security::SID_AND_ATTRIBUTES { + Sid: world_sid.as_mut_ptr().cast(), + Attributes: 0, + }; + assert_ne!( + Security::CreateRestrictedToken( + token.as_raw_handle(), + Security::DISABLE_MAX_PRIVILEGE, + 0, + ptr::null(), + 0, + ptr::null(), + 1, + &sid, + &mut restricted, + ), + 0 + ); + OwnedHandle::from_raw_handle(restricted) + } + } + + fn token_id(token: &OwnedHandle) -> (u32, i32) { + let mut stats = Security::TOKEN_STATISTICS::default(); + let mut size = 0; + // SAFETY: The token is valid and the output buffer matches the queried + // information class. + unsafe { + assert_ne!( + Security::GetTokenInformation( + token.as_raw_handle(), + Security::TokenStatistics, + ptr::from_mut(&mut stats).cast(), + size_of_val(&stats) as u32, + &mut size, + ), + 0 + ); + } + (stats.TokenId.LowPart, stats.TokenId.HighPart) + } + + struct TokenCheckingFs { + token_id: (u32, i32), + calls: Arc, + } + + impl TokenCheckingFs { + fn check_identity(&self) { + let token = thread_token().unwrap(); + assert_eq!(token_id(&token), self.token_id); + // SAFETY: The handle refers to a valid token. + assert_ne!( + unsafe { Security::IsTokenRestricted(token.as_raw_handle()) }, + 0 + ); + self.calls.fetch_add(1, Ordering::Relaxed); + } + } + + impl Fuse for TokenCheckingFs { + fn init(&self, _info: &mut SessionInfo) { + self.check_identity(); + } + + fn get_attr(&self, _request: &Request, _flags: u32, _fh: u64) -> lx::Result { + self.check_identity(); + Err(lx::Error::EACCES) + } + } + + #[derive(Default)] + struct TestSender(Vec); + + impl ReplySender for TestSender { + fn send(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result<()> { + self.0 = bufs.iter().flat_map(|buf| buf.iter().copied()).collect(); + Ok(()) + } + } + + #[test] + fn session_dispatch_impersonates_restricted_token_and_reverts() { + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + let token = restricted_token(); + let calls = Arc::new(AtomicUsize::new(0)); + let session = VirtioFsSession::new(TokenCheckingFs { + token_id: token_id(&token), + calls: calls.clone(), + }) + .with_user_token(Some(token)); + let init = fuse_init_in { + major: FUSE_KERNEL_VERSION, + minor: FUSE_KERNEL_MINOR_VERSION, + ..FromZeros::new_zeroed() + }; + let getattr = fuse_getattr_in::new_zeroed(); + for (opcode, args, expected_error) in [ + (FUSE_INIT, init.as_bytes(), 0), + (FUSE_GETATTR, getattr.as_bytes(), -lx::Error::EACCES.value()), + ] { + let header = fuse_in_header { + len: (size_of::() + args.len()) as u32, + opcode, + unique: 1, + nodeid: 1, + ..FromZeros::new_zeroed() + }; + let mut request = header.as_bytes().to_vec(); + request.extend_from_slice(args); + let mut sender = TestSender::default(); + session.dispatch(Request::new(request.as_slice()).unwrap(), &mut sender, None); + let (reply, _) = fuse_out_header::read_from_prefix(&sender.0).unwrap(); + assert_eq!(reply.error, expected_error); + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + } + assert_eq!(calls.load(Ordering::Relaxed), 2); + } +} diff --git a/vm/devices/virtio/virtiofs/src/integration_tests.rs b/vm/devices/virtio/virtiofs/src/integration_tests.rs index fdfc3b19c9e..96314d69926 100644 --- a/vm/devices/virtio/virtiofs/src/integration_tests.rs +++ b/vm/devices/virtio/virtiofs/src/integration_tests.rs @@ -9,6 +9,7 @@ use crate::VirtioFs; use crate::virtio::VirtioFsDevice; +use crate::virtio::VirtioFsDeviceOptions; use fuse::protocol::*; use guestmem::GuestMemory; use pal_async::DefaultDriver; @@ -72,7 +73,12 @@ impl TestHarness { let fs = VirtioFs::new(tmpdir.path(), None).unwrap(); let driver_source = VmTaskDriverSource::new(SingleDriverBackend::new(driver.clone())); - let device = VirtioFsDevice::new(&driver_source, "testfs", fs, 0, None); + let device = VirtioFsDevice::new( + &driver_source, + "testfs", + fs, + VirtioFsDeviceOptions::default(), + ); let queue_event = Event::new(); let interrupt_event = Event::new(); diff --git a/vm/devices/virtio/virtiofs/src/lib.rs b/vm/devices/virtio/virtiofs/src/lib.rs index fb7c3d559aa..ea17b642e38 100644 --- a/vm/devices/virtio/virtiofs/src/lib.rs +++ b/vm/devices/virtio/virtiofs/src/lib.rs @@ -6,12 +6,15 @@ mod aggregate; mod file; +#[cfg(windows)] +mod impersonation; mod inode; #[cfg(test)] mod integration_tests; pub mod resolver; #[cfg(windows)] mod section; +mod session; #[cfg(test)] mod test_helpers; mod util; diff --git a/vm/devices/virtio/virtiofs/src/resolver.rs b/vm/devices/virtio/virtiofs/src/resolver.rs index 797226b63bb..a44aa572363 100644 --- a/vm/devices/virtio/virtiofs/src/resolver.rs +++ b/vm/devices/virtio/virtiofs/src/resolver.rs @@ -5,16 +5,186 @@ use crate::VirtioFs; use crate::virtio::VirtioFsDevice; +use crate::virtio::VirtioFsDeviceOptions; use anyhow::Context as _; +use cfg_if::cfg_if; use lxutil::LxVolumeOptions; use virtio::resolve::ResolvedVirtioDevice; use virtio::resolve::VirtioResolveInput; +use virtio_resources::fs::VirtioFsAggregateChild; use virtio_resources::fs::VirtioFsBackend; use virtio_resources::fs::VirtioFsHandle; +use virtio_resources::fs::VirtioFsWindowsOptions; use vm_resource::ResolveResource; use vm_resource::declare_static_resolver; use vm_resource::kind::VirtioDeviceHandle; +cfg_if! { + if #[cfg(windows)] { + use crate::impersonation::duplicate_user_token; + use std::os::windows::io::OwnedHandle; + } +} + +const VIRTIO_FS_TAG_LEN: usize = 36; +#[cfg(windows)] +const SECTION_FS_SHARED_MEMORY_SIZE: u64 = 8 * 1024 * 1024 * 1024; + +struct ParsedVirtioFs { + tag: String, + backend: ParsedVirtioFsBackend, +} + +enum ParsedVirtioFsBackend { + HostFs { + root_path: String, + mount_options: LxVolumeOptions, + platform: ParsedHostFsPlatform, + }, + #[cfg(windows)] + SectionFs { root_path: String }, + Aggregate { + children: Vec, + }, +} + +cfg_if! { + if #[cfg(windows)] { + struct ParsedHostFsPlatform { + user_token: Option, + } + + impl ParsedHostFsPlatform { + fn open_fs( + &self, + root_path: &str, + mount_options: &LxVolumeOptions, + ) -> anyhow::Result { + let _impersonation = self + .user_token + .as_ref() + .map(crate::impersonation::ImpersonationGuard::new) + .transpose() + .context("failed to impersonate virtio-fs user token")?; + Ok(VirtioFs::new(root_path, Some(mount_options))?) + } + + fn device_options(self) -> VirtioFsDeviceOptions { + VirtioFsDeviceOptions { + user_token: self.user_token, + ..Default::default() + } + } + } + } else { + struct ParsedHostFsPlatform; + + impl ParsedHostFsPlatform { + fn open_fs( + &self, + root_path: &str, + mount_options: &LxVolumeOptions, + ) -> anyhow::Result { + Ok(VirtioFs::new(root_path, Some(mount_options))?) + } + + fn device_options(self) -> VirtioFsDeviceOptions { + Default::default() + } + } + } +} + +impl ParsedVirtioFs { + fn parse(resource: VirtioFsHandle) -> anyhow::Result { + let VirtioFsHandle { tag, fs } = resource; + anyhow::ensure!(!tag.is_empty(), "virtio-fs tag must not be empty"); + anyhow::ensure!( + !tag.contains('\0'), + "virtio-fs tag must not contain NUL bytes" + ); + anyhow::ensure!( + tag.len() <= VIRTIO_FS_TAG_LEN, + "virtio-fs tag exceeds the {VIRTIO_FS_TAG_LEN}-byte protocol limit" + ); + + let backend = match fs { + VirtioFsBackend::HostFs { + root_path, + mount_options, + windows_options, + } => { + anyhow::ensure!( + !root_path.is_empty(), + "virtio-fs root path must not be empty" + ); + anyhow::ensure!( + !root_path.contains('\0'), + "virtio-fs root path must not contain NUL bytes" + ); + + return parse_host_fs(root_path, mount_options, windows_options) + .map(|backend| Self { tag, backend }); + } + #[cfg(windows)] + VirtioFsBackend::SectionFs { root_path } => { + ParsedVirtioFsBackend::SectionFs { root_path } + } + #[cfg(not(windows))] + VirtioFsBackend::SectionFs { .. } => { + anyhow::bail!("section fs not supported on this platform") + } + VirtioFsBackend::Aggregate { children } => { + ParsedVirtioFsBackend::Aggregate { children } + } + }; + + Ok(Self { tag, backend }) + } +} + +cfg_if! { + if #[cfg(windows)] { + fn parse_host_fs( + root_path: String, + mount_options: String, + windows_options: Option, + ) -> anyhow::Result { + let user_token = windows_options + .as_ref() + .map(|options| options.user_token_handle) + .filter(|handle| *handle != 0) + .map(duplicate_user_token) + .transpose() + .context("invalid virtio-fs user token")?; + + Ok(ParsedVirtioFsBackend::HostFs { + root_path, + mount_options: LxVolumeOptions::from_option_string(&mount_options), + platform: ParsedHostFsPlatform { user_token }, + }) + } + } else { + fn parse_host_fs( + root_path: String, + mount_options: String, + windows_options: Option, + ) -> anyhow::Result { + if let Some(options) = windows_options { + anyhow::ensure!( + options.user_token_handle == 0, + "virtio-fs windows_options are only supported on Windows" + ); + } + Ok(ParsedVirtioFsBackend::HostFs { + root_path, + mount_options: LxVolumeOptions::from_option_string(&mount_options), + platform: ParsedHostFsPlatform, + }) + } + } +} + /// Resolver for virtiofs devices. pub struct VirtioFsResolver; @@ -32,35 +202,29 @@ impl ResolveResource for VirtioFsResolver { resource: VirtioFsHandle, input: VirtioResolveInput<'_>, ) -> Result { - let device = match &resource.fs { - VirtioFsBackend::HostFs { + let ParsedVirtioFs { tag, backend } = ParsedVirtioFs::parse(resource)?; + let device = match backend { + ParsedVirtioFsBackend::HostFs { root_path, mount_options, + platform, } => VirtioFsDevice::new( input.driver_source, - &resource.tag, - VirtioFs::new( - root_path, - Some(&LxVolumeOptions::from_option_string(mount_options)), - )?, - 0, - None, + &tag, + platform.open_fs(&root_path, &mount_options)?, + platform.device_options(), ), #[cfg(windows)] - VirtioFsBackend::SectionFs { root_path } => { - VirtioFsDevice::new( - input.driver_source, - &resource.tag, - crate::SectionFs::new(root_path)?, - 8 * 1024 * 1024 * 1024, // 8GB of shared memory, - None, - ) - } - #[cfg(not(windows))] - VirtioFsBackend::SectionFs { .. } => { - anyhow::bail!("section fs not supported on this platform") - } - VirtioFsBackend::Aggregate { children } => { + ParsedVirtioFsBackend::SectionFs { root_path } => VirtioFsDevice::new( + input.driver_source, + &tag, + crate::SectionFs::new(&root_path)?, + VirtioFsDeviceOptions { + shared_memory_size: SECTION_FS_SHARED_MEMORY_SIZE, + ..Default::default() + }, + ), + ParsedVirtioFsBackend::Aggregate { children } => { let fs = VirtioFs::new_aggregate(); for child in children { fs.add_child( @@ -75,9 +239,60 @@ impl ResolveResource for VirtioFsResolver { ) })?; } - VirtioFsDevice::new(input.driver_source, &resource.tag, fs, 0, None) + VirtioFsDevice::new(input.driver_source, &tag, fs, Default::default()) } }; Ok(device.into()) } } + +#[cfg(test)] +mod tests { + use super::ParsedVirtioFs; + use super::ParsedVirtioFsBackend; + use test_with_tracing::test; + use virtio_resources::fs::VirtioFsBackend; + use virtio_resources::fs::VirtioFsHandle; + + fn host_fs(tag: &str, root_path: &str, mount_options: &str) -> VirtioFsHandle { + VirtioFsHandle { + tag: tag.to_owned(), + fs: VirtioFsBackend::HostFs { + root_path: root_path.to_owned(), + mount_options: mount_options.to_owned(), + windows_options: None, + }, + } + } + + #[test] + fn parse_rejects_invalid_tag_and_root_path() { + assert!(ParsedVirtioFs::parse(host_fs("", "root", "")).is_err()); + assert!(ParsedVirtioFs::parse(host_fs("tag", "", "")).is_err()); + assert!(ParsedVirtioFs::parse(host_fs("tag", "root\0path", "")).is_err()); + } + + #[test] + fn parse_preserves_read_only_mount_option() { + let parsed = ParsedVirtioFs::parse(host_fs("tag", "root", "uid=1000;ro")) + .expect("parse virtio-fs resource"); + let ParsedVirtioFsBackend::HostFs { mount_options, .. } = parsed.backend else { + panic!("expected host filesystem backend"); + }; + assert!(mount_options.is_readonly()); + } + + #[cfg(not(windows))] + #[test] + fn parse_accepts_empty_windows_options_on_non_windows() { + let mut resource = host_fs("tag", "root", ""); + let VirtioFsBackend::HostFs { + windows_options, .. + } = &mut resource.fs + else { + unreachable!(); + }; + *windows_options = Some(Default::default()); + assert!(ParsedVirtioFs::parse(resource).is_ok()); + } +} diff --git a/vm/devices/virtio/virtiofs/src/session.rs b/vm/devices/virtio/virtiofs/src/session.rs new file mode 100644 index 00000000000..65d96f6e25c --- /dev/null +++ b/vm/devices/virtio/virtiofs/src/session.rs @@ -0,0 +1,67 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use fuse::Mapper; +use fuse::ReplySender; +use fuse::Request; + +pub(crate) struct VirtioFsSession { + session: fuse::Session, + #[cfg(windows)] + user_token: Option, +} + +impl VirtioFsSession { + pub(crate) fn new(fs: Fs) -> Self { + Self { + session: fuse::Session::new(fs), + #[cfg(windows)] + user_token: None, + } + } + + #[cfg(windows)] + pub(crate) fn with_user_token( + mut self, + token: Option, + ) -> Self { + self.user_token = token; + self + } + + pub(crate) fn dispatch( + &self, + request: Request, + sender: &mut impl ReplySender, + mapper: Option<&dyn Mapper>, + ) { + #[cfg(windows)] + let _impersonation = match self + .user_token + .as_ref() + .map(crate::impersonation::ImpersonationGuard::new) + .transpose() + { + Ok(guard) => guard, + Err(e) => { + tracelimit::error_ratelimited!( + error = &e as &dyn std::error::Error, + "failed to impersonate user token" + ); + if let Err(e) = sender.send_error(request.unique(), lx::Error::EIO.value()) { + tracelimit::error_ratelimited!( + error = &e as &dyn std::error::Error, + "failed to send error reply" + ); + } + return; + } + }; + + self.session.dispatch(request, sender, mapper); + } + + pub(crate) fn destroy(&self) { + self.session.destroy(); + } +} diff --git a/vm/devices/virtio/virtiofs/src/virtio.rs b/vm/devices/virtio/virtiofs/src/virtio.rs index a66fbe4efff..296e6ca4272 100644 --- a/vm/devices/virtio/virtiofs/src/virtio.rs +++ b/vm/devices/virtio/virtiofs/src/virtio.rs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +use crate::session::VirtioFsSession; use crate::virtio_util::VirtioPayloadReader; use crate::virtio_util::VirtioPayloadWriter; use anyhow::Context as _; @@ -38,8 +39,8 @@ use zerocopy::KnownLayout; /// /// Callers that know the appropriate concurrency for their environment /// (e.g., guest vCPU count for an in-VMM device, or host parallelism for -/// a host service like `wsldevicehost`) should pass an explicit value via -/// [`VirtioFsDevice::with_num_request_queues`]. +/// a host service like `wsldevicehost`) can set it in +/// [`VirtioFsDeviceOptions::num_request_queues`]. const DEFAULT_NUM_REQUEST_QUEUES: u32 = 2; /// Upper bound for the request queue count. Past this, virtio-fs's @@ -55,6 +56,21 @@ struct VirtioFsDeviceConfig { num_request_queues: u32, } +/// Optional configuration for a [`VirtioFsDevice`]. +#[derive(Default)] +pub struct VirtioFsDeviceOptions { + /// Shared-memory size exposed by the device. + pub shared_memory_size: u64, + /// Callback invoked when a guest corrupts a FUSE request. + pub notify_corruption: Option>, + /// Number of FUSE request queues. Defaults to two and is clamped to the + /// device-supported range. + pub num_request_queues: Option, + /// Token used for host filesystem access. `None` uses the process token. + #[cfg(windows)] + pub user_token: Option, +} + /// A virtio-fs PCI device. #[derive(InspectMut)] pub struct VirtioFsDevice { @@ -63,7 +79,7 @@ pub struct VirtioFsDevice { #[inspect(skip)] config: VirtioFsDeviceConfig, #[inspect(skip)] - fs: Arc, + fs: Arc, #[inspect(skip)] workers: Vec>, shmem_size: u64, @@ -75,58 +91,34 @@ pub struct VirtioFsDevice { } impl VirtioFsDevice { - /// Creates a new `VirtioFsDevice` with the specified mount tag. - /// - /// The number of FUSE request queues defaults to - /// `DEFAULT_NUM_REQUEST_QUEUES`. Callers that know the appropriate - /// concurrency for their environment (e.g., guest vCPU count for an - /// in-VMM device, or host parallelism for a host service) should use - /// [`Self::with_num_request_queues`] instead. + /// Creates a new `VirtioFsDevice` with the specified mount tag and options. pub fn new( driver_source: &VmTaskDriverSource, tag: &str, fs: Fs, - shmem_size: u64, - notify_corruption: Option>, + options: VirtioFsDeviceOptions, ) -> Self where Fs: 'static + fuse::Fuse + Send + Sync, { - Self::with_num_request_queues( - driver_source, - tag, - fs, - shmem_size, - notify_corruption, - DEFAULT_NUM_REQUEST_QUEUES, - ) - } - - /// Creates a new `VirtioFsDevice` with an explicit number of FUSE - /// request queues. The value is clamped to `[1, MAX_REQUEST_QUEUES]`. - pub fn with_num_request_queues( - driver_source: &VmTaskDriverSource, - tag: &str, - fs: Fs, - shmem_size: u64, - notify_corruption: Option>, - num_request_queues: u32, - ) -> Self - where - Fs: 'static + fuse::Fuse + Send + Sync, - { - let num_request_queues = num_request_queues.clamp(1, MAX_REQUEST_QUEUES); - + let num_request_queues = options + .num_request_queues + .unwrap_or(DEFAULT_NUM_REQUEST_QUEUES) + .clamp(1, MAX_REQUEST_QUEUES); let mut config = VirtioFsDeviceConfig { tag: [0; 36], num_request_queues, }; - let notify_corruption = if let Some(notify) = notify_corruption { + let notify_corruption = if let Some(notify) = options.notify_corruption { notify } else { Arc::new(|| {}) }; + #[cfg(windows)] + let fs = VirtioFsSession::new(fs).with_user_token(options.user_token); + #[cfg(not(windows))] + let fs = VirtioFsSession::new(fs); // Copy the tag into the config space (truncate it for now if too long). let length = std::cmp::min(tag.len(), config.tag.len()); @@ -136,9 +128,9 @@ impl VirtioFsDevice { task_name: format!("virtiofs-{}", tag).into(), driver: driver_source.simple(), config, - fs: Arc::new(fuse::Session::new(fs)), + fs: Arc::new(fs), workers: Vec::new(), - shmem_size, + shmem_size: options.shared_memory_size, shared_memory_region: None, notify_corruption, num_request_queues, @@ -266,7 +258,7 @@ impl VirtioDevice for VirtioFsDevice { } struct VirtioFsWorker { - fs: Arc, + fs: Arc, shared_memory_region: Option>, shared_memory_size: u64, notify_corruption: Arc, @@ -336,6 +328,7 @@ fn process_virtiofs_request( mem, bytes_written: 0, }; + let mapper = worker .shared_memory_region .as_ref() @@ -430,12 +423,15 @@ mod tests { let tmpdir = tempfile::tempdir().unwrap(); let fs = VirtioFs::new(tmpdir.path(), None).unwrap(); let driver_source = VmTaskDriverSource::new(SingleDriverBackend::new(driver.clone())); - let device = match num_request_queues { - Some(n) => { - VirtioFsDevice::with_num_request_queues(&driver_source, "testfs", fs, 0, None, n) - } - None => VirtioFsDevice::new(&driver_source, "testfs", fs, 0, None), - }; + let device = VirtioFsDevice::new( + &driver_source, + "testfs", + fs, + VirtioFsDeviceOptions { + num_request_queues, + ..Default::default() + }, + ); (device, tmpdir) } diff --git a/vmm_tests/vmm_tests/tests/tests/ttrpc.rs b/vmm_tests/vmm_tests/tests/tests/ttrpc.rs index eef2a977ba4..7e730024fc2 100644 --- a/vmm_tests/vmm_tests/tests/tests/ttrpc.rs +++ b/vmm_tests/vmm_tests/tests/tests/ttrpc.rs @@ -1093,6 +1093,7 @@ async fn test_ttrpc_interface( tag: "testfs".to_string(), root_path: virtiofs_root.to_string_lossy().into(), read_only: i == 0, + windows_options: None, }], // A SCSI controller keeps a request channel // alive for the lifetime of the VM, which used @@ -1273,6 +1274,7 @@ async fn test_ttrpc_interface( tag: "hotplugfs".to_string(), root_path: hotplug_virtiofs_root.to_string_lossy().into(), read_only: true, + windows_options: None, }, ))), }, @@ -1956,6 +1958,7 @@ fn virtio_fs_vpci_request( tag: tag.to_string(), root_path: root_path.to_string_lossy().into_owned(), read_only, + windows_options: None, }, ))), } From 493a38c6ed75fa594ae634387fcf9e222144fd3e Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Tue, 3 Nov 2026 10:47:15 -0800 Subject: [PATCH 2/8] virtiofs: consolidate resolver platform handling 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> --- vm/devices/virtio/virtiofs/src/resolver.rs | 209 +++++++++++---------- 1 file changed, 111 insertions(+), 98 deletions(-) diff --git a/vm/devices/virtio/virtiofs/src/resolver.rs b/vm/devices/virtio/virtiofs/src/resolver.rs index a44aa572363..c1615b23d6f 100644 --- a/vm/devices/virtio/virtiofs/src/resolver.rs +++ b/vm/devices/virtio/virtiofs/src/resolver.rs @@ -19,13 +19,6 @@ use vm_resource::ResolveResource; use vm_resource::declare_static_resolver; use vm_resource::kind::VirtioDeviceHandle; -cfg_if! { - if #[cfg(windows)] { - use crate::impersonation::duplicate_user_token; - use std::os::windows::io::OwnedHandle; - } -} - const VIRTIO_FS_TAG_LEN: usize = 36; #[cfg(windows)] const SECTION_FS_SHARED_MEMORY_SIZE: u64 = 8 * 1024 * 1024 * 1024; @@ -48,53 +41,6 @@ enum ParsedVirtioFsBackend { }, } -cfg_if! { - if #[cfg(windows)] { - struct ParsedHostFsPlatform { - user_token: Option, - } - - impl ParsedHostFsPlatform { - fn open_fs( - &self, - root_path: &str, - mount_options: &LxVolumeOptions, - ) -> anyhow::Result { - let _impersonation = self - .user_token - .as_ref() - .map(crate::impersonation::ImpersonationGuard::new) - .transpose() - .context("failed to impersonate virtio-fs user token")?; - Ok(VirtioFs::new(root_path, Some(mount_options))?) - } - - fn device_options(self) -> VirtioFsDeviceOptions { - VirtioFsDeviceOptions { - user_token: self.user_token, - ..Default::default() - } - } - } - } else { - struct ParsedHostFsPlatform; - - impl ParsedHostFsPlatform { - fn open_fs( - &self, - root_path: &str, - mount_options: &LxVolumeOptions, - ) -> anyhow::Result { - Ok(VirtioFs::new(root_path, Some(mount_options))?) - } - - fn device_options(self) -> VirtioFsDeviceOptions { - Default::default() - } - } - } -} - impl ParsedVirtioFs { fn parse(resource: VirtioFsHandle) -> anyhow::Result { let VirtioFsHandle { tag, fs } = resource; @@ -123,8 +69,12 @@ impl ParsedVirtioFs { "virtio-fs root path must not contain NUL bytes" ); - return parse_host_fs(root_path, mount_options, windows_options) - .map(|backend| Self { tag, backend }); + let platform = ParsedHostFsPlatform::parse(windows_options)?; + ParsedVirtioFsBackend::HostFs { + root_path, + mount_options: LxVolumeOptions::from_option_string(&mount_options), + platform, + } } #[cfg(windows)] VirtioFsBackend::SectionFs { root_path } => { @@ -143,48 +93,6 @@ impl ParsedVirtioFs { } } -cfg_if! { - if #[cfg(windows)] { - fn parse_host_fs( - root_path: String, - mount_options: String, - windows_options: Option, - ) -> anyhow::Result { - let user_token = windows_options - .as_ref() - .map(|options| options.user_token_handle) - .filter(|handle| *handle != 0) - .map(duplicate_user_token) - .transpose() - .context("invalid virtio-fs user token")?; - - Ok(ParsedVirtioFsBackend::HostFs { - root_path, - mount_options: LxVolumeOptions::from_option_string(&mount_options), - platform: ParsedHostFsPlatform { user_token }, - }) - } - } else { - fn parse_host_fs( - root_path: String, - mount_options: String, - windows_options: Option, - ) -> anyhow::Result { - if let Some(options) = windows_options { - anyhow::ensure!( - options.user_token_handle == 0, - "virtio-fs windows_options are only supported on Windows" - ); - } - Ok(ParsedVirtioFsBackend::HostFs { - root_path, - mount_options: LxVolumeOptions::from_option_string(&mount_options), - platform: ParsedHostFsPlatform, - }) - } - } -} - /// Resolver for virtiofs devices. pub struct VirtioFsResolver; @@ -246,6 +154,79 @@ impl ResolveResource for VirtioFsResolver { } } +cfg_if! { + if #[cfg(windows)] { + use crate::impersonation::duplicate_user_token; + use crate::impersonation::ImpersonationGuard; + use std::os::windows::io::OwnedHandle; + + struct ParsedHostFsPlatform { + user_token: Option, + } + + impl ParsedHostFsPlatform { + fn parse(windows_options: Option) -> anyhow::Result { + let user_token = windows_options + .as_ref() + .map(|options| options.user_token_handle) + .filter(|handle| *handle != 0) + .map(duplicate_user_token) + .transpose() + .context("invalid virtio-fs user token")?; + + Ok(Self { user_token }) + } + + fn open_fs( + &self, + root_path: &str, + mount_options: &LxVolumeOptions, + ) -> anyhow::Result { + let _impersonation = self + .user_token + .as_ref() + .map(ImpersonationGuard::new) + .transpose() + .context("failed to impersonate virtio-fs user token")?; + Ok(VirtioFs::new(root_path, Some(mount_options))?) + } + + fn device_options(self) -> VirtioFsDeviceOptions { + VirtioFsDeviceOptions { + user_token: self.user_token, + ..Default::default() + } + } + } + } else { + struct ParsedHostFsPlatform; + + impl ParsedHostFsPlatform { + fn parse(windows_options: Option) -> anyhow::Result { + if let Some(options) = windows_options { + anyhow::ensure!( + options.user_token_handle == 0, + "virtio-fs windows_options are only supported on Windows" + ); + } + Ok(Self) + } + + fn open_fs( + &self, + root_path: &str, + mount_options: &LxVolumeOptions, + ) -> anyhow::Result { + Ok(VirtioFs::new(root_path, Some(mount_options))?) + } + + fn device_options(self) -> VirtioFsDeviceOptions { + Default::default() + } + } + } +} + #[cfg(test)] mod tests { use super::ParsedVirtioFs; @@ -295,4 +276,36 @@ mod tests { *windows_options = Some(Default::default()); assert!(ParsedVirtioFs::parse(resource).is_ok()); } + + #[cfg(not(windows))] + #[test] + fn parse_rejects_windows_backends_and_tokens_on_non_windows() { + let mut resource = host_fs("tag", "root", ""); + let VirtioFsBackend::HostFs { + windows_options, .. + } = &mut resource.fs + else { + unreachable!(); + }; + *windows_options = Some(virtio_resources::fs::VirtioFsWindowsOptions { + user_token_handle: 1, + }); + let error = ParsedVirtioFs::parse(resource).err().unwrap(); + assert_eq!( + error.to_string(), + "virtio-fs windows_options are only supported on Windows" + ); + + let resource = VirtioFsHandle { + tag: "tag".to_owned(), + fs: VirtioFsBackend::SectionFs { + root_path: "root".to_owned(), + }, + }; + let error = ParsedVirtioFs::parse(resource).err().unwrap(); + assert_eq!( + error.to_string(), + "section fs not supported on this platform" + ); + } } From e9e58c33301468b74458b743fb656019db658dad Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Tue, 3 Nov 2026 11:09:18 -0800 Subject: [PATCH 3/8] pal: share Windows impersonation guard 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> --- support/pal/src/windows/security.rs | 102 ++++++++++++++++++ support/sparse_mmap/src/windows.rs | 26 +---- .../virtio/virtiofs/src/impersonation.rs | 38 +------ vm/devices/virtio/virtiofs/src/resolver.rs | 27 +++-- vm/devices/virtio/virtiofs/src/session.rs | 2 +- 5 files changed, 124 insertions(+), 71 deletions(-) diff --git a/support/pal/src/windows/security.rs b/support/pal/src/windows/security.rs index 15371c128ef..19d21990ab4 100644 --- a/support/pal/src/windows/security.rs +++ b/support/pal/src/windows/security.rs @@ -27,14 +27,17 @@ use windows_sys::Win32::Security::Authorization::SDDL_REVISION_1; use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; use windows_sys::Win32::Security::DeriveCapabilitySidsFromName; use windows_sys::Win32::Security::GROUP_SECURITY_INFORMATION; +use windows_sys::Win32::Security::ImpersonateSelf; use windows_sys::Win32::Security::LABEL_SECURITY_INFORMATION; use windows_sys::Win32::Security::OWNER_SECURITY_INFORMATION; use windows_sys::Win32::Security::PSECURITY_DESCRIPTOR; use windows_sys::Win32::Security::PSID; +use windows_sys::Win32::Security::RevertToSelf; use windows_sys::Win32::Security::SACL_SECURITY_INFORMATION; use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::Security::SECURITY_CAPABILITIES; use windows_sys::Win32::Security::SID_AND_ATTRIBUTES; +use windows_sys::Win32::Security::SecurityImpersonation; use windows_sys::Win32::Storage::FileSystem::CREATE_NEW; use windows_sys::Win32::Storage::FileSystem::CreateDirectoryW; use windows_sys::Win32::Storage::FileSystem::CreateFileW; @@ -43,9 +46,65 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; use windows_sys::Win32::System::SystemServices::SE_GROUP_ENABLED; +use windows_sys::Win32::System::Threading::SetThreadToken; const MAX_SUBAUTHORITY_COUNT: usize = 15; +/// Reverts the current thread to its process security context when dropped. +/// +/// The guard is thread-bound and must remain within a synchronous scope. It +/// does not restore a previous impersonation token, so impersonation scopes +/// must not be nested. +/// +/// # Panics +/// +/// Panics if reverting impersonation fails, since continuing under the wrong +/// security context is unsafe. +#[must_use = "dropping the guard immediately reverts impersonation"] +pub struct RevertToSelfGuard { + _not_send: PhantomData<*const ()>, +} + +impl RevertToSelfGuard { + /// Impersonates `token` on the current thread until the guard is dropped. + pub fn impersonate(token: impl AsHandle) -> std::io::Result { + // SAFETY: A null thread selects the current thread, and the borrowed + // token handle remains valid for the duration of the call. + if unsafe { SetThreadToken(null_mut(), token.as_handle().as_raw_handle()) } == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(Self { + _not_send: PhantomData, + }) + } + + /// Impersonates a private copy of the process token on the current thread. + /// + /// This allows adjusting thread-local privileges without changing the + /// process token. + pub fn impersonate_self() -> std::io::Result { + // SAFETY: Calling Win32 API as documented. + if unsafe { ImpersonateSelf(SecurityImpersonation) } == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(Self { + _not_send: PhantomData, + }) + } +} + +impl Drop for RevertToSelfGuard { + fn drop(&mut self) { + // SAFETY: Calling Win32 API as documented. + if unsafe { RevertToSelf() } == 0 { + panic!( + "failed to revert thread impersonation: {}", + std::io::Error::last_os_error() + ); + } + } +} + /// A Windows SID. #[derive(Clone, Copy)] #[repr(C)] @@ -453,11 +512,54 @@ mod tests { use std::path::PathBuf; use test_with_tracing::test; use widestring::U16CStr; + use windows_sys::Win32::Foundation::ERROR_NO_TOKEN; use windows_sys::Win32::Foundation::LocalFree; use windows_sys::Win32::Security::Authorization::ConvertSecurityDescriptorToStringSecurityDescriptorW; use windows_sys::Win32::Security::Authorization::SDDL_REVISION_1; use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; use windows_sys::Win32::Security::GetFileSecurityW; + use windows_sys::Win32::Security::TOKEN_QUERY; + use windows_sys::Win32::System::Threading::GetCurrentThread; + use windows_sys::Win32::System::Threading::OpenThreadToken; + + fn thread_token() -> io::Result { + let mut token = null_mut(); + // SAFETY: The output pointer is valid, and a successful call returns a + // new handle owned by this process. + unsafe { + if OpenThreadToken(GetCurrentThread(), TOKEN_QUERY, 1, &mut token) == 0 { + return Err(io::Error::last_os_error()); + } + Ok(OwnedHandle::from_raw_handle(token)) + } + } + + #[test] + fn impersonate_self_reverts_on_success_and_error() { + for fail in [false, true] { + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + let result = (|| -> io::Result<()> { + let _guard = RevertToSelfGuard::impersonate_self().unwrap(); + thread_token().unwrap(); + if fail { + return Err(io::Error::from(ErrorKind::PermissionDenied)); + } + Ok(()) + })(); + if fail { + assert_eq!(result.unwrap_err().kind(), ErrorKind::PermissionDenied); + } else { + result.unwrap(); + } + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + } + } struct TestDirectory(PathBuf); diff --git a/support/sparse_mmap/src/windows.rs b/support/sparse_mmap/src/windows.rs index e5475e5259e..66d792f7817 100644 --- a/support/sparse_mmap/src/windows.rs +++ b/support/sparse_mmap/src/windows.rs @@ -1208,45 +1208,23 @@ pub fn alloc_shared_memory_hugetlb( /// Fails loudly if the privilege is not held, rather than silently allowing the /// large-page allocation in `f` to fail with a less actionable error. fn with_lock_memory_privilege(f: impl FnOnce() -> io::Result) -> io::Result { + use pal::windows::security::RevertToSelfGuard; use windows_sys::Wdk::System::SystemServices::SE_LOCK_MEMORY_PRIVILEGE; use windows_sys::Win32::Foundation::ERROR_NOT_ALL_ASSIGNED; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::LUID; use windows_sys::Win32::Security::AdjustTokenPrivileges; - use windows_sys::Win32::Security::ImpersonateSelf; use windows_sys::Win32::Security::LUID_AND_ATTRIBUTES; - use windows_sys::Win32::Security::RevertToSelf; use windows_sys::Win32::Security::SE_PRIVILEGE_ENABLED; - use windows_sys::Win32::Security::SecurityImpersonation; use windows_sys::Win32::Security::TOKEN_ADJUST_PRIVILEGES; use windows_sys::Win32::Security::TOKEN_PRIVILEGES; use windows_sys::Win32::Security::TOKEN_QUERY; use windows_sys::Win32::System::Threading::GetCurrentThread; use windows_sys::Win32::System::Threading::OpenThreadToken; - /// Reverts the calling thread to its process security context on drop, so - /// the impersonation token is removed even if `f` panics or an early return - /// occurs after impersonation is established. - struct RevertToSelfGuard; - impl Drop for RevertToSelfGuard { - fn drop(&mut self) { - // SAFETY: no preconditions. - if unsafe { RevertToSelf() } == 0 { - panic!( - "failed to revert thread impersonation: {}", - Error::last_os_error() - ); - } - } - } - // Give the current thread an impersonation token copied from the process // token, so that the privilege change below is scoped to this thread. - // SAFETY: calling per API contract. - if unsafe { ImpersonateSelf(SecurityImpersonation) } == 0 { - return Err(Error::last_os_error()); - } - let _revert = RevertToSelfGuard; + let _revert = RevertToSelfGuard::impersonate_self()?; // Open this thread's impersonation token for privilege adjustment. // SAFETY: calling per API contract; the returned token handle is owned and diff --git a/vm/devices/virtio/virtiofs/src/impersonation.rs b/vm/devices/virtio/virtiofs/src/impersonation.rs index 4ebb3a27ef6..85b77cf6209 100644 --- a/vm/devices/virtio/virtiofs/src/impersonation.rs +++ b/vm/devices/virtio/virtiofs/src/impersonation.rs @@ -7,14 +7,11 @@ #![expect(unsafe_code)] use std::io; -use std::marker::PhantomData; -use std::os::windows::io::AsRawHandle; use std::os::windows::io::FromRawHandle; use std::os::windows::io::OwnedHandle; use std::os::windows::io::RawHandle; use std::ptr; use windows_sys::Win32::Security; -use windows_sys::Win32::System::Threading::SetThreadToken; /// Creates an impersonation token, owned by this process, from a token handle /// value duplicated into this process by the RPC caller. @@ -44,37 +41,6 @@ fn duplicate_impersonation_token(token: RawHandle) -> io::Result { } } -/// Impersonates a token on the current thread until dropped. -pub(crate) struct ImpersonationGuard { - _not_send: PhantomData<*const ()>, -} - -impl ImpersonationGuard { - pub fn new(token: &OwnedHandle) -> io::Result { - // SAFETY: A null thread selects the current thread, and `token` is a - // valid handle for the duration of the call. - if unsafe { SetThreadToken(ptr::null(), token.as_raw_handle()) } == 0 { - return Err(io::Error::last_os_error()); - } - - Ok(Self { - _not_send: PhantomData, - }) - } -} - -impl Drop for ImpersonationGuard { - fn drop(&mut self) { - // SAFETY: Calling Win32 API as documented. - if unsafe { Security::RevertToSelf() } == 0 { - panic!( - "failed to revert virtio-fs impersonation: {}", - io::Error::last_os_error() - ); - } - } -} - #[cfg(test)] mod tests { use super::*; @@ -84,6 +50,8 @@ mod tests { use fuse::Request; use fuse::SessionInfo; use fuse::protocol::*; + use pal::windows::security::RevertToSelfGuard; + use std::os::windows::io::AsRawHandle; use std::sync::Arc; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; @@ -148,7 +116,7 @@ mod tests { let token_handle = process_token.as_raw_handle() as usize as u64; let token = duplicate_user_token(token_handle).unwrap(); { - let _guard = ImpersonationGuard::new(&token).unwrap(); + let _guard = RevertToSelfGuard::impersonate(&token).unwrap(); thread_token().unwrap(); } diff --git a/vm/devices/virtio/virtiofs/src/resolver.rs b/vm/devices/virtio/virtiofs/src/resolver.rs index c1615b23d6f..69256dc8f10 100644 --- a/vm/devices/virtio/virtiofs/src/resolver.rs +++ b/vm/devices/virtio/virtiofs/src/resolver.rs @@ -60,15 +60,7 @@ impl ParsedVirtioFs { mount_options, windows_options, } => { - anyhow::ensure!( - !root_path.is_empty(), - "virtio-fs root path must not be empty" - ); - anyhow::ensure!( - !root_path.contains('\0'), - "virtio-fs root path must not contain NUL bytes" - ); - + validate_root_path(&root_path)?; let platform = ParsedHostFsPlatform::parse(windows_options)?; ParsedVirtioFsBackend::HostFs { root_path, @@ -78,6 +70,7 @@ impl ParsedVirtioFs { } #[cfg(windows)] VirtioFsBackend::SectionFs { root_path } => { + validate_root_path(&root_path)?; ParsedVirtioFsBackend::SectionFs { root_path } } #[cfg(not(windows))] @@ -93,6 +86,18 @@ impl ParsedVirtioFs { } } +fn validate_root_path(root_path: &str) -> anyhow::Result<()> { + anyhow::ensure!( + !root_path.is_empty(), + "virtio-fs root path must not be empty" + ); + anyhow::ensure!( + !root_path.contains('\0'), + "virtio-fs root path must not contain NUL bytes" + ); + Ok(()) +} + /// Resolver for virtiofs devices. pub struct VirtioFsResolver; @@ -157,7 +162,7 @@ impl ResolveResource for VirtioFsResolver { cfg_if! { if #[cfg(windows)] { use crate::impersonation::duplicate_user_token; - use crate::impersonation::ImpersonationGuard; + use pal::windows::security::RevertToSelfGuard; use std::os::windows::io::OwnedHandle; struct ParsedHostFsPlatform { @@ -185,7 +190,7 @@ cfg_if! { let _impersonation = self .user_token .as_ref() - .map(ImpersonationGuard::new) + .map(RevertToSelfGuard::impersonate) .transpose() .context("failed to impersonate virtio-fs user token")?; Ok(VirtioFs::new(root_path, Some(mount_options))?) diff --git a/vm/devices/virtio/virtiofs/src/session.rs b/vm/devices/virtio/virtiofs/src/session.rs index 65d96f6e25c..75d3f5d69d2 100644 --- a/vm/devices/virtio/virtiofs/src/session.rs +++ b/vm/devices/virtio/virtiofs/src/session.rs @@ -39,7 +39,7 @@ impl VirtioFsSession { let _impersonation = match self .user_token .as_ref() - .map(crate::impersonation::ImpersonationGuard::new) + .map(pal::windows::security::RevertToSelfGuard::impersonate) .transpose() { Ok(guard) => guard, From 4edb8839a4ac3e2aaae998fcdf3ec75c06bcfbb7 Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Tue, 3 Nov 2026 12:50:07 -0800 Subject: [PATCH 4/8] virtiofs: fix safety comment placement in impersonation test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- vm/devices/virtio/virtiofs/src/impersonation.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vm/devices/virtio/virtiofs/src/impersonation.rs b/vm/devices/virtio/virtiofs/src/impersonation.rs index 85b77cf6209..5ca2cd0aed4 100644 --- a/vm/devices/virtio/virtiofs/src/impersonation.rs +++ b/vm/devices/virtio/virtiofs/src/impersonation.rs @@ -193,10 +193,8 @@ mod tests { let token = thread_token().unwrap(); assert_eq!(token_id(&token), self.token_id); // SAFETY: The handle refers to a valid token. - assert_ne!( - unsafe { Security::IsTokenRestricted(token.as_raw_handle()) }, - 0 - ); + let restricted = unsafe { Security::IsTokenRestricted(token.as_raw_handle()) }; + assert_ne!(restricted, 0); self.calls.fetch_add(1, Ordering::Relaxed); } } From c529c192c4220f83db49d5555a276f2cb166dcdb Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Wed, 4 Nov 2026 16:26:19 -0800 Subject: [PATCH 5/8] align handle and fd passing --- .../src/reference/openvmm/management/grpc.md | 14 + openvmm/openvmm_entry/Cargo.toml | 1 + openvmm/openvmm_entry/src/ttrpc/fd_passing.rs | 307 ++------- .../openvmm_entry/src/ttrpc/handle_passing.rs | 193 ++++++ openvmm/openvmm_entry/src/ttrpc/mod.rs | 135 ++-- .../src/ttrpc/resource_passing.rs | 591 ++++++++++++++++++ .../openvmm_ttrpc_vmservice/src/fd_passing.md | 188 ------ .../src/resource_passing.md | 252 ++++++++ .../src/vmservice.proto | 12 +- support/pal/src/windows.rs | 31 + support/pal/src/windows/security.rs | 46 ++ vm/devices/virtio/virtio_resources/src/lib.rs | 5 +- .../virtio/virtiofs/src/impersonation.rs | 266 -------- .../virtio/virtiofs/src/integration_tests.rs | 210 +++++++ vm/devices/virtio/virtiofs/src/lib.rs | 2 - vm/devices/virtio/virtiofs/src/resolver.rs | 35 +- .../vmm_tests/tests/tests/ttrpc/fd_passing.rs | 2 +- 17 files changed, 1486 insertions(+), 804 deletions(-) create mode 100644 openvmm/openvmm_entry/src/ttrpc/handle_passing.rs create mode 100644 openvmm/openvmm_entry/src/ttrpc/resource_passing.rs delete mode 100644 openvmm/openvmm_ttrpc_vmservice/src/fd_passing.md create mode 100644 openvmm/openvmm_ttrpc_vmservice/src/resource_passing.md delete mode 100644 vm/devices/virtio/virtiofs/src/impersonation.rs diff --git a/Guide/src/reference/openvmm/management/grpc.md b/Guide/src/reference/openvmm/management/grpc.md index d0f8fe6c2b6..b7f7134a061 100644 --- a/Guide/src/reference/openvmm/management/grpc.md +++ b/Guide/src/reference/openvmm/management/grpc.md @@ -84,6 +84,19 @@ If a write fails, OpenVMM logs the error and disconnects the file backend. Further console output is discarded; the backend does not automatically retry or reopen the file. +## Passing handles on Windows + +Some API fields reference OS handles by name. For example, +`VirtioFs.WindowsOptions.user_token_name` names an access token that virtio-fs +impersonates for host file access. + +Clients register these handles over the same socket using the +[resource-passing protocol]. Because Windows Unix sockets cannot carry handles, +the client duplicates each handle into the OpenVMM process with +`DuplicateHandle`, which requires `PROCESS_DUP_HANDLE` access to it, and sends +the handle value under a name. OpenVMM makes its own duplicate; the client then +closes its copy in OpenVMM. + ## Graceful guest shutdown `ShutdownVM` requests a cooperative guest power-off, non-forced by default. @@ -246,3 +259,4 @@ When editing `vmservice.proto`, follow the [Protocol Buffers enum conventions](../../../dev_guide/contrib/code.md#protocol-buffers-enums). [`vmservice.proto`]: https://github.com/microsoft/openvmm/blob/main/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto +[resource-passing protocol]: https://github.com/microsoft/openvmm/blob/main/openvmm/openvmm_ttrpc_vmservice/src/resource_passing.md diff --git a/openvmm/openvmm_entry/Cargo.toml b/openvmm/openvmm_entry/Cargo.toml index a036ad0b564..e88274fccab 100644 --- a/openvmm/openvmm_entry/Cargo.toml +++ b/openvmm/openvmm_entry/Cargo.toml @@ -118,6 +118,7 @@ win_etw_tracing.workspace = true workspace = true features = [ "Win32_Foundation", + "Win32_Security", "Win32_Storage_FileSystem", ] diff --git a/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs b/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs index 8b7d74b661f..ae5dc18fe1b 100644 --- a/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs +++ b/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs @@ -10,47 +10,38 @@ //! //! It runs over the same `AF_UNIX` stream socket as ttrpc and gRPC, selected by //! a distinct magic first byte, and carries descriptors via `SCM_RIGHTS`. See -//! `openvmm_ttrpc_vmservice/src/fd_passing.md` for the wire specification. +//! `openvmm_ttrpc_vmservice/src/resource_passing.md` for the wire specification. +//! +//! This module implements only how descriptors arrive; the handshake, framing, +//! request semantics, and registry are shared with the Windows handle-passing +//! protocol in `resource_passing`. //! //! The entire protocol is UNIX-only (it relies on `SCM_RIGHTS`), so this whole //! module is compiled only on `cfg(unix)`. #![cfg(unix)] -use anyhow::Context as _; +use super::resource_passing::Resource; +use super::resource_passing::Transport; use pal_async::interest::InterestSlot; use pal_async::interest::PollEvents; use pal_async::socket::AsSockRef; use pal_async::socket::PolledSocket; -use parking_lot::Mutex; -use std::collections::HashMap; -use std::collections::VecDeque; use std::future::poll_fn; use std::io; use std::os::fd::AsFd; use std::os::unix::io::OwnedFd; -use std::sync::Arc; use unix_socket::UnixStream; -pub(super) use protocol::MAGIC_FIRST_BYTE; - /// The wire format of the fd-passing protocol: the on-the-socket constants that /// define the framing, independent of the implementation that reads and writes -/// them. See `openvmm_ttrpc_vmservice/src/fd_passing.md` for the full +/// them. See `openvmm_ttrpc_vmservice/src/resource_passing.md` for the full /// specification. mod protocol { - /// The first byte of the fd-passing handshake magic. Distinct from the ttrpc - /// (`0x00`) and gRPC (`b'P'`) first bytes, so the server can route a connection - /// by peeking a single byte. - pub(crate) const MAGIC_FIRST_BYTE: u8 = 0xFD; - /// The 4-byte handshake magic: `0xFD 'F' 'D' 0x01`. Encodes both protocol and /// revision. pub(super) const HANDSHAKE_MAGIC: [u8; 4] = [0xFD, b'F', b'D', 0x01]; - pub(super) const OPCODE_REGISTER: u8 = 1; - pub(super) const OPCODE_DEREGISTER: u8 = 2; - /// The number of descriptors a single connection's receiver can hold. Every /// message in this protocol carries at most one descriptor (a `Register` /// carries exactly one; nothing else carries any). A message with zero is @@ -59,196 +50,18 @@ mod protocol { pub(super) const MAX_MESSAGE_FDS: usize = 1; } -/// A registry of file descriptors passed in via the fd-passing protocol, keyed -/// by client-chosen name. -/// -/// The registry is shared (cheaply cloneable). Names live in a single global -/// namespace so that a descriptor registered on the fd-passing connection can -/// be resolved from a separate ttrpc/gRPC connection. Cleanup, however, is -/// per-connection: the fd-passing handler drops every name it registered when -/// its connection closes. -#[derive(Clone, Default)] -pub struct FdRegistry { - inner: Arc>>, -} +impl Resource for OwnedFd { + const KIND: &'static str = "file descriptor"; -impl FdRegistry { - /// Registers `fd` under `name`, failing (and dropping `fd`) if the name is - /// already registered by any connection. - fn register(&self, name: String, fd: OwnedFd) -> anyhow::Result<()> { - let mut map = self.inner.lock(); - if map.contains_key(&name) { - anyhow::bail!("name '{name}' is already registered"); - } - map.insert(name, fd); - Ok(()) - } - - /// Removes `name` from the registry, dropping (closing) its descriptor. - fn deregister(&self, name: &str) { - self.inner.lock().remove(name); - } - - /// Resolves `name` to a freshly duplicated descriptor. The registry entry - /// remains valid. - /// - /// Only the tap NIC backend (`cfg(target_os = "linux")`) and the unit tests - /// call this, so on other unix targets (e.g. macOS) it is otherwise - /// unused; suppress the resulting dead-code warning there. - #[cfg_attr(not(any(target_os = "linux", test)), expect(dead_code))] - pub fn resolve(&self, name: &str) -> anyhow::Result { - let map = self.inner.lock(); - let fd = map - .get(name) - .with_context(|| format!("no file descriptor registered under name '{name}'"))?; - fd.try_clone() - .context("failed to duplicate registered file descriptor") + fn duplicate(&self) -> io::Result { + self.try_clone() } } -/// Services a single fd-passing connection: validates the handshake, then loops -/// handling `Register`/`Deregister` requests until the peer closes the -/// connection. Any names registered by this connection are dropped on exit. -pub(super) async fn serve( +/// An fd-passing connection, holding the single descriptor received via +/// `SCM_RIGHTS` with the current message. +pub(super) struct Connection { conn: PolledSocket, - registry: &FdRegistry, -) -> anyhow::Result<()> { - let mut conn = Connection::new(conn); - - // Exchange handshakes: read and validate the client's, then send ours. - let handshake = conn - .read_exact(8) - .await - .context("failed to read fd-passing handshake")?; - if handshake[..4] != protocol::HANDSHAKE_MAGIC { - anyhow::bail!("invalid fd-passing handshake magic"); - } - // `features` (handshake[4..8]) is reserved and ignored in this revision. - // The handshake carries no ancillary data; a descriptor attached here is a - // protocol violation, so fail fast and close the connection. - if conn.fd.is_some() { - anyhow::bail!("fd-passing handshake must not carry a descriptor"); - } - - let mut server_handshake = [0u8; 8]; - server_handshake[..4].copy_from_slice(&protocol::HANDSHAKE_MAGIC); - conn.write_all(&server_handshake) - .await - .context("failed to send fd-passing handshake")?; - - // Track the names registered by this connection so they are dropped when - // it closes. `OwnedNames` deregisters on drop, so cleanup runs even if this - // future is cancelled (dropped) before `serve_requests` returns. - let mut owned_names = OwnedNames { - registry, - names: Vec::new(), - }; - serve_requests(&mut conn, &mut owned_names).await -} - -/// Tracks the names a connection has registered and deregisters them on drop, -/// so cleanup runs even if the serving future is dropped mid-flight (e.g. when -/// the accept loop is cancelled) rather than leaking them into the registry. -struct OwnedNames<'a> { - registry: &'a FdRegistry, - names: Vec, -} - -impl Drop for OwnedNames<'_> { - fn drop(&mut self) { - for name in self.names.drain(..) { - self.registry.deregister(&name); - } - } -} - -async fn serve_requests( - conn: &mut Connection, - owned_names: &mut OwnedNames<'_>, -) -> anyhow::Result<()> { - loop { - // Read the request header (opcode + name length). A clean EOF here (at - // a frame boundary) is a normal shutdown. - let Some(header) = conn - .try_read_exact(2) - .await - .context("failed to read request header")? - else { - return Ok(()); - }; - let opcode = header[0]; - let name_len = header[1] as usize; - let name_bytes = conn - .read_exact(name_len) - .await - .context("failed to read request name")?; - - // Take the descriptor (if any) that arrived with this message, clearing - // it so it never lingers into the next message. Only a register consumes - // one; any other message carrying a descriptor is malformed and is - // rejected below (or closes the connection). - let fd = conn.fd.take(); - - match opcode { - protocol::OPCODE_REGISTER => match parse_name(&name_bytes) { - Some(name) => match fd { - Some(fd) => match owned_names.registry.register(name.clone(), fd) { - Ok(()) => { - owned_names.names.push(name); - conn.write_response(None).await?; - } - Err(err) => conn.write_response(Some(&err.to_string())).await?, - }, - None => { - conn.write_response(Some( - "register requires exactly one attached descriptor", - )) - .await? - } - }, - None => { - // Invalid name; drop any attached descriptor. - drop(fd); - conn.write_response(Some("invalid name")).await?; - } - }, - protocol::OPCODE_DEREGISTER => match parse_name(&name_bytes) { - _ if fd.is_some() => { - conn.write_response(Some("deregister does not accept an attached descriptor")) - .await? - } - Some(name) if let Some(pos) = owned_names.names.iter().position(|n| *n == name) => { - owned_names.names.swap_remove(pos); - owned_names.registry.deregister(&name); - conn.write_response(None).await?; - } - Some(_) => { - conn.write_response(Some("name not registered by this connection")) - .await?; - } - None => conn.write_response(Some("invalid name")).await?, - }, - // An unknown opcode is unrecoverable: the stream has no length - // prefix to resynchronize on, so close the connection. Any received - // descriptors are dropped with `conn`. - other => anyhow::bail!("unknown fd-passing opcode {other}"), - } - } -} - -/// Validates a request name: it must be non-empty and valid UTF-8. -fn parse_name(bytes: &[u8]) -> Option { - match std::str::from_utf8(bytes) { - Ok(name) if !name.is_empty() => Some(name.to_owned()), - _ => None, - } -} - -/// Buffers a stream socket, reassembling length-known frames and holding the -/// single descriptor received via `SCM_RIGHTS` with the current message. -struct Connection { - conn: PolledSocket, - read_buf: VecDeque, receiver: unix_socket::ScmReceiver, /// The descriptor received with the message currently being read, if any. /// Every message carries at most one (`MAX_MESSAGE_FDS == 1`), and the @@ -259,31 +72,33 @@ struct Connection { } impl Connection { - fn new(conn: PolledSocket) -> Self { + pub(super) fn new(conn: PolledSocket) -> Self { Self { conn, - read_buf: VecDeque::new(), receiver: unix_socket::ScmReceiver::new(protocol::MAX_MESSAGE_FDS), fd: None, } } +} + +impl Transport for Connection { + type Resource = OwnedFd; + const MAGIC: [u8; 4] = protocol::HANDSHAKE_MAGIC; + /// Descriptors travel out of band, via `SCM_RIGHTS`. + const INLINE_RESOURCE_LEN: usize = 0; - /// Performs a single `recvmsg`, appending received bytes to the read buffer - /// and taking the at-most-one descriptor into `fd`. Returns the number of - /// bytes read (0 on EOF). - async fn fill(&mut self) -> io::Result { - let mut buf = [0u8; 512]; + /// Performs a single `recvmsg`, taking the at-most-one descriptor into + /// `fd`. + async fn read(&mut self, buf: &mut [u8]) -> io::Result { let n = poll_fn(|cx| { self.conn .poll_io(cx, InterestSlot::Read, PollEvents::IN, |this| { let sock = this.get().as_sock_ref(); - self.receiver.recv(sock.as_fd(), &mut buf) + self.receiver.recv(sock.as_fd(), buf) }) }) .await?; - self.read_buf.extend(&buf[..n]); - // Take the at-most-one descriptor that arrived with this `recv`. A - // conforming client attaches a descriptor only to a `Register` and + // A conforming client attaches a descriptor only to a `Register` and // reads the response before sending anything else, so the previous // message's descriptor has already been consumed and `fd` is empty // here. A misbehaving client can still attach descriptors to other @@ -302,61 +117,24 @@ impl Connection { Ok(n) } - /// Reads exactly `n` bytes, erroring on EOF. - async fn read_exact(&mut self, n: usize) -> io::Result> { - while self.read_buf.len() < n { - if self.fill().await? == 0 { - return Err(io::ErrorKind::UnexpectedEof.into()); - } - } - Ok(self.read_buf.drain(..n).collect()) - } - - /// Reads exactly `n` bytes, returning `None` if the peer closes the - /// connection at a frame boundary (before any of the `n` bytes arrive) and - /// erroring on a partial (mid-frame) EOF. - async fn try_read_exact(&mut self, n: usize) -> io::Result>> { - while self.read_buf.len() < n { - if self.fill().await? == 0 { - if self.read_buf.is_empty() { - return Ok(None); - } - return Err(io::ErrorKind::UnexpectedEof.into()); - } - } - Ok(Some(self.read_buf.drain(..n).collect())) - } - - /// Writes a response frame: `status: u8`, `msg_len: u16` (LE), `msg`. - /// `None` means success; `Some(msg)` is a generic failure with diagnostic - /// text. - async fn write_response(&mut self, error: Option<&str>) -> io::Result<()> { - let (status, msg) = match error { - None => (0u8, ""), - Some(msg) => (1u8, msg), - }; - let msg = msg.as_bytes(); - let msg_len = msg.len().min(u16::MAX as usize); - let mut frame = Vec::with_capacity(3 + msg_len); - frame.push(status); - frame.extend_from_slice(&(msg_len as u16).to_le_bytes()); - frame.extend_from_slice(&msg[..msg_len]); - self.write_all(&frame).await - } - async fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { use futures::AsyncWriteExt; self.conn.write_all(buf).await } + + fn take_received(&mut self) -> Option { + self.fd.take() + } } #[cfg(test)] mod tests { - use super::FdRegistry; + use super::Connection; use super::protocol::HANDSHAKE_MAGIC; - use super::protocol::OPCODE_DEREGISTER; - use super::protocol::OPCODE_REGISTER; - use super::serve; + use crate::ttrpc::ResourceRegistry; + use crate::ttrpc::resource_passing; + use crate::ttrpc::resource_passing::wire::OPCODE_DEREGISTER; + use crate::ttrpc::resource_passing::wire::OPCODE_REGISTER; use pal_async::DefaultPool; use pal_async::socket::PolledSocket; use pal_async::task::Spawn; @@ -432,7 +210,7 @@ mod tests { #[test] fn registry_semantics() { - let registry = FdRegistry::default(); + let registry = ResourceRegistry::default(); let (a, mut b) = socket_pair(); registry.register("x".to_owned(), a.into()).unwrap(); @@ -458,7 +236,7 @@ mod tests { #[test] fn end_to_end() { - let registry = FdRegistry::default(); + let registry = ResourceRegistry::default(); let (server_sock, client_sock) = socket_pair(); // Run the server on its own pal_async pool so the blocking client below @@ -466,10 +244,9 @@ mod tests { let (_thread, driver) = DefaultPool::spawn_on_thread("fd-passing-test"); let server = PolledSocket::new(&driver, UnixStream::from(server_sock)).unwrap(); let server_registry = registry.clone(); - let task = driver.spawn( - "serve", - async move { serve(server, &server_registry).await }, - ); + let task = driver.spawn("serve", async move { + resource_passing::serve(&mut Connection::new(server), &server_registry).await + }); let mut client = TestClient { sock: client_sock }; client.handshake(); diff --git a/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs b/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs new file mode 100644 index 00000000000..f541b243e81 --- /dev/null +++ b/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs @@ -0,0 +1,193 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! The OpenVMM handle-passing protocol. +//! +//! This is the Windows counterpart of the fd-passing protocol: it lets a client +//! hand OS handles (for example, a user's access token) to the management +//! server, each under a client-chosen name. A name can then be referenced from +//! the ordinary ttrpc/gRPC API (for example `VirtioFs.WindowsOptions` +//! `user_token_name`). +//! +//! It runs over the same `AF_UNIX` stream socket as ttrpc and gRPC, selected by +//! the same magic first byte as fd-passing. Windows `AF_UNIX` sockets cannot +//! carry handles, so the client duplicates each handle into this process with +//! `DuplicateHandle` and sends its value in the `Register` request. This is the +//! only place OpenVMM interprets a client-supplied handle value; the server +//! makes its own duplicate and never closes the client's. +//! +//! This module implements only how handles arrive; the handshake, framing, +//! request semantics, and registry are shared with fd-passing in +//! `resource_passing`. See `openvmm_ttrpc_vmservice/src/resource_passing.md` for +//! the wire specification. + +#![cfg(windows)] + +use super::resource_passing::Resource; +use super::resource_passing::Transport; +use anyhow::Context as _; +use pal_async::socket::PolledSocket; +use std::io; +use std::os::windows::io::OwnedHandle; +use std::os::windows::io::RawHandle; +use unix_socket::UnixStream; + +/// The wire format of the handle-passing transport. See +/// `openvmm_ttrpc_vmservice/src/resource_passing.md`. +mod protocol { + /// The 4-byte handshake magic: `0xFD 'H' 'P' 0x01`. Encodes both protocol + /// and revision. + pub(super) const HANDSHAKE_MAGIC: [u8; 4] = [0xFD, b'H', b'P', 0x01]; + + /// The length of the handle value that follows the name in a `Register` + /// request: a little-endian `u64`. + pub(super) const HANDLE_LEN: usize = 8; +} + +impl Resource for OwnedHandle { + const KIND: &'static str = "handle"; + + fn duplicate(&self) -> io::Result { + self.try_clone() + } +} + +/// A handle-passing connection. +pub(super) struct Connection(PolledSocket); + +impl Connection { + pub(super) fn new(conn: PolledSocket) -> Self { + Self(conn) + } +} + +impl Transport for Connection { + type Resource = OwnedHandle; + const MAGIC: [u8; 4] = protocol::HANDSHAKE_MAGIC; + const INLINE_RESOURCE_LEN: usize = protocol::HANDLE_LEN; + + async fn read(&mut self, buf: &mut [u8]) -> io::Result { + use futures::AsyncReadExt; + self.0.read(buf).await + } + + async fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + use futures::AsyncWriteExt; + self.0.write_all(buf).await + } + + fn decode_inline(data: &[u8]) -> anyhow::Result> { + let value = u64::from_le_bytes(data.try_into().context("truncated handle value")?); + // Reject null and the negative pseudo-handles (such as the current + // process or token), which would otherwise name this process's own + // objects. + anyhow::ensure!( + value != 0 && i64::try_from(value).is_ok(), + "invalid handle value {value:#x}" + ); + let handle = usize::try_from(value).context("invalid handle value")? as RawHandle; + let handle = pal::windows::duplicate_raw_handle(handle) + .with_context(|| format!("failed to duplicate handle {value:#x}"))?; + Ok(Some(handle)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ttrpc::ResourceRegistry; + use crate::ttrpc::resource_passing; + use crate::ttrpc::resource_passing::wire; + use pal_async::DefaultPool; + use pal_async::task::Spawn; + use std::io::Read as _; + use std::io::Write as _; + use std::os::windows::io::AsRawHandle; + use test_with_tracing::test; + + /// A blocking client, as an external process would implement. + struct Client(UnixStream); + + impl Client { + fn handshake(&mut self) { + let mut handshake = [0; wire::HANDSHAKE_LEN]; + handshake[..4].copy_from_slice(&protocol::HANDSHAKE_MAGIC); + self.0.write_all(&handshake).unwrap(); + let mut reply = [0; wire::HANDSHAKE_LEN]; + self.0.read_exact(&mut reply).unwrap(); + assert_eq!(reply[..4], protocol::HANDSHAKE_MAGIC); + } + + fn request(&mut self, opcode: u8, name: &str, handle: Option) -> u8 { + let mut frame = vec![opcode, name.len() as u8]; + frame.extend_from_slice(name.as_bytes()); + if let Some(handle) = handle { + frame.extend_from_slice(&handle.to_le_bytes()); + } + self.0.write_all(&frame).unwrap(); + let mut header = [0; 3]; + self.0.read_exact(&mut header).unwrap(); + let mut msg = vec![0; u16::from_le_bytes([header[1], header[2]]).into()]; + self.0.read_exact(&mut msg).unwrap(); + header[0] + } + } + + fn process_token() -> OwnedHandle { + use windows_sys::Win32::Security::TOKEN_DUPLICATE; + use windows_sys::Win32::Security::TOKEN_QUERY; + pal::windows::security::open_process_token(TOKEN_DUPLICATE | TOKEN_QUERY).unwrap() + } + + #[test] + fn end_to_end() { + let registry = ResourceRegistry::default(); + let (server, client) = UnixStream::pair().unwrap(); + let (_thread, driver) = DefaultPool::spawn_on_thread("handle-passing-test"); + let server = PolledSocket::new(&driver, server).unwrap(); + let task = driver.spawn("serve", { + let registry = registry.clone(); + async move { resource_passing::serve(&mut Connection::new(server), ®istry).await } + }); + + let mut client = Client(client); + client.handshake(); + + // The client and server are the same process here, so the token's own + // value stands in for one duplicated into the server. + let token = process_token(); + let value = token.as_raw_handle() as usize as u64; + + assert_eq!( + client.request(wire::OPCODE_REGISTER, "t0", Some(value)), + wire::STATUS_OK + ); + // The server made its own duplicate and did not take the client's. + drop(registry.resolve("t0").unwrap()); + token.try_clone().unwrap(); + + // Null and pseudo-handle values are rejected. + for bad in [0, u64::MAX, u64::MAX - 3] { + assert_ne!( + client.request(wire::OPCODE_REGISTER, "bad", Some(bad)), + wire::STATUS_OK + ); + } + registry.resolve("bad").unwrap_err(); + + assert_eq!( + client.request(wire::OPCODE_DEREGISTER, "t0", None), + wire::STATUS_OK + ); + registry.resolve("t0").unwrap_err(); + + // Closing the connection drops its names. + assert_eq!( + client.request(wire::OPCODE_REGISTER, "t1", Some(value)), + wire::STATUS_OK + ); + drop(client); + futures::executor::block_on(task).unwrap(); + registry.resolve("t1").unwrap_err(); + } +} diff --git a/openvmm/openvmm_entry/src/ttrpc/mod.rs b/openvmm/openvmm_entry/src/ttrpc/mod.rs index c106fc4e2ed..6bb34652718 100644 --- a/openvmm/openvmm_entry/src/ttrpc/mod.rs +++ b/openvmm/openvmm_entry/src/ttrpc/mod.rs @@ -5,19 +5,28 @@ #![cfg(any(feature = "ttrpc", feature = "grpc"))] -// The fd-passing protocol relies on `SCM_RIGHTS` and so exists only on unix. +// Clients pass OS resources to the server by name: file descriptors via the +// fd-passing protocol on unix (`SCM_RIGHTS`), and handles via the +// handle-passing protocol on Windows (`DuplicateHandle`). Both are built on the +// platform-independent core in `resource_passing`; each platform module +// provides only its `Connection` transport, selected here as +// `resource_transport`. #[cfg(unix)] mod fd_passing; +#[cfg(windows)] +mod handle_passing; +mod resource_passing; #[cfg(unix)] -use fd_passing::FdRegistry; +use fd_passing as resource_transport; +#[cfg(windows)] +use handle_passing as resource_transport; -/// On non-unix platforms the fd-passing protocol does not exist. The registry -/// is still threaded through the shared NIC configuration code, so provide an -/// empty placeholder there; it is never populated or resolved. -#[cfg(not(unix))] -#[derive(Clone, Default)] -struct FdRegistry {} +/// The registry of OS resources (fds or handles) passed in by clients, keyed by +/// client-chosen name. +type ResourceRegistry = resource_passing::Registry< + ::Resource, +>; use crate::cli_args::DiskCliKind; use crate::cli_args::GuestPowerAction; @@ -247,11 +256,12 @@ impl ResolvedTransport { /// so the first byte is `0x00`. /// * gRPC uses HTTP/2 cleartext, whose client connection preface begins with /// the ASCII bytes `"PRI "`, so the first byte is `b'P'`. -/// * The OpenVMM fd-passing protocol (UNIX only) begins with a handshake whose -/// first byte is `0xFD`, distinct from both of the above. +/// * The OpenVMM fd-passing (unix) and handle-passing (Windows) protocols begin +/// with a handshake whose first byte is `0xFD`, distinct from both of the +/// above. mod dispatch { - use super::FdRegistry; use super::ResolvedTransport; + use super::ResourceRegistry; use futures::FutureExt; use pal_async::driver::Driver; use pal_async::socket::AsSockRef; @@ -270,7 +280,7 @@ mod dispatch { listener: UnixListener, cancel: mesh::OneshotReceiver<()>, transport: ResolvedTransport, - registry: FdRegistry, + registry: ResourceRegistry, ) -> anyhow::Result<()> { let mut listener = PolledSocket::new(driver, listener)?; let mut tasks = FuturesUnordered::new(); @@ -307,7 +317,7 @@ mod dispatch { server: &mesh_rpc::Server, mut conn: PolledSocket, transport: ResolvedTransport, - registry: &FdRegistry, + registry: &ResourceRegistry, ) -> anyhow::Result<()> { // Wait for the client to send data (returning early if it hangs up // first) and classify the protocol from its first byte. @@ -315,19 +325,18 @@ mod dispatch { return Ok(()); }; - // The fd-passing protocol (UNIX only) is allowed in every transport - // mode; it shares the socket with ttrpc/gRPC and is selected by its - // distinct magic first byte. - #[cfg(not(unix))] - let _ = registry; - + // The fd-passing (UNIX) and handle-passing (Windows) protocols are + // allowed in every transport mode; they share the socket with + // ttrpc/gRPC and are selected by their common magic first byte. match first_byte { #[cfg(feature = "ttrpc")] 0x00 if transport.allows_ttrpc() => server.serve_connection(conn).await, #[cfg(feature = "grpc")] b'P' if transport.allows_grpc() => server.serve_connection_grpc(conn).await, - #[cfg(unix)] - super::fd_passing::MAGIC_FIRST_BYTE => super::fd_passing::serve(conn, registry).await, + super::resource_passing::wire::MAGIC_FIRST_BYTE => { + let mut conn = super::resource_transport::Connection::new(conn); + super::resource_passing::serve(&mut conn, registry).await + } byte => { anyhow::bail!("unrecognized or disallowed rpc protocol (first byte {byte:#04x})") } @@ -394,7 +403,7 @@ impl Worker for TtrpcWorker { lifecycle: VmLifecycle::Uninitialized, rpc_tasks: Vec::new(), transport: self.transport, - registry: FdRegistry::default(), + registry: ResourceRegistry::default(), }; let code = service.run(self.listener, recv).await?; self.exit_code.send(code); @@ -758,9 +767,10 @@ struct VmService { lifecycle: VmLifecycle, rpc_tasks: Vec>, transport: ResolvedTransport, - /// Registry of file descriptors passed in over the fd-passing protocol, - /// resolvable by name (e.g. for tap NIC backends). - registry: FdRegistry, + /// Registry of OS resources passed in over the fd-passing or + /// handle-passing protocol, resolvable by name (e.g. for tap NIC backends + /// or virtio-fs user tokens). + registry: ResourceRegistry, } fn grpc_error(err: anyhow::Error) -> Status { @@ -940,8 +950,8 @@ impl VmService { let vmgs = build_vmgs(req_config.vmgs.take()).await?; let iommufds = IommufdContexts::new(std::mem::take(&mut req_config.iommufds))?; - // Snapshot the fd registry so tap NIC backends can resolve descriptors - // passed in over the fd-passing protocol. + // Snapshot the registry so devices can resolve resources passed in + // over the fd-passing or handle-passing protocol. let registry = self.registry.clone(); // Serial ports are set up before the boot configuration because UEFI @@ -1325,7 +1335,7 @@ impl VmService { } for virtiofs in devices_config.virtiofs_config { - let resource = build_virtio_fs(virtiofs).into_resource(); + let resource = build_virtio_fs(virtiofs, ®istry)?.into_resource(); // Use VPCI when possible (currently only on Windows and macOS due // to KVM backend limitations). if cfg!(windows) || cfg!(target_os = "macos") { @@ -2176,7 +2186,7 @@ enum NicDevice { fn parse_nic_config( nic: vmservice::NicConfig, recv: Option>, - registry: &FdRegistry, + registry: &ResourceRegistry, ) -> anyhow::Result { use self::vmservice::nic_config::Backend; #[cfg(not(target_os = "linux"))] @@ -2379,7 +2389,7 @@ struct BuiltPcieTopology { async fn build_pcie_topology( topology: vmservice::PcieTopologyConfig, - registry: &FdRegistry, + registry: &ResourceRegistry, iommufds: &IommufdContexts, enable_legacy_pci_config_io: bool, ) -> anyhow::Result { @@ -2579,7 +2589,7 @@ fn walk_pcie_attachment( /// function, an NVMe controller, or a VFIO-assigned host device). async fn build_pci_device( device: vmservice::PcieDeviceKind, - registry: &FdRegistry, + registry: &ResourceRegistry, iommufds: &IommufdContexts, ) -> anyhow::Result> { use vmservice::pcie_device_kind::Kind; @@ -2742,7 +2752,7 @@ async fn build_nvme_controller( /// `VirtioDevice`. async fn build_virtio_device( device: vmservice::VirtioDevice, - registry: &FdRegistry, + registry: &ResourceRegistry, ) -> anyhow::Result> { use vmservice::virtio_device::Kind; let vmservice::VirtioDevice { kind } = device; @@ -2804,20 +2814,26 @@ async fn build_virtio_device( virtio_resources::console::VirtioConsoleHandle { backend }.into_resource() } Kind::VhostUser(vhost_user) => build_vhost_user_device(vhost_user)?, - Kind::Fs(config) => build_virtio_fs(config).into_resource(), + Kind::Fs(config) => build_virtio_fs(config, registry)?.into_resource(), }) } const OPENVMM_DEFAULT_VSOCK_GUEST_CID: u64 = 3; -fn build_virtio_fs(config: vmservice::VirtioFs) -> virtio_resources::fs::VirtioFsHandle { +fn build_virtio_fs( + config: vmservice::VirtioFs, + registry: &ResourceRegistry, +) -> anyhow::Result { let vmservice::VirtioFs { tag, root_path, read_only, windows_options, } = config; - virtio_resources::fs::VirtioFsHandle { + let windows_options = windows_options + .map(|options| build_virtio_fs_windows_options(options, registry)) + .transpose()?; + Ok(virtio_resources::fs::VirtioFsHandle { tag, fs: virtio_resources::fs::VirtioFsBackend::HostFs { root_path, @@ -2826,12 +2842,37 @@ fn build_virtio_fs(config: vmservice::VirtioFs) -> virtio_resources::fs::VirtioF } else { String::new() }, - windows_options: windows_options.map(|options| { - virtio_resources::fs::VirtioFsWindowsOptions { - user_token_handle: options.user_token_handle, - } - }), + windows_options, }, + }) +} + +/// Resolves the proto virtio-fs Windows options, looking up the user token +/// registered via the handle-passing protocol. +fn build_virtio_fs_windows_options( + options: vmservice::virtio_fs::WindowsOptions, + registry: &ResourceRegistry, +) -> anyhow::Result { + let vmservice::virtio_fs::WindowsOptions { user_token_name } = options; + #[cfg(windows)] + { + let user_token = (!user_token_name.is_empty()) + .then(|| { + registry.resolve(&user_token_name).with_context(|| { + format!("failed to resolve virtio-fs user token '{user_token_name}'") + }) + }) + .transpose()?; + Ok(virtio_resources::fs::VirtioFsWindowsOptions { user_token }) + } + #[cfg(not(windows))] + { + let _ = registry; + anyhow::ensure!( + user_token_name.is_empty(), + "virtio-fs user tokens are only supported on Windows" + ); + Ok(virtio_resources::fs::VirtioFsWindowsOptions {}) } } @@ -2932,7 +2973,7 @@ fn build_consomme_handle( /// Builds a host network endpoint resource from the proto `NicBackend`. fn build_nic_backend( backend: vmservice::NicBackend, - registry: &FdRegistry, + registry: &ResourceRegistry, ) -> anyhow::Result> { use vmservice::nic_backend::Kind; #[cfg(not(target_os = "linux"))] @@ -2962,7 +3003,7 @@ fn build_nic_backend( #[cfg(target_os = "linux")] fn build_tap_backend( tap: vmservice::TapBackend, - registry: &FdRegistry, + registry: &ResourceRegistry, ) -> anyhow::Result> { use vmservice::tap_backend::Source; let fd = match tap.source.context("missing tap source")? { @@ -3087,7 +3128,7 @@ mod tests { lifecycle: VmLifecycle::Running, rpc_tasks: Vec::new(), transport: ResolvedTransport::Auto, - registry: FdRegistry::default(), + registry: ResourceRegistry::default(), } } @@ -4009,7 +4050,7 @@ mod tests { lifecycle: VmLifecycle::Running, rpc_tasks: vec![driver.spawn("pending-rpc", std::future::pending())], transport: ResolvedTransport::Auto, - registry: FdRegistry::default(), + registry: ResourceRegistry::default(), }; event_send.send(VmControllerEvent::ExitRequested { code }); assert_eq!(service.run(listener, worker_recv).await.unwrap(), code); @@ -4376,7 +4417,7 @@ mod tests { }], ..Default::default() }, - &FdRegistry::default(), + &ResourceRegistry::default(), &IommufdContexts::default(), false, )); @@ -4423,7 +4464,7 @@ mod tests { let topology = futures::executor::block_on(build_pcie_topology( topology, - &FdRegistry::default(), + &ResourceRegistry::default(), &IommufdContexts::default(), true, )) @@ -4457,7 +4498,7 @@ mod tests { }], ..Default::default() }, - &FdRegistry::default(), + &ResourceRegistry::default(), &IommufdContexts::default(), false, )) diff --git a/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs b/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs new file mode 100644 index 00000000000..2bd3c0b8176 --- /dev/null +++ b/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs @@ -0,0 +1,591 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! The transport-independent core of the fd-passing (unix) and handle-passing +//! (Windows) protocols. +//! +//! Both protocols run over the same `AF_UNIX` stream socket as ttrpc and gRPC, +//! selected by the magic first byte [`wire::MAGIC_FIRST_BYTE`], and let a +//! client register OS resources under client-chosen names that the ttrpc/gRPC +//! API can then reference. They share the handshake, request and response +//! framing, request semantics, and the registry. They differ only in how a +//! `Register` request carries its resource, which a [`Transport`] implements: +//! fd-passing attaches a descriptor with `SCM_RIGHTS`, and handle-passing +//! appends a handle value the client duplicated into this process. + +use anyhow::Context as _; +use parking_lot::Mutex; +use std::collections::HashMap; +use std::collections::VecDeque; +use std::io; +use std::sync::Arc; + +/// Wire constants common to both transports. See +/// `openvmm_ttrpc_vmservice/src/resource_passing.md`. +pub(super) mod wire { + /// The first byte of every handshake magic. Distinct from the ttrpc + /// (`0x00`) and gRPC (`b'P'`) first bytes, so the server can route a + /// connection by peeking a single byte. + pub(in crate::ttrpc) const MAGIC_FIRST_BYTE: u8 = 0xFD; + + /// The length of the handshake: a 4-byte magic and a `u32` features field. + pub(in crate::ttrpc) const HANDSHAKE_LEN: usize = 8; + + pub(in crate::ttrpc) const OPCODE_REGISTER: u8 = 1; + pub(in crate::ttrpc) const OPCODE_DEREGISTER: u8 = 2; + + pub(in crate::ttrpc) const STATUS_OK: u8 = 0; + pub(in crate::ttrpc) const STATUS_FAILURE: u8 = 1; +} + +/// A connected client stream and how it carries resources. +pub(super) trait Transport { + type Resource: Resource; + + /// The 4-byte handshake magic, which identifies protocol and revision. + const MAGIC: [u8; 4]; + + /// The number of bytes of resource data that follow the name in a + /// `Register` request. + const INLINE_RESOURCE_LEN: usize; + + /// Performs a single read into `buf`, returning the number of bytes read + /// (0 on EOF). Resources received out of band are held for + /// [`Self::take_received`]. + async fn read(&mut self, buf: &mut [u8]) -> io::Result; + + /// Writes all of `buf`. + async fn write_all(&mut self, buf: &[u8]) -> io::Result<()>; + + /// Takes the resource received out of band since the last call, if any. + fn take_received(&mut self) -> Option { + None + } + + /// Decodes the [`Self::INLINE_RESOURCE_LEN`] bytes of resource data in a + /// `Register` request. + fn decode_inline(data: &[u8]) -> anyhow::Result> { + let _ = data; + Ok(None) + } +} + +/// Services a single connection: validates the handshake, then handles +/// `Register`/`Deregister` requests until the peer closes the connection. +/// Names registered by this connection are dropped on exit, including when +/// this future is cancelled. +pub(super) async fn serve( + transport: &mut T, + registry: &Registry, +) -> anyhow::Result<()> { + let mut conn = Stream::new(transport); + + // Exchange handshakes: read and validate the client's, then send ours. + let handshake = conn + .read_exact(wire::HANDSHAKE_LEN) + .await + .context("failed to read handshake")?; + anyhow::ensure!(handshake[..4] == T::MAGIC, "invalid handshake magic"); + // `features` is reserved and ignored in this revision. + anyhow::ensure!( + conn.transport.take_received().is_none(), + "handshake must not carry a {}", + T::Resource::KIND + ); + conn.transport + .write_all(&handshake_bytes(T::MAGIC)) + .await + .context("failed to send handshake")?; + + let mut registrations = Registrations::new(registry.clone()); + loop { + // A clean EOF at a frame boundary is a normal shutdown. + let Some(header) = conn + .try_read_exact(2) + .await + .context("failed to read request header")? + else { + return Ok(()); + }; + let opcode = header[0]; + let name = conn + .read_exact(header[1].into()) + .await + .context("failed to read request name")?; + let inline = if opcode == wire::OPCODE_REGISTER { + Some( + conn.read_exact(T::INLINE_RESOURCE_LEN) + .await + .context("failed to read request resource")?, + ) + } else { + None + }; + // Take any resource that arrived with this request, so it never + // lingers into the next one. + let received = conn.transport.take_received(); + + let result = inline + .map_or(Ok(None), |inline| T::decode_inline(&inline)) + .map_err(RequestError::Failed) + .and_then(|inline| { + let resource = match (received, inline) { + (Some(_), Some(_)) => { + return Err(RequestError::Failed(anyhow::anyhow!( + "a request may carry at most one {}", + T::Resource::KIND + ))); + } + (received, inline) => received.or(inline), + }; + registrations.handle(opcode, &name, resource) + }); + let error = match result { + Ok(()) => None, + Err(RequestError::Failed(err)) => Some(format!("{err:#}")), + // An unknown opcode is unrecoverable: the stream has no length + // prefix to resynchronize on, so close the connection. + Err(err @ RequestError::UnknownOpcode(_)) => { + return Err(anyhow::Error::from(err).context("protocol violation")); + } + }; + conn.transport + .write_all(&encode_response(error.as_deref())) + .await + .context("failed to send response")?; + } +} + +/// Buffers a [`Transport`], reassembling length-known frames. +struct Stream<'a, T> { + transport: &'a mut T, + read_buf: VecDeque, +} + +impl<'a, T: Transport> Stream<'a, T> { + fn new(transport: &'a mut T) -> Self { + Self { + transport, + read_buf: VecDeque::new(), + } + } + + /// Performs a single read, appending to the read buffer. Returns the number + /// of bytes read (0 on EOF). + async fn fill(&mut self) -> io::Result { + let mut buf = [0u8; 512]; + let n = self.transport.read(&mut buf).await?; + self.read_buf.extend(&buf[..n]); + Ok(n) + } + + /// Reads exactly `n` bytes, erroring on EOF. + async fn read_exact(&mut self, n: usize) -> io::Result> { + self.try_read_exact(n) + .await? + .ok_or_else(|| io::ErrorKind::UnexpectedEof.into()) + } + + /// Reads exactly `n` bytes, returning `None` if the peer closes the + /// connection at a frame boundary (before any of the `n` bytes arrive) and + /// erroring on a partial (mid-frame) EOF. + async fn try_read_exact(&mut self, n: usize) -> io::Result>> { + while self.read_buf.len() < n { + if self.fill().await? == 0 { + if self.read_buf.is_empty() { + return Ok(None); + } + return Err(io::ErrorKind::UnexpectedEof.into()); + } + } + Ok(Some(self.read_buf.drain(..n).collect())) + } +} + +/// An owned OS resource that can be passed by name. +pub(super) trait Resource: Sized { + /// A human-readable name for the resource kind, used in diagnostics. + const KIND: &'static str; + + /// Duplicates the resource, referring to the same underlying object. + fn duplicate(&self) -> io::Result; +} + +/// A registry of resources passed in by a client, keyed by client-chosen name. +/// +/// The registry is shared (cheaply cloneable). Names live in a single global +/// namespace so that a resource registered on a passing connection can be +/// resolved from a separate ttrpc/gRPC connection. Cleanup is per-connection; +/// see [`Registrations`]. +pub(super) struct Registry { + inner: Arc>>, +} + +impl Clone for Registry { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + +impl Default for Registry { + fn default() -> Self { + Self { + inner: Default::default(), + } + } +} + +impl Registry { + /// Registers `resource` under `name`, failing (and dropping `resource`) if + /// the name is already registered by any connection. + pub(super) fn register(&self, name: String, resource: T) -> anyhow::Result<()> { + let mut map = self.inner.lock(); + if map.contains_key(&name) { + anyhow::bail!("name '{name}' is already registered"); + } + map.insert(name, resource); + Ok(()) + } + + /// Removes `name` from the registry, dropping its resource. + pub(super) fn deregister(&self, name: &str) { + self.inner.lock().remove(name); + } + + /// Resolves `name` to a freshly duplicated resource. The registry entry + /// remains valid. + /// + /// Only the Linux tap backend, Windows virtio-fs, and the unit tests call + /// this, so it is otherwise unused (e.g. on macOS). + #[cfg_attr(not(any(target_os = "linux", windows, test)), expect(dead_code))] + pub(super) fn resolve(&self, name: &str) -> anyhow::Result { + let map = self.inner.lock(); + let resource = map + .get(name) + .with_context(|| format!("no {} registered under name '{name}'", T::KIND))?; + resource + .duplicate() + .with_context(|| format!("failed to duplicate registered {}", T::KIND)) + } +} + +/// An error handling a request. +#[derive(Debug, thiserror::Error)] +pub(super) enum RequestError { + /// The opcode is unknown. Stream transports cannot resynchronize after + /// this, so they close the connection. + #[error("unknown opcode {0}")] + UnknownOpcode(u8), + /// The request failed; the connection remains usable. + #[error(transparent)] + Failed(anyhow::Error), +} + +/// The names registered by one connection. +/// +/// Dropping this deregisters every name, so cleanup runs however the +/// connection ends, including when its serving future is cancelled. +pub(super) struct Registrations { + registry: Registry, + names: Vec, +} + +impl Registrations { + pub(super) fn new(registry: Registry) -> Self { + Self { + registry, + names: Vec::new(), + } + } + + /// Handles a decoded request: `opcode`, the raw `name` bytes, and the + /// resource (if any) that arrived with it. A resource not consumed by a + /// successful registration is dropped. + /// + /// The transport is responsible for rejecting requests that carry more + /// than one resource. + pub(super) fn handle( + &mut self, + opcode: u8, + name: &[u8], + resource: Option, + ) -> Result<(), RequestError> { + let failed = |err: anyhow::Error| RequestError::Failed(err); + match opcode { + wire::OPCODE_REGISTER => { + let name = parse_name(name).map_err(failed)?; + let resource = resource + .with_context(|| format!("register requires exactly one {}", T::KIND)) + .map_err(failed)?; + self.registry + .register(name.to_owned(), resource) + .map_err(failed)?; + self.names.push(name.to_owned()); + } + wire::OPCODE_DEREGISTER => { + if resource.is_some() { + return Err(failed(anyhow::anyhow!( + "deregister does not accept a {}", + T::KIND + ))); + } + let name = parse_name(name).map_err(failed)?; + let index = self + .names + .iter() + .position(|n| n == name) + .with_context(|| format!("name '{name}' is not registered by this connection")) + .map_err(failed)?; + self.names.swap_remove(index); + self.registry.deregister(name); + } + _ => return Err(RequestError::UnknownOpcode(opcode)), + } + Ok(()) + } +} + +impl Drop for Registrations { + fn drop(&mut self) { + for name in self.names.drain(..) { + self.registry.deregister(&name); + } + } +} + +/// Validates a request name: it must be non-empty and valid UTF-8. +fn parse_name(bytes: &[u8]) -> anyhow::Result<&str> { + match std::str::from_utf8(bytes) { + Ok(name) if !name.is_empty() => Ok(name), + _ => anyhow::bail!("invalid name: must be non-empty UTF-8"), + } +} + +/// Returns the handshake for `magic`, with `features = 0`. +fn handshake_bytes(magic: [u8; 4]) -> [u8; wire::HANDSHAKE_LEN] { + let mut handshake = [0; wire::HANDSHAKE_LEN]; + handshake[..4].copy_from_slice(&magic); + handshake +} + +/// Encodes a response frame: `status: u8`, `msg_len: u16` (LE), `msg`. `None` +/// is success; `Some(msg)` is a generic failure with diagnostic text, truncated +/// to fit on a character boundary. +fn encode_response(error: Option<&str>) -> Vec { + let (status, msg) = match error { + None => (wire::STATUS_OK, ""), + Some(msg) => (wire::STATUS_FAILURE, msg), + }; + let mut len = msg.len().min(u16::MAX.into()); + while !msg.is_char_boundary(len) { + len -= 1; + } + let mut frame = Vec::with_capacity(3 + len); + frame.push(status); + frame.extend_from_slice(&(len as u16).to_le_bytes()); + frame.extend_from_slice(&msg.as_bytes()[..len]); + frame +} + +#[cfg(test)] +mod tests { + use super::*; + use test_with_tracing::test; + + /// A stand-in resource that records its identity across duplication. + #[derive(Debug, PartialEq)] + struct Fake(u32); + + impl Resource for Fake { + const KIND: &'static str = "fake"; + + fn duplicate(&self) -> io::Result { + Ok(Fake(self.0)) + } + } + + fn register(r: &mut Registrations, name: &str, id: Option) -> Result<(), String> { + r.handle(wire::OPCODE_REGISTER, name.as_bytes(), id.map(Fake)) + .map_err(|e| e.to_string()) + } + + fn deregister(r: &mut Registrations, name: &str, id: Option) -> Result<(), String> { + r.handle(wire::OPCODE_DEREGISTER, name.as_bytes(), id.map(Fake)) + .map_err(|e| e.to_string()) + } + + #[test] + fn register_resolve_deregister() { + let registry = Registry::default(); + let mut conn = Registrations::new(registry.clone()); + + register(&mut conn, "a", Some(1)).unwrap(); + // Resolving duplicates and does not consume the registration. + assert_eq!(registry.resolve("a").unwrap(), Fake(1)); + assert_eq!(registry.resolve("a").unwrap(), Fake(1)); + + register(&mut conn, "a", Some(2)).unwrap_err(); + assert_eq!(registry.resolve("a").unwrap(), Fake(1)); + + deregister(&mut conn, "a", None).unwrap(); + registry.resolve("a").unwrap_err(); + deregister(&mut conn, "a", None).unwrap_err(); + } + + #[test] + fn rejects_invalid_requests() { + let registry = Registry::default(); + let mut conn = Registrations::new(registry.clone()); + + register(&mut conn, "a", None).unwrap_err(); + register(&mut conn, "", Some(1)).unwrap_err(); + conn.handle(wire::OPCODE_REGISTER, &[0xff], Some(Fake(1))) + .unwrap_err(); + assert!(matches!( + conn.handle(99, b"a", Some(Fake(1))), + Err(RequestError::UnknownOpcode(99)) + )); + + register(&mut conn, "a", Some(1)).unwrap(); + deregister(&mut conn, "a", Some(2)).unwrap_err(); + registry.resolve("a").unwrap(); + } + + #[test] + fn names_are_owned_per_connection() { + let registry = Registry::default(); + let mut owner = Registrations::new(registry.clone()); + let mut other = Registrations::new(registry.clone()); + + register(&mut owner, "a", Some(1)).unwrap(); + deregister(&mut other, "a", None).unwrap_err(); + register(&mut other, "a", Some(2)).unwrap_err(); + + drop(owner); + registry.resolve("a").unwrap_err(); + register(&mut other, "a", Some(2)).unwrap(); + } + + #[test] + fn response_encoding() { + assert_eq!(encode_response(None), [0, 0, 0]); + assert_eq!(encode_response(Some("boom")), b"\x01\x04\x00boom"); + // Truncation keeps whole characters. + let long = "é".repeat(40000); + let frame = encode_response(Some(&long)); + assert_eq!(u16::from_le_bytes([frame[1], frame[2]]), u16::MAX - 1); + } + + /// A transport over in-memory buffers whose `Register` requests carry an + /// inline little-endian `u32` resource, as handle-passing does. + struct InlineTransport { + input: VecDeque, + output: Vec, + } + + impl Transport for InlineTransport { + type Resource = Fake; + const MAGIC: [u8; 4] = [0xFD, b'X', b'Y', 0x01]; + const INLINE_RESOURCE_LEN: usize = 4; + + async fn read(&mut self, buf: &mut [u8]) -> io::Result { + // Return one byte at a time to exercise frame reassembly. + match self.input.pop_front() { + Some(b) if !buf.is_empty() => { + buf[0] = b; + Ok(1) + } + _ => Ok(0), + } + } + + async fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + self.output.extend_from_slice(buf); + Ok(()) + } + + fn decode_inline(data: &[u8]) -> anyhow::Result> { + let id = u32::from_le_bytes(data.try_into().unwrap()); + anyhow::ensure!(id != 0, "invalid resource"); + Ok(Some(Fake(id))) + } + } + + fn request(opcode: u8, name: &str, inline: &[u8]) -> Vec { + let mut frame = vec![opcode, name.len() as u8]; + frame.extend_from_slice(name.as_bytes()); + frame.extend_from_slice(inline); + frame + } + + /// Runs `serve` over `input`, returning the result, the response + /// statuses, and the registry. + fn run(input: Vec) -> (anyhow::Result<()>, Vec, Registry) { + let registry = Registry::default(); + let mut transport = InlineTransport { + input: input.into(), + output: Vec::new(), + }; + let result = futures::executor::block_on(serve(&mut transport, ®istry)); + let mut output = &transport.output[..]; + if output.len() >= wire::HANDSHAKE_LEN { + assert_eq!( + output[..wire::HANDSHAKE_LEN], + handshake_bytes(InlineTransport::MAGIC) + ); + output = &output[wire::HANDSHAKE_LEN..]; + } + let mut statuses = Vec::new(); + while let [status, lo, hi, rest @ ..] = output { + statuses.push(*status); + output = &rest[u16::from_le_bytes([*lo, *hi]).into()..]; + } + (result, statuses, registry) + } + + #[test] + fn serves_inline_requests() { + let mut input = handshake_bytes(InlineTransport::MAGIC).to_vec(); + input.extend(request(wire::OPCODE_REGISTER, "a", &7u32.to_le_bytes())); + input.extend(request(wire::OPCODE_REGISTER, "b", &0u32.to_le_bytes())); + input.extend(request(wire::OPCODE_REGISTER, "c", &8u32.to_le_bytes())); + input.extend(request(wire::OPCODE_DEREGISTER, "c", &[])); + input.extend(request(wire::OPCODE_DEREGISTER, "c", &[])); + let (result, statuses, _) = run(input); + result.unwrap(); + assert_eq!(statuses, [0, 1, 0, 0, 1]); + } + + #[test] + fn disconnect_drops_names() { + let registry = Registry::default(); + let mut input = handshake_bytes(InlineTransport::MAGIC).to_vec(); + input.extend(request(wire::OPCODE_REGISTER, "a", &7u32.to_le_bytes())); + let mut transport = InlineTransport { + input: input.into(), + output: Vec::new(), + }; + futures::executor::block_on(serve(&mut transport, ®istry)).unwrap(); + registry.resolve("a").unwrap_err(); + } + + #[test] + fn protocol_violations_close_the_connection() { + // Bad magic: no handshake reply. + let (result, statuses, _) = run(handshake_bytes([0xFD, b'F', b'D', 0x01]).to_vec()); + result.unwrap_err(); + assert!(statuses.is_empty()); + + // Unknown opcode. + let mut input = handshake_bytes(InlineTransport::MAGIC).to_vec(); + input.extend(request(99, "a", &[])); + run(input).0.unwrap_err(); + + // EOF in the middle of a request. + let mut input = handshake_bytes(InlineTransport::MAGIC).to_vec(); + input.extend(&request(wire::OPCODE_REGISTER, "a", &7u32.to_le_bytes())[..4]); + run(input).0.unwrap_err(); + } +} diff --git a/openvmm/openvmm_ttrpc_vmservice/src/fd_passing.md b/openvmm/openvmm_ttrpc_vmservice/src/fd_passing.md deleted file mode 100644 index 3b0e585db3c..00000000000 --- a/openvmm/openvmm_ttrpc_vmservice/src/fd_passing.md +++ /dev/null @@ -1,188 +0,0 @@ -# OpenVMM fd-passing protocol - -The **fd-passing protocol** lets a client hand pre-opened file descriptors to -the OpenVMM management server, each under a client-chosen name. A name can then -be referenced from the ordinary ttrpc/gRPC API — for example a `TapBackend` may -give an `fd_name` instead of a device `name`, so the server uses an already-open -tap fd instead of opening `/dev/net/tun` itself. - -It runs over the **same `AF_UNIX` stream socket** as ttrpc and gRPC, and carries -descriptors via `SCM_RIGHTS`. It is UNIX-only. - -Status: **revision 1** (handshake magic `0xFD 'F' 'D' 0x01`). - -## Conventions - -- Multi-byte integers are **little-endian**; `u8`/`u16`/`u32` are unsigned. -- **MUST** / **MAY** mark normative requirements. Blocks labelled *Rationale* - are non-normative and explain intent only. -- Descriptors are transferred with `SCM_RIGHTS` control messages on - `sendmsg`/`recvmsg` (see `unix(7)`, `cmsg(3)`). - -## Client procedure - -A client does the following, in order: - -1. **Connect** to the socket. -2. **Send** the 8-byte handshake (see [Handshake](#handshake)), with - `features = 0` and no ancillary data. -3. **Receive and validate** the server's 8-byte handshake. On magic mismatch or - EOF, abort — the server does not speak this revision. -4. **For each descriptor:** send one `Register` request carrying the fd (see - [Requests](#requests)), then read exactly one response (see - [Responses](#responses)) before doing anything else. -5. **Optionally** `Deregister` names no longer needed. -6. **Close** the connection when done; any names still registered are dropped - automatically (see [Lifetime and cleanup](#lifetime-and-cleanup)). - -Requests and responses are strictly one-for-one: a client MUST read the response -to a request before sending the next request. - -## Handshake - -Both sides send a fixed 8-byte handshake; the client sends first, then the -server replies. Neither side sends anything else until both handshakes have been -exchanged. - -``` -Handshake (8 bytes): - magic: [u8; 4] // 0xFD, 0x46 'F', 0x44 'D', 0x01 - features: u32 // 0 in this revision -``` - -- Both sides MUST send exactly these 8 bytes, with `features = 0` and **no - ancillary data**. -- Each side MUST verify the received 4-byte magic and MUST close the connection - on any mismatch. -- A client MUST ignore (not validate) the received `features` value. -- If the server closes before sending its handshake (EOF), the client MUST treat - it as not supporting this revision. - -> *Rationale.* The leading `0xFD` differs from the ttrpc (`0x00`) and gRPC -> (`'P'`) first bytes, letting the server route a connection by its first byte. -> The 4-byte magic encodes both protocol and revision: a wire-incompatible -> future revision uses a *different* magic (advertised out of band via ttrpc), -> so there is no separate version field. `features` reserves room for optional, -> backward-compatible capabilities; sending 0 and ignoring the peer's value lets -> a newer peer advertise features without breaking an older one. Keeping the -> handshake free of ancillary data means descriptors are only ever attached to -> `Register`, with no ambiguity about which message carries an fd. - -## Requests - -After the handshake the connection is a synchronous request/response loop. Each -request is a fixed frame: - -``` -opcode: u8 // 1 = Register, 2 = Deregister -name_len: u8 // name length in bytes, 1..=255 -name: [u8; name_len] // UTF-8 identifier chosen by the client -``` - -### Register (opcode 1) - -Registers one descriptor under `name`. - -- The request MUST attach **exactly one** descriptor, in a single `SCM_RIGHTS` - control message on the same `sendmsg` as the request bytes. -- Registration fails if `name` is already registered by **any** connection - (names share one global namespace). -- On success the server owns the descriptor; the client MAY close its own copy - once it has read the success response. - -### Deregister (opcode 2) - -Removes `name`. No descriptor is attached. - -- A connection MAY only deregister names it registered itself; deregistering an - unknown name, or a name owned by another connection, fails. -- On success the server drops its descriptor for that name. - -## Responses - -The server sends exactly one response per request, in order: - -``` -status: u8 // 0 = ok; non-zero = failure -msg_len: u16 // length of msg (0 on success) -msg: [u8; msg_len] // UTF-8 diagnostic text (empty on success) -``` - -- `status = 0` is success; any non-zero value is failure. -- This revision emits only `status = 1` (**generic failure**). A client MUST - treat every non-zero value as a single failure. -- `msg` is human-readable diagnostic text only; a client MUST NOT parse it. -- The connection stays usable after a failure response. - -Failures include, at minimum: registering an already-registered name; -registering with zero or more than one attached descriptor; an empty or -non-UTF-8 name; deregistering a name this connection did not register. - -The server MUST NOT panic on any input. On a frame it cannot interpret — an -unknown opcode, or a truncated request — it closes any received descriptors and -closes the connection. - -> *Rationale.* Reserving the non-zero code space lets a future revision assign -> specific codes while an older client keeps working by collapsing them to -> "failure". An uninterpretable frame cannot be skipped (there is no length -> prefix), so the byte stream cannot resynchronize and the connection must -> close. That is acceptable because the opcode set is fixed for a given -> handshake magic, and additive capabilities are negotiated via `features`, so a -> conforming client never sends one. - -## Lifetime and cleanup - -- Names are **owned by the registering connection**: when that connection closes - (cleanly or not), the server drops every descriptor it registered. -- The namespace is **global**: a name registered on one connection is resolvable - from a *different* connection. -- Resolving a name **dups** the descriptor; the registry entry stays valid. - Deregistering, or closing the registering connection, does not invalidate a - descriptor already handed to a running VM. - -> *Rationale.* A global namespace is required because the ttrpc/gRPC connection -> that creates a VM is separate from the fd-passing connection that registered -> the fd. Per-connection ownership guarantees cleanup even if the client -> crashes; dup-on-resolve lets a name be used more than once and decouples the -> VM's lifetime from the registration's. - -## Referencing a registered fd - -A registered name is used from the ordinary ttrpc/gRPC API. The first consumer -is the TAP backend: - -```proto -message TapBackend { - oneof source { - string name = 1; // open /dev/net/tun by device name (existing behavior) - string fd_name = 2; // use the fd registered under this name - } -} -``` - -With `fd_name`, the server resolves the name (dup) and uses it as the tap -backing fd; an unregistered name fails NIC creation. - -## Wire format examples - -Hex bytes; ancillary data is shown separately. - -| Message | Bytes | Ancillary | -|---------|-------|-----------| -| Handshake (either side) | `FD 46 44 01 00 00 00 00` | none | -| `Register("t0", fd)` | `01 02 74 30` | one fd via `SCM_RIGHTS` | -| `Deregister("t0")` | `02 02 74 30` | none | -| Response, success | `00 00 00` | none | -| Response, failure `"boom"` | `01 04 00 62 6F 6F 6D` | none | - -(`74 30` is the ASCII name `"t0"`; `04 00` is `msg_len = 4`, little-endian.) - -## Implementing a client - -Encoding and decoding are plain fixed byte buffers — `read_exact`/`write_all`, -no library needed. The only OS-specific step is attaching the fd on `Register`: -`std::os::unix::net::UnixStream` cannot pass a descriptor, so that one `sendmsg` -must build an `SCM_RIGHTS` control message. Options: call `libc::sendmsg` -directly; reuse OpenVMM's helper (the `SCM_RIGHTS` send/recv lives in the -`unix_socket` crate); or use a crate such as `sendfd`/`passfd`. The handshake, -`Deregister`, and all responses use no ancillary data. diff --git a/openvmm/openvmm_ttrpc_vmservice/src/resource_passing.md b/openvmm/openvmm_ttrpc_vmservice/src/resource_passing.md new file mode 100644 index 00000000000..41aff91b3e8 --- /dev/null +++ b/openvmm/openvmm_ttrpc_vmservice/src/resource_passing.md @@ -0,0 +1,252 @@ +# OpenVMM resource-passing protocol + +The **resource-passing protocol** lets a client hand OS resources to the OpenVMM +management server, each under a client-chosen name. A name can then be +referenced from the ordinary ttrpc/gRPC API — for example a `TapBackend` may give +an `fd_name` instead of a device `name`, so the server uses an already-open tap +fd instead of opening `/dev/net/tun` itself. + +It runs over the **same `AF_UNIX` stream socket** as ttrpc and gRPC. Framing and +semantics are common to all hosts; only how a resource travels differs, by +**transport**: + +| | fd-passing | handle-passing | +|-|------------|----------------| +| Host | UNIX | Windows | +| Resource | file descriptor | handle | +| Handshake magic | `FD 46 44 01` (`0xFD 'F' 'D' 0x01`) | `FD 48 50 01` (`0xFD 'H' 'P' 0x01`) | +| Resource in `Register` | one fd via `SCM_RIGHTS` | `handle: u64` after the name | +| Ancillary data | on `Register` only | never | + +A server speaks only its host's transport. Both use only documented OS APIs. + +Status: **revision 1** of each transport. + +## Conventions + +- Multi-byte integers are **little-endian**; `u8`/`u16`/`u32`/`u64` are + unsigned. +- **MUST** / **MAY** mark normative requirements. Blocks labelled *Rationale* + are non-normative and explain intent only. +- A *resource* is a file descriptor (fd-passing) or a handle (handle-passing). + +## Client procedure + +A client does the following, in order: + +1. **Prepare** (handle-passing only): open the OpenVMM process with + `PROCESS_DUP_HANDLE` access. A client that launched OpenVMM can use the + process handle it already holds. +2. **Connect** to the socket. +3. **Send** the 8-byte handshake for its transport (see [Handshake](#handshake)), + with `features = 0`. +4. **Receive and validate** the server's 8-byte handshake. On magic mismatch or + EOF, abort — the server does not speak this transport or revision. +5. **For each resource:** send one `Register` request carrying it (see + [Register](#register-opcode-1) and the transport's section), then read + exactly one response (see [Responses](#responses)) before doing anything + else. +6. **Optionally** `Deregister` names no longer needed. +7. **Close** the connection when done; any names still registered are dropped + automatically (see [Lifetime and cleanup](#lifetime-and-cleanup)). + +Requests and responses are strictly one-for-one: a client MUST read the response +to a request before sending the next request. + +## Handshake + +Both sides send a fixed 8-byte handshake; the client sends first, then the +server replies. Neither side sends anything else until both handshakes have been +exchanged. + +``` +Handshake (8 bytes): + magic: [u8; 4] // the transport's magic; see the table above + features: u32 // 0 in this revision +``` + +- Both sides MUST send exactly these 8 bytes, with `features = 0` and **no + ancillary data**. +- Each side MUST verify the received 4-byte magic and MUST close the connection + on any mismatch. In particular, a server closes a connection that sends + another host's magic. +- A client MUST ignore (not validate) the received `features` value. +- If the server closes before sending its handshake (EOF), the client MUST treat + it as not supporting this transport or revision. + +> *Rationale.* The leading `0xFD` differs from the ttrpc (`0x00`) and gRPC +> (`'P'`) first bytes, letting the server route a connection by its first byte. +> The 4-byte magic encodes transport and revision: a wire-incompatible future +> revision uses a *different* magic (advertised out of band via ttrpc), so there +> is no separate version field. `features` reserves room for optional, +> backward-compatible capabilities; sending 0 and ignoring the peer's value lets +> a newer peer advertise features without breaking an older one. Keeping the +> handshake free of ancillary data means resources are only ever attached to +> `Register`, with no ambiguity about which message carries one. + +## Requests + +After the handshake the connection is a synchronous request/response loop. Each +request begins with: + +``` +opcode: u8 // 1 = Register, 2 = Deregister +name_len: u8 // name length in bytes, 1..=255 +name: [u8; name_len] // UTF-8 identifier chosen by the client +``` + +### Register (opcode 1) + +Registers one resource under `name`. The resource travels as defined by the +transport: see [fd-passing](#fd-passing-unix) and +[handle-passing](#handle-passing-windows). + +- Registration fails if `name` is already registered by **any** connection + (names share one global namespace). + +### Deregister (opcode 2) + +Removes `name`. No resource is attached and no bytes follow the name. + +- A connection MAY only deregister names it registered itself; deregistering an + unknown name, or a name owned by another connection, fails. +- On success the server drops its resource for that name. + +## Responses + +The server sends exactly one response per request, in order: + +``` +status: u8 // 0 = ok; non-zero = failure +msg_len: u16 // length of msg (0 on success) +msg: [u8; msg_len] // UTF-8 diagnostic text (empty on success) +``` + +- `status = 0` is success; any non-zero value is failure. +- This revision emits only `status = 1` (**generic failure**). A client MUST + treat every non-zero value as a single failure. +- `msg` is human-readable diagnostic text only; a client MUST NOT parse it. +- The connection stays usable after a failure response. + +Failures include, at minimum: registering an already-registered name; +registering without a valid resource; an empty or non-UTF-8 name; deregistering +a name this connection did not register. + +The server MUST NOT panic on any input. On a frame it cannot interpret — an +unknown opcode, or a truncated request — it drops any received resources and +closes the connection. + +> *Rationale.* Reserving the non-zero code space lets a future revision assign +> specific codes while an older client keeps working by collapsing them to +> "failure". An uninterpretable frame cannot be skipped (there is no length +> prefix), so the byte stream cannot resynchronize and the connection must +> close. That is acceptable because the opcode set is fixed for a given +> handshake magic, and additive capabilities are negotiated via `features`, so a +> conforming client never sends one. + +## Lifetime and cleanup + +- Names are **owned by the registering connection**: when that connection closes + (cleanly or not), the server drops every resource it registered. +- The namespace is **global**: a name registered on one connection is resolvable + from a *different* connection. +- Resolving a name **duplicates** the resource; the registry entry stays valid. + Deregistering, or closing the registering connection, does not invalidate a + resource already handed to a running VM. + +> *Rationale.* A global namespace is required because the ttrpc/gRPC connection +> that creates a VM is separate from the connection that registered the +> resource. Per-connection ownership guarantees cleanup even if the client +> crashes; duplicate-on-resolve lets a name be used more than once and decouples +> the VM's lifetime from the registration's. + +## fd-passing (UNIX) + +Descriptors are transferred with `SCM_RIGHTS` control messages on +`sendmsg`/`recvmsg` (see `unix(7)`, `cmsg(3)`). + +- A `Register` request MUST attach **exactly one** descriptor, in a single + `SCM_RIGHTS` control message on the same `sendmsg` as the request bytes. No + bytes follow the name. +- No other message carries ancillary data. +- On success the server owns the descriptor; the client MAY close its own copy + once it has read the success response. + +### Implementing a client + +Encoding and decoding are plain fixed byte buffers — `read_exact`/`write_all`, +no library needed. The only OS-specific step is attaching the fd on `Register`: +`std::os::unix::net::UnixStream` cannot pass a descriptor, so that one `sendmsg` +must build an `SCM_RIGHTS` control message. Options: call `libc::sendmsg` +directly; reuse OpenVMM's helper (the `SCM_RIGHTS` send/recv lives in the +`unix_socket` crate); or use a crate such as `sendfd`/`passfd`. + +## handle-passing (Windows) + +Windows `AF_UNIX` sockets cannot carry handles, so the client first duplicates +each handle into the OpenVMM process with `DuplicateHandle` and sends the +resulting handle **value** in the request. No message carries ancillary data. + +A `Register` request is followed by the handle value: + +``` +opcode: u8 // 1 +name_len: u8 +name: [u8; name_len] +handle: u64 // a handle value valid in the OpenVMM process +``` + +To register a handle, the client: + +1. Duplicates it into OpenVMM: + `DuplicateHandle(GetCurrentProcess(), handle, openvmm, &remote, 0, FALSE, DUPLICATE_SAME_ACCESS)`. +2. Sends `Register` carrying `remote`, then reads the response. +3. Closes `remote` in OpenVMM, **whether or not registration succeeded**: + `DuplicateHandle(openvmm, remote, NULL, NULL, 0, FALSE, DUPLICATE_CLOSE_SOURCE)`. + +The client MUST NOT close `remote` until it has read the response. + +- `handle` MUST be a handle the client duplicated into the OpenVMM process. The + server does **not** take ownership of it: it makes its own duplicate (with the + same access) and registers that. +- Registration fails if `handle` is zero, a pseudo-handle (any value that is + negative as a signed 64-bit integer, such as `GetCurrentProcess()`), or not a + valid handle in the OpenVMM process. + +> *Rationale.* This is the only place OpenVMM interprets a client-supplied +> handle value; the rest of the API refers to handles by name. The server +> duplicates rather than adopts the value because it cannot tell which handles +> in its own process the value might alias: adopting, and later closing, an +> arbitrary value could close a handle owned by unrelated code. Duplicating +> cannot invalidate any other owner, and a client holding `PROCESS_DUP_HANDLE` +> on OpenVMM is already fully trusted over it. The client remains responsible +> for its `remote` copy, which it closes remotely with `DUPLICATE_CLOSE_SOURCE`. +> If the client exits before doing so, that one copy leaks until OpenVMM exits. + +## Referencing a registered resource + +A registered name is used from the ordinary ttrpc/gRPC API, which resolves it +(duplicating the resource) when the RPC is processed; an unregistered name fails +the RPC. Once the RPC succeeds, the name may be deregistered. + +| Field | Host | Resource | +|-------|------|----------| +| `TapBackend.fd_name` | Linux | tap device fd, used instead of opening `/dev/net/tun` | +| `VirtioFs.WindowsOptions.user_token_name` | Windows | access token that virtio-fs impersonates for host file access | + +## Wire format examples + +Hex bytes; ancillary data is shown separately. + +| Message | Bytes | Ancillary | +|---------|-------|-----------| +| Handshake, fd-passing (either side) | `FD 46 44 01 00 00 00 00` | none | +| Handshake, handle-passing (either side) | `FD 48 50 01 00 00 00 00` | none | +| `Register("t0", fd)`, fd-passing | `01 02 74 30` | one fd via `SCM_RIGHTS` | +| `Register("t0", 0x1A4)`, handle-passing | `01 02 74 30 A4 01 00 00 00 00 00 00` | none | +| `Deregister("t0")` | `02 02 74 30` | none | +| Response, success | `00 00 00` | none | +| Response, failure `"boom"` | `01 04 00 62 6F 6F 6D` | none | + +(`74 30` is the ASCII name `"t0"`; `A4 01 00 00 00 00 00 00` is the handle value +`0x1A4`; `04 00` is `msg_len = 4`.) diff --git a/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto b/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto index 088e68fec71..0850a2e53ff 100644 --- a/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto +++ b/openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto @@ -1000,7 +1000,7 @@ message TapBackend { // Open /dev/net/tun by this device name (existing behavior). string name = 1; // Use the file descriptor registered under this name via the - // fd-passing protocol (see fd_passing.md). + // resource-passing protocol (see resource_passing.md). string fd_name = 2; } } @@ -1087,11 +1087,11 @@ message VirtioFs { WindowsOptions windows_options = 5; message WindowsOptions { - // A token handle duplicated into the VM service process. OpenVMM - // duplicates the token for host filesystem access. The caller may - // close the handle after this RPC succeeds. Zero uses the VM service - // process token. - uint64 user_token_handle = 1; + // Name of an access token registered with the resource-passing + // protocol (see resource_passing.md). OpenVMM duplicates the token for host + // filesystem access. The name may be deregistered after this RPC + // succeeds. Empty uses the VM service process token. + string user_token_name = 1; } } diff --git a/support/pal/src/windows.rs b/support/pal/src/windows.rs index dcff2500866..3371b97da4d 100644 --- a/support/pal/src/windows.rs +++ b/support/pal/src/windows.rs @@ -117,6 +117,37 @@ impl BorrowedHandleExt for BorrowedHandle<'_> { } } +/// Duplicates the handle with value `handle` in this process, with the same +/// access, returning a new owned handle. +/// +/// This neither takes ownership of nor closes `handle`, so it cannot invalidate +/// a handle owned elsewhere in this process. It is intended for handle values +/// that another process duplicated into this one and passed by value. +// `handle` is an opaque handle value passed to the kernel, not dereferenced. +#[expect(clippy::not_unsafe_ptr_arg_deref)] +pub fn duplicate_raw_handle(handle: RawHandle) -> Result { + let mut duplicate = null_mut(); + // SAFETY: DuplicateHandle does not take ownership of or close `handle`; an + // invalid value fails the call. On success, `duplicate` is a new handle + // owned by this process. + unsafe { + let process = windows_sys::Win32::System::Threading::GetCurrentProcess(); + if windows_sys::Win32::Foundation::DuplicateHandle( + process, + handle, + process, + &mut duplicate, + 0, + 0, + windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS, + ) == 0 + { + return Err(Error::last_os_error()); + } + Ok(OwnedHandle::from_raw_handle(duplicate)) + } +} + pub trait OwnedSocketExt: Sized { /// Prepares the socket for being sent to another process. /// diff --git a/support/pal/src/windows/security.rs b/support/pal/src/windows/security.rs index 19d21990ab4..7c47b639410 100644 --- a/support/pal/src/windows/security.rs +++ b/support/pal/src/windows/security.rs @@ -11,6 +11,7 @@ use std::ops::Deref; use std::os::windows::prelude::*; use std::path::Path; use std::ptr::NonNull; +use std::ptr::null; use std::ptr::null_mut; use std::str::FromStr; use widestring::U16CStr; @@ -26,6 +27,7 @@ use windows_sys::Win32::Security::Authorization::ConvertStringSecurityDescriptor use windows_sys::Win32::Security::Authorization::SDDL_REVISION_1; use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; use windows_sys::Win32::Security::DeriveCapabilitySidsFromName; +use windows_sys::Win32::Security::DuplicateTokenEx; use windows_sys::Win32::Security::GROUP_SECURITY_INFORMATION; use windows_sys::Win32::Security::ImpersonateSelf; use windows_sys::Win32::Security::LABEL_SECURITY_INFORMATION; @@ -38,6 +40,10 @@ use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::Security::SECURITY_CAPABILITIES; use windows_sys::Win32::Security::SID_AND_ATTRIBUTES; use windows_sys::Win32::Security::SecurityImpersonation; +use windows_sys::Win32::Security::TOKEN_DUPLICATE; +use windows_sys::Win32::Security::TOKEN_IMPERSONATE; +use windows_sys::Win32::Security::TOKEN_QUERY; +use windows_sys::Win32::Security::TokenImpersonation; use windows_sys::Win32::Storage::FileSystem::CREATE_NEW; use windows_sys::Win32::Storage::FileSystem::CreateDirectoryW; use windows_sys::Win32::Storage::FileSystem::CreateFileW; @@ -46,6 +52,8 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; use windows_sys::Win32::System::SystemServices::SE_GROUP_ENABLED; +use windows_sys::Win32::System::Threading::GetCurrentProcess; +use windows_sys::Win32::System::Threading::OpenProcessToken; use windows_sys::Win32::System::Threading::SetThreadToken; const MAX_SUBAUTHORITY_COUNT: usize = 15; @@ -413,6 +421,44 @@ impl<'a> SidAndAttributes<'a> { } } +/// Opens the current process's primary token with `access` (a combination of +/// `TOKEN_*` access rights). +pub fn open_process_token(access: u32) -> std::io::Result { + let mut token = null_mut(); + // SAFETY: The output pointer is valid, and a successful call returns a new + // handle owned by this process. + unsafe { + if OpenProcessToken(GetCurrentProcess(), access, &mut token) == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(OwnedHandle::from_raw_handle(token)) + } +} + +/// Creates an impersonation token, owned by this process, from `token`, with +/// `TOKEN_IMPERSONATE | TOKEN_QUERY | TOKEN_DUPLICATE` access. +/// +/// Fails if `token` is not an access token or lacks `TOKEN_DUPLICATE` access. +pub fn duplicate_impersonation_token(token: BorrowedHandle<'_>) -> std::io::Result { + let mut duplicate = null_mut(); + // SAFETY: DuplicateTokenEx does not take ownership of `token`. On success, + // `duplicate` is a new handle owned by this process. + unsafe { + if DuplicateTokenEx( + token.as_raw_handle(), + TOKEN_IMPERSONATE | TOKEN_QUERY | TOKEN_DUPLICATE, + null(), + SecurityImpersonation, + TokenImpersonation, + &mut duplicate, + ) == 0 + { + return Err(std::io::Error::last_os_error()); + } + Ok(OwnedHandle::from_raw_handle(duplicate)) + } +} + /// Creates an app container token for `sid` with `capabilities`. pub fn create_app_container_token<'a, I, T>( sid: &Sid, diff --git a/vm/devices/virtio/virtio_resources/src/lib.rs b/vm/devices/virtio/virtio_resources/src/lib.rs index 0d4cbd09b12..caf2c788ba6 100644 --- a/vm/devices/virtio/virtio_resources/src/lib.rs +++ b/vm/devices/virtio/virtio_resources/src/lib.rs @@ -53,7 +53,10 @@ pub mod fs { #[derive(MeshPayload, Default)] pub struct VirtioFsWindowsOptions { - pub user_token_handle: u64, + /// Access token to impersonate for host file system access. `None` + /// uses the VMM process token. + #[cfg(windows)] + pub user_token: Option, } #[derive(MeshPayload)] diff --git a/vm/devices/virtio/virtiofs/src/impersonation.rs b/vm/devices/virtio/virtiofs/src/impersonation.rs deleted file mode 100644 index 5ca2cd0aed4..00000000000 --- a/vm/devices/virtio/virtiofs/src/impersonation.rs +++ /dev/null @@ -1,266 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -//! Serving a file system under a user token on Windows. - -// UNSAFETY: Calling Win32 APIs and constructing owned handles from their outputs. -#![expect(unsafe_code)] - -use std::io; -use std::os::windows::io::FromRawHandle; -use std::os::windows::io::OwnedHandle; -use std::os::windows::io::RawHandle; -use std::ptr; -use windows_sys::Win32::Security; - -/// Creates an impersonation token, owned by this process, from a token handle -/// value duplicated into this process by the RPC caller. -pub fn duplicate_user_token(token: u64) -> io::Result { - let token = usize::try_from(token).map_err(|_| io::Error::from(io::ErrorKind::InvalidInput))?; - duplicate_impersonation_token(token as RawHandle) -} - -fn duplicate_impersonation_token(token: RawHandle) -> io::Result { - let mut duplicate = ptr::null_mut(); - // SAFETY: DuplicateTokenEx does not take ownership of `token`. On success, - // `duplicate` is a new handle owned by this process. - unsafe { - if Security::DuplicateTokenEx( - token, - Security::TOKEN_IMPERSONATE | Security::TOKEN_QUERY | Security::TOKEN_DUPLICATE, - ptr::null(), - Security::SecurityImpersonation, - Security::TokenImpersonation, - &mut duplicate, - ) == 0 - { - return Err(io::Error::last_os_error()); - } - - Ok(OwnedHandle::from_raw_handle(duplicate)) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::session::VirtioFsSession; - use fuse::Fuse; - use fuse::ReplySender; - use fuse::Request; - use fuse::SessionInfo; - use fuse::protocol::*; - use pal::windows::security::RevertToSelfGuard; - use std::os::windows::io::AsRawHandle; - use std::sync::Arc; - use std::sync::atomic::AtomicUsize; - use std::sync::atomic::Ordering; - use test_with_tracing::test; - use windows_sys::Win32::Foundation::ERROR_NO_TOKEN; - use windows_sys::Win32::System::Threading::GetCurrentProcess; - use windows_sys::Win32::System::Threading::GetCurrentThread; - use windows_sys::Win32::System::Threading::OpenProcessToken; - use windows_sys::Win32::System::Threading::OpenThreadToken; - use zerocopy::FromBytes; - use zerocopy::FromZeros; - use zerocopy::IntoBytes; - - fn process_token() -> OwnedHandle { - let mut process_token = ptr::null_mut(); - let mut token = ptr::null_mut(); - // SAFETY: Calling Win32 APIs as documented. - unsafe { - assert_ne!( - OpenProcessToken( - GetCurrentProcess(), - Security::TOKEN_DUPLICATE, - &mut process_token - ), - 0 - ); - let process_token = OwnedHandle::from_raw_handle(process_token); - assert_ne!( - Security::DuplicateTokenEx( - process_token.as_raw_handle(), - Security::TOKEN_IMPERSONATE | Security::TOKEN_QUERY | Security::TOKEN_DUPLICATE, - ptr::null(), - Security::SecurityImpersonation, - Security::TokenImpersonation, - &mut token, - ), - 0 - ); - OwnedHandle::from_raw_handle(token) - } - } - - fn thread_token() -> io::Result { - let mut token = ptr::null_mut(); - // SAFETY: Calling Win32 API as documented. - unsafe { - if OpenThreadToken(GetCurrentThread(), Security::TOKEN_QUERY, 1, &mut token) == 0 { - return Err(io::Error::last_os_error()); - } - Ok(OwnedHandle::from_raw_handle(token)) - } - } - - #[test] - fn rejects_non_token_handle() { - duplicate_user_token(0).unwrap_err(); - } - - #[test] - fn impersonates_until_dropped() { - let process_token = process_token(); - let token_handle = process_token.as_raw_handle() as usize as u64; - let token = duplicate_user_token(token_handle).unwrap(); - { - let _guard = RevertToSelfGuard::impersonate(&token).unwrap(); - thread_token().unwrap(); - } - - let err = thread_token().unwrap_err(); - assert_eq!(err.raw_os_error(), Some(ERROR_NO_TOKEN as i32)); - } - - fn restricted_token() -> OwnedHandle { - let token = process_token(); - let mut world_sid = [0u32; Security::SECURITY_MAX_SID_SIZE as usize / size_of::()]; - let mut sid_size = size_of_val(&world_sid) as u32; - let mut restricted = ptr::null_mut(); - // SAFETY: The SID buffer is aligned and large enough for a well-known - // SID. All inputs remain valid until the new token handle is returned. - unsafe { - assert_ne!( - Security::CreateWellKnownSid( - Security::WinWorldSid, - ptr::null_mut(), - world_sid.as_mut_ptr().cast(), - &mut sid_size, - ), - 0 - ); - let sid = Security::SID_AND_ATTRIBUTES { - Sid: world_sid.as_mut_ptr().cast(), - Attributes: 0, - }; - assert_ne!( - Security::CreateRestrictedToken( - token.as_raw_handle(), - Security::DISABLE_MAX_PRIVILEGE, - 0, - ptr::null(), - 0, - ptr::null(), - 1, - &sid, - &mut restricted, - ), - 0 - ); - OwnedHandle::from_raw_handle(restricted) - } - } - - fn token_id(token: &OwnedHandle) -> (u32, i32) { - let mut stats = Security::TOKEN_STATISTICS::default(); - let mut size = 0; - // SAFETY: The token is valid and the output buffer matches the queried - // information class. - unsafe { - assert_ne!( - Security::GetTokenInformation( - token.as_raw_handle(), - Security::TokenStatistics, - ptr::from_mut(&mut stats).cast(), - size_of_val(&stats) as u32, - &mut size, - ), - 0 - ); - } - (stats.TokenId.LowPart, stats.TokenId.HighPart) - } - - struct TokenCheckingFs { - token_id: (u32, i32), - calls: Arc, - } - - impl TokenCheckingFs { - fn check_identity(&self) { - let token = thread_token().unwrap(); - assert_eq!(token_id(&token), self.token_id); - // SAFETY: The handle refers to a valid token. - let restricted = unsafe { Security::IsTokenRestricted(token.as_raw_handle()) }; - assert_ne!(restricted, 0); - self.calls.fetch_add(1, Ordering::Relaxed); - } - } - - impl Fuse for TokenCheckingFs { - fn init(&self, _info: &mut SessionInfo) { - self.check_identity(); - } - - fn get_attr(&self, _request: &Request, _flags: u32, _fh: u64) -> lx::Result { - self.check_identity(); - Err(lx::Error::EACCES) - } - } - - #[derive(Default)] - struct TestSender(Vec); - - impl ReplySender for TestSender { - fn send(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result<()> { - self.0 = bufs.iter().flat_map(|buf| buf.iter().copied()).collect(); - Ok(()) - } - } - - #[test] - fn session_dispatch_impersonates_restricted_token_and_reverts() { - assert_eq!( - thread_token().unwrap_err().raw_os_error(), - Some(ERROR_NO_TOKEN as i32) - ); - let token = restricted_token(); - let calls = Arc::new(AtomicUsize::new(0)); - let session = VirtioFsSession::new(TokenCheckingFs { - token_id: token_id(&token), - calls: calls.clone(), - }) - .with_user_token(Some(token)); - let init = fuse_init_in { - major: FUSE_KERNEL_VERSION, - minor: FUSE_KERNEL_MINOR_VERSION, - ..FromZeros::new_zeroed() - }; - let getattr = fuse_getattr_in::new_zeroed(); - for (opcode, args, expected_error) in [ - (FUSE_INIT, init.as_bytes(), 0), - (FUSE_GETATTR, getattr.as_bytes(), -lx::Error::EACCES.value()), - ] { - let header = fuse_in_header { - len: (size_of::() + args.len()) as u32, - opcode, - unique: 1, - nodeid: 1, - ..FromZeros::new_zeroed() - }; - let mut request = header.as_bytes().to_vec(); - request.extend_from_slice(args); - let mut sender = TestSender::default(); - session.dispatch(Request::new(request.as_slice()).unwrap(), &mut sender, None); - let (reply, _) = fuse_out_header::read_from_prefix(&sender.0).unwrap(); - assert_eq!(reply.error, expected_error); - assert_eq!( - thread_token().unwrap_err().raw_os_error(), - Some(ERROR_NO_TOKEN as i32) - ); - } - assert_eq!(calls.load(Ordering::Relaxed), 2); - } -} diff --git a/vm/devices/virtio/virtiofs/src/integration_tests.rs b/vm/devices/virtio/virtiofs/src/integration_tests.rs index 96314d69926..2f66ca0d81d 100644 --- a/vm/devices/virtio/virtiofs/src/integration_tests.rs +++ b/vm/devices/virtio/virtiofs/src/integration_tests.rs @@ -961,3 +961,213 @@ fn stale_directory_paths_do_not_follow_native_links() { assert!(!outside.path().join("renamed").exists()); } } + +/// Tests for serving a file system under a user token on Windows. +#[cfg(windows)] +mod user_token { + // UNSAFETY: Calling Win32 APIs and constructing owned handles from their + // outputs. + #![expect(unsafe_code)] + + use crate::session::VirtioFsSession; + use fuse::Fuse; + use fuse::ReplySender; + use fuse::Request; + use fuse::SessionInfo; + use fuse::protocol::*; + use pal::windows::security::RevertToSelfGuard; + use pal::windows::security::duplicate_impersonation_token; + use pal::windows::security::open_process_token; + use std::io; + use std::os::windows::io::AsHandle; + use std::os::windows::io::AsRawHandle; + use std::os::windows::io::FromRawHandle; + use std::os::windows::io::OwnedHandle; + use std::ptr; + use std::sync::Arc; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + use test_with_tracing::test; + use windows_sys::Win32::Foundation::ERROR_NO_TOKEN; + use windows_sys::Win32::Security; + use windows_sys::Win32::System::Threading::GetCurrentThread; + use windows_sys::Win32::System::Threading::OpenThreadToken; + use zerocopy::FromBytes; + use zerocopy::FromZeros; + use zerocopy::IntoBytes; + + fn process_token() -> OwnedHandle { + let token = open_process_token(Security::TOKEN_DUPLICATE).unwrap(); + duplicate_impersonation_token(token.as_handle()).unwrap() + } + + fn thread_token() -> io::Result { + let mut token = ptr::null_mut(); + // SAFETY: Calling Win32 API as documented. + unsafe { + if OpenThreadToken(GetCurrentThread(), Security::TOKEN_QUERY, 1, &mut token) == 0 { + return Err(io::Error::last_os_error()); + } + Ok(OwnedHandle::from_raw_handle(token)) + } + } + + #[test] + fn rejects_non_token_handle() { + let file = std::fs::File::open(std::env::current_exe().unwrap()).unwrap(); + duplicate_impersonation_token(file.as_handle()).unwrap_err(); + } + + #[test] + fn impersonates_until_dropped() { + let process_token = process_token(); + let token = duplicate_impersonation_token(process_token.as_handle()).unwrap(); + { + let _guard = RevertToSelfGuard::impersonate(&token).unwrap(); + thread_token().unwrap(); + } + + let err = thread_token().unwrap_err(); + assert_eq!(err.raw_os_error(), Some(ERROR_NO_TOKEN as i32)); + } + + fn restricted_token() -> OwnedHandle { + let token = process_token(); + let mut world_sid = [0u32; Security::SECURITY_MAX_SID_SIZE as usize / size_of::()]; + let mut sid_size = size_of_val(&world_sid) as u32; + let mut restricted = ptr::null_mut(); + // SAFETY: The SID buffer is aligned and large enough for a well-known + // SID. All inputs remain valid until the new token handle is returned. + unsafe { + assert_ne!( + Security::CreateWellKnownSid( + Security::WinWorldSid, + ptr::null_mut(), + world_sid.as_mut_ptr().cast(), + &mut sid_size, + ), + 0 + ); + let sid = Security::SID_AND_ATTRIBUTES { + Sid: world_sid.as_mut_ptr().cast(), + Attributes: 0, + }; + assert_ne!( + Security::CreateRestrictedToken( + token.as_raw_handle(), + Security::DISABLE_MAX_PRIVILEGE, + 0, + ptr::null(), + 0, + ptr::null(), + 1, + &sid, + &mut restricted, + ), + 0 + ); + OwnedHandle::from_raw_handle(restricted) + } + } + + fn token_id(token: &OwnedHandle) -> (u32, i32) { + let mut stats = Security::TOKEN_STATISTICS::default(); + let mut size = 0; + // SAFETY: The token is valid and the output buffer matches the queried + // information class. + unsafe { + assert_ne!( + Security::GetTokenInformation( + token.as_raw_handle(), + Security::TokenStatistics, + ptr::from_mut(&mut stats).cast(), + size_of_val(&stats) as u32, + &mut size, + ), + 0 + ); + } + (stats.TokenId.LowPart, stats.TokenId.HighPart) + } + + struct TokenCheckingFs { + token_id: (u32, i32), + calls: Arc, + } + + impl TokenCheckingFs { + fn check_identity(&self) { + let token = thread_token().unwrap(); + assert_eq!(token_id(&token), self.token_id); + // SAFETY: The handle refers to a valid token. + let restricted = unsafe { Security::IsTokenRestricted(token.as_raw_handle()) }; + assert_ne!(restricted, 0); + self.calls.fetch_add(1, Ordering::Relaxed); + } + } + + impl Fuse for TokenCheckingFs { + fn init(&self, _info: &mut SessionInfo) { + self.check_identity(); + } + + fn get_attr(&self, _request: &Request, _flags: u32, _fh: u64) -> lx::Result { + self.check_identity(); + Err(lx::Error::EACCES) + } + } + + #[derive(Default)] + struct TestSender(Vec); + + impl ReplySender for TestSender { + fn send(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result<()> { + self.0 = bufs.iter().flat_map(|buf| buf.iter().copied()).collect(); + Ok(()) + } + } + + #[test] + fn session_dispatch_impersonates_restricted_token_and_reverts() { + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + let token = restricted_token(); + let calls = Arc::new(AtomicUsize::new(0)); + let session = VirtioFsSession::new(TokenCheckingFs { + token_id: token_id(&token), + calls: calls.clone(), + }) + .with_user_token(Some(token)); + let init = fuse_init_in { + major: FUSE_KERNEL_VERSION, + minor: FUSE_KERNEL_MINOR_VERSION, + ..FromZeros::new_zeroed() + }; + let getattr = fuse_getattr_in::new_zeroed(); + for (opcode, args, expected_error) in [ + (FUSE_INIT, init.as_bytes(), 0), + (FUSE_GETATTR, getattr.as_bytes(), -lx::Error::EACCES.value()), + ] { + let header = fuse_in_header { + len: (size_of::() + args.len()) as u32, + opcode, + unique: 1, + nodeid: 1, + ..FromZeros::new_zeroed() + }; + let mut request = header.as_bytes().to_vec(); + request.extend_from_slice(args); + let mut sender = TestSender::default(); + session.dispatch(Request::new(request.as_slice()).unwrap(), &mut sender, None); + let (reply, _) = fuse_out_header::read_from_prefix(&sender.0).unwrap(); + assert_eq!(reply.error, expected_error); + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + } + assert_eq!(calls.load(Ordering::Relaxed), 2); + } +} diff --git a/vm/devices/virtio/virtiofs/src/lib.rs b/vm/devices/virtio/virtiofs/src/lib.rs index ea17b642e38..84546f5867a 100644 --- a/vm/devices/virtio/virtiofs/src/lib.rs +++ b/vm/devices/virtio/virtiofs/src/lib.rs @@ -6,8 +6,6 @@ mod aggregate; mod file; -#[cfg(windows)] -mod impersonation; mod inode; #[cfg(test)] mod integration_tests; diff --git a/vm/devices/virtio/virtiofs/src/resolver.rs b/vm/devices/virtio/virtiofs/src/resolver.rs index 69256dc8f10..85eedf44fd7 100644 --- a/vm/devices/virtio/virtiofs/src/resolver.rs +++ b/vm/devices/virtio/virtiofs/src/resolver.rs @@ -161,8 +161,9 @@ impl ResolveResource for VirtioFsResolver { cfg_if! { if #[cfg(windows)] { - use crate::impersonation::duplicate_user_token; + use pal::windows::security::duplicate_impersonation_token; use pal::windows::security::RevertToSelfGuard; + use std::os::windows::io::AsHandle; use std::os::windows::io::OwnedHandle; struct ParsedHostFsPlatform { @@ -172,10 +173,8 @@ cfg_if! { impl ParsedHostFsPlatform { fn parse(windows_options: Option) -> anyhow::Result { let user_token = windows_options - .as_ref() - .map(|options| options.user_token_handle) - .filter(|handle| *handle != 0) - .map(duplicate_user_token) + .and_then(|options| options.user_token) + .map(|token| duplicate_impersonation_token(token.as_handle())) .transpose() .context("invalid virtio-fs user token")?; @@ -208,12 +207,8 @@ cfg_if! { impl ParsedHostFsPlatform { fn parse(windows_options: Option) -> anyhow::Result { - if let Some(options) = windows_options { - anyhow::ensure!( - options.user_token_handle == 0, - "virtio-fs windows_options are only supported on Windows" - ); - } + // The options carry no fields on non-Windows hosts. + let _ = windows_options; Ok(Self) } @@ -284,23 +279,7 @@ mod tests { #[cfg(not(windows))] #[test] - fn parse_rejects_windows_backends_and_tokens_on_non_windows() { - let mut resource = host_fs("tag", "root", ""); - let VirtioFsBackend::HostFs { - windows_options, .. - } = &mut resource.fs - else { - unreachable!(); - }; - *windows_options = Some(virtio_resources::fs::VirtioFsWindowsOptions { - user_token_handle: 1, - }); - let error = ParsedVirtioFs::parse(resource).err().unwrap(); - assert_eq!( - error.to_string(), - "virtio-fs windows_options are only supported on Windows" - ); - + fn parse_rejects_windows_backends_on_non_windows() { let resource = VirtioFsHandle { tag: "tag".to_owned(), fs: VirtioFsBackend::SectionFs { diff --git a/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs b/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs index addf32758a7..f0fad3fb864 100644 --- a/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs +++ b/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs @@ -4,7 +4,7 @@ //! Integration test for OpenVMM's fd-passing protocol on the TTRPC endpoint. //! //! This boots a real `openvmm --rpc` process and drives the bespoke -//! fd-passing protocol (see `openvmm_ttrpc_vmservice/src/fd_passing.md`) over +//! fd-passing protocol (see `openvmm_ttrpc_vmservice/src/resource_passing.md`) over //! the same Unix socket, then confirms that a descriptor registered on the //! fd-passing connection is resolvable by name from a *separate* TTRPC //! `CreateVm` connection via a `TapBackend { fd_name }` NIC. From a8f4d1b26dcdc8b96601bf18cb3c78163d23b497 Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Wed, 4 Nov 2026 16:42:36 -0800 Subject: [PATCH 6/8] virtiofs: replace cfg_if with cfg_select main replaced the cfg-if crate with the standard cfg_select! macro (#4883). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Cargo.lock | 1 - vm/devices/virtio/virtiofs/Cargo.toml | 1 - vm/devices/virtio/virtiofs/src/resolver.rs | 8 ++++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2880922d7dd..0dd3703fe62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10620,7 +10620,6 @@ name = "virtiofs" version = "0.0.0" dependencies = [ "anyhow", - "cfg-if", "fuse", "futures", "guestmem", diff --git a/vm/devices/virtio/virtiofs/Cargo.toml b/vm/devices/virtio/virtiofs/Cargo.toml index a493026f5af..e106c08a389 100644 --- a/vm/devices/virtio/virtiofs/Cargo.toml +++ b/vm/devices/virtio/virtiofs/Cargo.toml @@ -22,7 +22,6 @@ task_control.workspace = true tracelimit.workspace = true anyhow.workspace = true -cfg-if.workspace = true futures.workspace = true pal_async.workspace = true parking_lot.workspace = true diff --git a/vm/devices/virtio/virtiofs/src/resolver.rs b/vm/devices/virtio/virtiofs/src/resolver.rs index 85eedf44fd7..13c83b5a096 100644 --- a/vm/devices/virtio/virtiofs/src/resolver.rs +++ b/vm/devices/virtio/virtiofs/src/resolver.rs @@ -7,7 +7,6 @@ use crate::VirtioFs; use crate::virtio::VirtioFsDevice; use crate::virtio::VirtioFsDeviceOptions; use anyhow::Context as _; -use cfg_if::cfg_if; use lxutil::LxVolumeOptions; use virtio::resolve::ResolvedVirtioDevice; use virtio::resolve::VirtioResolveInput; @@ -159,8 +158,8 @@ impl ResolveResource for VirtioFsResolver { } } -cfg_if! { - if #[cfg(windows)] { +cfg_select! { + windows => { use pal::windows::security::duplicate_impersonation_token; use pal::windows::security::RevertToSelfGuard; use std::os::windows::io::AsHandle; @@ -202,7 +201,8 @@ cfg_if! { } } } - } else { + } + _ => { struct ParsedHostFsPlatform; impl ParsedHostFsPlatform { From 5e20a6303e6f6c97183cd713502e20764b577446 Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Wed, 4 Nov 2026 16:55:12 -0800 Subject: [PATCH 7/8] Add test for handle passing --- Cargo.lock | 1 + openvmm/openvmm_entry/src/ttrpc/fd_passing.rs | 20 +- .../openvmm_entry/src/ttrpc/handle_passing.rs | 24 +-- .../src/ttrpc/resource_passing.rs | 28 ++- vmm_tests/vmm_tests/Cargo.toml | 1 + vmm_tests/vmm_tests/tests/tests/ttrpc.rs | 45 ++++- .../vmm_tests/tests/tests/ttrpc/fd_passing.rs | 35 +--- .../tests/tests/ttrpc/handle_passing.rs | 181 ++++++++++++++++++ 8 files changed, 258 insertions(+), 77 deletions(-) create mode 100644 vmm_tests/vmm_tests/tests/tests/ttrpc/handle_passing.rs diff --git a/Cargo.lock b/Cargo.lock index 0dd3703fe62..9f11b82a62e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11387,6 +11387,7 @@ dependencies = [ "vmgs_resources", "vmm_test_igvm_agent", "vmm_test_macros", + "windows-sys 0.61.0", "x509-cert", "x86defs", "zerocopy", diff --git a/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs b/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs index ae5dc18fe1b..8cb9da07d3c 100644 --- a/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs +++ b/openvmm/openvmm_entry/src/ttrpc/fd_passing.rs @@ -1,23 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! The OpenVMM fd-passing protocol. +//! The unix transport of the resource-passing protocol: fd-passing. //! -//! This is a small, bespoke protocol that lets a client hand pre-opened file -//! descriptors to the management server, each under a client-chosen name. A -//! name can then be referenced from the ordinary ttrpc/gRPC API (for example a -//! `TapBackend` may give an `fd_name` instead of a device `name`). +//! A `Register` request carries its file descriptor as `SCM_RIGHTS` ancillary +//! data attached to the request. Everything else is shared in +//! `resource_passing`. //! -//! It runs over the same `AF_UNIX` stream socket as ttrpc and gRPC, selected by -//! a distinct magic first byte, and carries descriptors via `SCM_RIGHTS`. See -//! `openvmm_ttrpc_vmservice/src/resource_passing.md` for the wire specification. -//! -//! This module implements only how descriptors arrive; the handshake, framing, -//! request semantics, and registry are shared with the Windows handle-passing -//! protocol in `resource_passing`. -//! -//! The entire protocol is UNIX-only (it relies on `SCM_RIGHTS`), so this whole -//! module is compiled only on `cfg(unix)`. +//! `SCM_RIGHTS` is unix-only, so this module is compiled only on `cfg(unix)`. #![cfg(unix)] diff --git a/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs b/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs index f541b243e81..dd429a47525 100644 --- a/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs +++ b/openvmm/openvmm_entry/src/ttrpc/handle_passing.rs @@ -1,25 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! The OpenVMM handle-passing protocol. +//! The Windows transport of the resource-passing protocol: handle-passing. //! -//! This is the Windows counterpart of the fd-passing protocol: it lets a client -//! hand OS handles (for example, a user's access token) to the management -//! server, each under a client-chosen name. A name can then be referenced from -//! the ordinary ttrpc/gRPC API (for example `VirtioFs.WindowsOptions` -//! `user_token_name`). -//! -//! It runs over the same `AF_UNIX` stream socket as ttrpc and gRPC, selected by -//! the same magic first byte as fd-passing. Windows `AF_UNIX` sockets cannot -//! carry handles, so the client duplicates each handle into this process with -//! `DuplicateHandle` and sends its value in the `Register` request. This is the -//! only place OpenVMM interprets a client-supplied handle value; the server -//! makes its own duplicate and never closes the client's. -//! -//! This module implements only how handles arrive; the handshake, framing, -//! request semantics, and registry are shared with fd-passing in -//! `resource_passing`. See `openvmm_ttrpc_vmservice/src/resource_passing.md` for -//! the wire specification. +//! Windows `AF_UNIX` sockets cannot carry handles, so the client duplicates +//! each handle into this process with `DuplicateHandle` and appends its value +//! to the `Register` request. This is the only place OpenVMM interprets a +//! client-supplied handle value; the server makes its own duplicate and never +//! closes the client's. Everything else is shared in `resource_passing`. #![cfg(windows)] diff --git a/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs b/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs index 2bd3c0b8176..190fdd81c75 100644 --- a/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs +++ b/openvmm/openvmm_entry/src/ttrpc/resource_passing.rs @@ -1,17 +1,25 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! The transport-independent core of the fd-passing (unix) and handle-passing -//! (Windows) protocols. +//! The OpenVMM resource-passing protocols: fd-passing (unix) and +//! handle-passing (Windows). //! -//! Both protocols run over the same `AF_UNIX` stream socket as ttrpc and gRPC, -//! selected by the magic first byte [`wire::MAGIC_FIRST_BYTE`], and let a -//! client register OS resources under client-chosen names that the ttrpc/gRPC -//! API can then reference. They share the handshake, request and response -//! framing, request semantics, and the registry. They differ only in how a -//! `Register` request carries its resource, which a [`Transport`] implements: -//! fd-passing attaches a descriptor with `SCM_RIGHTS`, and handle-passing -//! appends a handle value the client duplicated into this process. +//! These are small, bespoke protocols that let a client hand pre-opened OS +//! resources (file descriptors or handles) to the management server, each under +//! a client-chosen name. A name can then be referenced from the ordinary +//! ttrpc/gRPC API, for example a `TapBackend` `fd_name` or a `VirtioFs` +//! `WindowsOptions.user_token_name`. +//! +//! Both run over the same `AF_UNIX` stream socket as ttrpc and gRPC, selected +//! by the magic first byte [`wire::MAGIC_FIRST_BYTE`]. See +//! `openvmm_ttrpc_vmservice/src/resource_passing.md` for the wire +//! specification. +//! +//! This module implements everything the protocols share: the handshake, +//! request and response framing, request semantics, and the registry. They +//! differ only in how a `Register` request carries its resource, which a +//! [`Transport`] implements in the platform modules `fd_passing` and +//! `handle_passing`. use anyhow::Context as _; use parking_lot::Mutex; diff --git a/vmm_tests/vmm_tests/Cargo.toml b/vmm_tests/vmm_tests/Cargo.toml index 6e1bc02bda8..13d7324ac4a 100644 --- a/vmm_tests/vmm_tests/Cargo.toml +++ b/vmm_tests/vmm_tests/Cargo.toml @@ -94,6 +94,7 @@ virt_mshv.workspace = true [target.'cfg(windows)'.dev-dependencies] der.workspace = true virt_whp.workspace = true +windows-sys = { workspace = true, features = ["Win32_Security"] } x509-cert.workspace = true [build-dependencies] diff --git a/vmm_tests/vmm_tests/tests/tests/ttrpc.rs b/vmm_tests/vmm_tests/tests/tests/ttrpc.rs index 7e730024fc2..2fc799113fb 100644 --- a/vmm_tests/vmm_tests/tests/tests/ttrpc.rs +++ b/vmm_tests/vmm_tests/tests/tests/ttrpc.rs @@ -3,11 +3,13 @@ //! Integration tests for OpenVMM's TTRPC interface. -// Tests for the fd-passing protocol, which shares the TTRPC socket. It is -// UNIX-only and tap `fd_name` resolution is Linux-only, so it only builds on -// Linux. +// Tests for the resource-passing protocols, which share the TTRPC socket. The +// UNIX fd-passing test resolves a tap `fd_name`, which is Linux-only; the +// Windows handle-passing test resolves a virtio-fs user token. #[cfg(target_os = "linux")] mod fd_passing; +#[cfg(windows)] +mod handle_passing; use anyhow::Context; use futures::AsyncBufReadExt; @@ -2211,6 +2213,43 @@ async fn launch_openvmm( Ok((child, client, stderr_task)) } +/// Sends a resource-passing client handshake with `magic` and validates the +/// server's reply. See `openvmm_ttrpc_vmservice/src/resource_passing.md`. +#[cfg(any(target_os = "linux", windows))] +async fn resource_passing_handshake( + sock: &mut PolledSocket, + magic: [u8; 4], +) -> anyhow::Result<()> { + use futures::AsyncWriteExt as _; + + let mut client_handshake = [0u8; 8]; + client_handshake[..4].copy_from_slice(&magic); + sock.write_all(&client_handshake).await?; + let mut server_handshake = [0u8; 8]; + sock.read_exact(&mut server_handshake).await?; + anyhow::ensure!( + server_handshake[..4] == magic, + "bad server handshake magic: {:x?}", + &server_handshake[..4] + ); + Ok(()) +} + +/// Reads one resource-passing response frame: `status: u8`, `msg_len: u16` +/// (LE), `msg`. +#[cfg(any(target_os = "linux", windows))] +async fn read_resource_passing_response( + sock: &mut PolledSocket, +) -> anyhow::Result<(u8, String)> { + let mut hdr = [0u8; 3]; + sock.read_exact(&mut hdr).await?; + let status = hdr[0]; + let msg_len = u16::from_le_bytes([hdr[1], hdr[2]]) as usize; + let mut msg = vec![0u8; msg_len]; + sock.read_exact(&mut msg).await?; + Ok((status, String::from_utf8_lossy(&msg).into_owned())) +} + /// Builds a file-backed disk backend for the given path. fn file_disk(path: &Path) -> vmservice::DiskBackend { vmservice::DiskBackend { diff --git a/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs b/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs index f0fad3fb864..596151b5ce3 100644 --- a/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs +++ b/vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs @@ -14,7 +14,6 @@ //! `ttrpc` module. use anyhow::Context; -use futures::AsyncReadExt as _; use futures::AsyncWriteExt as _; use guid::Guid; use openvmm_ttrpc_vmservice as vmservice; @@ -68,7 +67,7 @@ async fn fd_passing_tap( let mut fd_conn = PolledSocket::connect_unix(&driver, &socket_path) .await .context("connecting fd-passing socket")?; - handshake(&mut fd_conn) + super::resource_passing_handshake(&mut fd_conn, HANDSHAKE_MAGIC) .await .context("fd-passing handshake")?; @@ -157,21 +156,6 @@ async fn fd_passing_tap( Ok(()) } -/// Sends the client handshake and validates the server's reply. -async fn handshake(sock: &mut PolledSocket) -> anyhow::Result<()> { - let mut client_handshake = [0u8; 8]; - client_handshake[..4].copy_from_slice(&HANDSHAKE_MAGIC); - sock.write_all(&client_handshake).await?; - let mut server_handshake = [0u8; 8]; - sock.read_exact(&mut server_handshake).await?; - anyhow::ensure!( - server_handshake[..4] == HANDSHAKE_MAGIC, - "bad server handshake magic: {:x?}", - &server_handshake[..4] - ); - Ok(()) -} - /// Sends a `Register` request carrying `fd` and returns the response. async fn register( sock: &mut PolledSocket, @@ -181,7 +165,7 @@ async fn register( let mut frame = vec![OPCODE_REGISTER, name.len() as u8]; frame.extend_from_slice(name.as_bytes()); send_frame(sock, &frame, &[fd]).await?; - read_response(sock).await + super::read_resource_passing_response(sock).await } /// Sends a `Register` request with no attached descriptor (an error case). @@ -192,7 +176,7 @@ async fn register_without_fd( let mut frame = vec![OPCODE_REGISTER, name.len() as u8]; frame.extend_from_slice(name.as_bytes()); sock.write_all(&frame).await?; - read_response(sock).await + super::read_resource_passing_response(sock).await } /// Sends a `Deregister` request and returns the response. @@ -203,7 +187,7 @@ async fn deregister( let mut frame = vec![OPCODE_DEREGISTER, name.len() as u8]; frame.extend_from_slice(name.as_bytes()); sock.write_all(&frame).await?; - read_response(sock).await + super::read_resource_passing_response(sock).await } /// Sends `frame` with `fds` attached, awaiting write readiness so the executor @@ -238,17 +222,6 @@ async fn send_frame( Ok(()) } -/// Reads one response frame: `status: u8`, `msg_len: u16` (LE), `msg`. -async fn read_response(sock: &mut PolledSocket) -> anyhow::Result<(u8, String)> { - let mut hdr = [0u8; 3]; - sock.read_exact(&mut hdr).await?; - let status = hdr[0]; - let msg_len = u16::from_le_bytes([hdr[1], hdr[2]]) as usize; - let mut msg = vec![0u8; msg_len]; - sock.read_exact(&mut msg).await?; - Ok((status, String::from_utf8_lossy(&msg).into_owned())) -} - /// Builds a minimal direct-boot `CreateVm` request with a single tap NIC whose /// backing descriptor is resolved by `fd_name`. fn create_vm_request( diff --git a/vmm_tests/vmm_tests/tests/tests/ttrpc/handle_passing.rs b/vmm_tests/vmm_tests/tests/tests/ttrpc/handle_passing.rs new file mode 100644 index 00000000000..19e9622bb5b --- /dev/null +++ b/vmm_tests/vmm_tests/tests/tests/ttrpc/handle_passing.rs @@ -0,0 +1,181 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Integration test for OpenVMM's handle-passing protocol on the TTRPC +//! endpoint. +//! +//! This boots a real `openvmm --rpc` process, registers an access token by +//! name over the handle-passing protocol (see +//! `openvmm_ttrpc_vmservice/src/resource_passing.md`), and confirms that a +//! virtio-fs share's `WindowsOptions.user_token_name` resolves through that +//! registration during `CreateVm`. How virtio-fs uses the token is covered by +//! the virtiofs crate's tests. + +use anyhow::Context; +use futures::AsyncWriteExt as _; +use openvmm_ttrpc_vmservice as vmservice; +use pal::windows::BorrowedHandleExt; +use pal::windows::security::open_process_token; +use pal_async::DefaultDriver; +use pal_async::socket::PolledSocket; +use petri::ResolvedArtifact; +use petri_artifacts_vmm_test::artifacts; +use std::os::windows::io::AsHandle; +use std::os::windows::io::AsRawHandle; +use std::path::Path; +use unix_socket::UnixStream; +use windows_sys::Win32::Security::TOKEN_DUPLICATE; +use windows_sys::Win32::Security::TOKEN_QUERY; + +/// The 4-byte handle-passing handshake magic: `0xFD 'H' 'P' 0x01`. +const HANDSHAKE_MAGIC: [u8; 4] = [0xFD, b'H', b'P', 0x01]; + +const OPCODE_REGISTER: u8 = 1; +const OPCODE_DEREGISTER: u8 = 2; + +const TOKEN_NAME: &str = "user-token"; + +petri::test!(handle_passing_virtiofs_user_token, |resolver| { + let openvmm = resolver.require(artifacts::OPENVMM_NATIVE); + let kernel = resolver.require(artifacts::loadable::LINUX_DIRECT_TEST_KERNEL_NATIVE); + let initrd = resolver.require(artifacts::loadable::LINUX_DIRECT_TEST_INITRD_NATIVE); + Some([openvmm.erase(), kernel.erase(), initrd.erase()]) +}); + +async fn handle_passing_virtiofs_user_token( + params: petri::PetriTestParams<'_>, + driver: DefaultDriver, + [openvmm, kernel_path, initrd_path]: [ResolvedArtifact; 3], +) -> anyhow::Result<()> { + let tempdir = tempfile::tempdir()?; + let socket_path = tempdir.path().join("ttrpc.sock"); + let pidfile_path = tempdir.path().join("openvmm.pid"); + let share_root = tempdir.path().join("share"); + std::fs::create_dir(&share_root)?; + + // A real client would `DuplicateHandle` the token into OpenVMM. Making it + // inheritable before spawning OpenVMM gives the child a copy with the same + // value, which avoids cross-process handle manipulation in this test. + let token = open_process_token(TOKEN_DUPLICATE | TOKEN_QUERY)? + .as_handle() + .duplicate(true, None)?; + let token_value = token.as_raw_handle() as u64; + let (mut child, client, stderr_task) = + super::launch_openvmm(&driver, ¶ms, &openvmm, &socket_path, &pidfile_path).await?; + drop(token); + + let mut conn = PolledSocket::connect_unix(&driver, &socket_path) + .await + .context("connecting handle-passing socket")?; + super::resource_passing_handshake(&mut conn, HANDSHAKE_MAGIC) + .await + .context("handle-passing handshake")?; + let (status, msg) = request(&mut conn, OPCODE_REGISTER, TOKEN_NAME, Some(token_value)).await?; + anyhow::ensure!(status == 0, "register '{TOKEN_NAME}' failed: {msg}"); + + // An unregistered name fails to resolve. + let err = client + .call() + .start( + vmservice::Vm::CreateVm, + create_vm_request(&kernel_path, &initrd_path, &share_root, "does-not-exist"), + ) + .await + .unwrap_err(); + anyhow::ensure!( + err.message + .contains("failed to resolve virtio-fs user token"), + "expected a user token resolution error, got: {}", + err.message + ); + + // The registered name resolves to the passed token. + client + .call() + .start( + vmservice::Vm::CreateVm, + create_vm_request(&kernel_path, &initrd_path, &share_root, TOKEN_NAME), + ) + .await + .map_err(|err| anyhow::anyhow!("CreateVm failed: {}", err.message))?; + + let (status, msg) = request(&mut conn, OPCODE_DEREGISTER, TOKEN_NAME, None).await?; + anyhow::ensure!(status == 0, "deregister '{TOKEN_NAME}' failed: {msg}"); + drop(conn); + + client + .call() + .start(vmservice::Vm::TeardownVm, ()) + .await + .map_err(|err| anyhow::anyhow!("TeardownVm failed: {}", err.message))?; + let _ = client.call().start(vmservice::Vm::Quit, ()).await; + let exit_status = child.wait().await?; + stderr_task + .await + .context("failed to drain openvmm stderr after process exit")?; + tracing::info!(?exit_status, "openvmm exited"); + anyhow::ensure!( + exit_status.success(), + "openvmm exited abnormally: {exit_status:?}" + ); + Ok(()) +} + +/// Sends a request, with an inline handle value for `Register`, and returns +/// the response. +async fn request( + sock: &mut PolledSocket, + opcode: u8, + name: &str, + handle: Option, +) -> anyhow::Result<(u8, String)> { + let mut frame = vec![opcode, name.len() as u8]; + frame.extend_from_slice(name.as_bytes()); + if let Some(handle) = handle { + frame.extend_from_slice(&handle.to_le_bytes()); + } + sock.write_all(&frame).await?; + super::read_resource_passing_response(sock).await +} + +/// Builds a direct-boot `CreateVm` request that exposes `share_root` over +/// virtio-fs, accessed as the token registered as `user_token_name`. +fn create_vm_request( + kernel_path: &ResolvedArtifact, + initrd_path: &ResolvedArtifact, + share_root: &Path, + user_token_name: &str, +) -> vmservice::CreateVmRequest { + vmservice::CreateVmRequest { + config: Some(vmservice::VmConfig { + memory_config: Some(vmservice::MemoryConfig { + memory_mb: 256, + ..Default::default() + }), + processor_config: Some(vmservice::ProcessorConfig { + processor_count: 1, + ..Default::default() + }), + boot_config: Some(vmservice::vm_config::BootConfig::DirectBoot( + vmservice::DirectBoot { + kernel_path: kernel_path.get().to_string_lossy().into_owned(), + initrd_path: initrd_path.get().to_string_lossy().into_owned(), + kernel_cmdline: String::new(), + }, + )), + devices_config: Some(vmservice::DevicesConfig { + virtiofs_config: vec![vmservice::VirtioFs { + tag: "userfs".to_string(), + root_path: share_root.to_string_lossy().into_owned(), + read_only: false, + windows_options: Some(vmservice::virtio_fs::WindowsOptions { + user_token_name: user_token_name.to_string(), + }), + }], + ..Default::default() + }), + ..Default::default() + }), + log_id: String::new(), + } +} From 44efd34a1b00ebe9ded25b1819046a186374c92b Mon Sep 17 00:00:00 2001 From: Daman Mulye Date: Wed, 4 Nov 2026 17:44:20 -0800 Subject: [PATCH 8/8] add more tests --- .../virtio/virtiofs/src/integration_tests.rs | 41 +++++++++++- vm/devices/virtio/virtiofs/src/resolver.rs | 63 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/vm/devices/virtio/virtiofs/src/integration_tests.rs b/vm/devices/virtio/virtiofs/src/integration_tests.rs index 2f66ca0d81d..f48e3afd73a 100644 --- a/vm/devices/virtio/virtiofs/src/integration_tests.rs +++ b/vm/devices/virtio/virtiofs/src/integration_tests.rs @@ -964,7 +964,7 @@ fn stale_directory_paths_do_not_follow_native_links() { /// Tests for serving a file system under a user token on Windows. #[cfg(windows)] -mod user_token { +pub(crate) mod user_token { // UNSAFETY: Calling Win32 APIs and constructing owned handles from their // outputs. #![expect(unsafe_code)] @@ -1001,7 +1001,7 @@ mod user_token { duplicate_impersonation_token(token.as_handle()).unwrap() } - fn thread_token() -> io::Result { + pub(crate) fn thread_token() -> io::Result { let mut token = ptr::null_mut(); // SAFETY: Calling Win32 API as documented. unsafe { @@ -1031,7 +1031,9 @@ mod user_token { assert_eq!(err.raw_os_error(), Some(ERROR_NO_TOKEN as i32)); } - fn restricted_token() -> OwnedHandle { + /// Returns an impersonation token for this process restricted to the + /// Everyone SID. + pub(crate) fn restricted_token() -> OwnedHandle { let token = process_token(); let mut world_sid = [0u32; Security::SECURITY_MAX_SID_SIZE as usize / size_of::()]; let mut sid_size = size_of_val(&world_sid) as u32; @@ -1170,4 +1172,37 @@ mod user_token { } assert_eq!(calls.load(Ordering::Relaxed), 2); } + + #[test] + fn session_dispatch_fails_closed_when_impersonation_fails() { + // A non-token handle makes impersonation fail. + let file = std::fs::File::open(std::env::current_exe().unwrap()).unwrap(); + let calls = Arc::new(AtomicUsize::new(0)); + let session = VirtioFsSession::new(TokenCheckingFs { + token_id: (0, 0), + calls: calls.clone(), + }) + .with_user_token(Some(file.into())); + let getattr = fuse_getattr_in::new_zeroed(); + let header = fuse_in_header { + len: (size_of::() + size_of_val(&getattr)) as u32, + opcode: FUSE_GETATTR, + unique: 7, + nodeid: 1, + ..FromZeros::new_zeroed() + }; + let mut request = header.as_bytes().to_vec(); + request.extend_from_slice(getattr.as_bytes()); + let mut sender = TestSender::default(); + session.dispatch(Request::new(request.as_slice()).unwrap(), &mut sender, None); + + let (reply, _) = fuse_out_header::read_from_prefix(&sender.0).unwrap(); + assert_eq!(reply.unique, 7); + assert_eq!(reply.error, -lx::Error::EIO.value()); + assert_eq!(calls.load(Ordering::Relaxed), 0); + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + } } diff --git a/vm/devices/virtio/virtiofs/src/resolver.rs b/vm/devices/virtio/virtiofs/src/resolver.rs index 13c83b5a096..553e28db413 100644 --- a/vm/devices/virtio/virtiofs/src/resolver.rs +++ b/vm/devices/virtio/virtiofs/src/resolver.rs @@ -292,4 +292,67 @@ mod tests { "section fs not supported on this platform" ); } + + #[cfg(windows)] + #[test] + fn open_fs_uses_user_token() { + use crate::integration_tests::user_token::restricted_token; + use crate::integration_tests::user_token::thread_token; + use pal::windows::security::LocalSecurityDescriptor; + use pal::windows::security::create_directory_with_security; + use virtio_resources::fs::VirtioFsWindowsOptions; + use windows_sys::Win32::Foundation::ERROR_NO_TOKEN; + + fn assert_no_thread_token() { + assert_eq!( + thread_token().unwrap_err().raw_os_error(), + Some(ERROR_NO_TOKEN as i32) + ); + } + + fn open(root: &std::path::Path, user_token: bool) -> anyhow::Result<()> { + let mut resource = host_fs("tag", root.to_str().unwrap(), ""); + let VirtioFsBackend::HostFs { + windows_options, .. + } = &mut resource.fs + else { + unreachable!(); + }; + *windows_options = Some(VirtioFsWindowsOptions { + user_token: user_token.then(restricted_token), + }); + let parsed = ParsedVirtioFs::parse(resource)?; + let ParsedVirtioFsBackend::HostFs { + root_path, + mount_options, + platform, + } = parsed.backend + else { + panic!("expected host filesystem backend"); + }; + platform.open_fs(&root_path, &mount_options)?; + Ok(()) + } + + // The restricted token passes an access check only if it is granted + // to Everyone, so it can open `public` but not `private`, which this + // process can open. + let dir = tempfile::tempdir().unwrap(); + let public = dir.path().join("public"); + let private = dir.path().join("private"); + for (path, sddl) in [ + (&public, "D:P(A;OICI;FA;;;WD)"), + (&private, "D:P(A;OICI;FA;;;AU)"), + ] { + let sd: LocalSecurityDescriptor = sddl.parse().unwrap(); + create_directory_with_security(path, &sd).unwrap(); + } + + open(&private, false).unwrap(); + assert_no_thread_token(); + open(&public, true).unwrap(); + assert_no_thread_token(); + open(&private, true).unwrap_err(); + assert_no_thread_token(); + } }