Skip to content

--harden-files strips the service account it is meant to grant, because it resolves the account from the caller #2371

Closed

Description

@erikdarlingdata

--harden-files (#2352) resolves the service account from the CALLING process, so running it the documented way — elevated, from something that is not the service — writes the ACL for the operator and STRIPS the account the service actually runs as. The install then fails on its next restart, and the command reports success while doing it.

Found dogfooding it on the third Darling box during the 3.5.0 soak, on the exact case it was built for.

What happened

The box was logging the #2352 error at start, so I ran the shipped remedy via SSM (which runs as NT AUTHORITY\SYSTEM — elevated, as the help text asks for).

Before — the service account holds (F) on both, which is correct:

C:\PerformanceMonitorDarling\darling.json  NT AUTHORITY\INTERACTIVE:(R)
                                           NT AUTHORITY\SYSTEM:(F)
                                           BUILTIN\Administrators:(F)
                                           NT SERVICE\PerformanceMonitor Darling:(F)
C:\ProgramData\PerformanceMonitorDarling   NT AUTHORITY\INTERACTIVE:(S,X)
                                           NT AUTHORITY\SYSTEM:(OI)(CI)(F)
                                           BUILTIN\Administrators:(OI)(CI)(F)
                                           NT SERVICE\PerformanceMonitor Darling:(OI)(CI)(F)

The run:

Hardening for service account: NT AUTHORITY\SYSTEM

  SECURED  C:\PerformanceMonitorDarling\darling.json (the live config)
  SECURED  C:\ProgramData\PerformanceMonitorDarling (the store directory)
  SECURED  C:\ProgramData\PerformanceMonitorDarling\pg-credential.dpapi (the store credential)
  SECURED  C:\ProgramData\PerformanceMonitorDarling\pg-admin-credential.dpapi (the admin credential)

All 4 item(s) secured. The service re-asserts these ACLs at every start.
exit_code | 0

After — the service account is gone from all four:

=== C:\PerformanceMonitorDarling\darling.json
   NT AUTHORITY\INTERACTIVE : Read, Synchronize
   NT AUTHORITY\SYSTEM : FullControl
   BUILTIN\Administrators : FullControl
=== C:\ProgramData\PerformanceMonitorDarling
   NT AUTHORITY\INTERACTIVE : ExecuteFile, Synchronize
   NT AUTHORITY\SYSTEM : FullControl
   BUILTIN\Administrators : FullControl
=== C:\ProgramData\PerformanceMonitorDarling\pg-credential.dpapi
   NT AUTHORITY\SYSTEM : FullControl
   BUILTIN\Administrators : FullControl
=== C:\ProgramData\PerformanceMonitorDarling\pg-admin-credential.dpapi
   NT AUTHORITY\SYSTEM : FullControl
   BUILTIN\Administrators : FullControl

while the SCM still says:

svc_account | NT SERVICE\PerformanceMonitor Darling

The service was Running throughout only because it already held open handles. On the next start it would have had no read on darling.json or on either DPAPI credential — the failure #2185/#2197 exist to diagnose, newly manufactured by the tool meant to prevent it.

Root cause

DarlingFileSecurity.ServiceAccountDisplayName (DarlingFileSecurity.cs:76):

public static string ServiceAccountDisplayName
{
    get
    {
        try
        {
            return WindowsIdentity.GetCurrent().Name;
        }
        ...

WindowsIdentity.GetCurrent() is the identity of whoever is running the process. For the property's ORIGINAL callers that is right and the value is exact — they all run inside the service, building a remediation string about themselves (DarlingWorker.cs:721, DarlingManagedPostgres.cs:1607, DarlingInstallLocation.cs:115).

#2352 gave the same property a caller with the opposite property: the CLI verb exists specifically to be run by someone who is not the service, because a virtual service account cannot re-ACL a file it does not own — that is the whole reason the verb was needed. So on its documented path the value is always wrong, and the harden grants the operator and drops the service.

The property's own doc comment already states the requirement it is missing:

The icacls /grant a harden failure prints must name the account the service RUNS AS — granting the virtual account on a re-homed install is a fix that cannot work, handed to the one operator who needs it.

That reasoning is correct and is exactly the bug, one level up: the value must come from the SERVICE, not from the caller. Win32_Service.StartName gives it directly and covers the re-homed domain-account and gMSA cases the comment is concerned about:

(Get-CimInstance Win32_Service -Filter "Name='PerformanceMonitor Darling'").StartName
  -> NT SERVICE\PerformanceMonitor Darling

Why verification did not catch it

The verify pass is the feature's best idea — it re-reads each target instead of claiming success. But it checks the ACL against the same wrongly-resolved principal it just wrote, so it confirms its own mistake and exits 0. It needs to assert the SERVICE's account specifically, which also makes it a genuine regression test.

Suggested fix

  1. Resolve from the SCM (StartName) rather than WindowsIdentity.GetCurrent(), at least for the CLI verb. Keep the current value as the fallback when the service is not registered.
  2. Have the verify step assert the resolved service account holds its rights, so a wrong principal fails loudly instead of passing.
  3. Refuse to remove the last ACE granting the SCM's StartName — a harden that would lock the service out of its own credentials should abort, not report SECURED.
  4. Worth a test that the resolved principal differs from the caller when invoked outside the service, since that is the only configuration the verb ever runs in.

Impact and state

Blocks 3.5.0 as far as I am concerned — #2352 ships a documented remedy that breaks a healthy install, and the failure is delayed until the next restart, so an operator would not connect the two.

The box is restored and healthy: I re-granted the account on all four paths and restarted the service to prove it, rather than assuming.

2026-08-19 17:32:07.374 [INFO ] [DarlingWorker] Postgres store ready (schema v79, 0 migration(s) applied)

No ACL errors on that start. Not running the verb on the other two boxes until this is fixed.

Activity

  1. added a commit that references this issue on Aug 19, 2026
    c610c99
  2. erikdarlingdata commented on Aug 19, 2026

    @erikdarlingdata
    OwnerAuthor

    Fixed in #2372 (merged to dev). Auto-close did not fire because the PR targets dev rather than the default branch.

    The account now comes from the SCM's ObjectName instead of WindowsIdentity.GetCurrent(), with a fallback to the caller only when the service is not registered, and the verify pass gained the half it was missing: a target that is private but excludes the service reports LOCKED OUT and exits non-zero instead of SECURED.

    Re-cutting the nightly so the soak candidate carries this, and I will run the verb against the fixed binary on the box where it was found before calling it good — the bug survived review precisely because nobody had run it on a real install.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions