From 0e8fe4fc80b58743ee2621a61d12ede587eac0dd Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 2 Oct 2026 13:16:48 +0200 Subject: [PATCH] Give each profile its own device key, and make the Linux PIN window able to open at all - The Windows CNG key and the macOS keychain tag were one name for every profile, so signing out of the nightly deleted the key the release build's device unlock was sealed to. DeviceKeyNames keeps the existing name for the default profile and gives every other profile its own; sign-out deletes only its own key, and a profile sealed before this keeps reading the shared one. - DevicePinWindow named two palette resources that do not exist and loaded itself with AvaloniaXamlLoader.Load, which never assigns the named fields, so it threw on construction and no PIN prompt was ever shown. A layout test now builds it against the real palette. --- .../Platform/DeviceKeyNames.cs | 48 +++++++++++++++++++ .../Platform/MacDeviceKeyStore.cs | 46 ++++++++++++++---- .../Platform/WindowsDeviceKeyStore.cs | 45 ++++++++++++----- .../Views/DevicePinWindow.axaml | 4 +- .../Views/DevicePinWindow.axaml.cs | 5 +- .../DevicePinWindowTests.cs | 39 +++++++++++++++ .../DeviceKeyNamesTests.cs | 48 +++++++++++++++++++ 7 files changed, 213 insertions(+), 22 deletions(-) create mode 100644 src/DodoSSH.Client.App/Platform/DeviceKeyNames.cs create mode 100644 tests/DodoSSH.Client.App.Layout.Tests/DevicePinWindowTests.cs create mode 100644 tests/DodoSSH.Client.App.Tests/DeviceKeyNamesTests.cs diff --git a/src/DodoSSH.Client.App/Platform/DeviceKeyNames.cs b/src/DodoSSH.Client.App/Platform/DeviceKeyNames.cs new file mode 100644 index 0000000..dca1e2c --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/DeviceKeyNames.cs @@ -0,0 +1,48 @@ +using System.Security.Cryptography; +using System.Text; +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Platform; + +/// +/// What a platform key store calls the hardware key behind one profile's device unlock. +/// +/// +/// +/// ◆ One key per profile, because the key outlives the file and is shared by name. The CNG key and +/// the Secure Enclave keychain item are found by name in a store that belongs to the user, not to the +/// profile directory. While every profile used the same name, the release build and the nightly — which +/// keeps apart precisely so neither can damage the other — shared one +/// key: signing out of either deleted it, and the other's device unlock silently stopped working. +/// +/// +/// The default profile keeps the name it has always had, so an existing install's sealed key still +/// opens. Every other profile gets that name with a digest of its directory on the end — a digest rather +/// than the directory's own name, because a portable copy can live in any folder and two such folders can +/// end in the same name. +/// +/// +internal static class DeviceKeyNames +{ + /// The name of the key behind ' device unlock. + /// The profile. + /// The name every profile used before names were per profile. + internal static string For(ClientPaths paths, string legacy) + { + ArgumentNullException.ThrowIfNull(paths); + + var directory = Normalise(paths.DataDirectory); + + if (string.Equals(directory, Normalise(ClientPaths.Default.DataDirectory), StringComparison.OrdinalIgnoreCase)) + { + return legacy; + } + + var digest = SHA256.HashData(Encoding.UTF8.GetBytes(directory.ToUpperInvariant())); + + return $"{legacy}.{Convert.ToHexStringLower(digest.AsSpan(0, 8))}"; + } + + private static string Normalise(string directory) => + Path.TrimEndingDirectorySeparator(Path.GetFullPath(directory)); +} diff --git a/src/DodoSSH.Client.App/Platform/MacDeviceKeyStore.cs b/src/DodoSSH.Client.App/Platform/MacDeviceKeyStore.cs index 2403c3b..e0f44c4 100644 --- a/src/DodoSSH.Client.App/Platform/MacDeviceKeyStore.cs +++ b/src/DodoSSH.Client.App/Platform/MacDeviceKeyStore.cs @@ -78,11 +78,21 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore private readonly ClientPaths paths; + /// This profile's own enclave key; see . + /// + /// for the default profile and a tag of its own for every other, so that signing + /// out of one installation cannot delete the key another's Touch ID unlock is sealed to. A profile + /// registered before this is still sealed to the shared tag and is read from there until it registers + /// again; it is never deleted from a profile that does not own it. + /// + private readonly string keyTag; + /// Creates the store. public MacDeviceKeyStore(ClientPaths paths) { ArgumentNullException.ThrowIfNull(paths); this.paths = paths; + keyTag = DeviceKeyNames.For(paths, KeyTag); } /// @@ -163,7 +173,7 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore ReadOnlyMemory devicePrivateKey, CancellationToken cancellationToken) { - var sealedKey = Seal(devicePrivateKey.Span) + var sealedKey = Seal(devicePrivateKey.Span, keyTag) ?? throw new InvalidOperationException( "The Secure Enclave would not seal the device key. Check IsAvailableAsync before offering to register one."); @@ -184,7 +194,8 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore var sealedKey = await File.ReadAllBytesAsync(paths.DeviceKeyFile, cancellationToken) .ConfigureAwait(false); - return Unseal(sealedKey); + // The shared tag only while this profile has no key of its own, so one unlock is one prompt. + return Unseal(sealedKey, !string.Equals(keyTag, KeyTag, StringComparison.Ordinal) && !HasKey(keyTag) ? KeyTag : keyTag); } /// @@ -202,7 +213,7 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore // Discarded, and that is deliberate: there is nothing a caller could do about a failure here, // and the file deleted above is the half that decides whether unlock will try at all. A key // left in the enclave with no ciphertext to open is inert. - _ = DeleteKey(symbols, KeyTag); + _ = DeleteKey(symbols, keyTag); } return ValueTask.CompletedTask; @@ -212,7 +223,26 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore /// Silent: it uses only the public half. Null on every failure, and the caller's answer to all of /// them is the same — do not offer a device unlock. /// - private static byte[]? Seal(ReadOnlySpan devicePrivateKey) + /// Whether a key is filed under this tag, asked without using it and so without a prompt. + private static bool HasKey(string tag) + { + try + { + using var scope = new CoreFoundationScope(); + + var symbols = MacSymbols.Resolve(); + + return symbols.Complete && FindKey(scope, symbols, tag, prompt: null) != IntPtr.Zero; + } + catch (Exception exception) when (exception is DllNotFoundException + or EntryPointNotFoundException + or BadImageFormatException) + { + return false; + } + } + + private static byte[]? Seal(ReadOnlySpan devicePrivateKey, string tag) { try { @@ -227,11 +257,11 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore // Created on first use rather than at registration, so that a device key re-registered after // a ForgetAsync gets a key again without anything having to notice that it had gone. - var privateKey = FindKey(scope, symbols, KeyTag, prompt: null); + var privateKey = FindKey(scope, symbols, tag, prompt: null); if (privateKey == IntPtr.Zero) { - privateKey = CreateEnclaveKey(scope, symbols, KeyTag); + privateKey = CreateEnclaveKey(scope, symbols, tag); } if (privateKey == IntPtr.Zero) @@ -281,7 +311,7 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore /// on a thread that is also expected to draw the window behind it. /// /// - private static byte[]? Unseal(byte[] sealedKey) + private static byte[]? Unseal(byte[] sealedKey, string tag) { try { @@ -294,7 +324,7 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore return null; } - var privateKey = FindKey(scope, symbols, KeyTag, ConsentPrompt); + var privateKey = FindKey(scope, symbols, tag, ConsentPrompt); if (privateKey == IntPtr.Zero) { diff --git a/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs b/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs index 7639534..c88b659 100644 --- a/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs +++ b/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs @@ -123,6 +123,10 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore /// rather than failing to open blobs written by a previous build. A device that cannot be opened /// falls back to the passphrase, which is survivable — but silently, and a user would only notice /// their gesture had stopped working. + /// + /// This is the default profile's key, and the name every profile shared before keys were per profile; + /// see and . + /// /// private const string KeyName = "DodoSSH.DeviceKey.v1"; @@ -133,11 +137,22 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore private readonly ClientPaths paths; + /// This profile's own key. + /// + /// for the default profile and a name of its own for every other — so signing out + /// of the nightly no longer deletes the key the release build's device unlock is sealed to. A profile + /// registered before this still has its blob sealed to the shared key, and + /// keeps opening it there until the next registration re-seals it here. It is never deleted from a + /// profile that does not own it, which is the half of this that matters. + /// + private readonly string keyName; + /// Creates the store. public WindowsDeviceKeyStore(ClientPaths paths) { ArgumentNullException.ThrowIfNull(paths); this.paths = paths; + keyName = DeviceKeyNames.For(paths, KeyName); } /// @@ -182,7 +197,7 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore ReadOnlyMemory devicePrivateKey, CancellationToken cancellationToken) { - using var key = OpenOrCreate(); + using var key = OpenOrCreate(keyName); using var rsa = new RSACng(key); var sealedKey = rsa.Encrypt(devicePrivateKey.Span, RSAEncryptionPadding.OaepSHA256); @@ -204,7 +219,13 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore var sealedKey = await File.ReadAllBytesAsync(paths.DeviceKeyFile, cancellationToken) .ConfigureAwait(false); - return Unseal(sealedKey); + // The shared key only while this profile has none of its own: a profile registered before keys were + // per profile is still sealed to it. Asking both would show two consent dialogs for one unlock. + var sealedTo = !string.Equals(keyName, KeyName, StringComparison.Ordinal) && !CngKey.Exists(keyName, new CngProvider(PlatformProvider)) + ? KeyName + : keyName; + + return Unseal(sealedKey, sealedTo); } /// @@ -215,9 +236,11 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore File.Delete(paths.DeviceKeyFile); } - if (CngKey.Exists(KeyName, new CngProvider(PlatformProvider))) + // This profile's key and never the shared one, unless this is the profile that owns it. Another + // profile's device unlock may still be sealed to it. + if (CngKey.Exists(keyName, new CngProvider(PlatformProvider))) { - using var key = CngKey.Open(KeyName, new CngProvider(PlatformProvider)); + using var key = CngKey.Open(keyName, new CngProvider(PlatformProvider)); key.Delete(); } @@ -231,16 +254,16 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore /// user and all have the same remedy, so none of them are worth telling apart here — see /// UnlockStatus.DeviceKeyUnavailable. /// - private static byte[]? Unseal(byte[] sealedKey) + private static byte[]? Unseal(byte[] sealedKey, string name) { try { - if (!CngKey.Exists(KeyName, new CngProvider(PlatformProvider))) + if (!CngKey.Exists(name, new CngProvider(PlatformProvider))) { return null; } - using var key = CngKey.Open(KeyName, new CngProvider(PlatformProvider)); + using var key = CngKey.Open(name, new CngProvider(PlatformProvider)); using var rsa = new RSACng(key); return rsa.Decrypt(sealedKey, RSAEncryptionPadding.OaepSHA256); @@ -256,18 +279,18 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore /// key rather than ever reconfiguring one: a key created without the policy would decrypt silently, and /// silently is the one behaviour this whole file exists to prevent. /// - private static CngKey OpenOrCreate() + private static CngKey OpenOrCreate(string name) { var provider = new CngProvider(PlatformProvider); - if (CngKey.Exists(KeyName, provider)) + if (CngKey.Exists(name, provider)) { - return CngKey.Open(KeyName, provider); + return CngKey.Open(name, provider); } return CngKey.Create( CngAlgorithm.Rsa, - KeyName, + name, new CngKeyCreationParameters { Provider = provider, diff --git a/src/DodoSSH.Client.App/Views/DevicePinWindow.axaml b/src/DodoSSH.Client.App/Views/DevicePinWindow.axaml index 8c6ec8f..c0e713c 100644 --- a/src/DodoSSH.Client.App/Views/DevicePinWindow.axaml +++ b/src/DodoSSH.Client.App/Views/DevicePinWindow.axaml @@ -4,7 +4,7 @@ Width="420" SizeToContent="Height" CanResize="False" ShowInTaskbar="False" WindowStartupLocation="CenterOwner" - Background="{StaticResource Surface}" + Background="{StaticResource Panel}" Title="DodoSSH">