fix(host/security): scope Windows shared-section SDDL to SYSTEM+LocalService (close #5)
The gamepad host<->UMDF-driver shared sections (Global\pfds-shm-*, pfxusb-shm-*) and the IDD-push frame ring/event (Global\pfvd-*) were created with `D:(A;;GA;;;WD)` — GENERIC_ALL to **Everyone** — on the assumption the driver's WUDFHost ran under a restricted token needing broad access. So any local unprivileged user could OpenFileMapping the section to inject controller input, tamper the trusted HID channel, or read captured screen frames (security-review 2026-06-28 #5). On-box validation (RTX box, 2026-06-29) disproved the restricted-token premise: the WUDFHost token is NT AUTHORITY\LocalService (S-1-5-19), SYSTEM integrity, with ZERO restricted SIDs. So the section only needs SYSTEM (the host creates + writes it) and LocalService (the driver opens it). Scope both SDDL sites to `D:(A;;GA;;;SY)(A;;GA;;;LS)`; rename the now-misnamed `permissive_sa` -> `shared_object_sa`; correct the stale "restricted-token / Everyone" docs. Validated live: a full DualSense + 1280x720x60 session — 6943 frames received, HID output round-tripped, device status OK (pf_dualsense + pf_vdisplay WUDFHosts both LocalService open the scoped sections fine), while OpenFileMapping from a non-SYSTEM admin session now returns ACCESS_DENIED (was a granted handle under WD). Host-only change (the SDDL is set when the host CREATES the section); drivers unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,8 @@
|
|||||||
//! P2 direct frame push (kill DDA) — HOST side. The pf-vdisplay driver runs in a restricted WUDFHost
|
//! P2 direct frame push (kill DDA) — HOST side. The pf-vdisplay driver's WUDFHost canNOT create named
|
||||||
//! token that canNOT create named kernel objects, so — exactly like the gamepad UMDF drivers
|
//! kernel objects, so — exactly like the gamepad UMDF drivers (`inject/dualsense_windows.rs`) — the
|
||||||
//! (`inject/dualsense_windows.rs`) — the HOST (privileged) CREATES the shared header + frame-ready
|
//! HOST (privileged) CREATES the shared header + frame-ready event + ring of keyed-mutex textures
|
||||||
//! event + ring of keyed-mutex textures (`Global\` names, permissive `D:(A;;GA;;;WD)` SDDL) on the
|
//! (`Global\` names, scoped `D:(A;;GA;;;SY)(A;;GA;;;LS)` to SYSTEM + the driver's LocalService host —
|
||||||
//! discrete render GPU, and the driver only OPENS them and copies frames in. We then consume the ring
|
//! see `shared_object_sa`) on the discrete render GPU, and the driver only OPENS them and copies frames in. We then consume the ring
|
||||||
//! straight into the zero-copy NVENC path — no DXGI Desktop Duplication, no `win32u` hook. Gated by
|
//! straight into the zero-copy NVENC path — no DXGI Desktop Duplication, no `win32u` hook. Gated by
|
||||||
//! `PUNKTFUNK_IDD_PUSH`. Driver counterpart: `packaging/windows/drivers/pf-vdisplay/src/
|
//! `PUNKTFUNK_IDD_PUSH`. Driver counterpart: `packaging/windows/drivers/pf-vdisplay/src/
|
||||||
//! frame_transport.rs`. The shared `SharedHeader` layout, `MAGIC`/`VERSION`/`RING_LEN`, the
|
//! frame_transport.rs`. The shared `SharedHeader` layout, `MAGIC`/`VERSION`/`RING_LEN`, the
|
||||||
@@ -236,13 +236,17 @@ pub struct IddPushCapturer {
|
|||||||
// ownership to one thread at a time with NO concurrent access; we do not (and must not) claim `Sync`.
|
// ownership to one thread at a time with NO concurrent access; we do not (and must not) claim `Sync`.
|
||||||
unsafe impl Send for IddPushCapturer {}
|
unsafe impl Send for IddPushCapturer {}
|
||||||
|
|
||||||
/// Build a permissive (Everyone:GenericAll) `SECURITY_ATTRIBUTES` so the restricted WUDFHost driver
|
/// Build a `SECURITY_ATTRIBUTES` granting GENERIC_ALL to **SYSTEM** (the host creates+publishes the
|
||||||
/// can OPEN the host-created objects — the same `D:(A;;GA;;;WD)` SDDL the gamepad shared section uses.
|
/// shared event + texture ring) and **LocalService** (the account the pf_vdisplay WUDFHost runs under,
|
||||||
/// The returned `psd` backing must outlive `sa`; both are dropped when the process exits.
|
/// which consumes them) — `D:(A;;GA;;;SY)(A;;GA;;;LS)`, the same scoping as the gamepad section. The
|
||||||
unsafe fn permissive_sa() -> Result<(SECURITY_ATTRIBUTES, PSECURITY_DESCRIPTOR)> {
|
/// old SDDL granted **Everyone** (`WD`), which let any local user open the `Global\pfvd-*` objects and
|
||||||
|
/// read captured screen frames (security-review 2026-06-28 #5). Verified on the RTX box (2026-06-29):
|
||||||
|
/// the WUDFHost token is `S-1-5-19` (LocalService), SYSTEM integrity, zero restricted SIDs — so SY+LS
|
||||||
|
/// suffices for the driver and excludes normal user processes. `psd` must outlive `sa`.
|
||||||
|
unsafe fn shared_object_sa() -> Result<(SECURITY_ATTRIBUTES, PSECURITY_DESCRIPTOR)> {
|
||||||
let mut psd = PSECURITY_DESCRIPTOR::default();
|
let mut psd = PSECURITY_DESCRIPTOR::default();
|
||||||
ConvertStringSecurityDescriptorToSecurityDescriptorW(
|
ConvertStringSecurityDescriptorToSecurityDescriptorW(
|
||||||
w!("D:(A;;GA;;;WD)"),
|
w!("D:(A;;GA;;;SY)(A;;GA;;;LS)"),
|
||||||
SDDL_REVISION_1,
|
SDDL_REVISION_1,
|
||||||
&mut psd,
|
&mut psd,
|
||||||
None,
|
None,
|
||||||
@@ -269,7 +273,7 @@ impl IddPushCapturer {
|
|||||||
h: u32,
|
h: u32,
|
||||||
format: DXGI_FORMAT,
|
format: DXGI_FORMAT,
|
||||||
) -> Result<Vec<HostSlot>> {
|
) -> Result<Vec<HostSlot>> {
|
||||||
let (sa, _psd) = permissive_sa()?;
|
let (sa, _psd) = shared_object_sa()?;
|
||||||
let mut slots = Vec::new();
|
let mut slots = Vec::new();
|
||||||
for k in 0..RING_LEN {
|
for k in 0..RING_LEN {
|
||||||
let desc = D3D11_TEXTURE2D_DESC {
|
let desc = D3D11_TEXTURE2D_DESC {
|
||||||
@@ -375,7 +379,7 @@ impl IddPushCapturer {
|
|||||||
// SAFETY: one block over the whole ring setup; every operation in it is sound:
|
// SAFETY: one block over the whole ring setup; every operation in it is sound:
|
||||||
// - `set_advanced_color`/`advanced_color_enabled` are `unsafe fn`s taking only a copy of the plain
|
// - `set_advanced_color`/`advanced_color_enabled` are `unsafe fn`s taking only a copy of the plain
|
||||||
// `u32` target id; they read/flip CCD display config and return owned values, borrowing nothing.
|
// `u32` target id; they read/flip CCD display config and return owned values, borrowing nothing.
|
||||||
// - `CreateDXGIFactory1`, `EnumAdapterByLuid`, `make_device`, `permissive_sa`, `CreateFileMappingW`,
|
// - `CreateDXGIFactory1`, `EnumAdapterByLuid`, `make_device`, `shared_object_sa`, `CreateFileMappingW`,
|
||||||
// `MapViewOfFile`, `CreateEventW`, and `create_ring_slots` are all `?`-checked, so every returned
|
// `MapViewOfFile`, `CreateEventW`, and `create_ring_slots` are all `?`-checked, so every returned
|
||||||
// interface/handle/view is non-error before use; `&sa`/`&adapter`/`&device`/the `&HSTRING` names
|
// interface/handle/view is non-error before use; `&sa`/`&adapter`/`&device`/the `&HSTRING` names
|
||||||
// are live borrows that outlive each synchronous call, and `sa.lpSecurityDescriptor` stays valid
|
// are live borrows that outlive each synchronous call, and `sa.lpSecurityDescriptor` stays valid
|
||||||
@@ -421,7 +425,7 @@ impl IddPushCapturer {
|
|||||||
.context("EnumAdapterByLuid(render adapter) for IDD push")?;
|
.context("EnumAdapterByLuid(render adapter) for IDD push")?;
|
||||||
let (device, context) = make_device(&adapter).context("make_device for IDD push")?;
|
let (device, context) = make_device(&adapter).context("make_device for IDD push")?;
|
||||||
|
|
||||||
let (sa, _psd) = permissive_sa()?;
|
let (sa, _psd) = shared_object_sa()?;
|
||||||
let bytes = std::mem::size_of::<SharedHeader>().max(64);
|
let bytes = std::mem::size_of::<SharedHeader>().max(64);
|
||||||
|
|
||||||
// Header.
|
// Header.
|
||||||
|
|||||||
@@ -21,10 +21,18 @@ use windows::Win32::System::Memory::{
|
|||||||
MEMORY_MAPPED_VIEW_ADDRESS, PAGE_READWRITE,
|
MEMORY_MAPPED_VIEW_ADDRESS, PAGE_READWRITE,
|
||||||
};
|
};
|
||||||
|
|
||||||
/// A named, anonymous (pagefile-backed) shared section + its mapped read/write view, created with the
|
/// A named, anonymous (pagefile-backed) shared section + its mapped read/write view. RAII: drop unmaps
|
||||||
/// permissive `D:(A;;GA;;;WD)` SDDL the restricted-token driver needs to open it. RAII: drop unmaps the
|
/// the view, then the [`OwnedHandle`] closes the section handle (in that order). Replaces the three
|
||||||
/// view, then the [`OwnedHandle`] closes the section handle (in that order). Replaces the three backends'
|
/// backends' hand-duplicated `CreateFileMappingW` + `MapViewOfFile` + manual `Drop`.
|
||||||
/// hand-duplicated `CreateFileMappingW` + `MapViewOfFile` + manual `Drop`.
|
///
|
||||||
|
/// SDDL `D:(A;;GA;;;SY)(A;;GA;;;LS)`: GENERIC_ALL to **SYSTEM** (the host creates the section and
|
||||||
|
/// writes the live HID input report into it) and **LocalService** (the account the UMDF driver's
|
||||||
|
/// WUDFHost runs under, which reads it). The old SDDL granted **Everyone** (`WD`) — on the (mistaken)
|
||||||
|
/// assumption the driver needed a restricted token's broad access — letting any local user
|
||||||
|
/// `OpenFileMapping` the section to inject controller input or tamper the trusted channel
|
||||||
|
/// (security-review 2026-06-28 #5). Verified on the RTX box (2026-06-29): the WUDFHost token is
|
||||||
|
/// `S-1-5-19` (LocalService), SYSTEM integrity, with **zero restricted SIDs** — so scoping to SY+LS is
|
||||||
|
/// sufficient for the driver and excludes normal (medium-IL, non-service) user processes.
|
||||||
pub(super) struct Shm {
|
pub(super) struct Shm {
|
||||||
/// Owns the section handle (closed on drop). Held only for ownership — never read after construction.
|
/// Owns the section handle (closed on drop). Held only for ownership — never read after construction.
|
||||||
_handle: OwnedHandle,
|
_handle: OwnedHandle,
|
||||||
@@ -40,7 +48,7 @@ impl Shm {
|
|||||||
// exit — acceptable for a host-lifetime object).
|
// exit — acceptable for a host-lifetime object).
|
||||||
unsafe {
|
unsafe {
|
||||||
ConvertStringSecurityDescriptorToSecurityDescriptorW(
|
ConvertStringSecurityDescriptorToSecurityDescriptorW(
|
||||||
w!("D:(A;;GA;;;WD)"),
|
w!("D:(A;;GA;;;SY)(A;;GA;;;LS)"),
|
||||||
SDDL_REVISION_1,
|
SDDL_REVISION_1,
|
||||||
&mut psd,
|
&mut psd,
|
||||||
None,
|
None,
|
||||||
|
|||||||
Reference in New Issue
Block a user