From 8f10c09a76a5437e090555f97338d1d695db72eb Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 15 Sep 2026 15:22:25 -0400 Subject: [PATCH] Always pin the server, let the registry own pins, and survive a revoke mid-upgrade Found in the Windows VM re-test. - A CA-flagged server certificate (common with a plain openssl self-signed cert) produced an installer package with no pin at all, so agent 1.20 refused the server. The package now always carries the SPKI pin of the certificate PCs receive; the CA flag only skips the trusted-root import. When an uploaded CA-flagged certificate differs from the server's own, both pins are included. The build response carries X-ShopDB-Build-Warning and the installer logs what older agents need. - Pins written into appsettings.json by the installer were merged with the registry, so removing a pin from HKLM never took effect. When the ServerPins value exists in the registry it is now authoritative, even when empty; appsettings pins apply only when it is absent. Set-ShopDBServerPins.ps1 -Clear deletes the value; new -None writes an empty value (no pins). - A script or package download sent with a credential revoked moments earlier got 401 CREDENTIAL_INVALID and failed the command. The agent now retries that download once with the shared key and records the fallback in the result. --- agent/ShopDbEndpointAgent.Core/AgentConfig.cs | 10 +- .../AgentHttpFactory.cs | 93 +++++++++------ .../CommandExecutor.cs | 83 ++++++++++++-- .../ShopDbEndpointAgent.csproj | 6 +- plugins/endpoints/AGENT-CONTRACT.md | 14 ++- plugins/endpoints/api/siteagent.py | 79 ++++++++++--- .../endpoints/client/Set-ShopDBServerPins.ps1 | 60 +++++++--- .../docs/PER-PC-CREDENTIALS-ROLLOUT.md | 20 +++- .../frontend/views/AgentKeySettings.vue | 9 +- .../test_endpoints_siteagent_build.py | 107 ++++++++++++++++-- 10 files changed, 381 insertions(+), 100 deletions(-) diff --git a/agent/ShopDbEndpointAgent.Core/AgentConfig.cs b/agent/ShopDbEndpointAgent.Core/AgentConfig.cs index 39f00a68..9ce09fd5 100644 --- a/agent/ShopDbEndpointAgent.Core/AgentConfig.cs +++ b/agent/ShopDbEndpointAgent.Core/AgentConfig.cs @@ -14,13 +14,15 @@ public sealed class AgentConfig [JsonPropertyName("cadenceseconds")] public int CadenceSeconds { get; set; } = 900; // Legacy pin: SHA-1 thumbprint of the server leaf certificate (hex, no - // separators). Still honoured next to ServerPins; breaks on every cert - // renewal, so prefer an SPKI pin. See AgentHttpFactory. + // separators). Honoured with serverpins only while the registry ServerPins + // value is absent; breaks on every cert renewal, so prefer an SPKI pin. + // See AgentHttpFactory. [JsonPropertyName("servercertthumbprint")] public string? ServerCertThumbprint { get; set; } // Server public key pins: lowercase hex SHA-256 of the leaf certificate's - // SubjectPublicKeyInfo. Merged with HKLM\SOFTWARE\GE\ShopDB ServerPins - // (REG_MULTI_SZ). Any pin configured = the leaf key must match one; chain + // SubjectPublicKeyInfo. Fallback only: HKLM\SOFTWARE\GE\ShopDB ServerPins + // (REG_MULTI_SZ), when it exists in either view, replaces these entirely, + // even when empty. Any pin in effect = the leaf key must match one; chain // and name errors are then ignored. None = normal Windows chain validation. [JsonPropertyName("serverpins")] public List ServerPins { get; set; } = new(); diff --git a/agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs b/agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs index e0ddd1d2..cccf3236 100644 --- a/agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs +++ b/agent/ShopDbEndpointAgent.Core/AgentHttpFactory.cs @@ -172,43 +172,55 @@ public sealed record AgentServerError(string Code, string Message) } } -// Server certificate trust. Pins are hex SHA-256 of the leaf SubjectPublicKeyInfo, -// merged from HKLM\SOFTWARE\GE\ShopDB ServerPins (both views) and appsettings -// serverpins; the legacy SHA-1 servercertthumbprint still counts. With any pin -// configured only a matching leaf is accepted and chain/name errors are ignored -// (self-signed servers on the air gap). Without pins: SslPolicyErrors.None. -// Registry pins re-read at most every RefreshInterval on the handshake path, so a -// GE-Enforce fix or a "next" rotation pin lands without a service restart. +// Server certificate trust. Pins are hex SHA-256 of the leaf SubjectPublicKeyInfo. +// HKLM\SOFTWARE\GE\ShopDB ServerPins is AUTHORITATIVE whenever the value exists +// in either view, even empty (empty = no pins, Windows chain validation), so a +// removed or rotated-out pin really goes away. Only when the value is absent in +// both views do appsettings serverpins and the legacy SHA-1 servercertthumbprint +// apply. With any pin in effect only a matching leaf is accepted and chain/name +// errors are ignored (self-signed servers on the air gap). Without pins: +// SslPolicyErrors.None. Registry re-read at most every RefreshInterval on the +// handshake path, so a GE-Enforce fix or a rotation lands without a restart. public sealed class ServerPinSet { public const string RegistryValue = "ServerPins"; public static readonly TimeSpan RefreshInterval = TimeSpan.FromSeconds(60); - private sealed record Snapshot(HashSet SpkiPins, string RawRegistry) + // Present = the value exists in at least one view. + private sealed record RegistryRead(bool Present, List Values) { - public string Signature => string.Join(",", SpkiPins.OrderBy(p => p, StringComparer.Ordinal)); + public string Raw => Present ? "registry:" + string.Join("|", Values) : "absent"; + } + + private sealed record Snapshot(HashSet SpkiPins, string? LegacyThumbprint, bool FromRegistry, string RawRegistry) + { + public string Signature => (FromRegistry ? "registry:" : "appsettings:") + + string.Join(",", SpkiPins.OrderBy(p => p, StringComparer.Ordinal)) + + (LegacyThumbprint is null ? "" : "+legacy"); + public bool Pinned => SpkiPins.Count > 0 || LegacyThumbprint is not null; + public string Source => FromRegistry ? "registry" : "appsettings.json"; } private readonly HashSet configPins; private readonly string? legacyThumbprint; private readonly ILogger logger; - private readonly Func?> readRegistry; + private readonly Func readRegistry; private readonly object refreshLock = new(); private volatile Snapshot current; private long nextRefreshTicks; private ServerPinSet(HashSet configPins, string? legacyThumbprint, ILogger logger, - Func?> readRegistry) + Func readRegistry) { this.configPins = configPins; this.legacyThumbprint = legacyThumbprint; this.logger = logger; this.readRegistry = readRegistry; - current = Build(readRegistry(false) ?? new List()); + current = Build(readRegistry(false) ?? new RegistryRead(false, new List())); nextRefreshTicks = Environment.TickCount64 + (long)RefreshInterval.TotalMilliseconds; } - public bool IsPinned => Current().SpkiPins.Count > 0 || legacyThumbprint is not null; + public bool IsPinned => Current().Pinned; public static ServerPinSet Load(AgentConfig config, ILogger logger) { @@ -221,11 +233,15 @@ public sealed class ServerPinSet thumbprint = config.ServerCertThumbprint.Replace(":", "").Replace(" ", "").Trim().ToUpperInvariant(); var set = new ServerPinSet(pins, thumbprint, logger, refresh => ReadRegistryPins(logger, refresh)); - var count = set.current.SpkiPins.Count; + var snapshot = set.current; logger.LogInformation( - "Server certificate trust: {Pins} SPKI pin(s){Legacy}; {Mode}.", - count, thumbprint is null ? "" : " + legacy thumbprint", - count > 0 || thumbprint is not null ? "pinned" : "Windows chain validation"); + "Server certificate trust from {Source}: {Pins} SPKI pin(s){Legacy}; {Mode}.", + snapshot.Source, snapshot.SpkiPins.Count, + snapshot.LegacyThumbprint is null ? "" : " + legacy thumbprint", + snapshot.Pinned ? "pinned" : "Windows chain validation"); + if (snapshot.FromRegistry && (pins.Count > 0 || thumbprint is not null)) + logger.LogInformation( + "Registry {Value} is set; ignoring appsettings.json serverpins/servercertthumbprint.", RegistryValue); return set; } @@ -238,33 +254,38 @@ public sealed class ServerPinSet { if (Environment.TickCount64 < nextRefreshTicks) return current; - var raw = readRegistry(true); + var read = readRegistry(true); Interlocked.Exchange(ref nextRefreshTicks, Environment.TickCount64 + (long)RefreshInterval.TotalMilliseconds); var previous = current; - if (raw is null || string.Join("|", raw) == previous.RawRegistry) + if (read is null || read.Raw == previous.RawRegistry) return previous; - var next = Build(raw); + var next = Build(read); current = next; if (next.Signature != previous.Signature) logger.LogInformation( - "Server pins changed: {Count} SPKI pin(s) [{Pins}]{Legacy}; {Mode}.", + "Server pins changed, source {Source}: {Count} SPKI pin(s) [{Pins}]{Legacy}; {Mode}.", + next.Source, next.SpkiPins.Count, string.Join(", ", next.SpkiPins.OrderBy(p => p, StringComparer.Ordinal).Select(p => p[..12])), - legacyThumbprint is null ? "" : " + legacy thumbprint", - next.SpkiPins.Count > 0 || legacyThumbprint is not null ? "pinned" : "Windows chain validation"); + next.LegacyThumbprint is null ? "" : " + legacy thumbprint", + next.Pinned ? "pinned" : "Windows chain validation"); return next; } } // Malformed-pin warnings only fire when the raw registry text changes. - private Snapshot Build(List raw) + private Snapshot Build(RegistryRead read) { - var pins = new HashSet(configPins, StringComparer.Ordinal); - foreach (var pin in raw) + if (!read.Present) + return new Snapshot(new HashSet(configPins, StringComparer.Ordinal), + legacyThumbprint, FromRegistry: false, read.Raw); + + var pins = new HashSet(StringComparer.Ordinal); + foreach (var pin in read.Values) AddPin(pins, pin, "registry", logger); - return new Snapshot(pins, string.Join("|", raw)); + return new Snapshot(pins, LegacyThumbprint: null, FromRegistry: true, read.Raw); } public bool Validate(X509Certificate? certificate, SslPolicyErrors errors, AgentHttpChannel channel) @@ -276,7 +297,7 @@ public sealed class ServerPinSet } var snapshot = Current(); - if (snapshot.SpkiPins.Count == 0 && legacyThumbprint is null) + if (!snapshot.Pinned) { if (errors == SslPolicyErrors.None) return true; @@ -293,8 +314,8 @@ public sealed class ServerPinSet var spki = SpkiSha256(leaf); if (snapshot.SpkiPins.Contains(spki)) return true; - if (legacyThumbprint is not null - && string.Equals(leaf.GetCertHashString(), legacyThumbprint, StringComparison.OrdinalIgnoreCase)) + if (snapshot.LegacyThumbprint is not null + && string.Equals(leaf.GetCertHashString(), snapshot.LegacyThumbprint, StringComparison.OrdinalIgnoreCase)) return true; logger.LogError( @@ -327,9 +348,10 @@ public sealed class ServerPinSet } // Startup keeps whatever views read. A refresh returns null on any failed view - // so a transient denial cannot silently drop pins (and the server with them). - private static List? ReadRegistryPins(ILogger logger, bool refresh) + // so a transient denial cannot silently swap the pin source (and the server with it). + private static RegistryRead? ReadRegistryPins(ILogger logger, bool refresh) { + var present = false; var found = new List(); foreach (var view in new[] { RegistryView.Registry64, RegistryView.Registry32 }) { @@ -337,7 +359,10 @@ public sealed class ServerPinSet { using var machine = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, view); using var key = machine.OpenSubKey(AgentConfigProvider.RegistryPath); - switch (key?.GetValue(RegistryValue)) + var value = key?.GetValue(RegistryValue); + if (value is not null) + present = true; + switch (value) { case string[] many: found.AddRange(many); @@ -357,6 +382,6 @@ public sealed class ServerPinSet logger.LogWarning(error, "Registry read of {Value} in {View} view failed", RegistryValue, view); } } - return found; + return new RegistryRead(present, found); } } diff --git a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs index c38ad02a..67b82f31 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs @@ -145,10 +145,16 @@ public sealed class CommandExecutor // installer pinned by SHA-256 and runs it silently. Both download // the body themselves with the collector credential and fail closed. case "run_script": + { + var auth = new DownloadAuth(); return RequireTarget(action, target) - ?? await RunScript(target, claim.Script, token); + ?? auth.Stamp(await RunScript(target, claim.Script, auth, token)); + } case "update_app": - return await UpdateApp(claim.App, token); + { + var auth = new DownloadAuth(); + return auth.Stamp(await UpdateApp(claim.App, auth, token)); + } // Fixed destructive action: reboot this endpoint immediately. Runs // as the SYSTEM agent, so shutdown does not need a user session. @@ -1158,7 +1164,7 @@ public sealed class CommandExecutor // is to trigger something already installed on the endpoint, use // scheduledtask_run instead: it is a much smaller grant. private async Task RunScript( - string name, ScriptDeliveryClaim? script, CancellationToken token) + string name, ScriptDeliveryClaim? script, DownloadAuth auth, CancellationToken token) { if (script is null || string.IsNullOrWhiteSpace(script.InstallPath)) { @@ -1211,10 +1217,7 @@ public sealed class CommandExecutor using var client = httpFactory.CreateClient( AgentHttpChannel.Download, TimeSpan.FromMinutes(5)); - using var httpRequest = new HttpRequestMessage(HttpMethod.Get, scriptUrl); - - using var response = await client.SendAsync( - httpRequest, HttpCompletionOption.ResponseHeadersRead, token); + using var response = await SendDownloadAsync(client, scriptUrl, "run_script", auth, token); if (!response.IsSuccessStatusCode) { return new CommandResult @@ -1872,7 +1875,8 @@ public sealed class CommandExecutor // credential, verifies the SHA-256 (when the server pinned one), then runs // an MSI via msiexec /qn or an EXE with the version's SilentArgs. Anything // missing or mismatched fails closed. - private async Task UpdateApp(AppDeliveryClaim? app, CancellationToken token) + private async Task UpdateApp( + AppDeliveryClaim? app, DownloadAuth auth, CancellationToken token) { if (app is null || string.IsNullOrWhiteSpace(app.InstallPath)) { @@ -1910,10 +1914,7 @@ public sealed class CommandExecutor using var client = httpFactory.CreateClient( AgentHttpChannel.Download, TimeSpan.FromMinutes(10)); - using var httpRequest = new HttpRequestMessage(HttpMethod.Get, packageUrl); - - using var response = await client.SendAsync( - httpRequest, HttpCompletionOption.ResponseHeadersRead, token); + using var response = await SendDownloadAsync(client, packageUrl, "update_app", auth, token); if (!response.IsSuccessStatusCode) { return new CommandResult @@ -2010,6 +2011,64 @@ public sealed class CommandExecutor } } + // What a download had to do about a revoked credential, stamped onto the + // command result (message suffix + detail.authfallback) however it ends. + private sealed class DownloadAuth + { + public string? Note { get; set; } + + public CommandResult Stamp(CommandResult result) + { + if (Note is null) + return result; + result.Message = $"{result.Message} ({Note})"; + result.Detail ??= new Dictionary(); + result.Detail["authfallback"] = Note; + return result; + } + } + + // GET for run_script/update_app bodies. A command claimed before a revoke + // downloads with the revoked credential and gets 401 CREDENTIAL_INVALID; the + // auth handler has already deleted it and woken enrollment. Resend ONCE, + // which picks up the shared key (or a credential enrolled meanwhile). + private async Task SendDownloadAsync( + HttpClient client, string url, string action, DownloadAuth auth, CancellationToken token) + { + using var request = new HttpRequestMessage(HttpMethod.Get, url); + var response = await client.SendAsync( + request, HttpCompletionOption.ResponseHeadersRead, token); + if (response.StatusCode != System.Net.HttpStatusCode.Unauthorized + || !request.Headers.Contains(AgentAuthHandler.CredentialHeader)) + return response; + + // Buffered by the auth handler on 401. + var body = await response.Content.ReadAsStringAsync(token); + if (AgentServerError.TryParse(body)?.Code != AgentAuthErrorCodes.CredentialInvalid) + return response; + + var credentials = httpFactory.Credentials; + if (credentials.Current is null && credentials.SharedKey is null) + { + auth.Note = "endpoint credential was revoked and no shared key is configured to retry the download; " + + "the PC must re-enroll"; + logger.LogWarning("{Action} download refused the revoked endpoint credential; no shared key to retry with", + action); + return response; + } + + response.Dispose(); + using var retry = new HttpRequestMessage(HttpMethod.Get, url); + var second = await client.SendAsync( + retry, HttpCompletionOption.ResponseHeadersRead, token); + var via = retry.Headers.Contains(AgentAuthHandler.SharedKeyHeader) + ? "the shared key" : "the newly enrolled credential"; + auth.Note = $"endpoint credential was revoked; download retried once with {via}: HTTP {(int)second.StatusCode}"; + logger.LogWarning("{Action} download refused the revoked endpoint credential; retried with {Via}: HTTP {Status}", + action, via, (int)second.StatusCode); + return second; + } + // MSI installs run through msiexec /i /qn (any SilentArgs appended); // an EXE runs directly with its version's SilentArgs (homegrown EXEs vary by // developer, so the switches are per-version server data, not guessed here). diff --git a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj index b173074e..1cfc1b15 100644 --- a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj +++ b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj @@ -274,8 +274,10 @@ fallback until the PC is enrolled, so the build also runs against an older server (a 404 on enroll parks enrollment for 6 hours). A revoked credential (CREDENTIAL_INVALID) is deleted and the PC re-enrolls without - a restart. Server trust moves to SPKI SHA-256 pins (ServerPins registry - value and serverpins in appsettings.json), checked by one AgentHttpFactory + a restart, and a script or package download refused that way retries + once. Server trust moves to SPKI SHA-256 pins (the ServerPins registry + value when it exists, even empty; otherwise serverpins in + appsettings.json), checked by one AgentHttpFactory that now builds every HttpClient, including script and package downloads that previously had their own unpinned handlers. The tray holds no secret any more: it reaches the service over the local named pipe diff --git a/plugins/endpoints/AGENT-CONTRACT.md b/plugins/endpoints/AGENT-CONTRACT.md index a25d4b07..a0ed91b8 100644 --- a/plugins/endpoints/AGENT-CONTRACT.md +++ b/plugins/endpoints/AGENT-CONTRACT.md @@ -641,7 +641,7 @@ All use the standard error envelope (`data.error.code`). | HTTP | Code | Meaning | Agent action | |---|---|---|---| -| 401 | `CREDENTIAL_INVALID` | unknown or revoked credential | delete credential.bin, fall back to the shared key, wake enrollment | +| 401 | `CREDENTIAL_INVALID` | unknown or revoked credential | delete credential.bin, fall back to the shared key, wake enrollment; a `scriptbody` / `apppackage` download retries once (shared key, or a credential enrolled meanwhile) and the command result carries `detail.authfallback`; with no shared key the command fails saying so | | 401 | `SHARED_KEY_REFUSED` | shared key sent, but this PC is enrolled or the shared key is switched off | wake enrollment; use the credential | | 403 | `ASSET_MISMATCH` | the credential's asset differs from the hostname/assetid named in the request | log; do not retry the same request | @@ -674,8 +674,16 @@ for the SYSTEM reporter scripts. ### 9.5 Server pins - `HKLM\SOFTWARE\GE\ShopDB` `ServerPins` (REG_MULTI_SZ, both registry views) - merged with `serverpins` (string array) in appsettings.json. Legacy - `servercertthumbprint` is still honoured. + is authoritative whenever the value EXISTS in either view, even empty: its + pins (possibly none) are the whole set and `serverpins` / legacy + `servercertthumbprint` in appsettings.json are ignored. Only when the value + is absent in both views do the appsettings.json pins apply. The agent logs + which source is in effect (`registry` or `appsettings.json`) at startup and + on every change. +- `Set-ShopDBServerPins.ps1 -Pins ` writes the set; `-None` writes an + empty value (registry authoritative, no pins, normal chain validation); + `-Clear` deletes the value (appsettings.json pins apply again). To retire an + old pin after a rotation, write only the new pin. - A pin is the lowercase hex SHA-256 of the server leaf certificate's SubjectPublicKeyInfo. The Server Certificate settings page shows it (`spkisha256` on `GET /api/endpoints/settings/server-cert`); diff --git a/plugins/endpoints/api/siteagent.py b/plugins/endpoints/api/siteagent.py index 9402e91f..6e1d1185 100644 --- a/plugins/endpoints/api/siteagent.py +++ b/plugins/endpoints/api/siteagent.py @@ -150,17 +150,42 @@ def _leaf_certificate_facts(pembytes): return _spki_sha256(block), isca -def _current_server_pin(): - """Pin of this server's own leaf certificate, or None when not found/CA.""" +def _current_server_facts(): + """(spkisha256, isca) of this server's own leaf certificate, else (None, None).""" path = _server_cert_path() if not path: - return None + return None, None try: with open(path, 'rb') as handle: - pin, isca = _leaf_certificate_facts(handle.read()) + return _leaf_certificate_facts(handle.read()) except OSError: - return None - return None if isca else pin + return None, None + + +def _current_server_pin(): + """Pin of this server's own leaf certificate, or None when not found. + + The SPKI pin does not depend on basicConstraints, so a CA-flagged + self-signed certificate is pinned like any other leaf. + """ + return _current_server_facts()[0] + + +# Shown to the admin at build time (response header and UI) when the pinned +# certificate carries basicConstraints CA:TRUE. +CA_CERT_BUILD_WARNING = ( + 'The server certificate has basicConstraints CA:TRUE, so install.ps1 will ' + 'not import it into LocalMachine\\Root. The package still pins it: agents ' + '1.20 and newer trust the server by pin alone. Agents older than 1.20 and ' + 'PowerShell 5.1 downloads without pin support will not trust this server. ' + 'Regenerate the server certificate in Settings > Endpoints > Server ' + 'certificate (the generator does not set CA), or trust a non-CA ' + 'certificate on those PCs with Install-ShopDBServerCertificate.ps1.') +CA_CERT_BOTH_PINS_WARNING = ( + 'The uploaded CA-flagged certificate does not match this server\'s own ' + 'certificate, so both pins were included in ServerPins: the uploaded ' + 'certificate and the server\'s own certificate.') +CA_WARNING_HEADER = 'X-ShopDB-Build-Warning' # The non-secret runtime config shipped in the package. baseurl and apikey stay @@ -485,6 +510,12 @@ if (-not [string]::IsNullOrWhiteSpace($CertFileName)) {{ }} if ($certIsCa) {{ Write-Warning "server certificate $CertFileName is a CA certificate; not importing it into LocalMachine\\Root." + if ($pinList.Count -gt 0) {{ + Write-Warning "ServerPins were written: agents 1.20 and newer trust the server by pin alone." + }} + Write-Warning "Agents older than 1.20, and PowerShell 5.1 downloads without pin support, will NOT trust this server." + Write-Warning "Fix: regenerate the server certificate with the ShopDB generator (Settings > Endpoints > Server certificate; it does not set CA)," + Write-Warning "or have an admin trust a non-CA server certificate on this PC with Install-ShopDBServerCertificate.ps1." }} else {{ Write-Host " trusting bundled server certificate $CertFileName ..." @@ -889,7 +920,9 @@ def package_siteagent(): Returns an in-memory ZIP with the staged shopdbagent.exe (and tray when present), a non-secret appsettings.json, install.ps1, and README.txt. The collector key is embedded only in install.ps1 as its default parameter and - is never stored or logged. + is never stored or logged. When the pinned certificate is CA-flagged the + response carries an X-ShopDB-Build-Warning header explaining that only + pinning (agents 1.20 and newer) will trust the server. """ if request.form or request.files: source = request.form @@ -959,29 +992,42 @@ def package_siteagent(): certfilename = 'server.crt' if (cert_pem or certpath) else None - # Pin the bundled leaf when there is one; a CA bundle cannot be pinned (the - # agent hashes the leaf) so fall back to this server's own certificate. + # Pin the leaf the server presents: the bundled/uploaded leaf, else this + # server's own certificate. The SPKI pin ignores basicConstraints, so a + # CA-flagged self-signed cert is still pinned; the CA flag only gates the + # Root import. trustcertinroot = True currentpin = None + pinisca = False if certfilename: if cert_pem: bundledbytes = cert_pem.encode('ascii') else: with open(certpath, 'rb') as handle: bundledbytes = handle.read() - bundledpin, bundledisca = _leaf_certificate_facts(bundledbytes) + currentpin, bundledisca = _leaf_certificate_facts(bundledbytes) trustcertinroot = not bundledisca - if not bundledisca: - currentpin = bundledpin + pinisca = bool(bundledisca) + # An uploaded CA-flagged cert may be the server's own self-signed cert or an + # internal CA root sent by mistake. Ship the server's own pin beside it so + # the agent (which accepts any listed pin) works either way. + serverownpin = None + if cert_pem and pinisca: + serverownpin = _current_server_pin() + if serverownpin == currentpin: + serverownpin = None if currentpin is None: - currentpin = _current_server_pin() + currentpin, pinisca = _current_server_facts() + warnings = [CA_CERT_BUILD_WARNING] if (currentpin and pinisca) else [] + if serverownpin: + warnings.append(CA_CERT_BOTH_PINS_WARNING) if nextserverpin and currentpin is None: return error_response( ErrorCodes.VALIDATION_ERROR, 'nextserverpin needs a current server pin; bundle or upload the ' 'server leaf certificate', http_code=400) - serverpins = [pin for pin in (currentpin, nextserverpin) if pin] + serverpins = [pin for pin in (currentpin, serverownpin, nextserverpin) if pin] serverpins = list(dict.fromkeys(serverpins)) appsettings['serverpins'] = serverpins @@ -1020,8 +1066,11 @@ def package_siteagent(): agentversion = agentstore.current_version('agent') download_name = ('shopdb-endpoint-agent-{0}.zip'.format(agentversion) if agentversion else 'shopdb-endpoint-agent.zip') - return send_file( + response = send_file( buffer, mimetype='application/zip', as_attachment=True, download_name=download_name) + if warnings: + response.headers[CA_WARNING_HEADER] = ' '.join(warnings) + return response diff --git a/plugins/endpoints/client/Set-ShopDBServerPins.ps1 b/plugins/endpoints/client/Set-ShopDBServerPins.ps1 index d06bcf78..df8bc04c 100644 --- a/plugins/endpoints/client/Set-ShopDBServerPins.ps1 +++ b/plugins/endpoints/client/Set-ShopDBServerPins.ps1 @@ -19,17 +19,27 @@ requested set first, and nothing is written when they already match, so it is safe every cycle. + THE REGISTRY WINS. When ServerPins exists in either view, even empty, the + agent uses it alone and ignores serverpins / servercertthumbprint in + appsettings.json (the installer and store upgrade write those). Only when + the value is absent in both views do the appsettings pins apply. + ROTATING THE SERVER KEY: pass the old pin AND the new pin until every PC has - both, switch the server certificate, then drop the old pin. A PC holding - only the old pin refuses the new certificate. + both, switch the server certificate, then write ONLY the new pin to retire + the old one. A PC holding only the old pin refuses the new certificate. .PARAMETER Pins One or more pins separated by commas, semicolons or spaces. Colons and case are ignored. Every value must be 64 hex characters or nothing is written. .PARAMETER Clear - Remove ServerPins from both views. Pinning is then off and the normal - Windows certificate checks apply. + Delete ServerPins from both views. The agent then falls back to the pins + in appsettings.json; with none there, normal Windows certificate checks. + +.PARAMETER None + Write an EMPTY ServerPins in both views. The registry stays authoritative + with no pins, so appsettings.json pins are ignored and the normal Windows + certificate checks apply. .PARAMETER LogPath Append a transcript here as well as writing to the pipeline. @@ -42,6 +52,14 @@ .\Set-ShopDBServerPins.ps1 -Pins "," -WhatIf Show what a key rotation would write without touching the registry. +.EXAMPLE + .\Set-ShopDBServerPins.ps1 -Pins + After the server switched certificates: retire the old pin. + +.EXAMPLE + .\Set-ShopDBServerPins.ps1 -None + Turn pinning off on a PC whose appsettings.json still carries pins. + .NOTES Exit codes: 0 written, or already correct @@ -51,6 +69,7 @@ param( [string]$Pins, [switch]$Clear, + [switch]$None, [string]$LogPath ) @@ -69,16 +88,20 @@ function Log { } } +# Returns @{ Present; Pins }. Present = the value exists, even empty. function Get-CurrentPins { param([Microsoft.Win32.RegistryView]$View) + $result = @{ Present = $false; Pins = @() } $base = [Microsoft.Win32.RegistryKey]::OpenBaseKey([Microsoft.Win32.RegistryHive]::LocalMachine, $View) try { $key = $base.OpenSubKey($SubKey) - if (-not $key) { return @() } + if (-not $key) { return $result } try { $value = $key.GetValue($ValueName) - if ($null -eq $value) { return @() } - return @(@($value) | ForEach-Object { ([string]$_).Trim().ToLowerInvariant() } | Where-Object { $_ }) + if ($null -eq $value) { return $result } + $result.Present = $true + $result.Pins = @(@($value) | ForEach-Object { ([string]$_).Trim().ToLowerInvariant() } | Where-Object { $_ }) + return $result } finally { $key.Dispose() } } finally { $base.Dispose() } } @@ -97,9 +120,15 @@ function Test-SamePins { # --- 1. validate the request ------------------------------------------------ $requested = @() -if (-not $Clear) { +if (@(@($Clear.IsPresent, $None.IsPresent, [bool]$Pins) | Where-Object { $_ }).Count -gt 1) { + Log 'ERROR use only one of -Pins, -Clear, -None' + exit 1 +} +if ($None) { + Log 'requested an empty ServerPins (registry authoritative, no pins)' +} elseif (-not $Clear) { if (-not $Pins -or -not $Pins.Trim()) { - Log 'ERROR no -Pins given (use -Clear to remove pinning)' + Log 'ERROR no -Pins given (-Clear deletes the value, -None writes it empty)' exit 1 } foreach ($raw in ($Pins -split '[,;\s]+')) { @@ -128,10 +157,11 @@ $views = @( $failed = $false foreach ($entry in $views) { try { - $current = @(Get-CurrentPins -View $entry.View) + $state = Get-CurrentPins -View $entry.View + $current = @($state.Pins) if ($Clear) { - if ($current.Count -eq 0) { + if (-not $state.Present) { Log "$($entry.Name): no ServerPins; nothing to do" continue } @@ -143,12 +173,12 @@ foreach ($entry in $views) { try { $key.DeleteValue($ValueName, $false) } finally { $key.Dispose() } } } finally { $base.Dispose() } - Log "$($entry.Name): removed ServerPins" + Log "$($entry.Name): deleted ServerPins (appsettings.json pins apply)" } continue } - if (Test-SamePins -Left $current -Right $requested) { + if ($state.Present -and (Test-SamePins -Left $current -Right $requested)) { Log "$($entry.Name): ServerPins already correct; nothing to do" continue } @@ -164,8 +194,8 @@ foreach ($entry in $views) { } finally { $base.Dispose() } # Read back rather than trust SetValue. - $after = @(Get-CurrentPins -View $entry.View) - if (-not (Test-SamePins -Left $after -Right $requested)) { + $after = Get-CurrentPins -View $entry.View + if (-not $after.Present -or -not (Test-SamePins -Left @($after.Pins) -Right $requested)) { Log "ERROR $($entry.Name): ServerPins did not read back as written" $failed = $true continue diff --git a/plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md b/plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md index 87e0a4e3..41ee05ac 100644 --- a/plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md +++ b/plugins/endpoints/docs/PER-PC-CREDENTIALS-ROLLOUT.md @@ -103,6 +103,18 @@ The script compares before it writes, so `Always` costs nothing per cycle. A running agent picks up a changed `ServerPins` within about 60 seconds; no service restart is needed. +**The registry wins.** The installer and the store upgrade also write +`serverpins` into appsettings.json. When `ServerPins` exists in the registry +(either view), even empty, the agent uses only the registry and ignores the +appsettings.json pins; only a PC with no `ServerPins` value falls back to them. +The agent log says which source is in effect (`Server certificate trust from +registry` / `from appsettings.json`, and `Server pins changed, source ...`). + +- `-Pins [,]`: write exactly this set. +- `-None`: write an EMPTY value. No pins, normal Windows certificate checks, + appsettings.json pins ignored. +- `-Clear`: DELETE the value. The appsettings.json pins apply again (if any). + **Gate:** on the pilot, `HKLM\SOFTWARE\GE\ShopDB` and `HKLM\SOFTWARE\WOW6432Node\GE\ShopDB` both hold `ServerPins` (REG_MULTI_SZ) equal to the pin on the settings page, and the next `report-asset-*.log` under @@ -113,7 +125,10 @@ RESPONSE line. certificate a new key (a regenerated or re-issued certificate) changes the pin. Add the NEXT pin everywhere first: put the old AND the new pin in `-Pins` (`","`), wait until every PC has both, only then switch the -certificate on the server, then drop the old pin. Switching the server first +certificate on the server, then retire the old pin by writing ONLY the new pin +to the registry (`-Pins `). Do not use `-Clear` for this: deleting the +value brings back whatever pins appsettings.json carries, which may be the old +one. Switching the server first cuts off every pinned PC, including its ability to receive the fixed pin list through anything that talks to ShopDB. A renewal that keeps the same key keeps the same pin. PCs trusted through the Root store (below) also need the new @@ -312,7 +327,8 @@ enrolled PC. Block re-enrollment FIRST. 4. Verify: the Fleet row reports the old agent version, and the reporter logs show `RESPONSE (shared key)`. 5. Remove the PC from the pin entry's targets only if pinning itself is the - problem (`Set-ShopDBServerPins.ps1 -Clear` removes the value). + problem. `Set-ShopDBServerPins.ps1 -None` turns pinning off outright; + `-Clear` deletes the value, which falls back to any appsettings.json pins. 6. Later, when moving the PC forward again, lift the block: `POST /api/endpoints/enrollment/assets//allow-reenroll` (or the same action in the UI). The PC enrolls on its next attempt once it runs diff --git a/plugins/endpoints/frontend/views/AgentKeySettings.vue b/plugins/endpoints/frontend/views/AgentKeySettings.vue index eedcf221..3941985a 100644 --- a/plugins/endpoints/frontend/views/AgentKeySettings.vue +++ b/plugins/endpoints/frontend/views/AgentKeySettings.vue @@ -357,8 +357,9 @@ For a reverse-proxy or internal-CA server this app cannot see, upload the server's public certificate in PEM format. It is validated, bundled with the package, pinned, and imported into the endpoint's - Trusted Root store by install.ps1 (a CA certificate is - bundled but never imported or pinned). Leave empty when the + Trusted Root store by install.ps1 (a CA-flagged + certificate is bundled and pinned but never imported, so only agents + 1.20 and newer will trust the server). Leave empty when the server uses a publicly trusted certificate. An upload takes precedence over the bundled server certificate above.

@@ -376,6 +377,7 @@

{{ agentmessage }}

+

{{ agentwarning }}

{{ agenterror }}