diff --git a/README.md b/README.md index 72534e8..9da831c 100644 --- a/README.md +++ b/README.md @@ -132,10 +132,16 @@ private key, then edit a host and pick that key from its **key** dropdown. From authenticates with it — on every machine, since the choice travels inside the host's encrypted payload — and its password box disappears. -Three of M1's known gaps are visible immediately, so they are worth expecting rather than diagnosing: -password authentication asks for the password every time, because credentials are not a synced entity type -yet (keys are — passwords are not); host key trust lasts one session, because known hosts do not live in the -vault yet; and unlock asks for the passphrase on every launch, because no device key is registered. +The first time you connect to a host you are asked to check its key fingerprint. That decision is stored in +the vault, so it is asked once per host rather than once per launch and it reaches your other machines with +the next sync. If a server is legitimately rebuilt and offers a new key, the connection is refused outright +with no way to continue from the warning — edit the host and choose **Forget host key**, which is deliberately +somewhere you have to go on purpose. + +Two of M1's known gaps are visible immediately, so they are worth expecting rather than diagnosing: password +authentication asks for the password every time, because nothing in the interface can create a vault +credential yet (they do sync — there is just no editor for one); and unlock asks for the passphrase on every +launch, because no device key is registered. ### End-to-end verification @@ -148,9 +154,9 @@ dotnet test tests/DodoSSH.SystemTests It brings up PostgreSQL, Keycloak and an OpenSSH server in containers, applies the committed migrations, starts the API as a child process out of its own build output, and then drives the real client: sign in -through Keycloak, enroll, unlock, create a host, sync it, read it back on a second simulated machine, -unlock again with no network, and open a shell on the `sshd`. Roughly 25 seconds once the images are -pulled. +through Keycloak, enroll, unlock, create an SSH key and a host bound to it, sync them, open a shell on the +`sshd` and approve its host key at the real first-contact refusal, then read all three back on a second +simulated machine and unlock again with no network. Roughly 25 seconds once the images are pulled. What makes it worth its weight is that it consumes the artefacts that ship — the realm file from `deploy/keycloak`, the EF migrations, the API's own `appsettings` — rather than a fixture written to match @@ -198,14 +204,19 @@ off-Windows. they are unverified. *Verified end to end:* `tests/DodoSSH.SystemTests` drives the whole slice against a real Keycloak, a real API, a real PostgreSQL and a real `sshd` — sign-in, the identity-provider key binding, enrollment, - offline unlock, a host through the vault to a second machine, and an interactive shell. See + offline unlock, a host and an SSH key through the vault to a second machine, an interactive shell, and the + host key approved at that shell's prompt reaching the second machine as well. See [End-to-end verification](#end-to-end-verification). - Known gaps in the client, stated rather than implied by the interface: credentials are not a synced - entity type yet, so password authentication still asks for the password each time — SSH keys *are* - synced, and binding one to a host is the way to connect without typing anything; known host keys live in - memory for one session instead of in the vault; and no device key is registered, so the passphrase is - needed on every launch until the OS keystore is wired. + Known gaps in the client, stated rather than implied by the interface: nothing in the interface can create + a vault credential yet, so password authentication still asks for the password each time — SSH keys *are* + editable, and binding one to a host is the way to connect without typing anything; and no device key is + registered, so the passphrase is needed on every launch until the OS keystore is wired. + + Host key trust *is* in the vault, which is what makes trust-on-first-use worth having: a fingerprint + approved on one machine is approved on all of them and survives a restart, and the server cannot drop a + pin to force a fresh first-use decision without the item visibly going missing. A changed host key stays a + hard refusal with no way past it; withdrawing a pin is a separate, deliberate act in the host's editor. Binding a key introduced the first payload schema version bump, and it is worth knowing how it behaves: a host is written at the *lowest* schema version that can represent it, so only hosts that actually bind diff --git a/src/DodoSSH.Api/Features/Sync/ItemKinds.cs b/src/DodoSSH.Api/Features/Sync/ItemKinds.cs index b25f2b6..9f2af5b 100644 --- a/src/DodoSSH.Api/Features/Sync/ItemKinds.cs +++ b/src/DodoSSH.Api/Features/Sync/ItemKinds.cs @@ -68,8 +68,10 @@ internal interface IItemKind internal static class ItemKinds { private static readonly Dictionary Supported = - new[] { (IItemKind)new HostKind(), new SshKeyKind(), new CredentialKind() } - .ToDictionary(kind => kind.WireType); + new[] + { + (IItemKind)new HostKind(), new SshKeyKind(), new CredentialKind(), new KnownHostKeyKind(), + }.ToDictionary(kind => kind.WireType); /// The kind for a wire type, or null when this server does not synchronise it yet. /// @@ -295,9 +297,9 @@ internal sealed class SshKeyKind : IItemKind /// Credentials: an envelope and nothing else. /// -/// The strictest of the three kinds about plaintext, and the reason is not symmetry. A key at least has a -/// fingerprint that is public by nature; a password has no part that is safe to expose, so this kind accepts -/// no plaintext fields at all and hydrates none. +/// As strict as a kind gets about plaintext — is the other one — and the +/// reason is not symmetry. A key at least has a fingerprint that is public by nature; a password has no part +/// that is safe to expose, so this kind accepts no plaintext fields at all and hydrates none. /// internal sealed class CredentialKind : IItemKind { @@ -387,3 +389,101 @@ internal sealed class CredentialKind : IItemKind /// public SyncPlaintextFields? Hydrate(IVaultItem item) => null; } + +/// Known host keys: an envelope and nothing else. +/// +/// As strict as about plaintext, for a reason that is about aggregation rather +/// than secrecy. A fingerprint is published so it can be compared and an address may already sit in a +/// relay-enabled host's columns — but the set of endpoints one user has approved is a map of their estate, +/// and this server has nothing to do with it. +/// +internal sealed class KnownHostKeyKind : IItemKind +{ + /// + public SyncEntityType WireType => SyncEntityType.KnownHostKey; + + /// + public ChangeEntityType ChangeType => ChangeEntityType.KnownHostKey; + + /// + public async Task FindAsync( + DodoDbContext database, + Guid id, + CancellationToken cancellationToken) => + await database.KnownHostKeys.SingleOrDefaultAsync(k => k.Id == id, cancellationToken) + .ConfigureAwait(false); + + /// + public async Task> LoadAsync( + DodoDbContext database, + Guid vaultId, + Guid[] ids, + CancellationToken cancellationToken) + { + var rows = await database.KnownHostKeys + .Where(k => k.VaultId == vaultId && ids.Contains(k.Id)) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + + return rows.ToDictionary(row => row.Id, row => (IVaultItem)row); + } + + /// + public IVaultItem Add(DodoDbContext database, Guid id, Guid vaultId) + { + var knownHost = new VaultKnownHostKey { Id = id, VaultId = vaultId }; + + database.KnownHostKeys.Add(knownHost); + + return knownHost; + } + + /// + /// Refuses every plaintext field there is. + /// + /// + /// The relay fields are refused although this type is the one that does hold an address, and + /// that is the point: the address belongs in the ciphertext. A client sending it here is either confused + /// or trying to get the server to keep a list it has no business keeping, and either way it should be + /// told rather than have the value quietly dropped. + /// + /// + public bool ValidateFields(SyncPlaintextFields fields, out string error) + { + ArgumentNullException.ThrowIfNull(fields); + + error = string.Empty; + + if (fields.RelayEnabled || fields.Hostname is not null || fields.Port is not null) + { + error = "A known host key is not something the server dials; its address stays encrypted."; + return false; + } + + if (fields.PublicKeyFingerprint is not null) + { + error = "A known host key's fingerprint stays inside its payload."; + return false; + } + + return true; + } + + /// Nothing to copy: this type has no plaintext columns to copy anything into. + /// + public void ApplyFields(IVaultItem item, SyncPlaintextFields fields) + { + } + + /// + public void ClearFieldsOnDelete(IVaultItem item) + { + } + + /// + /// Always null, as for a credential: this type has no plaintext columns, so there is nothing a pull could + /// hydrate even in principle. + /// + /// + public SyncPlaintextFields? Hydrate(IVaultItem item) => null; +} diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index 4edf364..425a2ac 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -52,10 +52,11 @@ internal sealed partial class DodoSshApp : Application var paths = ClientPaths.Default; var caches = ClientCacheFactory.ForFile(paths.CacheFile); - // Known hosts are still in memory. The plan puts them in the vault as a synced entity so trust - // follows the user to every device, and SyncEntityType.KnownHostKey is reserved for it — but that - // entity type is not synced yet, so trust currently lasts one session. - var knownHosts = new InMemoryKnownHostStore(); + // Known hosts live in the vault, so trust survives a restart and follows the user to every device. + // Composed here, once, because the connection factory below needs it now and outlives every unlock; + // the vault behind it is attached and detached as one is opened and locked. See VaultKnownHostStore + // for why the handshake is answered from a snapshot rather than by reading the vault per lookup. + var knownHosts = new VaultKnownHostStore(); var workspace = new TerminalWorkspace( new AvaloniaTerminalAssetProvider(), diff --git a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs index f1af6f4..ae549c8 100644 --- a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs @@ -3,7 +3,6 @@ using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; using DodoSSH.Client.Auth; using DodoSSH.Client.Session; -using DodoSSH.Client.Ssh; using DodoSSH.Client.Storage; using DodoSSH.Client.Terminal; using DodoSSH.Crypto; @@ -60,7 +59,15 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp private readonly ClientPaths paths; private readonly ClientCacheFactory caches; private readonly TerminalWorkspace workspace; - private readonly IKnownHostStore knownHosts; + + /// + /// The concrete store rather than IKnownHostStore, because this is where its lifecycle belongs: + /// the interface is what the handshake asks, and opening a vault behind it, refreshing it and forgetting + /// it are this state machine's business. The same instance was handed to the connection factory when the + /// application was composed. + /// + private readonly VaultKnownHostStore knownHosts; + private readonly SignInHandler signIn; private readonly TimeProvider clock; private readonly Argon2Profile? passphraseProfile; @@ -83,7 +90,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp ClientPaths paths, ClientCacheFactory caches, TerminalWorkspace workspace, - IKnownHostStore knownHosts, + VaultKnownHostStore knownHosts, SignInHandler signIn, TimeProvider clock, Argon2Profile? passphraseProfile = null) @@ -355,6 +362,22 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp Passphrase = string.Empty; + // Before the vault view model, so the first connection after an unlock already knows which + // host keys this user has approved. Reading them is one listing; doing it here rather than + // lazily is what keeps it off the SSH handshake thread. + try + { + await knownHosts.OpenAsync(outcome.Session!, cancellationToken).ConfigureAwait(true); + } + catch + { + // Nothing owns the session yet, so nothing else would ever dispose it — and an + // undisposed session is vault keys left in memory for the life of the process, which is + // precisely what unlocking must be able to undo. + await outcome.Session!.DisposeAsync().ConfigureAwait(true); + throw; + } + Vault = new VaultViewModel(outcome.Session!, workspace, knownHosts, () => connection); State = ShellState.Unlocked; @@ -398,6 +421,11 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp [RelayCommand] private async Task LockAsync() { + // First, and before the session it read from goes: a synchronisation pass may be in flight, and it + // ends by refreshing this store. Detaching now makes that refresh a no-op instead of a set of pins + // reappearing behind a lock screen. + knownHosts.Close(); + if (Vault is { } open) { Vault = null; @@ -420,6 +448,8 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp disposed = true; + knownHosts.Close(); + if (Vault is { } open) { await open.DisposeAsync().ConfigureAwait(false); diff --git a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs index 2effe80..2c96901 100644 --- a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs @@ -173,10 +173,12 @@ internal sealed class ConflictRowViewModel(ConflictNotice notice) /// A background pass is deliberately quieter than the button: see . /// /// -/// Keys are in the vault; passwords are not. An SSH key is a synced item, so it is stored once and -/// available on every machine. SyncEntityType.Credential exists in the contract and is still not -/// synced, so password authentication asks for the password each time. That is a real M1 limitation rather -/// than a design choice, and the interface says so rather than implying the vault holds more than it does. +/// Keys and host key trust are in the vault; passwords are not yet. An SSH key is a synced item, so +/// it is stored once and available on every machine, and so is a known host key — approving a fingerprint +/// here approves it on every device and survives a restart. Credentials are synced as well, but nothing in +/// this interface can create one, so password authentication still asks each time. That is a real M1 +/// limitation rather than a design choice, and the interface says so rather than implying the vault holds +/// more than it does. /// /// /// A key belongs to a host. Each host names the key it authenticates with, or none, and that choice @@ -188,7 +190,7 @@ internal sealed class ConflictRowViewModel(ConflictNotice notice) internal sealed partial class VaultViewModel( VaultSession session, TerminalWorkspace workspace, - IKnownHostStore knownHosts, + VaultKnownHostStore knownHosts, Func connection) : ObservableObject, IAsyncDisposable { /// @@ -276,6 +278,17 @@ internal sealed partial class VaultViewModel( /// The item being edited, or null when creating. private Guid? editingEntityId; + /// + /// Whether the editor is showing a host that could have a pinned key to forget. + /// + /// + /// Read by the editor to hide the button while a host is being created, where there is nothing to + /// withdraw yet. It does not claim a pin exists — answering that would mean a second question to the + /// known-host store for a button's visibility, and the command already says plainly when there was + /// nothing to forget. + /// + internal bool CanForgetHostKey => IsEditing && editingEntityId is not null; + // ---- The key editor ---- // A second set of editor state rather than a shared one. The two editors hold unrelated fields, and // sharing them would mean a half-typed host reappearing inside a key editor. @@ -310,7 +323,8 @@ internal sealed partial class VaultViewModel( // ---- Connecting ---- /// - /// Typed per connection because credentials are not a synced entity type yet. Never persisted. + /// Typed per connection because nothing in this interface can create a vault credential yet — not because + /// the vault cannot hold one. Never persisted. /// [ObservableProperty] private string connectPassword = string.Empty; @@ -531,6 +545,11 @@ internal sealed partial class VaultViewModel( await ReloadAsync(cancellationToken).ConfigureAwait(true); + // Host key trust arrives with the rest of the vault, and the store the SSH handshake asks holds a + // snapshot rather than reading per lookup — so a pass that pulled a pin has to hand it over here, + // or a host a colleague approved stays a first-contact prompt until the next unlock. + await knownHosts.RefreshAsync(cancellationToken).ConfigureAwait(true); + return report; } finally @@ -854,7 +873,15 @@ internal sealed partial class VaultViewModel( () => OpenSessionAsync(row, credential, cancellationToken)).ConfigureAwait(true); } - /// Pins the offered host key and retries. + /// + /// Pins the offered host key and retries. + /// + /// + /// The pin goes into the vault, so this writes to the local cache and queues a change for every other + /// machine — which is why the write is guarded and the connection is only retried once it has landed. + /// It used to be a dictionary insert that could not fail; reporting a failed write as a failed + /// connection would send the user looking at the host. + /// [RelayCommand] private async Task TrustHostKeyAsync(CancellationToken cancellationToken) { @@ -863,11 +890,28 @@ internal sealed partial class VaultViewModel( return; } - await knownHosts.TrustAsync(presentation, cancellationToken).ConfigureAwait(true); + try + { + await knownHosts.TrustAsync(presentation, cancellationToken).ConfigureAwait(true); + } + catch (OperationCanceledException) + { + Status = "Cancelled."; + return; + } + catch (Exception exception) + { + Status = $"The host key could not be stored, so nothing was connected: {exception.Message}"; + return; + } PendingHostKey = null; await ConnectAsync(cancellationToken).ConfigureAwait(true); + + // After connecting, not before. A pin is worth pushing straight away — the same host on another + // machine should not ask again — but not at the cost of delaying the connection the user asked for. + await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } /// Dismisses the trust prompt without pinning anything. @@ -878,6 +922,65 @@ internal sealed partial class VaultViewModel( Status = "The host key was not trusted, so nothing was connected."; } + /// + /// Withdraws trust from every key pinned for the host being edited. + /// + /// + /// + /// The counterpart to trust that now outlives the process, and the reason it exists at all: a mismatch is + /// a hard refusal with no way past it, so a server that is legitimately rebuilt would be unreachable for + /// ever without this. It is deliberately here — in the host's editor, reached by choosing to edit + /// a host — and not on the refusal itself. A "forget this key" button next to the warning is the same + /// button as "continue anyway" with two clicks instead of one. + /// + /// + /// Applies to the host's saved address rather than whatever the editor's boxes currently hold. + /// The pin belongs to the endpoint that was actually dialled, and someone halfway through retyping a + /// hostname has not moved it yet. + /// + /// + [RelayCommand] + private async Task ForgetHostKeyAsync(CancellationToken cancellationToken) + { + if (editingEntityId is not { } entityId + || Hosts.FirstOrDefault(row => row.EntityId == entityId) is not { } row) + { + return; + } + + var address = row.Host.Hostname; + var port = row.Host.Port; + + await RunAsync( + $"Forgetting the pinned host key for {address}…", + async () => + { + var forgotten = await knownHosts + .ForgetAsync(address, port, cancellationToken) + .ConfigureAwait(true); + + // The refusal that sent the user here is about a pin that no longer exists. + HostKeyMismatch = null; + + PendingChanges = await session + .PendingChangeCountAsync(cancellationToken) + .ConfigureAwait(true); + + if (forgotten == 0) + { + Status = $"Nothing was pinned for {address}:{port}."; + return; + } + + Status = $"Forgot the pinned host key for {address}:{port}. The next connection will " + + "ask you to check its fingerprint again."; + }).ConfigureAwait(true); + + // Pushed straight away, as a save or a deletion is: a withdrawal that stayed on this machine would + // leave the other ones refusing to connect to a server that has been rebuilt. + await AutoSyncAsync(cancellationToken).ConfigureAwait(true); + } + /// /// Marks every shown conflict as seen. /// @@ -1219,6 +1322,14 @@ internal sealed partial class VaultViewModel( partial void OnSelectedHostChanged(HostRowViewModel? value) => OnPropertyChanged(nameof(SelectedHostUsesAKey)); + /// + /// The editing id is set before this flips in every path that opens the editor, and cleared after it + /// flips back in every path that closes one, so this notification always observes the pair in a + /// consistent state. + /// + partial void OnIsEditingChanged(bool value) => + OnPropertyChanged(nameof(CanForgetHostKey)); + partial void OnPendingHostKeyChanged(HostKeyPresentation? value) => OnPropertyChanged(nameof(HasPendingHostKey)); diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml b/src/DodoSSH.Client.App/Views/MainWindow.axaml index 9c033da..6063abf 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml @@ -163,6 +163,19 @@