diff --git a/Logger.cs b/Logger.cs index 14c8765..8b4fdd1 100644 --- a/Logger.cs +++ b/Logger.cs @@ -72,7 +72,7 @@ public static void Initialize(string logDirectory, string version = "Unknown", b WriteToFile(LogLevel.Info, $"Process Architecture: {System.Runtime.InteropServices.RuntimeInformation.ProcessArchitecture}"); WriteToFile(LogLevel.Info, $"OS Architecture: {System.Runtime.InteropServices.RuntimeInformation.OSArchitecture}"); WriteToFile(LogLevel.Info, $"Working Directory: {Environment.CurrentDirectory}"); - WriteToFile(LogLevel.Info, $"Command Line: {Environment.CommandLine}"); + WriteToFile(LogLevel.Info, $"Command Line: {BootstrapMate.Core.CommandLineRedaction.Redact(Environment.CommandLine)}"); WriteToFile(LogLevel.Info, $"Is Interactive: {Environment.UserInteractive}"); WriteToFile(LogLevel.Info, $"Current User: {System.Security.Principal.WindowsIdentity.GetCurrent().Name}"); WriteToFile(LogLevel.Info, $"Verbose Console: {verboseConsole}"); diff --git a/Program.cs b/Program.cs index 55ba9f2..3896ad3 100644 --- a/Program.cs +++ b/Program.cs @@ -258,21 +258,27 @@ static int Main(string[] args) bool verboseMode = args.Any(arg => arg.Equals("--verbose", StringComparison.OrdinalIgnoreCase) || arg.Equals("-v", StringComparison.Ordinal)); + // Before anything is read from ProgramData\ManagedBootstrap: a standard user can + // create files under ProgramData, and this process trusts what it finds there. + // Before the configuration loads: stale 32-bit settings go, and credential + // headers move out of the user-readable policy and settings keys. + var startupNotes = new List(); + if (IsRunningAsAdministrator()) + { + startupNotes.AddRange(DataDirectoryGuard.Secure().Select(n => $"Data directory: {n}")); + startupNotes.AddRange(LegacyRegistry.RemoveStaleWow64Settings()); + startupNotes.AddRange(SecretStore.MigrateReadableCopies()); + } + // Policy or saved settings can turn either on; a CLI switch cannot turn them off. silentMode |= ConfigManager.Instance.Config.SilentMode; verboseMode |= ConfigManager.Instance.Config.VerboseMode; - // Before anything is read from ProgramData\ManagedBootstrap: a standard user can - // create files under ProgramData, and this process trusts what it finds there. - var dataDirectoryNotes = IsRunningAsAdministrator() - ? DataDirectoryGuard.Secure() - : new List(); - Logger.Initialize(LogDirectory, Version, verboseMode, silentMode); - Logger.Debug("Main() called with arguments: " + string.Join(" ", args)); - foreach (var note in dataDirectoryNotes) + Logger.Debug("Main() called with arguments: " + string.Join(" ", CommandLineRedaction.Redact(args))); + foreach (var note in startupNotes) { - Logger.Warning($"Data directory: {note}"); + Logger.Warning(note); } foreach (var retired in ConfigManager.Instance.RetiredSettingsPresent) { diff --git a/README.md b/README.md index d32d752..55ecda3 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,22 @@ Before building, set up your environment variables: 3. **Install your code signing certificate** in the Current User certificate store +## Where settings are read + +Settings come from policy (`HKLM\SOFTWARE\Policies\BootstrapMate`, written by Intune or Group +Policy) and then machine settings (`HKLM\SOFTWARE\BootstrapMate\Settings`). Both are read from +the 64-bit registry view only. A copy of the settings key in `WOW6432Node` is stale. The MSI +removes it, and the CLI removes it once and logs which values it removed. The policy key is +shared between the two views and is never removed. + +`AuthorizationHeader` and `ReportingHeader` carry credentials, so they live in +`HKLM\SOFTWARE\BootstrapMate\Secrets`. Only SYSTEM and Administrators can read that key. +Deliver a header through policy as usual. The next elevated run moves it into that key and +blanks the copy in policy, which every user can read. The policy value is emptied, not deleted, +so the setting still shows as managed. The GUI shows a saved header masked and saves a new one +only after Unlock. BootstrapMate never writes a header to its logs, `session.json` or +`last-run.json`, and it redacts `--headers` from every logged command line. + ## Reporting When a run completes, BootstrapMate can POST a vendor-neutral JSON run summary to an optional endpoint, turning "did this PC provision cleanly?" into a fleet-dashboard query. The payload is plain JSON and not tied to any specific backend — any service accepting a JSON POST (a custom collector, ReportMate, MunkiReport, etc.) can consume it. Both the Windows and macOS clients emit the same schema. diff --git a/SessionLog.cs b/SessionLog.cs index a92afef..8d7c73f 100644 --- a/SessionLog.cs +++ b/SessionLog.cs @@ -193,7 +193,7 @@ private void WriteSessionFile(string status, DateTime? end = null) ["os_version"] = System.Environment.OSVersion.ToString(), ["user"] = System.Environment.UserName, ["pid"] = System.Environment.ProcessId.ToString(), - ["command_line"] = System.Environment.CommandLine + ["command_line"] = BootstrapMate.Core.CommandLineRedaction.Redact(System.Environment.CommandLine) }, Summary = new SessionSummary { Events = _events, Errors = _errors, Warnings = _warnings } }; diff --git a/installer/Product.wxs b/installer/Product.wxs index 5d701d0..f74982b 100644 --- a/installer/Product.wxs +++ b/installer/Product.wxs @@ -97,6 +97,25 @@ + + + + + + + + + + + + + + @@ -173,6 +192,8 @@ + + diff --git a/src/BootstrapMate.App/ViewModels/PrefsViewModel.cs b/src/BootstrapMate.App/ViewModels/PrefsViewModel.cs index 5b8f209..28e5ba5 100644 --- a/src/BootstrapMate.App/ViewModels/PrefsViewModel.cs +++ b/src/BootstrapMate.App/ViewModels/PrefsViewModel.cs @@ -259,11 +259,8 @@ public List BuildRunArguments() args.Add(ManifestUrl); } - if (!string.IsNullOrWhiteSpace(AuthorizationHeader)) - { - args.Add("--headers"); - args.Add(AuthorizationHeader); - } + // No --headers: the header is saved to the protected store, where the elevated run + // reads it, and never travels on a command line. if (DryRun) args.Add("--dry-run"); if (SilentMode) args.Add("--silent"); diff --git a/src/BootstrapMate.Core/CommandLineRedaction.cs b/src/BootstrapMate.Core/CommandLineRedaction.cs new file mode 100644 index 0000000..15581f5 --- /dev/null +++ b/src/BootstrapMate.Core/CommandLineRedaction.cs @@ -0,0 +1,63 @@ +namespace BootstrapMate.Core; + +/// +/// Hides the value of every switch that carries a credential before a command line is +/// written to a log or session record. +/// +public static class CommandLineRedaction +{ + public const string Mask = ""; + + /// Switches whose value is a credential. + public static readonly string[] SecretSwitches = ["--headers", "--reporting-header"]; + + public static IEnumerable Redact(IEnumerable args) + { + bool hideNext = false; + foreach (var arg in args) + { + if (hideNext) + { + hideNext = false; + yield return Mask; + continue; + } + var match = SecretSwitches.FirstOrDefault(s => + arg.StartsWith(s + "=", StringComparison.OrdinalIgnoreCase) || + arg.StartsWith(s + ":", StringComparison.OrdinalIgnoreCase)); + if (match is not null) + { + yield return $"{arg[..(match.Length + 1)]}{Mask}"; + continue; + } + hideNext = SecretSwitches.Contains(arg, StringComparer.OrdinalIgnoreCase); + yield return arg; + } + } + + /// + /// The same for a whole command line as Windows quotes it. Quoting is honoured well + /// enough to find a value; the result is for display only. + /// + public static string Redact(string commandLine) => string.Join(" ", Redact(Split(commandLine)).Select(Quote)); + + private static IEnumerable Split(string commandLine) + { + var current = new System.Text.StringBuilder(); + bool quoted = false, any = false; + foreach (var c in commandLine) + { + if (c == '"') { quoted = !quoted; any = true; continue; } + if (char.IsWhiteSpace(c) && !quoted) + { + if (any) { yield return current.ToString(); current.Clear(); any = false; } + continue; + } + current.Append(c); + any = true; + } + if (any) yield return current.ToString(); + } + + private static string Quote(string arg) => arg.Length == 0 || arg.Any(char.IsWhiteSpace) ? $"\"{arg}\"" : arg; +} diff --git a/src/BootstrapMate.Core/ConfigManager.cs b/src/BootstrapMate.Core/ConfigManager.cs index 5d49998..13af99c 100644 --- a/src/BootstrapMate.Core/ConfigManager.cs +++ b/src/BootstrapMate.Core/ConfigManager.cs @@ -146,6 +146,12 @@ bool Skip(string key) => management.IsManaged(key) || void WriteString(string key, string? value) { if (Skip(key)) return; + if (OperatingSystem.IsWindows() && SecretStore.IsSecret(key)) + { + // Credentials never go to the user-readable settings key. + SecretStore.Write(key, value); + return; + } WriteRegistryValue(key, value ?? string.Empty, RegistryValueKind.String); } @@ -201,6 +207,21 @@ private void LoadManagementAndMachineSettings() LoadFromMachineRegistry(); LoadFromManagement(management); + LoadSecrets(); + } + + /// + /// Credential headers come from the protected store, which only an elevated process can + /// read; a readable copy left in policy or settings is used only until an elevated run + /// moves it there. + /// + private void LoadSecrets() + { + if (OperatingSystem.IsWindows()) + { + Config.AuthorizationHeader = SecretStore.Read("AuthorizationHeader") ?? Config.AuthorizationHeader; + Config.ReportingHeader = SecretStore.Read("ReportingHeader") ?? Config.ReportingHeader; + } } private void LoadFromManagement(ManagementDetector management) @@ -273,10 +294,9 @@ private void LoadFromManagement(ManagementDetector management) private void LoadFromMachineRegistry() { - // HKLM\SOFTWARE\BootstrapMate\Settings — read from both registry views so - // a 32-bit MSI write is visible to the 64-bit binary and vice-versa. + // HKLM\SOFTWARE\BootstrapMate\Settings, 64-bit view only: the MSI and the GUI write + // there, and a stale 32-bit copy (WOW6432Node) used to outrank them. LoadFromHive(RegistryHive.LocalMachine, ConfigSource.MachineSettings, RegistryView.Registry64); - LoadFromHive(RegistryHive.LocalMachine, ConfigSource.MachineSettings, RegistryView.Registry32); } private void LoadFromHive(RegistryHive hive, ConfigSource source, RegistryView view = RegistryView.Default) diff --git a/src/BootstrapMate.Core/LegacyRegistry.cs b/src/BootstrapMate.Core/LegacyRegistry.cs new file mode 100644 index 0000000..4c5acaf --- /dev/null +++ b/src/BootstrapMate.Core/LegacyRegistry.cs @@ -0,0 +1,57 @@ +using System.Runtime.Versioning; +using Microsoft.Win32; + +namespace BootstrapMate.Core; + +/// +/// Settings and policy are read from the 64-bit registry view only. Older builds also read +/// the 32-bit view (WOW6432Node), where stale copies outranked what the GUI and the MSI +/// write. Those copies are removed once, and each removal is reported. +/// +[SupportedOSPlatform("windows")] +public static class LegacyRegistry +{ + /// Set under HKLM\SOFTWARE\BootstrapMate once the 32-bit copies are gone. + public const string DoneMarker = "Wow64SettingsRemoved"; + + /// + /// 32-bit view keys that used to be read as settings. Only the settings key: the + /// policy key (SOFTWARE\Policies) is shared between the two views, so its "32-bit copy" + /// is the live policy itself and must never be removed. + /// + public static readonly string[] StaleKeys = [BootstrapMateConstants.MachineSettingsRegistryPath]; + + /// Removes the stale 32-bit keys once. Must run elevated. Returns what it removed. + public static List RemoveStaleWow64Settings() + { + var notes = new List(); + try + { + using var base64 = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64); + using var tool = base64.CreateSubKey(@"SOFTWARE\BootstrapMate", writable: true); + if (tool.GetValue(DoneMarker) is int done && done != 0) return notes; + + using var base32 = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry32); + foreach (var path in StaleKeys) + { + using (var key = base32.OpenSubKey(path)) + { + if (key is null) continue; + // Names only: a value here may be a credential. + var names = key.GetValueNames().Where(n => n.Length > 0).ToArray(); + var below = path[@"SOFTWARE\".Length..]; + notes.Add($@"Removed stale HKLM\SOFTWARE\WOW6432Node\{below}" + + (names.Length > 0 ? $" ({string.Join(", ", names)})" : "")); + } + base32.DeleteSubKeyTree(path, throwOnMissingSubKey: false); + } + + tool.SetValue(DoneMarker, 1, RegistryValueKind.DWord); + } + catch (Exception ex) + { + notes.Add($"Could not remove stale 32-bit settings: {ex.Message}"); + } + return notes; + } +} diff --git a/src/BootstrapMate.Core/ManagementDetector.cs b/src/BootstrapMate.Core/ManagementDetector.cs index 5fbade7..5f6f42a 100644 --- a/src/BootstrapMate.Core/ManagementDetector.cs +++ b/src/BootstrapMate.Core/ManagementDetector.cs @@ -103,10 +103,14 @@ public HashSet AllManagedKeys() private static string[] GetAliases(string canonicalKey) => KeyAliases.TryGetValue(canonicalKey, out var aliases) ? aliases : [canonicalKey]; + /// Every policy value name that sets . + public static string[] AliasesFor(string canonicalKey) => GetAliases(canonicalKey); + private static object? FindRegistryValue(string[] keyNames) { - // Check both 64-bit and 32-bit registry views - foreach (var view in new[] { RegistryView.Registry64, RegistryView.Registry32 }) + // 64-bit view only: Intune and Group Policy write there, and a stale 32-bit copy + // (WOW6432Node) must never outrank it. + foreach (var view in new[] { RegistryView.Registry64 }) { try { diff --git a/src/BootstrapMate.Core/SecretStore.cs b/src/BootstrapMate.Core/SecretStore.cs new file mode 100644 index 0000000..35692ae --- /dev/null +++ b/src/BootstrapMate.Core/SecretStore.cs @@ -0,0 +1,116 @@ +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; +using Microsoft.Win32; + +namespace BootstrapMate.Core; + +/// +/// Where the headers that carry credentials live: HKLM\SOFTWARE\BootstrapMate\Secrets, with +/// its own ACL (SYSTEM and Administrators full control, nobody else). The policy key and the +/// settings key are readable by every user, so a header that arrives there is moved here +/// and the readable copy blanked. +/// +/// +/// Only an elevated process can read or write the store. A standard user's process reads +/// nothing and treats the header as unset. Values are never logged. +/// +[SupportedOSPlatform("windows")] +public static class SecretStore +{ + public const string RegistryPath = @"SOFTWARE\BootstrapMate\Secrets"; + + /// The settings that hold credentials. + public static readonly string[] SecretNames = ["AuthorizationHeader", "ReportingHeader"]; + + public static bool IsSecret(string name) => SecretNames.Contains(name, StringComparer.OrdinalIgnoreCase); + + /// The stored value, or null when unset or this process may not read it. + public static string? Read(string name) + { + try + { + using var baseKey = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64); + using var key = baseKey.OpenSubKey(RegistryPath); + return key?.GetValue(name) is string s && s.Length > 0 ? s : null; + } + catch + { + return null; + } + } + + /// Stores , or removes the entry when it is empty. Must run elevated. + public static void Write(string name, string? value) + { + using var key = OpenProtected(); + if (string.IsNullOrEmpty(value)) + key.DeleteValue(name, throwOnMissingValue: false); + else + key.SetValue(name, value, RegistryValueKind.String); + } + + /// + /// Moves every credential header found in a user-readable place (the policy key under + /// any of its aliases, the settings key) into the store and blanks the readable copy. + /// A policy value is set to empty rather than deleted, so the setting still shows as + /// managed. Returns one line per move, naming the setting and place, never the value. + /// Must run elevated, before the configuration is loaded. + /// + public static List MigrateReadableCopies() + { + var notes = new List(); + try + { + using var baseKey = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64); + using (OpenProtected()) { } + + foreach (var name in SecretNames) + { + // Settings first, policy last: policy wins, as it does everywhere else. + using (var settings = baseKey.OpenSubKey(BootstrapMateConstants.MachineSettingsRegistryPath, writable: true)) + { + if (settings?.GetValue(name) is string s && s.Length > 0) + { + Write(name, s); + settings.DeleteValue(name, throwOnMissingValue: false); + notes.Add($"Moved {name} from HKLM\\{BootstrapMateConstants.MachineSettingsRegistryPath} to the protected store"); + } + } + + using var policy = baseKey.OpenSubKey(BootstrapMateConstants.PolicyRegistryPath, writable: true); + if (policy is null) continue; + foreach (var alias in ManagementDetector.AliasesFor(name)) + { + if (policy.GetValue(alias) is string p && p.Length > 0) + { + Write(name, p); + policy.SetValue(alias, string.Empty, RegistryValueKind.String); + notes.Add($"Moved {name} from HKLM\\{BootstrapMateConstants.PolicyRegistryPath}\\{alias} to the protected store and blanked the policy copy"); + } + } + } + } + catch (Exception ex) + { + notes.Add($"Could not move credential headers to the protected store: {ex.Message}"); + } + return notes; + } + + /// Opens the store for writing, creating it, and resets its ACL every time. + private static RegistryKey OpenProtected() + { + using var baseKey = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64); + var key = baseKey.CreateSubKey(RegistryPath, writable: true); + var security = new RegistrySecurity(); + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + foreach (var sid in new[] { WellKnownSidType.LocalSystemSid, WellKnownSidType.BuiltinAdministratorsSid }) + { + security.AddAccessRule(new RegistryAccessRule(new SecurityIdentifier(sid, null), + RegistryRights.FullControl, InheritanceFlags.ContainerInherit, PropagationFlags.None, AccessControlType.Allow)); + } + key.SetAccessControl(security); + return key; + } +} diff --git a/tests/BootstrapMate.Core.Tests/CommandLineRedactionTests.cs b/tests/BootstrapMate.Core.Tests/CommandLineRedactionTests.cs new file mode 100644 index 0000000..2f4e161 --- /dev/null +++ b/tests/BootstrapMate.Core.Tests/CommandLineRedactionTests.cs @@ -0,0 +1,51 @@ +using BootstrapMate.Core; +using Xunit; + +namespace BootstrapMate.Core.Tests; + +public class CommandLineRedactionTests +{ + [Fact] + public void TheValueAfterHeadersIsHidden() => + Assert.Equal(["--url", "https://x/m.json", "--headers", CommandLineRedaction.Mask, "--silent"], + CommandLineRedaction.Redact(["--url", "https://x/m.json", "--headers", "Bearer abc", "--silent"])); + + [Theory] + [InlineData("--headers=Bearer abc", "--headers=")] + [InlineData("--HEADERS:Basic xyz", "--HEADERS:")] + public void AnAttachedValueIsHidden(string arg, string expected) => + Assert.Equal([expected], CommandLineRedaction.Redact([arg])); + + [Fact] + public void AWholeCommandLineIsRedacted() + { + var line = CommandLineRedaction.Redact("\"C:/Program Files/BootstrapMate/managedbootstrapinstall.exe\" --headers \"Bearer secret token\" --silent"); + Assert.DoesNotContain("secret", line); + Assert.Contains("--headers --silent", line); + Assert.StartsWith("\"C:/Program Files/BootstrapMate/managedbootstrapinstall.exe\"", line); + } + + [Fact] + public void ACommandLineWithoutSecretsIsUnchangedInSubstance() => + Assert.Equal("app.exe --silent --no-dialog", CommandLineRedaction.Redact("app.exe --silent --no-dialog")); + + [Theory] + [InlineData("AuthorizationHeader")] + [InlineData("reportingheader")] + public void CredentialSettingsAreSecrets(string name) => Assert.True(SecretStore.IsSecret(name)); + + [Fact] + public void OtherSettingsAreNot() => Assert.False(SecretStore.IsSecret("ManifestUrl")); +} + +public class LegacyRegistryTests +{ + // SOFTWARE\Policies is shared between the 32- and 64-bit views: removing its "32-bit + // copy" deletes the live policy. Only the redirected settings key may be cleaned up. + [Fact] + public void ThePolicyKeyIsNeverTreatedAsAStale32BitCopy() + { + Assert.DoesNotContain(BootstrapMateConstants.PolicyRegistryPath, LegacyRegistry.StaleKeys); + Assert.Equal([BootstrapMateConstants.MachineSettingsRegistryPath], LegacyRegistry.StaleKeys); + } +}