Cluster 2 — Security Hardening (DEK lifecycle, Argon2id alignment, Backup KDF) [v2.0]

src/PassKey.Core/Services/BackupService.cs

@@ -8,7 +8,13 @@ namespace PassKey.Core.Services;
 public sealed class BackupService : IBackupService
 {
     private static readonly byte[] Magic = [0x50, 0x4B, 0x42, 0x4B]; // "PKBK"
-    private const byte Version = 0x01;
+
+    /// <summary>Legacy backup format derived with PBKDF2-SHA256 (600k iterations).</summary>
+    private const byte VersionPbkdf2 = 0x01;
+
+    /// <summary>Current backup format derived with Argon2id (OWASP 2023 parameters).</summary>
+    private const byte VersionArgon2Id = 0x02;
+
     private const int HeaderSize = 5; // magic(4) + version(1)
 
     private readonly ICryptoService _crypto;
@@ -18,6 +24,12 @@ public BackupService(ICryptoService crypto)
         _crypto = crypto ?? throw new ArgumentNullException(nameof(crypto));
     }
 
+    /// <summary>
+    /// Serialises and encrypts the supplied <paramref name="vault"/> to a self-describing
+    /// <c>.pkbak</c> blob. New backups always use Argon2id (version byte 0x02); the
+    /// <see cref="RestoreFromBlob"/> path still accepts legacy 0x01 (PBKDF2) blobs for
+    /// backward compatibility with v1.x backups produced before this change.
+    /// </summary>
     public byte[] CreateBackupBlob(Vault vault, ReadOnlySpan<char> backupPassword)
     {
         ArgumentNullException.ThrowIfNull(vault);
@@ -26,13 +38,23 @@ public byte[] CreateBackupBlob(Vault vault, ReadOnlySpan<char> backupPassword)
         try
         {
             var salt = _crypto.GenerateRandomBytes(CryptoConstants.SaltSizeBytes);
-            using var key = _crypto.DeriveKeyFromPassword(backupPassword, salt, CryptoConstants.DefaultKdfIterations);
+            // Argon2id for new backups — restores symmetry with the main vault KEK,
+            // which has been Argon2id-derived since v1.0.x.
+            // IMPORTANT: pass 0 as `iterations` so CryptoService falls back to the
+            // Argon2-tuned default (CryptoConstants.Argon2TimeCost = 3). Forwarding
+            // CryptoConstants.DefaultKdfIterations (600,000 — PBKDF2-shaped) to Argon2id
+            // would cost ~600,000 × 64 MB memory passes and never complete.
+            using var key = _crypto.DeriveKeyFromPassword(
+                backupPassword,
+                salt,
+                iterations: 0,
+                CryptoConstants.KdfAlgorithmArgon2Id);
 
             var encryptedPayload = _crypto.Encrypt(jsonBytes, key.ReadOnlySpan);
 
             var blob = new byte[HeaderSize + CryptoConstants.SaltSizeBytes + encryptedPayload.Length];
             Magic.CopyTo(blob, 0);
-            blob[4] = Version;
+            blob[4] = VersionArgon2Id;
             salt.CopyTo(blob.AsSpan(HeaderSize));
             encryptedPayload.CopyTo(blob.AsSpan(HeaderSize + CryptoConstants.SaltSizeBytes));
 
@@ -44,6 +66,11 @@ public byte[] CreateBackupBlob(Vault vault, ReadOnlySpan<char> backupPassword)
         }
     }
 
