🔒 fix: Weak Legacy Key Derivation (Empty Salt)

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 <[email protected]>

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:
+def _derive_legacy_key_from_password(
-    """Derive legacy Fernet key using password only (no fingerprint)."""
+    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(
+        # Try both user-specific fingerprint salt and legacy empty salt
-                        Vault(self, self.fingerprint_dir), self.fingerprint_dir
+        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)
+                    legacy_mgr = EncryptionManager(legacy_key, self.fingerprint_dir)
-                except Exception:  # pragma: no cover - best effort
+                    legacy_mgr._legacy_migrate_flag = False
-                    logger.error(
+                    result = legacy_mgr.decrypt_data(encrypted_data, context=context)
-                        "Failed to record PBKDF2 iteration count in config",
+                    try:  # record iteration count for future runs
-                        exc_info=True,
+                        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(
+                    return result
-                    "Data decrypted using legacy password-only key derivation."
+                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"

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