diff --git a/docs/adr/0007-device-key-protection.md b/docs/adr/0007-device-key-protection.md new file mode 100644 index 0000000..1a89f6e --- /dev/null +++ b/docs/adr/0007-device-key-protection.md @@ -0,0 +1,111 @@ +# ADR 0007 — What protects the device key on Windows + +**Status:** accepted, 2026-07-30 +**Supersedes nothing. Constrains** the device-unlock work described in the client roadmap. + +## Context + +Unlock asks for the vault passphrase on every launch, because no device key is registered. The +mechanism for one already exists: enrollment can generate an X25519 key pair, seal the +`UserSecretBundle` to it (`kind=device` in [crypto.md §3](../crypto.md)) and register the public half +with the server. What was never decided is **where the private half lives on this machine**, and that +decision is the whole security content of the feature. + +The device key is not a convenience token. It opens the same 92-byte bundle the passphrase opens — the +Ed25519 identity key plus the X25519 key that unwraps every vault key the user holds. It is +passphrase-equivalent, and recovery from its compromise is expensive: a passphrase change re-wraps one +row, but rotating the bundle means re-sealing every `VaultKey` to a new member key. + +Three candidates were considered: DPAPI, Windows Hello, and a TPM-resident key. + +### The constraint that reshapes the choice + +DSH1 fixes the device wrap as `SealTo(device_x25519_pk)`. Neither of the two hardware options can hold +that key: + +- **Windows Hello** (`KeyCredentialManager`) produces an RSA key that only *signs*. No key agreement, + no decryption. +- **The TPM**, through CNG's Platform Crypto Provider, does RSA and the NIST curves. Not X25519. + +So none of the three can *be* the device key. All three are ways to protect a stored 32-byte X25519 +key that still has to be reassembled in process memory to open the wrap. Any claim that "the key never +leaves hardware" would be false under all of them. + +## Decision + +**A Windows Hello gesture gating a protected blob, with the passphrase kept as a permanent fallback.** + +The gesture is what carries the security value: it requires **user presence** per unlock. Hello cannot +decrypt, so it is used to gate release of the wrapping key, and the passphrase path remains available +unconditionally. + +### Why not DPAPI alone + +DPAPI would be a **regression against the status quo**, which is worth stating plainly because it is +the option that looks like the obvious default. + +Today the root key exists only in the user's head and enters memory only while unlocked. Malware +running as the user must keylog the passphrase or scrape memory during a session. With DPAPI alone it +reads a file and calls `CryptUnprotectData` — no user present, no keylogging, at any moment. This is +the same reason browser cookie theft is trivial. Convenience would have been bought precisely against +the attacker most likely to turn up. + +DPAPI and a raw TPM key both defend the *stolen disk* case, which BitLocker already largely covers. +Neither defends the *local malware* case. The gesture does. + +### Why not extend the spec (yet) + +The only option that delivers what the TPM is usually credited with is to add a `SealTo` algorithm over +a curve the TPM can do — `alg_id = 4` over P-256 — so the device private key never exists in process +memory at all. That is **the recorded target**, not this decision. + +It is cheaper than "change a frozen spec" sounds, because a device wrap row is read only by the device +that created it: not by another client, and not by the server. The envelope already carries `alg_id` +and §5 requires readers to fail closed on what they do not understand, so the interop surface is +almost nil. Two things to check when it is taken up: `EnrollmentValidation` pins +`DevicePublicKey` to `CryptoSpec.PublicKeySize` (32 bytes; a P-256 public key is 33 or 65), and the +envelope's minimum-length rule. + +## Consequences + +### The cache key had to move, and the spec changed + +`LocalCacheProtector` derived its key from the passphrase master key. A device unlock produces the +bundle and never computes a master key, so it could have opened the identity and still not read the +cache it had itself written. The derivation now hangs off the bundle — `dsh1/localcache/v1` → +`v2`, [crypto.md §3.2](../crypto.md) — so every door reaches the same cache. + +Two consequences fell out of that, both improvements, neither planned: + +- **A passphrase change no longer discards the local cache.** The bundle is unchanged by a re-wrap. +- **Recovery-code unlock is fixed before it ships.** It derives a different master key from a different + secret and salt, so under v1 it would have silently orphaned every cached row. + +Existing caches become unreadable on upgrade and are discarded and re-pulled, which is the behaviour +already specified for a stale cache. + +### A stated guarantee weakened + +[crypto.md §10](../crypto.md) said locking meant "nothing on disk can be read again without the +passphrase." Where a device wrap exists that is no longer true, and it would have been untrue under +*either* candidate design. The wording now points here. The honest statement is that whatever guards +the device key on a machine is as strong as the passphrase for reading that machine's cache. + +This is why the enrollment screen's sentence — that the passphrase "is the only thing standing between +a stolen copy of the database and every credential in your vault" — stays true under this decision and +would have become false under DPAPI alone. A gesture is still something the attacker must produce. + +### Operational + +- **Hello is not always available.** No biometric hardware falls back to a Hello PIN, which is + TPM-bound and rate-limited and still satisfies the presence requirement. Some machines have no Hello + at all. The passphrase path is therefore required, not a nicety. +- **Hello keys are invalidated when the PIN is reset**, so the blob must be treated as losable at any + time; losing it degrades to a passphrase prompt and never to a locked-out vault. +- **Registering a device is a separate act from enrolling one.** `EnrollmentService.AddDevice` runs only + during enrollment, so every already-enrolled account — which is all of them — needs an endpoint to add + a device wrap while unlocked. Producing the wrap requires the bundle, so the client proves possession + by construction. +- **Revocation must delete the server row**, and un-enrolling the machine in front of the user must not + be able to lock them out: [ADR 0001](0001-e2ee-trust-model.md) makes an enrolled device a recovery + path, so it is now load-bearing for more than convenience. diff --git a/docs/crypto.md b/docs/crypto.md index b70d454..4ab7f77 100644 --- a/docs/crypto.md +++ b/docs/crypto.md @@ -98,8 +98,7 @@ vault passphrase ▼ MK — master key, RAM only, never persisted, never transmitted │ HKDF-SHA512-Expand with domain-separated info labels - ├── KEK_pp info = "dsh1/kek/passphrase/v1" 32 B - └── LocalCacheKey info = "dsh1/localcache/v1" 32 B + └── KEK_pp info = "dsh1/kek/passphrase/v1" 32 B ▼ UserSecretBundle — fixed binary, 92 B (see 3.1) stored server-side as N independent wraps of the SAME bundle: @@ -107,6 +106,8 @@ UserSecretBundle — fixed binary, 92 B (see 3.1) kind=device → SealTo(device_x25519_pk) one row per enrolled device kind=recovery → symmetric AEAD under KEK_rc = Argon2id(recovery code) kind=escrow → SealTo(team_breakglass_pk) opt-in, M5 + │ HKDF-SHA512 extract-and-expand over encode(bundle) — see 3.2 + ├── LocalCacheKey info = "dsh1/localcache/v2" 32 B ▼ VaultKey — 32 B CSPRNG, per vault, per key generation wrapped per member: SealTo(member_x25519_pk, VaultKey, aad) @@ -160,6 +161,38 @@ so a new field means bumping `version` — which a fixed layout handles as well Readers **must** reject a bundle whose length, label or version does not match exactly. This is the root of everything a user can read; there is no safe way to guess at a malformed one. +### 3.2 LocalCacheKey + +> **Changed 2026-07-30**, from deriving under `MK` with info `"dsh1/localcache/v1"` to deriving +> under the bundle with `"dsh1/localcache/v2"`. Recorded here because it reverses a stated choice. + +``` +LocalCacheKey = HKDF-SHA512(ikm = encode(bundle), salt = none, + info = "dsh1/localcache/v2", L = 32) +``` + +**Extract-and-expand, not expand alone.** Everything derived from `MK` uses HKDF-Expand directly, +which is sound because an Argon2id output is uniformly random over its whole length. `encode(bundle)` +is not: it opens with a fixed 14-byte label and carries a version, a generation and a timestamp before +reaching any key material. The extract step is what turns that into a pseudorandom key. + +Derived from the bundle rather than from `MK` because a passphrase is only one of four ways to open a +vault, and the cache has to be readable through all of them. Under v1: + +- a **device** unlock opens a `SealTo` wrap and never computes `MK`, so it could open the identity and + still not read the cache it had itself written; +- a **recovery-code** unlock derives a *different* `MK` — different secret, different salt — and so + would silently derive a different cache key and orphan every cached row; +- an **escrow** unlock (M5) has the same problem as device. + +Keying on the bundle also means a **passphrase change no longer discards the cache**, which is a +consequence worth stating rather than discovering: the bundle is unchanged by a re-wrap, so the cache +key is too. The cache becomes unreadable exactly when the *identity* is rotated, which is the correct +moment to discard it. + +The label is versioned, so a client holding a v1 cache fails to open it and re-pulls rather than +decrypting to nonsense. That is the whole reason for bumping rather than reusing the label. + ### Why the bundle is wrapped many ways This is the load-bearing structural choice. Because every wrap protects the *same* bundle: @@ -500,7 +533,11 @@ server, its operators, its backups and the network. It does **not** address: visible, as are host addresses for relay-enabled hosts; - a weak passphrase — §2 parameters and passphrase entropy are the whole defence; - **a locked vault on a machine with open sessions** — locking zeroes the identity keys, the vault - keys and the cache key, so nothing on disk can be read again without the passphrase. It does not + keys and the cache key, so nothing on disk can be read again without re-opening the bundle: a + passphrase, or any other wrap the user has registered. Where a device wrap exists, whatever guards + it on that machine is therefore as strong as the passphrase for reading the cache — which is the + decision recorded in [ADR 0007](adr/0007-device-key-protection.md), not a property of this spec. It + does not touch an SSH channel that is already open: that channel was authorised at connect time by a credential the remote host verified itself, and no vault key participates in keeping it alive. Sessions therefore survive lock **by design** (the client says so on its unlock screen, and the diff --git a/src/DodoSSH.Client.Session/SessionOpener.cs b/src/DodoSSH.Client.Session/SessionOpener.cs index d6321c1..4e47409 100644 --- a/src/DodoSSH.Client.Session/SessionOpener.cs +++ b/src/DodoSSH.Client.Session/SessionOpener.cs @@ -131,9 +131,9 @@ public sealed class SessionOpener( } /// - /// The master key lives only inside this method. Both things derived from it — the cache subkey and - /// the identity bundle — outlive it, which is why they are produced together here rather than by two - /// calls that would each need the master key again. + /// The master key lives only inside this method — it opens the bundle and is then done with. The cache + /// protector derives from the bundle rather than from the master key, which is what lets a device or + /// recovery unlock reach the same cache; see . /// private static UserSecretBundle? OpenBundle( StoredUnlockMaterial profile, @@ -155,7 +155,7 @@ public sealed class SessionOpener( try { - protector = LocalCacheProtector.From(master); + protector = LocalCacheProtector.From(bundle); return bundle; } catch diff --git a/src/DodoSSH.Client.Storage/LocalCacheProtector.cs b/src/DodoSSH.Client.Storage/LocalCacheProtector.cs index 8fda24f..d300c71 100644 --- a/src/DodoSSH.Client.Storage/LocalCacheProtector.cs +++ b/src/DodoSSH.Client.Storage/LocalCacheProtector.cs @@ -15,8 +15,8 @@ namespace DodoSSH.Client.Storage; /// definition. Both go through here. /// /// -/// What this is and is not worth. The key derives from the master key, so it exists only while -/// the vault is unlocked and is never written anywhere. That makes a stolen laptop, a stray backup or +/// What this is and is not worth. The key derives from the identity bundle, so it exists only +/// while the vault is unlocked and is never written anywhere. That makes a stolen laptop, a stray backup or /// a synced-to-cloud application folder yield nothing — which is the threat this addresses. It does /// not defend against a process running as the same user: that process can read this /// process's memory, and no on-disk measure changes it. docs/crypto.md §10 says the same about a @@ -33,21 +33,22 @@ public sealed class LocalCacheProtector : IDisposable private readonly byte[] key = new byte[CryptoSpec.SymmetricKeySize]; private bool disposed; - private LocalCacheProtector(MasterKey master) => master.DeriveLocalCacheKey(key); + private LocalCacheProtector(UserSecretBundle bundle) => bundle.DeriveLocalCacheKey(key); /// - /// Derives the cache key from an unlocked master key. + /// Derives the cache key from an opened identity bundle. /// /// - /// The master key is not retained. Only the subkey is, and it is domain-separated by its HKDF - /// label from the key that wraps the secret bundle — the two live in very different threat models - /// and must not be the same bytes. + /// From the bundle rather than the passphrase master key, so that every way of opening the vault + /// reaches the same cache — see . The bundle is not + /// retained. Only the subkey is, and it is domain-separated by its HKDF label from the key that wraps + /// the bundle: the two live in very different threat models and must not be the same bytes. /// - public static LocalCacheProtector From(MasterKey master) + public static LocalCacheProtector From(UserSecretBundle bundle) { - ArgumentNullException.ThrowIfNull(master); + ArgumentNullException.ThrowIfNull(bundle); - return new LocalCacheProtector(master); + return new LocalCacheProtector(bundle); } /// Seals a cache record, binding it to the row that will hold it. diff --git a/src/DodoSSH.Crypto/CryptoSpec.cs b/src/DodoSSH.Crypto/CryptoSpec.cs index fc17bcd..f7de2a1 100644 --- a/src/DodoSSH.Crypto/CryptoSpec.cs +++ b/src/DodoSSH.Crypto/CryptoSpec.cs @@ -166,8 +166,18 @@ public static class CryptoSpec /// Derives the key-encryption key that wraps the secret bundle. public static ReadOnlySpan PassphraseKek => "dsh1/kek/passphrase/v1"u8; - /// Derives the key that encrypts the client's on-disk cache. - public static ReadOnlySpan LocalCache => "dsh1/localcache/v1"u8; + /// + /// Derives the key that encrypts the client's on-disk cache. + /// + /// + /// v2 derives from the secret bundle, where v1 derived from the passphrase master key. The + /// cache key has to be reachable through every door that opens the vault — passphrase, device, + /// recovery code, escrow — and only the bundle is common to all of them. Under v1 a device unlock + /// could open the identity and still not read the cache it had written, and a recovery-code unlock + /// would silently derive a different key and orphan the whole cache. Bumping the label rather than + /// reusing it is what makes an old cache fail to open instead of decrypting to nonsense. + /// + public static ReadOnlySpan LocalCache => "dsh1/localcache/v2"u8; /// Prefix of the SealTo key-derivation info, concatenated with the AAD. public static ReadOnlySpan SealTo => "dsh1/sealto/v1|"u8; diff --git a/src/DodoSSH.Crypto/MasterKey.cs b/src/DodoSSH.Crypto/MasterKey.cs index fb11a7d..deebd47 100644 --- a/src/DodoSSH.Crypto/MasterKey.cs +++ b/src/DodoSSH.Crypto/MasterKey.cs @@ -62,16 +62,9 @@ public sealed class MasterKey : IDisposable } } - /// - /// Derives the key that encrypts the client's on-disk cache. - /// - /// - /// Domain-separated from the bundle's key-encryption key by its HKDF info label, so a cache - /// record can never be opened with the wrap key or the reverse — the two live in very different - /// threat models and must not share a key. - /// - public void DeriveLocalCacheKey(Span destination) => - DeriveSubkey(CryptoSpec.DerivationLabels.LocalCache, destination); + // The local cache key used to be derived here. It now derives from the secret bundle — see + // UserSecretBundle.DeriveLocalCacheKey — because a passphrase is only one of four ways to open a + // vault, and the cache key has to be the same one whichever was used. /// Wraps a bundle under the passphrase-derived key-encryption key. /// diff --git a/src/DodoSSH.Crypto/UserSecretBundle.cs b/src/DodoSSH.Crypto/UserSecretBundle.cs index 1584ce3..1e0d1d6 100644 --- a/src/DodoSSH.Crypto/UserSecretBundle.cs +++ b/src/DodoSSH.Crypto/UserSecretBundle.cs @@ -115,6 +115,56 @@ public sealed class UserSecretBundle : IDisposable KeyLogChain.TruncateTimestamp(createdAt)); } + /// + /// Derives the key that encrypts this client's on-disk cache. See docs/crypto.md §3.2. + /// + /// + /// + /// Derived from the bundle rather than from the passphrase master key, because the bundle is the one + /// thing every unlock path ends up holding. A device unlock opens a wrap and never + /// sees a master key at all, so under the old derivation it could open the identity and still not read + /// the cache it had itself written; a recovery-code unlock would derive a different key from a different + /// master and silently orphan every cached row. Keying on the bundle makes the cache readable through + /// whichever door was used, and unreadable exactly when the identity itself has been rotated — which is + /// the correct time to discard it. + /// + /// + /// Extract-and-expand, not expand alone. can expand directly because an + /// Argon2id output is already uniformly random over its whole length. The bundle's encoding is not: it + /// opens with a fixed 14-byte label and carries a version, a generation and a timestamp before it + /// reaches any key material. HKDF's extract step is what turns that into a pseudorandom key, and + /// skipping it would be feeding structured input to a function that assumes it has none. + /// + /// + public void DeriveLocalCacheKey(Span destination) + { + Alive(); + + if (destination.Length != CryptoSpec.SymmetricKeySize) + { + throw new ArgumentException( + $"The cache key is {CryptoSpec.SymmetricKeySize} bytes, got {destination.Length}.", + nameof(destination)); + } + + Span encoded = stackalloc byte[EncodedLength]; + try + { + Encode(encoded); + + HKDF.DeriveKey( + HashAlgorithmName.SHA512, + encoded, + destination, + salt: default, + info: CryptoSpec.DerivationLabels.LocalCache); + } + finally + { + CryptographicOperations.ZeroMemory(encoded); + } + } + /// /// Wraps the bundle under a symmetric key-encryption key. /// @@ -261,7 +311,12 @@ public sealed class UserSecretBundle : IDisposable && BinaryPrimitives.ReadUInt16BigEndian(encoded[OffsetVersion..]) == CurrentVersion && BinaryPrimitives.ReadUInt32BigEndian(encoded[OffsetKeyGeneration..]) >= 1; - private static UserSecretBundle? TryDecode(ReadOnlySpan encoded) + /// + /// Internal rather than private so the golden-vector generator can build a bundle from fixed bytes. + /// The cache key derives from this encoding, and a vector for it is worth nothing unless the input is + /// pinned — draws fresh randomness, so it cannot produce a reproducible one. + /// + internal static UserSecretBundle? TryDecode(ReadOnlySpan encoded) { if (!IsWellFormed(encoded)) { diff --git a/tests/DodoSSH.Client.Storage.Tests/CacheHarness.cs b/tests/DodoSSH.Client.Storage.Tests/CacheHarness.cs index d570505..72f2c5c 100644 --- a/tests/DodoSSH.Client.Storage.Tests/CacheHarness.cs +++ b/tests/DodoSSH.Client.Storage.Tests/CacheHarness.cs @@ -15,15 +15,16 @@ namespace DodoSSH.Client.Storage.Tests; /// internal sealed class CacheHarness : IDisposable { - private static readonly Argon2Profile CheapProfile = - Argon2Profile.FromStoredParameters(memoryKibibytes: 8 * 1024, passes: 1, parallelism: 1); + /// + /// Fixed rather than "now", because a bundle's encoding includes its creation timestamp and the cache + /// key derives from that encoding. Nothing here depends on the value; it only has to be a constant. + /// + private static readonly DateTimeOffset IdentityCreatedAt = + new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); - private readonly MasterKey master; - - private CacheHarness(ClientCacheFactory factory, MasterKey master, LocalCacheProtector protector) + private CacheHarness(ClientCacheFactory factory, LocalCacheProtector protector) { Factory = factory; - this.master = master; Protector = protector; Items = new ItemStore(factory, protector); @@ -54,8 +55,12 @@ internal sealed class CacheHarness : IDisposable internal ConflictStore Conflicts { get; } - internal static async Task CreateAsync( - string passphrase = "correct horse battery staple") + /// + /// Each harness gets a freshly generated identity, so two of them are two different users and their + /// cache keys differ. There is no passphrase here at all any more: the cache key derives from the + /// bundle, so a passphrase is not part of the question this harness sets up. + /// + internal static async Task CreateAsync() { var factory = ClientCacheFactory.ForMemory($"cache-{Guid.CreateVersion7():N}"); @@ -63,10 +68,11 @@ internal sealed class CacheHarness : IDisposable { await factory.MigrateAsync(TestContext.Current.CancellationToken); - var salt = new byte[CryptoSpec.SaltSize]; - var derived = MasterKey.Derive(passphrase, salt, CheapProfile); + // Disposed immediately: the protector copies the derived key, so the identity itself is not + // needed once it has answered. + using var identity = UserSecretBundle.Create(IdentityCreatedAt); - return new CacheHarness(factory, derived, LocalCacheProtector.From(derived)); + return new CacheHarness(factory, LocalCacheProtector.From(identity)); } catch { @@ -79,7 +85,6 @@ internal sealed class CacheHarness : IDisposable public void Dispose() { Protector.Dispose(); - master.Dispose(); Factory.Dispose(); } diff --git a/tests/DodoSSH.Client.Storage.Tests/CacheStoreTests.cs b/tests/DodoSSH.Client.Storage.Tests/CacheStoreTests.cs index 151cf27..1ea92e3 100644 --- a/tests/DodoSSH.Client.Storage.Tests/CacheStoreTests.cs +++ b/tests/DodoSSH.Client.Storage.Tests/CacheStoreTests.cs @@ -97,11 +97,15 @@ public sealed class CacheStoreTests : IAsyncLifetime } [Fact] - public async Task ARecordSealedUnderAnotherPassphrase_DoesNotOpen() + public async Task ARecordSealedByAnotherIdentity_DoesNotOpen() { + // Another *identity*, not another passphrase, and the distinction is the point. The cache key now + // derives from the secret bundle, so changing a passphrase deliberately keeps the cache readable — + // UserSecretBundleTests.TheLocalCacheKey_SurvivesAPassphraseChange pins that. What must still be + // unreadable is another user's cache, and that is what a second harness is. var entityId = Guid.CreateVersion7(); - using var stranger = await CreateAsync(passphrase: "a completely different passphrase"); + using var stranger = await CreateAsync(); var sealedFields = stranger.Protector.Protect( CryptoSpec.AadResourceType.Host, entityId, [1, 2, 3]); diff --git a/tests/DodoSSH.Client.Sync.Tests/SyncHarness.cs b/tests/DodoSSH.Client.Sync.Tests/SyncHarness.cs index 9231932..30b5921 100644 --- a/tests/DodoSSH.Client.Sync.Tests/SyncHarness.cs +++ b/tests/DodoSSH.Client.Sync.Tests/SyncHarness.cs @@ -14,13 +14,11 @@ namespace DodoSSH.Client.Sync.Tests; internal sealed class SyncDevice : IDisposable { private readonly ClientCacheFactory factory; - private readonly MasterKey master; private readonly LocalCacheProtector protector; private SyncDevice( string name, ClientCacheFactory factory, - MasterKey master, LocalCacheProtector protector, VaultKeyring keyring, FakeVaultServer server, @@ -28,7 +26,6 @@ internal sealed class SyncDevice : IDisposable { Name = name; this.factory = factory; - this.master = master; this.protector = protector; Keyring = keyring; @@ -80,14 +77,14 @@ internal sealed class SyncDevice : IDisposable { await cache.MigrateAsync(TestContext.Current.CancellationToken); - var derived = MasterKey.Derive( - $"passphrase-{name}", new byte[CryptoSpec.SaltSize], SyncHarness.CheapProfile); - // Opened through the real grant, so the keyring, the wrap and the AAD are all exercised. var keyring = VaultKeyring.Open(bundle, [vault]); + // Both simulated machines derive the same cache key, because they are the same user holding the + // same identity — which is what keying the cache on the bundle means. They still have separate + // cache databases, so nothing is shared between them but the key that would open either. return new SyncDevice( - name, cache, derived, LocalCacheProtector.From(derived), keyring, server, options); + name, cache, LocalCacheProtector.From(bundle), keyring, server, options); } catch { @@ -199,7 +196,6 @@ internal sealed class SyncDevice : IDisposable { Keyring.Dispose(); protector.Dispose(); - master.Dispose(); factory.Dispose(); } } diff --git a/tests/DodoSSH.Crypto.Tests/CryptoSpecTests.cs b/tests/DodoSSH.Crypto.Tests/CryptoSpecTests.cs index f838246..ff8545a 100644 --- a/tests/DodoSSH.Crypto.Tests/CryptoSpecTests.cs +++ b/tests/DodoSSH.Crypto.Tests/CryptoSpecTests.cs @@ -104,8 +104,11 @@ public sealed class CryptoSpecTests // These are HKDF info strings; changing one silently derives a different key. CryptoSpec.DerivationLabels.PassphraseKek.ToArray() .ShouldBe("dsh1/kek/passphrase/v1"u8.ToArray()); + // v2 since 2026-07-30: the cache key derives from the bundle rather than the master key, so that a + // device or recovery unlock reaches the same cache. docs/crypto.md §3.2. Bumping the label is what + // makes a v1 cache fail to open rather than decrypt to nonsense. CryptoSpec.DerivationLabels.LocalCache.ToArray() - .ShouldBe("dsh1/localcache/v1"u8.ToArray()); + .ShouldBe("dsh1/localcache/v2"u8.ToArray()); CryptoSpec.DerivationLabels.SealTo.ToArray() .ShouldBe("dsh1/sealto/v1|"u8.ToArray()); CryptoSpec.DerivationLabels.Fingerprint.ToArray() diff --git a/tests/DodoSSH.Crypto.Tests/GoldenVectors.cs b/tests/DodoSSH.Crypto.Tests/GoldenVectors.cs index 5285849..014b24a 100644 --- a/tests/DodoSSH.Crypto.Tests/GoldenVectors.cs +++ b/tests/DodoSSH.Crypto.Tests/GoldenVectors.cs @@ -1,3 +1,4 @@ +using System.Buffers.Binary; using System.Globalization; using System.Security.Cryptography; using System.Text; @@ -32,6 +33,7 @@ internal static class GoldenVectors ["envelope"] = BuildEnvelopeVectors(), ["aead"] = BuildAeadVectors(), ["hkdf"] = BuildHkdfVectors(), + ["localCacheKey"] = BuildLocalCacheKeyVectors(), ["argon2id"] = BuildArgon2Vectors(), ["fingerprint"] = BuildFingerprintVectors(), ["keyStatement"] = BuildKeyStatementVectors(), @@ -186,6 +188,12 @@ internal static class GoldenVectors ]; } + /// + /// Only the labels that really are HKDF-Expand over a master key belong here. The local cache key used + /// to be one of them and is not any more — it extracts and expands over the bundle's encoding + /// instead — so it has its own section rather than an entry here that would describe a derivation this + /// implementation no longer performs. See . + /// private static JsonArray BuildHkdfVectors() { var prk = Enumerable.Range(0, 64).Select(i => (byte)i).ToArray(); @@ -193,7 +201,6 @@ internal static class GoldenVectors (string Name, byte[] Info)[] cases = [ ("passphrase-kek", CryptoSpec.DerivationLabels.PassphraseKek.ToArray()), - ("local-cache", CryptoSpec.DerivationLabels.LocalCache.ToArray()), ]; var array = new JsonArray(); @@ -214,6 +221,65 @@ internal static class GoldenVectors return array; } + /// + /// The local cache key, over a bundle whose every byte is pinned. docs/crypto.md §3.2. + /// + /// + /// Built from a fixed encoding rather than from UserSecretBundle.Create, which draws fresh + /// randomness and so could never produce a reproducible vector. This is the one that matters for a + /// second implementation: it pins the extract-and-expand construction, the info label, and the fact + /// that the input is the bundle's canonical encoding rather than any key inside it. + /// + private static JsonArray BuildLocalCacheKeyVectors() + { + var encoded = FixedBundleEncoding(); + + using var bundle = UserSecretBundle.TryDecode(encoded) + ?? throw new InvalidOperationException("The fixed bundle encoding is not well-formed."); + + var cacheKey = new byte[CryptoSpec.SymmetricKeySize]; + bundle.DeriveLocalCacheKey(cacheKey); + + return + [ + new JsonObject + { + ["name"] = "local-cache-key-from-bundle", + ["algorithm"] = "HKDF-SHA512 extract-and-expand, no salt", + ["bundle"] = Hex(encoded), + ["info"] = Encoding.UTF8.GetString(CryptoSpec.DerivationLabels.LocalCache), + ["outputLength"] = CryptoSpec.SymmetricKeySize, + ["output"] = Hex(cacheKey), + }, + ]; + } + + /// + /// The §3.1 layout with every field a constant: label, version 1, generation 1, a fixed timestamp, and + /// two key scalars of recognisable byte patterns. Any 32 bytes is a valid X25519 scalar and a valid + /// Ed25519 seed, so nothing here needs to be a real generated key. + /// + private static byte[] FixedBundleEncoding() + { + var encoded = new byte[UserSecretBundle.EncodedLength]; + var span = encoded.AsSpan(); + + UserSecretBundle.Label.CopyTo(span); + BinaryPrimitives.WriteUInt16BigEndian(span[14..], UserSecretBundle.CurrentVersion); + BinaryPrimitives.WriteUInt32BigEndian(span[16..], 1u); + BinaryPrimitives.WriteInt64BigEndian( + span[20..], + new DateTimeOffset(2026, 1, 1, 0, 0, 0, TimeSpan.Zero).ToUnixTimeMilliseconds()); + + for (var i = 0; i < CryptoSpec.SymmetricKeySize; i++) + { + span[28 + i] = (byte)(0x40 + i); + span[60 + i] = (byte)(0x60 + i); + } + + return encoded; + } + private static JsonArray BuildArgon2Vectors() { var salt = Enumerable.Range(0, CryptoSpec.SaltSize).Select(i => (byte)(0x20 + i)).ToArray(); diff --git a/tests/DodoSSH.Crypto.Tests/UserSecretBundleTests.cs b/tests/DodoSSH.Crypto.Tests/UserSecretBundleTests.cs index a548bed..f738e05 100644 --- a/tests/DodoSSH.Crypto.Tests/UserSecretBundleTests.cs +++ b/tests/DodoSSH.Crypto.Tests/UserSecretBundleTests.cs @@ -473,30 +473,82 @@ public sealed class MasterKeyTests var wrap = master.WrapBundle(bundle, descriptor); Span cacheKey = stackalloc byte[CryptoSpec.SymmetricKeySize]; - master.DeriveLocalCacheKey(cacheKey); + bundle.DeriveLocalCacheKey(cacheKey); UserSecretBundle.TryOpenUnder(cacheKey, wrap, descriptor).ShouldBeNull(); } [Fact] - public void TheLocalCacheKey_IsStableForTheSamePassphraseAndSalt() + public void TheLocalCacheKey_IsStableAcrossAWrapAndUnwrap() { + // The same identity has to produce the same cache key after a round trip through a wrap, or every + // unlock would derive a different key and find its own cache unreadable. + var descriptor = DshAad.UserSecretBundle(Alice); + + using var master = Derive(); + using var original = UserSecretBundle.Create(CreatedAt); + using var reopened = master.TryOpenBundle(master.WrapBundle(original, descriptor), descriptor)!; + Span first = stackalloc byte[CryptoSpec.SymmetricKeySize]; Span second = stackalloc byte[CryptoSpec.SymmetricKeySize]; - using (var master = Derive()) - { - master.DeriveLocalCacheKey(first); - } - - using (var master = Derive()) - { - master.DeriveLocalCacheKey(second); - } + original.DeriveLocalCacheKey(first); + reopened.DeriveLocalCacheKey(second); first.SequenceEqual(second).ShouldBeTrue(); } + [Fact] + public void TheLocalCacheKey_SurvivesAPassphraseChange() + { + // What keying on the bundle rather than the master key actually buys, and the reason the derivation + // label went to v2. Under v1 this was false: a new passphrase derived a new master key, so changing + // it silently orphaned every cached row and the next launch re-pulled the whole vault. + var descriptor = DshAad.UserSecretBundle(Alice); + + using var bundle = UserSecretBundle.Create(CreatedAt); + + Span before = stackalloc byte[CryptoSpec.SymmetricKeySize]; + bundle.DeriveLocalCacheKey(before); + + using var changed = MasterKey.Derive( + "an entirely different passphrase", Salt, Argon2Profile.RandomSecret); + + using var reopened = changed.TryOpenBundle(changed.WrapBundle(bundle, descriptor), descriptor)!; + + Span after = stackalloc byte[CryptoSpec.SymmetricKeySize]; + reopened.DeriveLocalCacheKey(after); + + before.SequenceEqual(after).ShouldBeTrue(); + } + + [Fact] + public void TheLocalCacheKey_DiffersForADifferentIdentity() + { + // The other half of the property: the cache follows the identity, so a rotated one cannot read the + // cache the previous one wrote. That is the correct moment to discard it. + using var first = UserSecretBundle.Create(CreatedAt); + using var second = UserSecretBundle.Create(CreatedAt); + + Span one = stackalloc byte[CryptoSpec.SymmetricKeySize]; + Span two = stackalloc byte[CryptoSpec.SymmetricKeySize]; + + first.DeriveLocalCacheKey(one); + second.DeriveLocalCacheKey(two); + + one.SequenceEqual(two).ShouldBeFalse(); + } + + [Fact] + public void DerivingACacheKeyFromADisposedBundle_Throws() + { + var bundle = UserSecretBundle.Create(CreatedAt); + bundle.Dispose(); + + Should.Throw(() => + bundle.DeriveLocalCacheKey(new byte[CryptoSpec.SymmetricKeySize])); + } + [Fact] public void ASaltShorterThanTheSpecifiedMinimum_IsRejected() { @@ -519,19 +571,18 @@ public sealed class MasterKeyTests var master = Derive(); master.Dispose(); + using var bundle = UserSecretBundle.Create(CreatedAt); + Should.Throw(() => - { - var buffer = new byte[CryptoSpec.SymmetricKeySize]; - master.DeriveLocalCacheKey(buffer); - }); + master.WrapBundle(bundle, DshAad.UserSecretBundle(Alice))); } [Fact] - public void ASubkeyBufferOfTheWrongSize_IsRejected() + public void ACacheKeyBufferOfTheWrongSize_IsRejected() { - using var master = Derive(); + using var bundle = UserSecretBundle.Create(CreatedAt); - Should.Throw(() => master.DeriveLocalCacheKey(new byte[16])); + Should.Throw(() => bundle.DeriveLocalCacheKey(new byte[16])); } [Fact] diff --git a/tests/fixtures/crypto/vectors.json b/tests/fixtures/crypto/vectors.json index 358c370..10add81 100644 --- a/tests/fixtures/crypto/vectors.json +++ b/tests/fixtures/crypto/vectors.json @@ -128,14 +128,16 @@ "info": "dsh1/kek/passphrase/v1", "outputLength": 32, "output": "652b3a4a3ce03b235095ad32f1eed2cfdae915b5b0a98cc9f96face30853f4c7" - }, + } + ], + "localCacheKey": [ { - "name": "local-cache", - "algorithm": "HKDF-SHA512-Expand", - "prk": "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f", - "info": "dsh1/localcache/v1", + "name": "local-cache-key-from-bundle", + "algorithm": "HKDF-SHA512 extract-and-expand, no salt", + "bundle": "647368312f62756e646c652f76310001000000010000019b76daa800404142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f", + "info": "dsh1/localcache/v2", "outputLength": 32, - "output": "5b69ed9266ff5f297f11667ca693b0049b805365ee34d54d6e60b843e414b1f5" + "output": "65bcac61e28f526f6aabfa91dd4ff3e519eaa0724a5729e890a1ef71d69062a0" } ], "argon2id": [