fix(digstore-remote): owner-only Windows ACL + zeroize transient identity seed buffer - #41
Merged
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
…DACL on Windows RED proof for the Windows ACL-parity rider. The test is #[cfg(windows)] so it runs only on the windows-latest CI job; it asserts the persisted identity seed file carries an explicit PROTECTED (inheritance-blocking) owner-only DACL and that neither Everyone nor BUILTIN\Users has an ACE. Without the follow-up fix, write_secret_file does a bare std::fs::write whose inherited DACL is NOT protected, so this fails deterministically. Co-Authored-By: Claude <noreply@anthropic.com>
…ntity seed buffer Two at-rest hardening riders on the persistent user identity seed (identity_key.bin). 1. Windows ACL parity. write_secret_file previously did a bare std::fs::write on non-Unix, so the seed only INHERITED the profile-dir ACL. A redirected/roaming %APPDATA% or a loosened profile could silently widen read access. It now stamps an EXPLICIT, PROTECTED (inheritance-blocking) DACL granting only the current user (SetNamedSecurityInfoW + SetEntriesInAclW), the effective parity with the Unix 0o600 mode. Unix branch unchanged. 2. Zeroize transient seed material. The plaintext buffer read from disk in load_or_create_seed is wrapped in zeroize::Zeroizing so it is scrubbed on drop. The returned [u8;32] Copy that signer closures keep alive for the process lifetime is by-design and out of scope (the deferred boundary epic). Peer-identity / machine-key / peer_id derivation untouched. Public API unchanged. Bumps workspace version 0.23.1 -> 0.23.2 (patch; compatible hardening). Closes riders 1+2 of dig_ecosystem #2168. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
changed the title
…ippy mixed-attributes) The mod carried both an outer /// summary and inner //! docs, tripping clippy::mixed_attributes_style (-D warnings) on the windows-latest build. Fold the summary into the inner //! block; no code change. Co-Authored-By: Claude <noreply@anthropic.com>
…lippy) psd is PSECURITY_DESCRIPTOR (*mut c_void); casting it to *mut c_void tripped clippy::unnecessary_cast (-D warnings) on the windows-latest build. Pass psd directly to LocalFree. 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
2 participants
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.
What + why
The persistent USER-identity seed (
identity_key.bin,crates/digstore-remote/src/identity.rs) names the operator in everydig://<user>@hostorigin — exfiltrating it impersonates the user. Two at-rest weaknesses (dig_ecosystem #2168, the separable riders the issue body flags as landable first) are closed here:write_secret_file's#[cfg(not(unix))]branch was a barestd::fs::writewith no explicit ACL — it only inherited the profile-dir ACL ("inheritance, not design"), so a redirected/roaming%APPDATA%or a loosened profile silently widened read access. Now, on Windows, the seed file gets an explicit owner-only, inheritance-blocking (PROTECTED) DACL viaSetNamedSecurityInfoW(DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION)— a single ACE grantingFILE_ALL_ACCESSto the current-user SID and no one else — the effective parity with the Unix0o600path (which is unchanged). RAII guards free the process token and the allocated ACL.load_or_create_seedis nowzeroize::Zeroizing, scrubbed on drop, so transient plaintext seed copies don't linger.Scope (STRICT)
Riders only. The #2168 boundary fix (give dig-node a distinct machine keypair separate from the user identity, + a migration for provisioned nodes) is a decider-deferred cross-repo epic and is NOT touched here — peer-identity / machine-key /
peer_idderivation is untouched, public API signatures unchanged (load_or_create_seedstill returns[u8;32]). The by-design seedCopycaptured into the long-lived signer closures (to re-derive the BLS key per call) is left as-is — that lingering copy is the epic's concern, not this rider. #2168 stays OPEN after this merges.How verified
8101064= failing#[cfg(windows)]DACL test only;3d67fe5= fix. The test asserts two independent properties so a bare inherited-ACL write fails deterministically regardless of the CI tempdir's inherited ACEs: (1) the DACL isSE_DACL_PROTECTED(an inherited/default DACL is not — the load-bearing distinguisher), and (2) neitherEveryone(S-1-1-0) norBUILTIN\Usershas an ACE. digs CI runscargo nextest run --workspaceon windows-latest, so this gates.cargo test -p digstore-remotegreen (the stability test exercises theZeroizingdisk-read path);cargo fmt --checkclean;cargo clippy --all-targetszero warnings.Blast radius
Confined to
crates/digstore-remote/src/identity.rs+ itsCargo.toml(addedzeroize = "1"matching the graph's 1.9.0;windows-sys = "0.59"under[target.'cfg(windows)'.dependencies], matching the 0.59.0 already in the lockfile — no second version line). No caller signature changed. FFI symbols/structs verified against the local windows-sys 0.59.0 source.Version
Root workspace
Cargo.toml0.23.1→0.23.2(the fileensure-version-increment.ymldiffs). Patch — compatible defense-in-depth hardening, no API/behavior change on the read/sign path.Closes part of #2168 (riders only; issue remains open for the boundary epic).
Generated by Claude Code