From f432353efa7dd358009eae47c348f2e80d4a32b9 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sun, 30 Aug 2026 13:53:00 +0200 Subject: [PATCH] =?UTF-8?q?fix(security):=20startup=20self-update=20no=20l?= =?UTF-8?q?onger=20trusts=20the=20on-disk=20remote-config.json=20cache=20?= =?UTF-8?q?=E2=80=94=20Program.LoadCachedSelfUpdateContext=20(raw=20deseri?= =?UTF-8?q?alize,=20bypassed=20the=20read-side=20strip)=20is=20removed,=20?= =?UTF-8?q?the=20boot-time=20check=20runs=20with=20allowDowngrade:false=20?= =?UTF-8?q?and=20the=20version-v2.json=20manifest=20hash=20as=20its=20only?= =?UTF-8?q?=20integrity=20gate;=20LatestAgentSha256=20(update-ZIP=20hash,?= =?UTF-8?q?=20ranked=20above=20the=20manifest=20hash=20in=20SelfUpdater)?= =?UTF-8?q?=20joins=20the=20CacheConfig/LoadCachedConfig=20strip=20lists?= =?UTF-8?q?=20so=20neither=20the=20startup=20nor=20the=20FromCache=20runti?= =?UTF-8?q?me=20path=20can=20take=20a=20planted=20ZIP=20hash=20or=20downgr?= =?UTF-8?q?ade=20flag;=20verify+extract=20already=20share=20one=20exclusiv?= =?UTF-8?q?e=20handle=20(1daf631a)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../RemoteConfigServiceCacheHardeningTests.cs | 22 ++++++--- .../Configuration/RemoteConfigService.cs | 9 ++++ .../Monitoring/Runtime/SelfUpdater.cs | 8 ++-- .../Program.ImeMatchingMode.cs | 4 ++ .../AutopilotMonitor.Agent.V2/Program.cs | 45 +++++-------------- 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs index 2cf470af..d2f08227 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs @@ -17,7 +17,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration /// (RemoteConfigService.cs:234-311). The on-disk config cache MUST NOT persist — nor, /// on read, trust — security-sensitive fields: a single MITM during OOBE could otherwise /// plant attacker-controlled values (UnrestrictedMode / AllowAgentDowngrade / - /// LatestAgentExeSha256 / DeviceBlocked / DeviceKillSignal / UnblockAt) that survive every + /// LatestAgentExeSha256 / LatestAgentSha256 / DeviceBlocked / DeviceKillSignal / UnblockAt / + /// MigrateToApiBaseUrl) that survive every /// future cold boot. The strip is defence-in-depth on BOTH paths — CacheConfig (write) /// and LoadCachedConfig (read) — so both are asserted here. /// @@ -59,6 +60,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration UnrestrictedMode = true, AllowAgentDowngrade = true, LatestAgentExeSha256 = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + LatestAgentSha256 = "badc0ffebadc0ffebadc0ffebadc0ffebadc0ffebadc0ffebadc0ffebadc0ffe", DeviceBlocked = true, DeviceKillSignal = true, UnblockAt = new DateTime(2099, 1, 1, 0, 0, 0, DateTimeKind.Utc), @@ -75,13 +77,15 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration Assert.False(persisted.UnrestrictedMode); Assert.False(persisted.AllowAgentDowngrade); Assert.Null(persisted.LatestAgentExeSha256); + Assert.Null(persisted.LatestAgentSha256); Assert.False(persisted.DeviceBlocked); Assert.False(persisted.DeviceKillSignal); Assert.Null(persisted.UnblockAt); Assert.Null(persisted.MigrateToApiBaseUrl); - // The attacker EXE hash / re-home host must not appear anywhere in the persisted bytes. + // The attacker EXE hash / ZIP hash / re-home host must not appear anywhere in the persisted bytes. Assert.DoesNotContain("deadbeef", raw); + Assert.DoesNotContain("badc0ffe", raw); Assert.DoesNotContain("attacker.azurewebsites.net", raw); // Benign fields survive the strip. @@ -93,6 +97,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration Assert.True(config.UnrestrictedMode); Assert.True(config.AllowAgentDowngrade); Assert.Equal("deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", config.LatestAgentExeSha256); + Assert.Equal("badc0ffebadc0ffebadc0ffebadc0ffebadc0ffebadc0ffebadc0ffebadc0ffe", config.LatestAgentSha256); Assert.True(config.DeviceBlocked); Assert.True(config.DeviceKillSignal); Assert.Equal(new DateTime(2099, 1, 1, 0, 0, 0, DateTimeKind.Utc), config.UnblockAt); @@ -105,6 +110,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration [InlineData("UnrestrictedMode")] [InlineData("AllowAgentDowngrade")] [InlineData("LatestAgentExeSha256")] + [InlineData("LatestAgentSha256")] [InlineData("DeviceBlocked")] [InlineData("DeviceKillSignal")] [InlineData("UnblockAt")] @@ -122,6 +128,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration UnrestrictedMode = true, AllowAgentDowngrade = true, LatestAgentExeSha256 = "0011223344556677889900112233445566778899001122334455667788990011", + LatestAgentSha256 = "1122334455667788990011223344556677889900112233445566778899001122", DeviceBlocked = true, DeviceKillSignal = true, UnblockAt = new DateTime(2099, 1, 1, 0, 0, 0, DateTimeKind.Utc), @@ -143,6 +150,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration case "LatestAgentExeSha256": Assert.Null(loaded.LatestAgentExeSha256); break; + case "LatestAgentSha256": + // The update-ZIP hash outranks the version-v2.json manifest hash inside + // SelfUpdater — a planted value would let an attacker ZIP pass verification. + Assert.Null(loaded.LatestAgentSha256); + break; case "DeviceBlocked": Assert.False(loaded.DeviceBlocked); break; @@ -181,8 +193,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration NtpServer = "time.contoso.com", LogLevel = "Debug", DiagnosticsUploadMode = "OnFailure", - // A non-sensitive hash field (ZIP hash) that must NOT be stripped — only the - // EXE hash (LatestAgentExeSha256) is security-sensitive on the cache path. + // Both backend hashes are live-fetch-only — the round-trip must drop them + // while every benign field above survives untouched. LatestAgentSha256 = "cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe", }; @@ -199,7 +211,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration Assert.Equal("time.contoso.com", loaded.NtpServer); Assert.Equal("Debug", loaded.LogLevel); Assert.Equal("OnFailure", loaded.DiagnosticsUploadMode); - Assert.Equal("cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe", loaded.LatestAgentSha256); + Assert.Null(loaded.LatestAgentSha256); } // ── Helpers ───────────────────────────────────────────────────────── diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs index ec62ec10..478fd228 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs @@ -323,6 +323,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration // - AllowAgentDowngrade: gate for installing a lower agent version // - LatestAgentExeSha256: backend-advertised EXE hash for runtime integrity // check; a bad cached value would trigger a force-update to attacker bins + // - LatestAgentSha256: backend-advertised update ZIP hash; SelfUpdater ranks it + // ABOVE the version-v2.json manifest hash, so a planted cached value would + // make an attacker ZIP pass integrity verification (or pin the device by + // failing every genuine update). Without a live value the updater falls + // back to the manifest hash. // - DeviceBlocked/DeviceKillSignal/UnblockAt: kill is only honoured from a // live fetch — a planted cached kill would self-destruct every future // session, a stale cached kill would outlive the admin removing the rule @@ -332,6 +337,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration var liveUnrestricted = config.UnrestrictedMode; var liveAllowDowngrade = config.AllowAgentDowngrade; var liveExeHash = config.LatestAgentExeSha256; + var liveZipHash = config.LatestAgentSha256; var liveBlocked = config.DeviceBlocked; var liveKill = config.DeviceKillSignal; var liveUnblockAt = config.UnblockAt; @@ -339,6 +345,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration config.UnrestrictedMode = false; config.AllowAgentDowngrade = false; config.LatestAgentExeSha256 = null; + config.LatestAgentSha256 = null; config.DeviceBlocked = false; config.DeviceKillSignal = false; config.UnblockAt = null; @@ -347,6 +354,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration config.UnrestrictedMode = liveUnrestricted; config.AllowAgentDowngrade = liveAllowDowngrade; config.LatestAgentExeSha256 = liveExeHash; + config.LatestAgentSha256 = liveZipHash; config.DeviceBlocked = liveBlocked; config.DeviceKillSignal = liveKill; config.UnblockAt = liveUnblockAt; @@ -379,6 +387,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration config.UnrestrictedMode = false; config.AllowAgentDowngrade = false; config.LatestAgentExeSha256 = null; + config.LatestAgentSha256 = null; config.DeviceBlocked = false; config.DeviceKillSignal = false; config.UnblockAt = null; diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs index ad344ee5..57b57358 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs @@ -83,8 +83,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime /// /// Optional: backend-provided SHA-256 hash of the latest agent ZIP. - /// Set from the last AgentConfigResponse before calling CheckAndApplyUpdateAsync. - /// Takes priority over the hash in version-v2.json (separate trust channel). + /// Set only from a LIVE authenticated AgentConfigResponse (never from the on-disk + /// remote-config.json cache — the field is stripped there) before calling + /// CheckAndApplyUpdateAsync. Takes priority over the hash in version-v2.json + /// (separate trust channel); when unset the manifest hash is the integrity gate. /// public static string BackendExpectedSha256 { get; set; } @@ -294,7 +296,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime if (!string.IsNullOrEmpty(BackendExpectedSha256)) { expectedSha256 = BackendExpectedSha256; - log("Self-update: using backend hash for integrity verification (cached config — trusted channel)"); + log("Self-update: using backend hash for integrity verification (live remote config — separate trust channel)"); } else if (!string.IsNullOrEmpty(manifestSha256)) { diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.ImeMatchingMode.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.ImeMatchingMode.cs index f1272d76..6ef5d4db 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.ImeMatchingMode.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.ImeMatchingMode.cs @@ -40,6 +40,10 @@ namespace AutopilotMonitor.Agent.V2 private const string ImePatternsGuidPattern = @"(?[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12})"; private const string ImePatternsGitHubUrl = "https://raw.githubusercontent.com/okieselbach/Autopilot-Monitor/refs/heads/main/rules/dist/ime-log-patterns.json"; + // Debug-mode-only fallback source for IME patterns (offline --run-ime-matching). The + // runtime never reads this file directly — RemoteConfigService.LoadCachedConfig is the + // only production reader and strips the live-fetch-only security fields. + private const string CachedRemoteConfigPath = @"%ProgramData%\AutopilotMonitor\Config\remote-config.json"; internal static int RunImeMatchingMode(string[] args) { diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs index c8f1aad2..503664ca 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs @@ -35,7 +35,6 @@ namespace AutopilotMonitor.Agent.V2 /// Register ProcessExit → writes clean-exit.marker /// Ensure agent directories exist /// + - /// Load cached remote-config.json → + AllowAgentDowngrade /// — on success restarts the process; on failure continues with current binary /// — reads markers + event log to classify last shutdown /// Resolve TenantId (registry → bootstrap-config.json fallback) @@ -60,7 +59,6 @@ namespace AutopilotMonitor.Agent.V2 private const string DefaultAgentSubdirectory = "Agent"; private const string DefaultStateSubdirectory = "State"; private const string DefaultSpoolSubdirectory = "Spool"; - private const string CachedRemoteConfigPath = @"%ProgramData%\AutopilotMonitor\Config\remote-config.json"; public static int Main(string[] args) { @@ -132,23 +130,27 @@ namespace AutopilotMonitor.Agent.V2 tenantId: "(pre-runtime)", agentVersion: GetAgentVersion()); - // Startup self-update. Legacy parity: cleanup leftover .old files, load cached - // backend hash / downgrade policy, then attempt the update. Failures here never - // abort startup — we prefer to run the current version than delay. + // Startup self-update. Legacy parity: cleanup leftover .old files, then attempt the + // update against the version-v2.json manifest. Failures here never abort startup — + // we prefer to run the current version than delay. SelfUpdater.LogInit(GetAgentVersion()); var agentDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location) ?? string.Empty; SelfUpdater.CleanupPreviousUpdate(agentDir, msg => { if (consoleMode) Console.Out.WriteLine(msg); }); - var allowAgentDowngrade = LoadCachedSelfUpdateContext(); - + // SECURITY: the startup check runs pre-backend, so it deliberately reads NOTHING from + // the on-disk remote-config.json cache. AllowAgentDowngrade and the ZIP hash + // (LatestAgentSha256) are live-fetch-only fields (see RemoteConfigService.CacheConfig) + // — a planted cache file must neither lift the forward-only gate nor replace the + // version-v2.json manifest hash as the ZIP integrity check. Downgrades therefore only + // happen from the runtime trigger after an authenticated fetch. try { SelfUpdater.CheckAndApplyUpdateAsync( currentVersion: GetAgentVersion(), agentDir: agentDir, consoleMode: consoleMode, - allowDowngrade: allowAgentDowngrade).GetAwaiter().GetResult(); + allowDowngrade: false).GetAwaiter().GetResult(); } catch (Exception selfUpdateEx) { @@ -396,33 +398,6 @@ namespace AutopilotMonitor.Agent.V2 }; } - private static bool LoadCachedSelfUpdateContext() - { - try - { - var path = Environment.ExpandEnvironmentVariables(CachedRemoteConfigPath); - if (!File.Exists(path)) return false; - - var json = File.ReadAllText(path); - var cached = Newtonsoft.Json.JsonConvert.DeserializeObject(json); - if (cached == null) return false; - - if (!string.IsNullOrEmpty(cached.LatestAgentSha256)) - { - SelfUpdater.BackendExpectedSha256 = cached.LatestAgentSha256; - SelfUpdater.Log( - $"Self-update: loaded backend integrity hash from cached config (sha256={cached.LatestAgentSha256.Substring(0, Math.Min(12, cached.LatestAgentSha256.Length))}...)"); - } - - return cached.AllowAgentDowngrade; - } - catch (Exception ex) - { - SelfUpdater.Log($"Self-update: cached config read failed: {ex.Message}"); - return false; - } - } - internal static string GetArgValue(string[] args, params string[] names) { if (args == null || args.Length < 2 || names == null) return null;