[Benoît Cortier (CBenoit)]

The Agent installer gave LOCAL SERVICE write access to %ProgramData%\Devolutions\Agent, inherited by every file and folder below it, so any service running as that account could add entries or change files there. No Agent component runs as LOCAL SERVICE or needs that access.

The installer now:

• gives LOCAL SERVICE no entry on the folder, so only SYSTEM and administrators have any right to change what the agent stores there;
• on install, upgrade and repair of an existing folder, after resetting its owner and DACL on the handle it checked, removes every LOCAL SERVICE allow entry, explicit or inherited, from the DACL of each file and folder below it, including the pedm subfolders earlier installers created before locking pedm;
• leaves every other part of those DACLs as it was, and doesn't write a DACL that has no such entry, so the agent's identity folders and files keep exactly the protected DACL the agent provisions;
• never follows a link during that walk: a link loses LOCAL SERVICE from its own DACL, so that account can't retarget it, and what it points to is unchanged;
• removes those entries from a file with several hard links too, since removing them never widens any account's access, wherever the file's other names are;
• skips entries it can't open by their listed name, and fails the run if a folder is swapped while it walks;
• refuses install, upgrade and repair, with an error in the installer's language naming the entry and its owner, when a file or folder below the folder is owned by an account other than SYSTEM, Administrators, TrustedInstaller or a member of Administrators, because an owner can always change its entry's permissions; the check runs before the upgrade removes the installed product, and again during the walk for entries added since;
• never changes an owner below the folder, so a deployment never silently takes over an entry another account created;
• names the entry when the walk can't open one, and waits up to 2 seconds for a folder another process holds without sharing, as the running agent briefly does with its identity folder.

Issue: DGW-478
Security: yes

🤖 Generated with Claude Code

[Benoît Cortier (CBenoit)]

Evidence

No Agent component needs LOCAL SERVICE write access to %ProgramData%\Devolutions\Agent:

