Refuse a pinned host that answers with a key of another type, rather than asking whether to trust it

Pins are stored per algorithm, so an endpoint pinned for ssh-ed25519 that was answered with only an
ECDSA or RSA key found no pin and raised SshHostKeyUnknownException: the first-use question, where a
pin exists to produce a hard refusal. Anybody in the middle needed nothing more than a key of a
different type.

The connection factory now reads every pin for the endpoint before the handshake, offers the pinned
algorithms first so a server that still holds its pinned key negotiates it, and in the gate accepts a
key under an unpinned algorithm only when its fingerprint matches one of the endpoint's pins (one RSA
key signs as rsa-sha2-256 and rsa-sha2-512). Anything else is a mismatch. IKnownHostStore gains
FindForEndpointAsync for this.

src/DodoSSH.Client.Session/VaultKnownHostStore.cs

@@ -145,6 +145,26 @@ public sealed class VaultKnownHostStore : IKnownHostStore
         }
     }
 
+    /// <inheritdoc />
+    /// <remarks>From the snapshot, like <see cref="FindAsync"/>, and for the same reason.</remarks>
+    public ValueTask<IReadOnlyList<HostKeyPresentation>> FindForEndpointAsync(
+        string host,
+        int port,
+        CancellationToken cancellationToken)
+    {
+        lock (gate)
+        {
+            IReadOnlyList<HostKeyPresentation> found = pins.Values
+                .Where(pin =>
+                    KnownHostIdentity.Comparer.Equals(pin.Secret.Host, host) && pin.Secret.Port == port)
+                .Select(pin => new HostKeyPresentation(
+                    pin.Secret.Host, pin.Secret.Port, pin.Secret.Algorithm, pin.Secret.Fingerprint))
+                .ToArray();
+
+            return ValueTask.FromResult(found);
+        }
+    }
+
     /// <inheritdoc />
     /// <exception cref="InvalidOperationException">
     /// The vault is not open, so there is nowhere to record trust.

src/DodoSSH.Client.Ssh/HostKeyTrust.cs

@@ -64,6 +64,22 @@ public interface IKnownHostStore
     /// </remarks>
     ValueTask<string?> FindAsync(string host, int port, string algorithm, CancellationToken cancellationToken);
 
+    /// <summary>Returns every key pinned for one endpoint, whatever the algorithm.</summary>
+    /// <remarks>
+    /// <para>
+    /// ◆ <b>What stops a pin being sidestepped by changing algorithm.</b> <see cref="FindAsync"/> answers
+    /// per algorithm, so on its own a host pinned for <c>ssh-ed25519</c> that is answered by somebody offering
+    /// only <c>ecdsa-sha2-nistp256</c> looks like a host never seen before — and "never seen before" is a
+    /// question the user is asked, where "changed" is a refusal. Knowing the endpoint has pins at all is what
+    /// lets the connection factory tell the two apart; see <c>SshNetConnectionFactory</c>.
+    /// </para>
+    /// <para>The same rule as <see cref="FindAsync"/>: no I/O.</para>
+    /// </remarks>
+    ValueTask<IReadOnlyList<HostKeyPresentation>> FindForEndpointAsync(
+        string host,
+        int port,
+        CancellationToken cancellationToken);
+
     /// <summary>Records a host key as trusted.</summary>
     ValueTask TrustAsync(HostKeyPresentation presentation, CancellationToken cancellationToken);
 
@@ -156,6 +172,22 @@ public sealed class InMemoryKnownHostStore : IKnownHostStore
         }
     }
 
