From 6dd8e8fca1dce76ef0bdd50e3e7f31372e811f98 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 15 Sep 2026 11:28:52 -0400 Subject: [PATCH] Harden the agent's command execution and restore server certificate checks Agent 1.19.0. - With no pinned thumbprint the collector, command and fleet alert clients accepted any server certificate. They now require a valid chain again. Production uses a self-signed certificate, so this build must not reach bays before server pinning ships with it. - SYSTEM script runs read stdout and stderr while the script runs, cap what is kept, and kill the whole process tree after five minutes. A chatty script used to fill the pipe and hang the command channel until a service restart. - Scripts and update_app packages are staged in a directory only SYSTEM and Administrators can touch, under a random name, with the hash checked in memory and again on disk. A standard user could previously swap the file between the hash check and execution. - The user-context transcript lives in a SYSTEM-created directory, and SYSTEM refuses to follow a link when reading or deleting it. - Strict mode is inserted after any param block, #Requires or using lines, keeps line numbers and writes UTF-8 with a BOM. - A user-context run that reports no exit code now fails. - collect_log refuses setup answer files, appsettings files, the agent's own directories, key files and links. - The startup renewal check is capped at 15 seconds and exits non-zero when it wants a restart, so service recovery restarts the agent. --- .../CollectorClient.cs | 12 +- .../ShopDbEndpointAgent.Core/CommandClient.cs | 10 +- .../CommandExecutor.cs | 479 ++++++++++++++---- .../FleetAlertClient.cs | 82 ++- .../InteractiveProcessLauncher.cs | 37 ++ .../ProtectedFileSystem.cs | 285 +++++++++++ .../StrictModePreamble.cs | 293 +++++++++++ agent/ShopDbEndpointAgent/Program.cs | 44 +- .../ShopDbEndpointAgent.csproj | 19 +- 9 files changed, 1130 insertions(+), 131 deletions(-) create mode 100644 agent/ShopDbEndpointAgent.Core/ProtectedFileSystem.cs create mode 100644 agent/ShopDbEndpointAgent.Core/StrictModePreamble.cs diff --git a/agent/ShopDbEndpointAgent.Core/CollectorClient.cs b/agent/ShopDbEndpointAgent.Core/CollectorClient.cs index 6c668398..5789b617 100644 --- a/agent/ShopDbEndpointAgent.Core/CollectorClient.cs +++ b/agent/ShopDbEndpointAgent.Core/CollectorClient.cs @@ -52,7 +52,7 @@ public sealed class CollectorClient : IDisposable { var handler = new HttpClientHandler(); - // Server certificate validation: pinning when configured, otherwise accept all + // Server certificate validation: pinning when configured, otherwise chain validation handler.ServerCertificateCustomValidationCallback = (message, cert, chain, errors) => { if (cert == null) @@ -79,8 +79,14 @@ public sealed class CollectorClient : IDisposable return true; } - // No pinning configured: accept all (insecure, for rollout compatibility) - return true; + // No pinning configured: normal Windows chain validation + if (errors == System.Net.Security.SslPolicyErrors.None) + return true; + + logger.LogWarning( + "Server certificate rejected on collector channel: {Errors} ({Subject})", + errors, cert.Subject); + return false; }; // Attach client certificate if configured diff --git a/agent/ShopDbEndpointAgent.Core/CommandClient.cs b/agent/ShopDbEndpointAgent.Core/CommandClient.cs index 9fe33c3d..e27c31e5 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandClient.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandClient.cs @@ -88,8 +88,14 @@ public sealed class CommandClient : IDisposable return true; } - // No pinning configured: accept all (insecure, for rollout compatibility) - return true; + // No pinning configured: normal Windows chain validation + if (errors == System.Net.Security.SslPolicyErrors.None) + return true; + + logger.LogWarning( + "Server certificate rejected on command channel: {Errors} ({Subject})", + errors, cert.Subject); + return false; }, }; diff --git a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs index a96afbdf..e9d0e4f1 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs @@ -1,6 +1,8 @@ using System.Diagnostics; using System.Globalization; +using System.Security.AccessControl; using System.Security.Cryptography; +using System.Security.Principal; using System.ServiceProcess; using System.Text; using Microsoft.Extensions.Logging; @@ -306,14 +308,38 @@ public sealed class CommandExecutor }; } - var totalBytes = info.Length; - var truncated = totalBytes > MaxLogBytes; - - string content; - using (var stream = new FileStream( - path, FileMode.Open, FileAccess.Read, - FileShare.ReadWrite | FileShare.Delete)) + if (ProtectedFileSystem.IsReparsePoint(path)) { + logger.LogWarning("collect_log refused {Path}: reparse point", path); + return new CommandResult + { + Succeeded = false, + Message = $"{path} is a link or reparse point; only regular files can be collected", + }; + } + + long totalBytes; + bool truncated; + string content; + using (var stream = ProtectedFileSystem.OpenReadNoFollow(path)) + { + // Re-check where the handle really landed. A junction in the + // ancestry or a short 8.3 name would otherwise walk past the + // denylist, which only saw the typed path. + var finalPath = ProtectedFileSystem.FinalPath(stream.SafeFileHandle); + var finalRejection = finalPath is null + ? "Could not resolve the real path of the file" + : DeniedLogPathReason(finalPath); + if (finalRejection != null) + { + logger.LogWarning( + "collect_log refused {Path} (resolves to {FinalPath}): {Reason}", + path, finalPath, finalRejection); + return new CommandResult { Succeeded = false, Message = finalRejection }; + } + + totalBytes = stream.Length; + truncated = totalBytes > MaxLogBytes; if (truncated) { // Seek to the tail. This can land mid-line, so the partial @@ -450,6 +476,57 @@ public sealed class CommandExecutor return "That path is a directory, not a log file"; path = Path.GetFullPath(normalized); + return DeniedLogPathReason(path); + } + + // Files SYSTEM can read that an operator must never be able to pull over the + // wire: deployment answer files (local admin passwords), the agent's own + // configuration and state, and key material. Checked against the normalized + // typed path and again against the handle's final path. + private static string? DeniedLogPathReason(string fullPath) + { + if (fullPath.StartsWith(@"\\", StringComparison.Ordinal)) + return "UNC/network paths are not supported"; + + var windows = Environment.GetFolderPath(Environment.SpecialFolder.Windows); + var programData = Environment.GetFolderPath( + Environment.SpecialFolder.CommonApplicationData); + var programFiles = Environment.GetFolderPath( + Environment.SpecialFolder.ProgramFiles); + var deniedRoots = new[] + { + Path.Combine(windows, "Panther"), + Path.Combine(windows, "System32", "Sysprep"), + Path.Combine(programData, "ShopDB"), + Path.Combine(programFiles, "ShopDB", "EndpointAgent"), + AppContext.BaseDirectory, + }; + foreach (var root in deniedRoots) + { + if (string.IsNullOrWhiteSpace(root) || !Path.IsPathRooted(root)) + continue; + var normalizedRoot = Path.GetFullPath(root).TrimEnd('\\'); + if (fullPath.Equals(normalizedRoot, StringComparison.OrdinalIgnoreCase) + || fullPath.StartsWith(normalizedRoot + "\\", StringComparison.OrdinalIgnoreCase)) + { + return $"Collecting files under {normalizedRoot} is not permitted"; + } + } + + var fileName = Path.GetFileName(fullPath); + var extension = Path.GetExtension(fileName); + if ((fileName.StartsWith("unattend", StringComparison.OrdinalIgnoreCase) + && extension.Equals(".xml", StringComparison.OrdinalIgnoreCase)) + || fileName.Equals("autounattend.xml", StringComparison.OrdinalIgnoreCase) + || (fileName.StartsWith("appsettings", StringComparison.OrdinalIgnoreCase) + && extension.Equals(".json", StringComparison.OrdinalIgnoreCase)) + || extension.Equals(".pfx", StringComparison.OrdinalIgnoreCase) + || extension.Equals(".p12", StringComparison.OrdinalIgnoreCase) + || extension.Equals(".key", StringComparison.OrdinalIgnoreCase)) + { + return $"Collecting {fileName} is not permitted"; + } + return null; } @@ -1043,7 +1120,7 @@ public sealed class CommandExecutor } catch (Exception error) { - logger.LogDebug(error, "Could not kill timed-out schtasks.exe"); + logger.LogDebug(error, "Could not kill timed-out process tree"); } } @@ -1102,11 +1179,32 @@ public sealed class CommandExecutor } var scriptUrl = CombineUrl(config.BaseUrl, script.InstallPath!); - var tempPath = Path.Combine(GetStagingDirectory(), - $"shopdb-script-{script.ScriptId}.ps1"); + var runInUser = string.Equals( + script.RunContext, "user", StringComparison.OrdinalIgnoreCase); + string? tempPath = null; try { + // Resolved before anything is staged: a user-context script and its + // transcript are granted to that one account and nobody else. + SecurityIdentifier? userSid = null; + if (runInUser) + { + userSid = InteractiveProcessLauncher.TryGetActiveUserSid(out var sidError); + if (userSid is null) + { + logger.LogWarning( + "run_script for {Name} could not resolve the interactive user: {Error}", + name, sidError); + return new CommandResult + { + Succeeded = false, + Message = $"Script {name} could not run in the user session: {sidError}", + Detail = ScriptDetail(name, script, scriptPath: null, exitCode: null), + }; + } + } + using var handler = new HttpClientHandler(); using var client = new HttpClient(handler); client.Timeout = TimeSpan.FromMinutes(5); @@ -1125,15 +1223,22 @@ public sealed class CommandExecutor }; } - await using (var source = await response.Content.ReadAsStreamAsync(token)) - await using (var file = File.Create(tempPath)) + // Held in memory, never on disk, until it is verified. A body on + // disk before the check is a body that can be swapped under it. + var body = await ReadBoundedAsync(response.Content, MaxScriptBytes, token); + if (body is null) { - await source.CopyToAsync(file, token); + return new CommandResult + { + Succeeded = false, + Message = $"run_script download refused: script exceeds {MaxScriptBytes / (1024 * 1024)} MB", + Detail = ScriptDetail(name, script, scriptPath: null, exitCode: null), + }; } if (!string.IsNullOrWhiteSpace(script.Sha256)) { - var actual = HashFile(tempPath); + var actual = HashBytes(body); if (!actual.Equals(script.Sha256, StringComparison.OrdinalIgnoreCase)) { logger.LogWarning( @@ -1143,17 +1248,48 @@ public sealed class CommandExecutor { Succeeded = false, Message = "run_script integrity check failed: script SHA-256 does not match", - Detail = ScriptDetail(name, script, tempPath, exitCode: null), + Detail = ScriptDetail(name, script, scriptPath: null, exitCode: null), }; } } - var runInUser = string.Equals( - script.RunContext, "user", StringComparison.OrdinalIgnoreCase); - ApplyErrorMode(script, tempPath, name); - return runInUser - ? RunScriptInUserSession(name, script, tempPath) - : await RunScriptAsSystem(name, script, tempPath, token); + var staged = ApplyErrorMode(script, body, name); + + var stagingDirectory = runInUser + ? ProtectedFileSystem.EnsureDirectory(UserScriptDirectoryName, userSid) + : ProtectedFileSystem.EnsureDirectory(SystemStagingDirectoryName); + var runId = Guid.NewGuid().ToString("N"); + tempPath = Path.Combine(stagingDirectory, + $"shopdb-script-{script.ScriptId}-{runId}.ps1"); + await using (var file = ProtectedFileSystem.CreateFile( + tempPath, userSid, FileSystemRights.ReadAndExecute)) + { + await file.WriteAsync(staged, token); + } + + // The bytes about to execute, not the bytes that were delivered: + // after a strict-mode rewrite these differ, and both are pinned. + var expectedStaged = HashBytes(staged); + var actualStaged = HashFile(tempPath); + if (!actualStaged.Equals(expectedStaged, StringComparison.OrdinalIgnoreCase)) + { + logger.LogWarning( + "run_script staged file for {Name} changed on disk: expected {Expected} got {Actual}", + name, expectedStaged, actualStaged); + return new CommandResult + { + Succeeded = false, + Message = "run_script integrity check failed: staged script changed on disk", + Detail = ScriptDetail(name, script, tempPath, exitCode: null), + }; + } + + if (!runInUser) + return await RunScriptAsSystem(name, script, tempPath, token); + + var transcriptPath = TryCreateTranscript( + stagingDirectory, script.ScriptId, runId, userSid!, name); + return RunScriptInUserSession(name, script, tempPath, transcriptPath); } catch (OperationCanceledException) { @@ -1173,8 +1309,8 @@ public sealed class CommandExecutor { try { - if (File.Exists(tempPath)) - File.Delete(tempPath); + if (tempPath is not null) + ProtectedFileSystem.DeleteFileNoFollow(tempPath); } catch (Exception cleanupError) { @@ -1183,37 +1319,89 @@ public sealed class CommandExecutor } } - // Best-effort. A missing transcript directory must not stop the script from - // running: the capture is diagnostics, and losing diagnostics is a smaller - // failure than refusing to do the work that was asked for. - private void TryEnsureTranscriptDirectory(string transcriptPath, string name) + // Upper bound on a delivered script body. Scripts are text; anything near + // this is not a script, and the body is held in memory until verified. + private const int MaxScriptBytes = 8 * 1024 * 1024; + + // Upper bound on how much of a user-session transcript is read back. Far + // above what MaxCapturedOutputChars keeps, so the cap still reports truncation. + private const int MaxTranscriptBytes = 256 * 1024; + + // Subdirectories of %ProgramData%\ShopDB\EndpointAgent (Defender-excluded) + // created with a protected DACL. Standard users can create files anywhere + // else under ProgramData, so nothing SYSTEM executes or reads back is staged + // outside these. userscripts additionally lets the target operator traverse + // it, because a user-context run executes from there as that operator. + private const string SystemStagingDirectoryName = "staging"; + private const string UserScriptDirectoryName = "userscripts"; + + private static async Task ReadBoundedAsync( + HttpContent content, int maxBytes, CancellationToken token) { + if (content.Headers.ContentLength is long declared && declared > maxBytes) + return null; + + await using var source = await content.ReadAsStreamAsync(token); + using var buffer = new MemoryStream(); + var chunk = new byte[81920]; + int read; + while ((read = await source.ReadAsync(chunk, token)) > 0) + { + if (buffer.Length + read > maxBytes) + return null; + buffer.Write(chunk, 0, read); + } + return buffer.ToArray(); + } + + // The transcript a user-session run redirects into. SYSTEM creates it + // inside the protected directory and grants the operator read/write on this + // one file, so the operator can fill it but cannot swap it for a link that + // would steer the later SYSTEM read and delete. Best-effort: a run without + // a transcript still runs, it just reports no output. + private string? TryCreateTranscript( + string stagingDirectory, int scriptId, string runId, + SecurityIdentifier userSid, string name) + { + var transcriptPath = Path.Combine(stagingDirectory, + $"shopdb-script-{scriptId}-{runId}-output.txt"); try { - var directory = Path.GetDirectoryName(transcriptPath); - if (!string.IsNullOrWhiteSpace(directory)) - Directory.CreateDirectory(directory!); + using (ProtectedFileSystem.CreateFile(transcriptPath, userSid, + FileSystemRights.Read | FileSystemRights.Write)) + { + } + return transcriptPath; } catch (Exception error) { logger.LogWarning( error, - "run_script {Name}: could not prepare the output transcript folder; " + "run_script {Name}: could not prepare the output transcript; " + "the script will still run but its output will not be captured", name); + return null; } } // Read the transcript back and delete it. Deleted whether or not the read - // worked: it sits in the operator's own profile where they could read it, - // and a script's output is not theirs to keep lying around after the agent - // has reported it. - private string ReadAndClearTranscript(string transcriptPath, string name) + // worked, so a script's output does not linger after it has been reported. + // Never follows a reparse point for either operation. + private string ReadAndClearTranscript(string? transcriptPath, string name) { + if (transcriptPath is null) + return ""; + var output = ""; try { - if (File.Exists(transcriptPath)) - output = CombineScriptOutput(File.ReadAllText(transcriptPath), null); + if (ProtectedFileSystem.IsReparsePoint(transcriptPath)) + logger.LogWarning( + "run_script {Name}: output transcript {Path} is a reparse point; not read", + name, transcriptPath); + else if (ProtectedFileSystem.TryGetAttributes(transcriptPath) is not null) + output = CombineScriptOutput( + ProtectedFileSystem.ReadTextHeadNoFollow(transcriptPath, MaxTranscriptBytes), + null); } catch (Exception error) { @@ -1222,8 +1410,7 @@ public sealed class CommandExecutor } try { - if (File.Exists(transcriptPath)) - File.Delete(transcriptPath); + ProtectedFileSystem.DeleteFileNoFollow(transcriptPath); } catch (Exception error) { @@ -1280,36 +1467,46 @@ public sealed class CommandExecutor name, exitCode?.ToString() ?? "unknown", Environment.NewLine, output); } - // Prepended to a strict-mode body. Written on ONE line so every line number - // PowerShell reports still matches the body the author sees in the WebUI - // editor: a multi-line prologue would shift them and make the error text - // point at the wrong statement during exactly the debugging session that - // needed it. + // Inserted, followed by '; ', on the line of the first statement after any + // using / #Requires / comment help / attribute / param header (see + // StrictModePreamble). No newline is added, so line numbers PowerShell + // reports still match the body the author sees in the WebUI editor; only + // columns on that one line shift. private const string StrictModePrologue = "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue'"; // Turn a script's non-terminating errors fatal when the library says strict. // // MUST run AFTER the SHA-256 check. The digest pins the bytes the SERVER - // holds; prepending first would rewrite the file and fail its own integrity - // check. Verify what was delivered, then modify how it is interpreted. + // holds; rewriting first would fail its own integrity check. Verify what was + // delivered, then modify how it is interpreted; the rewritten bytes are + // hashed again once staged. // - // Best-effort by design: if the rewrite fails the script still runs under - // PowerShell's default rules. That is the behaviour it had before this - // existed, so a failed rewrite degrades to "as before" rather than blocking - // work, and the warning says the verdict is the weaker one. - private void ApplyErrorMode(ScriptDeliveryClaim script, string scriptPath, string name) + // Best-effort by design: if the rewrite fails or the header cannot be read + // with confidence, the script runs unmodified under PowerShell's default + // rules. That is the behaviour it had before this existed, and the warning + // says the verdict is the weaker one. + private byte[] ApplyErrorMode(ScriptDeliveryClaim script, byte[] body, string name) { if (!string.Equals(script.ErrorMode, "strict", StringComparison.OrdinalIgnoreCase)) - return; + return body; try { - var body = File.ReadAllText(scriptPath); - File.WriteAllText(scriptPath, StrictModePrologue + Environment.NewLine + body); + var rewritten = StrictModePreamble.TryApply(body, StrictModePrologue, out var reason); + if (rewritten is null) + { + logger.LogWarning( + "run_script {Name}: strict error mode NOT applied ({Reason}); running under " + + "PowerShell defaults, so a non-terminating error will NOT fail the command", + name, reason); + return body; + } + logger.LogInformation( "run_script {Name}: strict error mode applied; a non-terminating " + "error will now stop the script and fail the command", name); + return rewritten; } catch (Exception error) { @@ -1318,6 +1515,7 @@ public sealed class CommandExecutor "run_script {Name}: could not apply strict error mode; running under " + "PowerShell defaults, so a non-terminating error will NOT fail the command", name); + return body; } } @@ -1356,6 +1554,12 @@ public sealed class CommandExecutor // System context: run through the agent's own service account and wait for // the exit code, the same as before. Fine for machine-scoped scripts that do // not need the interactive user's session resources. + // + // Both streams are drained WHILE the script runs. Reading only after exit + // deadlocks as soon as a script fills a pipe buffer (it blocks on write, we + // block on exit), and ReadToEnd would hold unbounded output in memory. The + // run is bounded by SystemScriptTimeout; on expiry the whole process tree is + // killed, since a SYSTEM PowerShell left running is worse than a failed run. private async Task RunScriptAsSystem( string name, ScriptDeliveryClaim script, string scriptPath, CancellationToken token) { @@ -1399,14 +1603,46 @@ public sealed class CommandExecutor }; } - await process.WaitForExitAsync(token); - // Read only AFTER exit, for the reason spelled out on the schtasks path: - // ReadToEnd blocks until the streams close, which IS process exit, so - // reading first would make the caller's cancellation unable to fire. - var stdout = await process.StandardOutput.ReadToEndAsync(token); - var stderr = await process.StandardError.ReadToEndAsync(token); + var stdout = new BoundedCapture(MaxCapturedOutputChars + 1); + var stderr = new BoundedCapture(MaxCapturedOutputChars + 1); + var pumps = Task.WhenAll( + stdout.PumpAsync(process.StandardOutput), + stderr.PumpAsync(process.StandardError)); + + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(token); + timeout.CancelAfter(SystemScriptTimeout); + try + { + await process.WaitForExitAsync(timeout.Token); + } + catch (OperationCanceledException) + { + TryKill(process); + await DrainAsync(pumps); + if (token.IsCancellationRequested) + throw; + + var partial = CombineScriptOutput(stdout.ToString(), stderr.ToString()); + LogScriptOutput(name, partial, exitCode: null); + logger.LogWarning( + "run_script {Name} did not finish within {Timeout}; process tree killed", + name, SystemScriptTimeout); + return new CommandResult + { + Succeeded = false, + Message = $"Script {name} did not finish within " + + $"{SystemScriptTimeout.TotalMinutes:0} minutes; the script and its " + + "child processes were killed", + Detail = ScriptDetail(name, script, scriptPath, exitCode: null, + workingDirectory, partial), + }; + } + + // A grandchild that inherited the pipe can hold it open past exit, so + // the final drain is bounded too. + await DrainAsync(pumps); var exit = process.ExitCode; - var output = CombineScriptOutput(stdout, stderr); + var output = CombineScriptOutput(stdout.ToString(), stderr.ToString()); LogScriptOutput(name, output, exit); return new CommandResult { @@ -1419,6 +1655,54 @@ public sealed class CommandExecutor }; } + // The descriptor carries no per-script timeout, so SYSTEM runs get the same + // budget as user-session runs. + private static readonly TimeSpan SystemScriptTimeout = TimeSpan.FromMinutes(5); + + private static readonly TimeSpan StreamDrainTimeout = TimeSpan.FromSeconds(10); + + private static async Task DrainAsync(Task pumps) + => await Task.WhenAny(pumps, Task.Delay(StreamDrainTimeout)); + + // Reads a redirected stream to EOF, keeping at most `limit` characters and + // discarding the rest so the pipe never fills and memory stays bounded. + private sealed class BoundedCapture + { + private readonly StringBuilder buffer = new(); + private readonly object gate = new(); + private readonly int limit; + + public BoundedCapture(int limit) => this.limit = limit; + + public Task PumpAsync(StreamReader reader) => Task.Run(async () => + { + var chunk = new char[4096]; + try + { + int read; + while ((read = await reader.ReadAsync(chunk, 0, chunk.Length)) > 0) + { + lock (gate) + { + var room = limit - buffer.Length; + if (room > 0) + buffer.Append(chunk, 0, Math.Min(room, read)); + } + } + } + catch (Exception) + { + // Pipe torn down by a kill; keep what was captured. + } + }); + + public override string ToString() + { + lock (gate) + return buffer.ToString(); + } + } + // User context: launch PowerShell in the ACTIVE CONSOLE session so the script // runs at the interactive user's integrity and sees that user's resources // (mapped network drives, HKCU, profile). The launcher waits for the process @@ -1440,41 +1724,35 @@ public sealed class CommandExecutor // duplication). So PowerShell redirects to a file itself and the agent reads // it back after exit. // - // The file lands in the OPERATOR's profile, not the agent's ProgramData - // staging folder: staging is SYSTEM-owned and the signed-in operator cannot - // write there, which is the same constraint that put the tray's own log in - // the profile. Safe to rely on here because a user-context script only runs - // when someone is interactively signed in, and a signed-in account has a - // materialised profile by definition. - private static string UserTranscriptPath(int scriptId) - => Path.Combine( - FileLoggerProvider.UserProfileLogDirectory(), - $"shopdb-script-{scriptId}-output.txt"); - + // The file is created by SYSTEM in the protected userscripts directory and + // only that file is granted to the operator (see TryCreateTranscript). It + // used to live in the operator's own profile, where the operator could + // replace it with a junction or link and steer SYSTEM's read and delete. private CommandResult RunScriptInUserSession( - string name, ScriptDeliveryClaim script, string scriptPath) + string name, ScriptDeliveryClaim script, string scriptPath, string? transcriptPath) { var quotedPath = "\"" + scriptPath + "\""; // Redirect inside the user session rather than from the service. The // child is launched through CreateProcessAsUser, so the agent holds no // handles to its streams; -Command wraps the same -File invocation and - // sends both streams to a file the operator can write, which the agent - // reads back after exit. 2>&1 merges stderr into the same transcript - // because ordering between the two matters more than telling them apart - // when reconstructing what a script did. - var transcriptPath = UserTranscriptPath(script.ScriptId); - var quotedTranscript = "\"" + transcriptPath + "\""; - TryEnsureTranscriptDirectory(transcriptPath, name); + // sends every stream to the transcript the operator was granted, which + // the agent reads back after exit. *> merges them because ordering + // matters more than telling them apart when reconstructing what a + // script did. + var redirect = transcriptPath is null + ? "" + : " *> " + ("\"" + transcriptPath + "\"").Replace("\"", "\\\""); var arguments = "-NoProfile -NonInteractive -ExecutionPolicy Bypass -Command " + "\"& { & powershell.exe -NoProfile -NonInteractive -ExecutionPolicy " + "Bypass -File " + quotedPath.Replace("\"", "\\\"") - + " *> " + quotedTranscript.Replace("\"", "\\\"") + "; exit $LASTEXITCODE }\""; + + redirect + "; exit $LASTEXITCODE }\""; if (!TryResolveWorkingDirectory( script, scriptPath, out var workingDirectory, out var directoryError)) { logger.LogWarning( "run_script for {Name} not started: {Error}", name, directoryError); + ReadAndClearTranscript(transcriptPath, name); return new CommandResult { Succeeded = false, @@ -1518,15 +1796,16 @@ public sealed class CommandExecutor }; } + // Launched and waited on, yet no exit code: nothing proves it worked. var exit = outcome.ExitCode; - var ok = exit is null || exit == 0; + var ok = exit == 0; var output = ReadAndClearTranscript(transcriptPath, name); LogScriptOutput(name, output, exit); return new CommandResult { Succeeded = ok, Message = exit is null - ? $"Script {name} launched in the interactive user session" + ? $"Script {name} failed in the interactive user session: no exit code reported" : ok ? $"Script {name} completed (user context)" : $"Script {name} exited with code {exit}", @@ -1617,11 +1896,16 @@ public sealed class CommandExecutor var packageUrl = CombineUrl(config.BaseUrl, app.InstallPath!); var ext = Path.GetExtension(app.InstallPath) ?? ""; var packageKey = app.AppVersionId?.ToString() ?? $"app{app.AppId}"; - var tempPath = Path.Combine(GetStagingDirectory(), - $"shopdb-app-{packageKey}{ext}"); + string? tempPath = null; try { + // Protected directory and a random name, so no standard user can + // pre-place or swap the package between the hash check and msiexec. + tempPath = Path.Combine( + ProtectedFileSystem.EnsureDirectory(SystemStagingDirectoryName), + $"shopdb-app-{packageKey}-{Guid.NewGuid():N}{ext}"); + using var handler = new HttpClientHandler(); using var client = new HttpClient(handler); client.Timeout = TimeSpan.FromMinutes(10); @@ -1641,7 +1925,7 @@ public sealed class CommandExecutor } await using (var source = await response.Content.ReadAsStreamAsync(token)) - await using (var file = File.Create(tempPath)) + await using (var file = ProtectedFileSystem.CreateFile(tempPath)) { await source.CopyToAsync(file, token); } @@ -1716,8 +2000,8 @@ public sealed class CommandExecutor { try { - if (File.Exists(tempPath)) - File.Delete(tempPath); + if (tempPath is not null) + ProtectedFileSystem.DeleteFileNoFollow(tempPath); } catch (Exception cleanupError) { @@ -1743,28 +2027,17 @@ public sealed class CommandExecutor return (packagePath, extra); } - // Fixed, known staging folder we control and that is Defender-excluded in - // the environment. LocalSystem's Path.GetTempPath() resolves to - // C:\Windows\SystemTemp on Win11/Server2022+, which we do not use; stage - // downloads here instead so run_script and update_app land in a predictable - // spot. Created on demand. - private static string GetStagingDirectory() - { - var programData = Environment.GetFolderPath( - Environment.SpecialFolder.CommonApplicationData); - var staging = Path.Combine(programData, "ShopDB", "EndpointAgent"); - Directory.CreateDirectory(staging); - return staging; - } - private static string HashFile(string path) { - using var stream = File.OpenRead(path); + using var stream = ProtectedFileSystem.OpenReadNoFollow(path); using var sha = SHA256.Create(); var hash = sha.ComputeHash(stream); return Convert.ToHexString(hash).ToLowerInvariant(); } + private static string HashBytes(byte[] bytes) + => Convert.ToHexString(SHA256.HashData(bytes)).ToLowerInvariant(); + // Join the agent base URL to a ShopDB-relative package path, tolerating a // trailing slash on the base and a leading slash on the path. private static string CombineUrl(string baseUrl, string path) diff --git a/agent/ShopDbEndpointAgent.Core/FleetAlertClient.cs b/agent/ShopDbEndpointAgent.Core/FleetAlertClient.cs index 48a80da1..5d45dbab 100644 --- a/agent/ShopDbEndpointAgent.Core/FleetAlertClient.cs +++ b/agent/ShopDbEndpointAgent.Core/FleetAlertClient.cs @@ -76,8 +76,14 @@ public sealed class FleetAlertClient : IDisposable return true; } - // No pinning configured: accept all (insecure, for rollout compatibility) - return true; + // No pinning configured: normal Windows chain validation + if (errors == System.Net.Security.SslPolicyErrors.None) + return true; + + logger.LogWarning( + "Server certificate rejected on fleet alert channel: {Errors} ({Subject})", + errors, cert.Subject); + return false; }, }; @@ -158,18 +164,78 @@ public sealed class FleetAlertClient : IDisposable public void Dispose() => http.Dispose(); } +// Server-sent strings may be JSON null; setters coerce to "" so consumers +// never see null. Timeout null or <= 0 means no timeout. public sealed class FleetAlert { + private string typeName = ""; + private string typeColor = ""; + private string title = ""; + private string message = ""; + [JsonPropertyName("notificationId")] public int NotificationId { get; set; } - [JsonPropertyName("type")] public string TypeName { get; set; } = ""; - [JsonPropertyName("typeColor")] public string TypeColor { get; set; } = ""; - [JsonPropertyName("title")] public string Title { get; set; } = ""; - [JsonPropertyName("message")] public string Message { get; set; } = ""; - [JsonPropertyName("timeout")] public int Timeout { get; set; } + [JsonPropertyName("type")] public string TypeName { get => typeName; set => typeName = value ?? ""; } + [JsonPropertyName("typeColor")] public string TypeColor { get => typeColor; set => typeColor = value ?? ""; } + [JsonPropertyName("title")] public string Title { get => title; set => title = value ?? ""; } + [JsonPropertyName("message")] public string Message { get => message; set => message = value ?? ""; } + [JsonPropertyName("timeout")] public int? Timeout { get; set; } + + [JsonIgnore] public int TimeoutMinutes => Timeout is > 0 ? Timeout.Value : 0; } public sealed class FleetAlertEnvelope { [JsonPropertyName("status")] public string Status { get; set; } = ""; - [JsonPropertyName("data")] public List? Data { get; set; } + + [JsonPropertyName("data")] + [JsonConverter(typeof(TolerantFleetAlertListConverter))] + public List? Data { get; set; } +} + +// List of alerts that parsed, plus a note per element that did not. The +// caller logs Skipped; one malformed alert must not drop the rest. +public sealed class FleetAlertList : List +{ + public List Skipped { get; } = new(); +} + +// Deserializes the data array one element at a time. An element that fails +// to bind, or has no usable notificationId, is recorded and skipped. +public sealed class TolerantFleetAlertListConverter : JsonConverter> +{ + public override List? Read( + ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + return null; + if (reader.TokenType != JsonTokenType.StartArray) + throw new JsonException("Fleet alert data is not an array."); + + var list = new FleetAlertList(); + var index = 0; + while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + using var element = JsonDocument.ParseValue(ref reader); + try + { + var alert = element.RootElement.Deserialize(options); + if (alert is null || alert.NotificationId <= 0) + list.Skipped.Add($"element {index}: missing notificationId"); + else + list.Add(alert); + } + catch (Exception exception) when (exception is JsonException or InvalidOperationException) + { + list.Skipped.Add($"element {index}: {exception.Message}"); + } + index++; + } + return list; + } + + public override void Write( + Utf8JsonWriter writer, List value, JsonSerializerOptions options) + { + JsonSerializer.Serialize(writer, value.ToArray(), options); + } } diff --git a/agent/ShopDbEndpointAgent.Core/InteractiveProcessLauncher.cs b/agent/ShopDbEndpointAgent.Core/InteractiveProcessLauncher.cs index b9c45755..ac66f4a5 100644 --- a/agent/ShopDbEndpointAgent.Core/InteractiveProcessLauncher.cs +++ b/agent/ShopDbEndpointAgent.Core/InteractiveProcessLauncher.cs @@ -1,5 +1,6 @@ using System.ComponentModel; using System.Runtime.InteropServices; +using System.Security.Principal; using System.Text; namespace ShopDbEndpointAgent.Core; @@ -236,6 +237,42 @@ public static class InteractiveProcessLauncher } } + // SID of the active console user, so SYSTEM can grant that one account + // access to a file it stages. Null with a reason when nobody is signed in. + public static SecurityIdentifier? TryGetActiveUserSid(out string? error) + { + var sessionId = WTSGetActiveConsoleSessionId(); + if (sessionId == InvalidSessionId) + { + error = "No active console session (no interactive user is signed in)."; + return null; + } + + if (!WTSQueryUserToken(sessionId, out var userToken)) + { + error = "Could not obtain the interactive user token (WTSQueryUserToken failed): " + + new Win32Exception(Marshal.GetLastWin32Error()).Message; + return null; + } + + try + { + using var identity = new WindowsIdentity(userToken); + error = identity.User is null ? "Interactive user token carries no SID." : null; + return identity.User; + } + catch (Exception exception) + { + error = "Could not read the interactive user SID: " + exception.Message; + return null; + } + finally + { + if (userToken != IntPtr.Zero) + CloseHandle(userToken); + } + } + private enum SecurityImpersonationLevel { SecurityAnonymous, diff --git a/agent/ShopDbEndpointAgent.Core/ProtectedFileSystem.cs b/agent/ShopDbEndpointAgent.Core/ProtectedFileSystem.cs new file mode 100644 index 00000000..af63ff07 --- /dev/null +++ b/agent/ShopDbEndpointAgent.Core/ProtectedFileSystem.cs @@ -0,0 +1,285 @@ +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Text; +using Microsoft.Win32.SafeHandles; + +namespace ShopDbEndpointAgent.Core; + +// File access for paths the agent touches AS SYSTEM where a standard user could +// otherwise interfere. +// +// %ProgramData% lets any user create files and folders beneath it, so a staging +// path the agent does not lock down can be pre-created, swapped between the hash +// check and execution, or replaced with a junction that redirects a SYSTEM read +// or delete to an arbitrary file. Directories made here carry a protected DACL: +// SYSTEM and BUILTIN\Administrators only, inheritance off. SIDs are well-known +// strings so a localized OS resolves them the same way. +internal static class ProtectedFileSystem +{ + private static readonly SecurityIdentifier LocalSystemSid = new("S-1-5-18"); + private static readonly SecurityIdentifier AdministratorsSid = new("S-1-5-32-544"); + + private const uint GenericRead = 0x80000000; + private const uint FileShareRead = 0x00000001; + private const uint FileShareWrite = 0x00000002; + private const uint FileShareDelete = 0x00000004; + private const uint OpenExisting = 3; + private const uint FileFlagOpenReparsePoint = 0x00200000; + + // %ProgramData%\ShopDB\EndpointAgent. The Defender-excluded agent root. + public static string AgentDataRoot => Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + "ShopDB", "EndpointAgent"); + + // Create or re-secure \. readerSid, when set, gets + // list/traverse on the directory itself (not inherited) so a process in that + // user's session can run from it and open files it was granted explicitly. + // Throws when the directory cannot be proven safe; callers fail closed. + public static string EnsureDirectory(string name, SecurityIdentifier? readerSid = null) + { + var root = AgentDataRoot; + Directory.CreateDirectory(root); + RefuseReparseChain(root); + + var path = Path.Combine(root, name); + var attributes = TryGetAttributes(path); + if (attributes is FileAttributes existing + && (existing.HasFlag(FileAttributes.ReparsePoint) + || !existing.HasFlag(FileAttributes.Directory))) + { + // Link or planted file at our name. Removing a link never touches + // its target. + if (existing.HasFlag(FileAttributes.Directory)) + Directory.Delete(path); + else + File.Delete(path); + attributes = null; + } + + if (attributes is null) + new DirectoryInfo(path).Create(DirectorySecurityFor(readerSid)); + + var info = new DirectoryInfo(path); + if (!IsTrustedOwner(info)) + { + // Pre-created by someone else. The owner can always rewrite the + // DACL, so re-securing is not enough: replace the directory. + Directory.Delete(path, recursive: true); + new DirectoryInfo(path).Create(DirectorySecurityFor(readerSid)); + info = new DirectoryInfo(path); + if (!IsTrustedOwner(info)) + throw new IOException($"Staging directory {path} has an untrusted owner"); + } + + if (IsReparsePoint(path)) + throw new IOException($"Staging directory {path} is a reparse point"); + + info.SetAccessControl(DirectorySecurityFor(readerSid)); + return path; + } + + // New file inside a protected directory. Any existing entry at the name is + // removed first, then CreateNew so nothing can be slipped in between. The + // DACL is protected; userSid, when set, is granted userRights explicitly. + public static FileStream CreateFile( + string path, SecurityIdentifier? userSid = null, + FileSystemRights userRights = FileSystemRights.ReadAndExecute) + { + if (TryGetAttributes(path) is not null) + File.Delete(path); + + var security = new FileSecurity(); + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + security.AddAccessRule(new FileSystemAccessRule( + LocalSystemSid, FileSystemRights.FullControl, AccessControlType.Allow)); + security.AddAccessRule(new FileSystemAccessRule( + AdministratorsSid, FileSystemRights.FullControl, AccessControlType.Allow)); + if (userSid is not null) + security.AddAccessRule(new FileSystemAccessRule( + userSid, userRights, AccessControlType.Allow)); + + return new FileInfo(path).Create( + FileMode.CreateNew, FileSystemRights.Read | FileSystemRights.Write, + FileShare.Read, 4096, FileOptions.None, security); + } + + // Attributes of the entry itself (a link reports its own, not its target's). + public static FileAttributes? TryGetAttributes(string path) + { + try + { + return File.GetAttributes(path); + } + catch (FileNotFoundException) + { + return null; + } + catch (DirectoryNotFoundException) + { + return null; + } + } + + public static bool IsReparsePoint(string path) + => TryGetAttributes(path) is FileAttributes attributes + && attributes.HasFlag(FileAttributes.ReparsePoint); + + // Open for reading without following a reparse point at the final + // component, and refuse if the opened object is one. Share mode is wide so + // files a running application holds open still read. + public static FileStream OpenReadNoFollow(string path) + { + var handle = CreateFileW(path, GenericRead, + FileShareRead | FileShareWrite | FileShareDelete, IntPtr.Zero, + OpenExisting, FileFlagOpenReparsePoint, IntPtr.Zero); + if (handle.IsInvalid) + { + var hr = Marshal.GetHRForLastWin32Error(); + handle.Dispose(); + throw Marshal.GetExceptionForHR(hr) ?? new IOException($"Could not open {path}"); + } + + try + { + if (!GetFileInformationByHandle(handle, out var information)) + throw Marshal.GetExceptionForHR(Marshal.GetHRForLastWin32Error()) + ?? new IOException($"Could not query {path}"); + + var attributes = (FileAttributes)information.FileAttributes; + if (attributes.HasFlag(FileAttributes.ReparsePoint)) + throw new UnauthorizedAccessException($"{path} is a reparse point"); + if (attributes.HasFlag(FileAttributes.Directory)) + throw new UnauthorizedAccessException($"{path} is a directory"); + + return new FileStream(handle, FileAccess.Read); + } + catch + { + handle.Dispose(); + throw; + } + } + + // The path the handle really refers to after every junction and link in + // its ancestry is resolved. Null when it cannot be read. + public static string? FinalPath(SafeFileHandle handle) + { + var buffer = new StringBuilder(1024); + var length = GetFinalPathNameByHandleW(handle, buffer, (uint)buffer.Capacity, 0); + if (length == 0) + return null; + if (length >= buffer.Capacity) + { + buffer = new StringBuilder((int)length + 1); + length = GetFinalPathNameByHandleW(handle, buffer, (uint)buffer.Capacity, 0); + if (length == 0 || length >= buffer.Capacity) + return null; + } + + var final = buffer.ToString(); + if (final.StartsWith(@"\\?\UNC\", StringComparison.OrdinalIgnoreCase)) + return @"\\" + final[8..]; + if (final.StartsWith(@"\\?\", StringComparison.Ordinal)) + return final[4..]; + return final; + } + + // Read at most maxBytes of text from the head of a file, BOM-aware. + public static string ReadTextHeadNoFollow(string path, int maxBytes) + { + using var stream = OpenReadNoFollow(path); + var buffer = new byte[(int)Math.Min(maxBytes, Math.Max(stream.Length, 0))]; + var total = 0; + int read; + while (total < buffer.Length + && (read = stream.Read(buffer, total, buffer.Length - total)) > 0) + total += read; + + using var reader = new StreamReader( + new MemoryStream(buffer, 0, total), Encoding.UTF8, true); + return reader.ReadToEnd(); + } + + // Delete the entry itself. DeleteFile removes a link, never its target; + // a directory at a file path is left alone. + public static void DeleteFileNoFollow(string path) + { + if (TryGetAttributes(path) is not FileAttributes attributes) + return; + if (attributes.HasFlag(FileAttributes.Directory)) + throw new IOException($"{path} is a directory, not a file"); + File.Delete(path); + } + + private static DirectorySecurity DirectorySecurityFor(SecurityIdentifier? readerSid) + { + var security = new DirectorySecurity(); + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + const InheritanceFlags inherit = + InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit; + security.AddAccessRule(new FileSystemAccessRule(LocalSystemSid, + FileSystemRights.FullControl, inherit, PropagationFlags.None, + AccessControlType.Allow)); + security.AddAccessRule(new FileSystemAccessRule(AdministratorsSid, + FileSystemRights.FullControl, inherit, PropagationFlags.None, + AccessControlType.Allow)); + if (readerSid is not null) + security.AddAccessRule(new FileSystemAccessRule(readerSid, + FileSystemRights.ReadAndExecute, InheritanceFlags.None, + PropagationFlags.None, AccessControlType.Allow)); + return security; + } + + private static bool IsTrustedOwner(DirectoryInfo info) + { + var owner = info.GetAccessControl(AccessControlSections.Owner) + .GetOwner(typeof(SecurityIdentifier)) as SecurityIdentifier; + return owner is not null + && (owner.Equals(LocalSystemSid) || owner.Equals(AdministratorsSid)); + } + + // A junction anywhere from ProgramData down to the agent root would move + // every "protected" directory somewhere else. + private static void RefuseReparseChain(string root) + { + var programData = Environment.GetFolderPath( + Environment.SpecialFolder.CommonApplicationData).TrimEnd('\\'); + var current = Path.GetFullPath(root).TrimEnd('\\'); + while (current.Length > programData.Length + && current.StartsWith(programData, StringComparison.OrdinalIgnoreCase)) + { + if (IsReparsePoint(current)) + throw new IOException($"{current} is a reparse point; refusing to stage under it"); + current = Path.GetDirectoryName(current) ?? ""; + } + } + + [StructLayout(LayoutKind.Sequential)] + private struct ByHandleFileInformation + { + public uint FileAttributes; + public System.Runtime.InteropServices.ComTypes.FILETIME CreationTime; + public System.Runtime.InteropServices.ComTypes.FILETIME LastAccessTime; + public System.Runtime.InteropServices.ComTypes.FILETIME LastWriteTime; + public uint VolumeSerialNumber; + public uint FileSizeHigh; + public uint FileSizeLow; + public uint NumberOfLinks; + public uint FileIndexHigh; + public uint FileIndexLow; + } + + [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + private static extern SafeFileHandle CreateFileW( + string fileName, uint desiredAccess, uint shareMode, IntPtr securityAttributes, + uint creationDisposition, uint flagsAndAttributes, IntPtr templateFile); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool GetFileInformationByHandle( + SafeFileHandle file, out ByHandleFileInformation information); + + [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + private static extern uint GetFinalPathNameByHandleW( + SafeFileHandle file, StringBuilder filePath, uint filePathLength, uint flags); +} diff --git a/agent/ShopDbEndpointAgent.Core/StrictModePreamble.cs b/agent/ShopDbEndpointAgent.Core/StrictModePreamble.cs new file mode 100644 index 00000000..a37db6c5 --- /dev/null +++ b/agent/ShopDbEndpointAgent.Core/StrictModePreamble.cs @@ -0,0 +1,293 @@ +using System.Text; + +namespace ShopDbEndpointAgent.Core; + +// Inserts the strict error-mode statement into a PowerShell script body. +// +// The statement cannot simply go first: using statements, [CmdletBinding()] and +// param(...) must precede every other statement or the script stops parsing. +// So the scanner walks the leading header (whitespace, # and <# #> comments +// including #Requires and comment help, using lines, attributes, the param +// block) and inserts on the SAME line as the first real statement, followed by +// '; ', so every line number PowerShell reports still matches the stored body. +// Anything it cannot read with confidence (named begin/process/end blocks, +// subexpressions or here-strings inside the header, unbalanced brackets, bytes +// that are not valid text) returns null and the caller skips injection. +internal static class StrictModePreamble +{ + private static readonly UTF8Encoding StrictUtf8 = new(false, true); + private static readonly UTF8Encoding Utf8WithBom = new(true); + + private static readonly string[] NamedBlocks = + { "begin", "process", "end", "dynamicparam", "clean" }; + + // Returns the rewritten body as UTF-8 WITH BOM (Windows PowerShell 5.1 + // reads a BOM-less file as the ANSI code page), or null with a reason. + public static byte[]? TryApply(byte[] body, string statement, out string? reason) + { + if (!TryDecode(body, out var text)) + { + reason = "script is not valid UTF-8 or UTF-16 text"; + return null; + } + + var index = FindInsertionPoint(text, out reason); + if (index < 0) + return null; + + var rewritten = text.Insert(index, statement + "; "); + var preamble = Utf8WithBom.GetPreamble(); + var encoded = Utf8WithBom.GetBytes(rewritten); + var result = new byte[preamble.Length + encoded.Length]; + preamble.CopyTo(result, 0); + encoded.CopyTo(result, preamble.Length); + return result; + } + + private static bool TryDecode(byte[] body, out string text) + { + text = ""; + try + { + if (body.Length >= 3 && body[0] == 0xEF && body[1] == 0xBB && body[2] == 0xBF) + text = StrictUtf8.GetString(body, 3, body.Length - 3); + else if (body.Length >= 2 && body[0] == 0xFF && body[1] == 0xFE) + text = new UnicodeEncoding(false, false, true).GetString(body, 2, body.Length - 2); + else if (body.Length >= 2 && body[0] == 0xFE && body[1] == 0xFF) + text = new UnicodeEncoding(true, false, true).GetString(body, 2, body.Length - 2); + else + text = StrictUtf8.GetString(body); + return true; + } + catch (DecoderFallbackException) + { + return false; + } + } + + // Index of the first statement after the script header, or -1. + internal static int FindInsertionPoint(string text, out string? reason) + { + reason = null; + var index = SkipTrivia(text, 0); + while (index >= 0 && MatchWord(text, index, "using")) + { + var lineEnd = text.IndexOf('\n', index); + index = SkipTrivia(text, lineEnd < 0 ? text.Length : lineEnd); + } + if (index < 0) + return Uncertain("unterminated block comment in header", out reason); + + var cursor = index; + while (cursor < text.Length && text[cursor] == '[') + { + cursor = SkipBalanced(text, cursor); + if (cursor < 0) + return Uncertain("could not read attribute in header", out reason); + cursor = SkipTrivia(text, cursor); + if (cursor < 0) + return Uncertain("unterminated block comment in header", out reason); + } + + if (MatchWord(text, cursor, "param")) + { + var open = SkipTrivia(text, cursor + "param".Length); + if (open < 0 || open >= text.Length || text[open] != '(') + return Uncertain("could not read param block", out reason); + var close = SkipBalanced(text, open); + if (close < 0) + return Uncertain("could not read param block", out reason); + index = SkipTrivia(text, close); + if (index < 0) + return Uncertain("unterminated block comment after param block", out reason); + } + // Brackets not followed by param are a type literal starting the first + // statement, so index stays in front of them. + + if (index >= text.Length) + return Uncertain("no statements after the script header", out reason); + + foreach (var block in NamedBlocks) + { + if (MatchWord(text, index, block)) + return Uncertain($"script uses a named '{block}' block", out reason); + } + + return index; + } + + private static int Uncertain(string why, out string? reason) + { + reason = why; + return -1; + } + + // Skip whitespace, line comments and block comments. -1 on an unterminated + // block comment. + private static int SkipTrivia(string text, int index) + { + while (index < text.Length) + { + var character = text[index]; + if (char.IsWhiteSpace(character) || character == '\uFEFF') + { + index++; + } + else if (character == '<' && index + 1 < text.Length && text[index + 1] == '#') + { + var end = text.IndexOf("#>", index + 2, StringComparison.Ordinal); + if (end < 0) + return -1; + index = end + 2; + } + else if (character == '#') + { + var lineEnd = text.IndexOf('\n', index); + index = lineEnd < 0 ? text.Length : lineEnd + 1; + } + else + { + break; + } + } + return index; + } + + private static bool MatchWord(string text, int index, string word) + { + if (index < 0 || index + word.Length > text.Length) + return false; + if (string.Compare(text, index, word, 0, word.Length, StringComparison.OrdinalIgnoreCase) != 0) + return false; + var after = index + word.Length; + if (after == text.Length) + return true; + var next = text[after]; + return !(char.IsLetterOrDigit(next) || next == '_' || next == '-'); + } + + private static bool IsSingleQuote(char character) + => character is '\'' or '\u2018' or '\u2019' or '\u201A' or '\u201B'; + + private static bool IsDoubleQuote(char character) + => character is '"' or '\u201C' or '\u201D' or '\u201E'; + + // From an opening ( [ or { return the index just past its matching closer, + // honoring quotes and comments. -1 when unsure. + private static int SkipBalanced(string text, int index) + { + var closers = new Stack(); + while (index < text.Length) + { + var character = text[index]; + switch (character) + { + case '(': + closers.Push(')'); + index++; + break; + case '[': + closers.Push(']'); + index++; + break; + case '{': + closers.Push('}'); + index++; + break; + case ')': + case ']': + case '}': + if (closers.Count == 0 || closers.Pop() != character) + return -1; + index++; + if (closers.Count == 0) + return index; + break; + case '`': + index += 2; + break; + case '@' when index + 1 < text.Length + && (IsSingleQuote(text[index + 1]) || IsDoubleQuote(text[index + 1])): + // Here-string; not worth reading in a header. + return -1; + case '<' when index + 1 < text.Length && text[index + 1] == '#': + var end = text.IndexOf("#>", index + 2, StringComparison.Ordinal); + if (end < 0) + return -1; + index = end + 2; + break; + case '#': + if (index > 0 && (char.IsLetterOrDigit(text[index - 1]) || text[index - 1] == '_')) + return -1; + var lineEnd = text.IndexOf('\n', index); + if (lineEnd < 0) + return -1; + index = lineEnd + 1; + break; + default: + if (IsSingleQuote(character)) + { + index = SkipSingleQuoted(text, index + 1); + if (index < 0) + return -1; + } + else if (IsDoubleQuote(character)) + { + index = SkipDoubleQuoted(text, index + 1); + if (index < 0) + return -1; + } + else + { + index++; + } + break; + } + } + return -1; + } + + private static int SkipSingleQuoted(string text, int index) + { + while (index < text.Length) + { + if (IsSingleQuote(text[index])) + { + if (index + 1 < text.Length && IsSingleQuote(text[index + 1])) + { + index += 2; + continue; + } + return index + 1; + } + index++; + } + return -1; + } + + private static int SkipDoubleQuoted(string text, int index) + { + while (index < text.Length) + { + var character = text[index]; + if (character == '`') + { + index += 2; + continue; + } + if (character == '$' && index + 1 < text.Length && text[index + 1] == '(') + return -1; + if (IsDoubleQuote(character)) + { + if (index + 1 < text.Length && IsDoubleQuote(text[index + 1])) + { + index += 2; + continue; + } + return index + 1; + } + index++; + } + return -1; + } +} diff --git a/agent/ShopDbEndpointAgent/Program.cs b/agent/ShopDbEndpointAgent/Program.cs index 00409093..7ed76530 100644 --- a/agent/ShopDbEndpointAgent/Program.cs +++ b/agent/ShopDbEndpointAgent/Program.cs @@ -160,8 +160,7 @@ try if (!string.IsNullOrWhiteSpace(reportPayloadPath)) return await RunReportAsync(config, reportPayloadPath, logDirectory, faultMonitor); - await RunHostAsync(config, agentVersion, logDirectory, forceConsole); - return 0; + return await RunHostAsync(config, agentVersion, logDirectory, forceConsole); } catch (Exception error) { @@ -169,11 +168,20 @@ catch (Exception error) return 1; } -static async Task RunHostAsync(AgentConfig config, string agentVersion, +static async Task RunHostAsync(AgentConfig config, string agentVersion, string logDirectory, bool forceConsole) { + // Non-zero on purpose: SCM recovery only restarts a service that exits with + // a failure. Exit 0 after a renewal left the agent stopped until reboot. + const int RestartForRenewedCertificate = 3; + + // Well inside the SCM 30s start timeout. The host has not reported + // SERVICE_RUNNING yet, so a slow or unreachable server here would otherwise + // get the whole start killed. + var renewalBudget = TimeSpan.FromSeconds(15); + // Certificate renewal check: if client cert expires within 30 days, request - // renewal from server, install new cert, and gracefully stop so service manager + // renewal from server, install new cert, and exit so service manager // restarts us. This runs BEFORE the host starts so a renewed cert is picked up. var renewalLogger = LoggerFactory.Create(logging => ConfigureLogging(logging, logDirectory)).CreateLogger(); @@ -181,18 +189,25 @@ static async Task RunHostAsync(AgentConfig config, string agentVersion, try { - var didRenew = await renewal.CheckAndRenewIfNeeded(); - if (didRenew) + // Task.Run so a slow synchronous prefix cannot dodge the budget either. + var renewalTask = Task.Run(renewal.CheckAndRenewIfNeeded); + if (await Task.WhenAny(renewalTask, Task.Delay(renewalBudget)) != renewalTask) + { + renewalLogger.LogWarning( + "Certificate renewal check still running after {Budget}; starting agent " + + "with existing certificate. The expiry monitor retries later.", + renewalBudget); + } + else if (await renewalTask) { renewalLogger.LogInformation( - "Certificate renewed successfully. Service will exit gracefully " + - "and service manager will restart with new certificate."); - // Return immediately without starting the host. The service exits gracefully - // with code 0, and Windows Service Manager restarts it (if configured as - // Automatic). The new process picks up the updated thumbprint from - // appsettings.json. This is safer than Environment.Exit(0) which aborts - // in-flight work and can corrupt local state. - return; + "Certificate renewed successfully. Service will exit with code {Code} " + + "so service manager recovery restarts it with the new certificate.", + RestartForRenewedCertificate); + // Return without starting the host. The new process picks up the + // updated thumbprint from appsettings.json. Safer than + // Environment.Exit, which aborts in-flight work. + return RestartForRenewedCertificate; } } catch (Exception ex) @@ -221,6 +236,7 @@ static async Task RunHostAsync(AgentConfig config, string agentVersion, builder.Services.AddHostedService(sp => sp.GetRequiredService()); await builder.Build().RunAsync(); + return 0; } static async Task RunOnceAsync(AgentConfig config, string agentVersion, diff --git a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj index 8b7b22c6..40c8ab93 100644 --- a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj +++ b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj @@ -248,7 +248,24 @@ delete-all-insert-all to delta mode (only touch changed apps). Expected impact: 99% reduction in database writes (1.92M/day to 19K/day). Matches backend algorithm implemented in 1.17.0. Minor for feature completion. --> -1.18.0 + +1.19.0 true win-x64 true