• The Agent service runs as LocalSystem (package/AgentWindowsManaged/Program.cs, no account set).
• PEDM, the tunnel, PowerShell Universal, the package broker and the updater run in the service process or as SYSTEM children.
• devolutions-session runs as the logged-on user and writes under %APPDATA%.
• private-fs grants SYSTEM, Administrators and OWNER RIGHTS only.
• The updater rejects S-1-5-19 as a service account (service_account.rs), and the package broker treats LOCAL SERVICE as untrusted (policy_security.rs).
• The LOCAL SERVICE entry came in with the Agent bootstrap (feat(agent): Devolutions agent bootstrap #875, feat(agent): Devolutions Gateway service updater #889), copied from the Gateway SDDL, with no stated consumer.

Install sequence: I built the MSI locally with MSBuild and read InstallExecuteSequence:

• checkProgramDataDirectory runs at 1503, before RemoveExistingProducts at 1504.
• Next come createProgramDataDirectory at 3702, the PEDM create and permission actions at 3704 and 3706, and setProgramDataDirectoryPermissions at 3708.
• All of these run before InstallFiles at 4000, enrollAgentIdentity at 5887 and StartServices at 5900.
• All are deferred and not impersonated (type 3073), and none runs when REMOVE="ALL".

Tests: package/AgentWindowsManaged.Tests, unelevated, at f8eae07: 184 passed, 0 failed (baseline 161).

Lines changed: 1141 added, 64 removed.

Area	Added / removed
Installer actions	536 / 54
Localized strings and generated files	19 / 0
Tests and helpers	583 / 7
Broker doc comment	3 / 3

Intent audit of package/AgentWindowsManaged against INTENT.md §Data folder and crates/agent-identity/INTENT.md §Local state:

• Everything conforms, including the two §Data folder statements this PR adds: the removal of LOCAL SERVICE below Agent, hard-linked files included, and the refusal of untrusted owners below it.
• The walk writes no DACL that has no LOCAL SERVICE allow entry, so the agent's identity folders and files keep exactly the DACL the agent provisions. A binary descriptor comparison test checks this.

Deviations

• The walk differs from the Gateway installer's (fix(installer): refuse an untrusted data folder #2172). The Gateway walk re-inherits from the data folder and unprotects files. This one only removes LOCAL SERVICE allow entries and leaves the rest of each DACL as it is, because the agent refuses identity folders and files whose protected DACL differs from the one it provisions.
• Doc comment change outside the installer. In crates/now-package-broker/src/policy_store/windows.rs, a comment no longer says the installer grants LOCAL SERVICE write access.

Open questions for the maintainer

None. The owner refusal and the hard-link behaviour are now stated in package/AgentWindowsManaged/INTENT.md §Data folder, as approved by the maintainer.

Decisions and declined review findings

• Behaviour:
  • The walk removes only LOCAL SERVICE allow entries.
  • Any failure in the walk fails the install, including a sharing violation or access denied on an entry.
  • Entries the walk can't reach by their listed name are skipped and logged: deleted ones, and names ending in a dot or space.
  • Other unreachable names (device names, invalid characters, over-long paths) fail the run.
• Owners below Agent (maintainer's decision): an entry owned by an account other than SYSTEM, Administrators, TrustedInstaller or a member of Administrators is refused, never re-owned.
  • checkProgramDataDirectory (1503, before RemoveExistingProducts) walks the tree read-only.
  • The walk in createProgramDataDirectory checks again, because the old DACL lets LOCAL SERVICE add entries until Agent is reset. A refusal there leaves the entries already walked without LOCAL SERVICE, which only tightens them.
  • No writer in the repository leaves an untrusted owner: the agent and the installer create entries as SYSTEM with explicit owners, and the Gateway rewrites update.json in place.
• The running agent's lock: the agent holds identity without sharing for a moment around each state write. Hold retries a sharing violation every 25 ms for up to 2 s; a longer hold still fails the run, which can be repeated.
• Walk failures name the entry: any failure to open an entry other than not-found fails the run with an exception naming the path.
• Known limits, not handled:
  • An entry unreachable by its listed name (a name ending in a dot or space) is skipped, so its owner isn't checked.
  • A file pending deletion (deleted while another process still has it open) fails the run as access denied for that moment; a repeat run succeeds.
  • A sharing violation on Devolutions or Agent themselves, past the retry, is reported as unreadable security (unchanged from before).
• A hard link's other name under an outside folder: if that folder passes LOCAL SERVICE down by inheritance, a later propagation from it can put the entry back on the shared descriptor until the next install. Noted, not handled.
• Declined test changes: oracle re-citations, and logging a file's hard-link count.
• Test pruners: no mutation testing was run, so test-pruner was skipped. dependency-pruner was skipped because no manifest changed.

UNTESTED properties

Introduced:

• DataFolders.cs: the handle on Agent holds it until the walk below returns.
• SecureDirectory.cs, the walk:
  • each directory handle stays open until the entries below it are done;
  • a directory swapped between its two opens fails the walk.
• SecureDirectory.cs, OpenUnlessNotFound:
  • a directory that vanishes between its first open and its hold is skipped;
  • any other failure fails the walk; tests cover a sharing violation, not access denied.
• DataFolders.cs:
  • the actions pass no trust decision, so IsTrusted decides in Check (before RemoveExistingProducts) and in Create;
  • Check's handle on Agent stays open until the walk below returns.
• SecureDirectory.cs: a file symbolic link loses LOCAL SERVICE from its own DACL, not from its target's.

Still accurate, unchanged:

• CustomActions.cs: the refusal message, the cleanup on failure, and that a missing pedm folder doesn't fail the run.
• DataFolders.cs:
  • the refusal of a link or of an untrusted owner;
  • the handle on Devolutions;
  • setting the owner to SYSTEM.
• SecureDirectory.cs: trust through Administrators membership, and the handles held while the pedm folders are set. Membership now also decides owners below Agent: when Authz can't resolve groups, only direct members of the local Administrators group are trusted, so an entry owned by a domain administrator trusted only through a nested group may be refused on a machine that can't reach its domain.

Notes

• A junction inside Agent loses LOCAL SERVICE from its own DACL, and its target is left unchanged.
• The updater's update.json grants the Gateway service SID, not LOCAL SERVICE, so this change doesn't affect it.

Note

LLM-assisted content (no human feedback).