diff --git a/docs/auth_lock_unlock_access_review.md b/docs/auth_lock_unlock_access_review.md index b2497c6e..41346533 100644 --- a/docs/auth_lock_unlock_access_review.md +++ b/docs/auth_lock_unlock_access_review.md @@ -1,6 +1,6 @@ # SeedPass Auth, Lock/Unlock & Access-Control Review (Item 6) -Status: `In Progress` +Status: `Done` Date: `2026-02-25` ## Scope Reviewed @@ -12,7 +12,7 @@ Date: `2026-02-25` ## Exit Criteria Check (Item 6) -1. Lock/unlock/timeout/quick unlock behavior reviewed with tests: `In Progress` +1. Lock/unlock/timeout/quick unlock behavior reviewed with tests: `Done` ## Findings @@ -29,11 +29,13 @@ Date: `2026-02-25` - Config, relay, checksum, and selected profile-management routes now require unlocked state. 5. Restricted sensitive config key exposure/update. - `password_hash` and `pin_hash` access/update blocked via generic config API. +6. Added unlock-attempt throttling on API unlock route. + - Repeated failed unlock attempts now return `429` after threshold. ### 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` +2. Result: `65 passed` ### Existing posture @@ -41,13 +43,11 @@ Date: `2026-02-25` 2. Inactivity timeout enforcement exists via `AuthGuard`. 3. Quick unlock behavior is configurable and covered by existing tests. -### Remaining follow-up +### Policy decisions -1. Decide whether non-sensitive API routes (config/fingerprint/relays) should also require unlocked state. - - Completed for key admin/config routes. -2. Add API-level test coverage for timeout-triggered lock then unlock flow. - - Completed with lock->blocked->unlock->access restoration regression test. -3. Evaluate rate-limit tuning and brute-force controls for repeated password attempts on unlock endpoints. +1. Non-sensitive fingerprint list/select routes remain available while locked to support pre-unlock profile selection. +2. Admin/config/data routes are lock-gated. +3. Unlock brute-force resistance now combines general API rate limiting with dedicated failed-unlock throttling. ## Evidence diff --git a/docs/security_readiness_checklist.md b/docs/security_readiness_checklist.md index 54fc5895..aead8c9e 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 | 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` | +| 6 | Auth, lock/unlock, and access-control hardening | Done | 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`, `src/tests/test_unlock_sync.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 ecae9e6c..6933ab52 100644 --- a/memory-update.md +++ b/memory-update.md @@ -1,17 +1,25 @@ # Memory Update (2026-02-25) ## Security Checklist Progress -- Item 6 (`Auth, lock/unlock, and access-control hardening`) remains `In Progress` with stronger API-level access control. +- Item 6 (`Auth, lock/unlock, and access-control hardening`) moved to `Done`. -## Item 6 Work Completed This Pass -1. Added broader locked-state enforcement in `src/seedpass/api.py`. - - Protected/admin API routes now return `423` when the vault is locked. -2. Added sensitive config-key protection in API. - - Generic config API now blocks `password_hash` and `pin_hash` access/updates. -3. Added lock/unlock/access-control regressions in `src/tests/test_api_new_endpoints.py`: - - `test_config_endpoint_blocked_when_vault_locked` - - `test_get_config_denies_sensitive_keys` - - `test_lock_unlock_cycle_restores_entry_access` +## Final Item 6 Hardening Completed +1. Added locked-state API enforcement across protected admin/config/data routes (`423` when locked). +2. Added explicit API unlock endpoint (`POST /api/v1/vault/unlock`). +3. Added dedicated unlock-failure throttling in `seedpass/api.py` (returns `429` after repeated failed unlock attempts). +4. Blocked sensitive config keys in generic config API: + - `password_hash` + - `pin_hash` +5. Preserved pre-unlock operational path by keeping fingerprint list/select available while locked. + +## Regression Coverage Added +- `test_vault_unlock_endpoint` +- `test_vault_unlock_rate_limited_after_failed_attempts` +- `test_entry_endpoints_blocked_when_vault_locked` +- `test_generate_password_blocked_when_vault_locked` +- `test_config_endpoint_blocked_when_vault_locked` +- `test_get_config_denies_sensitive_keys` +- `test_lock_unlock_cycle_restores_entry_access` ## Validation - Executed: @@ -20,9 +28,7 @@ - `src/tests/test_vault_lock_flag.py` - `src/tests/test_inactivity_lock.py` - `src/tests/test_unlock_sync.py` -- Result: `65 passed`. +- Result: `66 passed`. -## Remaining Item 6 Focus -1. Evaluate whether fingerprint selection/list should be limited while locked in API mode. -2. Add explicit unlock-attempt abuse controls beyond general token rate limiting. -3. Finalize item-6 completion criteria and move to `Done` once remaining policy decisions are resolved. +## Next Checklist Candidate +- Item #8 (`Supply chain and release integrity`) is the next major `Not Started` security item. diff --git a/src/seedpass/api.py b/src/seedpass/api.py index b46b58c3..43e613fd 100644 --- a/src/seedpass/api.py +++ b/src/seedpass/api.py @@ -28,6 +28,8 @@ _RATE_LIMIT = int(os.getenv("SEEDPASS_RATE_LIMIT", "100")) _RATE_WINDOW = int(os.getenv("SEEDPASS_RATE_WINDOW", "60")) _RATE_LIMIT_STR = f"{_RATE_LIMIT}/{_RATE_WINDOW} seconds" _MAX_IMPORT_BYTES = int(os.getenv("SEEDPASS_MAX_IMPORT_BYTES", str(10 * 1024 * 1024))) +_UNLOCK_ATTEMPT_LIMIT = int(os.getenv("SEEDPASS_UNLOCK_ATTEMPT_LIMIT", "5")) +_UNLOCK_ATTEMPT_WINDOW = int(os.getenv("SEEDPASS_UNLOCK_ATTEMPT_WINDOW", "300")) app = FastAPI() @@ -70,9 +72,7 @@ def _check_token(request: Request, auth: str | None) -> None: def _enforce_rate_limit(request: Request, raw_token: str) -> None: now = time.monotonic() - client = request.client.host if request.client else "unknown" - token_tag = hashlib.blake2s(raw_token.encode("utf-8"), digest_size=8).hexdigest() - key = f"{client}:{token_tag}" + key = _request_rate_key(request, raw_token) buckets = getattr(request.app.state, "rate_limit_buckets", None) if buckets is None: buckets = defaultdict(deque) @@ -88,6 +88,42 @@ def _enforce_rate_limit(request: Request, raw_token: str) -> None: bucket.append(now) +def _request_rate_key(request: Request, raw_token: str) -> str: + client = request.client.host if request.client else "unknown" + token_tag = hashlib.blake2s(raw_token.encode("utf-8"), digest_size=8).hexdigest() + return f"{client}:{token_tag}" + + +def _enforce_unlock_attempt_limit(request: Request, raw_token: str) -> None: + """Enforce a stricter rate limit for repeated unlock attempts.""" + now = time.monotonic() + key = _request_rate_key(request, raw_token) + buckets = getattr(request.app.state, "unlock_attempt_buckets", None) + if buckets is None: + buckets = defaultdict(deque) + request.app.state.unlock_attempt_buckets = buckets + bucket = buckets[key] + while bucket and (now - bucket[0]) > _UNLOCK_ATTEMPT_WINDOW: + bucket.popleft() + if len(bucket) >= _UNLOCK_ATTEMPT_LIMIT: + raise HTTPException( + status_code=429, + detail=( + "Too many failed unlock attempts. " "Try again after cooldown period." + ), + ) + + +def _record_unlock_failure(request: Request, raw_token: str) -> None: + now = time.monotonic() + key = _request_rate_key(request, raw_token) + buckets = getattr(request.app.state, "unlock_attempt_buckets", None) + if buckets is None: + buckets = defaultdict(deque) + request.app.state.unlock_attempt_buckets = buckets + buckets[key].append(now) + + def _reload_relays(request: Request, relays: list[str]) -> None: """Reload the Nostr client with a new relay list.""" pm = _get_pm(request) @@ -118,6 +154,7 @@ def start_server(fingerprint: str | None = None) -> str: raw_token = secrets.token_urlsafe(32) app.state.token_hash = bcrypt.hashpw(raw_token.encode(), bcrypt.gensalt()) app.state.rate_limit_buckets = defaultdict(deque) + app.state.unlock_attempt_buckets = defaultdict(deque) return raw_token @@ -808,8 +845,14 @@ def unlock_vault( ) -> dict[str, float | str]: """Unlock the vault using the supplied master password.""" _check_token(request, authorization) - _require_password(request, password) + if authorization is None: + raise HTTPException(status_code=401, detail="Unauthorized") + raw_token = authorization.split(" ", 1)[1] + _enforce_unlock_attempt_limit(request, raw_token) pm = _get_pm(request) + if password is None or not pm.verify_password(password): + _record_unlock_failure(request, raw_token) + raise HTTPException(status_code=401, detail="Invalid password") duration = pm.unlock_vault(password) return {"status": "unlocked", "duration": float(duration)} diff --git a/src/tests/test_api_new_endpoints.py b/src/tests/test_api_new_endpoints.py index 6f96449b..daf87fc2 100644 --- a/src/tests/test_api_new_endpoints.py +++ b/src/tests/test_api_new_endpoints.py @@ -465,6 +465,24 @@ async def test_vault_unlock_endpoint(client): assert called["password"] == "pw" +@pytest.mark.anyio +async def test_vault_unlock_rate_limited_after_failed_attempts(client, monkeypatch): + cl, token = client + api.app.state.pm.verify_password = lambda _pw: False + api.app.state.pm.unlock_vault = lambda _pw: 0.01 + + monkeypatch.setattr(api, "_UNLOCK_ATTEMPT_LIMIT", 2) + monkeypatch.setattr(api, "_UNLOCK_ATTEMPT_WINDOW", 600) + + headers = {"Authorization": f"Bearer {token}", "X-SeedPass-Password": "bad"} + res1 = await cl.post("/api/v1/vault/unlock", headers=headers) + assert res1.status_code == 401 + res2 = await cl.post("/api/v1/vault/unlock", headers=headers) + assert res2.status_code == 401 + res3 = await cl.post("/api/v1/vault/unlock", headers=headers) + assert res3.status_code == 429 + + @pytest.mark.anyio async def test_entry_endpoints_blocked_when_vault_locked(client): cl, token = client