diff --git a/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs b/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs index 6f4b69a..2af46df 100644 --- a/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs +++ b/tests/DodoSSH.Client.Sync.Tests/AadResourceTypeTests.cs @@ -53,22 +53,55 @@ public sealed class AadResourceTypeTests + "Either the enums were renumbered or this pairing needs re-checking by hand."); } + /// + /// The specified pairing of wire type to AAD resource type, stated out of band, one row per cipher. + /// + /// + /// The single source for both tests below: what each cipher must use, and which types must have a cipher + /// pinned at all. Adding an item type without adding a row here fails + /// . + /// + private static readonly (SyncEntityType Wire, CryptoSpec.AadResourceType Resource)[] PinnedPairs = + [ + (SyncEntityType.Host, CryptoSpec.AadResourceType.Host), + (SyncEntityType.SshKey, CryptoSpec.AadResourceType.SshKey), + (SyncEntityType.Credential, CryptoSpec.AadResourceType.Credential), + ]; + + public static TheoryData Pinned + { + get + { + var data = new TheoryData(); + + foreach (var (wire, resource) in PinnedPairs) + { + data.Add(wire, resource); + } + + return data; + } + } + /// /// /// 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. + /// table names rather than the one the cipher holds. That is the whole point, and it is the property two + /// earlier versions of this file lacked: checking that a key payload does not open as a host is true + /// however both ciphers are misconfigured, because Seal and TryOpen share one constant. A + /// test that compares an implementation against itself cannot catch a self-consistent mistake. /// /// - /// 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. + /// Written as a table over every cipher, not one test per cipher, because the same hole was found three + /// times — twice by mutation testing after the fact. Pointing CredentialCipher at + /// AadResourceType.Vault passed the entire suite until this existed. /// /// - [Fact] - public void AKeyPayload_OpensUnderTheResourceTypeTheSpecificationNames() + [Theory] + [MemberData(nameof(Pinned))] + public void EveryCipher_SealsUnderTheResourceTypeTheSpecificationNames( + SyncEntityType wire, + CryptoSpec.AadResourceType resource) { var vaultKey = RandomNumberGenerator.GetBytes(32); @@ -76,51 +109,91 @@ public sealed class AadResourceTypeTests const uint Generation = 1; const uint Version = 1; - var payload = SshKeyCipher.Seal(NewKey(), vaultKey, entityId, Generation, (int)Version); + var payload = SealSample(wire, vaultKey, entityId, Generation, (int)Version); var dataKey = ItemKeys.TryUnwrapDataKey( - vaultKey, - payload.WrappedDataKey, - CryptoSpec.AadResourceType.SshKey, - entityId, - Generation, - Version); + vaultKey, payload.WrappedDataKey, resource, 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."); + $"The {wire} cipher must wrap its data key under AadResourceType.{resource}; a null here means " + + "it used some other resource type, which round-trips fine and violates docs/crypto.md."); ItemKeys.TryOpenPayload( dataKey, payload.Envelope, - CryptoSpec.AadResourceType.SshKey, + resource, 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. + /// + /// The guard that makes the table above self-maintaining. A fourth item type would otherwise sync, + /// encrypt and merge correctly while being sealed under any resource type at all, and nothing would say + /// so until another implementation refused the item — by which point the AAD is frozen into stored + /// ciphertext and only clients can re-encrypt it. + /// [Fact] - public void AHostPayload_OpensUnderTheResourceTypeTheSpecificationNames() + public void EverySynchronisedType_HasItsCipherPinnedHere() + { + PinnedPairs.Select(pair => pair.Wire) + .ShouldBe(ItemKinds.SyncedTypes, ignoreOrder: true); + } + + private static EncryptedPayload SealSample( + SyncEntityType wire, + byte[] vaultKey, + Guid entityId, + uint generation, + int version) => wire switch + { + SyncEntityType.Host => HostCipher.Seal( + new HostSecret { Label = "prod-db", Hostname = "db.internal" }, + vaultKey, + entityId, + generation, + version), + + SyncEntityType.SshKey => SshKeyCipher.Seal( + NewKey(), vaultKey, entityId, generation, version), + + SyncEntityType.Credential => CredentialCipher.Seal( + NewCredential(), vaultKey, entityId, generation, version), + + _ => throw new ArgumentOutOfRangeException( + nameof(wire), + wire, + "No sample exists for this item type. Add one when adding the type, or the pairing above " + + "cannot be checked."), + }; + + [Fact] + public void ACredentialPayload_OpensAsNothingElse() { 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); + var sealed_ = CredentialCipher.Seal( + NewCredential(), vaultKey, entityId, keyGeneration: 1, itemVersion: 1); - ItemKeys.TryUnwrapDataKey( - vaultKey, - payload.WrappedDataKey, - CryptoSpec.AadResourceType.Host, - entityId, - Generation, - Version) - .ShouldNotBeNull("HostCipher must wrap the data key under AadResourceType.Host."); + HostCipher.TryOpen(sealed_, vaultKey, entityId, itemVersion: 1).ShouldBeNull(); + SshKeyCipher.TryOpen(sealed_, vaultKey, entityId, itemVersion: 1).ShouldBeNull(); + CredentialCipher.TryOpen(sealed_, vaultKey, entityId, itemVersion: 1).ShouldNotBeNull(); + } + + [Fact] + public void ACredentialSealedAtOneVersion_DoesNotOpenAtAnother() + { + var vaultKey = RandomNumberGenerator.GetBytes(32); + + var entityId = Guid.CreateVersion7(); + + var payload = CredentialCipher.Seal( + NewCredential(), vaultKey, entityId, keyGeneration: 1, itemVersion: 2); + + CredentialCipher.TryOpen(payload, vaultKey, entityId, itemVersion: 3).ShouldBeNull(); } [Fact] @@ -183,6 +256,14 @@ public sealed class AadResourceTypeTests SshKeyCipher.TryOpen(payload, vaultKey, entityId, itemVersion: 3).ShouldBeNull(); } + private static CredentialSecret NewCredential() => new() + { + Label = "db-login", + Password = "hunter2", + Username = "postgres", + Notes = "used by CI", + }; + private static SshKeySecret NewKey() => new() { Label = "deploy",