From dd6cbea7de4052a1933ed2d62a9125545773e18d Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Wed, 25 Feb 2026 18:26:43 -0500 Subject: [PATCH] Start security item 6: API lock access control and unlock route --- docs/auth_lock_unlock_access_review.md | 52 ++++++++++++++++++++++++++ docs/security_readiness_checklist.md | 2 +- memory-update.md | 41 ++++++++++---------- src/seedpass/api.py | 35 +++++++++++++++++ src/tests/test_api_new_endpoints.py | 47 +++++++++++++++++++++++ 5 files changed, 155 insertions(+), 22 deletions(-) create mode 100644 docs/auth_lock_unlock_access_review.md diff --git a/docs/auth_lock_unlock_access_review.md b/docs/auth_lock_unlock_access_review.md new file mode 100644 index 00000000..b68c7d34 --- /dev/null +++ b/docs/auth_lock_unlock_access_review.md @@ -0,0 +1,52 @@ +# SeedPass Auth, Lock/Unlock & Access-Control Review (Item 6) + +Status: `In Progress` +Date: `2026-02-25` + +## Scope Reviewed + +1. `src/seedpass/core/manager.py` +2. `src/seedpass/api.py` +3. `src/seedpass/core/api.py` +4. `src/seedpass/core/config_manager.py` + +## Exit Criteria Check (Item 6) + +1. Lock/unlock/timeout/quick unlock behavior reviewed with tests: `In Progress` + +## Findings + +### Completed in this pass + +1. Added explicit locked-state gate for protected HTTP API routes. + - Sensitive endpoints now reject access with `423 Vault is locked` when lock flags are set. +2. Added explicit HTTP unlock endpoint. + - New route: `POST /api/v1/vault/unlock` using `X-SeedPass-Password`. +3. Added API regression tests for lock enforcement and unlock flow. + - Entry/password endpoints blocked while locked. + - Unlock endpoint clears locked state path. + +### Validation + +1. `pytest -q src/tests/test_api_new_endpoints.py src/tests/test_api.py src/tests/test_vault_lock_flag.py src/tests/test_inactivity_lock.py src/tests/test_unlock_sync.py` +2. Result: `62 passed` + +### Existing posture + +1. Core manager has `lock_vault()` / `unlock_vault()` and publishes lock events. +2. Inactivity timeout enforcement exists via `AuthGuard`. +3. Quick unlock behavior is configurable and covered by existing tests. + +### Remaining follow-up + +1. Decide whether non-sensitive API routes (config/fingerprint/relays) should also require unlocked state. +2. Add API-level test coverage for timeout-triggered lock then unlock flow. +3. Evaluate rate-limit tuning and brute-force controls for repeated password attempts on unlock endpoints. + +## Evidence + +1. `src/tests/test_api_new_endpoints.py` +2. `src/tests/test_vault_lock_flag.py` +3. `src/tests/test_inactivity_lock.py` +4. `src/seedpass/api.py` +5. `src/seedpass/core/manager.py` diff --git a/docs/security_readiness_checklist.md b/docs/security_readiness_checklist.md index 5af7ab28..54fc5895 100644 --- a/docs/security_readiness_checklist.md +++ b/docs/security_readiness_checklist.md @@ -9,7 +9,7 @@ Status legend: `Not Started`, `In Progress`, `Blocked`, `Done` | 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 | | +| 6 | Auth, lock/unlock, and access-control hardening | In Progress | Core | `docs/auth_lock_unlock_access_review.md`, `src/tests/test_api_new_endpoints.py`, `src/tests/test_vault_lock_flag.py`, `src/tests/test_inactivity_lock.py` | | 7 | Testing gates and quality thresholds | In Progress | QA | `scripts/check_critical_coverage.py`, CI workflows | | 8 | Supply chain and release integrity | Not Started | DevOps | | | 9 | Operational readiness and incident runbooks | Not Started | Security / Ops | | diff --git a/memory-update.md b/memory-update.md index f9e545db..cdfc181c 100644 --- a/memory-update.md +++ b/memory-update.md @@ -1,30 +1,29 @@ # Memory Update (2026-02-25) ## Security Checklist Progress -- Item 2 (`Crypto and key management review`) remains `Done`. -- Item 3 (`Secret handling and local data exposure hardening`) moved to `Done`. +- Item 6 (`Auth, lock/unlock, and access-control hardening`) moved from `Not Started` to `In Progress`. +- Added evidence links in `docs/security_readiness_checklist.md` and created a dedicated review doc. -## 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`. +## Item 6 Work Completed This Pass +1. Added locked-state access control for protected HTTP API routes in `src/seedpass/api.py`. + - Protected routes now return `423` with `"Vault is locked"` when lock flags are active. +2. Added explicit HTTP unlock route: + - `POST /api/v1/vault/unlock` using `X-SeedPass-Password`. +3. Added auth/access regression tests in `src/tests/test_api_new_endpoints.py`: + - `test_vault_unlock_endpoint` + - `test_entry_endpoints_blocked_when_vault_locked` + - `test_generate_password_blocked_when_vault_locked` ## Validation - 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`. + - `src/tests/test_api.py` + - `src/tests/test_vault_lock_flag.py` + - `src/tests/test_inactivity_lock.py` + - `src/tests/test_unlock_sync.py` +- Result: `62 passed`. -## Next Checklist Item -- Next highest-priority `Not Started` item is #6: Auth, lock/unlock, and access-control hardening. +## Next Item 6 Steps +1. Decide whether non-sensitive API routes (config/fingerprint/relays) should also require unlocked state. +2. Add timeout-to-lock to unlock route E2E test path. +3. Evaluate brute-force resistance/rate-limit behavior specific to unlock attempts. diff --git a/src/seedpass/api.py b/src/seedpass/api.py index 1b4483f4..9fb6cd52 100644 --- a/src/seedpass/api.py +++ b/src/seedpass/api.py @@ -126,6 +126,13 @@ def _require_password(request: Request, password: str | None) -> None: raise HTTPException(status_code=401, detail="Invalid password") +def _require_unlocked(request: Request) -> None: + """Ensure the active vault is unlocked before accessing protected routes.""" + pm = _get_pm(request) + if bool(getattr(pm, "is_locked", False) or getattr(pm, "locked", False)): + raise HTTPException(status_code=423, detail="Vault is locked") + + def _validate_encryption_path(request: Request, path: Path) -> Path: """Validate and normalize ``path`` within the active fingerprint directory. @@ -144,6 +151,7 @@ async def search_entry( request: Request, query: str, authorization: str | None = Header(None) ) -> List[Any]: _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) results = pm.entry_manager.search_entries(query) return [ @@ -168,6 +176,7 @@ async def get_entry( ) -> Any: _check_token(request, authorization) _require_password(request, password) + _require_unlocked(request) pm = _get_pm(request) entry = pm.entry_manager.retrieve_entry(entry_id) if entry is None: @@ -188,6 +197,7 @@ async def create_entry( ``password`` the behaviour matches the legacy password-entry API. """ _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) etype = (entry.get("type") or entry.get("kind") or "password").lower() @@ -301,6 +311,7 @@ def update_entry( specialized entry types (e.g. TOTP or key/value entries). """ _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) try: pm.entry_manager.modify_entry( @@ -325,6 +336,7 @@ def archive_entry( ) -> dict[str, str]: """Archive an entry.""" _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) pm.entry_manager.archive_entry(entry_id) return {"status": "archived"} @@ -336,6 +348,7 @@ def unarchive_entry( ) -> dict[str, str]: """Restore an archived entry.""" _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) pm.entry_manager.restore_entry(entry_id) return {"status": "active"} @@ -464,6 +477,7 @@ def export_totp( """Return all stored TOTP entries in JSON format.""" _check_token(request, authorization) _require_password(request, password) + _require_unlocked(request) pm = _get_pm(request) key = getattr(pm, "KEY_TOTP_DET", None) or getattr(pm, "parent_seed", None) return pm.entry_manager.export_totp_entries(key) @@ -478,6 +492,7 @@ def get_totp_codes( """Return active TOTP codes with remaining seconds.""" _check_token(request, authorization) _require_password(request, password) + _require_unlocked(request) pm = _get_pm(request) entries = pm.entry_manager.list_entries( filter_kinds=[EntryType.TOTP.value], include_archived=False @@ -501,6 +516,7 @@ def get_profile_stats( ) -> dict: """Return statistics about the active seed profile.""" _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) return pm.get_profile_stats() @@ -626,6 +642,7 @@ def export_vault( """Export the vault and return the encrypted file.""" _check_token(request, authorization) _require_password(request, password) + _require_unlocked(request) pm = _get_pm(request) path = pm.handle_export_database() if path is None: @@ -640,6 +657,7 @@ async def import_vault( ) -> dict[str, str]: """Import a vault backup from a file upload or a server path.""" _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) ctype = request.headers.get("content-type", "") @@ -693,6 +711,7 @@ def backup_parent_seed( """Create an encrypted backup of the parent seed after confirmation.""" _check_token(request, authorization) _require_password(request, password) + _require_unlocked(request) pm = _get_pm(request) if not data.get("confirm"): @@ -715,6 +734,7 @@ def change_password( ) -> dict[str, str]: """Change the master password for the active profile.""" _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) pm.change_password(data.get("old", ""), data.get("new", "")) @@ -727,6 +747,7 @@ def generate_password( ) -> dict[str, str]: """Generate a password using optional policy overrides.""" _check_token(request, authorization) + _require_unlocked(request) pm = _get_pm(request) length = int(data.get("length", 12)) @@ -758,6 +779,20 @@ def lock_vault( return {"status": "locked"} +@app.post("/api/v1/vault/unlock") +def unlock_vault( + request: Request, + authorization: str | None = Header(None), + password: str | None = Header(None, alias="X-SeedPass-Password"), +) -> dict[str, float | str]: + """Unlock the vault using the supplied master password.""" + _check_token(request, authorization) + _require_password(request, password) + pm = _get_pm(request) + duration = pm.unlock_vault(password) + return {"status": "unlocked", "duration": float(duration)} + + @app.post("/api/v1/shutdown") async def shutdown_server( request: Request, authorization: str | None = Header(None) diff --git a/src/tests/test_api_new_endpoints.py b/src/tests/test_api_new_endpoints.py index 779cceca..abb26afa 100644 --- a/src/tests/test_api_new_endpoints.py +++ b/src/tests/test_api_new_endpoints.py @@ -442,6 +442,53 @@ async def test_vault_lock_endpoint(client): assert api.app.state.pm.locked is False +@pytest.mark.anyio +async def test_vault_unlock_endpoint(client): + cl, token = client + called = {} + + def unlock(pw): + called["password"] = pw + api.app.state.pm.locked = False + api.app.state.pm.is_locked = False + return 0.123 + + api.app.state.pm.verify_password = lambda pw: pw == "pw" + api.app.state.pm.unlock_vault = unlock + api.app.state.pm.locked = True + api.app.state.pm.is_locked = True + + headers = {"Authorization": f"Bearer {token}", "X-SeedPass-Password": "pw"} + res = await cl.post("/api/v1/vault/unlock", headers=headers) + assert res.status_code == 200 + assert res.json()["status"] == "unlocked" + assert called["password"] == "pw" + + +@pytest.mark.anyio +async def test_entry_endpoints_blocked_when_vault_locked(client): + cl, token = client + api.app.state.pm.locked = True + api.app.state.pm.is_locked = True + + headers = {"Authorization": f"Bearer {token}"} + res = await cl.get("/api/v1/entry", params={"query": "x"}, headers=headers) + assert res.status_code == 423 + assert res.json()["detail"] == "Vault is locked" + + +@pytest.mark.anyio +async def test_generate_password_blocked_when_vault_locked(client): + cl, token = client + api.app.state.pm.locked = True + api.app.state.pm.is_locked = True + + headers = {"Authorization": f"Bearer {token}"} + res = await cl.post("/api/v1/password", headers=headers, json={"length": 12}) + assert res.status_code == 423 + assert res.json()["detail"] == "Vault is locked" + + @pytest.mark.anyio async def test_secret_mode_endpoint(client): cl, token = client