From eea63d7faf466f0cf6a87e9cc7d857d0c7b6bb9c Mon Sep 17 00:00:00 2001 From: Giuseppe Imperato Date: Mon, 8 Jun 2026 15:14:31 +0200 Subject: [PATCH 1/2] fix(security): IPC hardening + clipboard/backup/gitignore (audit giu-2026) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit di sicurezza post v2.0.0. Il nucleo crypto era solido; questi fix chiudono le superfici residue, soprattutto sul canale IPC verso le estensioni. IPC (BrowserIpcService + estensioni chrome/firefox): - Rimosso il fallback in PLAINTEXT di get-credential-password: la password viene ora SEMPRE cifrata AES-GCM con la chiave della sessione ECDH. Un client che salta l'handshake riceve "no-session" anziché il segreto in chiaro. - La sessione è legata alla richiesta tramite SessionId nel payload (GetCredentialPasswordRequest.SessionId) invece di "una sessione valida qualsiasi". Aggiornate entrambe le estensioni per inviarlo. - Throttling su unlock-vault: lockout 30s dopo 5 tentativi falliti. - Cap di 32 sessioni ECDH con evict del più vecchio (anti-DoS memoria). - Diagnostica Debug nei catch finora silenziosi. Altri fix: - ClipboardService: Flush() solo per contenuti non sensibili, così le password non sopravvivono alla chiusura dell'app (oltre all'auto-clear 30s). - BackupFileService: rotazione auto-backup (mantiene gli ultimi 10). - .gitignore: ignora *.pkbak/*.autobak/passkey.db e artefatti locali, per evitare il commit accidentale di vault cifrati reali. Build: 0 errori/0 warning. Test: 222/222 passati. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 14 ++ extensions/chrome/background.js | 34 ++-- extensions/firefox/background.js | 34 ++-- .../Services/BackupFileService.cs | 30 ++++ .../Services/BrowserIpcService.cs | 155 ++++++++++++------ .../Services/ClipboardService.cs | 17 +- src/PassKey.Desktop/Services/IpcModels.cs | 9 +- 7 files changed, 204 insertions(+), 89 deletions(-) diff --git a/.gitignore b/.gitignore index feac9fe..8ae5eb7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,14 @@ # ─── Sensitive — NEVER commit ─────────────────────────────────────────────── *.pem passkey-extension-key.pem +# Encrypted vault backups & exports (real user data — must never be published) +*.pkbak +*.autobak +PassKey_Backup*.pkbak +# Local SQLite vault database (should never live in the repo, but guard anyway) +passkey.db +*.db-wal +*.db-shm # ─── Build output ──────────────────────────────────────────────────────────── bin/ @@ -54,6 +62,12 @@ web-ext-artifacts/ # ─── Publish output ────────────────────────────────────────────────────────── publish/ +publish_test*/ + +# ─── Local build artefacts / scratch (never commit) ────────────────────────── +PassKey-Portable-x64.zip +Installer/*.exe +demo-data/ # ─── claude-code workspace artefacts (per-developer, never commit) ─────────── .claude/ diff --git a/extensions/chrome/background.js b/extensions/chrome/background.js index 070985d..bb01206 100644 --- a/extensions/chrome/background.js +++ b/extensions/chrome/background.js @@ -271,16 +271,16 @@ async function handleGetAllCredentials() { async function handleCopyCredential(credentialId) { try { await ensureSession(); - const req = buildRequest('get-credential-password', { id: credentialId }); + const req = buildRequest('get-credential-password', { id: credentialId, sessionId }); const resp = parseResponse(await sendNativeMessage(req)); if (!resp.success) return resp; - let password; - if (resp.payload.nonce && resp.payload.nonce.length > 0) { - password = await decryptPassword(sessionKey, resp.payload.nonce, resp.payload.encryptedPassword); - } else { - password = new TextDecoder().decode(base64ToUint8Array(resp.payload.encryptedPassword)); + // Desktop always returns an AES-GCM encrypted password tied to the ECDH session. + // A missing nonce means no valid session — never accept a plaintext fallback. + if (!resp.payload?.nonce || resp.payload.nonce.length === 0) { + return { success: false, error: 'no-session' }; } + const password = await decryptPassword(sessionKey, resp.payload.nonce, resp.payload.encryptedPassword); return { success: true, password }; } catch (err) { return { success: false, error: err.message || 'copy-failed' }; @@ -370,25 +370,23 @@ async function handleFillCredential(credentialId, username, tabId) { await ensureSession(); // Request encrypted password - const req = buildRequest('get-credential-password', { id: credentialId }); + const req = buildRequest('get-credential-password', { id: credentialId, sessionId }); const resp = parseResponse(await sendNativeMessage(req)); if (!resp.success) { return resp; } - // Decrypt password using session key - let password; - if (resp.payload.nonce && resp.payload.nonce.length > 0) { - password = await decryptPassword( - sessionKey, - resp.payload.nonce, - resp.payload.encryptedPassword - ); - } else { - // No session encryption — decode Base64 plaintext (fallback) - password = new TextDecoder().decode(base64ToUint8Array(resp.payload.encryptedPassword)); + // Desktop always returns an AES-GCM encrypted password tied to the ECDH session. + // A missing nonce means no valid session — never accept a plaintext fallback. + if (!resp.payload?.nonce || resp.payload.nonce.length === 0) { + return { success: false, error: 'no-session' }; } + const password = await decryptPassword( + sessionKey, + resp.payload.nonce, + resp.payload.encryptedPassword + ); // Send credentials to content script for form filling await chrome.tabs.sendMessage(tabId, { diff --git a/extensions/firefox/background.js b/extensions/firefox/background.js index b5f1b5a..05f64b8 100644 --- a/extensions/firefox/background.js +++ b/extensions/firefox/background.js @@ -274,16 +274,16 @@ async function handleGetAllCredentials() { async function handleCopyCredential(credentialId) { try { await ensureSession(); - const req = buildRequest('get-credential-password', { id: credentialId }); + const req = buildRequest('get-credential-password', { id: credentialId, sessionId }); const resp = parseResponse(await sendNativeMessage(req)); if (!resp.success) return resp; - let password; - if (resp.payload.nonce && resp.payload.nonce.length > 0) { - password = await decryptPassword(sessionKey, resp.payload.nonce, resp.payload.encryptedPassword); - } else { - password = new TextDecoder().decode(base64ToUint8Array(resp.payload.encryptedPassword)); + // Desktop always returns an AES-GCM encrypted password tied to the ECDH session. + // A missing nonce means no valid session — never accept a plaintext fallback. + if (!resp.payload?.nonce || resp.payload.nonce.length === 0) { + return { success: false, error: 'no-session' }; } + const password = await decryptPassword(sessionKey, resp.payload.nonce, resp.payload.encryptedPassword); return { success: true, password }; } catch (err) { return { success: false, error: err.message || 'copy-failed' }; @@ -373,25 +373,23 @@ async function handleFillCredential(credentialId, username, tabId) { await ensureSession(); // Request encrypted password - const req = buildRequest('get-credential-password', { id: credentialId }); + const req = buildRequest('get-credential-password', { id: credentialId, sessionId }); const resp = parseResponse(await sendNativeMessage(req)); if (!resp.success) { return resp; } - // Decrypt password using session key - let password; - if (resp.payload.nonce && resp.payload.nonce.length > 0) { - password = await decryptPassword( - sessionKey, - resp.payload.nonce, - resp.payload.encryptedPassword - ); - } else { - // No session encryption — decode Base64 plaintext (fallback) - password = new TextDecoder().decode(base64ToUint8Array(resp.payload.encryptedPassword)); + // Desktop always returns an AES-GCM encrypted password tied to the ECDH session. + // A missing nonce means no valid session — never accept a plaintext fallback. + if (!resp.payload?.nonce || resp.payload.nonce.length === 0) { + return { success: false, error: 'no-session' }; } + const password = await decryptPassword( + sessionKey, + resp.payload.nonce, + resp.payload.encryptedPassword + ); // Send credentials to content script for form filling await browser.tabs.sendMessage(tabId, { diff --git a/src/PassKey.Desktop/Services/BackupFileService.cs b/src/PassKey.Desktop/Services/BackupFileService.cs index 0c315ee..a9d36a7 100644 --- a/src/PassKey.Desktop/Services/BackupFileService.cs +++ b/src/PassKey.Desktop/Services/BackupFileService.cs @@ -23,11 +23,41 @@ public async Task ReadBackupAsync(string filePath) return await File.ReadAllBytesAsync(filePath); } + /// Number of most-recent automatic backups to retain; older ones are pruned. + private const int MaxAutoBackups = 10; + public async Task WriteAutoBackupAsync(byte[] currentEncryptedBlob) { Directory.CreateDirectory(BackupDir); var timestamp = DateTime.UtcNow.ToString("yyyyMMddTHHmmss"); var autoBackupPath = Path.Combine(BackupDir, $"vault.{timestamp}.autobak"); await File.WriteAllBytesAsync(autoBackupPath, currentEncryptedBlob); + + PruneOldAutoBackups(); + } + + /// + /// Keeps only the most recent vault.*.autobak files, + /// deleting older ones. Prevents the automatic-backup folder from growing without bound. + /// Deletion failures are ignored (a locked/transient file must not break the backup flow). + /// + private static void PruneOldAutoBackups() + { + try + { + var oldBackups = Directory.GetFiles(BackupDir, "vault.*.autobak") + .OrderByDescending(path => path, StringComparer.Ordinal) // timestamped name sorts chronologically + .Skip(MaxAutoBackups); + + foreach (var path in oldBackups) + { + try { File.Delete(path); } + catch { /* ignore individual deletion failures */ } + } + } + catch + { + // Enumeration failure must never break the backup operation itself. + } } } diff --git a/src/PassKey.Desktop/Services/BrowserIpcService.cs b/src/PassKey.Desktop/Services/BrowserIpcService.cs index 90eb3f2..925ed73 100644 --- a/src/PassKey.Desktop/Services/BrowserIpcService.cs +++ b/src/PassKey.Desktop/Services/BrowserIpcService.cs @@ -38,11 +38,25 @@ internal sealed class BrowserIpcService : IBrowserIpcService private const int LengthPrefixSize = 4; private static readonly TimeSpan SessionTimeout = TimeSpan.FromHours(1); + // Upper bound on concurrent ECDH sessions. A malicious same-user process could otherwise + // spam exchange-keys to grow the dictionary unbounded (sessions are only pruned hourly). + // When the cap is hit we evict the oldest session before accepting a new one. + private const int MaxSessions = 32; + + // unlock-vault brute-force throttling. The KDF (Argon2id, 64 MiB) already makes guessing + // slow, but we add an explicit lockout so a same-user process cannot hammer the pipe. + private const int MaxUnlockFailures = 5; + private static readonly TimeSpan UnlockLockout = TimeSpan.FromSeconds(30); + private readonly IVaultStateService _vaultState; private readonly ICryptoService _crypto; private readonly Dictionary _sessions = new(); private readonly Lock _sessionsLock = new(); + private readonly Lock _unlockLock = new(); + private int _unlockFailedCount; + private DateTime _unlockLockoutUntil = DateTime.MinValue; + private CancellationTokenSource? _cts; private Task? _serverTask; @@ -123,9 +137,11 @@ private async Task ServerLoopAsync(CancellationToken ct) { break; } - catch + catch (Exception ex) { - // Individual connection errors should not kill the server + // Individual connection errors should not kill the server; surface to Debug + // so diagnostics aren't lost while keeping the loop resilient. + System.Diagnostics.Debug.WriteLine($"[BrowserIpcService] connection error: {ex.GetType().Name}: {ex.Message}"); } finally { @@ -238,8 +254,9 @@ internal async Task ProcessMessageAsync(string requestJson) return SerializeResponse(response); } - catch + catch (Exception ex) { + System.Diagnostics.Debug.WriteLine($"[BrowserIpcService] handler '{request.Action}' failed: {ex.GetType().Name}: {ex.Message}"); return SerializeResponse(new IpcResponse { Action = request.Action, @@ -313,6 +330,22 @@ private IpcResponse HandleExchangeKeys(IpcRequest request) lock (_sessionsLock) { + // Bound the session table: drop expired entries first, then if still at the + // cap evict the oldest session so a flood of exchange-keys can't exhaust memory. + if (_sessions.Count >= MaxSessions) + { + PruneExpiredSessionsNoLock(); + if (_sessions.Count >= MaxSessions) + { + var oldest = _sessions.Values.MinBy(s => s.CreatedAt); + if (oldest is not null) + { + CryptographicOperations.ZeroMemory(oldest.SessionKey); + _sessions.Remove(oldest.SessionId); + } + } + } + _sessions[sessionId] = session; } @@ -483,71 +516,62 @@ private IpcResponse HandleGetCredentialPassword(IpcRequest request) }; } - // Validate session for password retrieval (requires exchange-keys first) + // Require a valid ECDH session BOUND to this request (by session ID). The password is + // always AES-GCM encrypted with that session's key — there is no plaintext fallback. + // A caller that hasn't completed exchange-keys (or whose session expired) is rejected, + // so the encrypted channel can never be bypassed by simply omitting the handshake. SessionInfo? session = null; - if (request.ClientId is not null) + if (!string.IsNullOrEmpty(payload.SessionId)) { lock (_sessionsLock) { - // Find session by clientId or any valid session - // For MVP, we accept any valid session - foreach (var s in _sessions.Values) + if (_sessions.TryGetValue(payload.SessionId, out var s) + && DateTime.UtcNow - s.CreatedAt < SessionTimeout) { - if (DateTime.UtcNow - s.CreatedAt < SessionTimeout) - { - session = s; - break; - } + session = s; } } } - var entry = _vaultState.GetCredentialById(payload.Id); - if (entry is null) + if (session is null) { return new IpcResponse { Action = request.Action, RequestId = request.RequestId, Success = false, - Error = "credential-not-found" + Error = "no-session" }; } - // If we have a session key, encrypt the password - if (session is not null) + var entry = _vaultState.GetCredentialById(payload.Id); + if (entry is null) { - var passwordBytes = Encoding.UTF8.GetBytes(entry.Password); - var encryptedBlob = _crypto.Encrypt(passwordBytes, session.SessionKey); - CryptographicOperations.ZeroMemory(passwordBytes); - - // Split blob into nonce + ciphertext+tag for the response - var nonce = Convert.ToBase64String(encryptedBlob.AsSpan(0, CryptoConstants.NonceSizeBytes)); - var encryptedData = Convert.ToBase64String(encryptedBlob.AsSpan(CryptoConstants.NonceSizeBytes)); - - var responsePayload = new GetCredentialPasswordResponse(encryptedData, nonce); - return new IpcResponse { Action = request.Action, RequestId = request.RequestId, - Success = true, - Payload = JsonSerializer.SerializeToElement(responsePayload, IpcJsonContext.Default.GetCredentialPasswordResponse) + Success = false, + Error = "credential-not-found" }; } - // No session — send password in plaintext (less secure, but functional) - // Browser extension should always do exchange-keys first - var plaintextPayload = new GetCredentialPasswordResponse( - Convert.ToBase64String(Encoding.UTF8.GetBytes(entry.Password)), - string.Empty); + var passwordBytes = Encoding.UTF8.GetBytes(entry.Password); + var encryptedBlob = _crypto.Encrypt(passwordBytes, session.SessionKey); + CryptographicOperations.ZeroMemory(passwordBytes); + + // Split blob into nonce + ciphertext+tag for the response + var nonce = Convert.ToBase64String(encryptedBlob.AsSpan(0, CryptoConstants.NonceSizeBytes)); + var encryptedData = Convert.ToBase64String(encryptedBlob.AsSpan(CryptoConstants.NonceSizeBytes)); + + var responsePayload = new GetCredentialPasswordResponse(encryptedData, nonce); return new IpcResponse { Action = request.Action, RequestId = request.RequestId, Success = true, - Payload = JsonSerializer.SerializeToElement(plaintextPayload, IpcJsonContext.Default.GetCredentialPasswordResponse) + Payload = JsonSerializer.SerializeToElement(responsePayload, IpcJsonContext.Default.GetCredentialPasswordResponse) }; } @@ -577,11 +601,41 @@ private async Task HandleUnlockVaultAsync(IpcRequest request) }; } + // Brute-force throttle: reject unlock attempts while a lockout window is active. + lock (_unlockLock) + { + if (DateTime.UtcNow < _unlockLockoutUntil) + { + return new IpcResponse + { + Action = request.Action, + RequestId = request.RequestId, + Success = false, + Error = "too-many-attempts" + }; + } + } + // Copy to char array so we can zero it after use var masterChars = payload.MasterPassword.ToCharArray(); try { var ok = await _vaultState.UnlockAsync(masterChars.AsMemory()); + + lock (_unlockLock) + { + if (ok) + { + _unlockFailedCount = 0; + _unlockLockoutUntil = DateTime.MinValue; + } + else if (++_unlockFailedCount >= MaxUnlockFailures) + { + _unlockLockoutUntil = DateTime.UtcNow + UnlockLockout; + _unlockFailedCount = 0; // reset the counter; the lockout window now applies + } + } + return new IpcResponse { Action = request.Action, @@ -732,18 +786,27 @@ private void CleanupExpiredSessions() { lock (_sessionsLock) { - var expiredKeys = _sessions - .Where(kv => DateTime.UtcNow - kv.Value.CreatedAt >= SessionTimeout) - .Select(kv => kv.Key) - .ToList(); + PruneExpiredSessionsNoLock(); + } + } + + /// + /// Removes and zeroes all expired sessions. The caller MUST already hold + /// . + /// + private void PruneExpiredSessionsNoLock() + { + var expiredKeys = _sessions + .Where(kv => DateTime.UtcNow - kv.Value.CreatedAt >= SessionTimeout) + .Select(kv => kv.Key) + .ToList(); - foreach (var key in expiredKeys) + foreach (var key in expiredKeys) + { + if (_sessions.TryGetValue(key, out var session)) { - if (_sessions.TryGetValue(key, out var session)) - { - CryptographicOperations.ZeroMemory(session.SessionKey); - _sessions.Remove(key); - } + CryptographicOperations.ZeroMemory(session.SessionKey); + _sessions.Remove(key); } } } diff --git a/src/PassKey.Desktop/Services/ClipboardService.cs b/src/PassKey.Desktop/Services/ClipboardService.cs index 7104158..3e99ea9 100644 --- a/src/PassKey.Desktop/Services/ClipboardService.cs +++ b/src/PassKey.Desktop/Services/ClipboardService.cs @@ -65,11 +65,18 @@ public void Copy(string content, CopyType type) Clipboard.SetContent(dataPackage); } - // Flush() rende i dati persistenti dopo chiusura app. - // Può fallire con COMException 0x800401D0 in app self-contained. - // Non critico: SetContent() è sufficiente per l'uso normale. - try { Clipboard.Flush(); } - catch (System.Runtime.InteropServices.COMException) { /* clipboard still works without Flush */ } + // Flush() rende i dati persistenti sulla clipboard anche DOPO la chiusura dell'app. + // Per i contenuti sensibili (password, CVV) questo è indesiderato: se l'app viene + // chiusa entro i 30s il timer di auto-clear muore con il processo e il segreto + // resterebbe nella clipboard a tempo indeterminato. Quindi facciamo Flush SOLO per + // i contenuti non sensibili; per quelli sensibili lasciamo che il dato venga rimosso + // dalla OS alla chiusura dell'app (oltre all'auto-clear a 30s). + // Può fallire con COMException 0x800401D0 in app self-contained → non critico. + if (type != CopyType.Sensitive) + { + try { Clipboard.Flush(); } + catch (System.Runtime.InteropServices.COMException) { /* clipboard still works without Flush */ } + } // Store hash of copied content for verification before clearing _copiedHash = SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(content)); diff --git a/src/PassKey.Desktop/Services/IpcModels.cs b/src/PassKey.Desktop/Services/IpcModels.cs index feeb25d..f0ae838 100644 --- a/src/PassKey.Desktop/Services/IpcModels.cs +++ b/src/PassKey.Desktop/Services/IpcModels.cs @@ -93,8 +93,13 @@ internal sealed record GetCredentialsResponse(List Credential // --- get-credential-password --- -/// Request payload for get-credential-password: the GUID of the entry whose password to retrieve. -internal sealed record GetCredentialPasswordRequest(Guid Id); +/// +/// Request payload for get-credential-password: the GUID of the entry whose password +/// to retrieve, plus the ECDH obtained from exchange-keys. +/// The session ID binds the response encryption to the exact session that established the +/// shared key; requests without a valid session are rejected (no plaintext fallback). +/// +internal sealed record GetCredentialPasswordRequest(Guid Id, string? SessionId = null); /// /// Response payload for get-credential-password. From a4ea2f479825b1abd80a41b2352c5520f4d7d7be Mon Sep 17 00:00:00 2001 From: Giuseppe Imperato Date: Mon, 8 Jun 2026 15:16:17 +0200 Subject: [PATCH 2/2] chore(ext): bump extension version to 1.0.2 (coordinated IPC protocol change) The get-credential-password wire protocol now requires sessionId and drops the plaintext fallback; extensions and Desktop must ship together. Bump both Chrome and Firefox manifests so the updated build is published to the stores. Co-Authored-By: Claude Opus 4.8 --- extensions/chrome/manifest.json | 2 +- extensions/firefox/manifest.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/chrome/manifest.json b/extensions/chrome/manifest.json index 548ce20..8dd0d4f 100644 --- a/extensions/chrome/manifest.json +++ b/extensions/chrome/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "PassKey", - "version": "1.0.1", + "version": "1.0.2", "author": "Giuseppe Imperato", "homepage_url": "https://github.com/pexatar/PassKey", "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAjdNrt0WikUv9qMTjwXhpg5jIYvCLS0WYu8lRlUCzcvdB/aEuHOEqaQ4ByEES+eISVoOo4Vmdr4u3Zo1+IyPwE1zGqwX1agNMsZbctr3Ur8esnHp/gMaLkrONFHDrOXhaVdUJy5BDqa5yXuTN5jsZWhk3cmqxpxlfWu70piFWN4bsPPwdKNXuDYPSBQCcdUFlUxUz3GXX8zkm1GsXQONg1vMSYE9E5GHD4ORk5oSWlRDccsTHtnWVlfaNzlBLmuiXTbmrCBRO2zfKU+8T3byMZTWGf/0ygD7+XuNbHvMAxQgFFwdAeP9RX/kXnBej2KwXxlVE65rd/kSwfa5hFLcubQIDAQAB", diff --git a/extensions/firefox/manifest.json b/extensions/firefox/manifest.json index 440aef4..89a1953 100644 --- a/extensions/firefox/manifest.json +++ b/extensions/firefox/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "PassKey", - "version": "1.0.1", + "version": "1.0.2", "author": "Giuseppe Imperato", "homepage_url": "https://github.com/pexatar/PassKey", "description": "Local password manager integration for PassKey desktop app. Autofill credentials, credit cards and identities stored securely on your PC — no cloud, no subscription.",