Public Access
Mutation testing found that pointing CredentialCipher at AadResourceType.Vault passed the entire suite. Every credential test compared the cipher against itself — round trips, cross-type refusals, two-machine sync — and all of those stay true when both halves of one cipher are wrong together, because Seal and TryOpen share the constant. A password sealed under the resource type for a vault encrypts cleanly, decrypts cleanly, syncs cleanly, and violates docs/crypto.md in a way nothing surfaces until another implementation refuses the item. By then the AAD is frozen into stored ciphertext and only clients can re-encrypt it. This is the third time that hole has appeared in this file, and the second time mutation testing rather than review is what found it. So the fix is structural rather than another hand-written test: one table of wire type to resource type, a theory that seals a sample through each cipher and opens it with the resource type the table names — never the one the cipher holds — and a guard asserting the table covers ItemKinds.SyncedTypes. A fourth item type can no longer be added without pinning its resource type: the coverage test fails, and the sample switch throws with an explanation. The two per-cipher tests it replaces said the same thing for hosts and keys, so nothing is lost and the credential row is no longer something someone has to remember. Verified by re-running the mutation matrix. All seven sabotages are now detected: the credential merge dropping its redaction, the key/credential exclusivity check disabled, the schema version ladder flattened so a key-bound host claims the credential version, a credential sending the server an empty fields record instead of none, CredentialKind claiming to be a host, CredentialCipher sealing under the wrong resource type, and the credential noun reading "host". Two of those were unproven before this run — one because the earlier sabotage did not compile, and one because it was genuinely undetected. Sync.Tests 88/88, Domain.Tests 117/117. Zero warnings, dotnet format clean.
276 lines
11 KiB
C#
276 lines
11 KiB
C#
using System.Security.Cryptography;
|
|
using DodoSSH.Client.Domain;
|
|
using DodoSSH.Client.Sync;
|
|
using DodoSSH.Contracts;
|
|
using DodoSSH.Crypto;
|
|
|
|
namespace DodoSSH.Client.Sync.Tests;
|
|
|
|
/// <summary>
|
|
/// Each item type must be sealed under its own AAD resource type, and the two enums that name item types
|
|
/// deliberately do not agree.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// <para>
|
|
/// <c>SyncEntityType</c> lists only syncable items, so <c>Host</c> is 1 and <c>SshKey</c> is 3.
|
|
/// <c>CryptoSpec.AadResourceType</c> 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.
|
|
/// </para>
|
|
/// <para>
|
|
/// 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.
|
|
/// </para>
|
|
/// </remarks>
|
|
public sealed class AadResourceTypeTests
|
|
{
|
|
/// <remarks>
|
|
/// The pairing stated as a table. If <c>AadResourceType</c> is ever renumbered, this is what says so —
|
|
/// loudly, and before anything is written under the new numbers.
|
|
/// </remarks>
|
|
[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.");
|
|
}
|
|
|
|
/// <summary>
|
|
/// The specified pairing of wire type to AAD resource type, stated out of band, one row per cipher.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// 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
|
|
/// <see cref="EverySynchronisedType_HasItsCipherPinnedHere"/>.
|
|
/// </remarks>
|
|
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<SyncEntityType, CryptoSpec.AadResourceType> Pinned
|
|
{
|
|
get
|
|
{
|
|
var data = new TheoryData<SyncEntityType, CryptoSpec.AadResourceType>();
|
|
|
|
foreach (var (wire, resource) in PinnedPairs)
|
|
{
|
|
data.Add(wire, resource);
|
|
}
|
|
|
|
return data;
|
|
}
|
|
}
|
|
|
|
/// <remarks>
|
|
/// <para>
|
|
/// Opened <em>independently</em>, through the low-level <c>ItemKeys</c> API with the resource type this
|
|
/// 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 <c>Seal</c> and <c>TryOpen</c> share one constant. A
|
|
/// test that compares an implementation against itself cannot catch a self-consistent mistake.
|
|
/// </para>
|
|
/// <para>
|
|
/// 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 <c>CredentialCipher</c> at
|
|
/// <c>AadResourceType.Vault</c> passed the entire suite until this existed.
|
|
/// </para>
|
|
/// </remarks>
|
|
[Theory]
|
|
[MemberData(nameof(Pinned))]
|
|
public void EveryCipher_SealsUnderTheResourceTypeTheSpecificationNames(
|
|
SyncEntityType wire,
|
|
CryptoSpec.AadResourceType resource)
|
|
{
|
|
var vaultKey = RandomNumberGenerator.GetBytes(32);
|
|
|
|
var entityId = Guid.CreateVersion7();
|
|
const uint Generation = 1;
|
|
const uint Version = 1;
|
|
|
|
var payload = SealSample(wire, vaultKey, entityId, Generation, (int)Version);
|
|
|
|
var dataKey = ItemKeys.TryUnwrapDataKey(
|
|
vaultKey, payload.WrappedDataKey, resource, entityId, Generation, Version);
|
|
|
|
dataKey.ShouldNotBeNull(
|
|
$"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,
|
|
resource,
|
|
entityId,
|
|
payload.DataKeyId,
|
|
Generation,
|
|
Version).ShouldNotBeNull("and it must seal the envelope under the same resource type.");
|
|
}
|
|
|
|
/// <remarks>
|
|
/// 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.
|
|
/// </remarks>
|
|
[Fact]
|
|
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();
|
|
|
|
var sealed_ = CredentialCipher.Seal(
|
|
NewCredential(), vaultKey, entityId, keyGeneration: 1, itemVersion: 1);
|
|
|
|
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]
|
|
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 CredentialSecret NewCredential() => new()
|
|
{
|
|
Label = "db-login",
|
|
Password = "hunter2",
|
|
Username = "postgres",
|
|
Notes = "used by CI",
|
|
};
|
|
|
|
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",
|
|
};
|
|
}
|