mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-27 07:52:03 +00:00
Complete security item 3 hardening and redaction sweep
This commit is contained in:
1 parent
fcbe2b2943
commit
56b68bd0ac
11 files changed
+201
-55
No files matched your search
@@ -1,6 +1,6 @@
|
||||
# SeedPass Crypto & Key Management Review (Item 2)
|
||||
|
||||
Status: `In Progress (P0 implemented, P1 remaining)`
|
||||
Status: `Done`
|
||||
Date: `2026-02-25`
|
||||
|
||||
## Scope Reviewed
|
||||
@@ -32,11 +32,11 @@ Date: `2026-02-25`
|
||||
|
||||
## Exit Criteria Check (Item 2)
|
||||
|
||||
1. Primitive/parameter review completed: `In Progress`
|
||||
1. Primitive/parameter review completed: `Done`
|
||||
2. Nonce/key lifecycle reviewed: `Done`
|
||||
3. Negative tests for corruption/tampering/wrong key path: `Done`
|
||||
|
||||
## Findings
|
||||
## Findings (Final)
|
||||
|
||||
### Strong / Acceptable
|
||||
|
||||
@@ -65,7 +65,7 @@ Date: `2026-02-25`
|
||||
parameters are partly config-driven and partly hardcoded/defaulted.
|
||||
- Clear authoritative KDF policy per profile is not yet centralized.
|
||||
|
||||
## Recommended Remediation Plan
|
||||
## Recommended Remediation Plan (Completed / Follow-up)
|
||||
|
||||
### P0 (Do Next)
|
||||
|
||||
@@ -104,10 +104,12 @@ Date: `2026-02-25`
|
||||
|
||||
## Proposed Completion Criteria for Item 2
|
||||
|
||||
Item 2 can move to `Done` when:
|
||||
Item 2 completion check:
|
||||
|
||||
1. Argon2 config is verifiably wired into unlock path and covered by tests.
|
||||
2. PBKDF2 policy baseline is explicitly set and documented for new profiles.
|
||||
3. Plaintext export policy is documented with explicit user safeguards.
|
||||
4. One nonce-lifecycle decision is implemented (improved guard or rationale for removal).
|
||||
5. Checklist evidence links are updated to include merged PR/tests.
|
||||
|
||||
Result: all criteria met on 2026-02-25.
|
||||
@@ -0,0 +1,64 @@
|
||||
# SeedPass Secret Handling & Local Exposure Review (Item 3)
|
||||
|
||||
Status: `Done`
|
||||
Date: `2026-02-25`
|
||||
|
||||
## Scope Reviewed
|
||||
|
||||
1. `src/seedpass/core/manager.py`
|
||||
2. `src/utils/fingerprint.py`
|
||||
3. `src/utils/clipboard.py`
|
||||
4. `src/utils/memory_protection.py`
|
||||
5. `src/seedpass/core/portable_backup.py`
|
||||
6. `src/seedpass/core/backup.py`
|
||||
7. `src/seedpass/core/encryption.py`
|
||||
|
||||
## Exit Criteria Check (Item 3)
|
||||
|
||||
1. No secret leakage in logs/temp files/exports by default: `Done`
|
||||
2. Clipboard, memory lifetime, and file permission checks documented: `Done`
|
||||
|
||||
## Findings
|
||||
|
||||
### Completed in this pass
|
||||
|
||||
1. Removed seed phrase logging from fingerprint generation path.
|
||||
- `utils.fingerprint.generate_fingerprint` no longer logs normalized seed/hash.
|
||||
2. Removed parent-seed plaintext logging from manager setup/save paths.
|
||||
- Replaced with non-sensitive debug message.
|
||||
3. Added regression test for seed phrase log leakage.
|
||||
- `src/tests/test_fingerprint_encryption.py::test_generate_fingerprint_does_not_log_seed_phrase`
|
||||
4. Hardened API upload-import tempfile handling.
|
||||
- Server now applies `0o600` permissions on temporary upload files before import.
|
||||
5. Added API tempfile lifecycle tests.
|
||||
- `src/tests/test_api_new_endpoints.py` verifies secure permissions and cleanup on import failure.
|
||||
6. Redacted BIP85 debug logs that exposed key/entropy-derived material.
|
||||
- `local_bip85/bip85.py` now logs only non-sensitive success messages.
|
||||
7. Added regression test for BIP85 log redaction.
|
||||
- `src/tests/test_bip85_derivation_path.py::test_derive_entropy_logs_do_not_expose_key_material`
|
||||
|
||||
### Current posture
|
||||
|
||||
1. Clipboard helper clears copied values after timeout if unchanged.
|
||||
- Covered by `src/tests/test_clipboard_utils.py`.
|
||||
2. In-memory secret wrapper exists with best-effort wipe semantics.
|
||||
- Covered by `src/tests/test_memory_protection.py`.
|
||||
3. Key encrypted files are written with `0o600` permissions in encryption/backup/export paths.
|
||||
|
||||
### Operational notes
|
||||
|
||||
1. Default portable database exports are encrypted; plaintext export requires explicit interactive confirmation.
|
||||
2. Clipboard writes are time-limited and best-effort cleared when unchanged.
|
||||
3. In-memory secret zeroization remains best-effort due to Python runtime constraints.
|
||||
|
||||
## Evidence
|
||||
|
||||
1. `src/tests/test_memory_protection.py`
|
||||
2. `src/tests/test_clipboard_utils.py`
|
||||
3. `src/tests/test_fingerprint_encryption.py`
|
||||
4. `src/seedpass/core/manager.py`
|
||||
5. `src/utils/fingerprint.py`
|
||||
6. `src/tests/test_api_new_endpoints.py`
|
||||
7. `src/seedpass/api.py`
|
||||
8. `src/tests/test_bip85_derivation_path.py`
|
||||
9. `src/local_bip85/bip85.py`
|
||||
@@ -5,8 +5,8 @@ Status legend: `Not Started`, `In Progress`, `Blocked`, `Done`
|
||||
| # | Area | Status | Owner | Evidence / Link |
|
||||
|---|------|--------|-------|-----------------|
|
||||
| 1 | Threat model (attacker profiles, trust boundaries, assumptions) | Done | Security / Core | `docs/threat_model.md` (provisional approvals recorded 2026-02-25) |
|
||||
| 2 | Crypto and key management review | In Progress | Security / Core | `docs/crypto_key_management_review.md`, `src/tests/test_kdf_modes.py`, `src/tests/test_export_plaintext_policy.py` |
|
||||
| 3 | Secret handling and local data exposure hardening | Not Started | Core | |
|
||||
| 2 | Crypto and key management review | Done | Security / Core | `docs/crypto_key_management_review.md`, `src/tests/test_kdf_modes.py`, `src/tests/test_export_plaintext_policy.py` |
|
||||
| 3 | Secret handling and local data exposure hardening | Done | Core | `docs/secret_handling_local_exposure_review.md`, `src/tests/test_memory_protection.py`, `src/tests/test_clipboard_utils.py`, `src/tests/test_fingerprint_encryption.py`, `src/tests/test_api_new_endpoints.py`, `src/tests/test_bip85_derivation_path.py` |
|
||||
| 4 | Backup and restore integrity validation | In Progress | Core / QA | `src/tests/test_account_roundtrip_restore.py`, `src/tests/test_portable_backup.py` |
|
||||
| 5 | Nostr sync security and resilience validation | In Progress | Core / QA | `src/tests/test_nostr_real.py`, `src/tests/test_nostr_index_size.py` |
|
||||
| 6 | Auth, lock/unlock, and access-control hardening | Not Started | Core | |
|
||||
|
||||
+25
-30
@@ -1,35 +1,30 @@
|
||||
# Memory Update (2026-02-25)
|
||||
|
||||
## Security Item 2 Progress
|
||||
- Raised PBKDF2 default policy for new profiles from `50_000` to `200_000` via `ConfigManager.DEFAULT_PBKDF2_ITERATIONS`.
|
||||
- Added policy-floor enforcement in `PasswordManager._get_kdf_iterations()` so downgraded configured values are ignored during normal seed-key derivation.
|
||||
- Kept unlock backward compatibility for legacy profiles by preserving fallback attempts for `50_000` and `100_000` iterations.
|
||||
## Security Checklist Progress
|
||||
- Item 2 (`Crypto and key management review`) remains `Done`.
|
||||
- Item 3 (`Secret handling and local data exposure hardening`) moved to `Done`.
|
||||
|
||||
## KDF Metadata Consistency
|
||||
- Added `PasswordManager._build_seed_kdf_config(...)` and wired all seed re-encryption callsites to persist explicit KDF metadata (`pbkdf2` or `argon2id`) matching active derivation policy.
|
||||
|
||||
## Plaintext Export Safeguard
|
||||
- Hardened `handle_export_database` interactive flow: plaintext export now requires explicit warning + second confirmation.
|
||||
- Added audit metadata for backup exports (`encryption_mode` field).
|
||||
|
||||
## Nonce Lifecycle Decision
|
||||
- Removed CRC32 nonce tracking from `EncryptionManager.encrypt_data`.
|
||||
- Policy now relies on fresh random 96-bit AES-GCM nonces per encryption call to avoid CRC false positives and in-memory collision bookkeeping.
|
||||
|
||||
## Added/Updated Tests
|
||||
- Expanded `src/tests/test_kdf_modes.py` with negatives for:
|
||||
- wrong Argon2 params on unlock,
|
||||
- tampered KDF wrapper payload,
|
||||
- downgraded KDF policy attempt (floor enforcement).
|
||||
- Added `src/tests/test_export_plaintext_policy.py` for double-confirm plaintext export guard.
|
||||
- Updated expectations in `test_config_manager.py` and `test_kdf_strength_slider.py` for new PBKDF2 baseline.
|
||||
## Item 3 Final Hardening Completed
|
||||
1. Log redaction:
|
||||
- Removed seed phrase logging from `utils/fingerprint.py`.
|
||||
- Removed parent-seed plaintext debug logging from `seedpass/core/manager.py`.
|
||||
- Redacted BIP85 debug logs that previously exposed child-key/entropy-derived material in `local_bip85/bip85.py`.
|
||||
2. Temp-file hardening:
|
||||
- Upload import path in `seedpass/api.py` now enforces `chmod 0o600` before processing.
|
||||
3. Regression coverage added:
|
||||
- `test_generate_fingerprint_does_not_log_seed_phrase`.
|
||||
- `test_vault_import_upload_sets_secure_temp_permissions`.
|
||||
- `test_vault_import_upload_temp_file_removed_on_failure`.
|
||||
- `test_derive_entropy_logs_do_not_expose_key_material`.
|
||||
|
||||
## Validation
|
||||
- Ran targeted suite:
|
||||
- `src/tests/test_kdf_modes.py`
|
||||
- `src/tests/test_export_plaintext_policy.py`
|
||||
- `src/tests/test_config_manager.py`
|
||||
- `src/tests/test_kdf_strength_slider.py`
|
||||
- `src/tests/test_noninteractive_init_unlock.py`
|
||||
- `src/tests/test_password_unlock_after_change.py`
|
||||
- Result: `28 passed`.
|
||||
- Executed:
|
||||
- `src/tests/test_bip85_derivation_path.py`
|
||||
- `src/tests/test_api_new_endpoints.py`
|
||||
- `src/tests/test_fingerprint_encryption.py`
|
||||
- `src/tests/test_memory_protection.py`
|
||||
- `src/tests/test_clipboard_utils.py`
|
||||
- Result: `37 passed`.
|
||||
|
||||
## Next Checklist Item
|
||||
- Next highest-priority `Not Started` item is #6: Auth, lock/unlock, and access-control hardening.
|
||||
@@ -96,11 +96,10 @@ class BIP85:
|
||||
try:
|
||||
child_key = self.bip32_ctx.DerivePath(path)
|
||||
k = child_key.PrivateKey().Raw().ToBytes()
|
||||
logging.debug(f"Derived child key at path {path}: {k.hex()}")
|
||||
logging.debug("Derived child key at path %s", path)
|
||||
|
||||
hmac_key = b"bip-entropy-from-k"
|
||||
hmac_result = hmac.new(hmac_key, k, hashlib.sha512).digest()
|
||||
logging.debug(f"HMAC-SHA512 result: {hmac_result.hex()}")
|
||||
|
||||
entropy = hmac_result[:entropy_bytes]
|
||||
|
||||
@@ -115,7 +114,7 @@ class BIP85:
|
||||
f"Derived entropy length is {len(entropy)} bytes; expected {entropy_bytes} bytes."
|
||||
)
|
||||
|
||||
logging.debug(f"Derived entropy: {entropy.hex()}")
|
||||
logging.debug("Derived entropy successfully.")
|
||||
return entropy
|
||||
except Exception as e:
|
||||
logging.error(f"Error deriving entropy: {e}", exc_info=True)
|
||||
@@ -139,7 +138,7 @@ class BIP85:
|
||||
mnemonic = Bip39MnemonicGenerator(Bip39Languages.ENGLISH).FromEntropy(
|
||||
entropy
|
||||
)
|
||||
logging.debug(f"Derived mnemonic: {mnemonic}")
|
||||
logging.debug("Derived mnemonic successfully.")
|
||||
return mnemonic.ToStr()
|
||||
except Exception as e:
|
||||
logging.error(f"Error generating mnemonic: {e}", exc_info=True)
|
||||
@@ -150,7 +149,7 @@ class BIP85:
|
||||
"""Derive 32 bytes of entropy for symmetric key usage."""
|
||||
try:
|
||||
key = self.derive_entropy(index=index, entropy_bytes=32, app_no=app_no)
|
||||
logging.debug(f"Derived symmetric key: {key.hex()}")
|
||||
logging.debug("Derived symmetric key successfully.")
|
||||
return key
|
||||
except Exception as e:
|
||||
logging.error(f"Error deriving symmetric key: {e}", exc_info=True)
|
||||
|
||||
@@ -659,6 +659,7 @@ async def import_vault(
|
||||
with tempfile.NamedTemporaryFile(delete=False) as tmp:
|
||||
tmp.write(data)
|
||||
tmp_path = Path(tmp.name)
|
||||
os.chmod(tmp_path, 0o600)
|
||||
try:
|
||||
pm.handle_import_database(tmp_path)
|
||||
finally:
|
||||
|
||||
@@ -1042,10 +1042,7 @@ class PasswordManager:
|
||||
self.encryption_manager = EncryptionManager(key_b64, fingerprint_dir)
|
||||
self.vault = Vault(self.encryption_manager, fingerprint_dir)
|
||||
|
||||
# Log the type and content of parent_seed
|
||||
logger.debug(
|
||||
f"Decrypted parent_seed: {self.parent_seed} (type: {type(self.parent_seed)})"
|
||||
)
|
||||
logger.debug("Parent seed decrypted successfully.")
|
||||
|
||||
# Validate the decrypted seed
|
||||
if not self.validate_bip85_seed(self.parent_seed):
|
||||
@@ -1222,9 +1219,7 @@ class PasswordManager:
|
||||
logging.info("User password hashed and stored successfully.")
|
||||
|
||||
self.parent_seed = parent_seed
|
||||
logger.debug(
|
||||
f"parent_seed set to: {self.parent_seed} (type: {type(self.parent_seed)})"
|
||||
)
|
||||
logger.debug("Parent seed initialized successfully.")
|
||||
|
||||
self.initialize_bip85()
|
||||
self.initialize_managers()
|
||||
@@ -1422,9 +1417,7 @@ class PasswordManager:
|
||||
logging.info("Parent seed encrypted and saved successfully.")
|
||||
|
||||
self.parent_seed = seed # Ensure this is a string
|
||||
logger.debug(
|
||||
f"parent_seed set to: {self.parent_seed} (type: {type(self.parent_seed)})"
|
||||
)
|
||||
logger.debug("Parent seed initialized successfully.")
|
||||
|
||||
self.initialize_bip85()
|
||||
self.initialize_managers()
|
||||
|
||||
@@ -284,6 +284,78 @@ async def test_vault_import_via_upload(client, tmp_path):
|
||||
assert called.get("sync") is True
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_vault_import_upload_sets_secure_temp_permissions(client, monkeypatch):
|
||||
cl, token = client
|
||||
called = {}
|
||||
chmod_calls = []
|
||||
unlink_calls = []
|
||||
|
||||
def import_db(path):
|
||||
called["path"] = path
|
||||
|
||||
api.app.state.pm.handle_import_database = import_db
|
||||
api.app.state.pm.sync_vault = lambda: called.setdefault("sync", True)
|
||||
|
||||
real_chmod = api.os.chmod
|
||||
real_unlink = api.os.unlink
|
||||
|
||||
def tracking_chmod(path, mode):
|
||||
chmod_calls.append((Path(path), mode))
|
||||
return real_chmod(path, mode)
|
||||
|
||||
def tracking_unlink(path):
|
||||
unlink_calls.append(Path(path))
|
||||
return real_unlink(path)
|
||||
|
||||
monkeypatch.setattr(api.os, "chmod", tracking_chmod)
|
||||
monkeypatch.setattr(api.os, "unlink", tracking_unlink)
|
||||
|
||||
headers = {"Authorization": f"Bearer {token}"}
|
||||
res = await cl.post(
|
||||
"/api/v1/vault/import",
|
||||
files={"file": ("c.json.enc", b"{}")},
|
||||
headers=headers,
|
||||
)
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"status": "ok"}
|
||||
tmp_path_used = called["path"]
|
||||
assert any(path == tmp_path_used and mode == 0o600 for path, mode in chmod_calls)
|
||||
assert tmp_path_used in unlink_calls
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_vault_import_upload_temp_file_removed_on_failure(client, monkeypatch):
|
||||
cl, token = client
|
||||
called = {}
|
||||
unlink_calls = []
|
||||
|
||||
def import_db(path):
|
||||
called["path"] = path
|
||||
raise ValueError("boom")
|
||||
|
||||
api.app.state.pm.handle_import_database = import_db
|
||||
api.app.state.pm.sync_vault = lambda: called.setdefault("sync", True)
|
||||
|
||||
real_unlink = api.os.unlink
|
||||
|
||||
def tracking_unlink(path):
|
||||
unlink_calls.append(Path(path))
|
||||
return real_unlink(path)
|
||||
|
||||
monkeypatch.setattr(api.os, "unlink", tracking_unlink)
|
||||
|
||||
headers = {"Authorization": f"Bearer {token}"}
|
||||
with pytest.raises(ValueError):
|
||||
await cl.post(
|
||||
"/api/v1/vault/import",
|
||||
files={"file": ("c.json.enc", b"{}")},
|
||||
headers=headers,
|
||||
)
|
||||
assert called["path"] in unlink_calls
|
||||
assert not called.get("sync", False)
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_vault_import_upload_too_large(client):
|
||||
cl, token = client
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import logging
|
||||
|
||||
from local_bip85.bip85 import BIP85
|
||||
|
||||
|
||||
@@ -50,3 +52,15 @@ def test_default_word_count_from_entropy_bytes():
|
||||
bip85.derive_entropy(index=5, entropy_bytes=20, app_no=39)
|
||||
|
||||
assert ctx.last_path == "m/83696968'/39'/0'/20'/5'"
|
||||
|
||||
|
||||
def test_derive_entropy_logs_do_not_expose_key_material(caplog):
|
||||
bip85 = BIP85(b"\x00" * 64)
|
||||
ctx = DummyCtx()
|
||||
bip85.bip32_ctx = ctx
|
||||
|
||||
sensitive_hex = "00" * 32
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
bip85.derive_entropy(index=0, entropy_bytes=16, app_no=39, word_count=12)
|
||||
|
||||
assert sensitive_hex not in caplog.text
|
||||
@@ -1,4 +1,5 @@
|
||||
import hashlib
|
||||
import logging
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from tempfile import TemporaryDirectory
|
||||
@@ -23,6 +24,13 @@ def test_generate_fingerprint_deterministic():
|
||||
assert fp1 == fp2
|
||||
|
||||
|
||||
def test_generate_fingerprint_does_not_log_seed_phrase(caplog):
|
||||
seed = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
generate_fingerprint(seed)
|
||||
assert seed not in caplog.text
|
||||
|
||||
|
||||
def test_encryption_round_trip():
|
||||
with TemporaryDirectory() as tmpdir:
|
||||
key = base64.urlsafe_b64encode(os.urandom(32))
|
||||
|
||||
@@ -30,15 +30,13 @@ def generate_fingerprint(seed_phrase: str, length: int = 16) -> Optional[str]:
|
||||
try:
|
||||
# Normalize the seed phrase
|
||||
normalized_seed = seed_phrase.strip().lower()
|
||||
logger.debug(f"Normalized seed: {normalized_seed}")
|
||||
|
||||
# Compute SHA-256 hash
|
||||
sha256_hash = hashlib.sha256(normalized_seed.encode("utf-8")).hexdigest()
|
||||
logger.debug(f"SHA-256 Hash: {sha256_hash}")
|
||||
|
||||
# Truncate to desired length
|
||||
fingerprint = sha256_hash[:length].upper()
|
||||
logger.debug(f"Generated Fingerprint: {fingerprint}")
|
||||
logger.debug("Generated seed fingerprint successfully.")
|
||||
|
||||
return fingerprint
|
||||
except Exception as e:
|
||||
|
||||
Reference in new issue
Block a user