diff --git a/src/pkg/proxy/trust.go b/src/pkg/proxy/trust.go index 4c8e1697..2ef7c0df 100644 --- a/src/pkg/proxy/trust.go +++ b/src/pkg/proxy/trust.go @@ -18,7 +18,11 @@ import ( const phaseAITrustNickname = "Phase AI Proxy CA" -const windowsPhaseCARotationScript = `$ErrorActionPreference='Stop'; $desired='__PHASE_SHA256__'; $phaseRoots=Get-ChildItem -Path Cert:\CurrentUser\Root | Where-Object { $_.Subject -eq $_.Issuer -and $_.Subject -match '(^|,\s*)CN=Phase AI Proxy CA(,|$)' -and $_.Subject -match '(^|,\s*)O=Phase(,|$)' -and ($_.Extensions | Where-Object { $_.Oid.Value -eq '2.5.29.19' -and $_.CertificateAuthority }) }; if ($phaseRoots | Where-Object { $_.GetCertHashString('SHA256').ToUpperInvariant() -eq $desired }) { Write-Output 'PHASE_CA_CURRENT'; exit 0 }; $phaseRoots | ForEach-Object { Remove-Item -LiteralPath $_.PSPath -Force }` +// windowsPhaseRootInspectionScript only reads the current-user Root store. It +// prints PHASE_CA_CURRENT when the CA being trusted is already there, otherwise +// one PHASE_CA_OBSOLETE line per older Phase-owned root. Deleting through the +// Cert: drive fails under -NonInteractive, so trustWindows removes them with certutil. +const windowsPhaseRootInspectionScript = `$ErrorActionPreference='Stop'; $desired='__PHASE_SHA256__'; $phaseRoots=Get-ChildItem -Path Cert:\CurrentUser\Root | Where-Object { $_.Subject -eq $_.Issuer -and $_.Subject -match '(^|,\s*)CN=Phase AI Proxy CA(,|$)' -and $_.Subject -match '(^|,\s*)O=Phase(,|$)' -and ($_.Extensions | Where-Object { $_.Oid.Value -eq '2.5.29.19' -and $_.CertificateAuthority }) }; if ($phaseRoots | Where-Object { $_.GetCertHashString('SHA256').ToUpperInvariant() -eq $desired }) { Write-Output 'PHASE_CA_CURRENT'; exit 0 }; $phaseRoots | ForEach-Object { Write-Output ('PHASE_CA_OBSOLETE ' + $_.Thumbprint) }` // ErrUserCATrustUnsupported means Phase cannot safely modify a trust store for // only the current user on this operating system. Callers can continue with @@ -128,7 +132,7 @@ func UserCATrustNotice(goos string) string { case "darwin": return "macOS asks for your login password or Touch ID once: the public Phase AI CA is added to your login keychain's trust settings. No sudo, nothing system-wide." case "windows": - return "Windows shows one security warning to confirm adding the public Phase AI CA to your user Root store. Nothing system-wide." + return "Windows shows a security warning to confirm adding the public Phase AI CA to your user Root store, plus one for each older Phase AI CA it removes. Nothing system-wide." default: return "" } @@ -153,7 +157,35 @@ func (osTrustCommandRunner) LookPath(file string) (string, error) { } func (osTrustCommandRunner) Run(name string, args ...string) ([]byte, error) { - return exec.Command(name, args...).CombinedOutput() + command := exec.Command(name, args...) + if strings.EqualFold(filepath.Base(name), "powershell.exe") { + command.Env = windowsPowerShellEnvironment(os.Environ()) + } + return command.CombinedOutput() +} + +// windowsPowerShellEnvironment gives Windows PowerShell its own module path. +// Launched from PowerShell 7 it inherits that shell's PSModulePath, cannot +// auto-load the certificate provider, and reports that the Cert: drive does +// not exist. +func windowsPowerShellEnvironment(base []string) []string { + systemRoot := os.Getenv("SystemRoot") + if systemRoot == "" { + systemRoot = `C:\Windows` + } + programFiles := os.Getenv("ProgramFiles") + if programFiles == "" { + programFiles = `C:\Program Files` + } + modulePath := "PSModulePath=" + programFiles + `\WindowsPowerShell\Modules;` + systemRoot + `\System32\WindowsPowerShell\v1.0\Modules` + environment := make([]string, 0, len(base)+1) + for _, entry := range base { + if key, _, _ := strings.Cut(entry, "="); strings.EqualFold(key, "PSModulePath") { + continue + } + environment = append(environment, entry) + } + return append(environment, modulePath) } type userCATrustInstaller struct { @@ -296,17 +328,29 @@ func (i userCATrustInstaller) trustWindows(certificatePath string, certificate * } fingerprint := sha256.Sum256(certificate.Raw) fingerprintHex := strings.ToUpper(hex.EncodeToString(fingerprint[:])) - rotationScript := strings.Replace(windowsPhaseCARotationScript, "__PHASE_SHA256__", fingerprintHex, 1) - rotationOutput, rotationErr := i.Runner.Run( + inspectionScript := strings.Replace(windowsPhaseRootInspectionScript, "__PHASE_SHA256__", fingerprintHex, 1) + inspectionOutput, inspectionErr := i.Runner.Run( powershell, "-NoLogo", "-NoProfile", "-NonInteractive", - "-Command", rotationScript, + "-Command", inspectionScript, ) - if rotationErr != nil { - return UserCATrustResult{}, trustCommandError("inspect and rotate Phase AI CA in Windows current-user Root store", rotationErr, rotationOutput) + if inspectionErr != nil { + return UserCATrustResult{}, trustCommandError("inspect Phase AI CA in Windows current-user Root store", inspectionErr, inspectionOutput) } - if strings.Contains(string(rotationOutput), "PHASE_CA_CURRENT") { + if strings.Contains(string(inspectionOutput), "PHASE_CA_CURRENT") { return UserCATrustResult{Store: "Windows current-user Root store", AlreadyTrusted: true}, nil } + obsoleteThumbprints, err := parseWindowsObsoletePhaseRoots(inspectionOutput) + if err != nil { + return UserCATrustResult{}, fmt.Errorf("inspect Phase AI CA in Windows current-user Root store: %w", err) + } + // Remove obsolete roots first so a rotation never leaves an old Phase root + // trusted. Windows confirms each removal, as it does the add below. + for _, thumbprint := range obsoleteThumbprints { + deleteOutput, deleteErr := i.Runner.Run(certutil, "-user", "-delstore", "Root", thumbprint) + if deleteErr != nil { + return UserCATrustResult{}, trustCommandError("remove obsolete Phase AI CA from Windows current-user Root store", deleteErr, deleteOutput) + } + } // -user is the security boundary: without it certutil targets the local // machine store and may elevate. -f makes re-enabling idempotent. output, err := i.Runner.Run(certutil, "-user", "-addstore", "-f", "Root", certificatePath) @@ -316,6 +360,24 @@ func (i userCATrustInstaller) trustWindows(certificatePath string, certificate * return UserCATrustResult{Store: "Windows current-user Root store"}, nil } +// parseWindowsObsoletePhaseRoots returns the SHA-1 thumbprints the inspection +// script printed. certutil -delstore also matches store indexes and common +// names, so anything but a 40-hex thumbprint is rejected. +func parseWindowsObsoletePhaseRoots(output []byte) ([]string, error) { + thumbprints := make([]string, 0) + for _, line := range strings.Split(string(output), "\n") { + thumbprint, found := strings.CutPrefix(strings.TrimSpace(line), "PHASE_CA_OBSOLETE ") + if !found { + continue + } + if raw, err := hex.DecodeString(thumbprint); err != nil || len(raw) != 20 { + return nil, fmt.Errorf("unexpected certificate thumbprint %q", thumbprint) + } + thumbprints = append(thumbprints, thumbprint) + } + return thumbprints, nil +} + func parsePEMCertificates(payload []byte) []*x509.Certificate { result := make([]*x509.Certificate, 0) rest := payload diff --git a/src/pkg/proxy/trust_test.go b/src/pkg/proxy/trust_test.go index 2de0b0d5..c6ccc974 100644 --- a/src/pkg/proxy/trust_test.go +++ b/src/pkg/proxy/trust_test.go @@ -189,10 +189,80 @@ func TestUserCATrustWindowsUsesUserStore(t *testing.T) { if !reflect.DeepEqual(runner.commands[1].Args, want) { t.Fatalf("certutil args = %v, want %v", runner.commands[1].Args, want) } - rotationArgs := strings.Join(runner.commands[0].Args, " ") - for _, required := range []string{"Cert:\\CurrentUser\\Root", "CN=Phase AI Proxy CA", "O=Phase", "Subject -eq $_.Issuer", "2.5.29.19", "Remove-Item -LiteralPath"} { - if !strings.Contains(rotationArgs, required) { - t.Fatalf("Windows rotation is not scoped to Phase-owned current-user roots; missing %q in %s", required, rotationArgs) + inspectionArgs := strings.Join(runner.commands[0].Args, " ") + for _, required := range []string{"Cert:\\CurrentUser\\Root", "CN=Phase AI Proxy CA", "O=Phase", "Subject -eq $_.Issuer", "2.5.29.19", "PHASE_CA_OBSOLETE"} { + if !strings.Contains(inspectionArgs, required) { + t.Fatalf("Windows inspection is not scoped to Phase-owned current-user roots; missing %q in %s", required, inspectionArgs) + } + } + // The Cert: drive refuses Root store removals under -NonInteractive. + if strings.Contains(inspectionArgs, "Remove-Item") { + t.Fatalf("Windows inspection must not delete through the Cert: drive: %s", inspectionArgs) + } +} + +func TestUserCATrustWindowsRemovesObsoletePhaseRootsWithCertutilBeforeTrusting(t *testing.T) { + certificatePath, _, _ := writeTrustTestCA(t) + obsolete := []string{strings.Repeat("A1", 20), strings.Repeat("0F", 20)} + runner := &recordingTrustRunner{run: func(_ string, args ...string) ([]byte, error) { + if len(args) > 0 && args[0] == "-NoLogo" { + return []byte("PHASE_CA_OBSOLETE " + obsolete[0] + "\r\nPHASE_CA_OBSOLETE " + obsolete[1] + "\r\n"), nil + } + return nil, nil + }} + result, err := (userCATrustInstaller{GOOS: "windows", Home: `C:\\Users\\phase`, Runner: runner}).Trust(certificatePath) + if err != nil { + t.Fatal(err) + } + if result.AlreadyTrusted || len(runner.commands) != 4 { + t.Fatalf("unexpected result=%+v commands=%v", result, runner.commands) + } + for index, thumbprint := range obsolete { + command := runner.commands[index+1] + want := []string{"-user", "-delstore", "Root", thumbprint} + if command.Name != "/usr/bin/certutil" || !reflect.DeepEqual(command.Args, want) { + t.Fatalf("removal %d = %s %v, want certutil %v", index, command.Name, command.Args, want) + } + } + want := []string{"-user", "-addstore", "-f", "Root", certificatePath} + if !reflect.DeepEqual(runner.commands[3].Args, want) { + t.Fatalf("certutil args = %v, want %v", runner.commands[3].Args, want) + } +} + +func TestUserCATrustWindowsRejectsMalformedObsoleteThumbprint(t *testing.T) { + certificatePath, _, _ := writeTrustTestCA(t) + // certutil -delstore also accepts store indexes: "0" is the first root. + runner := &recordingTrustRunner{run: func(_ string, args ...string) ([]byte, error) { + if len(args) > 0 && args[0] == "-NoLogo" { + return []byte("PHASE_CA_OBSOLETE 0\r\n"), nil + } + return nil, nil + }} + _, err := (userCATrustInstaller{GOOS: "windows", Home: `C:\\Users\\phase`, Runner: runner}).Trust(certificatePath) + if err == nil || len(runner.commands) != 1 { + t.Fatalf("malformed thumbprint err=%v commands=%v", err, runner.commands) + } +} + +func TestUserCATrustWindowsStopsWhenObsoleteRootRemovalFails(t *testing.T) { + certificatePath, _, _ := writeTrustTestCA(t) + runner := &recordingTrustRunner{run: func(_ string, args ...string) ([]byte, error) { + switch { + case len(args) > 0 && args[0] == "-NoLogo": + return []byte("PHASE_CA_OBSOLETE " + strings.Repeat("AB", 20) + "\r\n"), nil + case len(args) > 1 && args[1] == "-delstore": + return []byte("CertUtil: -delstore command FAILED: 0x800704c7 (WIN32: 1223 ERROR_CANCELLED)"), errors.New("exit status 2") + } + return nil, nil + }} + _, err := (userCATrustInstaller{GOOS: "windows", Home: `C:\\Users\\phase`, Runner: runner}).Trust(certificatePath) + if err == nil || !strings.Contains(err.Error(), "ERROR_CANCELLED") { + t.Fatalf("removal failure err = %v", err) + } + for _, command := range runner.commands { + if len(command.Args) > 1 && command.Args[1] == "-addstore" { + t.Fatalf("new CA was added after an obsolete root could not be removed: %v", runner.commands) } } } @@ -353,3 +423,19 @@ func TestUserCATrustLinuxMissingCertutilIsAMissingToolError(t *testing.T) { t.Fatalf("commands ran without certutil: %v", runner.commands) } } + +func TestWindowsPowerShellEnvironmentReplacesTheInheritedModulePath(t *testing.T) { + environment := windowsPowerShellEnvironment([]string{`PATH=C:\bin`, `PSModulePath=C:\Program Files\PowerShell\7\Modules`, `HOME=C:\Users\phase`}) + var modulePaths []string + for _, entry := range environment { + if strings.HasPrefix(strings.ToLower(entry), "psmodulepath=") { + modulePaths = append(modulePaths, entry) + } + } + if len(modulePaths) != 1 || !strings.Contains(modulePaths[0], `\WindowsPowerShell\v1.0\Modules`) || strings.Contains(modulePaths[0], `PowerShell\7`) { + t.Fatalf("module path = %v", modulePaths) + } + if len(environment) != 3 { + t.Fatalf("environment = %v", environment) + } +}