From ea02e50fc3a25c263d0588d6dddd857ddf1b2729 Mon Sep 17 00:00:00 2001 From: Rod Christiansen Date: Mon, 5 Oct 2026 12:56:45 -0700 Subject: [PATCH 1/2] Read settings from the 64-bit registry only, and keep credential headers where users cannot read them Settings: the CLI and the GUI read HKLM\SOFTWARE\BootstrapMate\Settings and the policy key from the 64-bit view only. A stale 32-bit copy of the settings key (WOW6432Node) used to outrank what the GUI and the MSI write; the MSI removes it through a 32-bit component, and the CLI removes it once and logs the value names it removed. The policy key is shared between the two views, so it is never touched. Credentials: AuthorizationHeader and ReportingHeader now live in HKLM\SOFTWARE\BootstrapMate\Secrets, whose ACL admits only SYSTEM and Administrators. A header found in the policy key (under any alias) or the settings key is moved there by an elevated run and the readable copy blanked; the policy value is emptied rather than deleted so it still shows as managed. The GUI saves headers there and no longer passes one on the command line. Command lines are redacted before they reach the log or session.json. --- Logger.cs | 2 +- Program.cs | 24 ++-- SessionLog.cs | 2 +- installer/Product.wxs | 21 ++++ .../ViewModels/PrefsViewModel.cs | 7 +- .../CommandLineRedaction.cs | 63 ++++++++++ src/BootstrapMate.Core/ConfigManager.cs | 26 +++- src/BootstrapMate.Core/LegacyRegistry.cs | 57 +++++++++ src/BootstrapMate.Core/ManagementDetector.cs | 8 +- src/BootstrapMate.Core/SecretStore.cs | 116 ++++++++++++++++++ .../CommandLineRedactionTests.cs | 51 ++++++++ 11 files changed, 356 insertions(+), 21 deletions(-) create mode 100644 src/BootstrapMate.Core/CommandLineRedaction.cs create mode 100644 src/BootstrapMate.Core/LegacyRegistry.cs create mode 100644 src/BootstrapMate.Core/SecretStore.cs create mode 100644 tests/BootstrapMate.Core.Tests/CommandLineRedactionTests.cs 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/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); + } +} From 0c6501e46698ce11ff7ebe02571c3d8b1009849f Mon Sep 17 00:00:00 2001 From: Rod Christiansen Date: Mon, 5 Oct 2026 12:57:15 -0700 Subject: [PATCH 2/2] Document where settings and credential headers are read from --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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.