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.
This commit is contained in:
jaap-jan committed 2026-10-02 13:16:48 +02:00
1 parent 2ff9dff560
commit 0e8fe4fc80
7 files changed
+213 -22

No files matched your search

@@ -0,0 +1,48 @@
using System.Security.Cryptography;
using System.Text;
using DodoSSH.Client.Session;
namespace DodoSSH.Client.App.Platform;
/// <summary>
/// What a platform key store calls the hardware key behind one profile's device unlock.
/// </summary>
/// <remarks>
/// <para>
/// ◆ <b>One key per profile, because the key outlives the file and is shared by name.</b> 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
/// <see cref="ClientPaths.ForChannel"/> 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.
/// </para>
/// <para>
/// <b>The default profile keeps the name it has always had</b>, 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.
/// </para>
/// </remarks>
internal static class DeviceKeyNames
{
/// <summary>The name of the key behind <paramref name="paths"/>' device unlock.</summary>
/// <param name="paths">The profile.</param>
/// <param name="legacy">The name every profile used before names were per profile.</param>
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));
}
@@ -78,11 +78,21 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore
private readonly ClientPaths paths; private readonly ClientPaths paths;
/// <summary>This profile's own enclave key; see <see cref="DeviceKeyNames"/>.</summary>
/// <remarks>
/// <see cref="KeyTag"/> 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.
/// </remarks>
private readonly string keyTag;
/// <summary>Creates the store.</summary> /// <summary>Creates the store.</summary>
public MacDeviceKeyStore(ClientPaths paths) public MacDeviceKeyStore(ClientPaths paths)
{ {
ArgumentNullException.ThrowIfNull(paths); ArgumentNullException.ThrowIfNull(paths);
this.paths = paths; this.paths = paths;
keyTag = DeviceKeyNames.For(paths, KeyTag);
} }
/// <summary> /// <summary>
@@ -163,7 +173,7 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore
ReadOnlyMemory<byte> devicePrivateKey, ReadOnlyMemory<byte> devicePrivateKey,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
var sealedKey = Seal(devicePrivateKey.Span) var sealedKey = Seal(devicePrivateKey.Span, keyTag)
?? throw new InvalidOperationException( ?? throw new InvalidOperationException(
"The Secure Enclave would not seal the device key. Check IsAvailableAsync before offering to register one."); "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) var sealedKey = await File.ReadAllBytesAsync(paths.DeviceKeyFile, cancellationToken)
.ConfigureAwait(false); .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);
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -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, // 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 // 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. // left in the enclave with no ciphertext to open is inert.
_ = DeleteKey(symbols, KeyTag); _ = DeleteKey(symbols, keyTag);
} }
return ValueTask.CompletedTask; 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 /// 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. /// them is the same — do not offer a device unlock.
/// </remarks> /// </remarks>
private static byte[]? Seal(ReadOnlySpan<byte> devicePrivateKey) /// <summary>Whether a key is filed under this tag, asked without using it and so without a prompt.</summary>
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<byte> devicePrivateKey, string tag)
{ {
try 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 // 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. // 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) if (privateKey == IntPtr.Zero)
{ {
privateKey = CreateEnclaveKey(scope, symbols, KeyTag); privateKey = CreateEnclaveKey(scope, symbols, tag);
} }
if (privateKey == IntPtr.Zero) 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. /// on a thread that is also expected to draw the window behind it.
/// </para> /// </para>
/// </remarks> /// </remarks>
private static byte[]? Unseal(byte[] sealedKey) private static byte[]? Unseal(byte[] sealedKey, string tag)
{ {
try try
{ {
@@ -294,7 +324,7 @@ public sealed partial class MacDeviceKeyStore : IDeviceKeyStore
return null; return null;
} }
var privateKey = FindKey(scope, symbols, KeyTag, ConsentPrompt); var privateKey = FindKey(scope, symbols, tag, ConsentPrompt);
if (privateKey == IntPtr.Zero) if (privateKey == IntPtr.Zero)
{ {
@@ -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 /// 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 /// falls back to the passphrase, which is survivable — but silently, and a user would only notice
/// their gesture had stopped working. /// their gesture had stopped working.
/// <para>
/// This is the default profile's key, and the name every profile shared before keys were per profile;
/// see <see cref="DeviceKeyNames"/> and <see cref="keyName"/>.
/// </para>
/// </remarks> /// </remarks>
private const string KeyName = "DodoSSH.DeviceKey.v1"; private const string KeyName = "DodoSSH.DeviceKey.v1";
@@ -133,11 +137,22 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore
private readonly ClientPaths paths; private readonly ClientPaths paths;
/// <summary>This profile's own key.</summary>
/// <remarks>
/// <see cref="KeyName"/> 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 <see cref="TryLoadAsync"/>
/// 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.
/// </remarks>
private readonly string keyName;
/// <summary>Creates the store.</summary> /// <summary>Creates the store.</summary>
public WindowsDeviceKeyStore(ClientPaths paths) public WindowsDeviceKeyStore(ClientPaths paths)
{ {
ArgumentNullException.ThrowIfNull(paths); ArgumentNullException.ThrowIfNull(paths);
this.paths = paths; this.paths = paths;
keyName = DeviceKeyNames.For(paths, KeyName);
} }
/// <summary> /// <summary>
@@ -182,7 +197,7 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore
ReadOnlyMemory<byte> devicePrivateKey, ReadOnlyMemory<byte> devicePrivateKey,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
using var key = OpenOrCreate(); using var key = OpenOrCreate(keyName);
using var rsa = new RSACng(key); using var rsa = new RSACng(key);
var sealedKey = rsa.Encrypt(devicePrivateKey.Span, RSAEncryptionPadding.OaepSHA256); 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) var sealedKey = await File.ReadAllBytesAsync(paths.DeviceKeyFile, cancellationToken)
.ConfigureAwait(false); .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);
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -215,9 +236,11 @@ public sealed class WindowsDeviceKeyStore : IDeviceKeyStore
File.Delete(paths.DeviceKeyFile); 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(); 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 /// user and all have the same remedy, so none of them are worth telling apart here — see
/// <c>UnlockStatus.DeviceKeyUnavailable</c>. /// <c>UnlockStatus.DeviceKeyUnavailable</c>.
/// </remarks> /// </remarks>
private static byte[]? Unseal(byte[] sealedKey) private static byte[]? Unseal(byte[] sealedKey, string name)
{ {
try try
{ {
if (!CngKey.Exists(KeyName, new CngProvider(PlatformProvider))) if (!CngKey.Exists(name, new CngProvider(PlatformProvider)))
{ {
return null; 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); using var rsa = new RSACng(key);
return rsa.Decrypt(sealedKey, RSAEncryptionPadding.OaepSHA256); 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 /// 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. /// silently is the one behaviour this whole file exists to prevent.
/// </remarks> /// </remarks>
private static CngKey OpenOrCreate() private static CngKey OpenOrCreate(string name)
{ {
var provider = new CngProvider(PlatformProvider); 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( return CngKey.Create(
CngAlgorithm.Rsa, CngAlgorithm.Rsa,
KeyName, name,
new CngKeyCreationParameters new CngKeyCreationParameters
{ {
Provider = provider, Provider = provider,
@@ -4,7 +4,7 @@
Width="420" SizeToContent="Height" Width="420" SizeToContent="Height"
CanResize="False" ShowInTaskbar="False" CanResize="False" ShowInTaskbar="False"
WindowStartupLocation="CenterOwner" WindowStartupLocation="CenterOwner"
Background="{StaticResource Surface}" Background="{StaticResource Panel}"
Title="DodoSSH"> Title="DodoSSH">
<!-- <!--
@@ -32,7 +32,7 @@
<TextBlock Classes="heading" FontSize="15" x:Name="Heading" TextWrapping="Wrap" /> <TextBlock Classes="heading" FontSize="15" x:Name="Heading" TextWrapping="Wrap" />
<TextBlock Foreground="{StaticResource MutedText}" FontSize="12" TextWrapping="Wrap" <TextBlock Foreground="{StaticResource TextDim}" FontSize="12" TextWrapping="Wrap"
x:Name="Explanation" /> x:Name="Explanation" />
<!-- <!--
@@ -39,7 +39,10 @@ public sealed partial class DevicePinWindow : Window
/// <summary>Creates the window. Avalonia's designer needs this; nothing else should call it.</summary> /// <summary>Creates the window. Avalonia's designer needs this; nothing else should call it.</summary>
public DevicePinWindow() public DevicePinWindow()
{ {
AvaloniaXamlLoader.Load(this); // InitializeComponent, not AvaloniaXamlLoader.Load: only the generated method assigns the x:Name
// fields below, and with Load the first of them was null — so this window threw on construction and
// no Linux PIN prompt was ever shown.
InitializeComponent();
Accept.Click += OnAccept; Accept.Click += OnAccept;
Cancel.Click += OnCancel; Cancel.Click += OnCancel;
@@ -0,0 +1,39 @@
using DodoSSH.Client.App.Views;
namespace DodoSSH.Client.App.Layout.Tests;
/// <summary>
/// The Linux device-key PIN window, which no other test constructs.
/// </summary>
/// <remarks>
/// It named two palette resources that have never existed, <c>Surface</c> and <c>MutedText</c>, and a
/// <c>StaticResource</c> that cannot be found throws when the window is built — so on the one platform that
/// shows it, registering or unlocking with the TPM failed before any PIN was asked for. Only constructing it
/// against the real palette can catch that, which is what the harness's application provides.
/// </remarks>
public sealed class DevicePinWindowTests
{
private static CancellationToken Token => TestContext.Current.CancellationToken;
[Fact]
public async Task TheWindowBuildsAgainstThePalette()
{
await LayoutHarness.OnTheUiThreadAsync(
() =>
{
var window = new DevicePinWindow();
try
{
LayoutHarness.Settle(window, window.Width, 240);
LayoutHarness.Unreachable(window).ShouldBeEmpty();
}
finally
{
window.Close();
}
},
Token);
}
}
@@ -0,0 +1,48 @@
using DodoSSH.Client.App.Platform;
using DodoSSH.Client.Session;
namespace DodoSSH.Client.App.Tests;
/// <summary>
/// <c>DeviceKeyNames</c>: which hardware key a profile's device unlock is sealed to.
/// </summary>
/// <remarks>
/// The stores themselves cannot be exercised here — every call past the probe raises a Windows Hello or
/// Touch ID dialog — so the rule that keeps two installations off one key is tested on its own. It is the
/// whole of what stops signing out of the nightly deleting the key the release build unlocks with.
/// </remarks>
public sealed class DeviceKeyNamesTests
{
private const string Legacy = "DodoSSH.DeviceKey.v1";
[Fact]
public void TheDefaultProfile_KeepsTheNameEveryExistingInstallIsSealedTo() =>
DeviceKeyNames.For(ClientPaths.Default, Legacy).ShouldBe(Legacy);
[Fact]
public void TheNightlyProfile_HasAKeyOfItsOwn()
{
var nightly = DeviceKeyNames.For(ClientPaths.ForChannel("nightly"), Legacy);
nightly.ShouldNotBe(Legacy);
nightly.ShouldStartWith(Legacy + ".");
}
[Fact]
public void TwoProfilesWhoseFoldersShareAName_HaveDifferentKeys()
{
var first = new ClientPaths(Path.Combine(Path.GetTempPath(), "one", "DodoSSH"));
var second = new ClientPaths(Path.Combine(Path.GetTempPath(), "two", "DodoSSH"));
DeviceKeyNames.For(first, Legacy).ShouldNotBe(DeviceKeyNames.For(second, Legacy));
}
[Fact]
public void OneProfile_AlwaysNamesTheSameKey()
{
var directory = Path.Combine(Path.GetTempPath(), "portable", "DodoSSH");
DeviceKeyNames.For(new ClientPaths(directory), Legacy)
.ShouldBe(DeviceKeyNames.For(new ClientPaths(directory + Path.DirectorySeparatorChar), Legacy));
}
}