diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f17e9..82e18a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `{MachineName}:{UserName}:{OSVersion}`, so a feature update (e.g. 25H2 → 26H2) changed `Environment.OSVersion`, rotated the KEK, and left every credential undecryptable with `AuthenticationTagMismatchException` and no migration path. - The KEK now derives from stable machine/user identity only — `OSVersion` is - dropped — so later OS updates cannot break the store. Machine and user name - still bind the keystore to this machine/user, and the security boundary is - unchanged (filesystem permissions on the credentials directory). + The KEK no longer folds in any ambient identity (see the format change below), + so neither an OS update nor a machine rename can break the store. The security + boundary is unchanged (filesystem permissions on the credentials directory). - **The Keychain test-retry budget is raised so `macos-15` stops flaking.** The `RetryHelper` default was 20 attempts × 25 ms ≈ 500 ms, which was too tight on @@ -32,16 +31,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **Keystore format bumped to version 2** (`.keystore` header). A version-1 - keystore — and a legacy headerless one, both sealed under the old - OSVersion-based KEK — is read with the legacy KEK and then **transparently - re-sealed as version 2 on first load**, so an existing, still-readable store - migrates itself on upgrade and becomes immune to future OS updates. The - re-seal is best-effort: it overwrites in place (safe, since the data key is - unchanged) and a persistence failure never fails an otherwise-successful read. - A keystore already broken by an OS update performed *before* this upgrade - cannot be recovered by the library; the integrity-error message now names that - cause. +- **The local keystore KEK no longer folds in machine, user, or OS identity; + format bumped to version 3.** The KEK now derives (PBKDF2) from a random + per-keystore salt stored in the `.keystore` header plus a fixed domain tag. + Ambient identity (`MachineName`/`UserName`, and previously `OSVersion`) was + only ever a tripwire, not a security boundary — it is discoverable — and it + broke the store on a machine rename or OS update, so it is gone. Older + keystores (version 1 and legacy headerless = machine/user/OSVersion KEK; + version 2 = machine/user KEK) are read with their old KEK and **transparently + re-sealed as version 3 on first load**. The re-seal is best-effort: it + overwrites in place (safe, since the data key is unchanged) and a persistence + failure never fails an otherwise-successful read. + + **Behaviour change:** a default-mode (no `AdditionalEntropy`) credentials + directory is now **portable** — copied whole to another machine or user it + still decrypts. This drops the "won't open elsewhere" tripwire, which was + never a real control. For genuine binding, supply `AdditionalEntropy` (a + per-machine or per-deployment secret) — now the file backend's only real + cryptographic boundary — or use the DPAPI / platform-keychain backends, which + bind to OS-managed secrets and survive renames and OS updates. + + A keystore already broken by an OS update performed *before* the version-1→3 + fix was installed cannot be recovered by the library; the integrity-error + message names the likely causes (corruption/tampering, or a wrong + `AdditionalEntropy`). ## [2.0.0] — 2026-08-28 diff --git a/CLAUDE.md b/CLAUDE.md index c59fa68..310e415 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,8 +44,12 @@ whenever the providers take a major, so check it against their `MoveFileEx` raises a sharing violation when the destination is open or two replacements race. See the remarks on `AtomicFile`. - **`LocalFileCredentialEncryption`'s security boundary is filesystem permissions**, not - the KEK — it is derived from non-secret machine identifiers unless the caller supplies - `AdditionalEntropy`. Do not describe it as protecting against same-user code. + the KEK — in default mode the KEK is derived from a non-secret random salt stored in the + `.keystore` header (v3), so a default-mode credentials directory is *portable* across + machines/users. The only real cryptographic boundary is `AdditionalEntropy` (or the + DPAPI/keychain backends). Do not describe the file backend as protecting against + same-user code, and do not reintroduce machine/user/OS identity into the KEK — that was + removed because it broke the store on OS updates and renames while never being a boundary. ## Repository baseline diff --git a/README.md b/README.md index ac2d1ca..5cffbe8 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,12 @@ Every command accepts `-v` / `--verbose` for full stack-trace output when someth ### Moving credentials between machines -Stored credentials are encrypted with a **machine-bound** key (the local KEK is derived from machine/user identity; the Keychain/libsecret/DPAPI backends key off the OS secret store). That means the files on disk can't simply be copied to another machine — they won't decrypt there. `accounts export` / `accounts import` solve this by re-encrypting the whole set under a passphrase you carry: +How portable your on-disk store is depends on the backend: + +- **Default file backend** (no `AdditionalEntropy`): the credentials directory is self-contained and **portable** — copy the whole directory, `.keystore` included, to another machine or user and it still decrypts. Convenient, but it means the files are only as protected as their filesystem permissions (see [Security model](#security-model)). +- **Hardened file backend** (`AdditionalEntropy` set) or the **Keychain / libsecret / DPAPI** backends: the store is bound to a caller secret or the OS secret store, so a raw copy won't decrypt elsewhere. + +For a portable transfer that works regardless of backend — and is safer than copying raw files, since it's protected by a passphrase rather than just filesystem permissions — use `accounts export` / `accounts import`, which re-encrypt the whole set under a passphrase you carry: ```console # On the old machine — you'll be prompted for a passphrase (and to confirm it): @@ -146,7 +151,7 @@ $ my-cli accounts import credentials.bundle - **Passphrase-only.** The archive is AES-256-GCM encrypted with a PBKDF2-derived key (600,000 iterations, random per-export salt). There is no plaintext export. For scripting, read the passphrase from an environment variable with `--passphrase-env MY_VAR` instead of being prompted. On Unix the archive file is written `0600`. - **Conflicts.** An imported credential is matched to an existing one on *(provider, account name, environment)*. On a match you're prompted to skip or overwrite; pass `--on-conflict skip` or `--on-conflict overwrite` to decide up front (a non-interactive run with no flag skips). - **Fidelity.** Account IDs and which credential is selected are preserved. Original creation timestamps are preserved on the file and libsecret backends; the macOS Keychain assigns its own, so imported items show a fresh timestamp there. -- **A credential that cannot be read is left out, not exported blank.** If the store holds an entry this machine's keystore can no longer decrypt — a credentials directory copied from another machine, say — `accounts export` skips it and tells you how many it skipped. The archive is then genuinely incomplete, which is why it says so: writing an empty payload instead would restore *over* a real secret at the far end. `accounts list` marks the same entries `(unreadable)` so you can see which they are. +- **A credential that cannot be read is left out, not exported blank.** If the store holds an entry the keystore can no longer decrypt — after the `AdditionalEntropy` value changed, say, or a file corrupted on disk — `accounts export` skips it and tells you how many it skipped. The archive is then genuinely incomplete, which is why it says so: writing an empty payload instead would restore *over* a real secret at the far end. `accounts list` marks the same entries `(unreadable)` so you can see which they are. > The archive contains **every stored secret**, protected only by your passphrase. Choose a strong one and treat the file as sensitive. @@ -154,22 +159,21 @@ $ my-cli accounts import credentials.bundle ## Security model -Credentials are encrypted with **AES-GCM** (authenticated — tampering is detected on decrypt). The data-encryption key is itself encrypted and stored in a `.keystore` file inside your credentials directory. The key-encryption key (KEK) is derived from machine + user identifiers via PBKDF2-HMAC-SHA256 (600,000 iterations). +Credentials are encrypted with **AES-GCM** (authenticated — tampering is detected on decrypt). The data-encryption key is itself encrypted and stored in a `.keystore` file inside your credentials directory. The key-encryption key (KEK) is derived via PBKDF2-HMAC-SHA256 (600,000 iterations) from a random salt stored in the `.keystore` header — with **no machine, user, or OS input**. (Earlier versions folded machine/user/OS identity into the KEK; that broke the store on a machine rename or OS update and was never a real security boundary, so it was removed.) **What this protects against:** - Other users on the same machine reading your credentials (filesystem permissions on the credentials directory enforce this). -- A casual attacker who ends up with a copy of the `.keystore` file but lacks knowledge of the originating machine and user. -- Undetected tampering of credential files (AES-GCM's authentication tag refuses decryption on any modification). +- Undetected tampering of the keystore or credential files (AES-GCM's authentication tag refuses decryption on any modification — a tampered header salt just yields a wrong key, which the tag then rejects). **What it does *not* protect against** (in default mode): -- A local attacker who has read access to the credentials directory **and** knows the machine hostname + username — the KEK is deterministic given those inputs. Close this gap either by supplying `AdditionalEntropy` (see below) or by using DPAPI / a platform keychain. +- **Anyone who obtains a copy of the credentials directory.** The KEK is derived from the (non-secret, stored) header salt and a fixed constant, so the `.keystore` is self-describing: whoever has the files can derive the KEK and decrypt. In default mode the store is portable *by design*, and the boundary is purely the filesystem permissions. Close this gap by supplying `AdditionalEntropy` (see below) or by using DPAPI / a platform keychain. - A compromised running process: once your CLI has decrypted a credential in memory, it's in memory. **Hardening with `AdditionalEntropy`:** -The default KEK is derived purely from machine state, so anyone who copies the `.keystore` file plus the machine's hostname/username can decrypt. Pass a secret into `CredentialStoreOptions.AdditionalEntropy` to close that gap: +The default KEK has no secret input, so anyone who copies the `.keystore` file can derive it and decrypt. Pass a secret into `CredentialStoreOptions.AdditionalEntropy` to make the KEK depend on something that is *not* in the file: ```csharp services.AddCredentialStore(opts => @@ -181,7 +185,7 @@ services.AddCredentialStore(opts => }); ``` -The entropy is mixed into the PBKDF2 password so the KEK now depends on both the machine AND this value. An attacker with the keystore file but without the entropy can't decrypt. Common sources: a per-deployment secret from env / HSM, a value from a secret manager, a user-entered passphrase. +The entropy is mixed into the PBKDF2 password so the KEK depends on this value, which is never stored in the keystore. An attacker with the keystore file but without the entropy can't decrypt. Common sources: a per-deployment secret from env / HSM, a value from a secret manager, a user-entered passphrase. If you specifically want machine binding, make the entropy a per-machine secret — that puts the binding on a value you control rather than on a discoverable identifier. Caveats: diff --git a/SECURITY.md b/SECURITY.md index cd07430..1dd99a1 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -22,11 +22,14 @@ pre-1.0 libraries and there are no long-term support branches. These libraries store credentials on the local filesystem or in an OS secret store. Two things are explicitly **not** claimed: -- `LocalFileCredentialEncryption` derives its key-encryption key from non-secret - machine and user identifiers. Its real security boundary is filesystem permissions. - It protects credentials at rest against another *user*; it does not protect them - against code running as the same user. Supply `AdditionalEntropy` for a KEK that - depends on a caller-held secret as well as the machine. +- `LocalFileCredentialEncryption` derives its key-encryption key from a non-secret + random salt stored in the keystore header — no machine, user, or OS input — so in + default mode the credentials directory is portable and its real security boundary is + filesystem permissions. It protects credentials at rest against another *user*; it + does not protect them against anyone who obtains a copy of the files, nor against code + running as the same user. Supply `AdditionalEntropy` (or use the DPAPI / platform + keychain backends) for a KEK that depends on a caller-held secret not present in the + keystore. - Nothing here defends against a compromised host, a debugger attached to the process, or a heap dump taken while credentials are decrypted in memory. diff --git a/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs b/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs index d86bec7..ee77da3 100644 --- a/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs +++ b/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs @@ -6,45 +6,50 @@ namespace NextIteration.SpectreConsole.Auth.Encryption { /// - /// File-based credential encryption using AES-GCM with a machine-derived - /// key-encryption key. Works on Windows, macOS, and Linux. + /// File-based credential encryption using AES-GCM with a key-encryption key + /// derived from a random per-keystore salt. Works on Windows, macOS, and Linux. /// /// /// Authenticated encryption (AES-GCM) detects tampering on decrypt. /// /// Default security model (no caller-supplied entropy): the data /// encryption key lives encrypted in a .keystore file inside the - /// credentials directory. That file is encrypted with a KEK derived via - /// PBKDF2 from stable machine/user identity - /// ({MachineName}:{UserName}). The OS version is deliberately - /// not an input: it once was, but a Windows feature update then - /// rotated the KEK and left the keystore unreadable, so it was removed - /// (see the format/migration note below). Because all KEK inputs are - /// discoverable on the machine, the real security boundary is the - /// filesystem permissions on the credentials directory, not the - /// cryptography. An attacker with read access to the keystore file on the - /// same machine/user can derive the KEK and decrypt credentials. + /// credentials directory. That file is sealed with a KEK derived via PBKDF2 + /// from a random per-keystore salt (stored in the keystore header) and a + /// fixed domain tag — no machine, user, or OS input. Every KEK input is + /// therefore either random-but-stored or a constant, so the KEK is not a + /// secret: the real security boundary is the filesystem permissions on the + /// credentials directory. A consequence, by design, is that a default-mode + /// credentials directory is portable — copied whole to another + /// machine or user it still decrypts. Earlier versions folded machine/user + /// (and once OS) identity into the KEK; that was only a tripwire, never a + /// boundary — the identity is discoverable — and it broke the store on a + /// machine rename or OS update, so it was removed. /// /// /// Hardened mode: supply additionalEntropy via the /// constructor (or ). /// The entropy is mixed into the PBKDF2 password so the KEK depends on - /// something an attacker cannot recover from the machine alone — for - /// example a per-deployment secret from an environment variable, - /// hardware token, or HSM. In this mode the keystore file alone is - /// insufficient to decrypt; the entropy value must also be known. + /// something an attacker cannot recover from the keystore alone — for + /// example a per-deployment secret from an environment variable, hardware + /// token, or HSM. This is the file backend's only real cryptographic + /// boundary and the supported way to bind a store: the keystore file AND the + /// entropy value are both required to decrypt. If machine binding is what + /// you want, make the entropy a per-machine secret. /// /// /// For the strongest protection, use DPAPI (on Windows) or a platform - /// keychain (macOS Keychain, Linux libsecret) instead. + /// keychain (macOS Keychain, Linux libsecret) instead — those bind to + /// OS-managed secrets and survive a machine rename or OS update. /// /// - /// The .keystore file written by this version carries a format - /// header (magic + one-byte version). A version-1 keystore — sealed under - /// the old OSVersion-based KEK — and a legacy headerless keystore are both - /// still read, with the old KEK, and then transparently re-sealed as the - /// current version so a later OS update cannot break them. A keystore - /// written by this version is not readable by pre-header library versions. + /// The .keystore carries a format header (magic + one-byte version). + /// Version 3 (current) stores a random salt and puts no identity in the KEK. + /// Older keystores — version 2 (machine/user KEK), and version 1 plus legacy + /// headerless (machine/user/OSVersion KEK) — are still read with their old + /// KEK and then transparently re-sealed as version 3 on first load, so an + /// existing, still-readable store migrates itself. A keystore written by + /// this version is not readable by older library versions. /// /// /// Implements : disposing zeroes the in-memory @@ -65,39 +70,55 @@ public class LocalFileCredentialEncryption : ICredentialEncryption, IDisposable // library versions have no header (they begin with a random 12-byte // nonce); those are still read, since the 8-byte magic can't plausibly // collide with a random nonce prefix (~2^-64). A keystore written by - // this version is not readable by pre-header library versions. + // this version is not readable by older library versions. // - // Version 1 sealed the data key under a KEK that included - // Environment.OSVersion; a Windows feature update changed OSVersion and - // left the keystore undecryptable with no migration path. Version 2 - // drops OSVersion from the KEK (see DeriveKeyEncryptionKey). A version-1 - // keystore — and a legacy headerless one, which used the same - // OSVersion-based KEK — is read with the legacy KEK and then - // transparently re-sealed as version 2 on first load, so an existing - // store survives the upgrade and is immune to later OS updates. A - // version this build does not know is rejected with a clear error - // rather than surfacing as an opaque integrity-check failure. + // Version history of the KEK the data key is sealed under: + // v1 (and legacy headerless): {MachineName}:{UserName}:{OSVersion}. + // A Windows feature update changed OSVersion and broke the store. + // v2: {MachineName}:{UserName}:{tag}. OSVersion dropped, but machine + // and user name are still ambient inputs that break on a rename and + // are not a real boundary (they are discoverable). + // v3 (current): a random per-keystore salt (stored in the header) + // plus a fixed tag — no machine/user/OS input. See DeriveCurrentKek. + // v3 layout is: magic | version(1) | salt(SaltSize) | AES-GCM payload. + // v1/v2 have no salt field (their salt is derived from machine/user). + // + // A v1 or v2 keystore is read with its old KEK and transparently + // re-sealed as v3 on first load. A version this build does not know is + // rejected with a clear error rather than an opaque integrity failure. private static readonly byte[] KeystoreMagic = "NISCA-KS"u8.ToArray(); private const byte LegacyOsVersionFormatVersion = 1; - private const byte KeystoreFormatVersion = 2; + private const byte MachineBoundFormatVersion = 2; + private const byte KeystoreFormatVersion = 3; + + // Random salt written into every v3 keystore header. 16 bytes is the + // usual PBKDF2 salt size; it is non-secret and its only job is to make + // each keystore's KEK independent and to give hardened-mode brute-force + // a per-keystore cost. + private const int SaltSize = 16; // PBKDF2-HMAC-SHA256 iteration count. OWASP 2023 guidance is // 600,000. In default mode (no caller entropy) iterations provide - // little benefit because the KEK inputs are all machine-derived; an + // little benefit because the KEK inputs are all non-secret; an // attacker with keystore access computes the KEK directly. In // hardened mode (caller entropy supplied) the iterations earn their // keep — they force the cost-per-guess on any offline brute-force // attempt against the caller-supplied secret. private const int Pbkdf2Iterations = 600_000; - // Stable, non-secret domain tag folded into the version-2 KEK password. - // It marks the OSVersion-free derivation (so a future KDF change can - // pick a new tag) and keeps the PBKDF2 password from being byte-for-byte - // the salt, which is the same machine/user string. + // Stable, non-secret domain tags folded into the KEK password. Each + // marks a KEK scheme so a future KDF change can pick a new tag; v2 also + // kept its password distinct from its machine/user salt. Both are kept + // so existing v1/v2 keystores can still be read and migrated. + private const string KekDomainV3 = "keystore/kek/v3"; private const string KekDomainV2 = "keystore/kek/v2"; private readonly string _keyFile; - private readonly byte[] _salt; + + // The machine/user salt used by the v1 and v2 KEKs. Retained only to + // read and migrate an existing v1/v2 keystore; v3 uses a random salt + // from the header instead. + private readonly byte[] _legacySalt; private readonly byte[]? _callerEntropy; // The data encryption key is derived once per instance lifetime and @@ -121,12 +142,12 @@ public class LocalFileCredentialEncryption : ICredentialEncryption, IDisposable /// /// /// Optional caller-supplied entropy mixed into the key-derivation - /// step. When non-null and non-empty, the KEK depends on this value - /// in addition to the machine-derived inputs — the file-based - /// backend then requires both the keystore file AND the entropy - /// value to decrypt. Changing the entropy invalidates any existing - /// keystore; callers who rotate the value must delete the keystore - /// and re-add credentials. + /// step. When non-null and non-empty, the KEK depends on this value — + /// the file-based backend then requires both the keystore file AND the + /// entropy value to decrypt, and it is the only real cryptographic + /// boundary the backend has. Changing the entropy invalidates any + /// existing keystore; callers who rotate the value must delete the + /// keystore and re-add credentials. /// /// /// is null, empty, or whitespace. @@ -137,10 +158,10 @@ public LocalFileCredentialEncryption(string credentialsDirectory, byte[]? additi _keyFile = Path.Join(credentialsDirectory, ".keystore"); - // PBKDF2 salt — non-secret, stable per machine/user. Caller - // entropy is mixed into the password side instead of the salt - // so it contributes to HMAC input during the key-stretch loop. - _salt = Encoding.UTF8.GetBytes($"{Environment.MachineName}:{Environment.UserName}"); + // Legacy (v1/v2) PBKDF2 salt — the machine/user string. Used only to + // read an existing v1/v2 keystore for migration; v3 keystores carry + // their own random salt in the header. + _legacySalt = Encoding.UTF8.GetBytes($"{Environment.MachineName}:{Environment.UserName}"); // Defensive copy — the caller may mutate or clear their buffer. _callerEntropy = additionalEntropy is { Length: > 0 } @@ -215,8 +236,8 @@ public async Task DecryptAsync(string encryptedText) // Message adapts to whether caller entropy is in play, so a // consumer who just changed their entropy knows where to look. var message = _callerEntropy is null - ? "Credential data failed integrity check. The file has been tampered with, or was encrypted with a different key (for example, the keystore was copied from another machine or user, or a Windows feature update changed the machine identity before this version — which no longer folds the OS version into the key — was installed)." - : "Credential data failed integrity check. The file has been tampered with, or was encrypted with a different additional-entropy value, or on a different machine."; + ? "Credential data failed integrity check. The keystore file is corrupt or has been tampered with (its header salt or ciphertext was modified)." + : "Credential data failed integrity check. The file has been tampered with, or was encrypted with a different additional-entropy value."; throw new InvalidOperationException(message, ex); } catch (InvalidOperationException) @@ -251,24 +272,28 @@ private async Task LoadOrCreateDataKeyAsync() } var stored = await File.ReadAllBytesAsync(_keyFile).ConfigureAwait(false); - var (encryptedKey, version) = ParseKeystore(stored); - - // Version 2 is sealed under the OSVersion-free KEK; version 1 (and a - // legacy headerless keystore) under the old OSVersion-based KEK. A - // wrong KEK surfaces as AuthenticationTagMismatchException here, - // which DecryptAsync turns into the actionable integrity error. - var kek = version == KeystoreFormatVersion - ? DeriveKeyEncryptionKey() - : DeriveLegacyKeyEncryptionKey(); + var (encryptedKey, version, headerSalt) = ParseKeystore(stored); + + // Pick the KEK for the version on disk. v3 uses the random header + // salt and no identity; v2 the machine/user KEK; v1 (and headerless) + // the machine/user/OSVersion KEK. A wrong KEK surfaces as + // AuthenticationTagMismatchException here, which DecryptAsync turns + // into the actionable integrity error. + var kek = version switch + { + KeystoreFormatVersion => DeriveCurrentKek(headerSalt!), + MachineBoundFormatVersion => DeriveV2Kek(), + _ => DeriveLegacyOsVersionKek(), + }; var dataKey = DecryptWithGcm(kek, encryptedKey); if (version != KeystoreFormatVersion) { - // Re-seal the recovered data key under the current KEK so a - // later OS update can't break the store. Best-effort: the - // decrypt has already succeeded, so a persistence failure must - // not fail the read. + // Re-seal the recovered data key as v3 (random salt, no identity) + // so a rename or OS update can't break the store. Best-effort: + // the decrypt has already succeeded, so a persistence failure + // must not fail the read. await TryMigrateToCurrentFormatAsync(dataKey).ConfigureAwait(false); } @@ -276,16 +301,18 @@ private async Task LoadOrCreateDataKeyAsync() } /// - /// Splits a keystore file into its AES-GCM payload and format version. - /// A keystore written by a header-carrying version begins with - /// followed by a one-byte version; a legacy - /// headerless keystore has no header and is reported as - /// (it used the same - /// OSVersion-based KEK). Throws when a header is present but its version - /// is not understood, so a keystore from a newer library fails clearly - /// rather than as an opaque integrity error. + /// Splits a keystore file into its AES-GCM payload, format version, and + /// (for v3) the random KEK salt from the header. A header-carrying + /// keystore begins with and a one-byte + /// version; a v3 keystore then carries a -byte + /// salt before the payload, while v1/v2 have none (their salt is derived + /// from machine/user, so the returned salt is ). A + /// legacy headerless keystore has no header and is reported as + /// . Throws when a header is + /// present but its version is not understood, so a keystore from a newer + /// library fails clearly rather than as an opaque integrity error. /// - private static (byte[] EncryptedKey, byte Version) ParseKeystore(byte[] stored) + private static (byte[] EncryptedKey, byte Version, byte[]? Salt) ParseKeystore(byte[] stored) { var headerLength = KeystoreMagic.Length + 1; if (stored.Length < headerLength || @@ -293,17 +320,30 @@ private static (byte[] EncryptedKey, byte Version) ParseKeystore(byte[] stored) { // No recognisable header — a pre-header keystore, sealed under // the legacy OSVersion-based KEK. - return (stored, LegacyOsVersionFormatVersion); + return (stored, LegacyOsVersionFormatVersion, null); } var version = stored[KeystoreMagic.Length]; - if (version is not (LegacyOsVersionFormatVersion or KeystoreFormatVersion)) + if (version is not (LegacyOsVersionFormatVersion or MachineBoundFormatVersion or KeystoreFormatVersion)) { throw new InvalidOperationException( $"Unsupported keystore format version {version}. This build supports versions {LegacyOsVersionFormatVersion}–{KeystoreFormatVersion}; the keystore was likely written by a newer version of the library."); } - return (stored[headerLength..], version); + if (version != KeystoreFormatVersion) + { + // v1/v2: no salt field, payload follows the version byte directly. + return (stored[headerLength..], version, null); + } + + // v3: a random salt precedes the payload. + if (stored.Length < headerLength + SaltSize) + { + throw new InvalidOperationException("Keystore is truncated: the version-3 salt is missing."); + } + + var salt = stored[headerLength..(headerLength + SaltSize)]; + return (stored[(headerLength + SaltSize)..], version, salt); } private async Task CreateKeyFileAsync() @@ -330,13 +370,12 @@ private async Task CreateKeyFileAsync() } /// - /// Re-seals an already-recovered data key under the current - /// (OSVersion-free) KEK, upgrading a version-1 or legacy headerless - /// keystore to the current format in place. Best-effort: any I/O or - /// permission failure is swallowed because the caller already holds a - /// valid in-memory data key and its read has succeeded — a persistence - /// failure must not turn a working decrypt into an error. The next run - /// retries the migration. + /// Re-seals an already-recovered data key as a v3 keystore (random salt, + /// no identity), upgrading a version-1, version-2, or legacy headerless + /// keystore in place. Best-effort: any I/O or permission failure is + /// swallowed because the caller already holds a valid in-memory data key + /// and its read has succeeded — a persistence failure must not turn a + /// working decrypt into an error. The next run retries the migration. /// private async Task TryMigrateToCurrentFormatAsync(byte[] dataKey) { @@ -361,19 +400,23 @@ await AtomicFile.WriteAllBytesAsync( } /// - /// Encrypts under the current KEK and frames - /// it with the current format header, ready to write as a keystore. + /// Encrypts under a fresh v3 KEK (a new random + /// salt) and frames it as a v3 keystore: + /// magic | version | salt(SaltSize) | AES-GCM payload, ready to + /// write. A new salt each seal is why re-migrating the same data key is + /// safe and idempotent. /// private byte[] SealDataKey(byte[] dataKey) { - var encryptedKey = EncryptWithGcm(DeriveKeyEncryptionKey(), dataKey); - - // Frame the payload with the format header so the version is - // self-describing on the next read. - var framed = new byte[KeystoreMagic.Length + 1 + encryptedKey.Length]; - Buffer.BlockCopy(KeystoreMagic, 0, framed, 0, KeystoreMagic.Length); - framed[KeystoreMagic.Length] = KeystoreFormatVersion; - Buffer.BlockCopy(encryptedKey, 0, framed, KeystoreMagic.Length + 1, encryptedKey.Length); + var salt = RandomNumberGenerator.GetBytes(SaltSize); + var encryptedKey = EncryptWithGcm(DeriveCurrentKek(salt), dataKey); + + var offset = KeystoreMagic.Length; + var framed = new byte[offset + 1 + SaltSize + encryptedKey.Length]; + Buffer.BlockCopy(KeystoreMagic, 0, framed, 0, offset); + framed[offset] = KeystoreFormatVersion; + Buffer.BlockCopy(salt, 0, framed, offset + 1, SaltSize); + Buffer.BlockCopy(encryptedKey, 0, framed, offset + 1 + SaltSize, encryptedKey.Length); return framed; } @@ -387,43 +430,56 @@ private void EnsureKeystoreDirectory() } /// - /// Derives the current key-encryption key. The OS version is - /// deliberately excluded: it changed on every Windows feature update - /// (e.g. 25H2 → 26H2), which rotated the KEK and left the keystore - /// undecryptable. Machine and user name still bind the keystore to this - /// machine/user. + /// Derives the current (v3) key-encryption key from the per-keystore + /// random and a fixed domain tag. No machine, + /// user, or OS input — so a rename or OS update cannot rotate it, and a + /// default-mode keystore is portable. Caller entropy, when present, is + /// the only secret in the password and the only real boundary. + /// + private byte[] DeriveCurrentKek(byte[] salt) + => DeriveKek(KekDomainV3, salt); + + /// + /// Derives the version-2 KEK (machine/user identity plus a tag, salted + /// by the machine/user string). Used only to read an existing v2 + /// keystore so it can be re-sealed as v3. /// - private byte[] DeriveKeyEncryptionKey() - => DeriveKek($"{Environment.MachineName}:{Environment.UserName}:{KekDomainV2}"); + private byte[] DeriveV2Kek() + => DeriveKek($"{Environment.MachineName}:{Environment.UserName}:{KekDomainV2}", _legacySalt); /// - /// Derives the pre-version-2 key-encryption key, which folded the - /// volatile into the identity. Used - /// only to read an existing version-1 (or legacy headerless) keystore so - /// it can be re-sealed under . + /// Derives the version-1 KEK, which folded the volatile + /// into the identity. Used only to + /// read an existing v1 (or legacy headerless) keystore so it can be + /// re-sealed as v3. /// - private byte[] DeriveLegacyKeyEncryptionKey() - => DeriveKek($"{Environment.MachineName}:{Environment.UserName}:{Environment.OSVersion}"); + private byte[] DeriveLegacyOsVersionKek() + => DeriveKek($"{Environment.MachineName}:{Environment.UserName}:{Environment.OSVersion}", _legacySalt); - private byte[] DeriveKek(string machineIdentity) + /// + /// PBKDF2-HMAC-SHA256 over and + /// . Caller entropy, when present, is prepended to + /// the context (null-separated) so it contributes to every HMAC block. + /// + private byte[] DeriveKek(string context, byte[] salt) { if (_callerEntropy is null) { - // Default mode — the machine identity is the whole password. - return Rfc2898DeriveBytes.Pbkdf2(machineIdentity, _salt, Pbkdf2Iterations, HashAlgorithmName.SHA256, KeySize); + // Default mode — the context is the whole password. + return Rfc2898DeriveBytes.Pbkdf2(context, salt, Pbkdf2Iterations, HashAlgorithmName.SHA256, KeySize); } // Hardened mode — caller entropy is concatenated with a null - // separator in front of the machine identity. Using the byte - // overload rather than string interpolation so caller-supplied - // bytes don't have to be valid UTF-8. - var machineBytes = Encoding.UTF8.GetBytes(machineIdentity); - var password = new byte[_callerEntropy.Length + 1 + machineBytes.Length]; + // separator in front of the context. Using the byte overload rather + // than string interpolation so caller-supplied bytes don't have to + // be valid UTF-8. + var contextBytes = Encoding.UTF8.GetBytes(context); + var password = new byte[_callerEntropy.Length + 1 + contextBytes.Length]; Buffer.BlockCopy(_callerEntropy, 0, password, 0, _callerEntropy.Length); password[_callerEntropy.Length] = 0x00; - Buffer.BlockCopy(machineBytes, 0, password, _callerEntropy.Length + 1, machineBytes.Length); + Buffer.BlockCopy(contextBytes, 0, password, _callerEntropy.Length + 1, contextBytes.Length); - return Rfc2898DeriveBytes.Pbkdf2(password, _salt, Pbkdf2Iterations, HashAlgorithmName.SHA256, KeySize); + return Rfc2898DeriveBytes.Pbkdf2(password, salt, Pbkdf2Iterations, HashAlgorithmName.SHA256, KeySize); } /// diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Encryption/LocalFileCredentialEncryptionTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Encryption/LocalFileCredentialEncryptionTests.cs index 142545f..5848c92 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Encryption/LocalFileCredentialEncryptionTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Encryption/LocalFileCredentialEncryptionTests.cs @@ -347,11 +347,12 @@ public async Task Entropy_DefensivelyCopied_MutationAfterConstructIsIgnored() private static readonly byte[] KeystoreMagic = "NISCA-KS"u8.ToArray(); // Crypto constants mirrored from LocalFileCredentialEncryption so the - // tests can forge a genuine version-1 keystore. + // tests can forge a genuine version-1 or version-2 keystore. private const int LegacyPbkdf2Iterations = 600_000; private const int LegacyKeySize = 32; private const int LegacyNonceSize = 12; private const int LegacyTagSize = 16; + private const string KekDomainV2 = "keystore/kek/v2"; /// /// Reads the derived data key out of an instance whose key has already @@ -366,46 +367,63 @@ private static async Task GetDataKeyAsync(LocalFileCredentialEncryption } /// - /// Writes a keystore in the pre-version-2 shape: the data key sealed - /// under the old OSVersion-based KEK, optionally with the version-1 - /// header (or headerless, as the earliest builds wrote). + /// Writes a version-1 keystore: the data key sealed under the old + /// OSVersion-based KEK, optionally with the version-1 header (or + /// headerless, as the earliest builds wrote). /// - private static async Task WriteLegacyKeystoreAsync(string directory, byte[] dataKey, bool withHeader, byte[]? entropy = null) + private static Task WriteLegacyKeystoreAsync(string directory, byte[] dataKey, bool withHeader, byte[]? entropy = null) + { + var kek = DeriveTestKek($"{Environment.MachineName}:{Environment.UserName}:{Environment.OSVersion}", entropy); + return WriteKeystoreFileAsync(directory, kek, dataKey, withHeader ? (byte)1 : null); + } + + /// + /// Writes a version-2 keystore: the data key sealed under the machine/user + /// KEK (with the v2 domain tag), framed with the version-2 header. + /// + private static Task WriteV2KeystoreAsync(string directory, byte[] dataKey, byte[]? entropy = null) + { + var kek = DeriveTestKek($"{Environment.MachineName}:{Environment.UserName}:{KekDomainV2}", entropy); + return WriteKeystoreFileAsync(directory, kek, dataKey, version: 2); + } + + private static async Task WriteKeystoreFileAsync(string directory, byte[] kek, byte[] dataKey, byte? version) { - var kek = DeriveLegacyKek(entropy); var wrapped = LegacyGcmEncrypt(kek, dataKey); byte[] onDisk; - if (withHeader) + if (version is byte v) { onDisk = new byte[KeystoreMagic.Length + 1 + wrapped.Length]; Buffer.BlockCopy(KeystoreMagic, 0, onDisk, 0, KeystoreMagic.Length); - onDisk[KeystoreMagic.Length] = 1; // legacy format version + onDisk[KeystoreMagic.Length] = v; Buffer.BlockCopy(wrapped, 0, onDisk, KeystoreMagic.Length + 1, wrapped.Length); } else { - onDisk = wrapped; + onDisk = wrapped; // headerless (earliest v1 shape) } await File.WriteAllBytesAsync(Path.Join(directory, ".keystore"), onDisk, TestContext.Current.CancellationToken); } - private static byte[] DeriveLegacyKek(byte[]? entropy) + // Legacy (v1/v2) KEK derivation: PBKDF2 over a context string with the + // machine/user salt, entropy prepended null-separated. Mirrors + // LocalFileCredentialEncryption.DeriveKek for the pre-v3 salt. + private static byte[] DeriveTestKek(string context, byte[]? entropy) { var salt = System.Text.Encoding.UTF8.GetBytes($"{Environment.MachineName}:{Environment.UserName}"); - var machineIdentity = $"{Environment.MachineName}:{Environment.UserName}:{Environment.OSVersion}"; if (entropy is null || entropy.Length == 0) { - return Rfc2898DeriveBytes.Pbkdf2(machineIdentity, salt, LegacyPbkdf2Iterations, HashAlgorithmName.SHA256, LegacyKeySize); + return Rfc2898DeriveBytes.Pbkdf2(context, salt, LegacyPbkdf2Iterations, HashAlgorithmName.SHA256, LegacyKeySize); } - var machineBytes = System.Text.Encoding.UTF8.GetBytes(machineIdentity); - var password = new byte[entropy.Length + 1 + machineBytes.Length]; + var contextBytes = System.Text.Encoding.UTF8.GetBytes(context); + var password = new byte[entropy.Length + 1 + contextBytes.Length]; Buffer.BlockCopy(entropy, 0, password, 0, entropy.Length); password[entropy.Length] = 0x00; - Buffer.BlockCopy(machineBytes, 0, password, entropy.Length + 1, machineBytes.Length); + Buffer.BlockCopy(contextBytes, 0, password, entropy.Length + 1, contextBytes.Length); return Rfc2898DeriveBytes.Pbkdf2(password, salt, LegacyPbkdf2Iterations, HashAlgorithmName.SHA256, LegacyKeySize); } @@ -434,13 +452,31 @@ public async Task Keystore_WrittenByThisVersion_CarriesFormatHeader() _ = await encryption.EncryptAsync("trigger keystore creation"); var bytes = await File.ReadAllBytesAsync(Path.Join(temp.Path, ".keystore"), TestContext.Current.CancellationToken); - Assert.True(bytes.Length > KeystoreMagic.Length + 1); + // Header is magic | version(3) | salt(16) | payload. + Assert.True(bytes.Length > KeystoreMagic.Length + 1 + 16); Assert.Equal(KeystoreMagic, bytes[..KeystoreMagic.Length]); - Assert.Equal(2, bytes[KeystoreMagic.Length]); // format version (2 = OSVersion-free KEK) + Assert.Equal(3, bytes[KeystoreMagic.Length]); // format version (3 = random-salt, no identity) + } + + [Fact] + public async Task Keystore_V3_HeaderSaltIsRandomPerKeystore() + { + using var tempA = new TempDir(); + using var tempB = new TempDir(); + _ = await new LocalFileCredentialEncryption(tempA.Path).EncryptAsync("a"); + _ = await new LocalFileCredentialEncryption(tempB.Path).EncryptAsync("b"); + + var a = await File.ReadAllBytesAsync(Path.Join(tempA.Path, ".keystore"), TestContext.Current.CancellationToken); + var b = await File.ReadAllBytesAsync(Path.Join(tempB.Path, ".keystore"), TestContext.Current.CancellationToken); + + var offset = KeystoreMagic.Length + 1; + var saltA = a[offset..(offset + 16)]; + var saltB = b[offset..(offset + 16)]; + Assert.NotEqual(saltA, saltB); } [Fact] - public async Task Keystore_LegacyV1_IsMigratedToV2_AndStillDecrypts() + public async Task Keystore_LegacyV1_IsMigratedToV3_AndStillDecrypts() { using var temp = new TempDir(); var keystorePath = Path.Join(temp.Path, ".keystore"); @@ -458,10 +494,10 @@ public async Task Keystore_LegacyV1_IsMigratedToV2_AndStillDecrypts() var reader = new LocalFileCredentialEncryption(temp.Path); Assert.Equal("bound to a v1 keystore", await reader.DecryptAsync(cipher)); - // The first load migrated the file in place: the version byte is now 2. + // The first load migrated the file in place: the version byte is now 3. var bytes = await File.ReadAllBytesAsync(keystorePath, TestContext.Current.CancellationToken); Assert.Equal(KeystoreMagic, bytes[..KeystoreMagic.Length]); - Assert.Equal(2, bytes[KeystoreMagic.Length]); + Assert.Equal(3, bytes[KeystoreMagic.Length]); // And the migrated keystore opens under a further fresh instance. var afterMigration = new LocalFileCredentialEncryption(temp.Path); @@ -469,7 +505,7 @@ public async Task Keystore_LegacyV1_IsMigratedToV2_AndStillDecrypts() } [Fact] - public async Task Keystore_LegacyHeaderless_IsMigratedToV2_AndStillReadable() + public async Task Keystore_LegacyHeaderless_IsMigratedToV3_AndStillReadable() { using var temp = new TempDir(); var keystorePath = Path.Join(temp.Path, ".keystore"); @@ -488,11 +524,11 @@ public async Task Keystore_LegacyHeaderless_IsMigratedToV2_AndStillReadable() // Migrated up to the current header + version. var bytes = await File.ReadAllBytesAsync(keystorePath, TestContext.Current.CancellationToken); Assert.Equal(KeystoreMagic, bytes[..KeystoreMagic.Length]); - Assert.Equal(2, bytes[KeystoreMagic.Length]); + Assert.Equal(3, bytes[KeystoreMagic.Length]); } [Fact] - public async Task Keystore_LegacyV1_WithCallerEntropy_IsMigratedToV2() + public async Task Keystore_LegacyV1_WithCallerEntropy_IsMigratedToV3() { using var temp = new TempDir(); var keystorePath = Path.Join(temp.Path, ".keystore"); @@ -509,7 +545,49 @@ public async Task Keystore_LegacyV1_WithCallerEntropy_IsMigratedToV2() Assert.Equal("v1 with entropy", await reader.DecryptAsync(cipher)); var bytes = await File.ReadAllBytesAsync(keystorePath, TestContext.Current.CancellationToken); - Assert.Equal(2, bytes[KeystoreMagic.Length]); + Assert.Equal(3, bytes[KeystoreMagic.Length]); + } + + [Fact] + public async Task Keystore_LegacyV2_IsMigratedToV3_AndStillDecrypts() + { + using var temp = new TempDir(); + var keystorePath = Path.Join(temp.Path, ".keystore"); + + // Forge a genuine version-2 keystore (machine/user KEK, no OSVersion) + // wrapping the same data key — the shape the previous release wrote. + var writer = new LocalFileCredentialEncryption(temp.Path); + var cipher = await writer.EncryptAsync("bound to a v2 keystore"); + var dataKey = await GetDataKeyAsync(writer); + await WriteV2KeystoreAsync(temp.Path, dataKey); + + var reader = new LocalFileCredentialEncryption(temp.Path); + Assert.Equal("bound to a v2 keystore", await reader.DecryptAsync(cipher)); + + // Re-sealed as v3 (random salt, no identity). + var bytes = await File.ReadAllBytesAsync(keystorePath, TestContext.Current.CancellationToken); + Assert.Equal(KeystoreMagic, bytes[..KeystoreMagic.Length]); + Assert.Equal(3, bytes[KeystoreMagic.Length]); + Assert.True(bytes.Length > KeystoreMagic.Length + 1 + 16); + } + + [Fact] + public async Task Keystore_LegacyV2_WithCallerEntropy_IsMigratedToV3() + { + using var temp = new TempDir(); + var keystorePath = Path.Join(temp.Path, ".keystore"); + var entropy = "deployment-secret"u8.ToArray(); + + var writer = new LocalFileCredentialEncryption(temp.Path, entropy); + var cipher = await writer.EncryptAsync("v2 with entropy"); + var dataKey = await GetDataKeyAsync(writer); + await WriteV2KeystoreAsync(temp.Path, dataKey, entropy); + + var reader = new LocalFileCredentialEncryption(temp.Path, entropy); + Assert.Equal("v2 with entropy", await reader.DecryptAsync(cipher)); + + var bytes = await File.ReadAllBytesAsync(keystorePath, TestContext.Current.CancellationToken); + Assert.Equal(3, bytes[KeystoreMagic.Length]); } [Fact]