+    /// <inheritdoc />
+    public ValueTask<IReadOnlyList<HostKeyPresentation>> FindForEndpointAsync(
+        string host,
+        int port,
+        CancellationToken cancellationToken)
+    {
+        lock (gate)
+        {
+            IReadOnlyList<HostKeyPresentation> found = pins.Values
+                .Where(pin => KnownHostIdentity.Comparer.Equals(pin.Host, host) && pin.Port == port)
+                .ToArray();
+
+            return ValueTask.FromResult(found);
+        }
+    }
+
     /// <inheritdoc />
     public ValueTask TrustAsync(HostKeyPresentation presentation, CancellationToken cancellationToken)
     {

src/DodoSSH.Client.Ssh/SshNetConnectionFactory.cs

@@ -187,7 +187,13 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
         IProgress<SshConnectionPhase>? progress,
         CancellationToken cancellationToken)
     {
-        var gate = new HostKeyGate(knownHosts, request, progress, cancellationToken);
+        var endpointPins = await knownHosts
+            .FindForEndpointAsync(request.Host, request.Port, cancellationToken)
+            .ConfigureAwait(false);
+
+        PreferPinnedAlgorithms(client.ConnectionInfo, endpointPins);
+
+        var gate = new HostKeyGate(knownHosts, request, endpointPins, progress, cancellationToken);
 
         client.HostKeyReceived += gate.OnHostKeyReceived;
 
@@ -217,17 +223,73 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
         return gate;
     }
 
+    /// <summary>
+    /// Offers the host key algorithms this endpoint has pins for ahead of every other.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// The server takes the first algorithm in the client's list that it also supports, so this is what
+    /// makes a server that still holds its pinned key negotiate that key — rather than, after it gained an
+    /// <c>ed25519</c> key alongside the <c>rsa</c> one that was pinned, a key nobody has approved. Without it
+    /// the gate below would have to choose between refusing a legitimate server and prompting for one that
+    /// may not be, which is exactly the choice it must not be left with.
+    /// </para>
+    /// <para>
+    /// Reordered rather than restricted, so a host whose pinned keys are all gone still completes the key
+    /// exchange and reaches the gate, which reports it as a changed key with both fingerprints. Restricting
+    /// would fail the exchange with "no matching host key algorithm", which is true and tells nobody what
+    /// happened. Cleared and refilled because that is the only way a dictionary promises an order.
+    /// </para>
+    /// </remarks>
+    private static void PreferPinnedAlgorithms(
+        ConnectionInfo info,
+        IReadOnlyList<HostKeyPresentation> pins)
+    {
+        if (pins.Count == 0)
+        {
+            return;
+        }
+
+        var pinned = pins
+            .Select(pin => pin.Algorithm)
+            .ToHashSet(StringComparer.Ordinal);
+
+        var ordered = info.HostKeyAlgorithms
+            .OrderBy(algorithm => pinned.Contains(algorithm.Key) ? 0 : 1)
+            .ToList();
+
+        info.HostKeyAlgorithms.Clear();
+
+        foreach (var algorithm in ordered)
+        {
+            info.HostKeyAlgorithms.Add(algorithm.Key, algorithm.Value);
+        }
+    }
+
     /// <summary>
     /// Decides host key trust during the handshake, and remembers enough to explain a refusal.
     /// </summary>
     /// <remarks>
+    /// <para>
     /// Separate from the connect method because the decision is the security-relevant part and reads
     /// better on its own: pinned and equal accepts, pinned and different refuses as a mismatch,
     /// unpinned refuses as unknown. There is no fourth branch, and there is no prompt.
