Public Access
Sync and authenticate with SSH keys on the client
Completes the client half of SSH keys: they sync alongside hosts, appear in their own list, and can be selected to authenticate a connection instead of typing a password. The reconciler and the repository were Host-typed throughout, so the choice was to generalise them or to keep a second copy per item type. Generalised, because ItemReconciler's whole premise is that the pull and the push paths must answer the same collision the same way — two copies would drift the first time one of them was fixed. What is genuinely per-type now arrives through IItemKind<TSecret>: the cipher, the merge, the plaintext columns, and the noun to use when telling a person what happened to their item. Generic where the server's IItemKind is not, and for the reason that reverses there — the client needs the concrete type, because it merges field by field. The pull filter is derived from the same registry that builds the reconcilers. That is the specific failure being designed out: an item type that encrypts, merges and lists perfectly and is never once requested from the server, so it works on the machine that made it and exists nowhere else. No client cache migration. The item table's primary key and the outbox's unique index already carry the entity type, and AadResourceTypes already mapped SshKey — so a host and a key may share an id and never see each other's rows, which SshKeySyncTests now arranges deliberately. A key hands the server nothing in plaintext. There is a public_key_fingerprint column and it would be accepted; leaving it null is deliberate. A fingerprint is not secret but it is a stable identifier for a key pair, so filling it would let an operator tell which of their users hold the same key and correlate one across vaults, for a column nothing reads. The design allows itself one plaintext concession — the relay address, which the relay cannot work without — and this is not that. A key is chosen per connection rather than bound to a host, which works the way ssh -i does. Binding one needs a field on HostSecret and therefore a payload schema bump, which makes every host written afterwards read-only on an older build; worth doing deliberately rather than as a side effect of adding keys. Three things this found, all of them by being falsified rather than by review: - Making the reconciler generic silently turned a record comparison into reference equality, because == on a type parameter is not value equality. The effect would have been a conflict recorded on every pass for an unacknowledged create that had in fact landed. Sabotaging the fix left all 73 tests passing — nothing covered that branch — so ConflictMatrixTests now has AnUnacknowledgedCreateThatDidLand_IsDroppedQuietly, which fails without it. - A test asserting that a blank passphrase reaches SSH.NET as null was vacuous: it exercised the editor, not the credential path, and passed with the guard deleted. Resolved by making SshKeySecret.Passphrase normalise an empty string to null, so there is one spelling of one state — which also keeps two clients from producing different payload bytes for an identical key. That exposed a wider gap: SshKeySecret, its codec and its merge had no direct unit tests at all. They have 25 now. - The reason first given for that normalisation was false. It claimed SSH.NET rejects a passphrase supplied for an unprotected key; measured against a real sshd it ignores it and authenticates anyway. Corrected everywhere it was stated and recorded in docs/platform-flags.md. The same test file also closes a real hole: SshPrivateKeyCredential had never been exercised against a server, because the existing key test builds SSH.NET's auth method directly and bypasses the path a vault-held key actually takes. Only one editor may be open at a time. Both sit in the same 340-pixel column as Auto rows and their heights together exceed it at the window's minimum size, so two open editors put the lower one's Save and Cancel past the bottom edge — the same failure this window already shipped once with the setup screens. Expressed as a state rule because that is the only form of it this repository can check: nothing here loads a .axaml. The refusal keeps what was typed, since in the key editor that is a pasted private key the user may have nowhere else. The end-to-end slice now carries a key as well as a host, so both item types go through the real API, the real PostgreSQL and the real crypto in one pass — the three hand-kept mappings between enums that do not line up are the reason that is worth doing rather than trusting the unit suites. 735 tests green, including the container-backed SSH and end-to-end suites. Zero warnings, dotnet format clean.
This commit is contained in:
jaap-jan committed
1 parent
586cb303d5
commit
e3fd3e1728
26 files changed
+2804
-505
No files matched your search
@@ -0,0 +1,250 @@
|
||||
using DodoSSH.Client.Domain;
|
||||
using DodoSSH.Client.Storage;
|
||||
using DodoSSH.Contracts;
|
||||
|
||||
namespace DodoSSH.Client.Sync;
|
||||
|
||||
/// <summary>A decrypted item, and whether this build may write it back.</summary>
|
||||
/// <param name="Secret">The item.</param>
|
||||
/// <param name="IsReadOnly">
|
||||
/// Whether a newer client wrote it, in which case re-encoding it here would drop fields this build has
|
||||
/// no concept of.
|
||||
/// </param>
|
||||
internal sealed record OpenedItem<TSecret>(TSecret Secret, bool IsReadOnly)
|
||||
where TSecret : class, IVaultSecret;
|
||||
|
||||
/// <summary>A merged item, and everything that had to be overridden to produce it.</summary>
|
||||
/// <param name="Merged">The item to store and push.</param>
|
||||
/// <param name="Conflicts">Empty when the two sides were reconcilable field by field.</param>
|
||||
internal sealed record MergedItem<TSecret>(
|
||||
TSecret Merged,
|
||||
IReadOnlyList<HostFieldConflict> Conflicts)
|
||||
where TSecret : class, IVaultSecret;
|
||||
|
||||
/// <summary>
|
||||
/// Everything about one item type that the shared sync path cannot know.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The reconciler holds the six answers a collision can have — merge, adopt, resurrect, abandon, park,
|
||||
/// refuse — and every one of them is identical for a host and for an SSH key. Only the encoding, the
|
||||
/// merge and the plaintext columns differ, and those arrive through here. A second copy of the
|
||||
/// reconciler per item type is the alternative, and it is not a real one: the file's whole premise is
|
||||
/// that the pull and push paths must answer the same situation the same way, and two copies would drift
|
||||
/// the moment one of them was fixed.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Generic, unlike the server's <c>IItemKind</c>, and for a reason that reverses there: the client
|
||||
/// <em>does</em> need the concrete type. It merges two versions of an item field by field and hands the
|
||||
/// result to a codec, so erasing the type would only move the downcasts inside the reconciler, where
|
||||
/// they would be a cast per branch instead of none.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal interface IItemKind<TSecret>
|
||||
where TSecret : class, IVaultSecret
|
||||
{
|
||||
/// <summary>The type as the wire contract names it.</summary>
|
||||
SyncEntityType EntityType { get; }
|
||||
|
||||
/// <summary>
|
||||
/// What to call one of these when telling a person what happened to it.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Lower case and singular, because every use is mid-sentence. This exists because the conflict log
|
||||
/// is read by people: "this host could not be decrypted" is actively misleading when the item was a
|
||||
/// private key, and a user who is told the wrong noun looks in the wrong place.
|
||||
/// </remarks>
|
||||
string Noun { get; }
|
||||
|
||||
/// <inheritdoc cref="HostCipher.TryOpen" />
|
||||
OpenedItem<TSecret>? TryOpen(
|
||||
EncryptedPayload payload,
|
||||
ReadOnlySpan<byte> vaultKey,
|
||||
Guid entityId,
|
||||
int itemVersion);
|
||||
|
||||
/// <inheritdoc cref="HostCipher.Seal" />
|
||||
EncryptedPayload Seal(
|
||||
TSecret secret,
|
||||
ReadOnlySpan<byte> vaultKey,
|
||||
Guid entityId,
|
||||
uint keyGeneration,
|
||||
int itemVersion);
|
||||
|
||||
/// <summary>
|
||||
/// The plaintext columns the server gets, or null when this type gives it nothing.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Nullable rather than an all-defaults record, because the difference is visible on the wire and to
|
||||
/// a reader: <c>SyncPlaintextFields</c> with nothing set still serialises <c>relayEnabled: false</c>,
|
||||
/// which invites the belief that the type has a relay setting which happens to be off.
|
||||
/// </remarks>
|
||||
SyncPlaintextFields? Fields(TSecret secret);
|
||||
|
||||
/// <summary>Merges two divergent versions against the version they both started from.</summary>
|
||||
MergedItem<TSecret> Merge(TSecret ancestor, TSecret local, TSecret remote);
|
||||
|
||||
/// <summary>The same item under a new name, for a resurrection.</summary>
|
||||
TSecret Relabel(TSecret secret, string label);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The item types this client synchronises.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>One list, and the pull filter is derived from it.</b> The engine asks the server for exactly the
|
||||
/// types in <see cref="Registry"/> and refuses to apply a change of any other type, so adding a kind
|
||||
/// cannot leave the filter behind — which is the specific way this would otherwise break: an item type
|
||||
/// that reads and writes perfectly in every unit test and is never once requested from the server.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A reconciler per type rather than one shared instance, because each closes the generic over its own
|
||||
/// secret type. They are cheap — four fields and no state — and building them once per engine keeps the
|
||||
/// per-change path a dictionary lookup.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static class ItemKinds
|
||||
{
|
||||
private static readonly (SyncEntityType Type, ReconcilerFactory Create)[] Registry =
|
||||
[
|
||||
(SyncEntityType.Host, static (outbox, conflicts, keyring) =>
|
||||
new ItemReconciler<HostSecret>(HostKind.Instance, outbox, conflicts, keyring)),
|
||||
|
||||
(SyncEntityType.SshKey, static (outbox, conflicts, keyring) =>
|
||||
new ItemReconciler<SshKeySecret>(SshKeyKind.Instance, outbox, conflicts, keyring)),
|
||||
];
|
||||
|
||||
/// <summary>The types to ask the server for, in a fixed order.</summary>
|
||||
internal static IReadOnlyList<SyncEntityType> SyncedTypes { get; } =
|
||||
[.. Registry.Select(entry => entry.Type)];
|
||||
|
||||
private delegate IItemReconciler ReconcilerFactory(
|
||||
OutboxStore outbox,
|
||||
ConflictStore conflicts,
|
||||
VaultKeyring keyring);
|
||||
|
||||
/// <summary>Builds one reconciler per synchronised type.</summary>
|
||||
internal static Dictionary<SyncEntityType, IItemReconciler> Reconcilers(
|
||||
OutboxStore outbox,
|
||||
ConflictStore conflicts,
|
||||
VaultKeyring keyring) =>
|
||||
Registry.ToDictionary(
|
||||
entry => entry.Type,
|
||||
entry => entry.Create(outbox, conflicts, keyring));
|
||||
}
|
||||
|
||||
/// <summary>Hosts.</summary>
|
||||
internal sealed class HostKind : IItemKind<HostSecret>
|
||||
{
|
||||
internal static HostKind Instance { get; } = new();
|
||||
|
||||
/// <inheritdoc />
|
||||
public SyncEntityType EntityType => SyncEntityType.Host;
|
||||
|
||||
/// <inheritdoc />
|
||||
public string Noun => "host";
|
||||
|
||||
/// <inheritdoc />
|
||||
public OpenedItem<HostSecret>? TryOpen(
|
||||
EncryptedPayload payload,
|
||||
ReadOnlySpan<byte> vaultKey,
|
||||
Guid entityId,
|
||||
int itemVersion)
|
||||
{
|
||||
var document = HostCipher.TryOpen(payload, vaultKey, entityId, itemVersion);
|
||||
|
||||
return document is null ? null : new OpenedItem<HostSecret>(document.Host, document.IsReadOnly);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public EncryptedPayload Seal(
|
||||
HostSecret secret,
|
||||
ReadOnlySpan<byte> vaultKey,
|
||||
Guid entityId,
|
||||
uint keyGeneration,
|
||||
int itemVersion) =>
|
||||
HostCipher.Seal(secret, vaultKey, entityId, keyGeneration, itemVersion);
|
||||
|
||||
/// <inheritdoc />
|
||||
public SyncPlaintextFields? Fields(HostSecret secret) => HostFields.From(secret);
|
||||
|
||||
/// <inheritdoc />
|
||||
public MergedItem<HostSecret> Merge(HostSecret ancestor, HostSecret local, HostSecret remote)
|
||||
{
|
||||
var merged = HostSecretMerge.Merge(ancestor, local, remote);
|
||||
|
||||
return new MergedItem<HostSecret>(merged.Merged, merged.Conflicts);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public HostSecret Relabel(HostSecret secret, string label)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(secret);
|
||||
|
||||
return secret with { Label = label };
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>SSH keys.</summary>
|
||||
internal sealed class SshKeyKind : IItemKind<SshKeySecret>
|
||||
{
|
||||
internal static SshKeyKind Instance { get; } = new();
|
||||
|
||||
/// <inheritdoc />
|
||||
public SyncEntityType EntityType => SyncEntityType.SshKey;
|
||||
|
||||
/// <inheritdoc />
|
||||
public string Noun => "SSH key";
|
||||
|
||||
/// <inheritdoc />
|
||||
public OpenedItem<SshKeySecret>? TryOpen(
|
||||
EncryptedPayload payload,
|
||||
ReadOnlySpan<byte> vaultKey,
|
||||
Guid entityId,
|
||||
int itemVersion)
|
||||
{
|
||||
var document = SshKeyCipher.TryOpen(payload, vaultKey, entityId, itemVersion);
|
||||
|
||||
return document is null ? null : new OpenedItem<SshKeySecret>(document.Key, document.IsReadOnly);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public EncryptedPayload Seal(
|
||||
SshKeySecret secret,
|
||||
ReadOnlySpan<byte> vaultKey,
|
||||
Guid entityId,
|
||||
uint keyGeneration,
|
||||
int itemVersion) =>
|
||||
SshKeyCipher.Seal(secret, vaultKey, entityId, keyGeneration, itemVersion);
|
||||
|
||||
/// <summary>
|
||||
/// Nothing at all.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The server has a <c>public_key_fingerprint</c> column and would accept one here, and this client
|
||||
/// deliberately declines to fill it. A fingerprint is not a secret, but it is a stable identifier for
|
||||
/// a key pair, and handing it over would let the operator tell which of their users hold the same key
|
||||
/// and correlate one key across vaults — for a column nothing in the product reads. The design allows
|
||||
/// itself exactly one plaintext concession, the relay address, and it is a concession because the
|
||||
/// relay cannot work without it. This is not that. See ADR 0004.
|
||||
/// </remarks>
|
||||
/// <inheritdoc />
|
||||
public SyncPlaintextFields? Fields(SshKeySecret secret) => null;
|
||||
|
||||
/// <inheritdoc />
|
||||
public MergedItem<SshKeySecret> Merge(SshKeySecret ancestor, SshKeySecret local, SshKeySecret remote)
|
||||
{
|
||||
var merged = SshKeySecretMerge.Merge(ancestor, local, remote);
|
||||
|
||||
return new MergedItem<SshKeySecret>(merged.Merged, merged.Conflicts);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public SshKeySecret Relabel(SshKeySecret secret, string label)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(secret);
|
||||
|
||||
return secret with { Label = label };
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user