diff --git a/internal/sandbox/runtime_state.go b/internal/sandbox/runtime_state.go index a738396fc..8f71f0db1 100644 --- a/internal/sandbox/runtime_state.go +++ b/internal/sandbox/runtime_state.go @@ -36,6 +36,24 @@ type SandboxRuntime struct { Temp string `json:"temp,omitempty"` } +// sandboxRuntimeRootFor derives the per-workspace runtime root. It is separated +// from prepareSandboxRuntime because the elevated Windows setup path needs the +// same answer WITHOUT taking a lease or creating anything: a sandbox principal +// is a separate account with no inherited rights under the user cache, so setup +// has to grant it write access to this tree before any command runs. +// +// Both callers must agree exactly. If they ever drift, setup grants the ACE on +// one directory while commands write to another, and the failure is a bare +// ACCESS_DENIED from npm or go build with nothing pointing at the sandbox. +func sandboxRuntimeRootFor(workspaceRoot string, cacheRoot string) (string, error) { + digest := sha256.Sum256([]byte(workspaceRoot)) + root := filepath.Join(cacheRoot, "zero", "runtime", "v1", hex.EncodeToString(digest[:8])) + if !pathWithinRoot(workspaceRoot, root) { + return root, nil + } + return fallbackSandboxRuntimeRoot(workspaceRoot) +} + func prepareSandboxRuntime(workspaceRoot string) (SandboxRuntime, func(), error) { workspaceRoot = filepath.Clean(strings.TrimSpace(workspaceRoot)) if workspaceRoot == "" || workspaceRoot == "." { @@ -49,13 +67,9 @@ func prepareSandboxRuntime(workspaceRoot string) (SandboxRuntime, func(), error) if cacheRoot == "" || cacheRoot == "." { return SandboxRuntime{}, nil, errors.New("user cache directory is unavailable") } - digest := sha256.Sum256([]byte(workspaceRoot)) - root := filepath.Join(cacheRoot, "zero", "runtime", "v1", hex.EncodeToString(digest[:8])) - if pathWithinRoot(workspaceRoot, root) { - root, err = fallbackSandboxRuntimeRoot(workspaceRoot) - if err != nil { - return SandboxRuntime{}, nil, err - } + root, err := sandboxRuntimeRootFor(workspaceRoot, cacheRoot) + if err != nil { + return SandboxRuntime{}, nil, err } lease, err := prepareSandboxRuntimeLease(root) if err != nil { diff --git a/internal/sandbox/windows_acl_apply_windows.go b/internal/sandbox/windows_acl_apply_windows.go index c666aa9ef..d53927a08 100644 --- a/internal/sandbox/windows_acl_apply_windows.go +++ b/internal/sandbox/windows_acl_apply_windows.go @@ -223,7 +223,33 @@ func windowsExplicitAccessEntries(entries []WindowsACLEntry, isDir bool) ([]wind func windowsACLAccess(action WindowsACLAction) (windows.ACCESS_MODE, windows.ACCESS_MASK, error) { switch action { case WindowsACLAllowWrite: - return windows.GRANT_ACCESS, windows.FILE_GENERIC_READ | windows.FILE_GENERIC_WRITE | windows.FILE_GENERIC_EXECUTE, nil + // DELETE and FILE_DELETE_CHILD are part of the grant, not extras. + // FILE_GENERIC_WRITE covers creating and modifying but not removing or + // renaming, and a rename needs delete access on the source. Under the + // old same-user token that gap was invisible, because the caller already + // held inherited rights on its own tree; a sandbox principal is a + // separate account with no such inheritance, so without these it can + // write a file it can never delete. Ordinary editing and most git + // operations rewrite files by replacing them, so the omission fails + // normal work rather than an edge case. + // + // WindowsACLDenyWrite below already treats delete as part of write. This + // keeps the grant symmetric with the deny instead of covering less. + // WRITE_DAC and WRITE_OWNER stay out on purpose: they are in the deny + // mask to stop the principal rewriting its own restrictions, and + // granting them here would hand back exactly that. + return windows.GRANT_ACCESS, windows.FILE_GENERIC_READ | windows.FILE_GENERIC_WRITE | windows.FILE_GENERIC_EXECUTE | windows.DELETE | windowsFileDeleteChild, nil + case WindowsACLAllowRead: + // Read and traverse without write. A sandbox principal is a separate + // account with no inherent access to the caller's tree, so a read-only + // root has to be granted rather than assumed. Deliberately omits + // FILE_GENERIC_WRITE, DELETE and WRITE_DAC. + return windows.GRANT_ACCESS, windows.FILE_GENERIC_READ | windows.FILE_GENERIC_EXECUTE, nil + case windowsACLRevoke: + // REVOKE_ACCESS drops every ACE naming the trustee regardless of the mask, + // so the mask is ignored here. Used to retire a principal without having + // to remember which access each path was granted. + return windows.REVOKE_ACCESS, 0, nil case WindowsACLDenyRead: return windows.DENY_ACCESS, windows.FILE_GENERIC_READ | windows.FILE_GENERIC_EXECUTE, nil case WindowsACLDenyWrite: diff --git a/internal/sandbox/windows_command_runner_windows.go b/internal/sandbox/windows_command_runner_windows.go index 0b9e8f64e..22d290d3e 100644 --- a/internal/sandbox/windows_command_runner_windows.go +++ b/internal/sandbox/windows_command_runner_windows.go @@ -75,6 +75,31 @@ func runWindowsSandboxCommand(config WindowsSandboxCommandConfig, stderr io.Writ // reads under that flag (#612). Profiles with DenyRead keep the fully // restricted token, trading spawn capability for read-deny enforcement. writeRestricted := len(config.PermissionProfile.FileSystem.DenyRead) == 0 + + // A provisioned sandbox principal replaces the restricted token entirely: it + // is a separate account, so reads outside its granted roots are denied by the + // filesystem rather than left open the way a same-user restricted token has + // to leave them (#662). Absent, unprovisioned or opted-out, ok is false and + // the restricted-token backend below runs exactly as before. + principalToken, ok, err := windowsSandboxPrincipalToken(config) + if err != nil { + // The one path here that does not fall back, because a provisioned but + // unusable principal means the sandbox is broken rather than absent. Say + // how to get out of it, since the whole backend is opt-in. + fmt.Fprintf(stderr, "%s: sandbox principal is provisioned but unusable: %v. Re-run `zero sandbox setup` from an elevated terminal, or unset %s to fall back to the restricted-token sandbox.\n", + WindowsSandboxCommandRunnerName, err, windowsSandboxIdentityEnv) + return 1 + } + if ok { + defer principalToken.Close() + exitCode, err := runWindowsCommandAsUser(principalToken, config) + if err != nil { + fmt.Fprintln(stderr, WindowsSandboxCommandRunnerName+": "+err.Error()) + return 1 + } + return exitCode + } + token, err := createWindowsRestrictedTokenForCapabilitySIDs(tokenSIDs, writeRestricted) if err != nil { fmt.Fprintln(stderr, WindowsSandboxCommandRunnerName+": "+err.Error()) diff --git a/internal/sandbox/windows_identity_acl.go b/internal/sandbox/windows_identity_acl.go new file mode 100644 index 000000000..bdbeba608 --- /dev/null +++ b/internal/sandbox/windows_identity_acl.go @@ -0,0 +1,174 @@ +package sandbox + +// ACLs for a sandbox principal. +// +// The capability-SID model this sits beside starts from "the caller can already +// read everything" and narrows writes, because the sandboxed child runs as the +// caller. A principal inverts that: a separate local account has no access to +// the caller's profile at all, so the interesting direction is what to GRANT. +// +// That inversion is the point. Credential stores under the user's profile are +// unreachable because the principal is a different account, not because a deny +// rule enumerated them, which is what makes this able to close #662 and #675 on +// Windows where a deny-read ACE against the caller's own SID never could. Deny +// rules stay useful only for objects that are readable by everyone. +// +// Grants are explicit and narrow: the workspace and any extra write roots get +// read+write, declared read-only roots get read, and the protected metadata +// carve-outs the profile already defines stay denied so .git internals and +// .zero/.agents cannot be rewritten from inside the sandbox. + +import ( + "errors" + "fmt" + "path/filepath" +) + +// WindowsACLAllowRead grants read and execute without write. It exists for the +// principal model, where a read root must be granted rather than assumed. +const WindowsACLAllowRead WindowsACLAction = "allow-read" + +// windowsPrincipalACLInput is everything needed to describe a principal's +// access. It is deliberately a plain struct rather than the full command config +// so the plan can be built and tested without a live sandbox. +type windowsPrincipalACLInput struct { + // PrincipalSID is the string SID of the sandbox account every ACE names. + PrincipalSID string + // WriteRoots receive read+write+execute. The workspace lives here. + WriteRoots []WritableRoot + // ReadRoots receive read+execute only. + ReadRoots []string + // DenyRead covers objects a principal could otherwise reach because they are + // world-readable; per-user secrets need no entry. + DenyRead []string + // DenyWrite carries the policy's own deny-write paths. The capability plan + // has always emitted these; the principal plan denied write only on + // protected metadata and read-only subpaths inside write roots, so a policy + // deny sitting anywhere else was simply not enforced once the runner used a + // principal token, and a shell child could write where the restricted-token + // backend would have blocked it. + DenyWrite []string +} + +// buildWindowsPrincipalACLPlan turns a principal's access into ACL entries. +// +// Ordering matters at apply time: deny entries are emitted before allows so a +// carve-out inside a granted root survives, which mirrors how Windows evaluates +// an explicit DACL (deny ACEs first). +func buildWindowsPrincipalACLPlan(input windowsPrincipalACLInput) (WindowsACLPlan, error) { + if input.PrincipalSID == "" { + return WindowsACLPlan{}, errors.New("windows principal ACL plan requires a principal SID") + } + if len(input.WriteRoots) == 0 && len(input.ReadRoots) == 0 { + return WindowsACLPlan{}, errors.New("windows principal ACL plan requires at least one root") + } + + entries := make([]WindowsACLEntry, 0, len(input.WriteRoots)*2+len(input.ReadRoots)+len(input.DenyRead)) + + // Deny first. A deny ACE inside a write root (protected metadata, git + // internals) has to win over the grant that follows it. + // Materialized, matching the capability plan. applyWindowsACLPlan skips a + // target that does not exist, so without this a deny-read path created after + // setup ran never got an ACE at all and the principal could read it. The + // deny has to be in place before the object is. + for _, path := range normalizeProfilePaths(input.DenyRead) { + entries = append(entries, WindowsACLEntry{ + Action: WindowsACLDenyRead, + Path: path, + Capability: input.PrincipalSID, + Materialize: true, + }) + } + for _, path := range normalizeProfilePaths(input.DenyWrite) { + entries = append(entries, WindowsACLEntry{ + Action: WindowsACLDenyWrite, + Path: path, + Capability: input.PrincipalSID, + Materialize: true, + }) + } + for _, root := range input.WriteRoots { + // Normalized the same way as read and deny paths: a write root may arrive + // with "~" or as a relative path, and an ACE has to name the same absolute, + // symlink-resolved object the deny entries do or the two disagree. + cleaned := normalizeProfilePath(root.Root) + if cleaned == "" { + return WindowsACLPlan{}, fmt.Errorf("windows principal ACL plan: unusable write root %q", root.Root) + } + // Materialized, like the metadata and policy denies below and above. + // + // These are the git control-plane carveouts (.git/config, .git/hooks). On a + // workspace where git has not run yet they do not exist at setup time, and + // applyWindowsACLPlan skips a target that is absent, so the ACEs were never + // written. Once git created those paths the principal still held inherited + // write access to the workspace and could install a hook or rewrite + // credential.helper. The capability plan gets away without this because its + // child runs as the caller; a separate principal account does not. + for _, subpath := range normalizeProfilePaths(root.ReadOnlySubpaths) { + entries = append(entries, WindowsACLEntry{ + Action: WindowsACLDenyWrite, + Path: subpath, + Capability: input.PrincipalSID, + Materialize: true, + }) + } + for _, name := range root.ProtectedMetadataNames { + entries = append(entries, WindowsACLEntry{ + Action: WindowsACLDenyWrite, + Path: filepath.Join(cleaned, name), + Capability: input.PrincipalSID, + Materialize: true, + }) + } + } + + // Then the grants the principal cannot work without. + for _, root := range input.WriteRoots { + cleaned := normalizeProfilePath(root.Root) + if cleaned == "" { + continue + } + entries = append(entries, WindowsACLEntry{ + Action: WindowsACLAllowWrite, + Path: cleaned, + Capability: input.PrincipalSID, + }) + } + for _, path := range normalizeProfilePaths(input.ReadRoots) { + entries = append(entries, WindowsACLEntry{ + Action: WindowsACLAllowRead, + Path: path, + Capability: input.PrincipalSID, + }) + } + + return WindowsACLPlan{Entries: dedupeWindowsACLEntries(entries)}, nil +} + +// windowsPrincipalRevokePlan returns the entries whose ACEs should be removed +// when a principal is retired. Revocation is by TRUSTEE rather than by path: +// every ACE naming this principal is dropped, so cleanup does not depend on +// remembering which paths were granted, and a grant added by an older version +// is still removed. +// +// This is the removal path the capability-SID model never had, where a synthetic +// SID left ACEs behind on the user's tree with nothing to match them against. +func windowsPrincipalRevokePlan(principalSID string, paths []string) (WindowsACLPlan, error) { + if principalSID == "" { + return WindowsACLPlan{}, errors.New("windows principal revoke plan requires a principal SID") + } + cleaned := normalizeProfilePaths(paths) + entries := make([]WindowsACLEntry, 0, len(cleaned)) + for _, path := range cleaned { + entries = append(entries, WindowsACLEntry{ + Action: windowsACLRevoke, + Path: path, + Capability: principalSID, + }) + } + return WindowsACLPlan{Entries: entries}, nil +} + +// windowsACLRevoke removes every ACE naming the trustee on a path, whatever +// access it granted or denied. +const windowsACLRevoke WindowsACLAction = "revoke" diff --git a/internal/sandbox/windows_identity_acl_test.go b/internal/sandbox/windows_identity_acl_test.go new file mode 100644 index 000000000..b9026337a --- /dev/null +++ b/internal/sandbox/windows_identity_acl_test.go @@ -0,0 +1,182 @@ +package sandbox + +import ( + "path/filepath" + "strings" + "testing" +) + +const testPrincipalSID = "S-1-5-21-1111111111-2222222222-3333333333-1005" + +func testPrincipalInput() windowsPrincipalACLInput { + return windowsPrincipalACLInput{ + PrincipalSID: testPrincipalSID, + WriteRoots: []WritableRoot{{ + Root: filepath.FromSlash("/ws/project"), + ReadOnlySubpaths: []string{filepath.FromSlash("/ws/project/.git/config")}, + ProtectedMetadataNames: []string{".zero", ".agents"}, + }}, + ReadRoots: []string{filepath.FromSlash("/usr/lib")}, + DenyRead: []string{filepath.FromSlash("/shared/secrets")}, + } +} + +// Windows evaluates an explicit DACL deny-before-allow, so a carve-out inside a +// granted root only survives if its deny ACE is written first. If the grant on +// the workspace landed before the deny on .zero, the protected metadata would be +// writable from inside the sandbox. +func TestPrincipalACLPlanEmitsDeniesBeforeAllows(t *testing.T) { + plan, err := buildWindowsPrincipalACLPlan(testPrincipalInput()) + if err != nil { + t.Fatalf("build: %v", err) + } + lastDeny, firstAllow := -1, -1 + for index, entry := range plan.Entries { + switch entry.Action { + case WindowsACLDenyRead, WindowsACLDenyWrite: + lastDeny = index + case WindowsACLAllowWrite, WindowsACLAllowRead: + if firstAllow == -1 { + firstAllow = index + } + } + } + if firstAllow == -1 || lastDeny == -1 { + t.Fatalf("plan is missing a deny or an allow: %+v", plan.Entries) + } + if lastDeny > firstAllow { + t.Fatalf("deny at %d comes after allow at %d; carve-outs would be overridden", lastDeny, firstAllow) + } +} + +// Every ACE must name the sandbox principal. An entry with any other trustee +// would change access for a real user. +func TestPrincipalACLPlanNamesOnlyThePrincipal(t *testing.T) { + plan, err := buildWindowsPrincipalACLPlan(testPrincipalInput()) + if err != nil { + t.Fatalf("build: %v", err) + } + for _, entry := range plan.Entries { + if entry.Capability != testPrincipalSID { + t.Fatalf("entry %+v names %q, want the principal SID", entry, entry.Capability) + } + } +} + +// A principal is a separate account with no inherent access, so a write root +// must be granted read+write and a read root granted read. Without the grant the +// sandbox cannot open its own workspace. +func TestPrincipalACLPlanGrantsRoots(t *testing.T) { + plan, err := buildWindowsPrincipalACLPlan(testPrincipalInput()) + if err != nil { + t.Fatalf("build: %v", err) + } + var grantedWrite, grantedRead bool + for _, entry := range plan.Entries { + if entry.Action == WindowsACLAllowWrite && entry.Path == normalizeProfilePath(filepath.FromSlash("/ws/project")) { + grantedWrite = true + } + if entry.Action == WindowsACLAllowRead && entry.Path == normalizeProfilePath(filepath.FromSlash("/usr/lib")) { + grantedRead = true + } + } + if !grantedWrite { + t.Fatal("write root was not granted; the sandbox could not write its workspace") + } + if !grantedRead { + t.Fatal("read root was not granted; the sandbox could not read it") + } +} + +// Protected metadata is denied write and marked Materialize so the ACE is +// created even when the directory does not exist yet, closing the window where +// a sandboxed command creates .zero before the deny lands. +func TestPrincipalACLPlanProtectsMetadata(t *testing.T) { + plan, err := buildWindowsPrincipalACLPlan(testPrincipalInput()) + if err != nil { + t.Fatalf("build: %v", err) + } + found := map[string]WindowsACLEntry{} + for _, entry := range plan.Entries { + found[entry.Path] = entry + } + for _, name := range []string{".zero", ".agents"} { + path := filepath.Join(normalizeProfilePath(filepath.FromSlash("/ws/project")), name) + entry, ok := found[path] + if !ok { + t.Fatalf("no entry protecting %s", path) + } + if entry.Action != WindowsACLDenyWrite { + t.Fatalf("%s has action %q, want deny-write", path, entry.Action) + } + if !entry.Materialize { + t.Fatalf("%s must be materialized so the deny exists before the directory does", path) + } + } +} + +// A missing principal SID must be a hard error: an empty trustee would either +// fail at apply time or, worse, be interpreted as some other account. +func TestPrincipalACLPlanRequiresSID(t *testing.T) { + input := testPrincipalInput() + input.PrincipalSID = "" + if _, err := buildWindowsPrincipalACLPlan(input); err == nil { + t.Fatal("an empty principal SID must be rejected") + } +} + +// A plan with no roots at all is a caller mistake rather than a valid empty +// grant, since the resulting sandbox could not run anything. +func TestPrincipalACLPlanRequiresRoots(t *testing.T) { + input := windowsPrincipalACLInput{PrincipalSID: testPrincipalSID} + if _, err := buildWindowsPrincipalACLPlan(input); err == nil { + t.Fatal("a plan with no roots must be rejected") + } +} + +// Revocation is keyed to the trustee, so retiring a principal removes every ACE +// naming it without having to remember what was granted. This is the cleanup +// path the capability-SID model lacks. +func TestPrincipalRevokePlanTargetsTrustee(t *testing.T) { + paths := []string{filepath.FromSlash("/ws/project"), filepath.FromSlash("/usr/lib")} + plan, err := windowsPrincipalRevokePlan(testPrincipalSID, paths) + if err != nil { + t.Fatalf("revoke plan: %v", err) + } + if len(plan.Entries) != len(paths) { + t.Fatalf("got %d entries, want %d", len(plan.Entries), len(paths)) + } + for _, entry := range plan.Entries { + if entry.Action != windowsACLRevoke { + t.Fatalf("entry %+v is not a revoke", entry) + } + if entry.Capability != testPrincipalSID { + t.Fatalf("revoke names %q, want the principal", entry.Capability) + } + } +} + +func TestPrincipalRevokePlanRequiresSID(t *testing.T) { + if _, err := windowsPrincipalRevokePlan("", []string{"/ws"}); err == nil { + t.Fatal("revoking without a principal SID must be rejected") + } +} + +// The action strings end up in a serialized plan consumed by the elevated +// helper, so they must stay stable and distinct from the existing actions. +func TestPrincipalACLActionsAreDistinct(t *testing.T) { + actions := []WindowsACLAction{ + WindowsACLAllowWrite, WindowsACLAllowRead, + WindowsACLDenyRead, WindowsACLDenyWrite, windowsACLRevoke, + } + seen := map[WindowsACLAction]bool{} + for _, action := range actions { + if strings.TrimSpace(string(action)) == "" { + t.Fatal("an action string is empty") + } + if seen[action] { + t.Fatalf("duplicate action %q", action) + } + seen[action] = true + } +} diff --git a/internal/sandbox/windows_identity_dpapi_windows.go b/internal/sandbox/windows_identity_dpapi_windows.go new file mode 100644 index 000000000..d9a0bbf1d --- /dev/null +++ b/internal/sandbox/windows_identity_dpapi_windows.go @@ -0,0 +1,76 @@ +//go:build windows + +package sandbox + +// DPAPI wrapping for the stored sandbox principal password. +// +// The file ACL is the primary control and remains the thing that keeps the +// sandbox principal itself from reading its own credential. This adds the layer +// the ACL cannot: an ACL is only meaningful while the filesystem is being asked +// to enforce it, so a backup, a mounted disk image, or a copy taken by anyone +// who can bypass the DACL yields the password in the clear. CryptProtectData +// binds the ciphertext to the invoking user's logon secret, so an offline copy +// is inert without that user's credentials. +// +// The principal name is passed as optional entropy, which makes a blob usable +// only for the account it was minted for; moving one secret file over another +// then fails to decrypt instead of silently authenticating the wrong principal. + +import ( + "errors" + "fmt" + "unsafe" + + "golang.org/x/sys/windows" +) + +// protectWindowsSecret encrypts a password to the current user. +// +// CRYPTPROTECT_UI_FORBIDDEN matters here: this runs inside a CLI and, on the +// setup path, potentially without an interactive desktop, so DPAPI must fail +// rather than try to prompt. +func protectWindowsSecret(plaintext string, entropy string) ([]byte, error) { + if plaintext == "" { + return nil, errors.New("windows sandbox secret: refusing to protect an empty password") + } + in := windows.DataBlob{ + Size: uint32(len(plaintext)), + Data: &[]byte(plaintext)[0], + } + entropyBytes := []byte(entropy) + var entropyBlob *windows.DataBlob + if len(entropyBytes) > 0 { + entropyBlob = &windows.DataBlob{Size: uint32(len(entropyBytes)), Data: &entropyBytes[0]} + } + var out windows.DataBlob + if err := windows.CryptProtectData(&in, nil, entropyBlob, 0, nil, windows.CRYPTPROTECT_UI_FORBIDDEN, &out); err != nil { + return nil, fmt.Errorf("protect sandbox secret: %w", err) + } + // DPAPI allocates the output with LocalAlloc; copy it out and hand it back. + defer windows.LocalFree(windows.Handle(unsafe.Pointer(out.Data))) + return append([]byte(nil), unsafe.Slice(out.Data, out.Size)...), nil +} + +// unprotectWindowsSecret reverses protectWindowsSecret. It fails for any user +// other than the one that wrote the blob, and for a blob minted with a different +// principal name as entropy. +func unprotectWindowsSecret(ciphertext []byte, entropy string) (string, error) { + if len(ciphertext) == 0 { + return "", errWindowsSandboxIdentityUnavailable + } + in := windows.DataBlob{ + Size: uint32(len(ciphertext)), + Data: &ciphertext[0], + } + entropyBytes := []byte(entropy) + var entropyBlob *windows.DataBlob + if len(entropyBytes) > 0 { + entropyBlob = &windows.DataBlob{Size: uint32(len(entropyBytes)), Data: &entropyBytes[0]} + } + var out windows.DataBlob + if err := windows.CryptUnprotectData(&in, nil, entropyBlob, 0, nil, windows.CRYPTPROTECT_UI_FORBIDDEN, &out); err != nil { + return "", fmt.Errorf("unprotect sandbox secret: %w", err) + } + defer windows.LocalFree(windows.Handle(unsafe.Pointer(out.Data))) + return string(unsafe.Slice(out.Data, out.Size)), nil +} diff --git a/internal/sandbox/windows_identity_logon_windows.go b/internal/sandbox/windows_identity_logon_windows.go new file mode 100644 index 000000000..45ebdaa5f --- /dev/null +++ b/internal/sandbox/windows_identity_logon_windows.go @@ -0,0 +1,270 @@ +//go:build windows + +package sandbox + +// Minting a token for a sandbox principal. +// +// A provisioned account is inert until something can log on as it. Windows +// gates that behind account rights held in the local security policy, so setup +// grants the principal exactly one: the right to be logged on as a batch job, +// which is what a non-interactive service-style logon needs. It is deliberately +// NOT granted interactive, network or remote-interactive logon, and those three +// are explicitly DENIED, so the account cannot be used to sign in at the +// console, over SMB, or through RDP even if its password leaked. The password +// exists only so LogonUser can mint a token; nobody is meant to type it. +// +// Rights are granted at setup (elevated) because LsaAddAccountRights requires +// administrator privileges. The per-command path only calls LogonUser, which +// needs no special privilege once the batch right is in place. + +import ( + "errors" + "fmt" + "runtime" + "unsafe" + + "golang.org/x/sys/windows" +) + +const ( + // Logon type/provider for a non-interactive token. Batch is the closest + // match for "run this command as a service-like principal": it produces a + // full token without a desktop or network-credential footprint. + logon32LogonBatch = 4 + logon32ProviderDefault = 0 + + // LSA policy access rights needed to add account rights. + policyCreateAccount = 0x00000010 + policyLookupNames = 0x00000800 + + // Account rights. The sandbox principal gets the batch right and is denied + // every interactive path. + seBatchLogonRight = "SeBatchLogonRight" + seDenyInteractiveLogonRight = "SeDenyInteractiveLogonRight" + seDenyNetworkLogonRight = "SeDenyNetworkLogonRight" + seDenyRemoteInteractiveRight = "SeDenyRemoteInteractiveLogonRight" + seDenyServiceLogonRightName = "SeDenyServiceLogonRight" + windowsIdentityLogonRightsNote = "granted by `zero sandbox setup`" +) + +var ( + procLogonUserW = windows.NewLazySystemDLL("advapi32.dll").NewProc("LogonUserW") + procLsaOpenPolicy = windows.NewLazySystemDLL("advapi32.dll").NewProc("LsaOpenPolicy") + procLsaClose = windows.NewLazySystemDLL("advapi32.dll").NewProc("LsaClose") + procLsaAddAccountRights = windows.NewLazySystemDLL("advapi32.dll").NewProc("LsaAddAccountRights") + // Retiring a principal has to drop its rights as well as its account, or the + // LSA policy database keeps an entry keyed to a SID that no longer resolves. + procLsaRemoveAccountRights = windows.NewLazySystemDLL("advapi32.dll").NewProc("LsaRemoveAccountRights") + procLsaNtStatusToWinErr = windows.NewLazySystemDLL("advapi32.dll").NewProc("LsaNtStatusToWinError") +) + +// lsaUnicodeString mirrors LSA_UNICODE_STRING. Length and MaximumLength are +// BYTE counts, not rune counts, which is the usual source of bugs here. +type lsaUnicodeString struct { + Length uint16 + MaximumLength uint16 + Buffer *uint16 +} + +// lsaObjectAttributes mirrors LSA_OBJECT_ATTRIBUTES. Every field except Length +// is unused for LsaOpenPolicy, but the struct must still be the right size. +type lsaObjectAttributes struct { + Length uint32 + RootDirectory windows.Handle + ObjectName *lsaUnicodeString + Attributes uint32 + SecurityDescriptor unsafe.Pointer + SecurityQualityOfService unsafe.Pointer +} + +// newLSAString builds an LSA_UNICODE_STRING over a UTF-16 buffer the caller +// keeps alive. The returned value borrows that buffer, so the buffer must +// outlive every use of the string. +func newLSAString(buffer []uint16) lsaUnicodeString { + if len(buffer) == 0 { + return lsaUnicodeString{} + } + // The buffer is NUL-terminated; the LSA length counts bytes WITHOUT the + // terminator, while MaximumLength counts bytes WITH it. + runes := len(buffer) - 1 + return lsaUnicodeString{ + Length: uint16(runes * 2), + MaximumLength: uint16(len(buffer) * 2), + Buffer: &buffer[0], + } +} + +// lsaStatusError converts an NTSTATUS from an Lsa* call into a Go error, going +// through LsaNtStatusToWinError so the message is the familiar Win32 one rather +// than a raw NTSTATUS. +func lsaStatusError(call string, status uintptr) error { + if status == 0 { + return nil + } + winErr, _, _ := procLsaNtStatusToWinErr.Call(status) + return fmt.Errorf("%s: %w", call, windows.Errno(winErr)) +} + +// grantWindowsSandboxLogonRights gives the principal the batch logon right and +// denies every interactive logon path. Idempotent: LsaAddAccountRights silently +// succeeds when the account already holds a right, so setup can re-run. +// +// Requires an elevated caller. +func grantWindowsSandboxLogonRights(sid *windows.SID) error { + if sid == nil { + return fmt.Errorf("grant sandbox logon rights: nil SID") + } + var attributes lsaObjectAttributes + attributes.Length = uint32(unsafe.Sizeof(attributes)) + var policy windows.Handle + status, _, _ := procLsaOpenPolicy.Call( + 0, // local system + uintptr(unsafe.Pointer(&attributes)), + uintptr(policyCreateAccount|policyLookupNames), + uintptr(unsafe.Pointer(&policy)), + ) + // LsaOpenPolicy borrows the attributes struct by address, so it has to stay + // reachable until the call has returned. + runtime.KeepAlive(attributes) + if err := lsaStatusError("LsaOpenPolicy", status); err != nil { + return err + } + defer procLsaClose.Call(uintptr(policy)) + + rights := []string{ + seBatchLogonRight, + seDenyInteractiveLogonRight, + seDenyNetworkLogonRight, + seDenyRemoteInteractiveRight, + seDenyServiceLogonRightName, + } + // Each right is added on its own call so one unsupported name on an odd SKU + // cannot silently drop the others. + for _, right := range rights { + buffer, err := windows.UTF16FromString(right) + if err != nil { + return err + } + entry := newLSAString(buffer) + status, _, _ := procLsaAddAccountRights.Call( + uintptr(policy), + uintptr(unsafe.Pointer(sid)), + uintptr(unsafe.Pointer(&entry)), + 1, + ) + // Both the descriptor and the buffer it points at are borrowed by the + // call. Kept alive before the error check, not after, so the failure path + // does not return with them already collectable. + runtime.KeepAlive(entry) + runtimeKeepAliveUint16(buffer) + if err := lsaStatusError("LsaAddAccountRights("+right+")", status); err != nil { + return err + } + } + return nil +} + +// revokeWindowsSandboxLogonRights drops every account right held by a principal +// and removes its entry from the LSA policy database. +// +// This is the logon-rights counterpart to revoking ACEs by trustee, and it has +// the same reason to exist: deleting the account on its own leaves the rights +// behind, keyed to a SID that no longer resolves, which is the orphaned residue +// this model is supposed to avoid. It must therefore run BEFORE the account is +// deleted, while the SID is still resolvable. +// +// Removing all rights rather than naming them is deliberate. The principal is +// being retired, so anything keyed to it should go, including rights a previous +// version of setup granted and this one no longer knows about. +// +// Requires an elevated caller. A principal that holds no rights is not an error: +// LsaRemoveAccountRights reports ERROR_FILE_NOT_FOUND for an account with no LSA +// entry, which is the state teardown is trying to reach anyway. +func revokeWindowsSandboxLogonRights(sid *windows.SID) error { + if sid == nil { + return fmt.Errorf("revoke sandbox logon rights: nil SID") + } + var attributes lsaObjectAttributes + attributes.Length = uint32(unsafe.Sizeof(attributes)) + var policy windows.Handle + status, _, _ := procLsaOpenPolicy.Call( + 0, // local system + uintptr(unsafe.Pointer(&attributes)), + uintptr(policyCreateAccount|policyLookupNames), + uintptr(unsafe.Pointer(&policy)), + ) + runtime.KeepAlive(attributes) + if err := lsaStatusError("LsaOpenPolicy", status); err != nil { + return err + } + defer procLsaClose.Call(uintptr(policy)) + + status, _, _ = procLsaRemoveAccountRights.Call( + uintptr(policy), + uintptr(unsafe.Pointer(sid)), + 1, // AllRights: drop everything and delete the LSA account object + 0, // UserRights ignored when AllRights is set + 0, // CountOfRights likewise + ) + runtime.KeepAlive(sid) + if err := lsaStatusError("LsaRemoveAccountRights", status); err != nil { + if errors.Is(err, windows.ERROR_FILE_NOT_FOUND) { + return nil + } + return err + } + return nil +} + +// logonWindowsSandboxPrincipal mints a primary token for the sandbox account. +// The caller owns the returned token and must Close it. +// +// This needs no elevation: the batch logon right granted at setup is what makes +// it work, which is why the per-command path can run unelevated once setup has +// been done once. +func logonWindowsSandboxPrincipal(username string, password string) (windows.Token, error) { + user, err := windows.UTF16PtrFromString(username) + if err != nil { + return 0, err + } + // "." is the local machine, so the lookup never leaves this host even if the + // machine is domain-joined and a same-named domain account exists. + domain, err := windows.UTF16PtrFromString(".") + if err != nil { + return 0, err + } + secret, err := windows.UTF16PtrFromString(password) + if err != nil { + return 0, err + } + var token windows.Token + result, _, callErr := procLogonUserW.Call( + uintptr(unsafe.Pointer(user)), + uintptr(unsafe.Pointer(domain)), + uintptr(unsafe.Pointer(secret)), + logon32LogonBatch, + logon32ProviderDefault, + uintptr(unsafe.Pointer(&token)), + ) + // The three strings are borrowed for the duration of the call. + runtime.KeepAlive(user) + runtime.KeepAlive(domain) + runtime.KeepAlive(secret) + if result == 0 { + if callErr != nil && callErr != windows.ERROR_SUCCESS { + return 0, fmt.Errorf("LogonUser(%s): %w", username, callErr) + } + return 0, fmt.Errorf("LogonUser(%s) failed", username) + } + return token, nil +} + +// runtimeKeepAliveUint16 keeps a UTF-16 buffer reachable across a syscall that +// borrows it. Declared rather than inlined so the intent is explicit at each +// call site; the compiler must not free the slice while LSA holds the pointer. +func runtimeKeepAliveUint16(buffer []uint16) { + if len(buffer) == 0 { + return + } + _ = buffer[0] +} diff --git a/internal/sandbox/windows_identity_logon_windows_test.go b/internal/sandbox/windows_identity_logon_windows_test.go new file mode 100644 index 000000000..503d12922 --- /dev/null +++ b/internal/sandbox/windows_identity_logon_windows_test.go @@ -0,0 +1,41 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "testing" + + "golang.org/x/sys/windows" +) + +// revokeWindowsSandboxLogonRights treats "this account holds no rights" as +// success, because that is the state teardown is trying to reach anyway. That +// relies on the NTSTATUS for it surviving the trip through LsaNtStatusToWinError +// as an error errors.Is can still match, which is exactly the kind of Windows +// errno assumption that quietly turns out to be false. Assert it rather than +// trust it. +// +// Needs no privilege: LsaNtStatusToWinError is a pure status translation, so +// this runs everywhere rather than joining the gated set. +func TestLsaStatusErrorMapsObjectNameNotFound(t *testing.T) { + // STATUS_OBJECT_NAME_NOT_FOUND, what LsaRemoveAccountRights reports for an + // account that has no LSA entry. + const statusObjectNameNotFound = 0xC0000034 + + err := lsaStatusError("LsaRemoveAccountRights", statusObjectNameNotFound) + if err == nil { + t.Fatal("a nonzero NTSTATUS produced no error") + } + if !errors.Is(err, windows.ERROR_FILE_NOT_FOUND) { + t.Fatalf("error = %v, want one errors.Is matches against ERROR_FILE_NOT_FOUND; "+ + "without that, revoking a principal that simply holds no rights fails teardown", err) + } + + // The tolerance must be specific. If any failure matched it, revoke would + // swallow a real one and teardown would report success having done nothing. + const statusAccessDenied = 0xC0000022 + if other := lsaStatusError("LsaRemoveAccountRights", statusAccessDenied); errors.Is(other, windows.ERROR_FILE_NOT_FOUND) { + t.Fatalf("access denied matched the not-found tolerance: %v", other) + } +} diff --git a/internal/sandbox/windows_identity_policy_windows_test.go b/internal/sandbox/windows_identity_policy_windows_test.go new file mode 100644 index 000000000..f1137cebd --- /dev/null +++ b/internal/sandbox/windows_identity_policy_windows_test.go @@ -0,0 +1,391 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "golang.org/x/sys/windows" +) + +// Policy deny-write has to reach the principal plan. +// +// The capability plan has always emitted these. The principal plan denied write +// only on protected metadata and read-only subpaths inside write roots, so once +// the runner used a principal token a policy deny sitting anywhere else was not +// enforced at the OS layer at all, and a shell child could write where the +// restricted-token backend would have stopped it. +func TestPrincipalACLPlanCarriesPolicyDenyWrite(t *testing.T) { + root := filepath.Join(t.TempDir(), "workspace") + denied := filepath.Join(root, "protected", "keep-out") + + plan, err := buildWindowsPrincipalACLPlan(windowsPrincipalACLInput{ + PrincipalSID: "S-1-5-21-0-0-0-1001", + WriteRoots: []WritableRoot{{Root: root}}, + DenyWrite: []string{denied}, + }) + if err != nil { + t.Fatalf("buildWindowsPrincipalACLPlan: %v", err) + } + + entry, ok := findPrincipalACLEntry(plan, WindowsACLDenyWrite, denied) + if !ok { + t.Fatalf("no deny-write ACE for the policy path; plan = %+v", plan.Entries) + } + // Materialized for the same reason the capability plan does it: the applier + // skips targets that do not exist, so a deny on a path created after setup + // would never be written. + if !entry.Materialize { + t.Error("policy deny-write ACE is not materialized, so it is skipped when the path does not exist yet") + } +} + +// Deny-read has to be materialized too, which it was not. +func TestPrincipalACLPlanMaterializesDenyRead(t *testing.T) { + root := filepath.Join(t.TempDir(), "workspace") + secret := filepath.Join(t.TempDir(), "elsewhere", "creds") + + plan, err := buildWindowsPrincipalACLPlan(windowsPrincipalACLInput{ + PrincipalSID: "S-1-5-21-0-0-0-1001", + WriteRoots: []WritableRoot{{Root: root}}, + DenyRead: []string{secret}, + }) + if err != nil { + t.Fatalf("buildWindowsPrincipalACLPlan: %v", err) + } + entry, ok := findPrincipalACLEntry(plan, WindowsACLDenyRead, secret) + if !ok { + t.Fatalf("no deny-read ACE emitted; plan = %+v", plan.Entries) + } + if !entry.Materialize { + t.Fatal("deny-read ACE is not materialized, so a path created after setup never gets one") + } +} + +// Deny entries must still precede the grants they carve out of, which is what +// makes them win under Windows DACL evaluation. Adding deny-write to the plan is +// only safe if it did not disturb that ordering. +func TestPrincipalACLPlanKeepsDeniesBeforeGrants(t *testing.T) { + root := filepath.Join(t.TempDir(), "workspace") + plan, err := buildWindowsPrincipalACLPlan(windowsPrincipalACLInput{ + PrincipalSID: "S-1-5-21-0-0-0-1001", + WriteRoots: []WritableRoot{{Root: root}}, + DenyWrite: []string{filepath.Join(root, "nope")}, + DenyRead: []string{filepath.Join(root, "secret")}, + }) + if err != nil { + t.Fatalf("buildWindowsPrincipalACLPlan: %v", err) + } + firstGrant := -1 + for i, entry := range plan.Entries { + switch entry.Action { + case WindowsACLAllowWrite, WindowsACLAllowRead: + if firstGrant == -1 { + firstGrant = i + } + case WindowsACLDenyRead, WindowsACLDenyWrite: + if firstGrant != -1 { + t.Fatalf("deny entry at %d follows a grant at %d; the grant would win", i, firstGrant) + } + } + } +} + +func findPrincipalACLEntry(plan WindowsACLPlan, action WindowsACLAction, path string) (WindowsACLEntry, bool) { + want := normalizeProfilePath(path) + for _, entry := range plan.Entries { + if entry.Action == action && entry.Path == want { + return entry, true + } + } + return WindowsACLEntry{}, false +} + +// An adopted account must not have its password rotated during provisioning. +// +// Rotating there left every later step running against an account whose password +// had already been replaced with nothing on disk holding it. Any failure in +// between stranded a working principal: the rollback correctly declined to +// delete an account it had not created, so what remained was a live account +// authenticated by a password no longer stored anywhere, and the command path +// read the absent secret as "not provisioned" and quietly fell back to the +// weaker backend. +func TestProvisionWindowsSandboxIdentityDefersPasswordRotation(t *testing.T) { + stubWindowsProvisioning(t, true, nil, nil) + + rotated := false + previous := resetWindowsSandboxUserPasswordFn + t.Cleanup(func() { resetWindowsSandboxUserPasswordFn = previous }) + resetWindowsSandboxUserPasswordFn = func(string, string) error { + rotated = true + return nil + } + + if _, _, _, err := provisionWindowsSandboxIdentity("workspacekey"); err != nil { + t.Fatalf("provisioning an adopted account: %v", err) + } + if rotated { + t.Fatal("provisioning rotated the password; the window this closes lasts until the secret is committed") + } +} + +// The ownership comment carries the full workspace key, so two workspaces whose +// digests collide in the 11 characters the account name can hold are refused +// rather than silently sharing one account, one secret and one ACL identity. +func TestWindowsSandboxUserCommentDistinguishesWorkspaces(t *testing.T) { + first := windowsSandboxUserCommentFor("aaaaaaaaaaaabbbbbbbb") + second := windowsSandboxUserCommentFor("aaaaaaaaaaaacccccccc") + if first == second { + t.Fatal("two workspaces produced the same ownership comment, so a name collision would be adopted") + } + if !strings.HasPrefix(first, windowsSandboxUserComment) { + t.Fatalf("comment %q lost the marker prefix that identifies it as ours", first) + } + // The names DO collide, which is the whole reason the comment has to carry + // the key. If this stops being true the test is no longer covering anything. + if windowsSandboxUserName("aaaaaaaaaaaabbbbbbbb") != windowsSandboxUserName("aaaaaaaaaaaacccccccc") { + t.Skip("account names no longer collide for these keys; revisit what this test is for") + } +} + +// Rollback must not strip an adopted principal's logon rights. +// +// revokeWindowsSandboxLogonRights passes AllRights, which drops every right the +// account holds and deletes its LSA object. On an account this run created that +// is a rollback; on one it adopted it destroys the SeBatchLogonRight and +// deny-logon rights an earlier setup established, which is the working +// principal this path exists to preserve. +func TestSetupRollbackRevokesRightsOnlyForCreatedPrincipals(t *testing.T) { + for name, testCase := range map[string]struct { + existed bool + wantRevoked bool + }{ + "adopted principal": {existed: true, wantRevoked: false}, + "created principal": {existed: false, wantRevoked: true}, + } { + t.Run(name, func(t *testing.T) { + stubWindowsProvisioning(t, testCase.existed, nil, nil) + + revoked := false + prevGrant, prevRevoke := grantWindowsSandboxLogonRightsFn, revokeWindowsSandboxLogonRightsFn + t.Cleanup(func() { + grantWindowsSandboxLogonRightsFn, revokeWindowsSandboxLogonRightsFn = prevGrant, prevRevoke + }) + // Fail the grant so the undo path runs with rights already attempted, + // which is the state that used to revoke unconditionally. + grantWindowsSandboxLogonRightsFn = func(*windows.SID) error { + return errors.New("LSA grant refused by policy") + } + revokeWindowsSandboxLogonRightsFn = func(*windows.SID) error { + revoked = true + return nil + } + + config := WindowsSandboxCommandConfig{ + SandboxHome: t.TempDir(), + WorkspaceRoots: []string{`C:\ws`}, + } + if _, _, err := provisionWindowsSandboxPrincipalForSetup(config); err == nil { + t.Fatal("provisioning reported success despite an injected grant failure") + } + if revoked != testCase.wantRevoked { + if testCase.wantRevoked { + t.Fatal("rights were not revoked for an account this run created, leaving LSA entries keyed to a SID about to be deleted") + } + t.Fatal("rights were revoked for an adopted account; AllRights drops its pre-existing rights and deletes the LSA object") + } + }) + } +} + +// A secret the current user cannot read is unavailability, not breakage. +// +// The secret's DACL names whoever ran setup. An operator who elevated with a +// separate administrative account, via runas or an over-the-shoulder UAC +// prompt, leaves a secret their ordinary account cannot open. Treating that as a +// hard error made every sandboxed command fail on a machine that was merely set +// up by a different admin; it belongs in the same fail-soft path as a missing +// secret, so the warning fires and the restricted token takes over. +func TestReadWindowsSandboxSecretTreatsPermissionDeniedAsUnavailable(t *testing.T) { + previous := readWindowsSandboxSecretFile + t.Cleanup(func() { readWindowsSandboxSecretFile = previous }) + readWindowsSandboxSecretFile = func(string) ([]byte, error) { + return nil, &os.PathError{Op: "open", Path: "secret", Err: windows.ERROR_ACCESS_DENIED} + } + + if _, err := readWindowsSandboxSecret(`C:\anything.secret`); err != errWindowsSandboxIdentityUnavailable { + t.Fatalf("permission-denied read returned %v, want errWindowsSandboxIdentityUnavailable so the command falls back", err) + } +} + +// A workspace must not bind to another workspace's principal. +// +// The account name carries only 11 characters of the workspace digest, so two +// workspaces can derive the same name. Provisioning refuses that case by +// checking the full key in the account comment, but the command path resolved +// the name straight to a SID. The workspace that lost the race would have failed +// setup and then quietly run as the other one's principal, using its secret and +// its ACL identity. +func TestLookupWindowsSandboxIdentityRejectsForeignWorkspace(t *testing.T) { + previous := windowsSandboxUserIsManagedFn + t.Cleanup(func() { windowsSandboxUserIsManagedFn = previous }) + + prevSID := resolveWindowsSandboxSIDFn + t.Cleanup(func() { resolveWindowsSandboxSIDFn = prevSID }) + // The account resolves; whether it BELONGS to this workspace is the question. + resolveWindowsSandboxSIDFn = func(string) (*windows.SID, error) { + return windows.CreateWellKnownSid(windows.WinLocalSystemSid) + } + + var askedKey string + windowsSandboxUserIsManagedFn = func(_ string, workspaceKey string) (bool, error) { + askedKey = workspaceKey + return false, nil + } + _, err := lookupWindowsSandboxIdentity("workspacekey") + if err == nil { + t.Fatal("lookup accepted an account belonging to another workspace") + } + if errors.Is(err, errWindowsSandboxIdentityUnavailable) { + t.Fatal("a foreign account must not read as unprovisioned; that would silently fall back instead of reporting the conflict") + } + if askedKey != "" && askedKey != "workspacekey" { + t.Fatalf("ownership was checked against %q, want the caller's workspace key", askedKey) + } +} + +// An account that does not exist must stay the unavailable sentinel rather than +// becoming a collision error, since that is the ordinary not-set-up state. +func TestLookupWindowsSandboxIdentityAbsentAccountIsUnavailable(t *testing.T) { + previous := windowsSandboxUserIsManagedFn + t.Cleanup(func() { windowsSandboxUserIsManagedFn = previous }) + windowsSandboxUserIsManagedFn = func(string, string) (bool, error) { + t.Fatal("ownership must not be consulted for an account that does not resolve") + return false, nil + } + if _, err := lookupWindowsSandboxIdentity("nosuchworkspacekey"); !errors.Is(err, errWindowsSandboxIdentityUnavailable) { + t.Fatalf("absent account returned %v, want errWindowsSandboxIdentityUnavailable", err) + } +} + +// The git control-plane carveouts must be materialized. +// +// gitMetadataWriteCarveouts supplies .git/config and .git/hooks as +// ReadOnlySubpaths of the workspace. On a workspace where git has not run yet +// they do not exist when setup applies the plan, and applyWindowsACLPlan skips +// an absent target, so the deny ACEs were never written. Once git created those +// paths the principal still held inherited write access and could install a +// hook or rewrite credential.helper. +func TestPrincipalACLPlanMaterializesReadOnlySubpaths(t *testing.T) { + root := filepath.Join(t.TempDir(), "workspace") + carveout := filepath.Join(root, ".git", "config") + + plan, err := buildWindowsPrincipalACLPlan(windowsPrincipalACLInput{ + PrincipalSID: "S-1-5-21-0-0-0-1001", + WriteRoots: []WritableRoot{{Root: root, ReadOnlySubpaths: []string{carveout}}}, + }) + if err != nil { + t.Fatalf("buildWindowsPrincipalACLPlan: %v", err) + } + entry, ok := findPrincipalACLEntry(plan, WindowsACLDenyWrite, carveout) + if !ok { + t.Fatalf("no deny-write ACE for the git carveout; plan = %+v", plan.Entries) + } + if !entry.Materialize { + t.Fatal("git carveout deny-write is not materialized, so it is skipped on a workspace where .git does not exist yet") + } +} + +// Setup must grant the principal the same runtime root that commands write to. +// +// permissionProfileWithRuntime appends this root to WriteRoots on every command +// and redirects HOME, GOCACHE and npm_config_cache into it, but it lives under +// the user cache rather than the workspace, so the profile setup sees never +// contains it. A principal is a separate account with no rights there, so +// without a grant every npm install or go build fails on a cache write. +// +// The assertion that matters is that the two derivations agree. If they drift, +// setup writes the ACE on one directory while commands use another, and the +// symptom is a bare ACCESS_DENIED with nothing pointing at the sandbox. +func TestSetupGrantsTheRuntimeRootCommandsActuallyUse(t *testing.T) { + workspace := filepath.Join(t.TempDir(), "ws") + if err := os.MkdirAll(workspace, 0o700); err != nil { + t.Fatal(err) + } + + granted, err := setupWindowsSandboxRuntimeRoot(WindowsSandboxCommandConfig{ + WorkspaceRoots: []string{workspace}, + }) + if err != nil { + t.Fatalf("setupWindowsSandboxRuntimeRoot: %v", err) + } + if granted == "" { + t.Fatal("no runtime root resolved for a configured workspace") + } + if info, err := os.Stat(granted); err != nil || !info.IsDir() { + t.Fatalf("runtime root %q was not created; applyWindowsACLPlan skips absent targets so the grant would no-op (stat err %v)", granted, err) + } + + // What a command would actually use. + runtimeState, release, err := prepareSandboxRuntime(workspace) + if err != nil { + t.Fatalf("prepareSandboxRuntime: %v", err) + } + if release != nil { + defer release() + } + if filepath.Clean(runtimeState.Root) != filepath.Clean(granted) { + t.Fatalf("setup granted %q but commands write to %q", granted, runtimeState.Root) + } +} + +// No workspace root means nothing to grant, which is not an error. +func TestSetupRuntimeRootWithoutWorkspaceIsNotAnError(t *testing.T) { + granted, err := setupWindowsSandboxRuntimeRoot(WindowsSandboxCommandConfig{}) + if err != nil { + t.Fatalf("no workspace root should not error: %v", err) + } + if granted != "" { + t.Fatalf("granted %q with no workspace configured", granted) + } +} + +// An account in a privileged group must not be adopted, even when name and +// ownership comment say it is ours. +// +// Adoption resets the password and hands the account to the sandbox. If that +// account is also in Administrators, the sandbox gains the rights the sandbox +// exists to withhold: rewriting the ACLs confining it, reading the secret locked +// to the invoking user, and stopping Zero. +func TestProvisionWindowsSandboxIdentityRefusesPrivilegedAccount(t *testing.T) { + stubWindowsProvisioning(t, true, nil, nil) + + previous := windowsSandboxUserIsPrivilegedFn + t.Cleanup(func() { windowsSandboxUserIsPrivilegedFn = previous }) + windowsSandboxUserIsPrivilegedFn = func(string) (bool, error) { return true, nil } + + _, _, created, err := provisionWindowsSandboxIdentity("workspacekey") + if !errors.Is(err, errWindowsSandboxPrivilegedAccount) { + t.Fatalf("provisioning adopted a privileged account, err = %v", err) + } + if created { + t.Fatal("created must stay false for an account this run refused to adopt") + } +} + +// The ordinary adopted account is unaffected. +func TestProvisionWindowsSandboxIdentityAdoptsUnprivilegedAccount(t *testing.T) { + stubWindowsProvisioning(t, true, nil, nil) + + previous := windowsSandboxUserIsPrivilegedFn + t.Cleanup(func() { windowsSandboxUserIsPrivilegedFn = previous }) + windowsSandboxUserIsPrivilegedFn = func(string) (bool, error) { return false, nil } + + if _, _, _, err := provisionWindowsSandboxIdentity("workspacekey"); err != nil { + t.Fatalf("an unprivileged managed account must still be adopted: %v", err) + } +} diff --git a/internal/sandbox/windows_identity_rollback_windows_test.go b/internal/sandbox/windows_identity_rollback_windows_test.go new file mode 100644 index 000000000..e4713d8e4 --- /dev/null +++ b/internal/sandbox/windows_identity_rollback_windows_test.go @@ -0,0 +1,160 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "testing" + + "golang.org/x/sys/windows" +) + +// stubWindowsProvisioning replaces the four provisioning syscalls so the +// function can run on an ordinary machine. Every one of them needs an elevated +// caller and a real local account, so without this the test would stop at the +// first call and never reach the behaviour it is named for. +func stubWindowsProvisioning(t *testing.T, existed bool, groupErr error, sidErr error) { + t.Helper() + prevGroup, prevUser := ensureWindowsSandboxGroupFn, ensureWindowsSandboxUserFn + prevAdd, prevSID := addWindowsSandboxUserToGroupFn, resolveWindowsSandboxSIDFn + prevManaged := windowsSandboxUserIsManagedFn + t.Cleanup(func() { + ensureWindowsSandboxGroupFn, ensureWindowsSandboxUserFn = prevGroup, prevUser + addWindowsSandboxUserToGroupFn, resolveWindowsSandboxSIDFn = prevAdd, prevSID + windowsSandboxUserIsManagedFn = prevManaged + }) + + ensureWindowsSandboxGroupFn = func() error { return nil } + // Adopted accounts are ours in these tests; the ownership check is a real + // syscall and would otherwise refuse before the code under test runs. + windowsSandboxUserIsManagedFn = func(string, string) (bool, error) { return true, nil } + ensureWindowsSandboxUserFn = func(string, string, string) (bool, error) { return existed, nil } + addWindowsSandboxUserToGroupFn = func(string) error { return groupErr } + resolveWindowsSandboxSIDFn = func(username string) (*windows.SID, error) { + if sidErr != nil { + return nil, sidErr + } + return windows.CreateWellKnownSid(windows.WinLocalSystemSid) + } +} + +// A failure after the account has been created must still hand back the name. +// +// The caller's rollback deletes by identity.Username, so returning a zero +// identity alongside created=true asked it to delete "" and quietly left the +// account this run had just made. Group attachment is the case that matters +// most: it is the enforcement boundary and it can fail under local policy. +func TestProvisionWindowsSandboxIdentityReturnsNameForRollback(t *testing.T) { + groupFailure := errors.New("group attachment refused by policy") + sidFailure := errors.New("sid lookup failed") + + for name, testCase := range map[string]struct { + groupErr error + sidErr error + }{ + "group attachment fails": {groupErr: groupFailure}, + "sid resolution fails": {sidErr: sidFailure}, + } { + t.Run(name, func(t *testing.T) { + stubWindowsProvisioning(t, false, testCase.groupErr, testCase.sidErr) + + identity, _, created, err := provisionWindowsSandboxIdentity("workspacekey") + if err == nil { + t.Fatal("provisioning reported success despite an injected failure") + } + if !created { + t.Fatal("created = false, so the rollback would skip an account this run made") + } + // The whole point: without a name there is nothing to delete. + if identity.Username == "" { + t.Fatal("identity carries no username, so the rollback deletes \"\" and strands the account") + } + if want := windowsSandboxUserName("workspacekey"); identity.Username != want { + t.Fatalf("username = %q, want %q", identity.Username, want) + } + }) + } +} + +// An account that already existed must not be deleted because a later step +// failed. created=false is what stops the rollback turning a partial failure +// into the loss of a working principal from an earlier setup. +func TestProvisionWindowsSandboxIdentityDoesNotClaimPreexistingAccount(t *testing.T) { + stubWindowsProvisioning(t, true, errors.New("group attachment refused"), nil) + + _, _, created, err := provisionWindowsSandboxIdentity("workspacekey") + if err == nil { + t.Fatal("provisioning reported success despite an injected failure") + } + // created is the whole assertion. It is what stops the rollback deleting an + // account it did not make, and it must stay false however provisioning + // fails. The identity is deliberately not checked: an adopted account exits + // early at the ownership check, which is a real syscall and not stubbed + // here, so asserting on the name would be testing the stub rather than the + // contract. + if created { + t.Fatal("created = true for an account this run did not create; rollback would delete a working principal") + } +} + +// The write grant has to include delete. +// +// FILE_GENERIC_WRITE covers creating and modifying but not removing or +// renaming, and a rename needs delete on the source. The old same-user token hid +// this because the caller already held inherited rights on its own tree; a +// principal is a separate account with none, so without these it can write files +// it can never remove, which fails ordinary editing and most git operations +// rather than an edge case. +func TestWindowsACLAllowWriteGrantsDelete(t *testing.T) { + mode, mask, err := windowsACLAccess(WindowsACLAllowWrite) + if err != nil { + t.Fatalf("windowsACLAccess: %v", err) + } + if mode != windows.GRANT_ACCESS { + t.Fatalf("mode = %v, want GRANT_ACCESS", mode) + } + // Atomic bits only. FILE_GENERIC_READ and FILE_GENERIC_WRITE both carry + // READ_CONTROL and SYNCHRONIZE, so testing a composite constant with & is + // satisfied by any grant at all and proves nothing. + for label, bit := range map[string]windows.ACCESS_MASK{ + "DELETE": windows.DELETE, + "FILE_DELETE_CHILD": windowsFileDeleteChild, + "FILE_WRITE_DATA": windows.FILE_WRITE_DATA, + } { + if mask&bit == 0 { + t.Errorf("write grant is missing %s", label) + } + } + // Granting these would let the principal rewrite the very restrictions + // placed on it. They are in the deny mask for that reason and must not + // appear here. + for label, bit := range map[string]windows.ACCESS_MASK{ + "WRITE_DAC": windows.WRITE_DAC, + "WRITE_OWNER": windows.WRITE_OWNER, + } { + if mask&bit != 0 { + t.Errorf("write grant unexpectedly includes %s", label) + } + } +} + +// The read grant must stay read-only. Widening the write mask above is only +// safe if this one did not move with it. +func TestWindowsACLAllowReadGrantsNoDelete(t *testing.T) { + _, mask, err := windowsACLAccess(WindowsACLAllowRead) + if err != nil { + t.Fatalf("windowsACLAccess: %v", err) + } + // Atomic bits, for the same reason as above: the read and write composites + // overlap on the standard rights, so a composite check here would report a + // failure that is not real. + for label, bit := range map[string]windows.ACCESS_MASK{ + "DELETE": windows.DELETE, + "FILE_DELETE_CHILD": windowsFileDeleteChild, + "FILE_WRITE_DATA": windows.FILE_WRITE_DATA, + } { + if mask&bit != 0 { + t.Errorf("read grant unexpectedly includes %s", label) + } + } +} diff --git a/internal/sandbox/windows_identity_runtime_windows.go b/internal/sandbox/windows_identity_runtime_windows.go new file mode 100644 index 000000000..916cf27f0 --- /dev/null +++ b/internal/sandbox/windows_identity_runtime_windows.go @@ -0,0 +1,405 @@ +//go:build windows + +package sandbox + +// Using a sandbox principal at command time. +// +// This is the seam between the identity model and the existing runner. It is +// deliberately fail-soft: when no principal is provisioned, when the secret is +// missing, or when the opt-in is off, it reports "not available" and the caller +// keeps using today's restricted-token backend. Only an outright failure to log +// on with a principal that IS provisioned surfaces as an error, because that +// means setup ran but the identity is broken, and silently downgrading the +// sandbox in that case would be the wrong kind of quiet. + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "golang.org/x/sys/windows" +) + +// windowsSandboxIdentityEnv opts a machine into the principal backend while it +// is still experimental. Provisioning is inert without it, so an existing +// install keeps the restricted-token behaviour until someone turns this on. +const windowsSandboxIdentityEnv = "ZERO_WINDOWS_SANDBOX_IDENTITY" + +// windowsSandboxIdentityEnabled reports whether the principal backend is opted +// into. Kept as a function so the check reads the environment at call time, +// which is what lets a test or an elevated setup run flip it. +func windowsSandboxIdentityEnabled(env map[string]string) bool { + if value, ok := env[windowsSandboxIdentityEnv]; ok { + return strings.TrimSpace(value) == "1" + } + return strings.TrimSpace(os.Getenv(windowsSandboxIdentityEnv)) == "1" +} + +// windowsSandboxWorkspaceKey derives the per-workspace key a principal is named +// after. It hashes the workspace root the same way the sandbox runtime keys its +// own state, so the account name leaks no path and one workspace always maps to +// one principal. +func windowsSandboxWorkspaceKey(workspaceRoots []string) string { + root := "" + for _, candidate := range workspaceRoots { + if trimmed := strings.TrimSpace(candidate); trimmed != "" { + root = normalizeProfilePath(trimmed) + break + } + } + if root == "" { + root = "default" + } + digest := sha256.Sum256([]byte(strings.ToLower(root))) + return hex.EncodeToString(digest[:]) +} + +// windowsSandboxPrincipalEligible reports whether the principal backend may be +// used for this command at all, before any account or secret is consulted. +// +// Kept separate from the lookup so the decision is observable on its own: on a +// machine with nothing provisioned the lookup declines anyway, which would let a +// missing guard here pass unnoticed. +func windowsSandboxPrincipalEligible(config WindowsSandboxCommandConfig) bool { + if !windowsSandboxIdentityEnabled(config.Env) { + return false + } + // Network denial is enforced by WFP filters keyed to the offline-marker SID, + // which the restricted token carries and a principal token cannot: LogonUser + // mints a token for the account, not for a synthetic capability SID. Using a + // principal here would leave those filters matching nothing and silently drop + // network enforcement, which is a worse trade than the read confinement it + // buys. Fall back to the restricted token, which still enforces the network, + // until the filters are also keyed to the principal's own SID. + return config.PermissionProfile.Network.Mode != NetworkDeny +} + +// windowsSandboxPrincipalToken returns a token for this workspace's sandbox +// principal. +// +// ok is false, with a nil error, whenever the principal backend simply is not in +// play: the opt-in is off, setup has not provisioned an account, or no secret is +// stored. The caller falls back to the restricted token in those cases. An error +// means the identity exists but could not be used, which is worth surfacing +// rather than downgrading around. +func windowsSandboxPrincipalToken(config WindowsSandboxCommandConfig) (windows.Token, bool, error) { + if !windowsSandboxPrincipalEligible(config) { + return 0, false, nil + } + key := windowsSandboxWorkspaceKey(config.WorkspaceRoots) + identity, err := lookupWindowsSandboxIdentity(key) + if err != nil { + if errors.Is(err, errWindowsSandboxIdentityUnavailable) { + // Not provisioned: fall back quietly, this is the default state. + return 0, false, nil + } + // The name resolves to something that is not a usable principal, most + // likely squatted by a local group or alias. That is a conflict an + // operator has to see, not a reason to pretend setup never ran. + return 0, false, err + } + secretPath, err := windowsSandboxSecretPath(config.SandboxHome, identity.Username) + if err != nil { + return 0, false, err + } + password, err := readWindowsSandboxSecret(secretPath) + if err != nil { + if errors.Is(err, errWindowsSandboxIdentityUnavailable) { + // The account exists but its password does not. Setup was interrupted + // or the secret was removed; fall back rather than fail the command. + // + // Falling back is right, staying quiet about it was not. The opt-in is + // set and an account IS provisioned, so the operator asked for + // principal isolation and is silently getting the weaker same-user + // restricted token instead. That is the one fail-soft case worth + // announcing: the others mean the backend was never set up, while this + // one means it was and has broken since. + warnWindowsSandboxPrincipalUnavailable(identity.Username) + return 0, false, nil + } + return 0, false, err + } + token, err := logonWindowsSandboxPrincipal(identity.Username, password) + if err != nil { + // Provisioned but unusable. Surface it: a wrong password or a revoked + // batch-logon right is a broken sandbox, not an absent one. + return 0, false, err + } + return token, true, nil +} + +// warnWindowsSandboxPrincipalUnavailable tells the operator once per process +// that the backend they opted into is not the one running. +// +// Once, because this sits on the command path: a warning per command would be +// noise on every tool call for the whole session, and noise that repeats gets +// filtered out by the reader rather than acted on. Indirected through a var so a +// test can observe it without capturing stderr. +var warnWindowsSandboxPrincipalUnavailable = func(username string) { + windowsSandboxPrincipalWarnOnce.Do(func() { + fmt.Fprintf(os.Stderr, + "[zero] %s is set and sandbox principal %q is provisioned, but its stored password is missing or unreadable. "+ + "Falling back to the restricted-token sandbox, which does not confine reads. "+ + "Re-run `zero sandbox setup` from an elevated terminal to restore it.\n", + windowsSandboxIdentityEnv, username) + }) +} + +var windowsSandboxPrincipalWarnOnce sync.Once + +// provisionWindowsSandboxPrincipalForSetup does the elevated half: create the +// account, grant it the batch logon right, and store its password locked to the +// invoking user. Called from `zero sandbox setup`. +// +// The password is written BEFORE the caller applies any ACL plan, so a setup +// that fails partway leaves a principal that can at least be logged on and +// therefore cleaned up, rather than an account nothing holds the secret for. +func provisionWindowsSandboxPrincipalForSetup(config WindowsSandboxCommandConfig) (windowsSandboxIdentity, bool, error) { + key := windowsSandboxWorkspaceKey(config.WorkspaceRoots) + identity, password, created, err := provisionWindowsSandboxIdentity(key) + + // Undo whatever this run actually did, in reverse, on any failure after the + // account exists. Without it a failure between creating the account and + // storing its secret left the account behind with no caller able to remove + // it: the rollback the setup path installs is only built once this function + // has returned successfully. + // + // Scoped to what THIS run created on purpose. An account that already existed + // and belongs to Zero is a working principal from an earlier setup, and + // deleting it because a later run failed would turn a partial failure into a + // total one. + rightsAttempted := false + rotated := false + // Resolved from the account name rather than the identity, so it is known + // before anything can fail. + secretPath, secretPathErr := windowsSandboxSecretPath(config.SandboxHome, windowsSandboxUserName(key)) + undo := func() { + // Only when this run invalidated it. The secret is removed if this run + // created the account, or if it rotated an existing account's password, + // because in both cases what is on disk cannot authenticate and absent + // beats stale: the command path treats a missing secret as "not + // provisioned" and falls back, while a stale one fails the logon and + // reports a broken sandbox. + // + // Removing it unconditionally, as this used to, destroyed a WORKING + // secret whenever setup failed before rotation on a machine that was + // already provisioned. The account kept its old password, the only copy + // of it was deleted, and the sandbox silently degraded. + if secretPath != "" && (created || rotated) { + _ = removeWindowsSandboxSecret(secretPath) + } + // Only for an account this run created, and attempted rather than + // completed. + // + // Attempted, because grantWindowsSandboxLogonRights adds rights one at a + // time and returns on the first failure, so a partial grant is possible + // and gating on success left those entries behind, keyed to a SID that + // deleting the account then made unresolvable. + // + // Created, because revokeWindowsSandboxLogonRights passes AllRights, which + // drops every right the account holds and deletes its LSA object outright. + // On an adopted principal that is not a rollback, it is destruction: a + // transient failure anywhere below would strip the SeBatchLogonRight and + // deny-logon rights a previous setup established, leaving exactly the + // broken-but-present principal this whole function exists to avoid. The + // rights this run granted are the ones the account is supposed to have, so + // leaving them in place on an adopted account is the safe direction. + if identity.SID != nil && rightsAttempted && created { + _ = revokeWindowsSandboxLogonRightsFn(identity.SID) + } + if created { + _ = removeWindowsSandboxIdentity(identity.Username) + } + } + + if err != nil { + // provisionWindowsSandboxIdentity can fail after creating the account, so + // this path needs the same cleanup even though nothing below ran. + undo() + return windowsSandboxIdentity{}, false, err + } + rightsAttempted = true + if err := grantWindowsSandboxLogonRightsFn(identity.SID); err != nil { + undo() + return windowsSandboxIdentity{}, false, err + } + if secretPathErr != nil { + undo() + return windowsSandboxIdentity{}, false, secretPathErr + } + // Rotation happens HERE, immediately before the secret is committed, rather + // than inside provisioning where it used to. + // + // A new account already has this password from NetUserAdd, so only an + // adopted one needs setting. Doing it at the top of provisioning meant every + // step in between ran with the account's password already replaced and no + // copy of it stored, so any failure there stranded a working principal. The + // two operations are now adjacent, which is the smallest window this can + // have without a way to restore the previous password, which Windows does + // not offer. + if !created { + if err := resetWindowsSandboxUserPasswordFn(identity.Username, password); err != nil { + undo() + return windowsSandboxIdentity{}, false, err + } + rotated = true + } + if err := writeWindowsSandboxSecret(secretPath, password); err != nil { + undo() + return windowsSandboxIdentity{}, false, err + } + return identity, created, nil +} + +// setupWindowsSandboxPrincipal provisions this workspace's principal and grants +// it the filesystem access its permission profile describes. It returns a +// rollback that undoes everything it created, so a later setup step failing does +// not leave a half-provisioned account behind. +// +// Rollback order is the inverse of creation and matters: ACEs are revoked BEFORE +// the account is deleted, because removing the account first would leave ACEs +// naming a SID that no longer resolves, which is the orphaned-entry residue this +// model exists to avoid. +func setupWindowsSandboxPrincipal(config WindowsSandboxCommandConfig) (func() error, error) { + identity, created, err := provisionWindowsSandboxPrincipalForSetup(config) + if err != nil { + return nil, err + } + // Scoped to what this run created, the same contract provisioning already + // applies to its own rollback. + // + // Unconditional removal here meant a transient ACL failure during a re-run of + // elevated setup deleted a principal that was working before the run started, + // taking its secret and logon rights with it. Provisioning was careful not to + // do that and then this undid the care one level up. A pre-existing principal + // is left alone: its ACEs are still reverted, since this run applied them, + // but the account itself is not this run's to destroy. + removePrincipal := func() error { + if !created { + return nil + } + return removeWindowsSandboxPrincipalForSetup(config) + } + + filesystem := config.PermissionProfile.FileSystem + writeRoots := filesystem.WriteRoots + // The runtime tree has to be granted here, at setup, because nothing grants it + // later. + // + // permissionProfileWithRuntime appends the per-workspace runtime root to + // WriteRoots on every COMMAND, and redirects HOME, GOCACHE, npm_config_cache + // and friends into it. That root lives under the user cache, not the + // workspace, so the profile setup sees never contains it. On the + // restricted-token path that costs nothing, since the child still runs as the + // caller and already has rights there. A principal is a separate local account + // with none, so without this every npm install, go build or pip install fails + // on a cache write with a bare ACCESS_DENIED and nothing pointing at the + // sandbox as the cause. + if runtimeRoot, err := setupWindowsSandboxRuntimeRoot(config); err != nil { + _ = removePrincipal() + return nil, err + } else if runtimeRoot != "" { + writeRoots = append(append([]WritableRoot{}, writeRoots...), WritableRoot{Root: runtimeRoot}) + } + plan, err := buildWindowsPrincipalACLPlan(windowsPrincipalACLInput{ + PrincipalSID: identity.SID.String(), + WriteRoots: writeRoots, + ReadRoots: filesystem.ReadRoots, + DenyRead: filesystem.DenyRead, + DenyWrite: filesystem.DenyWrite, + }) + if err != nil { + _ = removePrincipal() + return nil, err + } + revertACL, err := applyWindowsACLPlan(plan) + if err != nil { + _ = removePrincipal() + return nil, err + } + return func() error { + aclErr := revertACL() + // Remove the principal even when the ACL revert failed, so a broken + // rollback does not also strand an account; report the ACL error since it + // is the one that leaves state behind. + removeErr := removePrincipal() + if aclErr != nil { + return aclErr + } + return removeErr + }, nil +} + +// removeWindowsSandboxPrincipalForSetup retires a workspace's principal: secret +// first, then the account. ACE revocation is the caller's job and must happen +// before this, or ACEs naming a deleted SID are left behind. +func removeWindowsSandboxPrincipalForSetup(config WindowsSandboxCommandConfig) error { + key := windowsSandboxWorkspaceKey(config.WorkspaceRoots) + username := windowsSandboxUserName(key) + secretPath, err := windowsSandboxSecretPath(config.SandboxHome, username) + if err != nil { + return err + } + if err := removeWindowsSandboxSecret(secretPath); err != nil { + return err + } + // Drop the LSA account rights before the account itself. Deleting the account + // first would leave its rights behind keyed to a SID that no longer resolves, + // which is the same orphaned residue the trustee-keyed ACE revocation exists + // to avoid. A principal that was never provisioned has no SID to resolve and + // nothing to revoke, so that case is not an error. + if identity, err := lookupWindowsSandboxIdentity(windowsSandboxWorkspaceKey(config.WorkspaceRoots)); err == nil { + if err := revokeWindowsSandboxLogonRights(identity.SID); err != nil { + return err + } + } else if !errors.Is(err, errWindowsSandboxIdentityUnavailable) { + return err + } + return removeWindowsSandboxIdentity(username) +} + +// setupWindowsSandboxRuntimeRoot resolves this workspace's runtime root and +// makes sure it exists, so the principal ACL plan can name it. +// +// It is created here rather than left to the first command because +// applyWindowsACLPlan skips a target that does not exist: granting write on a +// directory that setup never made would silently no-op, and the failure would +// only show up later as a denied cache write. Creating it under the elevated +// setup process is safe, since it lives under the invoking user's own cache +// directory and prepareSandboxRuntime would create it on the same path anyway. +// +// An empty return means there is no runtime root to grant (no workspace root +// configured), which is not an error: the caller simply grants nothing extra. +func setupWindowsSandboxRuntimeRoot(config WindowsSandboxCommandConfig) (string, error) { + workspaceRoot := "" + for _, candidate := range config.WorkspaceRoots { + if trimmed := strings.TrimSpace(candidate); trimmed != "" { + workspaceRoot = filepath.Clean(trimmed) + break + } + } + if workspaceRoot == "" { + return "", nil + } + cacheRoot, err := sandboxUserCacheDir() + if err != nil { + return "", fmt.Errorf("resolve user cache directory for sandbox runtime: %w", err) + } + cacheRoot = filepath.Clean(strings.TrimSpace(cacheRoot)) + if cacheRoot == "" || cacheRoot == "." { + return "", errors.New("user cache directory is unavailable for sandbox runtime") + } + root, err := sandboxRuntimeRootFor(workspaceRoot, cacheRoot) + if err != nil { + return "", err + } + if err := os.MkdirAll(root, 0o700); err != nil { + return "", fmt.Errorf("create sandbox runtime root: %w", err) + } + return root, nil +} diff --git a/internal/sandbox/windows_identity_runtime_windows_test.go b/internal/sandbox/windows_identity_runtime_windows_test.go new file mode 100644 index 000000000..d8658a144 --- /dev/null +++ b/internal/sandbox/windows_identity_runtime_windows_test.go @@ -0,0 +1,95 @@ +//go:build windows + +package sandbox + +import "testing" + +// Setup must stay inert unless the principal backend is explicitly opted into. +// This is the property that makes the branch safe to merge while the privileged +// paths are still being validated: without the opt-in, `zero sandbox setup` +// creates no local account and the capability-SID backend is the whole of setup. +func TestWindowsSandboxIdentityGating(t *testing.T) { + for name, testCase := range map[string]struct { + env map[string]string + want bool + }{ + "absent": {env: map[string]string{}, want: false}, + "empty": {env: map[string]string{windowsSandboxIdentityEnv: ""}, want: false}, + "zero": {env: map[string]string{windowsSandboxIdentityEnv: "0"}, want: false}, + "true not one": {env: map[string]string{windowsSandboxIdentityEnv: "true"}, want: false}, + "one": {env: map[string]string{windowsSandboxIdentityEnv: "1"}, want: true}, + "one with space": {env: map[string]string{windowsSandboxIdentityEnv: " 1 "}, want: true}, + } { + t.Run(name, func(t *testing.T) { + if got := windowsSandboxIdentityEnabled(testCase.env); got != testCase.want { + t.Fatalf("enabled = %v, want %v for %q", got, testCase.want, testCase.env[windowsSandboxIdentityEnv]) + } + }) + } +} + +// The command environment wins over the process environment, so a run can opt in +// or out without depending on how the parent shell was launched. +func TestWindowsSandboxIdentityEnvOverridesProcess(t *testing.T) { + t.Setenv(windowsSandboxIdentityEnv, "1") + if windowsSandboxIdentityEnabled(map[string]string{windowsSandboxIdentityEnv: "0"}) { + t.Fatal("command env set to 0 must override a process env of 1") + } + if !windowsSandboxIdentityEnabled(map[string]string{}) { + t.Fatal("with no command-env entry the process env should apply") + } +} + +// One workspace maps to one principal, and different workspaces must not share +// an account, or two projects would run under the same identity and could reach +// each other's granted roots. +func TestWindowsSandboxWorkspaceKeyIsStableAndDistinct(t *testing.T) { + first := windowsSandboxWorkspaceKey([]string{`C:\ws\alpha`}) + again := windowsSandboxWorkspaceKey([]string{`C:\ws\alpha`}) + other := windowsSandboxWorkspaceKey([]string{`C:\ws\beta`}) + if first != again { + t.Fatalf("key is not stable: %q vs %q", first, again) + } + if first == other { + t.Fatal("two different workspaces produced the same principal key") + } + if first == "" { + t.Fatal("empty key") + } + // An empty root list still has to yield a usable key rather than a blank one. + if windowsSandboxWorkspaceKey(nil) == "" { + t.Fatal("no workspace roots produced an empty key") + } +} + +// Network denial is enforced by WFP filters keyed to the offline-marker SID, +// which only the restricted token carries. A principal token would leave those +// filters matching nothing, so the principal backend must stand down whenever +// the network is denied rather than silently trading network enforcement for +// read confinement. +// The eligibility predicate is asserted rather than the token lookup, because on +// a machine with no principal provisioned the lookup declines for its own reasons +// and would report success here whether or not the guard existed. +func TestPrincipalBackendDefersToRestrictedTokenWhenNetworkDenied(t *testing.T) { + eligible := func(mode NetworkMode, optIn string) bool { + config := WindowsSandboxCommandConfig{ + SandboxHome: t.TempDir(), + WorkspaceRoots: []string{t.TempDir()}, + Env: map[string]string{windowsSandboxIdentityEnv: optIn}, + } + config.PermissionProfile.Network.Mode = mode + return windowsSandboxPrincipalEligible(config) + } + + if eligible(NetworkDeny, "1") { + t.Fatal("principal backend eligible with the network denied; the WFP filters key on the offline-marker SID, which a logon token does not carry, so egress would be unenforced") + } + // The guard must be specific to denial, not a blanket disable that would make + // the whole backend dead code. + if !eligible(NetworkAllow, "1") { + t.Fatal("principal backend refused with the network allowed; the guard is over-broad and disables the backend entirely") + } + if eligible(NetworkAllow, "0") { + t.Fatal("principal backend eligible without the opt-in") + } +} diff --git a/internal/sandbox/windows_identity_secret_windows.go b/internal/sandbox/windows_identity_secret_windows.go new file mode 100644 index 000000000..87463940b --- /dev/null +++ b/internal/sandbox/windows_identity_secret_windows.go @@ -0,0 +1,237 @@ +//go:build windows + +package sandbox + +// Storing a sandbox principal's password. +// +// The elevated setup path provisions the account, but the per-command path runs +// UNELEVATED and needs the password to call LogonUser. So the secret has to +// cross that boundary on disk, and the only thing standing between it and the +// sandboxed child is the file's ACL. +// +// The file is locked to the invoking user: an explicit, INHERITANCE-PROTECTED +// DACL granting that user and SYSTEM, and nobody else. The sandbox principal is +// deliberately absent from it, which is the property that matters, because a +// principal that could read this file could mint its own token and the whole +// identity boundary would be decorative. Administrators are not added either; +// an admin can already take ownership, so naming them buys nothing and widens +// the visible grant. +// +// Ordering is load-bearing: the ACL is applied to an EMPTY file before the +// password is written, so the bytes never exist under the directory's inherited +// permissions even briefly. + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/windows" +) + +// windowsSandboxSecretDirName holds per-principal secrets under the Zero config +// directory. Kept in its own directory so the whole set can be removed when the +// sandbox is torn down. +const windowsSandboxSecretDirName = "windows-sandbox" + +// windowsSandboxSecretPath returns where a principal's password lives. The +// account name is already sanitised to [a-z0-9-] by windowsSandboxUserName, so +// it cannot escape the directory. +func windowsSandboxSecretPath(configDir string, username string) (string, error) { + if strings.TrimSpace(configDir) == "" { + return "", errors.New("windows sandbox secret: empty config directory") + } + if strings.TrimSpace(username) == "" { + return "", errors.New("windows sandbox secret: empty principal name") + } + // Defence in depth against a caller passing something windowsSandboxUserName + // did not produce: refuse anything with a separator or a parent reference. + if strings.ContainsAny(username, `\/:`) || strings.Contains(username, "..") { + return "", fmt.Errorf("windows sandbox secret: unsafe principal name %q", username) + } + return filepath.Join(configDir, windowsSandboxSecretDirName, username+".secret"), nil +} + +// currentTokenUserSID returns the SID of the user this process runs as. Under +// UAC the elevated token keeps the same user SID as the desktop session, so +// setup and the later unelevated command path agree on the owner, which is what +// makes an owner-scoped ACL usable across the elevation boundary. +func currentTokenUserSID() (*windows.SID, error) { + var token windows.Token + if err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY, &token); err != nil { + return nil, fmt.Errorf("open process token: %w", err) + } + defer token.Close() + user, err := token.GetTokenUser() + if err != nil { + return nil, fmt.Errorf("get token user: %w", err) + } + // The SID points into a buffer owned by the Tokenuser, so copy it out before + // that buffer goes away. + copied, err := user.User.Sid.Copy() + if err != nil { + return nil, fmt.Errorf("copy token user SID: %w", err) + } + return copied, nil +} + +// lockWindowsSecretToOwner replaces a file's DACL with an explicit, +// inheritance-protected one granting only owner and SYSTEM. PROTECTED is what +// drops any ACE inherited from the config directory; without it a permissive +// parent would still grant access to whoever it names. +func lockWindowsSecretToOwner(path string, owner *windows.SID) error { + if owner == nil { + return errors.New("windows sandbox secret: nil owner SID") + } + system, err := windows.CreateWellKnownSid(windows.WinLocalSystemSid) + if err != nil { + return fmt.Errorf("resolve SYSTEM SID: %w", err) + } + entries := []windows.EXPLICIT_ACCESS{ + { + AccessPermissions: windows.GENERIC_ALL, + AccessMode: windows.GRANT_ACCESS, + Inheritance: windows.NO_INHERITANCE, + Trustee: windows.TRUSTEE{ + TrusteeForm: windows.TRUSTEE_IS_SID, + TrusteeType: windows.TRUSTEE_IS_USER, + TrusteeValue: windows.TrusteeValueFromSID(owner), + }, + }, + { + AccessPermissions: windows.GENERIC_ALL, + AccessMode: windows.GRANT_ACCESS, + Inheritance: windows.NO_INHERITANCE, + Trustee: windows.TRUSTEE{ + TrusteeForm: windows.TRUSTEE_IS_SID, + TrusteeType: windows.TRUSTEE_IS_USER, + TrusteeValue: windows.TrusteeValueFromSID(system), + }, + }, + } + acl, err := windows.ACLFromEntries(entries, nil) + if err != nil { + return fmt.Errorf("build secret ACL: %w", err) + } + if err := windows.SetNamedSecurityInfo( + path, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, + nil, + acl, + nil, + ); err != nil { + return fmt.Errorf("lock secret to owner: %w", err) + } + return nil +} + +// writeWindowsSandboxSecret stores a principal's password readable only by the +// invoking user. +// +// The file is created empty, locked down, and only then written, so the secret +// is never on disk under the directory's inherited ACL. An existing file is +// replaced rather than appended, since a stale password would make LogonUser +// fail in a way that looks like a sandbox bug. +func writeWindowsSandboxSecret(path string, password string) error { + owner, err := currentTokenUserSID() + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return fmt.Errorf("create secret directory: %w", err) + } + // Truncate any previous secret first: the ACL below is applied to whatever + // inode ends up at this path, so create it before locking it. + file, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600) + if err != nil { + return fmt.Errorf("create secret file: %w", err) + } + if err := file.Close(); err != nil { + return fmt.Errorf("close secret file: %w", err) + } + if err := lockWindowsSecretToOwner(path, owner); err != nil { + // Do not leave an unprotected empty file behind. + _ = os.Remove(path) + return err + } + // Encrypt to the invoking user on top of the ACL, so a copy taken outside the + // filesystem's enforcement (backup, disk image) is inert. The principal name is + // the entropy, which keeps one principal's blob from authenticating another. + sealed, err := protectWindowsSecret(password, windowsSandboxSecretEntropy(path)) + if err != nil { + _ = os.Remove(path) + return err + } + if err := os.WriteFile(path, sealed, 0o600); err != nil { + _ = os.Remove(path) + return fmt.Errorf("write secret: %w", err) + } + return nil +} + +// windowsSandboxSecretEntropy derives the DPAPI entropy from the secret's own +// filename, which is the principal name. Deriving it rather than threading the +// name through keeps read and write agreeing by construction. +func windowsSandboxSecretEntropy(path string) string { + return strings.TrimSuffix(filepath.Base(path), ".secret") +} + +// Seamed so the permission-denied mapping in readWindowsSandboxSecret is +// testable. Producing a real ERROR_ACCESS_DENIED needs DACL surgery on Windows, +// since a 0000 file is still readable and reading a directory reports +// "Incorrect function", so a test built that way would exercise the platform +// rather than the mapping. +var readWindowsSandboxSecretFile = os.ReadFile + +// readWindowsSandboxSecret loads a principal's password. A missing file means +// setup has not run for this workspace, which the caller turns into a fallback +// rather than a hard failure. +func readWindowsSandboxSecret(path string) (string, error) { + data, err := readWindowsSandboxSecretFile(path) + if err != nil { + if os.IsNotExist(err) { + return "", errWindowsSandboxIdentityUnavailable + } + // Permission denied is unavailability, not breakage. The secret's DACL + // names whoever ran setup, so an operator who elevated with a separate + // administrative account, through runas or an over-the-shoulder UAC + // prompt, ends up with a secret their ordinary account cannot open. That + // is the documented fail-soft case: fall back to the restricted token and + // let the warning say so. Treating it as a hard error instead made every + // sandboxed command fail on a machine that was merely set up by a + // different admin, which is a common way to run an elevated setup. + if os.IsPermission(err) { + return "", errWindowsSandboxIdentityUnavailable + } + return "", fmt.Errorf("read sandbox secret: %w", err) + } + if len(data) == 0 { + return "", errWindowsSandboxIdentityUnavailable + } + secret, err := unprotectWindowsSecret(data, windowsSandboxSecretEntropy(path)) + if err != nil { + // A blob written by another user, for another principal, or by an older + // build that stored the password in the clear. Report it as unavailable so + // the caller falls back to the restricted token; the next elevated setup + // rewrites the secret in the current format. + return "", errWindowsSandboxIdentityUnavailable + } + if strings.TrimSpace(secret) == "" { + return "", errWindowsSandboxIdentityUnavailable + } + return secret, nil +} + +// removeWindowsSandboxSecret deletes a stored password. Called before the +// account itself is removed so a secret never outlives the principal it +// authenticates. +func removeWindowsSandboxSecret(path string) error { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove sandbox secret: %w", err) + } + return nil +} diff --git a/internal/sandbox/windows_identity_secret_windows_test.go b/internal/sandbox/windows_identity_secret_windows_test.go new file mode 100644 index 000000000..0293ec573 --- /dev/null +++ b/internal/sandbox/windows_identity_secret_windows_test.go @@ -0,0 +1,300 @@ +//go:build windows + +package sandbox + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "unsafe" + + "golang.org/x/sys/windows" +) + +func TestWindowsSandboxSecretRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "cfg", "zero-sbx-test.secret") + const password = "Zs1!EXAMPLEPASSWORDVALUE" + + if err := writeWindowsSandboxSecret(path, password); err != nil { + t.Fatalf("write: %v", err) + } + got, err := readWindowsSandboxSecret(path) + if err != nil { + t.Fatalf("read: %v", err) + } + if got != password { + t.Fatalf("read %q, want the stored password", got) + } +} + +// THE security property: the stored password must be readable only by the user +// who owns it. If any other trustee appears in the DACL, and in particular the +// sandbox principal, that account could mint its own token and the identity +// boundary would be worthless. +func TestWindowsSandboxSecretIsLockedToOwner(t *testing.T) { + path := filepath.Join(t.TempDir(), "locked.secret") + if err := writeWindowsSandboxSecret(path, "Zs1!SECRET"); err != nil { + t.Fatalf("write: %v", err) + } + + descriptor, err := windows.GetNamedSecurityInfo( + path, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION, + ) + if err != nil { + t.Fatalf("read back security info: %v", err) + } + dacl, _, err := descriptor.DACL() + if err != nil { + t.Fatalf("read DACL: %v", err) + } + if dacl == nil { + t.Fatal("secret has a nil DACL, which grants everyone access") + } + + owner, err := currentTokenUserSID() + if err != nil { + t.Fatalf("owner SID: %v", err) + } + system, err := windows.CreateWellKnownSid(windows.WinLocalSystemSid) + if err != nil { + t.Fatalf("SYSTEM SID: %v", err) + } + + entries, err := windowsSecretACEList(dacl) + if err != nil { + t.Fatalf("enumerate ACEs: %v", err) + } + if len(entries) == 0 { + t.Fatal("secret DACL has no ACEs") + } + for _, sid := range entries { + if sid.Equals(owner) || sid.Equals(system) { + continue + } + t.Fatalf("secret DACL grants an unexpected trustee %s; only the owner and SYSTEM may appear", sid) + } +} + +// The DACL must be inheritance-protected, otherwise a permissive ACE on the +// config directory would still reach the secret. +func TestWindowsSandboxSecretDaclIsProtected(t *testing.T) { + path := filepath.Join(t.TempDir(), "protected.secret") + if err := writeWindowsSandboxSecret(path, "Zs1!SECRET"); err != nil { + t.Fatalf("write: %v", err) + } + descriptor, err := windows.GetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) + if err != nil { + t.Fatalf("read back security info: %v", err) + } + control, _, err := descriptor.Control() + if err != nil { + t.Fatalf("read control bits: %v", err) + } + if control&windows.SE_DACL_PROTECTED == 0 { + t.Fatal("secret DACL is not protected, so inherited ACEs still apply") + } +} + +// Rewriting must replace the previous secret rather than append to it, or +// LogonUser would be handed two concatenated passwords. +func TestWindowsSandboxSecretOverwrites(t *testing.T) { + path := filepath.Join(t.TempDir(), "rewrite.secret") + if err := writeWindowsSandboxSecret(path, "Zs1!FIRST"); err != nil { + t.Fatalf("first write: %v", err) + } + if err := writeWindowsSandboxSecret(path, "Zs1!SECOND"); err != nil { + t.Fatalf("second write: %v", err) + } + got, err := readWindowsSandboxSecret(path) + if err != nil { + t.Fatalf("read: %v", err) + } + if got != "Zs1!SECOND" { + t.Fatalf("read %q, want only the newest password", got) + } +} + +// A workspace whose setup has not run must report the actionable sentinel so the +// command path falls back to the restricted-token backend instead of failing. +func TestWindowsSandboxSecretMissingIsSentinel(t *testing.T) { + path := filepath.Join(t.TempDir(), "absent.secret") + if _, err := readWindowsSandboxSecret(path); err != errWindowsSandboxIdentityUnavailable { + t.Fatalf("missing secret returned %v, want errWindowsSandboxIdentityUnavailable", err) + } +} + +// An empty file is a half-written secret, not a valid empty password. +// +// Both seeds matter and only one of them tests what the name says. A +// zero-length file is the truncated-write case, and it is the only one that +// reaches the length check. Whitespace is several bytes, so it travels on to +// DPAPI and fails to unprotect instead, which is the path +// TestWindowsSandboxSecretRejectsLegacyPlaintext already covers. Seeding only +// the whitespace, as this did, left the branch in the test's own name +// unexercised. +func TestWindowsSandboxSecretEmptyIsSentinel(t *testing.T) { + for name, seed := range map[string][]byte{ + "truncated write": {}, + "whitespace only": []byte(" \r\n"), + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "empty.secret") + if err := os.WriteFile(path, seed, 0o600); err != nil { + t.Fatalf("seed: %v", err) + } + if _, err := readWindowsSandboxSecret(path); err != errWindowsSandboxIdentityUnavailable { + t.Fatalf("secret of %d bytes returned %v, want the unavailable sentinel", len(seed), err) + } + }) + } +} + +// The principal name lands in a filename, so anything that could escape the +// directory has to be refused even though windowsSandboxUserName already +// sanitises its output. +func TestWindowsSandboxSecretPathRejectsTraversal(t *testing.T) { + for _, name := range []string{`..\evil`, "sub/dir", `C:\abs`, "..", ""} { + if _, err := windowsSandboxSecretPath(`C:\cfg`, name); err == nil { + t.Fatalf("principal name %q was accepted", name) + } + } + if _, err := windowsSandboxSecretPath("", "zero-sbx-a"); err == nil { + t.Fatal("empty config directory was accepted") + } + path, err := windowsSandboxSecretPath(`C:\cfg`, "zero-sbx-abc") + if err != nil { + t.Fatalf("valid name rejected: %v", err) + } + if !strings.HasSuffix(path, `zero-sbx-abc.secret`) { + t.Fatalf("unexpected secret path %q", path) + } +} + +// Removal must be idempotent so teardown converges the same way provisioning +// does, and must actually delete the secret. +func TestWindowsSandboxSecretRemoveIsIdempotent(t *testing.T) { + path := filepath.Join(t.TempDir(), "gone.secret") + if err := writeWindowsSandboxSecret(path, "Zs1!SECRET"); err != nil { + t.Fatalf("write: %v", err) + } + if err := removeWindowsSandboxSecret(path); err != nil { + t.Fatalf("first remove: %v", err) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("secret still present after removal (stat err %v)", err) + } + if err := removeWindowsSandboxSecret(path); err != nil { + t.Fatalf("removing an absent secret must succeed, got %v", err) + } +} + +// windowsSecretACEList returns the trustee SID of every ACE in a DACL so a test +// can assert exactly who is named. +func windowsSecretACEList(dacl *windows.ACL) ([]*windows.SID, error) { + var out []*windows.SID + for index := uint32(0); index < uint32(dacl.AceCount); index++ { + var ace *windows.ACCESS_ALLOWED_ACE + if err := windows.GetAce(dacl, index, &ace); err != nil { + return nil, err + } + // GetAce hands back a generic ACE_HEADER and we reinterpret it. That is + // only sound for the fixed-layout types: an object ACE carries Flags and + // two GUIDs ahead of the trustee, so SidStart would land mid-structure + // and Copy would read whatever bytes happen to be there. The caller's + // "unexpected trustee" assertion would then print a nonsense SID instead + // of naming the ACE that does not belong, which is the opposite of what + // a failing test should do. Nothing under test builds anything but + // allowed ACEs today, so this exists to keep the failure legible if that + // ever changes. + if ace.Header.AceType != windows.ACCESS_ALLOWED_ACE_TYPE { + return nil, fmt.Errorf("ACE %d has type %d, want ACCESS_ALLOWED_ACE_TYPE (%d); refusing to decode its trustee", index, ace.Header.AceType, windows.ACCESS_ALLOWED_ACE_TYPE) + } + sid := (*windows.SID)(unsafe.Pointer(&ace.SidStart)) + copied, err := sid.Copy() + if err != nil { + return nil, err + } + out = append(out, copied) + } + return out, nil +} + +// DPAPI round-trip through the real store, which needs no privilege and so is +// genuine coverage rather than a gated stub. +func TestWindowsSandboxSecretRoundTripsThroughDPAPI(t *testing.T) { + path, err := windowsSandboxSecretPath(t.TempDir(), "zero-sbx-roundtrip") + if err != nil { + t.Fatalf("secret path: %v", err) + } + const password = "S0me-Sandbox-P@ssw0rd-value" + if err := writeWindowsSandboxSecret(path, password); err != nil { + t.Fatalf("write secret: %v", err) + } + got, err := readWindowsSandboxSecret(path) + if err != nil { + t.Fatalf("read secret: %v", err) + } + if got != password { + t.Fatalf("round-trip returned %q, want %q", got, password) + } + // The point of the exercise: the password must not be recoverable by reading + // the file, or the encryption layer is decorative. + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read raw secret file: %v", err) + } + if bytes.Contains(raw, []byte(password)) { + t.Fatal("the password appears verbatim in the stored file; it was not encrypted") + } +} + +// Entropy is the principal name, so a blob moved onto another principal's secret +// path must fail to decrypt rather than authenticate the wrong account. +func TestWindowsSandboxSecretDoesNotTransferBetweenPrincipals(t *testing.T) { + home := t.TempDir() + minePath, err := windowsSandboxSecretPath(home, "zero-sbx-mine") + if err != nil { + t.Fatalf("secret path: %v", err) + } + theirsPath, err := windowsSandboxSecretPath(home, "zero-sbx-theirs") + if err != nil { + t.Fatalf("secret path: %v", err) + } + if err := writeWindowsSandboxSecret(minePath, "a-password-for-mine"); err != nil { + t.Fatalf("write secret: %v", err) + } + blob, err := os.ReadFile(minePath) + if err != nil { + t.Fatalf("read blob: %v", err) + } + if err := os.WriteFile(theirsPath, blob, 0o600); err != nil { + t.Fatalf("plant blob: %v", err) + } + if _, err := readWindowsSandboxSecret(theirsPath); !errors.Is(err, errWindowsSandboxIdentityUnavailable) { + t.Fatalf("a blob planted at another principal's path decrypted; got err = %v", err) + } +} + +// An older plaintext secret must degrade to a fallback rather than being handed +// to LogonUser as if it were a password. +func TestWindowsSandboxSecretRejectsLegacyPlaintext(t *testing.T) { + path, err := windowsSandboxSecretPath(t.TempDir(), "zero-sbx-legacy") + if err != nil { + t.Fatalf("secret path: %v", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(path, []byte("plaintext-password"), 0o600); err != nil { + t.Fatalf("write legacy secret: %v", err) + } + if _, err := readWindowsSandboxSecret(path); !errors.Is(err, errWindowsSandboxIdentityUnavailable) { + t.Fatalf("legacy plaintext secret was accepted; got err = %v", err) + } +} diff --git a/internal/sandbox/windows_identity_windows.go b/internal/sandbox/windows_identity_windows.go new file mode 100644 index 000000000..819ef94f3 --- /dev/null +++ b/internal/sandbox/windows_identity_windows.go @@ -0,0 +1,682 @@ +//go:build windows + +package sandbox + +// Windows sandbox principals. +// +// Every other Windows backend here derives its token from the CALLING user via +// CreateRestrictedToken, which is why the sandbox can constrain writes but not +// reads: a deny ACE that would stop the sandboxed child reading a credential +// store names the same account Zero itself runs as, so it would lock Zero out +// too. Reads therefore stay on the caller's identity and +// credentialDenyReadPaths is a no-op on Windows (#662, #675). +// +// This file provisions a SEPARATE local account per workspace, held in one +// managed local group, so the sandbox has an identity of its own. A deny-read +// ACE naming that principal denies the sandboxed child and nothing else, and +// the same SID is what a firewall rule or a write grant can be keyed to. The +// accounts are created by the elevated `zero sandbox setup` path because +// NetUserAdd requires administrator rights; nothing here runs unelevated. +// +// Provisioning is idempotent: the "already exists" status from each API is a +// success, so setup can be re-run safely and a partially provisioned machine +// converges. + +import ( + "crypto/rand" + "encoding/base32" + "errors" + "fmt" + "runtime" + "strings" + "unsafe" + + "golang.org/x/sys/windows" +) + +const ( + // windowsSandboxGroupName holds every sandbox principal. Grouping them means + // an ACE can name the group once instead of enumerating accounts, and it + // gives setup a single place to find what it previously created. + windowsSandboxGroupName = "ZeroSandboxUsers" + windowsSandboxGroupComment = "Zero sandbox principals (managed by zero sandbox setup)" + + // windowsSandboxUserPrefix keeps the accounts recognisable in `net user` and + // lets cleanup identify what belongs to Zero. Windows caps a local account + // name at 20 characters, which windowsSandboxUserName respects. + windowsSandboxUserPrefix = "zero-sbx-" + // The comment doubles as the ownership marker AND records which workspace + // the account belongs to. The account NAME can only carry 11 characters of + // the workspace digest because of the 20-character local-account limit, so + // two workspaces whose digests share that prefix derive the same name. The + // full key here turns that from a silent share of one account, one secret + // and one ACL identity into a refusal. + windowsSandboxUserComment = "Zero sandbox principal (managed)" + windowsSandboxUserCommentKey = windowsSandboxUserComment + " key=" + windowsSandboxUserNameMax = 20 +) + +// Win32 status codes that mean "already there". Treated as success so +// provisioning converges instead of failing on a second run. +const ( + nerrSuccess = 0 + nerrGroupExists = 2223 + nerrUserExists = 2224 + errorAliasExists = 1379 + errorMemberInAlias = 1378 + errorAccessDenied32 = 5 + nerrUserNotFound = 2221 +) + +// USER_INFO_1 privilege and flag values. +const ( + usrPrivUser = 1 + ufScript = 0x0001 + ufNormalAccount = 0x0200 + ufDontExpirePasswd = 0x10000 + windowsPasswordLength = 24 +) + +var ( + netapi32 = windows.NewLazySystemDLL("netapi32.dll") + procNetUserAdd = netapi32.NewProc("NetUserAdd") + procNetLocalGroupAdd = netapi32.NewProc("NetLocalGroupAdd") + procNetLocalGroupAddMembers = netapi32.NewProc("NetLocalGroupAddMembers") + procNetUserDel = netapi32.NewProc("NetUserDel") + procNetUserSetInfo = netapi32.NewProc("NetUserSetInfo") + procNetUserGetInfo = netapi32.NewProc("NetUserGetInfo") + procNetApiBufferFree = netapi32.NewProc("NetApiBufferFree") + procNetUserGetLocalGroups = netapi32.NewProc("NetUserGetLocalGroups") +) + +// userInfo1 mirrors USER_INFO_1. Field order and widths must match the Win32 +// struct exactly; it is passed to NetUserAdd as a raw buffer. +type userInfo1 struct { + Name *uint16 + Password *uint16 + PasswordAge uint32 + Priv uint32 + HomeDir *uint16 + Comment *uint16 + Flags uint32 + ScriptPath *uint16 +} + +// userInfo1003 mirrors USER_INFO_1003, the password-only form NetUserSetInfo +// takes when nothing else about the account should change. +type userInfo1003 struct { + Password *uint16 +} + +// localGroupInfo1 mirrors LOCALGROUP_INFO_1. +type localGroupInfo1 struct { + Name *uint16 + Comment *uint16 +} + +// localGroupMembersInfo3 mirrors LOCALGROUP_MEMBERS_INFO_3, which identifies a +// member by name rather than SID. +type localGroupMembersInfo3 struct { + DomainAndName *uint16 +} + +// windowsSandboxIdentity is a provisioned sandbox principal: the account name +// and the SID that ACEs, tokens and firewall rules are keyed to. +type windowsSandboxIdentity struct { + Username string + SID *windows.SID +} + +// String renders the identity for logs without exposing the password, which is +// never stored on this struct. +func (identity windowsSandboxIdentity) String() string { + if identity.SID == nil { + return identity.Username + } + return identity.Username + " (" + identity.SID.String() + ")" +} + +// windowsSandboxUserName derives a stable account name for a workspace key. The +// key is hashed by the caller (see windowsSandboxWorkspaceKey) so the name reveals no +// path, and it is truncated to the 20-character local-account limit. The same +// workspace always maps to the same account, so re-running setup reuses the +// principal instead of accumulating accounts. +func windowsSandboxUserName(workspaceKey string) string { + cleaned := strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= '0' && r <= '9': + return r + case r >= 'A' && r <= 'Z': + return r + ('a' - 'A') + default: + return -1 + } + }, workspaceKey) + if cleaned == "" { + cleaned = "default" + } + name := windowsSandboxUserPrefix + cleaned + if len(name) > windowsSandboxUserNameMax { + name = name[:windowsSandboxUserNameMax] + } + return name +} + +// windowsSandboxUserCommentFor returns the ownership comment for a workspace, +// carrying the full key the account name could only hold 11 characters of. +func windowsSandboxUserCommentFor(workspaceKey string) string { + return windowsSandboxUserCommentKey + workspaceKey +} + +// newWindowsSandboxPassword returns a random password for a sandbox principal. +// The account is never signed into interactively: the password exists only so +// LogonUser can mint a token for it, so it is generated per provisioning run, +// handed straight to the caller, and never persisted by this file. Base32 of +// crypto/rand bytes keeps it alphanumeric, which satisfies complexity policies +// that reject unusual punctuation, and a fixed suffix guarantees the mixed-case +// and digit classes even if the random draw happens to omit one. +func newWindowsSandboxPassword() (string, error) { + raw := make([]byte, windowsPasswordLength) + if _, err := rand.Read(raw); err != nil { + return "", fmt.Errorf("generate sandbox password: %w", err) + } + encoded := base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(raw) + if len(encoded) > windowsPasswordLength { + encoded = encoded[:windowsPasswordLength] + } + return "Zs1!" + encoded, nil +} + +// netAPIStatus converts a netapi32 return value into an error, treating the +// supplied status codes as success so callers can spell out which "already +// exists" results are expected. +func netAPIStatus(call string, status uintptr, okStatuses ...uintptr) error { + if status == nerrSuccess { + return nil + } + for _, ok := range okStatuses { + if status == ok { + return nil + } + } + if status == errorAccessDenied32 { + return fmt.Errorf("%s: access denied (run `zero sandbox setup` from an elevated terminal)", call) + } + return fmt.Errorf("%s: status %d", call, status) +} + +// ensureWindowsSandboxGroup creates the managed local group, or leaves it alone +// when it already exists. +func ensureWindowsSandboxGroup() error { + name, err := windows.UTF16PtrFromString(windowsSandboxGroupName) + if err != nil { + return err + } + comment, err := windows.UTF16PtrFromString(windowsSandboxGroupComment) + if err != nil { + return err + } + info := localGroupInfo1{Name: name, Comment: comment} + status, _, _ := procNetLocalGroupAdd.Call( + 0, // local machine + 1, // level: LOCALGROUP_INFO_1 + uintptr(unsafe.Pointer(&info)), + 0, + ) + // The struct holds pointers into Go memory that the syscall dereferences, so + // it has to stay reachable until the call has returned. + runtime.KeepAlive(info) + runtime.KeepAlive(name) + runtime.KeepAlive(comment) + return netAPIStatus("NetLocalGroupAdd", status, nerrGroupExists, errorAliasExists) +} + +// ensureWindowsSandboxUser creates a sandbox account with the supplied password. +// The account is a plain local user with no home directory or logon script, +// flagged so its password never expires (nobody is there to rotate it) and so it +// is a normal, enabled account LogonUser can authenticate. +// +// It reports whether the account already existed, because NetUserAdd leaves such +// an account completely untouched, password included. The caller has to reset it +// or the secret it goes on to store would not be the account's password at all. +func ensureWindowsSandboxUser(username string, password string, workspaceKey string) (bool, error) { + name, err := windows.UTF16PtrFromString(username) + if err != nil { + return false, err + } + secret, err := windows.UTF16PtrFromString(password) + if err != nil { + return false, err + } + comment, err := windows.UTF16PtrFromString(windowsSandboxUserCommentFor(workspaceKey)) + if err != nil { + return false, err + } + info := userInfo1{ + Name: name, + Password: secret, + Priv: usrPrivUser, + Comment: comment, + Flags: ufScript | ufNormalAccount | ufDontExpirePasswd, + } + status, _, _ := procNetUserAdd.Call( + 0, // local machine + 1, // level: USER_INFO_1 + uintptr(unsafe.Pointer(&info)), + 0, + ) + // The struct holds pointers into Go memory that the call dereferences, so + // everything it borrows has to outlive the call. + runtime.KeepAlive(info) + runtime.KeepAlive(name) + runtime.KeepAlive(secret) + runtime.KeepAlive(comment) + if status == nerrUserExists { + return true, nil + } + return false, netAPIStatus("NetUserAdd", status) +} + +// resetWindowsSandboxUserPassword sets the password on an account that already +// existed, so the secret the caller stores is actually the account's password. +// +// Without this, re-running setup produced a fresh random password, wrote it to +// disk, and left the account authenticating with the old one, so every later +// command failed to log on with a principal that looked correctly provisioned. +func resetWindowsSandboxUserPassword(username string, password string) error { + name, err := windows.UTF16PtrFromString(username) + if err != nil { + return err + } + secret, err := windows.UTF16PtrFromString(password) + if err != nil { + return err + } + // USER_INFO_1003 is a password-only update, so nothing else about the + // account is disturbed. + info := userInfo1003{Password: secret} + status, _, _ := procNetUserSetInfo.Call( + 0, // local machine + uintptr(unsafe.Pointer(name)), + 1003, // level: USER_INFO_1003 + uintptr(unsafe.Pointer(&info)), + 0, + ) + runtime.KeepAlive(info) + runtime.KeepAlive(name) + runtime.KeepAlive(secret) + return netAPIStatus("NetUserSetInfo", status) +} + +// addWindowsSandboxUserToGroup puts a principal in the managed group, ignoring +// the status that means it is already a member. +func addWindowsSandboxUserToGroup(username string) error { + group, err := windows.UTF16PtrFromString(windowsSandboxGroupName) + if err != nil { + return err + } + member, err := windows.UTF16PtrFromString(username) + if err != nil { + return err + } + entry := localGroupMembersInfo3{DomainAndName: member} + status, _, _ := procNetLocalGroupAddMembers.Call( + 0, // local machine + uintptr(unsafe.Pointer(group)), + 3, // level: LOCALGROUP_MEMBERS_INFO_3 + uintptr(unsafe.Pointer(&entry)), + 1, // one member + ) + runtime.KeepAlive(entry) + runtime.KeepAlive(group) + runtime.KeepAlive(member) + return netAPIStatus("NetLocalGroupAddMembers", status, errorMemberInAlias) +} + +// errWindowsSandboxNameCollision reports that the derived account name is taken +// by a local account Zero did not create. Setup refuses rather than adopting it. +var errWindowsSandboxPrivilegedAccount = errors.New("the local account matching Zero's derived sandbox name belongs to a privileged group (Administrators, Power Users or Backup Operators); refusing to adopt it as a sandbox principal") + +var errWindowsSandboxNameCollision = errors.New("a local account with Zero's derived sandbox name already exists and was not created by Zero") + +// windowsSandboxUserIsManaged reports whether a local account is one Zero +// created, by reading back the comment provisioning stamps on it. +// +// This is the gate on adopting an existing account. The name is derived, not +// discovered, so an account can be sitting on it for reasons that have nothing +// to do with Zero, and taking it over means resetting a stranger's password. +// +// A missing account is not managed rather than an error, so callers can use this +// as a plain question without special-casing absence. +func windowsSandboxUserIsManaged(username string, workspaceKey string) (bool, error) { + name, err := windows.UTF16PtrFromString(username) + if err != nil { + return false, err + } + var buffer *byte + status, _, _ := procNetUserGetInfo.Call( + 0, // local machine + uintptr(unsafe.Pointer(name)), + 1, // level: USER_INFO_1 + uintptr(unsafe.Pointer(&buffer)), + ) + runtime.KeepAlive(name) + if status == nerrUserNotFound { + return false, nil + } + if err := netAPIStatus("NetUserGetInfo", status); err != nil { + return false, err + } + if buffer == nil { + return false, nil + } + defer procNetApiBufferFree.Call(uintptr(unsafe.Pointer(buffer))) + info := (*userInfo1)(unsafe.Pointer(buffer)) + if info.Comment == nil { + return false, nil + } + comment := windows.UTF16PtrToString(info.Comment) + // An account provisioned before the key was recorded is still ours; it + // predates this check and cannot be attributed to a workspace, so it is + // adopted and its comment rewritten on the way through. + if comment == windowsSandboxUserComment { + return true, nil + } + return comment == windowsSandboxUserCommentFor(workspaceKey), nil +} + +// localGroupUsersInfo0 mirrors LOCALGROUP_USERS_INFO_0: one group name pointer. +type localGroupUsersInfo0 struct { + Name *uint16 +} + +// windowsSandboxUserIsPrivileged reports whether an account belongs to a local +// group that would make it a poor sandbox principal. +// +// Adoption is the reason this exists. Provisioning will take over an account +// whose name and ownership comment match, and an account that is also in +// Administrators would hand the sandbox exactly the rights the sandbox is meant +// to withhold: it could rewrite the ACLs confining it, read the secret locked to +// the invoking user, and terminate Zero. The name is derived rather than +// discovered, so an account can end up matching without anyone intending it. +// +// Membership is resolved by SID rather than by name so a localised install, where +// the group is called Administrateurs or Administratoren, is still recognised. +func windowsSandboxUserIsPrivileged(username string) (bool, error) { + name, err := windows.UTF16PtrFromString(username) + if err != nil { + return false, err + } + var ( + buffer *byte + entries uint32 + total uint32 + ) + status, _, _ := procNetUserGetLocalGroups.Call( + 0, // local machine + uintptr(unsafe.Pointer(name)), + 0, // level: LOCALGROUP_USERS_INFO_0 + 0, // flags: direct membership only + uintptr(unsafe.Pointer(&buffer)), + uintptr(^uint32(0)), // MAX_PREFERRED_LENGTH + uintptr(unsafe.Pointer(&entries)), + uintptr(unsafe.Pointer(&total)), + ) + runtime.KeepAlive(name) + if status == nerrUserNotFound { + return false, nil + } + if err := netAPIStatus("NetUserGetLocalGroups", status); err != nil { + return false, err + } + if buffer == nil || entries == 0 { + return false, nil + } + defer procNetApiBufferFree.Call(uintptr(unsafe.Pointer(buffer))) + + privileged, err := privilegedLocalGroupNames() + if err != nil { + return false, err + } + groups := unsafe.Slice((*localGroupUsersInfo0)(unsafe.Pointer(buffer)), entries) + for _, group := range groups { + if group.Name == nil { + continue + } + if privileged[strings.ToLower(windows.UTF16PtrToString(group.Name))] { + return true, nil + } + } + return false, nil +} + +// privilegedLocalGroupNames resolves the local names of the groups a sandbox +// principal must not belong to. Resolved from well-known SIDs so the comparison +// survives a localised Windows install. +func privilegedLocalGroupNames() (map[string]bool, error) { + out := map[string]bool{} + for _, wellKnown := range []windows.WELL_KNOWN_SID_TYPE{ + windows.WinBuiltinAdministratorsSid, + windows.WinBuiltinPowerUsersSid, + windows.WinBuiltinBackupOperatorsSid, + } { + sid, err := windows.CreateWellKnownSid(wellKnown) + if err != nil { + // A group this build of Windows does not define is not a membership + // anyone can hold, so it cannot make an account privileged. + continue + } + account, _, _, err := sid.LookupAccount("") + if err != nil { + continue + } + out[strings.ToLower(account)] = true + } + if len(out) == 0 { + return nil, errors.New("could not resolve any privileged local group name") + } + return out, nil +} + +// resolveWindowsSandboxSID looks up the SID for a provisioned principal. The SID +// is the durable handle: account names can collide with a pre-existing local +// user, so every ACE and firewall rule is keyed to the SID rather than the name. +func resolveWindowsSandboxSID(username string) (*windows.SID, error) { + sid, _, accountType, err := windows.LookupSID("", username) + if err != nil { + return nil, fmt.Errorf("look up sandbox principal %q: %w", username, err) + } + if accountType != windows.SidTypeUser { + return nil, fmt.Errorf("sandbox principal %q resolves to a non-user account (type %d)", username, accountType) + } + return sid, nil +} + +// Indirected so a test can drive provisioning end to end and inject a failure +// at the two points that occur AFTER the account exists. Those are the paths +// whose return value the caller's rollback depends on. +// +// All four are seamed rather than just the last two: every step here needs an +// elevated caller and a real local account, so a test that only replaced the +// post-creation pair would never get past ensureWindowsSandboxGroup on an +// ordinary machine and would pass without reaching the code it names. +var ( + ensureWindowsSandboxGroupFn = ensureWindowsSandboxGroup + ensureWindowsSandboxUserFn = ensureWindowsSandboxUser + addWindowsSandboxUserToGroupFn = addWindowsSandboxUserToGroup + resolveWindowsSandboxSIDFn = resolveWindowsSandboxSID + resetWindowsSandboxUserPasswordFn = resetWindowsSandboxUserPassword + windowsSandboxUserIsManagedFn = windowsSandboxUserIsManaged + windowsSandboxUserIsPrivilegedFn = windowsSandboxUserIsPrivileged + grantWindowsSandboxLogonRightsFn = grantWindowsSandboxLogonRights + revokeWindowsSandboxLogonRightsFn = revokeWindowsSandboxLogonRights +) + +// provisionWindowsSandboxIdentity ensures the managed group and one sandbox +// principal for workspaceKey exist, and returns the identity plus the password +// the caller needs to mint a token with LogonUser. It is idempotent, so setup +// can run repeatedly. +// +// The password is returned rather than stored, so no credential is written to +// disk here; that happens a layer up where the secret has somewhere safe to +// live. The returned value is always the account's actual password, including +// when the account already existed, because that case is reset explicitly +// below. +func provisionWindowsSandboxIdentity(workspaceKey string) (windowsSandboxIdentity, string, bool, error) { + if err := ensureWindowsSandboxGroupFn(); err != nil { + return windowsSandboxIdentity{}, "", false, err + } + username := windowsSandboxUserName(workspaceKey) + password, err := newWindowsSandboxPassword() + if err != nil { + return windowsSandboxIdentity{}, "", false, err + } + existed, err := ensureWindowsSandboxUserFn(username, password, workspaceKey) + if err != nil { + return windowsSandboxIdentity{}, "", false, err + } + if existed { + // Prove the account is ours before touching it. The name is derived from a + // workspace hash rather than discovered, so it can be occupied by an + // account that has nothing to do with Zero, whether by coincidence or + // because somebody created it deliberately. Adopting one means resetting + // its password, which is not something to do on the strength of a name + // matching a pattern we generate ourselves. + managed, err := windowsSandboxUserIsManagedFn(username, workspaceKey) + if err != nil { + return windowsSandboxIdentity{}, "", false, err + } + if !managed { + return windowsSandboxIdentity{}, "", false, fmt.Errorf("%w: %q", errWindowsSandboxNameCollision, username) + } + // Ours by name and comment is not enough to adopt it. An account that also + // sits in Administrators (or Power Users, or Backup Operators) would give + // the sandbox the rights the sandbox exists to withhold: it could rewrite + // the ACLs confining it, read the secret locked to the invoking user, and + // stop Zero. Refuse rather than quietly take it over, and say which account + // so an operator can look at it. + privileged, err := windowsSandboxUserIsPrivilegedFn(username) + if err != nil { + return windowsSandboxIdentity{}, "", false, err + } + if privileged { + return windowsSandboxIdentity{}, "", false, fmt.Errorf("%w: %q", errWindowsSandboxPrivilegedAccount, username) + } + // Deliberately NOT resetting the password here. + // + // NetUserAdd left an existing account untouched, so the password above is + // not yet its password and something has to set it. Doing that here, at + // the top of provisioning, opened a window that lasted until the secret + // was written several steps later: a failure anywhere in between left a + // live account whose password nothing on disk knew, and because the + // account already existed the rollback correctly declined to delete it. + // The command path then read the absent secret as "not provisioned" and + // quietly fell back to the weaker backend, so the sandbox was downgraded + // for good with nothing to show for it. + // + // The caller rotates instead, immediately before committing the secret, + // which narrows that window to a single operation. Until it does, the + // account keeps its old password and the old secret on disk still + // authenticates, so a failure before that point costs nothing. + } + // Both failures below can happen AFTER NetUserAdd created the account, so the + // name has to come back with them. The caller's rollback deletes by + // identity.Username, and returning a zero identity alongside created=true + // asked it to delete "", which silently stranded the account this run had + // just made. Group attachment in particular is not a formality: it can fail + // under local policy, and it is the enforcement boundary, so a half-created + // principal is exactly the state worth not leaving behind. The SID is absent + // here, which the rollback already tolerates, since nothing has been granted + // to it yet. + if err := addWindowsSandboxUserToGroupFn(username); err != nil { + return windowsSandboxIdentity{Username: username}, "", !existed, err + } + sid, err := resolveWindowsSandboxSIDFn(username) + if err != nil { + return windowsSandboxIdentity{Username: username}, "", !existed, err + } + return windowsSandboxIdentity{Username: username, SID: sid}, password, !existed, nil +} + +// removeWindowsSandboxIdentity deletes a provisioned principal. Callers must +// revoke the principal's ACEs FIRST (see windowsPrincipalRevokePlan): deleting +// the account leaves any surviving ACE naming an unresolvable SID, which is what +// shows up in Explorer as an orphaned entry and is exactly the residue this +// model is meant to avoid. +// +// A missing account is success, so teardown converges the same way provisioning +// does. Requires an elevated caller. +func removeWindowsSandboxIdentity(username string) error { + name, err := windows.UTF16PtrFromString(username) + if err != nil { + return err + } + status, _, _ := procNetUserDel.Call(0, uintptr(unsafe.Pointer(name))) + return netAPIStatus("NetUserDel", status, nerrUserNotFound) +} + +// errWindowsSandboxIdentityUnavailable reports that no sandbox principal has +// been provisioned yet, so callers can fall back to the restricted-token +// backend instead of failing the command. +var errWindowsSandboxIdentityUnavailable = errors.New("no Zero sandbox principal is provisioned; run `zero sandbox setup` from an elevated terminal") + +// lookupWindowsSandboxIdentity resolves an already-provisioned principal without +// creating anything, so the unelevated command path can discover whether an +// identity exists. It returns errWindowsSandboxIdentityUnavailable when setup +// has not run. +func lookupWindowsSandboxIdentity(workspaceKey string) (windowsSandboxIdentity, error) { + username := windowsSandboxUserName(workspaceKey) + // Ownership is checked here as well as at provisioning, because the account + // NAME cannot carry the whole workspace key. + // + // The name keeps 11 characters of the digest; the comment holds all of it. + // Provisioning refuses a name whose comment names a different workspace, and + // without the same check here the workspace that LOST that race would still + // resolve the name to a SID and quietly use the other workspace's principal, + // its secret and its ACL identity. Setup would have failed for it, so this is + // the path that decides whether the refusal actually holds. + // + // A collision is very unlikely with real keys, roughly 2^-44 per pair, but the + // cost of being wrong is one workspace running as another's identity, and the + // check is one syscall on a path that is already doing several. + // SID resolution runs FIRST so "no such account" stays the unavailable + // sentinel. windowsSandboxUserIsManaged answers false for both an absent + // account and one belonging to someone else, so checking it before this would + // report an unprovisioned workspace as a name collision and turn the ordinary + // not-set-up case into an error the operator has to interpret. + sid, err := resolveWindowsSandboxSIDFn(username) + if err != nil { + return windowsSandboxIdentity{}, classifyWindowsSandboxLookupError(err) + } + managed, err := windowsSandboxUserIsManagedFn(username, workspaceKey) + if err != nil { + return windowsSandboxIdentity{}, err + } + if !managed { + return windowsSandboxIdentity{}, fmt.Errorf("%w: %q", errWindowsSandboxNameCollision, username) + } + return windowsSandboxIdentity{Username: username, SID: sid}, nil +} + +// classifyWindowsSandboxLookupError decides whether a failed SID resolution +// means "setup has not run" or "this principal exists but is unusable". +// +// Only "no such account" is the former. Every other failure is a principal the +// caller must not paper over, including the deliberate refusal in +// resolveWindowsSandboxSID of a name squatted by a group or alias. Collapsing +// those into the unavailable sentinel would turn a real conflict into a silent +// fall back to the restricted token, which is exactly the case that should +// reach the operator rather than be absorbed. +// +// Split out from the lookup so the decision can be asserted on its own: the +// lookup derives its account name from a workspace key, so a test cannot hand +// it a name that resolves to a group. +func classifyWindowsSandboxLookupError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, windows.ERROR_NONE_MAPPED) { + return errWindowsSandboxIdentityUnavailable + } + return err +} diff --git a/internal/sandbox/windows_identity_windows_test.go b/internal/sandbox/windows_identity_windows_test.go new file mode 100644 index 000000000..11d64c741 --- /dev/null +++ b/internal/sandbox/windows_identity_windows_test.go @@ -0,0 +1,461 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "fmt" + "os" + "strings" + "testing" + "unsafe" + + "golang.org/x/sys/windows" +) + +// A local Windows account name is capped at 20 characters, so the derived name +// must truncate rather than produce a name NetUserAdd rejects. +func TestWindowsSandboxUserNameRespectsLengthLimit(t *testing.T) { + name := windowsSandboxUserName(strings.Repeat("a", 64)) + if len(name) > windowsSandboxUserNameMax { + t.Fatalf("name %q is %d chars, want at most %d", name, len(name), windowsSandboxUserNameMax) + } + if !strings.HasPrefix(name, windowsSandboxUserPrefix) { + t.Fatalf("name %q lost the managed prefix", name) + } +} + +// The same workspace must map to the same principal, otherwise re-running setup +// would accumulate a new local account every time. +func TestWindowsSandboxUserNameIsStable(t *testing.T) { + first := windowsSandboxUserName("abc123") + second := windowsSandboxUserName("abc123") + if first != second { + t.Fatalf("name is not stable: %q vs %q", first, second) + } + if other := windowsSandboxUserName("def456"); other == first { + t.Fatalf("different workspaces produced the same principal %q", first) + } +} + +// The key is sanitised to characters a local account name accepts, so a hash or +// path fragment cannot smuggle a separator or a space into the name. +func TestWindowsSandboxUserNameRejectsUnsafeCharacters(t *testing.T) { + name := windowsSandboxUserName(`C:\Users\me\proj ect`) + for _, r := range strings.TrimPrefix(name, windowsSandboxUserPrefix) { + isLower := r >= 'a' && r <= 'z' + isDigit := r >= '0' && r <= '9' + if !isLower && !isDigit { + t.Fatalf("name %q contains unsafe rune %q", name, r) + } + } + if name == windowsSandboxUserPrefix { + t.Fatal("sanitising removed every character, leaving a bare prefix") + } +} + +// An empty or fully-sanitised-away key must still yield a usable name rather +// than the bare prefix. +func TestWindowsSandboxUserNameHandlesEmptyKey(t *testing.T) { + for _, key := range []string{"", "///", " "} { + if got := windowsSandboxUserName(key); got == windowsSandboxUserPrefix { + t.Fatalf("key %q produced a bare prefix", key) + } + } +} + +// The password must be fresh per call and carry the character classes a default +// Windows complexity policy demands, or NetUserAdd fails with ERROR_PASSWORD_RESTRICTION. +func TestNewWindowsSandboxPasswordIsRandomAndComplex(t *testing.T) { + first, err := newWindowsSandboxPassword() + if err != nil { + t.Fatalf("generate: %v", err) + } + second, err := newWindowsSandboxPassword() + if err != nil { + t.Fatalf("generate: %v", err) + } + if first == second { + t.Fatal("two generated passwords are identical, so they are not random") + } + if len(first) < 12 { + t.Fatalf("password is only %d chars", len(first)) + } + var hasUpper, hasLower, hasDigit bool + for _, r := range first { + switch { + case r >= 'A' && r <= 'Z': + hasUpper = true + case r >= 'a' && r <= 'z': + hasLower = true + case r >= '0' && r <= '9': + hasDigit = true + } + } + if !hasUpper || !hasLower || !hasDigit { + t.Fatalf("password %q lacks a required character class", first) + } +} + +// "Already exists" is the normal result of re-running setup and must not surface +// as an error, while a genuine failure must. +func TestNetAPIStatusTreatsExistingAsSuccess(t *testing.T) { + if err := netAPIStatus("NetUserAdd", nerrSuccess); err != nil { + t.Fatalf("success status returned %v", err) + } + if err := netAPIStatus("NetUserAdd", nerrUserExists, nerrUserExists); err != nil { + t.Fatalf("existing user must be success, got %v", err) + } + if err := netAPIStatus("NetLocalGroupAdd", nerrGroupExists, nerrGroupExists, errorAliasExists); err != nil { + t.Fatalf("existing group must be success, got %v", err) + } + if err := netAPIStatus("NetUserAdd", 2245); err == nil { + t.Fatal("an unexpected status must surface as an error") + } +} + +// Access-denied is the status an unelevated run gets, and it must say so rather +// than reporting a bare number the user cannot act on. +func TestNetAPIStatusExplainsAccessDenied(t *testing.T) { + err := netAPIStatus("NetUserAdd", errorAccessDenied32) + if err == nil { + t.Fatal("access denied must be an error") + } + if !strings.Contains(err.Error(), "elevated") { + t.Fatalf("error %q should point at elevation", err) + } +} + +// The Win32 structs are passed to netapi32 as raw buffers, so their layout must +// match what the API expects. A wrong size means silent memory corruption. +func TestWindowsIdentityStructLayouts(t *testing.T) { + ptr := unsafe.Sizeof(uintptr(0)) + if got, want := unsafe.Sizeof(localGroupMembersInfo3{}), ptr; got != want { + t.Fatalf("LOCALGROUP_MEMBERS_INFO_3 size = %d, want %d", got, want) + } + if got, want := unsafe.Sizeof(localGroupInfo1{}), 2*ptr; got != want { + t.Fatalf("LOCALGROUP_INFO_1 size = %d, want %d", got, want) + } + // USER_INFO_1 is four pointers plus three DWORDs, with the compiler padding + // each DWORD pair up to pointer alignment on amd64. + if got := unsafe.Sizeof(userInfo1{}); got < 4*ptr { + t.Fatalf("USER_INFO_1 size = %d, smaller than its four pointer fields", got) + } + if unsafe.Offsetof(userInfo1{}.Password) != ptr { + t.Fatal("USER_INFO_1.Password must directly follow Name") + } +} + +// LSA_UNICODE_STRING counts BYTES, not runes, and excludes the NUL terminator +// from Length while including it in MaximumLength. Getting either wrong makes +// LsaAddAccountRights read past the buffer or silently match no right, so pin it. +func TestNewLSAStringUsesByteLengths(t *testing.T) { + buffer, err := windows.UTF16FromString("SeBatchLogonRight") + if err != nil { + t.Fatalf("encode: %v", err) + } + entry := newLSAString(buffer) + const runes uint16 = uint16(len("SeBatchLogonRight")) + if entry.Length != runes*2 { + t.Fatalf("Length = %d, want %d (bytes, excluding NUL)", entry.Length, runes*2) + } + if entry.MaximumLength != (runes+1)*2 { + t.Fatalf("MaximumLength = %d, want %d (bytes, including NUL)", entry.MaximumLength, (runes+1)*2) + } + if entry.Buffer == nil { + t.Fatal("Buffer must point at the encoded string") + } +} + +// An empty buffer must not produce a struct pointing at nothing with a nonzero +// length, which would hand LSA a wild pointer. +func TestNewLSAStringHandlesEmptyBuffer(t *testing.T) { + entry := newLSAString(nil) + if entry.Buffer != nil || entry.Length != 0 || entry.MaximumLength != 0 { + t.Fatalf("empty buffer produced %+v, want a zero value", entry) + } +} + +// The LSA structs are passed to advapi32 as raw buffers, so their sizes must +// match the Win32 definitions. +func TestLSAStructLayouts(t *testing.T) { + ptr := unsafe.Sizeof(uintptr(0)) + // LSA_UNICODE_STRING: two uint16 then a pointer, padded to pointer alignment. + if got, want := unsafe.Sizeof(lsaUnicodeString{}), 2*ptr; got != want { + t.Fatalf("LSA_UNICODE_STRING size = %d, want %d", got, want) + } + if unsafe.Offsetof(lsaUnicodeString{}.Buffer) != ptr { + t.Fatal("LSA_UNICODE_STRING.Buffer must sit at the second pointer slot") + } + var attributes lsaObjectAttributes + if unsafe.Sizeof(attributes) < 6*ptr-ptr { + t.Fatalf("LSA_OBJECT_ATTRIBUTES size = %d, smaller than its fields", unsafe.Sizeof(attributes)) + } + if unsafe.Offsetof(attributes.ObjectName) == 0 { + t.Fatal("LSA_OBJECT_ATTRIBUTES.ObjectName must not alias Length") + } +} + +// Provisioning creates real local accounts, so it only runs when explicitly +// opted into on an elevated machine. Everything above covers the logic that can +// be exercised without touching the account database. +func TestProvisionWindowsSandboxIdentityRoundTrip(t *testing.T) { + if os.Getenv("ZERO_WINDOWS_IDENTITY_PROVISION_TEST") != "1" { + // Spelled out per shell because `set VAR=1` is cmd syntax and silently + // sets a shell variable rather than an environment variable in + // PowerShell, which makes this skip look like the elevation check failing. + t.Skip("provisioning test not enabled: PowerShell `$env:ZERO_WINDOWS_IDENTITY_PROVISION_TEST = \"1\"`, cmd `set ZERO_WINDOWS_IDENTITY_PROVISION_TEST=1`, bash `export ZERO_WINDOWS_IDENTITY_PROVISION_TEST=1` (also needs an elevated terminal)") + } + if !windowsProcessIsElevated() { + t.Skip("provisioning requires an elevated process") + } + // Starting clean keeps a failure here from being explained by residue from a + // previous run. Provisioning no longer resets an adopted account's password, + // so a leftover account would otherwise be adopted with a password this test + // never learns. + _ = removeWindowsSandboxIdentity(windowsSandboxUserName("ziptest01")) + + identity, password, _, err := provisionWindowsSandboxIdentity("ziptest01") + if err != nil { + t.Fatalf("provision: %v", err) + } + // Registered immediately after provisioning so every failure path below is + // covered. This test grants a real batch-logon right to a real local account; + // leaving either behind on a developer machine is not acceptable residue, and + // rights are revoked before the account so nothing is left keyed to a SID that + // no longer resolves. + t.Cleanup(func() { + if err := revokeWindowsSandboxLogonRights(identity.SID); err != nil { + t.Errorf("cleanup: revoke logon rights: %v", err) + } + if err := removeWindowsSandboxIdentity(identity.Username); err != nil { + t.Errorf("cleanup: remove principal: %v", err) + } + }) + if identity.SID == nil { + t.Fatal("provisioned identity has no SID") + } + if password == "" { + t.Fatal("provisioning returned an empty password") + } + // Re-running must converge on the same principal rather than failing or + // creating a second account. + again, secondPassword, _, err := provisionWindowsSandboxIdentity("ziptest01") + if err != nil { + t.Fatalf("second provision: %v", err) + } + if again.Username != identity.Username || !again.SID.Equals(identity.SID) { + t.Fatalf("provisioning is not idempotent: %s then %s", identity, again) + } + if secondPassword == "" { + t.Fatal("second provision returned an empty password") + } + // Deliberately NOT logging on with secondPassword. Provisioning does not + // rotate an adopted account any more, so that value is a fresh random string + // the account does not hold; rotation happens in + // provisionWindowsSandboxPrincipalForSetup, immediately before the secret is + // written, to keep the window where no stored password authenticates as small + // as possible. + // + // The guarantee worth asserting is therefore the one the setup path makes: + // after it returns, the stored secret logs the principal on. That covers + // rotation, the secret write and the logon right in one assertion, and it is + // the thing a broken re-setup would actually break. + config := WindowsSandboxCommandConfig{ + SandboxHome: t.TempDir(), + WorkspaceRoots: []string{`C:\ziptest01`}, + } + setupIdentity, _, err := provisionWindowsSandboxPrincipalForSetup(config) + if err != nil { + t.Fatalf("setup provision: %v", err) + } + secretPath, err := windowsSandboxSecretPath(config.SandboxHome, setupIdentity.Username) + if err != nil { + t.Fatalf("secret path: %v", err) + } + storedPassword, err := readWindowsSandboxSecret(secretPath) + if err != nil { + t.Fatalf("read stored secret: %v", err) + } + token, err := logonWindowsSandboxPrincipal(setupIdentity.Username, storedPassword) + if err != nil { + t.Fatalf("logon with the secret the setup path stored: %v", err) + } + _ = token.Close() + t.Cleanup(func() { + _ = revokeWindowsSandboxLogonRights(setupIdentity.SID) + _ = removeWindowsSandboxIdentity(setupIdentity.Username) + }) + // Lookup must find what provisioning created. + found, err := lookupWindowsSandboxIdentity("ziptest01") + if err != nil { + t.Fatalf("lookup after provision: %v", err) + } + if !found.SID.Equals(identity.SID) { + t.Fatalf("lookup returned %s, want %s", found, identity) + } +} + +// The other half of the privileged chain: granting logon rights and actually +// minting a token. Provisioning proves the account exists; this proves it is +// USABLE, which is the part the runner depends on. +// +// The batch logon doubles as the assertion that LsaAddAccountRights worked. A +// LOGON32_LOGON_BATCH logon fails with ERROR_LOGON_TYPE_NOT_GRANTED (1385) +// unless SeBatchLogonRight is actually held, so a token coming back is proof the +// grant landed rather than merely that the call returned success. +// +// Creates a real local account and removes it again, so it is gated the same way +// as the provisioning round-trip. +func TestGrantLogonRightsAndMintPrincipalToken(t *testing.T) { + if os.Getenv("ZERO_WINDOWS_IDENTITY_PROVISION_TEST") != "1" { + t.Skip("provisioning test not enabled: PowerShell `$env:ZERO_WINDOWS_IDENTITY_PROVISION_TEST = \"1\"`, cmd `set ZERO_WINDOWS_IDENTITY_PROVISION_TEST=1`, bash `export ZERO_WINDOWS_IDENTITY_PROVISION_TEST=1` (also needs an elevated terminal)") + } + if !windowsProcessIsElevated() { + t.Skip("granting logon rights requires an elevated process") + } + + const key = "ziplogon01" + // A leftover account from an interrupted run would keep its old password, + // which the freshly generated one will not match, so start from a clean slate. + _ = removeWindowsSandboxIdentity(windowsSandboxUserName(key)) + + identity, password, _, err := provisionWindowsSandboxIdentity(key) + if err != nil { + t.Fatalf("provision: %v", err) + } + t.Cleanup(func() { + // Rights first, then the account: the reverse order strands an LSA entry + // keyed to a SID that no longer resolves. + if err := revokeWindowsSandboxLogonRights(identity.SID); err != nil { + t.Errorf("cleanup: revoke logon rights: %v", err) + } + if err := removeWindowsSandboxIdentity(identity.Username); err != nil { + t.Errorf("cleanup: remove principal: %v", err) + } + }) + + // Exercises LsaAddAccountRights, including the LSA_UNICODE_STRING byte-length + // handling that nothing else has run. + if err := grantWindowsSandboxLogonRights(identity.SID); err != nil { + t.Fatalf("grant logon rights: %v", err) + } + // Idempotent: setup re-runs must not fail on rights the account already holds. + if err := grantWindowsSandboxLogonRights(identity.SID); err != nil { + t.Fatalf("granting logon rights twice must succeed: %v", err) + } + + token, err := logonWindowsSandboxPrincipal(identity.Username, password) + if err != nil { + t.Fatalf("logon as principal: %v", err) + } + defer token.Close() + + // The token must BE the principal. If this came back as the caller, the whole + // identity boundary would be an illusion and reads would still run as the user. + user, err := token.GetTokenUser() + if err != nil { + t.Fatalf("token user: %v", err) + } + if !user.User.Sid.Equals(identity.SID) { + t.Fatalf("token belongs to %s, want the principal %s", user.User.Sid, identity.SID) + } + t.Logf("minted a token for %s", identity) +} + +// A workspace with no provisioned principal must report the actionable +// "run setup" error rather than a raw lookup failure, so the command path can +// fall back instead of surfacing a Win32 code. +func TestLookupWindowsSandboxIdentityUnprovisioned(t *testing.T) { + _, err := lookupWindowsSandboxIdentity("nosuchworkspacekey9z") + if err == nil { + t.Skip("a principal for this key unexpectedly exists on this machine") + } + if err != errWindowsSandboxIdentityUnavailable { + t.Fatalf("error = %v, want errWindowsSandboxIdentityUnavailable", err) + } +} + +// A name that resolves to something other than a user account is a conflict, +// not an absent principal, and must not be reported as "setup has not run": the +// command path treats that sentinel as permission to fall back silently, so +// collapsing the two would hide a squatted account behind a quiet downgrade to +// the restricted token. +// +// Every machine already has well-known non-user names to test against, so this +// needs no privilege and no provisioning. +func TestLookupWindowsSandboxIdentityRejectsNonUserAccount(t *testing.T) { + // Groups that exist on any Windows install. Whichever resolves first is + // enough; localized machines may not carry the English name. + for _, group := range []string{"Administrators", "Users", "Guests"} { + sid, _, accountType, err := windows.LookupSID("", group) + if err != nil || sid == nil { + continue + } + if accountType == windows.SidTypeUser { + continue + } + resolveErr := func() error { + _, err := resolveWindowsSandboxSID(group) + return err + }() + if resolveErr == nil { + t.Fatalf("resolveWindowsSandboxSID(%q) accepted a non-user account (type %d)", group, accountType) + } + // The classification is the part that matters: the sentinel is what + // licenses the command path to fall back silently, so this refusal must + // survive it rather than be folded into it. + if classified := classifyWindowsSandboxLookupError(resolveErr); errors.Is(classified, errWindowsSandboxIdentityUnavailable) { + t.Fatalf("non-user account %q classified as unprovisioned, which would silently downgrade to the restricted token: %v", group, classified) + } + return + } + t.Skip("no well-known non-user account resolved on this machine") +} + +// The account name is derived from a workspace hash, not discovered, so it can +// be occupied by a local account that has nothing to do with Zero. Adopting one +// means resetting a stranger's password, so provisioning has to prove ownership +// first and refuse otherwise. +// +// Driven against real accounts every Windows install carries, which are +// definitively not ours. Unprivileged: it only has to establish that they are +// not classified as managed, so nothing is ever created or modified. +func TestWindowsSandboxUserIsManagedRefusesForeignAccounts(t *testing.T) { + checked := 0 + for _, name := range []string{"Administrator", "Guest", "DefaultAccount"} { + managed, err := windowsSandboxUserIsManaged(name, "workspacekey") + if err != nil { + // Localized or disabled installs may not carry every one of these. + continue + } + checked++ + if managed { + t.Fatalf("%q classified as a Zero sandbox principal; provisioning would reset its password", name) + } + } + if checked == 0 { + t.Skip("no well-known local account could be queried on this machine") + } + // An absent account must answer false rather than error, since provisioning + // asks this question about names that usually do not exist yet. + managed, err := windowsSandboxUserIsManaged("zero-sbx-nosuchacct", "workspacekey") + if err != nil { + t.Fatalf("querying a missing account: %v", err) + } + if managed { + t.Fatal("a missing account was classified as managed") + } +} + +// The refusal has to be a typed, recognisable collision rather than a generic +// failure, so setup can say what is wrong instead of reporting a Win32 status. +func TestWindowsSandboxNameCollisionIsTyped(t *testing.T) { + wrapped := fmt.Errorf("%w: %q", errWindowsSandboxNameCollision, "zero-sbx-dexample") + if !errors.Is(wrapped, errWindowsSandboxNameCollision) { + t.Fatal("collision error does not match its sentinel") + } + if !strings.Contains(wrapped.Error(), "not created by Zero") { + t.Fatalf("collision message = %q, want it to say the account is not ours", wrapped.Error()) + } +} diff --git a/internal/sandbox/windows_setup_windows.go b/internal/sandbox/windows_setup_windows.go index 888355397..ac8dc1243 100644 --- a/internal/sandbox/windows_setup_windows.go +++ b/internal/sandbox/windows_setup_windows.go @@ -35,6 +35,33 @@ func runWindowsSandboxSetup(config WindowsSandboxSetupConfig, stderr io.Writer) fmt.Fprintln(stderr, WindowsSandboxSetupName+": "+err.Error()) return 1 } + // Provision this workspace's sandbox principal, when opted in. A principal is + // a separate local account, so it is created only on an explicit opt-in: it + // is visible in `net user`, and account creation is exactly the kind of thing + // endpoint protection and enterprise policy object to. Without the opt-in the + // capability-SID backend above is the whole of setup, unchanged. + if windowsSandboxIdentityEnabled(config.commandConfig().Env) { + principalRollback, err := setupWindowsSandboxPrincipal(config.commandConfig()) + if err != nil { + if rollbackErr := rollback(); rollbackErr != nil { + fmt.Fprintf(stderr, "%s: %v; rollback failed: %v\n", WindowsSandboxSetupName, err, rollbackErr) + return 1 + } + fmt.Fprintln(stderr, WindowsSandboxSetupName+": "+err.Error()) + return 1 + } + // Fold the principal into the existing rollback so every later failure + // path undoes it too, rather than each one having to remember. + aclRollback := rollback + rollback = func() error { + principalErr := principalRollback() + aclErr := aclRollback() + if principalErr != nil { + return principalErr + } + return aclErr + } + } if err := applyWindowsNetworkPlan(networkPlan); err != nil { if rollbackErr := rollback(); rollbackErr != nil { fmt.Fprintf(stderr, "%s: %v; rollback failed: %v\n", WindowsSandboxSetupName, err, rollbackErr)