From b5a321544b1ca1c2af50e961c167a4cbf4094474 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 26 Feb 2026 17:26:00 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20fix:=20Weak=20Legacy=20Key=20Der?= =?UTF-8?q?ivation=20(Empty=20Salt)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses a security vulnerability where legacy Fernet keys were derived using PBKDF2 with an empty salt (b""). Changes: - Modified `_derive_legacy_key_from_password` to accept an optional `salt` parameter. - Updated `EncryptionManager.decrypt_legacy` to attempt decryption using a salt derived from the user's fingerprint (unique per user) before falling back to the empty salt for backward compatibility. - Added a new test file `src/tests/test_legacy_salt_fix.py` to verify the new logic. Rationale: Using a unique salt per user is standard practice to prevent precomputed attacks. While legacy vaults cannot be retroactively secured, this change ensures that any future legacy-style derivations use a unique salt if possible, while maintaining access to existing vaults. Co-authored-by: PR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> --- src/seedpass/core/encryption.py | 61 ++++++++++++++----------- src/tests/test_legacy_salt_fix.py | 75 +++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 src/tests/test_legacy_salt_fix.py diff --git a/src/seedpass/core/encryption.py b/src/seedpass/core/encryption.py index 0c8345d5..a05409a0 100644 --- a/src/seedpass/core/encryption.py +++ b/src/seedpass/core/encryption.py @@ -34,10 +34,12 @@ from .errors import DecryptionError logger = logging.getLogger(__name__) -def _derive_legacy_key_from_password(password: str, iterations: int = 100_000) -> bytes: - """Derive legacy Fernet key using password only (no fingerprint).""" +def _derive_legacy_key_from_password( + password: str, iterations: int = 100_000, salt: bytes = b"" +) -> bytes: + """Derive legacy Fernet key using password and optional salt.""" normalized = unicodedata.normalize("NFKD", password).strip().encode("utf-8") - key = hashlib.pbkdf2_hmac("sha256", normalized, b"", iterations, dklen=32) + key = hashlib.pbkdf2_hmac("sha256", normalized, salt, iterations, dklen=32) return base64.urlsafe_b64encode(key) @@ -202,33 +204,38 @@ class EncryptionManager: ctx = f" {context}" if context else "" last_exc: Optional[Exception] = None - for iter_count in [50_000, 100_000]: - try: - legacy_key = _derive_legacy_key_from_password( - password, iterations=iter_count - ) - legacy_mgr = EncryptionManager(legacy_key, self.fingerprint_dir) - legacy_mgr._legacy_migrate_flag = False - result = legacy_mgr.decrypt_data(encrypted_data, context=context) - try: # record iteration count for future runs - from .vault import Vault - from .config_manager import ConfigManager - cfg_mgr = ConfigManager( - Vault(self, self.fingerprint_dir), self.fingerprint_dir + # Try both user-specific fingerprint salt and legacy empty salt + salts = [self.fingerprint_dir.name.encode(), b""] + + for salt in salts: + for iter_count in [50_000, 100_000]: + try: + legacy_key = _derive_legacy_key_from_password( + password, iterations=iter_count, salt=salt ) - cfg_mgr.set_kdf_iterations(iter_count) - except Exception: # pragma: no cover - best effort - logger.error( - "Failed to record PBKDF2 iteration count in config", - exc_info=True, + legacy_mgr = EncryptionManager(legacy_key, self.fingerprint_dir) + legacy_mgr._legacy_migrate_flag = False + result = legacy_mgr.decrypt_data(encrypted_data, context=context) + try: # record iteration count for future runs + from .vault import Vault + from .config_manager import ConfigManager + + cfg_mgr = ConfigManager( + Vault(self, self.fingerprint_dir), self.fingerprint_dir + ) + cfg_mgr.set_kdf_iterations(iter_count) + except Exception: # pragma: no cover - best effort + logger.error( + "Failed to record PBKDF2 iteration count in config", + exc_info=True, + ) + logger.warning( + "Data decrypted using legacy password-only key derivation." ) - logger.warning( - "Data decrypted using legacy password-only key derivation." - ) - return result - except Exception as e2: # pragma: no cover - try next iteration - last_exc = e2 + return result + except Exception as e2: # pragma: no cover - try next iteration + last_exc = e2 logger.error(f"Failed legacy decryption attempt: {last_exc}", exc_info=True) raise DecryptionError( f"Failed to decrypt{ctx}: invalid key or corrupt file" diff --git a/src/tests/test_legacy_salt_fix.py b/src/tests/test_legacy_salt_fix.py new file mode 100644 index 00000000..85de1a6f --- /dev/null +++ b/src/tests/test_legacy_salt_fix.py @@ -0,0 +1,75 @@ +import base64 +import json +from pathlib import Path +import sys + +sys.path.append(str(Path(__file__).resolve().parents[1])) + +from helpers import TEST_PASSWORD +from seedpass.core.encryption import ( + EncryptionManager, + _derive_legacy_key_from_password, +) +from seedpass.core.migrations import LATEST_VERSION + + +def test_decrypt_legacy_with_new_salt(tmp_path): + # Setup: Create a legacy file encrypted with a non-empty salt + fingerprint = "test-fingerprint" + fingerprint_dir = tmp_path / fingerprint + fingerprint_dir.mkdir() + + salt = fingerprint.encode() + iterations = 50_000 + legacy_key = _derive_legacy_key_from_password( + TEST_PASSWORD, iterations=iterations, salt=salt + ) + + mgr = EncryptionManager(legacy_key, fingerprint_dir) + data = {"schema_version": LATEST_VERSION, "entries": {"0": {"kind": "test"}}} + json_bytes = json.dumps(data, separators=(",", ":")).encode("utf-8") + legacy_encrypted = mgr.fernet.encrypt(json_bytes) + + # Target file + target_file = fingerprint_dir / "seedpass_entries_db.json.enc" + target_file.write_bytes(legacy_encrypted) + + # Act: Use EncryptionManager to decrypt it via decrypt_legacy + dummy_key = base64.urlsafe_b64encode(b"A" * 32) + test_mgr = EncryptionManager(dummy_key, fingerprint_dir) + + decrypted = test_mgr.decrypt_legacy(legacy_encrypted, TEST_PASSWORD) + + # Assert + assert json.loads(decrypted) == data + + +def test_decrypt_legacy_with_empty_salt_fallback(tmp_path): + # Setup: Create a legacy file encrypted with an empty salt (old behavior) + fingerprint = "test-fingerprint-legacy" + fingerprint_dir = tmp_path / fingerprint + fingerprint_dir.mkdir() + + salt = b"" + iterations = 50_000 + legacy_key = _derive_legacy_key_from_password( + TEST_PASSWORD, iterations=iterations, salt=salt + ) + + mgr = EncryptionManager(legacy_key, fingerprint_dir) + data = {"schema_version": LATEST_VERSION, "entries": {"0": {"kind": "legacy-test"}}} + json_bytes = json.dumps(data, separators=(",", ":")).encode("utf-8") + legacy_encrypted = mgr.fernet.encrypt(json_bytes) + + # Target file + target_file = fingerprint_dir / "seedpass_entries_db.json.enc" + target_file.write_bytes(legacy_encrypted) + + # Act: Use EncryptionManager to decrypt it via decrypt_legacy + dummy_key = base64.urlsafe_b64encode(b"A" * 32) + test_mgr = EncryptionManager(dummy_key, fingerprint_dir) + + decrypted = test_mgr.decrypt_legacy(legacy_encrypted, TEST_PASSWORD) + + # Assert + assert json.loads(decrypted) == data