From 365954c5412e151f77668b14c97af832548b1f54 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 2 Oct 2026 13:59:37 +0200 Subject: [PATCH] Desktop: probe for the TPM once, off the UI thread, instead of on every unlock --- .../Platform/ProbedDeviceKeyStore.cs | 62 +++++++++++++++++++ .../Platform/WindowsDeviceKeyStore.cs | 14 ++++- .../ProbedDeviceKeyStoreTests.cs | 56 +++++++++++++++++ .../WindowsDeviceKeyStoreTests.cs | 6 +- 4 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs create mode 100644 tests/DodoSSH.Client.App.Tests/ProbedDeviceKeyStoreTests.cs diff --git a/src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs b/src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs new file mode 100644 index 0000000..e8c290a --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs @@ -0,0 +1,62 @@ +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Platform; + +/// +/// The device key store this machine supports, chosen once and off the UI thread. +/// +/// +/// +/// 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 — 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. +/// +/// +/// 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. +/// +/// +internal sealed class ProbedDeviceKeyStore : IDeviceKeyStore +{ + private readonly Task chosen; + + /// Starts choosing at once, on the thread pool. + internal ProbedDeviceKeyStore(Func choose) + { + ArgumentNullException.ThrowIfNull(choose); + chosen = Task.Run(choose); + } + + /// The store the probe settled on, for a test that needs to know which. + internal Task Chosen => chosen; + + /// + public async ValueTask IsAvailableAsync(CancellationToken cancellationToken) => + await chosen.WaitAsync(cancellationToken).ConfigureAwait(false) is not UnavailableDeviceKeyStore; + + /// + public async ValueTask SaveAsync(ReadOnlyMemory devicePrivateKey, CancellationToken cancellationToken) + { + var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false); + await store.SaveAsync(devicePrivateKey, cancellationToken).ConfigureAwait(false); + } + + /// + public async ValueTask TryLoadAsync(CancellationToken cancellationToken) + { + var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false); + return await store.TryLoadAsync(cancellationToken).ConfigureAwait(false); + } + + /// + public async ValueTask ForgetAsync(CancellationToken cancellationToken) + { + var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false); + await store.ForgetAsync(cancellationToken).ConfigureAwait(false); + } +} diff --git a/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs b/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs index c88b659..d3f7fb6 100644 --- a/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs +++ b/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs @@ -41,7 +41,12 @@ namespace DodoSSH.Client.App.Platform; /// public static class DesktopDeviceKeyStores { - /// The best store this machine supports. + /// The best store this machine supports, chosen on the thread pool. + /// + /// Returns at once: the probing that decides between them is slow — a TPM key created and deleted on + /// Windows — and runs behind rather than here, which is on the UI + /// thread at startup. + /// /// Where a stored key lives. /// /// 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> askForPin) + { if (OperatingSystem.IsWindows() && WindowsDeviceKeyStore.IsSupported()) { return new WindowsDeviceKeyStore(paths); diff --git a/tests/DodoSSH.Client.App.Tests/ProbedDeviceKeyStoreTests.cs b/tests/DodoSSH.Client.App.Tests/ProbedDeviceKeyStoreTests.cs new file mode 100644 index 0000000..0ea8c0b --- /dev/null +++ b/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; + +/// +/// That the slow probe choosing a device key store runs once, and not on the thread that asked. +/// +/// +/// 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 . +/// +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(); + + 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()); + await inner.Received(1).TryLoadAsync(Arg.Any()); + } + + [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); + } +} diff --git a/tests/DodoSSH.Client.App.Tests/WindowsDeviceKeyStoreTests.cs b/tests/DodoSSH.Client.App.Tests/WindowsDeviceKeyStoreTests.cs index 18a35c9..3ddb835 100644 --- a/tests/DodoSSH.Client.App.Tests/WindowsDeviceKeyStoreTests.cs +++ b/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(); + (await store.ShouldBeOfType().Chosen).ShouldBeOfType(); (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) {