+    /// <summary>
+    /// Restores a vault from a <c>.pkbak</c> blob, dispatching to the appropriate KDF
+    /// based on the version byte: <see cref="VersionPbkdf2"/> (legacy v1.x backups,
+    /// PBKDF2-SHA256) or <see cref="VersionArgon2Id"/> (current, Argon2id).
+    /// </summary>
     public Vault RestoreFromBlob(ReadOnlySpan<byte> blob, ReadOnlySpan<char> backupPassword)
     {
         var minSize = HeaderSize + CryptoConstants.SaltSizeBytes
@@ -55,13 +82,29 @@ public Vault RestoreFromBlob(ReadOnlySpan<byte> blob, ReadOnlySpan<char> backupP
         if (!blob[..4].SequenceEqual(Magic))
             throw new InvalidDataException("Not a valid PassKey backup file (bad magic).");
 
-        if (blob[4] != Version)
-            throw new NotSupportedException($"Unsupported backup version: {blob[4]}.");
+        var version = blob[4];
+        var kdfAlgorithm = version switch
+        {
+            VersionPbkdf2   => CryptoConstants.KdfAlgorithmPbkdf2,
+            VersionArgon2Id => CryptoConstants.KdfAlgorithmArgon2Id,
+            _ => throw new NotSupportedException($"Unsupported backup version: {version}."),
+        };
 
         var salt = blob.Slice(HeaderSize, CryptoConstants.SaltSizeBytes).ToArray();
         var payload = blob[(HeaderSize + CryptoConstants.SaltSizeBytes)..];
 
-        using var key = _crypto.DeriveKeyFromPassword(backupPassword, salt, CryptoConstants.DefaultKdfIterations);
+        // Iterations is only meaningful for PBKDF2 (legacy v0x01 backups); for Argon2id
+        // CryptoService ignores positive non-default iterations only when the value is 0,
+        // so we pass 0 for the Argon2id path. For PBKDF2 we honour the historic count.
+        var iterations = kdfAlgorithm == CryptoConstants.KdfAlgorithmArgon2Id
+            ? 0
+            : CryptoConstants.DefaultKdfIterations;
+
+        using var key = _crypto.DeriveKeyFromPassword(
+            backupPassword,
+            salt,
+            iterations,
+            kdfAlgorithm);
         var jsonBytes = _crypto.Decrypt(payload, key.ReadOnlySpan);
         try
         {

src/PassKey.Core/Services/CryptoService.cs

@@ -22,12 +22,17 @@ public PinnedSecureBuffer DeriveKeyFromPassword(ReadOnlySpan<char> password, byt
 
                 if (kdfAlgorithm == CryptoConstants.KdfAlgorithmArgon2Id)
                 {
+                    // Honour an explicit caller-supplied iteration count when positive; fall back
+                    // to the OWASP-tuned default in CryptoConstants for the (legitimate) callers
+                    // that pass 0 — e.g. unit tests or backward-compat code paths that don't know
+                    // about Argon2-specific tunings. This keeps the Argon2id path symmetric with
+                    // the PBKDF2 branch below, which already threads the parameter through.
                     using var argon2 = new Argon2id(passwordBytes)
                     {
                         Salt                = salt,
                         DegreeOfParallelism = CryptoConstants.Argon2Parallelism,
                         MemorySize          = CryptoConstants.Argon2MemoryCostKiB,
-                        Iterations          = CryptoConstants.Argon2TimeCost
+                        Iterations          = iterations > 0 ? iterations : CryptoConstants.Argon2TimeCost,
                     };
                     var derived = argon2.GetBytes(CryptoConstants.KeySizeBytes);
                     try   { derived.CopyTo(buffer.Span); }

src/PassKey.Desktop/Services/VaultStateService.cs

@@ -71,28 +71,53 @@ public async Task<bool> InitializeAsync(ReadOnlyMemory<char> masterPassword)
     /// </summary>
     /// <param name="masterPassword">The master password to verify.</param>
     /// <returns>True if the password is correct and the vault is now unlocked; false otherwise.</returns>
+    /// <remarks>
+    /// <b>DEK lifecycle.</b> The derived <see cref="PinnedSecureBuffer"/> is held in a local
+    /// variable until <see cref="IVaultService.DecryptVault"/> has succeeded. Only then is it
+    /// promoted to the long-lived <see cref="_dek"/> field and <see cref="CurrentVault"/>
+    /// assigned. If decryption throws (corrupt blob, GCM tag failure, unexpected DEK length,
+    /// etc.), the local buffer is zeroed and disposed in the <c>catch</c>/<c>finally</c> path,
+    /// so plaintext key material is never retained in memory after a failed unlock.
+    /// </remarks>
     public async Task<bool> UnlockAsync(ReadOnlyMemory<char> masterPassword)
     {
         var metadata = await _repository.LoadMetadataAsync();
         if (metadata is null) return false;
 
+        PinnedSecureBuffer? candidateDek;
         try
         {
-            _dek = _vaultService.UnlockVault(masterPassword.Span, metadata);
+            candidateDek = _vaultService.UnlockVault(masterPassword.Span, metadata);
         }
-        catch
+        catch (Exception ex)
         {
+            // Wrong password / unsupported KDF / corrupted metadata — nothing to clean up,
+            // the constructor of the failed buffer (if any) is the implementation's
+            // responsibility. Surface the exception type/message to a Debug listener so
+            // diagnostics aren't lost during development; we still return false so the
+            // user-facing path stays generic ("incorrect master password").
+            System.Diagnostics.Debug.WriteLine($"[VaultStateService] UnlockVault failed: {ex.GetType().Name}: {ex.Message}");
             return false;
         }
 
-        var encryptedBlob = await _repository.LoadEncryptedVaultAsync();
-        if (encryptedBlob is null)
+        try
         {
-            CurrentVault = new Vault();
+            var encryptedBlob = await _repository.LoadEncryptedVaultAsync();
+            Vault decryptedVault = encryptedBlob is null
+                ? new Vault()
+                : _vaultService.DecryptVault(encryptedBlob, candidateDek.ReadOnlySpan);
+
+            // Promote only after decryption succeeds — the previous implementation assigned
+            // _dek before this line, leaving the plaintext key in memory if decryption threw.
+            _dek = candidateDek;
+            CurrentVault = decryptedVault;
+            candidateDek = null; // ownership transferred to _dek; skip the cleanup below
         }
-        else
+        finally
         {
-            CurrentVault = _vaultService.DecryptVault(encryptedBlob, _dek.ReadOnlySpan);
+            // If we never promoted (decrypt failed or threw) zero and release the candidate
+            // immediately so the DEK does not outlive a failed unlock attempt.
+            candidateDek?.Dispose();
         }
 
         VaultUnlocked?.Invoke();

src/PassKey.Desktop/ViewModels/LoginViewModel.cs

@@ -91,8 +91,12 @@ private async Task LoginAsync(string password)
                 ErrorMessage = "IncorrectPassword"; // Localization key
             }
         }
-        catch
+        catch (Exception ex)
         {
+            // Generic catch — UI shows a localized "UnlockFailed" key without leaking the
+            // underlying error to the user. Forward the original exception to Debug so it's
+            // still visible to developers during local diagnostics.
+            System.Diagnostics.Debug.WriteLine($"[LoginViewModel] Login failed: {ex.GetType().Name}: {ex.Message}");
             HasError = true;
             ErrorMessage = "UnlockFailed"; // Localization key
         }

src/PassKey.Tests/BackupServiceTests.cs

@@ -42,7 +42,48 @@ public void CreateBackupBlob_HasCorrectMagicHeader()
         Assert.Equal(0x4B, blob[1]); // 'K'
         Assert.Equal(0x42, blob[2]); // 'B'
         Assert.Equal(0x4B, blob[3]); // 'K'
-        Assert.Equal(0x01, blob[4]); // version
+        // PassKey 2.0 backups are written with version 0x02 (Argon2id KDF). The 0x01
+        // version (PBKDF2-SHA256) is still accepted on restore for v1.x compatibility.
+        Assert.Equal(0x02, blob[4]);
+    }
+
+    [Fact]
+    public void CreateBackupBlob_UsesArgon2id_VersionByteIs2()
+    {
+        // Regression guard: ensure new backups never silently regress to the legacy
+        // PBKDF2 format. The version byte at offset 4 must be exactly 0x02.
+        var vault = new Vault();
+        var blob = _backup.CreateBackupBlob(vault, "AnyP@ssword1!".AsSpan());
+        Assert.Equal(0x02, blob[4]);
+    }
+
+    [Fact]
+    public void RestoreFromBlob_LegacyV1Pbkdf2Format_StillRestores()
+    {
+        // Hand-craft a legacy v1.x backup blob — derive key with PBKDF2, write 0x01 in
+        // the version byte — and verify that PassKey 2.0 can still restore it. This
+        // guarantees that .pkbak files produced by previous releases remain usable.
+        var password = "Legacy@Backup1!".AsSpan();
+        var vault = CreateSampleVault();
+        var jsonBytes = System.Text.Json.JsonSerializer.SerializeToUtf8Bytes(vault, VaultJsonContext.Default.Vault);
+        var salt = _crypto.GenerateRandomBytes(PassKey.Core.Constants.CryptoConstants.SaltSizeBytes);
+        using var key = _crypto.DeriveKeyFromPassword(
+            password,
+            salt,
+            PassKey.Core.Constants.CryptoConstants.DefaultKdfIterations,
+            PassKey.Core.Constants.CryptoConstants.KdfAlgorithmPbkdf2);
+        var encrypted = _crypto.Encrypt(jsonBytes, key.ReadOnlySpan);
+
+        var legacyBlob = new byte[5 + PassKey.Core.Constants.CryptoConstants.SaltSizeBytes + encrypted.Length];
+        legacyBlob[0] = 0x50; legacyBlob[1] = 0x4B; legacyBlob[2] = 0x42; legacyBlob[3] = 0x4B;
+        legacyBlob[4] = 0x01; // legacy version byte
+        salt.CopyTo(legacyBlob.AsSpan(5));
+        encrypted.CopyTo(legacyBlob.AsSpan(5 + PassKey.Core.Constants.CryptoConstants.SaltSizeBytes));
+
+        var restored = _backup.RestoreFromBlob(legacyBlob, password);
+
+        Assert.Single(restored.Passwords);
+        Assert.Equal("GitHub", restored.Passwords[0].Title);
     }
 
     [Fact]

src/PassKey.Tests/CryptoServiceTests.cs

@@ -144,4 +144,38 @@ public void PinnedSecureBuffer_ZerosOnDispose()
         // After dispose, accessing Span should throw
         Assert.Throws<ObjectDisposedException>(() => buffer.Span.ToArray());
     }
+
+    [Fact]
+    public void DeriveKeyFromPassword_Argon2id_RespectsIterationsParam()
+    {
+        // The Argon2id branch must honour an explicit `iterations` value when positive,
+        // not silently fall back to CryptoConstants.Argon2TimeCost as the v1.x code did.
+        // Verified indirectly: a derivation with iterations=1 must NOT equal a derivation
+        // with the OWASP-tuned default (Argon2TimeCost > 1).
+        var salt = _crypto.GenerateRandomBytes(CryptoConstants.SaltSizeBytes);
+        var password = "Argon2-Iterations-Test".AsSpan();
+
+        using var keyExplicit1 = _crypto.DeriveKeyFromPassword(password, salt, iterations: 1, CryptoConstants.KdfAlgorithmArgon2Id);
+        using var keyDefault   = _crypto.DeriveKeyFromPassword(password, salt, iterations: 0, CryptoConstants.KdfAlgorithmArgon2Id);
+
+        // Different iteration counts → different key material (same password+salt).
+        // If the iterations parameter were ignored, the two keys would be identical.
+        Assert.False(keyExplicit1.Span.SequenceEqual(keyDefault.Span),
+            "Argon2id derivation must use the supplied iterations count, not the default.");
+    }
+
+    [Fact]
+    public void DeriveKeyFromPassword_Argon2id_ZeroIterations_FallsBackToDefault()
+    {
+        // Passing iterations=0 documents the contract that the default (Argon2TimeCost)
+        // is used. Two derivations with iterations=0 must produce identical keys.
+        var salt = _crypto.GenerateRandomBytes(CryptoConstants.SaltSizeBytes);
+        var password = "Argon2-Default-Fallback".AsSpan();
+
+        using var keyA = _crypto.DeriveKeyFromPassword(password, salt, iterations: 0, CryptoConstants.KdfAlgorithmArgon2Id);
+        using var keyB = _crypto.DeriveKeyFromPassword(password, salt, iterations: 0, CryptoConstants.KdfAlgorithmArgon2Id);
+
+        Assert.True(keyA.Span.SequenceEqual(keyB.Span),
+            "Two Argon2id derivations with the same password+salt and iterations=0 must produce the same key.");
+    }
 }