Skip to content

fix(agent): enforce client identity for user-scope execution in dev mode - #1892

Draft
Vladyslav Nikonov (vnikonov-devolutions) wants to merge 1 commit into
masterfrom
vnikonov-devolutions-dgw-436-user-scope-execution-context
Draft

fix(agent): enforce client identity for user-scope execution in dev mode#1892
Vladyslav Nikonov (vnikonov-devolutions) wants to merge 1 commit into
masterfrom
vnikonov-devolutions-dgw-436-user-scope-execution-context

Conversation

@vnikonov-devolutions

Copy link
Copy Markdown
Contributor

Ensures package operations always run in the requesting user's own context.

In service (SYSTEM) mode the broker already launches operations with the client user's real session token, so user-scope installs land in the correct profile (%LOCALAPPDATA%, HKCU) and interactive installer UIs appear on the user's desktop rather than in session 0.

This change closes a gap in development (non-SYSTEM) mode: the executor ran commands under the broker process token regardless of which user connected to the pipe, so a user-scope install requested by a different user would silently land in the broker user's profile. Such requests are now rejected with a clear error.

The error reported when the target user has no active logon session is also clarified, since user-scope and interactive operations require the user to be logged on.

Issue: DGW-436

The broker Windows executor's development (non-SYSTEM) mode ran commands
under the broker process token regardless of which user connected to the
pipe, so a user-scope install requested by a different user would land in
the broker user's profile. Reject execution when the pipe client SID does
not match the broker process user SID.

Also clarify the SYSTEM-mode error when the target user has no active
logon session, since user-scope and interactive operations require one.

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

Copy link
Copy Markdown

Let maintainers know that an action is required on their side

  • Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

  • Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

  • Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

  • Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

@vnikonov-devolutions

Copy link
Copy Markdown
Contributor Author

Implementation notes (audit findings for DGW-436):

Verified as already correct (SYSTEM/service mode)execute_as_system in devolutions-agent/src/broker/executor/windows/mod.rs:

  • Obtains the client user's real session token via WTSQueryUserToken (matched by SID, not name), duplicates it as primary, and assigns the session ID before CreateProcessAsUserW.
  • Loads the environment block from that token, so %LOCALAPPDATA%, %USERPROFILE%, and HKCU resolve to the actual client user — per-user scope installs land in the right profile.
  • Processes are created on WinSta0\Default in the user's session, so --interactive installer UIs are visible on the user's desktop, never stuck in session 0.
  • No active logon session for the user → the operation fails with a clear error instead of installing into the SYSTEM profile (error message clarified in this PR).

Gap fixed (development/non-SYSTEM mode)execute_as_current_user ignored ctx.user_sid and ran everything under the broker process token. Since the pipe DACL grants BUILTIN\Users connect access, a client running as a different user would get its user-scope install written into the broker user's profile. The executor now compares the authenticated pipe client SID against the broker process token SID and rejects mismatches.

Why not a capability flag: ManagerCapability in now-policy-api 0.2 has no interactive/user-scope execution field to gate on, and interactive mode is already gated per-manager (builders reject it where unsupported) and per-policy (AllowInteractive). The remaining gap was an executor identity check, addressed here.

Testing: new unit test dev_mode_rejects_client_user_different_from_broker_user (uses the Everyone SID, which never matches the test process user). Existing SYSTEM-mode behavior is covered by the ignored psexec -s test. cargo clippy --tests -D warnings clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants