diff --git a/src/PassKey.Core/Services/BackupService.cs b/src/PassKey.Core/Services/BackupService.cs index 8fc3fa3..9c84457 100644 --- a/src/PassKey.Core/Services/BackupService.cs +++ b/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; + + /// Legacy backup format derived with PBKDF2-SHA256 (600k iterations). + private const byte VersionPbkdf2 = 0x01; + + /// Current backup format derived with Argon2id (OWASP 2023 parameters). + 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)); } + /// + /// Serialises and encrypts the supplied to a self-describing + /// .pkbak blob. New backups always use Argon2id (version byte 0x02); the + /// path still accepts legacy 0x01 (PBKDF2) blobs for + /// backward compatibility with v1.x backups produced before this change. + /// public byte[] CreateBackupBlob(Vault vault, ReadOnlySpan backupPassword) { ArgumentNullException.ThrowIfNull(vault); @@ -26,13 +38,23 @@ public byte[] CreateBackupBlob(Vault vault, ReadOnlySpan 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 backupPassword) } } + /// + /// Restores a vault from a .pkbak blob, dispatching to the appropriate KDF + /// based on the version byte: (legacy v1.x backups, + /// PBKDF2-SHA256) or (current, Argon2id). + /// public Vault RestoreFromBlob(ReadOnlySpan blob, ReadOnlySpan backupPassword) { var minSize = HeaderSize + CryptoConstants.SaltSizeBytes @@ -55,13 +82,29 @@ public Vault RestoreFromBlob(ReadOnlySpan blob, ReadOnlySpan 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 { diff --git a/src/PassKey.Core/Services/CryptoService.cs b/src/PassKey.Core/Services/CryptoService.cs index 4bbcf01..dd90e0e 100644 --- a/src/PassKey.Core/Services/CryptoService.cs +++ b/src/PassKey.Core/Services/CryptoService.cs @@ -22,12 +22,17 @@ public PinnedSecureBuffer DeriveKeyFromPassword(ReadOnlySpan 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); } diff --git a/src/PassKey.Desktop/Services/VaultStateService.cs b/src/PassKey.Desktop/Services/VaultStateService.cs index 0215219..2088d60 100644 --- a/src/PassKey.Desktop/Services/VaultStateService.cs +++ b/src/PassKey.Desktop/Services/VaultStateService.cs @@ -71,28 +71,53 @@ public async Task InitializeAsync(ReadOnlyMemory masterPassword) /// /// The master password to verify. /// True if the password is correct and the vault is now unlocked; false otherwise. + /// + /// DEK lifecycle. The derived is held in a local + /// variable until has succeeded. Only then is it + /// promoted to the long-lived field and + /// assigned. If decryption throws (corrupt blob, GCM tag failure, unexpected DEK length, + /// etc.), the local buffer is zeroed and disposed in the catch/finally path, + /// so plaintext key material is never retained in memory after a failed unlock. + /// public async Task UnlockAsync(ReadOnlyMemory 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(); diff --git a/src/PassKey.Desktop/ViewModels/LoginViewModel.cs b/src/PassKey.Desktop/ViewModels/LoginViewModel.cs index af6fe3f..c96a44b 100644 --- a/src/PassKey.Desktop/ViewModels/LoginViewModel.cs +++ b/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 } diff --git a/src/PassKey.Tests/BackupServiceTests.cs b/src/PassKey.Tests/BackupServiceTests.cs index db37f03..86c0a08 100644 --- a/src/PassKey.Tests/BackupServiceTests.cs +++ b/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] diff --git a/src/PassKey.Tests/CryptoServiceTests.cs b/src/PassKey.Tests/CryptoServiceTests.cs index 0e8e90e..214ec09 100644 --- a/src/PassKey.Tests/CryptoServiceTests.cs +++ b/src/PassKey.Tests/CryptoServiceTests.cs @@ -144,4 +144,38 @@ public void PinnedSecureBuffer_ZerosOnDispose() // After dispose, accessing Span should throw Assert.Throws(() => 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."); + } }