Desktop: probe for the TPM once, off the UI thread, instead of on every unlock

src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs

@@ -0,0 +1,62 @@
+using DodoSSH.Client.Session;
+
+namespace DodoSSH.Client.App.Platform;
+
+/// <summary>
+/// The device key store this machine supports, chosen once and off the UI thread.
+/// </summary>
+/// <remarks>
+/// <para>
+/// Choosing a store means probing for it, and on Windows the probe is creating an RSA-2048 key inside the TPM
+/// and deleting it again — see <see cref="WindowsDeviceKeyStore.IsSupported"/> — which is a slow device doing
+/// real work. It used to run synchronously on the UI thread three times over: once to choose the store at
+/// startup, again when the unlock card asked whether to offer the gesture, and again after every unlock when
+/// the security page asked whether to offer registering. A machine does not gain or lose a TPM while this
+/// process runs, so the answer is worked out once, on the thread pool, and every question after it is a
+/// lookup.
+/// </para>
+/// <para>
+/// Available means the choice landed on a real store, which is what the probe that chose it established.
+/// Everything else waits for the choice and hands over to it — in practice it has long since finished,
+/// since nothing reaches for the device key until somebody is at the unlock card.
+/// </para>
+/// </remarks>
+internal sealed class ProbedDeviceKeyStore : IDeviceKeyStore
+{
+    private readonly Task<IDeviceKeyStore> chosen;
+
+    /// <summary>Starts choosing at once, on the thread pool.</summary>
+    internal ProbedDeviceKeyStore(Func<IDeviceKeyStore> choose)
+    {
+        ArgumentNullException.ThrowIfNull(choose);
+        chosen = Task.Run(choose);
+    }
+
+    /// <summary>The store the probe settled on, for a test that needs to know which.</summary>
+    internal Task<IDeviceKeyStore> Chosen => chosen;
+
+    /// <inheritdoc />
+    public async ValueTask<bool> IsAvailableAsync(CancellationToken cancellationToken) =>
+        await chosen.WaitAsync(cancellationToken).ConfigureAwait(false) is not UnavailableDeviceKeyStore;
+
+    /// <inheritdoc />
+    public async ValueTask SaveAsync(ReadOnlyMemory<byte> devicePrivateKey, CancellationToken cancellationToken)
+    {
+        var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false);
+        await store.SaveAsync(devicePrivateKey, cancellationToken).ConfigureAwait(false);
+    }
+
+    /// <inheritdoc />
+    public async ValueTask<byte[]?> TryLoadAsync(CancellationToken cancellationToken)
+    {
+        var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false);
+        return await store.TryLoadAsync(cancellationToken).ConfigureAwait(false);
+    }
+
+    /// <inheritdoc />
+    public async ValueTask ForgetAsync(CancellationToken cancellationToken)
+    {
+        var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false);
+        await store.ForgetAsync(cancellationToken).ConfigureAwait(false);
+    }
+}

src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs

@@ -41,7 +41,12 @@ namespace DodoSSH.Client.App.Platform;
 /// </remarks>
 public static class DesktopDeviceKeyStores
 {
-    /// <summary>The best store this machine supports.</summary>
+    /// <summary>The best store this machine supports, chosen on the thread pool.</summary>
+    /// <remarks>
+    /// Returns at once: the probing that decides between them is slow — a TPM key created and deleted on
+    /// Windows — and runs behind <see cref="ProbedDeviceKeyStore"/> rather than here, which is on the UI
+    /// thread at startup.
+    /// </remarks>
     /// <param name="paths">Where a stored key lives.</param>
     /// <param name="askForPin">
     /// Shows the Linux PIN prompt, and is never called on the other two platforms — there the platform
@@ -57,6 +62,13 @@ public static class DesktopDeviceKeyStores
         ArgumentNullException.ThrowIfNull(paths);
         ArgumentNullException.ThrowIfNull(askForPin);
 
+        return new ProbedDeviceKeyStore(() => Choose(paths, askForPin));
+    }
+
+    private static IDeviceKeyStore Choose(
+        ClientPaths paths,
+        Func<DevicePinPurpose, CancellationToken, ValueTask<string?>> askForPin)
+    {
         if (OperatingSystem.IsWindows() && WindowsDeviceKeyStore.IsSupported())
         {
             return new WindowsDeviceKeyStore(paths);

tests/DodoSSH.Client.App.Tests/ProbedDeviceKeyStoreTests.cs

@@ -0,0 +1,56 @@
+using DodoSSH.Client.App.Platform;
+using DodoSSH.Client.Session;
+using NSubstitute;
+
+namespace DodoSSH.Client.App.Tests;
+
+/// <summary>
+/// That the slow probe choosing a device key store runs once, and not on the thread that asked.
+/// </summary>
+/// <remarks>
+/// On Windows the probe creates and deletes a TPM key, and it used to run synchronously on the UI thread at
+/// startup and again after every unlock. See <see cref="ProbedDeviceKeyStore"/>.
+/// </remarks>
+public sealed class ProbedDeviceKeyStoreTests
+{
+    private static CancellationToken Token => TestContext.Current.CancellationToken;
+
+    [Fact]
+    public async Task TheChoiceIsMadeOnce_OffTheCallersThread()
+    {
+        var asking = Environment.CurrentManagedThreadId;
+        var probes = 0;
+        var probedOn = 0;
+
+        var inner = Substitute.For<IDeviceKeyStore>();
+
+        var store = new ProbedDeviceKeyStore(() =>
+        {
+            Interlocked.Increment(ref probes);
+            probedOn = Environment.CurrentManagedThreadId;
+            return inner;
+        });
+
+        (await store.IsAvailableAsync(Token)).ShouldBeTrue();
+        (await store.IsAvailableAsync(Token)).ShouldBeTrue();
+        await store.TryLoadAsync(Token);
+
+        probes.ShouldBe(1);
+        probedOn.ShouldNotBe(asking);
+
+        // Availability is the choice, not a second probe through the store it chose.
+        await inner.DidNotReceive().IsAvailableAsync(Arg.Any<CancellationToken>());
+        await inner.Received(1).TryLoadAsync(Arg.Any<CancellationToken>());
+    }
+
+    [Fact]
+    public async Task AMachineWithNoStore_IsUnavailable_AndForgettingIsHarmless()
+    {
+        var store = new ProbedDeviceKeyStore(static () => new UnavailableDeviceKeyStore());
+
+        (await store.IsAvailableAsync(Token)).ShouldBeFalse();
+        (await store.TryLoadAsync(Token)).ShouldBeNull();
+
+        await store.ForgetAsync(Token);
+    }
+}

tests/DodoSSH.Client.App.Tests/WindowsDeviceKeyStoreTests.cs

@@ -53,7 +53,7 @@ public sealed class WindowsDeviceKeyStoreTests
 
         var store = DesktopDeviceKeyStores.ForThisMachine(new ClientPaths(Path.GetTempPath()), NoPin);
 
-        store.ShouldBeOfType<WindowsDeviceKeyStore>();
+        (await store.ShouldBeOfType<ProbedDeviceKeyStore>().Chosen).ShouldBeOfType<WindowsDeviceKeyStore>();
         (await store.IsAvailableAsync(Token)).ShouldBeTrue();
     }
 
@@ -77,9 +77,7 @@ public sealed class WindowsDeviceKeyStoreTests
 
     private static void SkipUnlessSupported()
     {
-        var supported = OperatingSystem.IsWindows()
-            && DesktopDeviceKeyStores.ForThisMachine(new ClientPaths(Path.GetTempPath()), NoPin)
-                is WindowsDeviceKeyStore;
+        var supported = OperatingSystem.IsWindows() && WindowsDeviceKeyStore.IsSupported();
 
         if (!supported)
         {