From 972708175394b161580dcba57f50b8702266abc5 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Mon, 15 Jun 2026 10:05:29 +0800 Subject: [PATCH 1/2] Encrypt SSH proxy password at rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSH proxy password lived as plaintext inside the machine endpoint JSON column. Encrypt it at rest via the shared AtRestSecretProtector, at the two write seams (register + update) and decrypt at the two read seams (the deploy variable contributor + the health-check connection builder). The read seams are synchronous (IEndpointVariableContributor.ContributeVariables is sync), and decrypt is pure CPU work, so add a synchronous Decrypt to IVariableEncryptionService (the already-sync core of DecryptAsync, which now delegates to it) and a synchronous Unprotect to IAtRestSecretProtector. This keeps the contributor interface sync — no async ripple across every transport. Non-breaking: - ProxyPassword is write-only from the API (no GET/response DTO returns it), so encrypting at rest changes nothing a client observes. - Read-both: a legacy plaintext proxy password in an existing machine's endpoint passes through Unprotect verbatim — no migration. - The protector is an optional ctor param on all four seams (register, update, contributor, health check); DI injects the real one in production, and any test that constructs them without it degrades to the pre-feature plaintext path. Encrypt is idempotent so a re-save (or the unchanged value carried through an update merge) never double-wraps. Tests: - Unit: contributor decrypts an envelope before contributing the SSH connection variable; legacy plaintext passes through; no-protector leaves the value untouched. - Integration (real Postgres + real DI + real AES-256-GCM): a proxy password encrypted via the real protector and stored in a machine's endpoint jsonb is decrypted by the DI-resolved contributor (proves the protector is wired into the read seam); legacy plaintext reads back verbatim. - Full unit suite (6048) green. --- .../SshEndpointVariableContributor.cs | 20 +++- .../Ssh/Transport/SshHealthCheckStrategy.cs | 16 ++- .../MachineRegistrationService.Ssh.cs | 10 +- .../Machines/MachineRegistrationService.cs | 7 +- .../Services/Machines/MachineService.cs | 16 ++- .../Security/AtRestSecretProtector.cs | 9 ++ .../Security/IVariableEncryptionService.cs | 5 + .../Security/VariableEncryptionService.cs | 8 +- .../Deployments/Machine/SshEndpointDto.cs | 6 + .../IntegrationSshProxyPasswordAtRest.cs | 108 ++++++++++++++++++ .../Ssh/SshProxyPasswordAtRestTests.cs | 75 ++++++++++++ 11 files changed, 270 insertions(+), 10 deletions(-) create mode 100644 tests/Squid.IntegrationTests/Services/Machines/IntegrationSshProxyPasswordAtRest.cs create mode 100644 tests/Squid.UnitTests/Services/Deployments/Ssh/SshProxyPasswordAtRestTests.cs diff --git a/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshEndpointVariableContributor.cs b/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshEndpointVariableContributor.cs index 4463d41b..bff345f9 100644 --- a/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshEndpointVariableContributor.cs +++ b/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshEndpointVariableContributor.cs @@ -1,5 +1,6 @@ using Squid.Core.Services.DeploymentExecution.Transport; using Squid.Core.Services.DeploymentExecution.Variables; +using Squid.Core.Services.Security; using Squid.Message.Constants; using Squid.Message.Models.Deployments.Machine; using Squid.Message.Models.Deployments.Variable; @@ -8,6 +9,17 @@ namespace Squid.Core.Services.DeploymentExecution.Ssh; public class SshEndpointVariableContributor : IEndpointVariableContributor { + private readonly IAtRestSecretProtector _protector; + + // Optional: when DI-resolved on the deploy path the real protector decrypts the at-rest ProxyPassword + // before it is contributed as the SSH connection variable. When constructed without one (the SSH + // health check's internal use — which reads ProxyPassword separately — or tests), the value passes + // through; since Unprotect is read-both, a legacy plaintext is identical either way. + public SshEndpointVariableContributor(IAtRestSecretProtector protector = null) + { + _protector = protector; + } + public EndpointResourceReferences ParseResourceReferences(string endpointJson) { var endpoint = EndpointVariableFactory.TryDeserialize(endpointJson); @@ -28,6 +40,12 @@ public List ContributeVariables(EndpointContext context) var accountData = context.GetAccountData(); + // Decrypt the at-rest proxy password before it becomes the SSH connection variable. Read-both: + // a legacy unprefixed plaintext passes through unchanged; no protector (non-deploy use) = passthrough. + var proxyPassword = _protector != null + ? _protector.Unprotect(endpoint.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope) + : endpoint.ProxyPassword; + var vars = new List { EndpointVariableFactory.Make(SpecialVariables.Machine.Hostname, endpoint.Host ?? string.Empty), @@ -39,7 +57,7 @@ public List ContributeVariables(EndpointContext context) EndpointVariableFactory.Make(SpecialVariables.Ssh.ProxyHost, endpoint.ProxyHost ?? string.Empty), EndpointVariableFactory.Make(SpecialVariables.Ssh.ProxyPort, endpoint.ProxyPort.ToString()), EndpointVariableFactory.Make(SpecialVariables.Ssh.ProxyUsername, endpoint.ProxyUsername ?? string.Empty), - EndpointVariableFactory.Make(SpecialVariables.Ssh.ProxyPassword, endpoint.ProxyPassword ?? string.Empty, isSensitive: true), + EndpointVariableFactory.Make(SpecialVariables.Ssh.ProxyPassword, proxyPassword ?? string.Empty, isSensitive: true), EndpointVariableFactory.Make(SpecialVariables.Ssh.PackageBaseDirectory, PackageBaseDirectory(endpoint.RemoteWorkingDirectory)) }; diff --git a/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshHealthCheckStrategy.cs b/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshHealthCheckStrategy.cs index 78753390..15a9614e 100644 --- a/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshHealthCheckStrategy.cs +++ b/src/Squid.Core/Services/DeploymentExecution/Targets/Ssh/Transport/SshHealthCheckStrategy.cs @@ -2,6 +2,7 @@ using Squid.Core.Persistence.Entities.Deployments; using Squid.Core.Services.DeploymentExecution.Transport; using Squid.Core.Services.DeploymentExecution.Variables; +using Squid.Core.Services.Security; using Squid.Message.Constants; using Squid.Message.Enums; using Squid.Message.Models.Deployments.Account; @@ -16,11 +17,16 @@ public class SshHealthCheckStrategy : IHealthCheckStrategy private readonly IEndpointContextBuilder _endpointContextBuilder; private readonly ISshConnectionFactory _connectionFactory; + private readonly IAtRestSecretProtector _protector; - public SshHealthCheckStrategy(IEndpointContextBuilder endpointContextBuilder, ISshConnectionFactory connectionFactory) + // Protector optional: DI supplies the real one so the at-rest proxy password is decrypted before the + // health probe builds its connection. Without one (tests), the read-both Unprotect passes a legacy + // plaintext through unchanged. + public SshHealthCheckStrategy(IEndpointContextBuilder endpointContextBuilder, ISshConnectionFactory connectionFactory, IAtRestSecretProtector protector = null) { _endpointContextBuilder = endpointContextBuilder; _connectionFactory = connectionFactory; + _protector = protector; } public async Task CheckHealthAsync(Machine machine, MachineConnectivityPolicyDto connectivityPolicy, CancellationToken ct, MachineHealthCheckPolicyDto healthCheckPolicy = null) @@ -62,7 +68,13 @@ private async Task BuildConnectionInfoAsync(Machine machine, var timeout = TimeSpan.FromSeconds(connectivityPolicy?.ConnectTimeoutSeconds ?? DefaultConnectTimeoutSeconds); - return new SshConnectionInfo(endpoint.Host, endpoint.Port, username, privateKey, passphrase, password, endpoint.Fingerprint, timeout, endpoint.ProxyType, endpoint.ProxyHost, endpoint.ProxyPort, endpoint.ProxyUsername, endpoint.ProxyPassword); + // Decrypt the at-rest proxy password before building the probe connection. Read-both: a legacy + // plaintext passes through; no protector (tests) = passthrough. + var proxyPassword = _protector != null + ? _protector.Unprotect(endpoint.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope) + : endpoint.ProxyPassword; + + return new SshConnectionInfo(endpoint.Host, endpoint.Port, username, privateKey, passphrase, password, endpoint.Fingerprint, timeout, endpoint.ProxyType, endpoint.ProxyHost, endpoint.ProxyPort, endpoint.ProxyUsername, proxyPassword); } private HealthCheckResult ProbeConnectivity(SshConnectionInfo connectionInfo, string customScript) diff --git a/src/Squid.Core/Services/Machines/MachineRegistrationService.Ssh.cs b/src/Squid.Core/Services/Machines/MachineRegistrationService.Ssh.cs index 362f13cc..194f2a4c 100644 --- a/src/Squid.Core/Services/Machines/MachineRegistrationService.Ssh.cs +++ b/src/Squid.Core/Services/Machines/MachineRegistrationService.Ssh.cs @@ -23,8 +23,14 @@ public async Task RegisterSshAsync(RegisterSshComma }; } - private static string BuildSshEndpointJson(RegisterSshCommand command) + private string BuildSshEndpointJson(RegisterSshCommand command) { + // Encrypt the proxy password at rest before it lands in the endpoint JSON column. Idempotent + + // read-both via the shared protector; without a protector (tests) it is stored as-is. + var proxyPassword = _protector != null + ? _protector.Protect(command.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope) + : command.ProxyPassword; + return JsonSerializer.Serialize(new SshEndpointDto { CommunicationStyle = nameof(CommunicationStyleEnum.Ssh), @@ -36,7 +42,7 @@ private static string BuildSshEndpointJson(RegisterSshCommand command) ProxyHost = command.ProxyHost, ProxyPort = command.ProxyPort, ProxyUsername = command.ProxyUsername, - ProxyPassword = command.ProxyPassword, + ProxyPassword = proxyPassword, ResourceReferences = command.ResourceReferences }); } diff --git a/src/Squid.Core/Services/Machines/MachineRegistrationService.cs b/src/Squid.Core/Services/Machines/MachineRegistrationService.cs index 9d892a86..d18d0ed1 100644 --- a/src/Squid.Core/Services/Machines/MachineRegistrationService.cs +++ b/src/Squid.Core/Services/Machines/MachineRegistrationService.cs @@ -31,19 +31,24 @@ public partial class MachineRegistrationService : IMachineRegistrationService private readonly IEnvironmentDataProvider _environmentDataProvider; private readonly IPollingTrustDistributor _trustDistributor; private readonly SelfCertSetting _selfCertSetting; + private readonly Security.IAtRestSecretProtector _protector; + // Protector optional: DI supplies the real one so an SSH proxy password is encrypted at rest before + // the endpoint JSON is persisted. Without one (tests), it is stored as-is (pre-feature behaviour). public MachineRegistrationService( IMachineDataProvider dataProvider, IMachinePolicyDataProvider policyDataProvider, IEnvironmentDataProvider environmentDataProvider, IPollingTrustDistributor trustDistributor, - SelfCertSetting selfCertSetting) + SelfCertSetting selfCertSetting, + Security.IAtRestSecretProtector protector = null) { _dataProvider = dataProvider; _policyDataProvider = policyDataProvider; _environmentDataProvider = environmentDataProvider; _trustDistributor = trustDistributor; _selfCertSetting = selfCertSetting; + _protector = protector; } private async Task EnsureUniqueNameAsync(string name, int spaceId, CancellationToken ct) diff --git a/src/Squid.Core/Services/Machines/MachineService.cs b/src/Squid.Core/Services/Machines/MachineService.cs index be6f094e..97045748 100644 --- a/src/Squid.Core/Services/Machines/MachineService.cs +++ b/src/Squid.Core/Services/Machines/MachineService.cs @@ -27,13 +27,17 @@ public class MachineService : IMachineService private readonly IMachineDataProvider _machineDataProvider; private readonly IPollingTrustDistributor _trustDistributor; private readonly IMachineRuntimeCapabilitiesCache _runtimeCache; + private readonly Security.IAtRestSecretProtector _protector; - public MachineService(IMapper mapper, IMachineDataProvider machineDataProvider, IPollingTrustDistributor trustDistributor, IMachineRuntimeCapabilitiesCache runtimeCache) + // Protector optional: DI supplies the real one so an updated SSH proxy password is re-encrypted at + // rest. Without one (tests), the value is stored as-is (pre-feature behaviour). + public MachineService(IMapper mapper, IMachineDataProvider machineDataProvider, IPollingTrustDistributor trustDistributor, IMachineRuntimeCapabilitiesCache runtimeCache, Security.IAtRestSecretProtector protector = null) { _mapper = mapper; _machineDataProvider = machineDataProvider; _trustDistributor = trustDistributor; _runtimeCache = runtimeCache; + _protector = protector; } public async Task GetMachinesAsync(GetMachinesRequest request, CancellationToken cancellationToken) @@ -233,7 +237,7 @@ void Check(bool isSet, string fieldName, params CommunicationStyle[] allowed) /// endpoint.X = command.X ?? endpoint.X ladder — any field the /// command doesn't set is preserved from the existing endpoint JSON. /// - private static void ApplyEndpointUpdate(Persistence.Entities.Deployments.Machine machine, CommunicationStyle style, UpdateMachineCommand c) + private void ApplyEndpointUpdate(Persistence.Entities.Deployments.Machine machine, CommunicationStyle style, UpdateMachineCommand c) { machine.Endpoint = style switch { @@ -281,7 +285,7 @@ private static string MergeOpenClaw(string json, UpdateMachineCommand c) return JsonSerializer.Serialize(e); } - private static string MergeSsh(string json, UpdateMachineCommand c) + private string MergeSsh(string json, UpdateMachineCommand c) { var e = Deserialise(json); e.Host = c.Host ?? e.Host; @@ -294,6 +298,12 @@ private static string MergeSsh(string json, UpdateMachineCommand c) e.ProxyUsername = c.ProxyUsername ?? e.ProxyUsername; e.ProxyPassword = c.ProxyPassword ?? e.ProxyPassword; e.ResourceReferences = c.ResourceReferences ?? e.ResourceReferences; + + // Encrypt the merged proxy password at rest. Protect is idempotent: a plaintext from the command + // is encrypted; an unchanged value carried from the existing (already-encrypted) endpoint passes + // through untouched. Without a protector (tests), the value is stored as-is. + if (_protector != null) e.ProxyPassword = _protector.Protect(e.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope); + return JsonSerializer.Serialize(e); } diff --git a/src/Squid.Core/Services/Security/AtRestSecretProtector.cs b/src/Squid.Core/Services/Security/AtRestSecretProtector.cs index b3d9e2aa..3075cdda 100644 --- a/src/Squid.Core/Services/Security/AtRestSecretProtector.cs +++ b/src/Squid.Core/Services/Security/AtRestSecretProtector.cs @@ -37,6 +37,11 @@ public interface IAtRestSecretProtector : IScopedDependency /// Decrypt an at-rest , read-both: a legacy unprefixed plaintext /// is returned verbatim. must match the scope used to Protect it. Task UnprotectAsync(string value, int kdfScope); + + /// Synchronous — decrypt is pure CPU work (no I/O), so a caller + /// on a synchronous path (e.g. an endpoint variable contributor) can decrypt without an async + /// signature. Same read-both contract: legacy unprefixed plaintext passes through verbatim. + string Unprotect(string value, int kdfScope); } public sealed class AtRestSecretProtector(IVariableEncryptionService encryption) : IAtRestSecretProtector @@ -54,4 +59,8 @@ public string Protect(string value, int kdfScope) public Task UnprotectAsync(string value, int kdfScope) // DecryptAsync is read-both: an unprefixed plaintext value passes through verbatim. => encryption.DecryptAsync(value, kdfScope); + + public string Unprotect(string value, int kdfScope) + // Decrypt is the synchronous core of DecryptAsync (pure CPU); same read-both passthrough. + => encryption.Decrypt(value, kdfScope); } diff --git a/src/Squid.Core/Services/Security/IVariableEncryptionService.cs b/src/Squid.Core/Services/Security/IVariableEncryptionService.cs index a2924501..ebc74288 100644 --- a/src/Squid.Core/Services/Security/IVariableEncryptionService.cs +++ b/src/Squid.Core/Services/Security/IVariableEncryptionService.cs @@ -8,6 +8,11 @@ public interface IVariableEncryptionService : IScopedDependency Task DecryptAsync(string encryptedText, int variableSetId); + /// Synchronous decrypt — DecryptAsync's work is pure CPU (AES-GCM + PBKDF2, no I/O), so + /// callers on a sync path can decrypt without an async signature. Read-both: a value lacking the + /// envelope prefix (legacy plaintext) is returned verbatim. + string Decrypt(string encryptedText, int variableSetId); + Task> EncryptSensitiveVariablesAsync( List variables, int variableSetId); diff --git a/src/Squid.Core/Services/Security/VariableEncryptionService.cs b/src/Squid.Core/Services/Security/VariableEncryptionService.cs index 2d978ba8..17e8557b 100644 --- a/src/Squid.Core/Services/Security/VariableEncryptionService.cs +++ b/src/Squid.Core/Services/Security/VariableEncryptionService.cs @@ -255,7 +255,13 @@ public string EncryptAsync(string plainText, int variableSetId) } } - public async Task DecryptAsync(string encryptedText, int variableSetId) + // DecryptAsync's work is entirely synchronous CPU (AES-GCM + PBKDF2 — no I/O), so it delegates to + // this sync core. Callers on a synchronous path (e.g. IEndpointVariableContributor.ContributeVariables) + // can decrypt without forcing their signatures async. The "Async" name is kept for the existing API. + public Task DecryptAsync(string encryptedText, int variableSetId) + => Task.FromResult(Decrypt(encryptedText, variableSetId)); + + public string Decrypt(string encryptedText, int variableSetId) { if (string.IsNullOrEmpty(encryptedText) || !IsValidEncryptedValue(encryptedText)) return encryptedText; diff --git a/src/Squid.Message/Models/Deployments/Machine/SshEndpointDto.cs b/src/Squid.Message/Models/Deployments/Machine/SshEndpointDto.cs index d6fe1eba..3d2ac6d8 100644 --- a/src/Squid.Message/Models/Deployments/Machine/SshEndpointDto.cs +++ b/src/Squid.Message/Models/Deployments/Machine/SshEndpointDto.cs @@ -4,6 +4,12 @@ namespace Squid.Message.Models.Deployments.Machine; public class SshEndpointDto { + /// KDF scope used when encrypting/decrypting at rest. The V2 + /// envelope uses a random per-payload salt so this value is not security-relevant — it is a shared + /// constant so the write seam (machine register/update) and the read seams (deploy variable + /// contributor, SSH health check) agree. Mirrors the KdfScope=0 the column-based providers use. + public const int ProxyPasswordKdfScope = 0; + public string CommunicationStyle { get; set; } public string Host { get; set; } public int Port { get; set; } = 22; diff --git a/tests/Squid.IntegrationTests/Services/Machines/IntegrationSshProxyPasswordAtRest.cs b/tests/Squid.IntegrationTests/Services/Machines/IntegrationSshProxyPasswordAtRest.cs new file mode 100644 index 00000000..bc39450b --- /dev/null +++ b/tests/Squid.IntegrationTests/Services/Machines/IntegrationSshProxyPasswordAtRest.cs @@ -0,0 +1,108 @@ +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using Squid.Core.Persistence.Db; +using Squid.Core.Persistence.Entities.Deployments; +using Squid.Core.Services.DeploymentExecution.Ssh; +using Squid.Core.Services.DeploymentExecution.Transport; +using Squid.Core.Services.Machines; +using Squid.Core.Services.Security; +using Squid.Message.Constants; +using Squid.Message.Models.Deployments.Machine; + +namespace Squid.IntegrationTests.Services.Machines; + +/// +/// End-to-end (real Postgres + real DI + real AES-256-GCM) coverage of the SSH proxy-password at-rest +/// READ seam: a proxy password encrypted via the real and persisted +/// inside a machine's endpoint JSON (Postgres jsonb) is transparently decrypted by the DI-resolved +/// — proving the protector is actually injected into the +/// contributor (the read seam is not a silent no-op). A legacy plaintext endpoint reads back verbatim +/// (read-both / non-breaking). +/// +public class IntegrationSshProxyPasswordAtRest : TestBase +{ + public IntegrationSshProxyPasswordAtRest() + : base("SshProxyPasswordAtRest", "squid_it_ssh_proxy_atrest") + { + } + + [Fact] + public async Task EncryptedProxyPassword_IsStoredAsEnvelope_AndDiContributorDecryptsIt() + { + const string secret = "proxy-secret-9c3f-VALUE"; + + var envelope = await Run(p => + Task.FromResult(p.Protect(secret, SshEndpointDto.ProxyPasswordKdfScope))).ConfigureAwait(false); + + envelope.ShouldStartWith("SQUID_ENCRYPTED_V2:", customMessage: "the real protector must emit a V2 envelope."); + envelope.ShouldNotContain(secret); + + var machineId = await SeedSshMachineAsync(envelope).ConfigureAwait(false); + + await Run>(async (machines, contributors) => + { + var machine = await machines.GetMachinesByIdAsync(machineId, CancellationToken.None).ConfigureAwait(false); + + // The raw column carries the envelope, never the cleartext. + machine.Endpoint.ShouldContain("SQUID_ENCRYPTED_V2:"); + machine.Endpoint.ShouldNotContain(secret); + + var ssh = contributors.OfType().Single(); + var vars = ssh.ContributeVariables(new EndpointContext { EndpointJson = machine.Endpoint }); + + vars.First(v => v.Name == SpecialVariables.Ssh.ProxyPassword).Value.ShouldBe(secret, + customMessage: "the DI-resolved SSH contributor must decrypt the at-rest proxy password — proves the protector is wired into the read seam."); + }).ConfigureAwait(false); + } + + [Fact] + public async Task LegacyPlaintextProxyPassword_ReadsBackVerbatim() + { + // A machine registered before encryption has a plaintext proxy password in its endpoint JSON. + // Read-both: the DI contributor returns it unchanged — non-breaking for existing machines. + const string legacy = "legacy-plaintext-proxy"; + + var machineId = await SeedSshMachineAsync(legacy).ConfigureAwait(false); + + await Run>(async (machines, contributors) => + { + var machine = await machines.GetMachinesByIdAsync(machineId, CancellationToken.None).ConfigureAwait(false); + + var ssh = contributors.OfType().Single(); + var vars = ssh.ContributeVariables(new EndpointContext { EndpointJson = machine.Endpoint }); + + vars.First(v => v.Name == SpecialVariables.Ssh.ProxyPassword).Value.ShouldBe(legacy); + }).ConfigureAwait(false); + } + + private async Task SeedSshMachineAsync(string proxyPassword) + { + var endpointJson = JsonSerializer.Serialize(new SshEndpointDto + { + CommunicationStyle = "Ssh", Host = "ssh-host", Port = 22, ProxyPassword = proxyPassword + }); + + var machineId = 0; + + await Run(async (repository, unitOfWork) => + { + var machine = new Machine + { + Name = $"ssh-{Guid.NewGuid():N}", + IsDisabled = false, + Roles = "[]", + EnvironmentIds = "[]", + SpaceId = 1, + Endpoint = endpointJson, + Slug = $"ssh-{Guid.NewGuid():N}" + }; + + await repository.InsertAsync(machine).ConfigureAwait(false); + await unitOfWork.SaveChangesAsync().ConfigureAwait(false); + machineId = machine.Id; + }).ConfigureAwait(false); + + return machineId; + } +} diff --git a/tests/Squid.UnitTests/Services/Deployments/Ssh/SshProxyPasswordAtRestTests.cs b/tests/Squid.UnitTests/Services/Deployments/Ssh/SshProxyPasswordAtRestTests.cs new file mode 100644 index 00000000..1d756bf5 --- /dev/null +++ b/tests/Squid.UnitTests/Services/Deployments/Ssh/SshProxyPasswordAtRestTests.cs @@ -0,0 +1,75 @@ +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using Microsoft.Extensions.Configuration; +using Squid.Core.Services.DeploymentExecution.Ssh; +using Squid.Core.Services.DeploymentExecution.Transport; +using Squid.Core.Services.Security; +using Squid.Core.Settings.Security; +using Squid.Message.Constants; +using Squid.Message.Models.Deployments.Machine; + +namespace Squid.UnitTests.Services.Deployments.Ssh; + +/// +/// Unit coverage for the SSH proxy-password at-rest READ seam: the variable contributor must DECRYPT the +/// envelope stored in the endpoint JSON before contributing it as the SSH connection variable, must pass a +/// legacy plaintext through verbatim (read-both / non-breaking), and — when no protector is supplied (the +/// health check's internal use, or tests) — must leave the value untouched. +/// +public sealed class SshProxyPasswordAtRestTests +{ + private static IAtRestSecretProtector RealProtector() + { + var key = new byte[32]; + for (var i = 0; i < key.Length; i++) key[i] = (byte)(i + 1); + + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Security:VariableEncryption:MasterKey"] = System.Convert.ToBase64String(key) + }) + .Build(); + + return new AtRestSecretProtector(new VariableEncryptionService(new SecuritySetting(config))); + } + + private static string ContributedProxyPassword(SshEndpointVariableContributor contributor, string endpointProxyPassword) + { + var json = JsonSerializer.Serialize(new SshEndpointDto { Host = "h", Port = 22, ProxyPassword = endpointProxyPassword }); + + var vars = contributor.ContributeVariables(new EndpointContext { EndpointJson = json }); + + return vars.First(v => v.Name == SpecialVariables.Ssh.ProxyPassword).Value; + } + + [Fact] + public void Contributor_WithProtector_DecryptsEnvelope_BeforeContributing() + { + var protector = RealProtector(); + var envelope = protector.Protect("proxy-secret", SshEndpointDto.ProxyPasswordKdfScope); + + envelope.ShouldStartWith("SQUID_ENCRYPTED_V2:", customMessage: "test setup: the stored value must be an envelope."); + + ContributedProxyPassword(new SshEndpointVariableContributor(protector), envelope) + .ShouldBe("proxy-secret", + customMessage: "the contributor must decrypt the at-rest envelope so the SSH connection gets the real proxy password."); + } + + [Fact] + public void Contributor_WithProtector_LegacyPlaintext_PassesThrough_ReadBoth() + { + // An existing machine has a plaintext proxy password in its endpoint JSON. Read-both: it flows + // through unchanged, so the feature is non-breaking for machines registered before encryption. + ContributedProxyPassword(new SshEndpointVariableContributor(RealProtector()), "legacy-plaintext") + .ShouldBe("legacy-plaintext"); + } + + [Fact] + public void Contributor_WithoutProtector_LeavesValueUntouched() + { + // No protector (health check's internal contributor use, or tests) → no decrypt attempt. + ContributedProxyPassword(new SshEndpointVariableContributor(), "whatever-value") + .ShouldBe("whatever-value"); + } +} From 61b20149f9f08c229a237c24ee3057b7a5bd627a Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Mon, 15 Jun 2026 10:21:52 +0800 Subject: [PATCH 2/2] Cover the SSH proxy at-rest WRITE seams + health-check decrypt (review blockers) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review found the write-encrypt seams and the health-check decrypt seam had zero test coverage — a regression dropping Protect()/Unprotect() would silently leak plaintext (or break proxy auth) with a green suite. Production code was verified correct; these close the coverage gaps: - Register: RegisterSshAsync with the real protector persists an encrypted envelope (not plaintext) for the proxy password. - Update (MergeSsh): a new plaintext password is encrypted; an unrelated edit does NOT re-wrap an already-encrypted value (idempotency — a double-wrap would break the single read-side Unprotect); a legacy plaintext is opportunistically migrated to encrypted on an unrelated edit. - Health check: the connection builder decrypts the at-rest proxy password before building the SSH connection. All drive the real AtRestSecretProtector (the default-ctor services have no protector, so these are the only tests exercising the encrypt/decrypt branches). Full unit suite (6053) green. --- .../Ssh/SshHealthCheckStrategyTests.cs | 51 +++++++++++ .../MachineRegistrationServiceTests.cs | 47 ++++++++++ .../Services/Machines/MachineServiceTests.cs | 87 +++++++++++++++++++ 3 files changed, 185 insertions(+) diff --git a/tests/Squid.UnitTests/Services/Deployments/Ssh/SshHealthCheckStrategyTests.cs b/tests/Squid.UnitTests/Services/Deployments/Ssh/SshHealthCheckStrategyTests.cs index 02436e8d..e8052eb7 100644 --- a/tests/Squid.UnitTests/Services/Deployments/Ssh/SshHealthCheckStrategyTests.cs +++ b/tests/Squid.UnitTests/Services/Deployments/Ssh/SshHealthCheckStrategyTests.cs @@ -1,8 +1,12 @@ +using System.Collections.Generic; using System.Text.Json; +using Microsoft.Extensions.Configuration; using Squid.Core.Persistence.Entities.Deployments; using Squid.Core.Services.DeploymentExecution.Kubernetes; using Squid.Core.Services.DeploymentExecution.Ssh; using Squid.Core.Services.DeploymentExecution.Transport; +using Squid.Core.Services.Security; +using Squid.Core.Settings.Security; using Squid.Message.Enums; using Squid.Message.Models.Deployments.Machine; @@ -15,6 +19,53 @@ public class SshHealthCheckStrategyTests private SshHealthCheckStrategy CreateStrategy() => new(_endpointContextBuilder.Object, _connectionFactory.Object); + [Fact] + public async Task HealthCheck_DecryptsAtRestProxyPassword_BeforeBuildingConnection() + { + // READ seam (health path): the at-rest proxy password must be decrypted before the probe builds + // its SSH connection — otherwise the proxy hop gets the raw envelope as the password and fails. + var protector = RealProtector(); + var envelope = protector.Protect("proxy-secret", SshEndpointDto.ProxyPasswordKdfScope); + + var endpoint = JsonSerializer.Serialize(new SshEndpointDto + { + CommunicationStyle = "Ssh", Host = "h", Port = 22, ProxyHost = "px", ProxyPort = 8080, + ProxyUsername = "u", ProxyPassword = envelope, ResourceReferences = new List() + }); + var machine = new Machine { Id = 1, Name = "ssh", Endpoint = endpoint, Roles = "[]" }; + + _endpointContextBuilder.Setup(b => b.BuildAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new EndpointContext { EndpointJson = endpoint }); + + SshConnectionInfo captured = null; + _connectionFactory.Setup(f => f.CreateScope(It.IsAny())) + .Callback(info => captured = info) + .Throws(new InvalidOperationException("probe-stop")); + + var strategy = new SshHealthCheckStrategy(_endpointContextBuilder.Object, _connectionFactory.Object, protector); + + await strategy.CheckHealthAsync(machine, null, CancellationToken.None); + + captured.ShouldNotBeNull(); + captured.ProxyPassword.ShouldBe("proxy-secret", + customMessage: "the health check must decrypt the at-rest proxy password before building the SSH connection."); + } + + private static IAtRestSecretProtector RealProtector() + { + var key = new byte[32]; + for (var i = 0; i < key.Length; i++) key[i] = (byte)(i + 1); + + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Security:VariableEncryption:MasterKey"] = System.Convert.ToBase64String(key) + }) + .Build(); + + return new AtRestSecretProtector(new VariableEncryptionService(new SecuritySetting(config))); + } + private static Machine MakeSshMachine(string host = "ssh.example.com", int port = 22, string fingerprint = "abc123", string remoteWorkDir = null) { var endpoint = new SshEndpointDto diff --git a/tests/Squid.UnitTests/Services/Machines/MachineRegistrationServiceTests.cs b/tests/Squid.UnitTests/Services/Machines/MachineRegistrationServiceTests.cs index 645fb0cd..c4bbe71a 100644 --- a/tests/Squid.UnitTests/Services/Machines/MachineRegistrationServiceTests.cs +++ b/tests/Squid.UnitTests/Services/Machines/MachineRegistrationServiceTests.cs @@ -1,12 +1,16 @@ +using System.Collections.Generic; using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text.Json; +using Microsoft.Extensions.Configuration; using Squid.Core.Halibut; using Squid.Core.Persistence.Entities.Deployments; using Squid.Core.Services.Deployments.Environments; using Squid.Core.Services.Machines; using Squid.Core.Services.Machines.Exceptions; +using Squid.Core.Services.Security; +using Squid.Core.Settings.Security; using Squid.Core.Settings.SelfCert; using Squid.Message.Commands.Machine; using Squid.Message.Models.Deployments.Machine; @@ -547,6 +551,49 @@ public async Task RegisterSsh_PersistsMachineWithSshEndpointJson() endpoint.ResourceReferences[0].Type.ShouldBe(Squid.Message.Enums.EndpointResourceType.AuthenticationAccount); } + [Fact] + public async Task RegisterSsh_ProxyPassword_StoredEncryptedAtRest() + { + // WRITE seam: a plaintext proxy password supplied at registration must be encrypted before it + // lands in the persisted endpoint JSON. Build the service WITH the real protector (DI injects it + // in production); the default-ctor _service has no protector so it never exercises this branch. + var protector = RealProtector(); + var service = new MachineRegistrationService( + _machineDataProvider.Object, _policyDataProvider.Object, _environmentDataProvider.Object, _trustDistributor.Object, _selfCertSetting, protector); + + Machine captured = null; + _machineDataProvider + .Setup(x => x.AddMachineAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((m, _, _) => captured = m) + .Returns(Task.CompletedTask); + + await service.RegisterSshAsync(new RegisterSshCommand + { + MachineName = "ssh-proxy", SpaceId = 1, Host = "h", Port = 22, ProxyPassword = "proxy-pw" + }, CancellationToken.None); + + var endpoint = JsonSerializer.Deserialize(captured.Endpoint); + endpoint.ProxyPassword.ShouldStartWith("SQUID_ENCRYPTED_V2:", + customMessage: "the registered proxy password must be encrypted at rest, not persisted plaintext."); + endpoint.ProxyPassword.ShouldNotContain("proxy-pw"); + (await protector.UnprotectAsync(endpoint.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope)).ShouldBe("proxy-pw"); + } + + private static IAtRestSecretProtector RealProtector() + { + var key = new byte[32]; + for (var i = 0; i < key.Length; i++) key[i] = (byte)(i + 1); + + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Security:VariableEncryption:MasterKey"] = Convert.ToBase64String(key) + }) + .Build(); + + return new AtRestSecretProtector(new VariableEncryptionService(new SecuritySetting(config))); + } + [Fact] public async Task RegisterSsh_DefaultsPortTo22WhenZero() { diff --git a/tests/Squid.UnitTests/Services/Machines/MachineServiceTests.cs b/tests/Squid.UnitTests/Services/Machines/MachineServiceTests.cs index 6f149532..3b8f75b4 100644 --- a/tests/Squid.UnitTests/Services/Machines/MachineServiceTests.cs +++ b/tests/Squid.UnitTests/Services/Machines/MachineServiceTests.cs @@ -1,9 +1,13 @@ +using System.Collections.Generic; using System.Linq; using System.Text.Json; +using Microsoft.Extensions.Configuration; using Squid.Core.Halibut; using Squid.Core.Persistence.Entities.Deployments; using Squid.Core.Services.DeploymentExecution.Tentacle; using Squid.Core.Services.Machines; +using Squid.Core.Services.Security; +using Squid.Core.Settings.Security; using Squid.Core.Services.Machines.Exceptions; using Squid.Message.Commands.Machine; using Squid.Message.Enums; @@ -491,6 +495,89 @@ public async Task UpdateMachine_Ssh_HostAndPort_ApplyCleanly() updated.Fingerprint.ShouldBe("SHA256:xyz", "existing Fingerprint must survive partial update"); } + // ── SSH proxy password at-rest WRITE seam (update / MergeSsh) ────────────── + + [Fact] + public async Task UpdateMachine_Ssh_NewProxyPassword_StoredEncryptedAtRest() + { + var protector = RealProtector(); + var service = NewServiceWithProtector(protector); + + var machine = MakeMachine(31, SshEndpoint); + _machineDataProvider.Setup(p => p.GetMachinesByIdAsync(31, It.IsAny())).ReturnsAsync(machine); + + await service.UpdateMachineAsync(new UpdateMachineCommand { MachineId = 31, ProxyPassword = "proxy-pw" }, CancellationToken.None); + + var updated = JsonSerializer.Deserialize(machine.Endpoint); + updated.ProxyPassword.ShouldStartWith("SQUID_ENCRYPTED_V2:", + customMessage: "a proxy password supplied on update must be encrypted at rest, not stored plaintext."); + updated.ProxyPassword.ShouldNotContain("proxy-pw"); + (await protector.UnprotectAsync(updated.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope)).ShouldBe("proxy-pw"); + } + + [Fact] + public async Task UpdateMachine_Ssh_UnrelatedEdit_DoesNotReWrapAlreadyEncryptedProxyPassword() + { + // The idempotency invariant: an update that does NOT touch the proxy password (e.g. a Host edit) + // flows the already-encrypted stored value back through Protect, which MUST pass it through + // unchanged. A double-wrap would make the single read-side Unprotect yield the inner envelope + // instead of the password — silently breaking proxied SSH after any unrelated machine edit. + var protector = RealProtector(); + var service = NewServiceWithProtector(protector); + + var envelope = protector.Protect("proxy-pw", SshEndpointDto.ProxyPasswordKdfScope); + var endpoint = JsonSerializer.Serialize(new SshEndpointDto { CommunicationStyle = "Ssh", Host = "h", Port = 22, ProxyPassword = envelope }); + var machine = MakeMachine(32, endpoint); + _machineDataProvider.Setup(p => p.GetMachinesByIdAsync(32, It.IsAny())).ReturnsAsync(machine); + + await service.UpdateMachineAsync(new UpdateMachineCommand { MachineId = 32, Host = "new-host" }, CancellationToken.None); + + var updated = JsonSerializer.Deserialize(machine.Endpoint); + updated.Host.ShouldBe("new-host"); + updated.ProxyPassword.ShouldBe(envelope, + customMessage: "an already-encrypted proxy password must be byte-identical after an unrelated update (no double-wrap)."); + (await protector.UnprotectAsync(updated.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope)).ShouldBe("proxy-pw", + customMessage: "the password must still decrypt with a SINGLE Unprotect — proves no double-wrap occurred."); + } + + [Fact] + public async Task UpdateMachine_Ssh_LegacyPlaintext_UnrelatedEdit_MigratesToEncrypted() + { + // Lazy migration: an existing machine with a PLAINTEXT proxy password, edited on an unrelated + // field, gets its proxy password opportunistically encrypted on write. + var protector = RealProtector(); + var service = NewServiceWithProtector(protector); + + var endpoint = JsonSerializer.Serialize(new SshEndpointDto { CommunicationStyle = "Ssh", Host = "h", Port = 22, ProxyPassword = "legacy-plaintext" }); + var machine = MakeMachine(33, endpoint); + _machineDataProvider.Setup(p => p.GetMachinesByIdAsync(33, It.IsAny())).ReturnsAsync(machine); + + await service.UpdateMachineAsync(new UpdateMachineCommand { MachineId = 33, Host = "new-host" }, CancellationToken.None); + + var updated = JsonSerializer.Deserialize(machine.Endpoint); + updated.ProxyPassword.ShouldStartWith("SQUID_ENCRYPTED_V2:", + customMessage: "an unrelated edit must opportunistically encrypt a legacy plaintext proxy password."); + (await protector.UnprotectAsync(updated.ProxyPassword, SshEndpointDto.ProxyPasswordKdfScope)).ShouldBe("legacy-plaintext"); + } + + private MachineService NewServiceWithProtector(IAtRestSecretProtector protector) => + new(_mapper.Object, _machineDataProvider.Object, _trustDistributor.Object, _runtimeCache, protector); + + private static IAtRestSecretProtector RealProtector() + { + var key = new byte[32]; + for (var i = 0; i < key.Length; i++) key[i] = (byte)(i + 1); + + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Security:VariableEncryption:MasterKey"] = System.Convert.ToBase64String(key) + }) + .Build(); + + return new AtRestSecretProtector(new VariableEncryptionService(new SecuritySetting(config))); + } + [Fact] public async Task UpdateMachine_KubernetesAgent_ChartRefUpdate_ApplyCleanly() {