From c4dbd85da0bfe1a8d2fa68d089f3d07eb744daae Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Wed, 29 Jul 2026 15:33:28 +0200 Subject: [PATCH] Add the client's SSH key model, codec, merge and cipher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client can now seal and open an SSH key item. Nothing consumes it yet — the repository, the sync engine's per-type handling and the UI come next — but this is the layer everything above it depends on, and it is the layer where the crypto has to be right. SshKeySecret holds the private key as an ordinary string, deliberately, and says so: a .NET string cannot be wiped, so the material lives until the GC reuses the memory. libsodium's guarded memory was considered and rejected because the passphrase protecting the key, the password on the next item and the JSON the codec just parsed are all strings on the same heap — protecting one field among them reads as security and buys nothing. What the design does give is that the key never reaches the disk in plaintext, never reaches the server at all, and is handed to SSH.NET through a MemoryStream so there is no temporary key file to leak. Validation refuses a public key by name. ssh-keygen writes two files whose names differ by four characters, and pasting the wrong one otherwise produces a vault item that looks fine and fails at connection time with an authentication error that says nothing about which file you chose. The merge redacts the private key and its passphrase from the conflict log. A host conflict shows both values so the loser can be put back; doing that for a private key would write the discarded key into a log that is designed to be read rather than used and is deliberately retained after acknowledgement. Two different private keys are not something anyone reconciles by reading them side by side. And the lesson worth recording, because it nearly shipped: the first version of AadResourceTypeTests proved nothing. It checked that a key payload does not open as a host and vice versa — true however both ciphers are misconfigured, because Seal and TryOpen share one constant, so changing it changes both and the round trip still works. Sealing every private key as if it were a vault passed all twelve tests. The tests now open a sealed payload independently through ItemKeys with the resource type named out of band, and that does fail under the same sabotage. A test that only compares an implementation against itself cannot catch a self-consistent mistake. The trap it defends: SyncEntityType.SshKey is 3, AadResourceType.SshKey is 6, because the crypto enum also carries None, User, Device and Vault ahead of the item types. A cast between them is a specification violation that encrypts cleanly and would only surface when another implementation refused the item. --- src/DodoSSH.Client.Domain/SshKeySecret.cs | 113 ++++++++++ .../SshKeySecretCodec.cs | 126 ++++++++++++ .../SshKeySecretMerge.cs | 120 +++++++++++ src/DodoSSH.Client.Sync/SshKeyCipher.cs | 130 ++++++++++++ .../AadResourceTypeTests.cs | 194 ++++++++++++++++++ 5 files changed, 683 insertions(+) create mode 100644 src/DodoSSH.Client.Domain/SshKeySecret.cs create mode 100644 src/DodoSSH.Client.Domain/SshKeySecretCodec.cs create mode 100644 src/DodoSSH.Client.Domain/SshKeySecretMerge.cs create mode 100644 src/DodoSSH.Client.Sync/SshKeyCipher.cs create mode 100644 tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs diff --git a/src/DodoSSH.Client.Domain/SshKeySecret.cs b/src/DodoSSH.Client.Domain/SshKeySecret.cs new file mode 100644 index 0000000..bd6b22b --- /dev/null +++ b/src/DodoSSH.Client.Domain/SshKeySecret.cs @@ -0,0 +1,113 @@ +using System.Diagnostics.CodeAnalysis; + +namespace DodoSSH.Client.Domain; + +/// +/// An SSH key pair as the user sees it, decrypted. +/// +/// +/// +/// The private key is the most valuable thing this product stores, and it is held here as an ordinary +/// string, in managed memory, exactly like and every passphrase in the +/// client. That is a deliberate choice rather than an oversight, and it is worth being plain about what it +/// does and does not give you. +/// +/// +/// It does not give you a zeroable buffer. A .NET string cannot be wiped, so the key material lives until +/// the garbage collector reuses the memory, and a process dump taken in the meantime contains it. The +/// alternative — libsodium's guarded memory, which DodoSSH.Crypto uses for the vault keys — was +/// considered and rejected here for one reason: the passphrase protecting this key, the password on the next +/// item, and the decoded JSON the codec produced are all strings on the same heap. Protecting one field +/// among them would be a measure that reads as security and buys nothing, and the honest place for that +/// effort is the threat this design actually addresses, which is the server and the disk rather than another +/// process running as the same user. See LocalCacheProtector, which says the same thing about the +/// cache. +/// +/// +/// What it does give you: the key never reaches the disk in plaintext and never reaches the server at all. +/// It is sealed under the vault key before it leaves this type, and SSH.NET is handed it through a +/// MemoryStream rather than a file, so there is no temporary key file to leak or forget. +/// +/// +public sealed record SshKeySecret +{ + /// What the user calls this key. + public required string Label { get; init; } + + /// + /// The private key, in the armoured form ssh-keygen writes. + /// + /// + /// Stored verbatim, including its header and trailer. Not reformatted, not re-encoded, not normalised: + /// OpenSSH, PKCS#1 and PKCS#8 all round-trip through here untouched, and a client that rewrote them + /// would eventually rewrite one it did not fully understand. + /// + public required string PrivateKeyPem { get; init; } + + /// + /// The passphrase protecting the private key, when it has one. + /// + /// + /// Kept with the key rather than typed per connection, which is the entire point of a vault: the + /// passphrase defends the key file on a disk, and inside a vault the key is not on a disk. Storing both + /// together means the vault passphrase is what protects them, which is the guarantee this product is + /// built to make. A user who wants the second factor can leave this null and be prompted. + /// + public string? Passphrase { get; init; } + + /// + /// The public half, in authorized_keys form, when it is known. + /// + /// + /// Not derived from the private key, and deliberately optional. Deriving it means parsing every key + /// format this type accepts verbatim, which is the thing above that it declines to do. It is worth + /// keeping because installing a key on a host needs exactly this line, and a user who imported only a + /// private key should be told the public half is missing rather than have one silently reconstructed. + /// + public string? PublicKey { get; init; } + + /// Free text. + public string? Notes { get; init; } + + /// + /// Whether this key is storable, and why not if it is not. + /// + /// + /// The public-key mix-up is checked explicitly because it is the mistake a person actually makes: + /// ssh-keygen writes two files whose names differ by four characters, and pasting the wrong one + /// produces a vault item that looks fine and fails at connection time with an authentication error that + /// says nothing about which file you chose. + /// + public bool TryValidate([NotNullWhen(false)] out string? error) + { + if (string.IsNullOrWhiteSpace(Label)) + { + error = "A key needs a name."; + return false; + } + + if (string.IsNullOrWhiteSpace(PrivateKeyPem)) + { + error = "A key needs its private key material."; + return false; + } + + var material = PrivateKeyPem.TrimStart(); + + if (material.StartsWith("ssh-", StringComparison.Ordinal) + || material.StartsWith("ecdsa-", StringComparison.Ordinal)) + { + error = "That is a public key. Paste the private key — the file without the .pub extension."; + return false; + } + + if (!material.StartsWith("-----BEGIN", StringComparison.Ordinal)) + { + error = "That does not look like a private key; it should begin with \"-----BEGIN\"."; + return false; + } + + error = null; + return true; + } +} diff --git a/src/DodoSSH.Client.Domain/SshKeySecretCodec.cs b/src/DodoSSH.Client.Domain/SshKeySecretCodec.cs new file mode 100644 index 0000000..e8b147d --- /dev/null +++ b/src/DodoSSH.Client.Domain/SshKeySecretCodec.cs @@ -0,0 +1,126 @@ +using System.Diagnostics.CodeAnalysis; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace DodoSSH.Client.Domain; + +/// A decoded SSH key payload, together with the schema version it was written at. +/// The key. +/// The version the writing client used. +public sealed record SshKeySecretDocument(SshKeySecret Key, int SchemaVersion) +{ + /// + /// + /// The consequence is sharper for a key than for a host. Re-encoding an item written by a newer client + /// drops the fields this build has no concept of — and if one of those fields were, say, a certificate + /// or a second key format, the item would still decrypt, still look complete, and no longer + /// authenticate. Refusing to write is the only safe answer. + /// + public bool IsReadOnly => SchemaVersion > SshKeySecretCodec.CurrentSchemaVersion; +} + +/// +/// Encodes and decodes the plaintext inside an SSH key item's encrypted payload. +/// +/// +/// Mirrors , for the same reasons and with the same guarantees: JSON so a field +/// can be added without a migration, deterministic property order so an unchanged key does not look like a +/// change to the sync engine, and a separate mutable document type so a decode failure cannot produce a +/// half-built key that looks valid downstream. +/// +public static class SshKeySecretCodec +{ + /// The schema version this build writes. + public const int CurrentSchemaVersion = 1; + + /// Serialises a key to the bytes that get sealed. + /// The key is not valid for storage. + public static byte[] Encode(SshKeySecret key) + { + ArgumentNullException.ThrowIfNull(key); + + if (!key.TryValidate(out var error)) + { + throw new ArgumentException(error, nameof(key)); + } + + var document = new SshKeyPayloadDocument + { + SchemaVersion = CurrentSchemaVersion, + Label = key.Label, + PrivateKeyPem = key.PrivateKeyPem, + Passphrase = key.Passphrase, + PublicKey = key.PublicKey, + Notes = key.Notes, + }; + + return JsonSerializer.SerializeToUtf8Bytes( + document, SshKeyPayloadJsonContext.Default.SshKeyPayloadDocument); + } + + /// Parses a decrypted payload. + /// + public static bool TryDecode( + ReadOnlySpan payload, + [NotNullWhen(true)] out SshKeySecretDocument? document) + { + document = null; + + SshKeyPayloadDocument? parsed; + try + { + parsed = JsonSerializer.Deserialize( + payload, SshKeyPayloadJsonContext.Default.SshKeyPayloadDocument); + } + catch (JsonException) + { + return false; + } + + if (parsed is null || parsed.SchemaVersion < 1) + { + return false; + } + + var candidate = new SshKeySecret + { + Label = parsed.Label ?? string.Empty, + PrivateKeyPem = parsed.PrivateKeyPem ?? string.Empty, + Passphrase = parsed.Passphrase, + PublicKey = parsed.PublicKey, + Notes = parsed.Notes, + }; + + if (!candidate.TryValidate(out _)) + { + return false; + } + + document = new SshKeySecretDocument(candidate, parsed.SchemaVersion); + return true; + } +} + +/// The serialised shape. Mutable and nullable because it models untrusted input. +/// +internal sealed class SshKeyPayloadDocument +{ + public int SchemaVersion { get; set; } + + public string? Label { get; set; } + + public string? PrivateKeyPem { get; set; } + + public string? Passphrase { get; set; } + + public string? PublicKey { get; set; } + + public string? Notes { get; set; } +} + +[JsonSourceGenerationOptions( + PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip)] +[JsonSerializable(typeof(SshKeyPayloadDocument))] +internal sealed partial class SshKeyPayloadJsonContext : JsonSerializerContext; diff --git a/src/DodoSSH.Client.Domain/SshKeySecretMerge.cs b/src/DodoSSH.Client.Domain/SshKeySecretMerge.cs new file mode 100644 index 0000000..a6a8cd6 --- /dev/null +++ b/src/DodoSSH.Client.Domain/SshKeySecretMerge.cs @@ -0,0 +1,120 @@ +namespace DodoSSH.Client.Domain; + +/// The merged key, and everything that had to be overridden to produce it. +/// The key to store and push. +/// Empty when the two sides were reconcilable field by field. +public sealed record SshKeyMergeResult( + SshKeySecret Merged, + IReadOnlyList Conflicts) +{ + /// Whether anything had to be overridden. + public bool HasConflicts => Conflicts.Count > 0; +} + +/// +/// Merges two divergent versions of an SSH key against the version they both started from. +/// +/// +/// +/// Every field is a scalar, so this is simpler than the host merge — there is no per-name map to reconcile +/// and no ordered list whose order is its meaning. It reuses rather than +/// declaring a parallel type: the conflict log, the storage that persists it and the interface that shows it +/// are all shared, and a second record with identical members would have to be mapped to the first at every +/// boundary. +/// +/// +/// The conflict detail never carries key material. A host conflict shows both values so the user can +/// put back the one that lost; doing that for a private key would write the discarded key into the conflict +/// log, which is a copy of a secret in a place designed to be read rather than used — and the log is +/// deliberately retained after acknowledgement. So the private key and its passphrase report only +/// that they differed. This is the one place where showing less is the safer answer, and it costs +/// the user nothing they can act on: two different private keys are not something anyone reconciles by +/// reading them side by side. +/// +/// +public static class SshKeySecretMerge +{ + /// Produces the merged key. + /// The version both sides branched from. + /// The pending local version. + /// The server's current version. + public static SshKeyMergeResult Merge(SshKeySecret ancestor, SshKeySecret local, SshKeySecret remote) + { + ArgumentNullException.ThrowIfNull(ancestor); + ArgumentNullException.ThrowIfNull(local); + ArgumentNullException.ThrowIfNull(remote); + + var conflicts = new List(); + + var merged = new SshKeySecret + { + // Null-forgiving on the two required fields, as HostSecretMerge does for the same reason: the + // merge returns one of the three inputs, and all three are non-null here by construction. + Label = Shown( + nameof(SshKeySecret.Label), ancestor.Label, local.Label, remote.Label, conflicts)!, + PrivateKeyPem = Redacted( + nameof(SshKeySecret.PrivateKeyPem), + ancestor.PrivateKeyPem, + local.PrivateKeyPem, + remote.PrivateKeyPem, + conflicts)!, + Passphrase = Redacted( + nameof(SshKeySecret.Passphrase), + ancestor.Passphrase, + local.Passphrase, + remote.Passphrase, + conflicts), + PublicKey = Shown( + nameof(SshKeySecret.PublicKey), + ancestor.PublicKey, + local.PublicKey, + remote.PublicKey, + conflicts), + Notes = Shown( + nameof(SshKeySecret.Notes), ancestor.Notes, local.Notes, remote.Notes, conflicts), + }; + + return new SshKeyMergeResult(merged, conflicts); + } + + /// A field whose overridden value is safe to show. + private static string? Shown( + string name, + string? ancestor, + string? local, + string? remote, + List conflicts) => + Resolve(name, ancestor, local, remote, conflicts, redact: false); + + /// A field whose overridden value must not be written to the conflict log. + private static string? Redacted( + string name, + string? ancestor, + string? local, + string? remote, + List conflicts) => + Resolve(name, ancestor, local, remote, conflicts, redact: true); + + private static string? Resolve( + string name, + string? ancestor, + string? local, + string? remote, + List conflicts, + bool redact) + { + var merge = ThreeWayMerge.Scalar(ancestor, local, remote, StringComparer.Ordinal); + + if (merge.IsConflicted) + { + conflicts.Add(new HostFieldConflict( + name, + MergeSide.Local, + redact ? "(kept the server's value)" : merge.Value, + redact ? "(a different value was discarded)" : merge.Discarded, + DiscardedWasRemoval: false)); + } + + return merge.Value; + } +} diff --git a/src/DodoSSH.Client.Sync/SshKeyCipher.cs b/src/DodoSSH.Client.Sync/SshKeyCipher.cs new file mode 100644 index 0000000..6a70d7d --- /dev/null +++ b/src/DodoSSH.Client.Sync/SshKeyCipher.cs @@ -0,0 +1,130 @@ +using System.Security.Cryptography; +using DodoSSH.Client.Domain; +using DodoSSH.Contracts; +using DodoSSH.Crypto; + +namespace DodoSSH.Client.Sync; + +/// +/// Turns an SSH key into an item payload and back. +/// +/// +/// +/// Mirrors exactly, including the rule that a payload is sealed at the version the +/// server will assign rather than the one it replaces — see . +/// +/// +/// The resource type is the one thing not to copy. The AAD binds it, and the two enums that name item +/// types do not agree: SyncEntityType.SshKey is 3 while CryptoSpec.AadResourceType.SshKey is 6, +/// because the crypto enum also carries None, User, Device and Vault ahead of the item types. Casting one to +/// the other would seal a private key under the resource type for a vault — which encrypts +/// perfectly, decrypts perfectly on the machine that wrote it, and is a specification violation that no test +/// would notice until an interoperating client refused the item. So it is named here as a constant, and +/// AadResourceTypeTests pins the pairing. +/// +/// +public static class SshKeyCipher +{ + private const CryptoSpec.AadResourceType Resource = CryptoSpec.AadResourceType.SshKey; + + /// Encrypts an SSH key. + /// The key. Must be valid for storage. + /// The vault key, which the data key is wrapped under. + /// The item id, which the AAD binds. + /// The vault's current key generation. + /// The version this payload will hold once the server accepts it. + public static EncryptedPayload Seal( + SshKeySecret key, + ReadOnlySpan vaultKey, + Guid entityId, + uint keyGeneration, + int itemVersion) + { + ArgumentNullException.ThrowIfNull(key); + ArgumentOutOfRangeException.ThrowIfLessThan(itemVersion, 1); + + var plaintext = SshKeySecretCodec.Encode(key); + var dataKey = ItemKeys.CreateDataKey(); + + try + { + var dataKeyId = Guid.CreateVersion7(); + + var wrappedDataKey = ItemKeys.WrapDataKey( + dataKey, vaultKey, Resource, entityId, keyGeneration, (uint)itemVersion); + + var envelope = ItemKeys.SealPayload( + dataKey, plaintext, Resource, entityId, dataKeyId, keyGeneration, (uint)itemVersion); + + return new EncryptedPayload( + envelope, wrappedDataKey, dataKeyId, keyGeneration, CryptoSpec.CurrentAadVersion); + } + finally + { + CryptographicOperations.ZeroMemory(dataKey); + + // The encoded key material, wiped. This is the one buffer in the client whose contents are a + // private key and which can actually be cleared — the strings the codec read it from cannot be. + CryptographicOperations.ZeroMemory(plaintext); + } + } + + /// Decrypts an SSH key. + /// + public static SshKeySecretDocument? TryOpen( + EncryptedPayload payload, + ReadOnlySpan vaultKey, + Guid entityId, + int itemVersion) + { + ArgumentNullException.ThrowIfNull(payload); + + if (itemVersion < 1 || payload.WrappedDataKey.Length == 0) + { + return null; + } + + var dataKey = ItemKeys.TryUnwrapDataKey( + vaultKey, + payload.WrappedDataKey, + Resource, + entityId, + payload.KeyGeneration, + (uint)itemVersion); + + if (dataKey is null) + { + return null; + } + + try + { + var plaintext = ItemKeys.TryOpenPayload( + dataKey, + payload.Envelope, + Resource, + entityId, + payload.DataKeyId, + payload.KeyGeneration, + (uint)itemVersion); + + if (plaintext is null) + { + return null; + } + + try + { + return SshKeySecretCodec.TryDecode(plaintext, out var document) ? document : null; + } + finally + { + CryptographicOperations.ZeroMemory(plaintext); + } + } + finally + { + CryptographicOperations.ZeroMemory(dataKey); + } + } +} diff --git a/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs b/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs new file mode 100644 index 0000000..6f4b69a --- /dev/null +++ b/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs @@ -0,0 +1,194 @@ +using System.Security.Cryptography; +using DodoSSH.Client.Domain; +using DodoSSH.Client.Sync; +using DodoSSH.Contracts; +using DodoSSH.Crypto; + +namespace DodoSSH.Client.Sync.Tests; + +/// +/// Each item type must be sealed under its own AAD resource type, and the two enums that name item types +/// deliberately do not agree. +/// +/// +/// +/// SyncEntityType lists only syncable items, so Host is 1 and SshKey is 3. +/// CryptoSpec.AadResourceType also covers users, devices and vaults, so the same two are 4 and 6. A +/// cipher written by copying its neighbour and casting the wire type would therefore seal a private key as +/// if it were a vault — encrypting cleanly, decrypting cleanly on the machine that wrote it, and violating +/// docs/crypto.md in a way that surfaces only when another implementation reads the item. +/// +/// +/// These tests are cheap and the alternative is a comment. The payload's AAD is frozen, so getting this +/// wrong is not something a later release can quietly correct: only clients can re-encrypt, and they can +/// only do it if they can still open what is there. +/// +/// +public sealed class AadResourceTypeTests +{ + /// + /// The pairing stated as a table. If AadResourceType is ever renumbered, this is what says so — + /// loudly, and before anything is written under the new numbers. + /// + [Theory] + [InlineData(SyncEntityType.Host, CryptoSpec.AadResourceType.Host)] + [InlineData(SyncEntityType.Credential, CryptoSpec.AadResourceType.Credential)] + [InlineData(SyncEntityType.SshKey, CryptoSpec.AadResourceType.SshKey)] + [InlineData(SyncEntityType.HostGroup, CryptoSpec.AadResourceType.HostGroup)] + [InlineData(SyncEntityType.Tag, CryptoSpec.AadResourceType.Tag)] + [InlineData(SyncEntityType.Snippet, CryptoSpec.AadResourceType.Snippet)] + [InlineData(SyncEntityType.PortForward, CryptoSpec.AadResourceType.PortForward)] + [InlineData(SyncEntityType.KnownHostKey, CryptoSpec.AadResourceType.KnownHostKey)] + public void TheTwoEnums_AreNamedAlikeAndNumberedDifferently( + SyncEntityType wire, + CryptoSpec.AadResourceType resource) + { + Enum.GetName(wire).ShouldBe(Enum.GetName(resource)); + + // The point of the whole file: same name, different number. A test asserting equality here would be + // asserting the bug. + ((int)wire).ShouldNotBe( + (int)resource, + $"{wire} happens to share a value with its resource type, which makes a cast look correct. " + + "Either the enums were renumbered or this pairing needs re-checking by hand."); + } + + /// + /// + /// Opened independently, through the low-level ItemKeys API with the resource type this + /// test names itself. That is the whole point, and the first version of this file got it wrong in an + /// instructive way: it checked only that a key payload does not open as a host and vice versa, which is + /// true however both ciphers are misconfigured. Seal and TryOpen share one constant, so + /// changing it changes both, the round trip still works, and the two ciphers still differ from each + /// other. Sealing every private key as if it were a vault passed all of it. + /// + /// + /// A test that only compares an implementation against itself cannot catch a self-consistent mistake. + /// This one states the specified value out of band and refuses anything else. + /// + /// + [Fact] + public void AKeyPayload_OpensUnderTheResourceTypeTheSpecificationNames() + { + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + const uint Generation = 1; + const uint Version = 1; + + var payload = SshKeyCipher.Seal(NewKey(), vaultKey, entityId, Generation, (int)Version); + + var dataKey = ItemKeys.TryUnwrapDataKey( + vaultKey, + payload.WrappedDataKey, + CryptoSpec.AadResourceType.SshKey, + entityId, + Generation, + Version); + + dataKey.ShouldNotBeNull( + "SshKeyCipher must wrap the data key under AadResourceType.SshKey; if this is null it used " + + "some other resource type, which round-trips fine and violates docs/crypto.md."); + + ItemKeys.TryOpenPayload( + dataKey, + payload.Envelope, + CryptoSpec.AadResourceType.SshKey, + entityId, + payload.DataKeyId, + Generation, + Version).ShouldNotBeNull("and it must seal the envelope under the same resource type."); + } + + /// Pins the host cipher the same way, since the two are now easy to confuse for each other. + [Fact] + public void AHostPayload_OpensUnderTheResourceTypeTheSpecificationNames() + { + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + const uint Generation = 1; + const uint Version = 1; + + var host = new HostSecret { Label = "prod-db", Hostname = "db.internal" }; + var payload = HostCipher.Seal(host, vaultKey, entityId, Generation, (int)Version); + + ItemKeys.TryUnwrapDataKey( + vaultKey, + payload.WrappedDataKey, + CryptoSpec.AadResourceType.Host, + entityId, + Generation, + Version) + .ShouldNotBeNull("HostCipher must wrap the data key under AadResourceType.Host."); + } + + [Fact] + public void AKeyPayload_DoesNotOpenAsAHost() + { + // Weaker than the two above and kept anyway: it is the property a reader expects to see, and it + // covers the case where one cipher is corrected and the other is not. + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + + var sealedKey = SshKeyCipher.Seal(NewKey(), vaultKey, entityId, keyGeneration: 1, itemVersion: 1); + + HostCipher.TryOpen(sealedKey, vaultKey, entityId, itemVersion: 1).ShouldBeNull(); + SshKeyCipher.TryOpen(sealedKey, vaultKey, entityId, itemVersion: 1).ShouldNotBeNull(); + } + + [Fact] + public void AHostPayload_DoesNotOpenAsAKey() + { + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + + var host = new HostSecret { Label = "prod-db", Hostname = "db.internal" }; + var sealedHost = HostCipher.Seal(host, vaultKey, entityId, keyGeneration: 1, itemVersion: 1); + + SshKeyCipher.TryOpen(sealedHost, vaultKey, entityId, itemVersion: 1).ShouldBeNull(); + } + + [Fact] + public void AKey_RoundTripsThroughTheCipher() + { + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + var key = NewKey(); + + var payload = SshKeyCipher.Seal(key, vaultKey, entityId, keyGeneration: 1, itemVersion: 3); + var opened = SshKeyCipher.TryOpen(payload, vaultKey, entityId, itemVersion: 3); + + opened.ShouldNotBeNull(); + opened.Key.ShouldBe(key); + opened.SchemaVersion.ShouldBe(SshKeySecretCodec.CurrentSchemaVersion); + opened.IsReadOnly.ShouldBeFalse(); + } + + [Fact] + public void AKeySealedAtOneVersion_DoesNotOpenAtAnother() + { + // The item version is in the AAD, which is what stops a server rolling a row back to earlier + // ciphertext. Asserted for keys as well as hosts because it is the property most easily lost by + // copying a cipher and adjusting the wrong argument. + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + + var payload = SshKeyCipher.Seal(NewKey(), vaultKey, entityId, keyGeneration: 1, itemVersion: 2); + + SshKeyCipher.TryOpen(payload, vaultKey, entityId, itemVersion: 3).ShouldBeNull(); + } + + private static SshKeySecret NewKey() => new() + { + Label = "deploy", + PrivateKeyPem = "-----BEGIN OPENSSH PRIVATE KEY-----\nnot-a-real-key\n-----END OPENSSH PRIVATE KEY-----", + Passphrase = "a passphrase", + PublicKey = "ssh-ed25519 AAAAC3Nz deploy@example", + Notes = "used by CI", + }; +}