+    /// </para>
+    /// <para>
+    /// ◆ <b>"Pinned" means pinned for this endpoint, not for this algorithm.</b> Pins are stored per
+    /// algorithm, and an endpoint whose pins are all for other algorithms used to fall through to "unknown"
+    /// — so somebody in the middle who simply offered a different key type got the first-use question
+    /// instead of the hard refusal a pin exists to produce. Now a key under an algorithm with no pin of its
+    /// own is accepted only if it is the same key as one that is pinned (an RSA key is one key under
+    /// <c>rsa-sha2-256</c> and <c>rsa-sha2-512</c>, and its fingerprint says so), and is otherwise a
+    /// mismatch against the endpoint's pin. <see cref="PreferPinnedAlgorithms"/> is what keeps that from
+    /// refusing a server that still has its pinned key.
+    /// </para>
     /// </remarks>
     private sealed class HostKeyGate(
         IKnownHostStore knownHosts,
         SshConnectionRequest request,
+        IReadOnlyList<HostKeyPresentation> endpointPins,
         IProgress<SshConnectionPhase>? progress,
         CancellationToken cancellationToken)
     {
@@ -264,13 +326,21 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
                 .GetAwaiter()
                 .GetResult();
 
-            if (pinned is null)
+            if (pinned is null && endpointPins.Count == 0)
             {
                 // Refused, not prompted. The caller decides, off the handshake thread.
                 e.CanTrust = false;
                 return;
             }
 
+            // Nothing pinned under this algorithm, but the endpoint has pins: the same key under another
+            // algorithm's name if any pin's fingerprint is this one, and otherwise a changed key, measured
+            // against the endpoint's first pin. See the class remarks.
+            pinned ??= endpointPins
+                .FirstOrDefault(pin => SshHostKeyFingerprint.Equal(pin.Fingerprint, presentation.Fingerprint))
+                ?.Fingerprint
+                ?? endpointPins[0].Fingerprint;
+
             var matches = SshHostKeyFingerprint.Equal(pinned, presentation.Fingerprint);
             mismatch = !matches;
             e.CanTrust = matches;

tests/DodoSSH.Client.Ssh.Tests/HostKeyPinningTests.cs

@@ -0,0 +1,69 @@
+namespace DodoSSH.Client.Ssh.Tests;
+
+/// <summary>
+/// A pin is a pin for the endpoint, whichever key type the server answers with.
+/// </summary>
+/// <remarks>
+/// Pins are stored per algorithm, and the gap these close was that an endpoint whose only pins were for
+/// other algorithms read as a host never seen before. Somebody in the middle needed nothing more than a key
+/// of a different type to turn the hard refusal a pin exists for into a first-use question the user is
+/// invited to click through.
+/// </remarks>
+[Collection(SshCollection.Name)]
+public sealed class HostKeyPinningTests(SshServerFixture fixture)
+{
+    private static CancellationToken Token => TestContext.Current.CancellationToken;
+
+    /// <summary>
+    /// The endpoint is pinned under a key type this server does not have, which is exactly what a server in
+    /// the middle looks like from here: the pinned key is not on offer, and a key of another type is.
+    /// </summary>
+    [Fact]
+    public async Task AKeyOfAnotherTypeOnAPinnedEndpoint_IsAMismatchRatherThanAnUnknownHost()
+    {
+        var knownHosts = new InMemoryKnownHostStore();
+
+        await knownHosts.TrustAsync(
+            new HostKeyPresentation(fixture.Host, fixture.Port, "ssh-dss", "SHA256:the-key-that-was-approved"),
+            Token);
+
+        var mismatch = await Should.ThrowAsync<SshHostKeyMismatchException>(async () =>
+            await new SshNetConnectionFactory(knownHosts).ConnectAsync(Request(), progress: null, Token));
+
+        mismatch.PinnedFingerprint.ShouldBe("SHA256:the-key-that-was-approved");
+    }
+
+    /// <summary>
+    /// The same public key under another algorithm's name — an RSA key signs as <c>rsa-sha2-256</c> or
+    /// <c>rsa-sha2-512</c> — is the key that was approved, and its fingerprint is what says so.
+    /// </summary>
+    [Fact]
+    public async Task TheApprovedKeyUnderAnotherAlgorithmName_IsStillTrusted()
+    {
+        var learned = new InMemoryKnownHostStore();
+
+        var unknown = await Should.ThrowAsync<SshHostKeyUnknownException>(async () =>
+            await new SshNetConnectionFactory(learned).ConnectAsync(Request(), progress: null, Token));
+
+        var knownHosts = new InMemoryKnownHostStore();
+
+        await knownHosts.TrustAsync(unknown.Presentation with { Algorithm = "x-renamed-algorithm" }, Token);
+
+        await using var connection = await new SshNetConnectionFactory(knownHosts)
+            .ConnectAsync(Request(), progress: null, Token);
+
+        connection.ShouldNotBeNull();
+    }
+
+    [Fact]
+    public async Task AnEndpointWithNoPinsAtAll_IsStillAnUnknownHost() =>
+        await Should.ThrowAsync<SshHostKeyUnknownException>(async () =>
+            await new SshNetConnectionFactory(new InMemoryKnownHostStore())
+                .ConnectAsync(Request(), progress: null, Token));
+
+    private SshConnectionRequest Request() => new(
+        fixture.Host,
+        fixture.Port,
+        SshServerFixture.Username,
+        new SshPasswordCredential(SshServerFixture.Password));
+}