fix(service): PowerShell-free Win32 ACL readback so the SYSTEM service mints the control token - #53
Merged
Conversation
…e mints the control token The #501 control-token state-dir hardening readback-verified the DACL by spawning `powershell Get-Acl`. On a host where PowerShell cannot autoload `Microsoft.PowerShell.Security` that cmdlet throws, the spawn exits non-zero, and `windows_harden_dir` read that as a hardening FAILURE and removed the state dir (fail-closed). The LocalSystem service then had no dir to mint `control-token` into, so every `dign`/`control.*` call failed UNAUTHORIZED. This is the #849-induced failure, not a universal mint bug (a pristine box with working PS minted fine). Two coherent parts, one family: 1. PowerShell-free readback. Owner SID + DACL ACEs are now read directly through the Win32 security API (`GetNamedSecurityInfoW` for owner + DACL, `GetAce`, `ConvertSidToStringSidW`) in `security.rs`, rendered into the SAME SID-based `OWNER;/ACE;` line format the pure §565 policy parser already consumes — no shell, no module autoload, no localized-name parsing. `path_owner_sid` + `read_and_verify_acl` + `discover_existing_read_grant` all delegate here; `acl_verify_ps_command` is removed. 2. Readback never destroys a correctly-hardened dir. `readback_decision` (pure) separates a readable-but-VIOLATING DACL (fail closed — remove + regenerate) from a DACL that cannot be read AT ALL (trust the applied lockdown SET, preserve the dir). The service startup path thus converges to {hardened dir + minted token}: an unreadable readback no longer removes the dir, so the token is always minted (mint-on-startup idempotent). The §501/§565 DACL policy is byte-unchanged — only the readback MECHANISM (PowerShell -> Win32) and the transient-unreadable handling changed. Absorbs #849; moots #772's remedy classification. Regression tests (kept): readback_decision_does_not_fail_a_correct_dir_when_the_readback_is_unavailable, readback_decision_fails_closed_on_a_genuinely_wrong_dacl, readback_decision_accepts_a_correctly_hardened_dacl, windows_acl_readback_reads_a_real_dir_without_powershell, mint_on_a_pre_existing_dir_is_idempotent. All §565 custody assertions retained. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause (establishment control-plane fix A1, epic #856; absorbs #849, moots #772's remedy classification)
On a Windows box,
dign/control.*commands fail-32030 UNAUTHORIZEDbecause the LocalSystem service never reliably mints the control token. Chain:dig-node's feat(mirror): activate bond promotion on the coin's own peer declaration #501 state-dir hardening readback-verified the DACL by spawningpowershell Get-Acl(state.rs::acl_verify_ps_command+path_owner_sid).Microsoft.PowerShell.Security,Get-Aclthrows → the readback spawn exits non-zero.windows_harden_dirtreated that as a hardening FAILURE →remove_dir_all(fail-closed) → the state dir is deleted.control-tokeninto → every control call is unauthorized.On a pristine box (working PS) it minted fine — so this is the #849-induced failure, NOT a universal mint bug.
Fix (two parts, one family)
1. PowerShell-free ACL readback (Win32, not
Get-Acl)Owner SID + DACL ACEs are read directly through the Win32 security API —
GetNamedSecurityInfoW(owner + DACL),GetAce,ConvertSidToStringSidW— insecurity.rs(read_owner_sid_stringmadepub(crate); newread_acl_verify_lines). It renders the SAME SID-basedOWNER;/ACE;line format the pure §565 policy parser (parse_acl_verify/parse_first_user_read_sid) already consumes, so the policy is byte-unchanged — only the readback mechanism moved. No shell, no module autoload, no localized-name parsing.Why Win32 over icacls:
security.rsalready ships this exact Win32 machinery for the #565 owner probe (which itself moved off apowershellspawn — a planted-powershell.exeLPE), andwindows-sysis already a dependency with the needed features. Reusing it is DRY, avoids parsing localized icacls name output, and needs no new dependency.path_owner_sid,read_and_verify_acl, anddiscover_existing_read_grantall delegate to it;acl_verify_ps_commandis removed.2. Idempotent mint-on-startup — the readback never destroys a correctly-hardened dir
New pure
readback_decisionseparates:Err→ fail closed (remove + regenerate), exactly as before; andOk→ trust the applied lockdown SET (setowner+reset+grant already succeeded — the authoritative security action) and PRESERVE the dir.So the service startup path converges to {hardened dir + minted token}: an unreadable readback no longer removes the dir, and
load_or_create_token_atalways mints on a pre-existing/half-hardened/recreated dir (idempotent).§565 policy: byte-unchanged
Only the readback MECHANISM (PowerShell → Win32) and the transient-unreadable handling changed. The protected DACL {SYSTEM:F, Administrators:F, [install-user:R]}, SYSTEM owner, non-recursive per-level, and fail-closed-on-violation semantics are all identical. Every existing §565/§501 custody test is retained and green.
Tests (TDD, kept)
state::readback_decision_does_not_fail_a_correct_dir_when_the_readback_is_unavailable— the regression: a readback-tool-unavailable condition does NOT cause a correct dir to be treated as failed/removed.state::readback_decision_fails_closed_on_a_genuinely_wrong_dacl— a wrong DACL (world-readable / squatter owner) still fails closed.state::readback_decision_accepts_a_correctly_hardened_dacl.security::windows_acl_readback_reads_a_real_dir_without_powershell— the Win32 mechanism reads a real dir's owner+ACEs (anti-false-fail proof at the mechanism level).control::mint_on_a_pre_existing_dir_is_idempotent— mint-on-startup converges + is idempotent.Blast radius
state.rsreadback path (path_owner_sid,read_and_verify_acl,discover_existing_read_grant, newreadback_decision; removedacl_verify_ps_command) +security.rs(new Win32 ACL reader) + service startup mint (control.rs, unchanged logic, new test). No other repo consumes these (all references were contained instate.rs). SPEC.md steps 6-7 updated to match.Verification (Windows dev host)
cargo fmt --all -- --check— cleancargo clippy -p dig-node-service --all-targets --all-features -- -D warnings— cleancargo test -p dig-node-service— 232 lib + 48 integration + 0 doc tests pass, 0 failedVersion
0.38.5→0.38.6(patch —fix:, no public-API/behaviour change beyond the bugfix).SECURITY-SENSITIVE (§565 DACL path) — will go through loop-security before merge; the diff is kept minimal + auditable.
🤖 Generated with Claude Code