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.
This commit is contained in:
cproudlock
2026-09-15 15:22:25 -04:00
parent 1c2ba677c3
commit 8f10c09a76
10 changed files with 381 additions and 100 deletions

View File

@@ -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<string> ServerPins { get; set; } = new();

View File

@@ -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<string> SpkiPins, string RawRegistry)
// Present = the value exists in at least one view.
private sealed record RegistryRead(bool Present, List<string> 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<string> 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<string> configPins;
private readonly string? legacyThumbprint;
private readonly ILogger logger;
private readonly Func<bool, List<string>?> readRegistry;
private readonly Func<bool, RegistryRead?> readRegistry;
private readonly object refreshLock = new();
private volatile Snapshot current;
private long nextRefreshTicks;
private ServerPinSet(HashSet<string> configPins, string? legacyThumbprint, ILogger logger,
Func<bool, List<string>?> readRegistry)
Func<bool, RegistryRead?> readRegistry)
{
this.configPins = configPins;
this.legacyThumbprint = legacyThumbprint;
this.logger = logger;
this.readRegistry = readRegistry;
current = Build(readRegistry(false) ?? new List<string>());
current = Build(readRegistry(false) ?? new RegistryRead(false, new List<string>()));
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<string> raw)
private Snapshot Build(RegistryRead read)
{
var pins = new HashSet<string>(configPins, StringComparer.Ordinal);
foreach (var pin in raw)
if (!read.Present)
return new Snapshot(new HashSet<string>(configPins, StringComparer.Ordinal),
legacyThumbprint, FromRegistry: false, read.Raw);
var pins = new HashSet<string>(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<string>? 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<string>();
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);
}
}

View File

@@ -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<CommandResult> 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<CommandResult> UpdateApp(AppDeliveryClaim? app, CancellationToken token)
private async Task<CommandResult> 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<string, object>();
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<HttpResponseMessage> 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 <pkg> /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).

View File

@@ -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