Compare commits

..
7 Commits
Author SHA1 Message Date
nngo 0d7d9c1403 Aug 26 - Update password detect against off field 2
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 15:05:48 -04:00
nngo 2f1afb143c Aug 26 - Update password detect against off field
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 14:47:16 -04:00
nngo b84a6d9245 Aug 26 - Enhance security 4
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 14:19:25 -04:00
nngo cc216b0d98 Aug 26 - Enhance security 3
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 13:54:48 -04:00
nngo 6c1bef73c8 Aug 26 - Enhance security 2
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 12:54:17 -04:00
nngo 82dd7c5aef Aug 26 - Add gunicorn.conf.py
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 10:58:55 -04:00
nngo 0304095e53 Aug 26 - Enhance security
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
2026-08-26 10:37:00 -04:00
50 changed files with 14407 additions and 396 deletions
+46 -8
View File
@@ -8,7 +8,8 @@
# lint-python — flake8 style + error check # lint-python — flake8 style + error check
# syntax-check — ast.parse all Python files # syntax-check — ast.parse all Python files
# migration-check — verify Alembic chain has single head # migration-check — verify Alembic chain has single head
# js-syntax — node syntax check on all JS files # js-syntax — node syntax check on all JS files + PSL matching tests
# tests — pytest suite (in-memory SQLite, no MySQL needed)
# build-extension — zip Chrome and Firefox extensions # build-extension — zip Chrome and Firefox extensions
name: CI name: CI
@@ -48,12 +49,23 @@ jobs:
- name: Check all Python files parse cleanly - name: Check all Python files parse cleanly
run: | run: |
python3 - << 'EOF' python3 - << 'EOF'
import ast, sys, pathlib import ast, sys, pathlib, itertools
# Root-level modules (wsgi, run, reset_db, gunicorn.conf) were not
# covered before, so a syntax error in the Gunicorn config or the WSGI
# entrypoint reached production without CI noticing.
paths = list(itertools.chain(
pathlib.Path('app').rglob('*.py'),
pathlib.Path('tests').rglob('*.py'),
pathlib.Path('scripts').rglob('*.py'),
pathlib.Path('migrations/versions').rglob('*.py'),
pathlib.Path('.').glob('*.py'),
))
failures = [] failures = []
for path in pathlib.Path('app').rglob('*.py'): for path in paths:
try: try:
ast.parse(path.read_text()) ast.parse(path.read_text(encoding='utf-8'))
except SyntaxError as e: except SyntaxError as e:
failures.append(f"{path}: {e}") failures.append(f"{path}: {e}")
@@ -61,8 +73,7 @@ jobs:
print(f"FAIL: {f}") print(f"FAIL: {f}")
if failures: if failures:
sys.exit(1) sys.exit(1)
count = len(list(pathlib.Path('app').rglob('*.py'))) print(f"OK: {len(paths)} Python files parsed cleanly")
print(f"OK: {count} Python files parsed cleanly")
EOF EOF
# ── Alembic migration chain ────────────────────────────────────────────────── # ── Alembic migration chain ──────────────────────────────────────────────────
@@ -121,7 +132,7 @@ jobs:
extension/background.firefox.js \ extension/background.firefox.js \
extension/content/content.js \ extension/content/content.js \
extension/bridge/bridge.js \ extension/bridge/bridge.js \
extension/shared/crypto.js; do extension/shared/crypto.js \n extension/shared/psl.js; do
if [ -f "$f" ]; then if [ -f "$f" ]; then
node -e "new Function(require('fs').readFileSync('$f','utf8'))" 2>/dev/null || \ node -e "new Function(require('fs').readFileSync('$f','utf8'))" 2>/dev/null || \
{ echo "FAIL: $f"; FAILED=1; } { echo "FAIL: $f"; FAILED=1; }
@@ -130,11 +141,38 @@ jobs:
[ $FAILED -eq 0 ] && echo "OK: all JS files parsed cleanly" [ $FAILED -eq 0 ] && echo "OK: all JS files parsed cleanly"
exit $FAILED exit $FAILED
- name: PSL matching tests
# Guards the autofill same-site check. A wrong answer here means
# credentials offered on an attacker's neighbouring subdomain.
run: node tests/js/test_psl.js
- name: Field heuristic tests
# Guards login-field detection. A wrong answer here means autofill
# silently does nothing on real login pages.
run: node tests/js/test_field_heuristics.js
# ── Test suite ───────────────────────────────────────────────────────────────
# Runs against in-memory SQLite (see app/config.py TestingConfig) so no MySQL
# service is needed on the host-mode runner. That means these tests cover
# application logic and flow, not MySQL-specific behaviour — schema changes
# still need a real `flask db upgrade` against MySQL before deploying.
tests:
name: Pytest
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Install dependencies
run: pip3 install -r requirements.txt -r requirements-dev.txt --quiet --break-system-packages
- name: Run test suite
run: python3 -m pytest tests/ -q
# ── Extension build ────────────────────────────────────────────────────────── # ── Extension build ──────────────────────────────────────────────────────────
build-extension: build-extension:
name: Build extension zip name: Build extension zip
runs-on: ubuntu-latest runs-on: ubuntu-latest
needs: [syntax-check, js-syntax] needs: [syntax-check, js-syntax, tests]
steps: steps:
- uses: actions/checkout@v3 - uses: actions/checkout@v3
+179 -5
View File
@@ -53,6 +53,7 @@ passkeeper/
│ │ ├── emergency_access.py # State machine │ │ ├── emergency_access.py # State machine
│ │ ├── recovery_challenge.py # Server-side recovery challenge (multi-worker safe) │ │ ├── recovery_challenge.py # Server-side recovery challenge (multi-worker safe)
│ │ ├── webauthn_credential.py # Passkey / WebAuthn credentials (one row per key) │ │ ├── webauthn_credential.py # Passkey / WebAuthn credentials (one row per key)
│ │ ├── login_attempt.py # Failed-login lockout scoped to (user, IP)
│ │ └── audit_log.py │ │ └── audit_log.py
│ ├── routes/ │ ├── routes/
│ │ ├── auth.py # Register, login, MFA, logout, refresh, change-password, recovery │ │ ├── auth.py # Register, login, MFA, logout, refresh, change-password, recovery
@@ -82,6 +83,7 @@ passkeeper/
│ ├── background.firefox.js # Firefox: in-memory session shim, setTimeout idle lock │ ├── background.firefox.js # Firefox: in-memory session shim, setTimeout idle lock
│ ├── shared/ │ ├── shared/
│ │ ├── crypto.js # PBKDF2+AES-GCM; encryptName/decryptName; extractable key │ │ ├── crypto.js # PBKDF2+AES-GCM; encryptName/decryptName; extractable key
│ │ ├── psl.js # GENERATED — vendored Public Suffix List + PkPsl.isSameSite
│ │ └── browser-polyfill.js # chrome=browser alias for Firefox content scripts │ │ └── browser-polyfill.js # chrome=browser alias for Firefox content scripts
│ ├── popup/ │ ├── popup/
│ │ ├── popup.html # Tabs: All relevant / All items / Favorites / Recents │ │ ├── popup.html # Tabs: All relevant / All items / Favorites / Recents
@@ -106,9 +108,27 @@ passkeeper/
│ ├── g7h8i9j0k1l2_encrypt_shared_item_name.py # enc_name/iv_name on shared_items │ ├── g7h8i9j0k1l2_encrypt_shared_item_name.py # enc_name/iv_name on shared_items
│ └── h8i9j0k1l2m3_add_webauthn_credentials_table.py # Passkey / WebAuthn credentials │ └── h8i9j0k1l2m3_add_webauthn_credentials_table.py # Passkey / WebAuthn credentials
├── scripts/ ├── scripts/
│ ├── update_psl.py # regenerates extension/shared/psl.js
│ ├── reencrypt_totp_secrets.py │ ├── reencrypt_totp_secrets.py
│ ├── backup_db.sh / backup.cron / passkeeper-logrotate │ ├── backup_db.sh / backup.cron / passkeeper-logrotate
│ ├── passkeeper-nginx.conf / passkeeper.service │ ├── passkeeper-nginx.conf / passkeeper.service
├── tests/ # pytest suite — in-memory SQLite, no MySQL needed
│ ├── conftest.py # app/client fixtures + register/login helpers
│ ├── test_mfa_gate.py # enc_key_salt withheld until MFA; proof not forgeable
│ ├── test_key_rotation.py # re-encryption completeness guard
│ ├── test_session_revocation.py # token_epoch revocation; deleted-account 401
│ ├── test_webauthn_uv.py # user verification required on both ceremonies
│ ├── test_sharing_expiry.py # expires_days fails closed
│ ├── test_registration_privacy.py # register does not disclose account existence
│ ├── test_emergency_visibility.py # grantor sees requests + retrievals
│ ├── test_login_lockout.py # per-IP lockout; no disclosure, no DoS
│ ├── test_deploy_config.py # nginx/gunicorn/systemd/extension packaging guards
│ └── js/
│ ├── test_psl.js # PSL same-site matching (node, run in CI)
│ └── test_field_heuristics.js # login-field detection predicates
├── gunicorn.conf.py # worker class, timeouts, preload_app=False
├── pytest.ini
├── requirements-dev.txt
├── reset_db.py ├── reset_db.py
├── requirements.txt ├── requirements.txt
├── wsgi.py / run.py ├── wsgi.py / run.py
@@ -135,7 +155,9 @@ CREATE TABLE users (
sharing_private_key_enc TEXT, sharing_private_key_enc TEXT,
sharing_private_key_iv VARCHAR(64), sharing_private_key_iv VARCHAR(64),
recovery_enc_salt VARCHAR(128), recovery_enc_salt VARCHAR(128),
recovery_iv VARCHAR(64) recovery_iv VARCHAR(64),
recovery_verifier VARCHAR(64), -- HMAC key for the recovery challenge
token_epoch INT NOT NULL DEFAULT 0 -- session generation counter
); );
-- Vault Items -- Vault Items
@@ -225,6 +247,11 @@ CREATE TABLE webauthn_credentials (
| `f6a7b8c9d0e1` | Add totp_used_codes table (TOTP replay prevent.) | | `f6a7b8c9d0e1` | Add totp_used_codes table (TOTP replay prevent.) |
| `g7h8i9j0k1l2` | Add enc_name/iv_name to shared_items | | `g7h8i9j0k1l2` | Add enc_name/iv_name to shared_items |
| `h8i9j0k1l2m3` | Add webauthn_credentials table (passkeys) | | `h8i9j0k1l2m3` | Add webauthn_credentials table (passkeys) |
| `i9j0k1l2m3n4` | Add expires_at to shared_items |
| `j0k1l2m3n4o5` | Add recovery_verifier (decouple recovery proof) |
| `k1l2m3n4o5p6` | Add token_epoch (revoke sessions on pw change) |
| `l2m3n4o5p6q7` | Add emergency vault retrieval tracking |
| `m3n4o5p6q7r8` | Add login_attempts (per-IP lockout) |
--- ---
@@ -237,17 +264,72 @@ CREATE TABLE webauthn_credentials (
- **Item name:** `enc_name`/`iv_name` in `vault_items`; server `name` column = item type only - **Item name:** `enc_name`/`iv_name` in `vault_items`; server `name` column = item type only
- **Shared item name:** `enc_name`/`iv_name` encrypted with ECDH shared key; server `item_name` = item type only - **Shared item name:** `enc_name`/`iv_name` encrypted with ECDH shared key; server `item_name` = item type only
- **Tags:** `plain.tags: string[]` inside `enc_data`; server never sees them - **Tags:** `plain.tags: string[]` inside `enc_data`; server never sees them
- **Login lockout:** scoped to (account, source IP) in `login_attempts`, NOT
global. A global counter made it a DoS primitive — anyone knowing an address
could lock the real owner out for 15 minutes, repeatedly. Every failure mode
(unknown account / wrong password / locked out) returns one identical 401 with
matching timing, so it discloses nothing. `users.failed_login_count` and
`locked_until` remain as an aggregate audit signal only; they no longer gate
authentication.
- **Argon2id:** double-hashes `authHash` server-side; transparently rehashes on login if parameters are upgraded - **Argon2id:** double-hashes `authHash` server-side; transparently rehashes on login if parameters are upgraded
- **JWT:** HS256, 15 min access / 7 day refresh, JTI blacklisted on logout - **JWT:** HS256, 15 min access / 7 day refresh, JTI blacklisted on logout.
Every token carries an `epoch` claim checked against `users.token_epoch`;
changing the master password or recovering the account increments it, which
revokes every outstanding access AND refresh token. Tokens minted before the
claim existed decode as `epoch 0` and remain valid until the next change.
- **MFA gate:** `enc_key_salt` is NOT returned by `/login` when TOTP is enabled —
it is released by `/mfa/verify` once both factors are proven. Returning it
early let a password-only attacker forge a recovery proof (see below).
- **MFA:** TOTP secret AES-256-GCM encrypted at rest; each code is single-use (replay prevented via `totp_used_codes` table, 120s TTL) - **MFA:** TOTP secret AES-256-GCM encrypted at rest; each code is single-use (replay prevented via `totp_used_codes` table, 120s TTL)
- **Passkeys / WebAuthn:** server authentication via FIDO2; ZK model preserved — WebAuthn proves identity to the server but the vault key is still derived from the master password client-side; `sign_count` updated on every assertion for clone detection - **Passkeys / WebAuthn:** server authentication via FIDO2; ZK model preserved — WebAuthn proves identity to the server but the vault key is still derived from the master password client-side; `sign_count` updated on every assertion for clone detection
- **Sharing:** ECDH P-256 zero-knowledge re-encryption; item name also encrypted with shared key - **Sharing:** ECDH P-256 zero-knowledge re-encryption; item name also encrypted with shared key
- **Autofill same-site rule:** `PkPsl.isSameSite()` compares *registrable
domains* using the vendored Public Suffix List — never suffix comparison.
`host.endsWith("." + h)` treated `evil.github.io` and `victim.github.io` as
the same site and let an item saved for a bare TLD match everything under it.
Used identically by `content.js`, `popup.js`, `background.js` and
`background.firefox.js`; all four fall back to exact hostname equality if
`psl.js` fails to load (strict, so a failure loses matches rather than
leaking credentials). The PRIVATE section of the list is required — that is
where `github.io` / `vercel.app` / `herokuapp.com` live.
- **Password generator:** fully CSPRNG (`_cryptoRandInt` rejection-sampling) - **Password generator:** fully CSPRNG (`_cryptoRandInt` rejection-sampling)
- **Decrypted vault data:** `chrome.storage.session` only — never to disk - **Decrypted vault data:** `chrome.storage.session` only — never to disk
- **Clipboard auto-clear:** 30 s after any password/username copy (web + extension) - **Clipboard auto-clear:** 30 s after any password/username copy (web + extension)
- **Breach detection:** HIBP k-anonymity — only 5-char SHA-1 prefix transmitted - **Breach detection:** HIBP k-anonymity — only 5-char SHA-1 prefix transmitted
- **Recovery proof key:** `HMAC-SHA256(key=recovery_verifier, msg=nonce)`.
`recovery_verifier` = `PBKDF2(recoveryCode, "passkeeper-recovery-verifier:" + email, 200k)`,
derived client-side from the recovery code alone and used for nothing else.
It must NEVER be `enc_key_salt`: that value doubles as the vault-key PBKDF2
salt and is disclosed to the client at login, so keying the proof with it
allowed anyone holding the master password to pull the entire vault from the
unauthenticated `/recovery/items` — bypassing MFA. Accounts whose recovery
code predates the column fall back to the legacy key and are flagged via
`recovery_is_legacy` on `/recovery/status`.
- **Key rotation completeness:** `change_password` and `/recover` refuse (409
`incomplete_reencryption`) unless the client's `items` payload covers every
vault item the user owns — a short payload would rotate `enc_key_salt` and
leave the missing items permanently undecryptable. `/recover` accepts
`allow_partial: true` after the user confirms the loss (otherwise one corrupt
item locks them out forever); `change_password` has no such override.
- **Passkeys:** both ceremonies use `UserVerificationRequirement.REQUIRED` and
`require_user_verification=True`. A passkey replaces password AND TOTP, so
possession of an unlocked device must not be sufficient.
- **Account recovery:** challenge-response via HMAC-SHA256; `enc_key_salt` NOT returned by `/recovery/data` — client must derive it by decrypting the recovery blob (proves possession of recovery code without transmitting it); challenge rotated on each `/recovery/items` call to prevent proof replay; recovery key derived with the user's email as a per-user PBKDF2 salt — legacy fixed salt `'passkeeper-recovery'` accepted transparently for codes created before this change - **Account recovery:** challenge-response via HMAC-SHA256; `enc_key_salt` NOT returned by `/recovery/data` — client must derive it by decrypting the recovery blob (proves possession of recovery code without transmitting it); challenge rotated on each `/recovery/items` call to prevent proof replay; recovery key derived with the user's email as a per-user PBKDF2 salt — legacy fixed salt `'passkeeper-recovery'` accepted transparently for codes created before this change
- **folder_id ownership:** validated server-side on all create/update/import operations — user cannot assign items to another user's folder - **folder_id ownership:** validated server-side on all create/update/import operations — user cannot assign items to another user's folder
- **Registration privacy:** `POST /api/auth/register` returns an identical 202
whether or not the address exists, and performs an equivalent Argon2id hash on
both branches so timing does not reinstate the oracle. Duplicate attempts are
audited under `auth.register_duplicate`. Fully closing this needs email
verification so the address owner is told — until then the oracle is removed
but the owner cannot be notified.
- **Emergency access visibility:** `accept`, `request` and `vault_retrieved` are
audited under BOTH parties' user_ids. `/api/auth/audit-log` filters by
`user_id`, so an entry written only under the acting user is invisible to the
other — which meant a grantee could request and retrieve a vault snapshot
without anything reaching the grantor. `vault_retrieved_at` /
`vault_retrieval_count` on `emergency_access` record every fetch. Retrieval is
intentionally NOT blocked after the first time (the grantor may be unable to
re-provision); the wait period is the gate, and the grantor can revoke.
- **Audit logs:** never contain plaintext item names, shared item names, or vault data - **Audit logs:** never contain plaintext item names, shared item names, or vault data
--- ---
@@ -548,7 +630,61 @@ In both `content.js` and `popup.js`. Prevents silent match failures for bare dom
1. **YES:** `autocomplete="username|email|tel"` 1. **YES:** `autocomplete="username|email|tel"`
2. **NO:** non-credential autocomplete (`name`, `organization`, `search`, etc.) 2. **NO:** non-credential autocomplete (`name`, `organization`, `search`, etc.)
3. **YES:** `name/id/placeholder/aria-label` matches `user|email|mail|login|phone|tel|mobile|account` 3. **YES:** `name/id/placeholder/aria-label` matches `user|email|mail|login|phone|tel|mobile|account`
4. **Otherwise:** not decorated 4. **Then:** `_hasPasswordSibling()` must also pass
5. **Otherwise:** not decorated
**`autocomplete="off"` is NOT a negative signal** and must never be added back to
`NON_CRED_AC`. Routers, banks and admin panels set it on login fields precisely
to discourage password managers. It previously caused step 2 to reject fields as
obvious as `<input id="login_username" placeholder="Username" autocomplete="off">`
before step 3 ever ran (ASUS RT-AX88U admin login). Letting it fall through is
safe — the field still needs a credential keyword AND a nearby password input.
### Credential capture without a `<form>`
Many login UIs never use a `<form>` — the ASUS router admin page submits with
`<div class="button" onclick="preLogin();">Sign In</div>`, so no `submit` event
is ever dispatched and the save-credentials banner never appeared.
`watchSubmissions()` therefore registers three triggers, all routed through
`maybeCaptureCredentials(scope)`:
1. `submit` on any form (scope = the form)
2. `click` on anything `_looksLikeSubmitControl()` accepts — `<button>`,
`input[type=submit|button|image]`, `[role=button]`, any element with an
inline `onclick`, or a button-ish class name (scope = document)
3. `Enter` keydown inside a password or likely-username field (scope = document)
`_captureCooldown` (2 s) prevents two triggers double-prompting for one login.
### Insecure-page warning
The extension keeps `http://*/*` permission deliberately: routers, NAS boxes,
printers and self-hosted panels are often reachable only over plain HTTP on the
LAN, and those are exactly the devices whose passwords get reused.
`_isTrustworthyOrigin()` classifies the page. HTTPS, `file:`, `localhost`,
reserved TLDs (`.local` / `.lan` / `.home` / `.internal`) and RFC1918 /
loopback / link-local / RFC4193 addresses are accepted silently. Any other
`http://` origin gets a red warning row prepended to the suggestion dropdown.
**IPv4 checks must match in FULL** (`$`-anchored). A prefix test like
`hostname.startsWith("127.")` also accepts the registrable
`127.0.0.1.evil.com`, which would silently suppress the warning on a hostile
site. Guarded by `tests/js/test_field_heuristics.js`.
Filling is never automatic, so this warns rather than blocks — silently
offering nothing would just look like a broken extension.
### Autologin without a `<form>`
`_findSubmitControl(pwField)` locates the control that submits the login,
walking up to 5 ancestors when there is no `<form>`. It skips invisible
elements, wrappers containing other inputs, labels over 40 chars, and anything
matching `_NEGATIVE_CONTROL` (cancel / reset / back / forgot / register / sign
up), then clicks it — `form.submit()` is only the last resort because it
bypasses site handlers entirely. Returns null when nothing is convincing;
leaving a filled form for the user beats clicking the wrong thing.
### MutationObserver guard ### MutationObserver guard
@@ -585,11 +721,27 @@ Audit log details **never** contain plaintext item names, shared item names, or
- `#vault-list` ID must not be renamed — `vault.js` renders into it directly - `#vault-list` ID must not be renamed — `vault.js` renders into it directly
- `_validate_folder_id` must be called for any user-supplied `folder_id` before DB write - `_validate_folder_id` must be called for any user-supplied `folder_id` before DB write
- `verify_auth_token` must receive `user=user` at both `login` and `change_password` to enable Argon2 rehash - `verify_auth_token` must receive `user=user` at both `login` and `change_password` to enable Argon2 rehash
- APScheduler cleanup job handles `TokenBlacklist`, `RecoveryChallenge`, AND `TotpUsedCode`; guard with `os.environ.get('WERKZEUG_RUN_MAIN') == 'true'` in Flask debug mode to prevent double-start - APScheduler cleanup job handles `TokenBlacklist`, `RecoveryChallenge`, `TotpUsedCode`, expired shares AND `LoginAttempt`; guard with `os.environ.get('WERKZEUG_RUN_MAIN') == 'true'` in Flask debug mode to prevent double-start
- `password_changed_at` lives inside `plain` (encrypted) — never in the server schema - `password_changed_at` lives inside `plain` (encrypted) — never in the server schema
- WebAuthn `attachment`: `"cross-platform"` for security keys; `"platform"` for device biometrics (default) - WebAuthn `attachment`: `"cross-platform"` for security keys; `"platform"` for device biometrics (default)
- `enc_vault_is_legacy` check in `EmergencyAccess.to_dict()` is pure JSON inspection — no decryption - `enc_vault_is_legacy` check in `EmergencyAccess.to_dict()` is pure JSON inspection — no decryption
- Audit entries are only visible to the user_id they are written under — mirror cross-party events (use `_log_for_both` in `emergency.py`) or the other party never sees them
- `/register` must return the SAME body and status for new and existing addresses, and hash on both paths — returning early on duplicate reinstates a timing oracle
- Never return `str(e)` from exception handlers — log with `_log.exception(...)` and return a generic user-facing message to avoid leaking DB schema details or query fragments - Never return `str(e)` from exception handlers — log with `_log.exception(...)` and return a generic user-facing message to avoid leaking DB schema details or query fragments
- `extension/shared/psl.js` is GENERATED — never hand-edit; run `python scripts/update_psl.py`. It must load BEFORE content.js / popup.js / background.js in every manifest
- Host/IP allowlists must be `$`-anchored — `startsWith("127.")` also matches the attacker-registrable `127.0.0.1.evil.com`
- `escHtml` must escape `&`, `<`, `>`, `"` AND `'` in all three copies (vault.js, popup.js, content.js) — templates mix quote styles
- Login lockout lives in `login_attempts` keyed by (user, IP); never move it back to a global per-account counter
- Never add `off` back to `NON_CRED_AC` in `content.js``tests/js/test_field_heuristics.js` fails the build if you do
- Login detection must not assume a `<form>` exists; route new capture triggers through `maybeCaptureCredentials()`
- Never reintroduce `endsWith("." + host)` host matching anywhere in the extension — `tests/test_deploy_config.py` fails the build if it reappears
- nginx rate zones: mind `r/s` vs `r/m`. `api_limit` was `60r/m` (1 req/s for the whole API) and caused spurious 429s on normal vault use
- `preload_app` must stay `False` in `gunicorn.conf.py` — APScheduler's thread does not survive `fork()`, so `--preload` silently disables the cleanup job
- nginx `proxy_read_timeout` must stay BELOW gunicorn `timeout`, else a slow request returns 502 instead of 504
- `WatchdogSec` in the systemd unit requires `Type=notify` + `NotifyAccess=main`; without them systemd SIGKILLs the service on a loop
- `ExecReload` must not use `USR2` — it forks a second master and strands `$MAINPID`
- `ProductionConfig.validate()` is called from `create_app()`, NOT at class-definition time — running it in the class body made `import app.config` fail without production secrets, breaking tests and local tooling
- Always pass `user.token_epoch` to `generate_tokens()` — the `0` default exists only so stale call sites fail visibly
- `SharingCrypto.decryptPrivateKey` imports the key with `extractable: false` — use raw `SubtleCrypto` calls when you need the key bytes (e.g. re-encryption on password change) - `SharingCrypto.decryptPrivateKey` imports the key with `extractable: false` — use raw `SubtleCrypto` calls when you need the key bytes (e.g. re-encryption on password change)
--- ---
@@ -599,8 +751,9 @@ Audit log details **never** contain plaintext item names, shared item names, or
| Module | Action | Trigger | | Module | Action | Trigger |
| -------------- | ------------------------------------------------------ | ------------------------------ | | -------------- | ------------------------------------------------------ | ------------------------------ |
| `auth.py` | `auth.register` | New account | | `auth.py` | `auth.register` | New account |
| `auth.py` | `auth.register_duplicate` | Register attempt on an existing address (no email in detail) |
| `auth.py` | `auth.login` / `auth.login_failed` | Login success/fail | | `auth.py` | `auth.login` / `auth.login_failed` | Login success/fail |
| `auth.py` | `auth.account_locked` | Failed login lockout | | `auth.py` | `auth.account_locked` / `auth.login_blocked` | Per-IP lockout triggered / hit |
| `auth.py` | `auth.mfa_enable/disable/verify` | TOTP actions | | `auth.py` | `auth.mfa_enable/disable/verify` | TOTP actions |
| `auth.py` | `auth.mfa_backup_code_used` | Backup code login | | `auth.py` | `auth.mfa_backup_code_used` | Backup code login |
| `auth.py` | `auth.mfa_backup_codes_regenerated` | Backup code regen | | `auth.py` | `auth.mfa_backup_codes_regenerated` | Backup code regen |
@@ -614,6 +767,7 @@ Audit log details **never** contain plaintext item names, shared item names, or
| `sharing.py` | `sharing_keys.create/update` | ECDH key setup | | `sharing.py` | `sharing_keys.create/update` | ECDH key setup |
| `sharing.py` | `shared_item.create/delete/accept` | Sharing (detail: type + id) | | `sharing.py` | `shared_item.create/delete/accept` | Sharing (detail: type + id) |
| `emergency.py` | `emergency_access.*` | All EA state transitions | | `emergency.py` | `emergency_access.*` | All EA state transitions |
| `emergency.py` | `emergency_access.accept/request/vault_retrieved` | Logged under BOTH grantor and grantee user_ids |
| `webauthn.py` | `webauthn.register` | Passkey registered | | `webauthn.py` | `webauthn.register` | Passkey registered |
| `webauthn.py` | `webauthn.auth_success` / `webauthn.auth_failed` | Passkey login attempt | | `webauthn.py` | `webauthn.auth_success` / `webauthn.auth_failed` | Passkey login attempt |
| `webauthn.py` | `webauthn.rename` / `webauthn.delete` | Credential management | | `webauthn.py` | `webauthn.rename` / `webauthn.delete` | Credential management |
@@ -785,6 +939,25 @@ page itself.
Features planned for future implementation. Ordered by priority within each category. Features planned for future implementation. Ordered by priority within each category.
### Recently completed (Aug 2026)
- MFA bypass closed: `enc_key_salt` withheld until the second factor; recovery
proof rebased onto `recovery_verifier`
- Key-rotation completeness guard on `change_password` / `/recover`
- Session revocation via `token_epoch`; `require_jwt` now verifies the user exists
- Passkey ceremonies require user verification
- Autofill matching moved onto the Public Suffix List (registrable domains)
- Share `expires_days` fails closed instead of silently meaning "never"
- nginx `api_limit` corrected from 60r/m to 10r/s
- Registration no longer discloses account existence (status, body and timing)
- Login lockout scoped per-IP: no account disclosure, no lock-out-the-owner DoS
- `escHtml` escapes single quotes; dead `User.check_password` removed
- Extension warns before filling on plaintext public pages; autologin works
without a `<form>`
- Emergency access: requests and vault retrievals are now visible to the grantor
- pytest suite (77 tests) + 2 node test files + CI jobs; `gunicorn.conf.py`;
systemd watchdog removed
### High priority — user-facing ### High priority — user-facing
**1. One-time share links** **1. One-time share links**
@@ -872,6 +1045,7 @@ Extend `backup.cron` to run a weekly restore test into a throwaway DB, verifying
2. `syntax-check``ast.parse` all `app/` Python files 2. `syntax-check``ast.parse` all `app/` Python files
3. `migration-check` — single Alembic head, no duplicate revision IDs 3. `migration-check` — single Alembic head, no duplicate revision IDs
4. `js-syntax``node -e "new Function(...)"` on all vault/extension JS files 4. `js-syntax``node -e "new Function(...)"` on all vault/extension JS files
4b. `tests` — pytest suite against in-memory SQLite (`TestingConfig`); gates `build-extension`
5. `build-extension` — produces `passkeeper-extension-chrome.zip` and `passkeeper-extension-firefox.zip` as artifacts (30-day retention) 5. `build-extension` — produces `passkeeper-extension-chrome.zip` and `passkeeper-extension-firefox.zip` as artifacts (30-day retention)
**Runner:** self-hosted host-mode runner on the production server. Requires `python3`, `pip3`, `node`, `zip` on the host. No Docker needed. **Runner:** self-hosted host-mode runner on the production server. Requires `python3`, `pip3`, `node`, `zip` on the host. No Docker needed.
+18 -4
View File
@@ -266,12 +266,22 @@ atomically on any error.
## Step 8 — Test Gunicorn manually ## Step 8 — Test Gunicorn manually
Before installing the systemd service, verify Gunicorn can start the app: Before installing the systemd service, verify Gunicorn can start the app.
> **Do not add `--preload`.** `create_app()` starts an APScheduler thread for the
> hourly cleanup of `token_blacklist` / `recovery_challenges` / `totp_used_codes`
> / expired shares. Threads do not survive `fork()`, so under `--preload` the
> scheduler would live only in the arbiter — which serves no requests — and the
> cleanup would silently never run. `gunicorn.conf.py` pins `preload_app = False`
> for this reason.
```bash ```bash
source /home/spuser/.venv/bin/activate source /home/spuser/.venv/bin/activate
cd /home/spuser/PassKeeper cd /home/spuser/PassKeeper
gunicorn --workers 4 --bind 127.0.0.1:5000 --preload wsgi:app gunicorn -c gunicorn.conf.py wsgi:app
# Validate the config without starting the server:
gunicorn --check-config -c gunicorn.conf.py wsgi:app
``` ```
You should see lines like: You should see lines like:
@@ -527,8 +537,12 @@ pip install -r requirements.txt
export FLASK_APP=wsgi.py FLASK_ENV=production export FLASK_APP=wsgi.py FLASK_ENV=production
flask db upgrade flask db upgrade
# Reload Gunicorn zero-downtime (sends USR2 to master) # Restart Gunicorn to pick up the new code.
sudo systemctl reload passkeeper # NOT `reload` — ExecReload sends HUP, which re-reads gunicorn.conf.py and
# recycles workers but does NOT reload changed Python source. Using reload after
# a code deploy leaves the old code running and looks like the deploy silently
# did nothing.
sudo systemctl restart passkeeper
# Verify # Verify
sudo systemctl status passkeeper sudo systemctl status passkeeper
+14 -2
View File
@@ -42,7 +42,14 @@ def create_app(config_name: str = 'development') -> Flask:
# IP keys spoof-resistant — a client cannot bypass per-IP limits by injecting # IP keys spoof-resistant — a client cannot bypass per-IP limits by injecting
# an arbitrary IP into the XFF header. # an arbitrary IP into the XFF header.
app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1) app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1)
app.config.from_object(config[config_name]) selected_config = config[config_name]
# Reject insecure production settings before the app is handed back, so no
# request is ever served under one. Deliberately not done at class-definition
# time — that made importing app.config impossible without production
# secrets, breaking tests and local tooling.
if hasattr(selected_config, 'validate'):
selected_config.validate()
app.config.from_object(selected_config)
# Extensions # Extensions
db.init_app(app) db.init_app(app)
@@ -107,6 +114,7 @@ def create_app(config_name: str = 'development') -> Flask:
from .models.recovery_challenge import RecoveryChallenge from .models.recovery_challenge import RecoveryChallenge
from .models.totp_used_code import TotpUsedCode from .models.totp_used_code import TotpUsedCode
from .models.webauthn_credential import WebAuthnCredential from .models.webauthn_credential import WebAuthnCredential
from .models.login_attempt import LoginAttempt
@login_manager.user_loader @login_manager.user_loader
def load_user(user_id): def load_user(user_id):
@@ -197,9 +205,11 @@ def create_app(config_name: str = 'development') -> Flask:
from app.models.recovery_challenge import RecoveryChallenge from app.models.recovery_challenge import RecoveryChallenge
from app.models.totp_used_code import TotpUsedCode from app.models.totp_used_code import TotpUsedCode
from app.models.shared_item import SharedItem from app.models.shared_item import SharedItem
from app.models.login_attempt import LoginAttempt
TokenBlacklist.cleanup_expired() TokenBlacklist.cleanup_expired()
RecoveryChallenge.cleanup_expired() RecoveryChallenge.cleanup_expired()
TotpUsedCode.cleanup_expired() TotpUsedCode.cleanup_expired()
LoginAttempt.cleanup_expired()
# Delete expired unaccepted shares. # Delete expired unaccepted shares.
from datetime import datetime, timezone from datetime import datetime, timezone
SharedItem.query.filter( SharedItem.query.filter(
@@ -230,7 +240,9 @@ def create_app(config_name: str = 'development') -> Flask:
# WERKZEUG_RUN_MAIN == 'true' only in the child (the actual server), # WERKZEUG_RUN_MAIN == 'true' only in the child (the actual server),
# so we skip the scheduler in the parent reloader to avoid duplicate jobs. # so we skip the scheduler in the parent reloader to avoid duplicate jobs.
import os import os
if not app.debug or os.environ.get('WERKZEUG_RUN_MAIN') == 'true': if app.config.get('SCHEDULER_ENABLED', True) and (
not app.debug or os.environ.get('WERKZEUG_RUN_MAIN') == 'true'
):
scheduler.start() scheduler.start()
# ── Production safety checks ─────────────────────────────────────────────── # ── Production safety checks ───────────────────────────────────────────────
+55 -13
View File
@@ -78,6 +78,11 @@ class BaseConfig:
# Set via .env: STATIC_VERSION=20260418 # Set via .env: STATIC_VERSION=20260418
STATIC_VERSION = os.environ.get('STATIC_VERSION', '1') STATIC_VERSION = os.environ.get('STATIC_VERSION', '1')
# Background cleanup scheduler (token_blacklist / recovery_challenges /
# totp_used_codes / expired shares). Disabled under test so the suite does
# not spawn a daemon thread per app fixture.
SCHEDULER_ENABLED = True
# Session cookie defaults — applied in all environments. # Session cookie defaults — applied in all environments.
# SECURE is intentionally left out of BaseConfig so dev HTTP still works. # SECURE is intentionally left out of BaseConfig so dev HTTP still works.
# See ProductionConfig below for the full hardened set. # See ProductionConfig below for the full hardened set.
@@ -90,6 +95,35 @@ class DevelopmentConfig(BaseConfig):
RATELIMIT_ENABLED = False RATELIMIT_ENABLED = False
class TestingConfig(BaseConfig):
"""
In-memory SQLite, no rate limiting, no background threads.
SQLite is viable here because the only MySQL-specific construct in the
models is mysql.INTEGER(unsigned=True), which SQLAlchemy renders as a plain
INTEGER on other dialects. That means these tests cover application logic
and flow, NOT MySQL-specific behaviour (collation, ON UPDATE NOW(), unsigned
range) — schema changes still need a real migration run against MySQL.
"""
TESTING = True
DEBUG = False
SQLALCHEMY_DATABASE_URI = 'sqlite:///:memory:'
SQLALCHEMY_ENGINE_OPTIONS = {}
RATELIMIT_ENABLED = False
SCHEDULER_ENABLED = False
WTF_CSRF_ENABLED = False
SECRET_KEY = 'test-secret-not-used-in-production'
JWT_SECRET_KEY = 'test-jwt-secret-not-used-in-production'
TOTP_ENCRYPTION_KEY = '00' * 32
CORS_ORIGINS = 'http://localhost'
WEBAUTHN_RP_ID = 'localhost'
WEBAUTHN_ORIGINS = ['http://localhost']
# Keep Argon2 cheap so the suite is not dominated by password hashing.
ARGON2_TIME_COST = 1
ARGON2_MEMORY_COST = 8
ARGON2_PARALLELISM = 1
class ProductionConfig(BaseConfig): class ProductionConfig(BaseConfig):
DEBUG = False DEBUG = False
RATELIMIT_ENABLED = True RATELIMIT_ENABLED = True
@@ -98,40 +132,48 @@ class ProductionConfig(BaseConfig):
SESSION_COOKIE_HTTPONLY = True SESSION_COOKIE_HTTPONLY = True
SESSION_COOKIE_SAMESITE = 'Lax' SESSION_COOKIE_SAMESITE = 'Lax'
# ── Critical security checks — fail loudly at startup, not silently at runtime ── @classmethod
# These checks run at class definition time (i.e. at import / app startup). def validate(cls):
# Any misconfiguration raises RuntimeError immediately so the process never """
# serves a single request with an insecure configuration. Fail loudly on insecure production configuration.
_secret_key = os.environ.get('SECRET_KEY', '') Called from create_app() when this config is selected — NOT at class
if not _secret_key or _secret_key in _INSECURE_SECRET_DEFAULTS: definition time. Running it in the class body meant merely *importing*
app.config raised unless production secrets were present in the
environment, which broke the test suite and any local tooling that
imports the app (including `flask db upgrade` on a dev box).
The fail-loud property is preserved: create_app('production') raises
before the app is returned, so the process still never serves a request
under an insecure configuration.
"""
secret_key = os.environ.get('SECRET_KEY', '')
if not secret_key or secret_key in _INSECURE_SECRET_DEFAULTS:
raise RuntimeError( raise RuntimeError(
'[PassKeeper] SECRET_KEY is not set or uses an insecure placeholder. ' '[PassKeeper] SECRET_KEY is not set or uses an insecure placeholder. '
'Generate a strong key with: python -c "import secrets; print(secrets.token_hex(32))" ' 'Generate a strong key with: python -c "import secrets; print(secrets.token_hex(32))" '
'and add SECRET_KEY=<value> to your production .env file.' 'and add SECRET_KEY=<value> to your production .env file.'
) )
SECRET_KEY = _secret_key
_jwt_secret = os.environ.get('JWT_SECRET_KEY', '') jwt_secret = os.environ.get('JWT_SECRET_KEY', '')
if not _jwt_secret or _jwt_secret in _INSECURE_SECRET_DEFAULTS: if not jwt_secret or jwt_secret in _INSECURE_SECRET_DEFAULTS:
raise RuntimeError( raise RuntimeError(
'[PassKeeper] JWT_SECRET_KEY is not set or uses an insecure placeholder. ' '[PassKeeper] JWT_SECRET_KEY is not set or uses an insecure placeholder. '
'Generate a strong key with: python -c "import secrets; print(secrets.token_hex(32))" ' 'Generate a strong key with: python -c "import secrets; print(secrets.token_hex(32))" '
'and add JWT_SECRET_KEY=<value> to your production .env file.' 'and add JWT_SECRET_KEY=<value> to your production .env file.'
) )
JWT_SECRET_KEY = _jwt_secret
_cors = os.environ.get('CORS_ORIGINS', '') cors = os.environ.get('CORS_ORIGINS', '')
if not _cors or _cors.strip() == '*': if not cors or cors.strip() == '*':
raise RuntimeError( raise RuntimeError(
'[PassKeeper] CORS_ORIGINS must be set to a specific origin in production ' '[PassKeeper] CORS_ORIGINS must be set to a specific origin in production '
'(e.g. CORS_ORIGINS=https://pwkeeper.ngodanguyen.tech). ' '(e.g. CORS_ORIGINS=https://pwkeeper.ngodanguyen.tech). '
'A wildcard "*" is not permitted in production.' 'A wildcard "*" is not permitted in production.'
) )
CORS_ORIGINS = _cors
config = { config = {
'development': DevelopmentConfig, 'development': DevelopmentConfig,
'testing': TestingConfig,
'production': ProductionConfig, 'production': ProductionConfig,
} }
+14
View File
@@ -16,6 +16,10 @@ class EmergencyAccess(db.Model):
pending → grantor calls /deny → ready (reset, grantee can request again) pending → grantor calls /deny → ready (reset, grantee can request again)
pending (wait_days elapsed) → grantable (grantee fetches vault) pending (wait_days elapsed) → grantable (grantee fetches vault)
Retrieval does not change `status`: the grant stays 'pending' so the grantor
keeps seeing it as active and can revoke it. What retrieval does change is
vault_retrieved_at / vault_retrieval_count, which the grantor's UI surfaces.
Zero-knowledge: enc_vault is a JSON array of vault items re-encrypted by the grantor Zero-knowledge: enc_vault is a JSON array of vault items re-encrypted by the grantor
using the ECDH shared secret (grantor private key + grantee public key). using the ECDH shared secret (grantor private key + grantee public key).
""" """
@@ -40,6 +44,12 @@ class EmergencyAccess(db.Model):
# JSON string: [{ id, name, item_type, enc_data, iv }, ...] # JSON string: [{ id, name, item_type, enc_data, iv }, ...]
enc_vault = db.Column(db.Text, nullable=True) enc_vault = db.Column(db.Text, nullable=True)
created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc).replace(tzinfo=None), nullable=False) created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc).replace(tzinfo=None), nullable=False)
# Retrieval tracking — makes grantee access to the snapshot visible to the
# grantor. Retrieval is not blocked after the first time (the grantor may be
# unable to re-provision, which is the entire premise of emergency access);
# the wait period is the gate, and these make use of it auditable.
vault_retrieved_at = db.Column(db.DateTime, nullable=True)
vault_retrieval_count = db.Column(db.Integer, default=0, nullable=False, server_default='0')
@property @property
def wait_elapsed(self): def wait_elapsed(self):
@@ -62,6 +72,10 @@ class EmergencyAccess(db.Model):
self.request_initiated_at.isoformat() if self.request_initiated_at else None self.request_initiated_at.isoformat() if self.request_initiated_at else None
), ),
'created_at': self.created_at.isoformat() if self.created_at else None, 'created_at': self.created_at.isoformat() if self.created_at else None,
'vault_retrieved_at': (
self.vault_retrieved_at.isoformat() if self.vault_retrieved_at else None
),
'vault_retrieval_count': self.vault_retrieval_count or 0,
# True when enc_vault contains items in the old format (has a plaintext # True when enc_vault contains items in the old format (has a plaintext
# 'name' field instead of enc_name/iv_name). Grantor should re-provision. # 'name' field instead of enc_name/iv_name). Grantor should re-provision.
'enc_vault_is_legacy': self._enc_vault_is_legacy(), 'enc_vault_is_legacy': self._enc_vault_is_legacy(),
+112
View File
@@ -0,0 +1,112 @@
from datetime import datetime, timedelta, timezone
from sqlalchemy.dialects.mysql import INTEGER
from app import db
class LoginAttempt(db.Model):
"""
Failed-login tracking scoped to (account, source IP).
The lockout used to live on the users table as a single global counter, which
made it a denial-of-service primitive: anyone who knew an address could send
five wrong passwords and lock the real owner out for 15 minutes, repeatedly
and indefinitely. Locking someone out of their password manager is a serious
harm on its own — it can mean losing access to everything at the worst
possible moment — and it cost an attacker almost nothing.
Scoping by IP means an attacker locks out only themselves. The victim signing
in from their own address is unaffected. A distributed attacker has to rotate
IPs, and each one is independently capped by Flask-Limiter (10/min on
/login) plus the Nginx auth_limit zone.
users.failed_login_count / users.locked_until still exist and are still
maintained, but ONLY as an aggregate signal for the audit log and security
dashboard. They no longer gate authentication — enforcement is here.
"""
__tablename__ = 'login_attempts'
id = db.Column(INTEGER(unsigned=True), autoincrement=True, primary_key=True)
user_id = db.Column(
INTEGER(unsigned=True),
db.ForeignKey('users.id', ondelete='CASCADE'),
nullable=False,
)
# 45 chars covers IPv6; may be empty when the proxy supplies no address.
ip_address = db.Column(db.String(45), nullable=False, default='')
failed_count = db.Column(db.Integer, nullable=False, default=0, server_default='0')
locked_until = db.Column(db.DateTime, nullable=True)
updated_at = db.Column(
db.DateTime,
nullable=False,
default=lambda: datetime.now(timezone.utc).replace(tzinfo=None),
)
__table_args__ = (
db.UniqueConstraint('user_id', 'ip_address', name='uq_login_attempt_user_ip'),
)
MAX_FAILED = 5
LOCKOUT_MINUTES = 15
# Rows older than this carry no information and are pruned by the scheduler.
RETENTION_HOURS = 24
@staticmethod
def _now():
return datetime.now(timezone.utc).replace(tzinfo=None)
@classmethod
def get(cls, user_id: int, ip_address: str):
return cls.query.filter_by(
user_id=user_id, ip_address=ip_address or ''
).first()
@classmethod
def is_locked(cls, user_id: int, ip_address: str) -> bool:
"""True if this IP is currently locked out of this account."""
row = cls.get(user_id, ip_address)
if not row or not row.locked_until:
return False
if row.locked_until > cls._now():
return True
# Expired — reset so the next failure starts a fresh count.
row.failed_count = 0
row.locked_until = None
row.updated_at = cls._now()
return False
@classmethod
def record_failure(cls, user_id: int, ip_address: str) -> bool:
"""
Count a failed attempt. Returns True if this attempt triggered a lockout.
Caller commits.
"""
now = cls._now()
row = cls.get(user_id, ip_address)
if row is None:
row = cls(user_id=user_id, ip_address=ip_address or '', failed_count=0)
db.session.add(row)
row.failed_count = (row.failed_count or 0) + 1
row.updated_at = now
if row.failed_count >= cls.MAX_FAILED:
row.locked_until = now + timedelta(minutes=cls.LOCKOUT_MINUTES)
return True
return False
@classmethod
def clear(cls, user_id: int, ip_address: str) -> None:
"""Successful authentication — drop this IP's failure history."""
row = cls.get(user_id, ip_address)
if row is not None:
db.session.delete(row)
@classmethod
def cleanup_expired(cls) -> int:
"""Delete rows untouched for RETENTION_HOURS. Called by the scheduler."""
cutoff = cls._now() - timedelta(hours=cls.RETENTION_HOURS)
return cls.query.filter(cls.updated_at <= cutoff).delete()
def __repr__(self):
return f'<LoginAttempt user={self.user_id} ip={self.ip_address} n={self.failed_count}>'
+26 -8
View File
@@ -1,7 +1,5 @@
from datetime import datetime, timezone from datetime import datetime, timezone
from flask_login import UserMixin from flask_login import UserMixin
from argon2 import PasswordHasher
from argon2.exceptions import VerifyMismatchError, VerificationError, InvalidHashError
from sqlalchemy.dialects.mysql import INTEGER from sqlalchemy.dialects.mysql import INTEGER
from app import db from app import db
@@ -39,6 +37,16 @@ class User(db.Model, UserMixin):
# The server never sees the recovery code — only the ciphertext of enc_key_salt. # The server never sees the recovery code — only the ciphertext of enc_key_salt.
recovery_enc_salt = db.Column(db.String(128), nullable=True) recovery_enc_salt = db.Column(db.String(128), nullable=True)
recovery_iv = db.Column(db.String(64), nullable=True) recovery_iv = db.Column(db.String(64), nullable=True)
# recovery_verifier: 64 hex chars (256 bits), derived client-side from the
# recovery code ALONE:
# PBKDF2(recovery_code, "passkeeper-recovery-verifier:" + email, 200k, SHA-256)
# Used only as the HMAC key for the recovery challenge-response proof.
# It is deliberately independent of enc_key_salt: enc_key_salt doubles as the
# vault-key PBKDF2 salt and is handed to the client at login, so keying the
# proof with it let anyone holding the password forge a proof and pull the
# whole encrypted vault from /recovery/items without a second factor.
# NULL = legacy recovery code; the proof falls back to enc_key_salt.
recovery_verifier = db.Column(db.String(64), nullable=True)
# Brute-force lockout — incremented on every failed login attempt, # Brute-force lockout — incremented on every failed login attempt,
# reset to 0 on success. locked_until is set to now()+15min after # reset to 0 on success. locked_until is set to now()+15min after
# MAX_FAILED_LOGINS consecutive failures. # MAX_FAILED_LOGINS consecutive failures.
@@ -48,16 +56,26 @@ class User(db.Model, UserMixin):
# Each code is consumed (removed from the array) on use. # Each code is consumed (removed from the array) on use.
# NULL means no backup codes have been generated yet. # NULL means no backup codes have been generated yet.
mfa_backup_codes = db.Column(db.Text, nullable=True) mfa_backup_codes = db.Column(db.Text, nullable=True)
# Session generation counter. Every issued JWT carries the value current at
# the time it was minted; require_jwt rejects tokens whose claim no longer
# matches. Incrementing this revokes every outstanding access and refresh
# token at once, which is what a master-password change must do — otherwise
# a stolen refresh token outlives the password it was obtained under.
# Tokens issued before this column existed decode with epoch 0 and stay
# valid until the next credential change.
token_epoch = db.Column(db.Integer, default=0, nullable=False, server_default='0')
folders = db.relationship('Folder', backref='owner', lazy='dynamic', cascade='all, delete-orphan') folders = db.relationship('Folder', backref='owner', lazy='dynamic', cascade='all, delete-orphan')
vault_items = db.relationship('VaultItem', backref='owner', lazy='dynamic', cascade='all, delete-orphan') vault_items = db.relationship('VaultItem', backref='owner', lazy='dynamic', cascade='all, delete-orphan')
def check_password(self, auth_hash: str) -> bool: # NOTE: there is intentionally no check_password() here.
ph = PasswordHasher() #
try: # It existed, was called from nowhere, and used default Argon2 parameters
return ph.verify(self.master_hash, auth_hash) # with no rehash-on-login handling — so any caller that found it would have
except (VerifyMismatchError, VerificationError, InvalidHashError): # silently bypassed the transparent parameter upgrade in
return False # auth_service.verify_auth_token(). Verification goes through
# verify_auth_token(auth_hash, user.master_hash, user=user) so the stored
# hash is upgraded when ARGON2_* settings change.
def __repr__(self): def __repr__(self):
return f'<User {self.email}>' return f'<User {self.email}>'
+5 -3
View File
@@ -23,9 +23,11 @@ class VaultItem(db.Model):
user_id = db.Column(INTEGER(unsigned=True), db.ForeignKey('users.id', ondelete='CASCADE'), nullable=False) user_id = db.Column(INTEGER(unsigned=True), db.ForeignKey('users.id', ondelete='CASCADE'), nullable=False)
folder_id = db.Column(INTEGER(unsigned=True), db.ForeignKey('folders.id', ondelete='SET NULL'), nullable=True) folder_id = db.Column(INTEGER(unsigned=True), db.ForeignKey('folders.id', ondelete='SET NULL'), nullable=True)
item_type = db.Column(db.String(20), nullable=False, default=ItemType.PASSWORD.value) item_type = db.Column(db.String(20), nullable=False, default=ItemType.PASSWORD.value)
# name is stored in plaintext for display in the vault list. # NOT the user-visible name — that lives encrypted in enc_name/iv_name below.
# All other sensitive fields (username, password, URL, notes, etc.) # This column holds the item TYPE string only (the same value as item_type),
# are inside enc_data and are encrypted client-side with AES-256-GCM. # kept because the column is NOT NULL and predates enc_name. Writing a real
# item name here would hand the server plaintext the zero-knowledge model
# promises it never sees.
name = db.Column(db.String(255), nullable=False) name = db.Column(db.String(255), nullable=False)
enc_data = db.Column(db.Text, nullable=False) # base64-encoded AES-256-GCM ciphertext enc_data = db.Column(db.Text, nullable=False) # base64-encoded AES-256-GCM ciphertext
iv = db.Column(db.String(64), nullable=False) # base64-encoded 12-byte GCM nonce iv = db.Column(db.String(64), nullable=False) # base64-encoded 12-byte GCM nonce
+358 -127
View File
@@ -4,7 +4,6 @@ import time
from flask import Blueprint, request, jsonify, g from flask import Blueprint, request, jsonify, g
_log = logging.getLogger(__name__)
from app import db, limiter, client_ip from app import db, limiter, client_ip
from app.models.user import User from app.models.user import User
from app.models.audit_log import AuditLog from app.models.audit_log import AuditLog
@@ -12,6 +11,7 @@ from app.services.auth_service import (
hash_auth_token, hash_auth_token,
verify_auth_token, verify_auth_token,
generate_tokens, generate_tokens,
load_user_for_token,
generate_mfa_token, generate_mfa_token,
decode_token, decode_token,
blacklist_token, blacklist_token,
@@ -26,13 +26,112 @@ from app.services.auth_service import (
mark_totp_code_used, mark_totp_code_used,
) )
_log = logging.getLogger(__name__)
auth_bp = Blueprint('auth', __name__) auth_bp = Blueprint('auth', __name__)
EMAIL_RE = re.compile(r'^[^@\s]+@[^@\s]+\.[^@\s]+$') EMAIL_RE = re.compile(r'^[^@\s]+@[^@\s]+\.[^@\s]+$')
# Fixed-length hex validator for the client-supplied recovery verifier.
_HEX64_RE = re.compile(r'^[0-9a-f]{64}$')
def _recovery_proof_key(user) -> bytes:
"""
Return the HMAC key the recovery challenge-response is computed over.
Preferred: user.recovery_verifier — a 256-bit value derived client-side from
the recovery code alone. It is used for nothing else, so learning it grants
no decryption ability, and knowing enc_key_salt does not yield it.
Legacy fallback: user.enc_key_salt, for recovery codes created before the
verifier existed. This is weaker — enc_key_salt is also the vault-key PBKDF2
salt and is disclosed to the client on login, so anyone holding the master
password can forge a proof and pull the vault from /recovery/items without a
second factor. Accounts on this path are flagged via /recovery/status so the
settings UI can prompt the user to regenerate.
"""
if user.recovery_verifier:
return user.recovery_verifier.encode()
return user.enc_key_salt.encode()
class IncompleteReencryption(Exception):
"""
Raised when a key-rotation payload does not cover every vault item the user
owns. Rotating enc_key_salt while some ciphertext is still under the old key
renders those items permanently undecryptable, so the whole transaction is
refused unless the caller explicitly opts into partial coverage.
"""
def __init__(self, expected: int, received: int):
self.expected = expected
self.received = received
super().__init__(f'expected {expected} item(s), received {received}')
def _apply_reencrypted_items(user_id: int, items, allow_partial: bool = False) -> tuple[int, int]:
"""
Apply client-supplied re-encrypted ciphertext to the user's vault items.
Both the password-change and account-recovery flows rotate enc_key_salt,
which invalidates every ciphertext encrypted under the previous vault key.
The client is responsible for re-encrypting each item and sending it back;
any item missing from that payload is silently orphaned by the rotation.
This helper therefore counts what it actually wrote and compares it against
the number of items the user owns. On a shortfall it raises
IncompleteReencryption so the caller can roll back rather than commit a
rotation that destroys data.
allow_partial=True skips the guard. The web client only sets it after
showing the user exactly how many items will be lost and getting explicit
confirmation — it exists so a single corrupt item cannot lock someone out of
recovery entirely.
Returns (updated_count, total_count). Does not commit.
"""
from app.models.vault_item import VaultItem
total = VaultItem.query.filter_by(user_id=user_id).count()
item_ids = [i.get('id') for i in (items or []) if i.get('id')]
existing = {
v.id: v
for v in VaultItem.query.filter(
VaultItem.user_id == user_id,
VaultItem.id.in_(item_ids),
).all()
} if item_ids else {}
updated = 0
for item_data in (items or []):
item_id = item_data.get('id')
enc_data = item_data.get('enc_data', '')
iv = item_data.get('iv', '')
if not item_id or not enc_data or not iv:
continue
vault_item = existing.get(item_id)
if not vault_item:
continue
vault_item.enc_data = enc_data
vault_item.iv = iv
# Re-encrypt the name ciphertext if the client sent updated enc_name/iv_name.
if item_data.get('enc_name'):
vault_item.enc_name = item_data['enc_name']
if item_data.get('iv_name'):
vault_item.iv_name = item_data['iv_name']
updated += 1
if updated != total and not allow_partial:
raise IncompleteReencryption(expected=total, received=updated)
return updated, total
@auth_bp.route('/register', methods=['POST']) @auth_bp.route('/register', methods=['POST'])
@limiter.limit('10 per minute') @limiter.limit('5 per minute')
def register(): def register():
data = request.get_json(silent=True) or {} data = request.get_json(silent=True) or {}
email = (data.get('email') or '').strip().lower() email = (data.get('email') or '').strip().lower()
@@ -48,8 +147,46 @@ def register():
if not enc_key_salt: if not enc_key_salt:
return jsonify({'error': 'enc_key_salt is required'}), 400 return jsonify({'error': 'enc_key_salt is required'}), 400
# ── Account-existence must not be observable ────────────────────────────
#
# This used to answer 409 "Email already registered", which let anyone probe
# whether a given address has a PassKeeper account — a useful target list for
# phishing, and exactly the kind of thing a password manager should not leak.
#
# Both branches now return the identical 202 body. The wording sends the user
# to the sign-in page either way, which is the correct next step in both
# cases: registering an address that already exists is harmless because the
# user simply signs in with the password they already have.
#
# Timing has to match too. Creating an account runs Argon2id (deliberately
# slow); returning early without it would make "exists" measurably faster and
# reinstate the oracle through the side door. So the existing-account branch
# performs and discards an equivalent hash.
#
# NOTE: fully closing this needs email verification (roadmap item 4) so the
# address owner is told when someone tries to register it. Until then this
# removes the oracle but cannot notify the legitimate owner.
generic_response = jsonify({
'message': (
'If that email address was available, your account has been created. '
'Please sign in.'
)
}), 202
time.sleep(0.1) # flatten timing across both branches
if User.query.filter_by(email=email).first(): if User.query.filter_by(email=email).first():
return jsonify({'error': 'Email already registered'}), 409 hash_auth_token(auth_hash) # equalise work; result intentionally discarded
AuditLog.log(
user_id=0, # no account to attribute this to
action='auth.register_duplicate',
resource_type='user',
resource_id=None,
detail='Registration attempted for an address that already exists',
ip_address=client_ip(),
)
db.session.commit()
return generic_response
master_hash = hash_auth_token(auth_hash) master_hash = hash_auth_token(auth_hash)
user = User(email=email, master_hash=master_hash, enc_key_salt=enc_key_salt) user = User(email=email, master_hash=master_hash, enc_key_salt=enc_key_salt)
@@ -66,18 +203,15 @@ def register():
) )
db.session.commit() db.session.commit()
return jsonify({'message': 'Account created successfully'}), 201 return generic_response
@auth_bp.route('/login', methods=['POST']) @auth_bp.route('/login', methods=['POST'])
@limiter.limit('10 per minute') @limiter.limit('10 per minute')
def login(): def login():
from datetime import datetime, timezone, timedelta from datetime import datetime, timezone
from sqlalchemy.exc import OperationalError from sqlalchemy.exc import OperationalError, ProgrammingError
from app.models.login_attempt import LoginAttempt
# Number of consecutive failures before a temporary lockout is applied.
MAX_FAILED_LOGINS = 5
LOCKOUT_MINUTES = 15
data = request.get_json(silent=True) or {} data = request.get_json(silent=True) or {}
email = (data.get('email') or '').strip().lower() email = (data.get('email') or '').strip().lower()
@@ -88,73 +222,91 @@ def login():
if not email or not auth_hash: if not email or not auth_hash:
return jsonify({'error': 'Email and auth_hash are required'}), 400 return jsonify({'error': 'Email and auth_hash are required'}), 400
# ── One response for every failure mode ─────────────────────────────────
#
# Unknown account, wrong password, and locked-out must be indistinguishable.
# The lockout branch used to answer 429 "Account temporarily locked. Try
# again in N minute(s)", which confirmed the address had an account — the
# same disclosure /register was just fixed for.
#
# The trailing hint is shown for ALL of these, so it explains a lockout to
# the legitimate owner without revealing anything to someone probing.
def _reject():
return jsonify({
'error': (
'Invalid email or password. If you have made several failed '
'attempts, wait a few minutes and try again.'
)
}), 401
ip = client_ip()
user = User.query.filter_by(email=email).first() user = User.query.filter_by(email=email).first()
# Per-account lockout check. # Lockout is scoped to (account, IP) — see app/models/login_attempt.py.
# Guarded with try/except so that a deployment where the migration has not # Guarded so a deployment where the migration has not yet run degrades to
# yet been run (columns missing) degrades gracefully instead of returning # "no lockout" rather than returning an HTML 500 that breaks JSON parsing
# an HTML 500 page that breaks JSON parsing in the extension. # in the extension.
locked = False
if user:
try: try:
if user and user.locked_until: locked = LoginAttempt.is_locked(user.id, ip)
now = datetime.now(timezone.utc).replace(tzinfo=None) db.session.commit()
if user.locked_until > now: except (OperationalError, ProgrammingError):
remaining = int((user.locked_until - now).total_seconds() // 60) + 1 db.session.rollback() # table missing — migration pending
if locked:
# Do the Argon2 work anyway. Returning early would make the locked
# branch measurably faster than a wrong password and reinstate the
# existence oracle through timing.
verify_auth_token(auth_hash, user.master_hash)
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
action='auth.login_blocked', action='auth.login_blocked',
resource_type='user', resource_type='user',
resource_id=user.id, resource_id=user.id,
detail=f'Login blocked — account locked for {remaining} more minute(s)', detail='Login blocked — this IP is temporarily locked out',
ip_address=client_ip(), ip_address=ip,
) )
db.session.commit() db.session.commit()
return jsonify({ return _reject()
'error': f'Account temporarily locked. Try again in {remaining} minute(s).'
}), 429
else:
# Lockout has expired — reset the counter.
user.failed_login_count = 0
user.locked_until = None
except OperationalError:
# Columns do not exist yet — migration pending. Skip lockout check.
db.session.rollback()
if not user or not verify_auth_token(auth_hash, user.master_hash, user=user): if not user or not verify_auth_token(auth_hash, user.master_hash, user=user):
if user: if user:
try: try:
user.failed_login_count = (user.failed_login_count or 0) + 1 triggered = LoginAttempt.record_failure(user.id, ip)
if user.failed_login_count >= MAX_FAILED_LOGINS: except (OperationalError, ProgrammingError):
user.locked_until = datetime.now(timezone.utc).replace(tzinfo=None) + timedelta(minutes=LOCKOUT_MINUTES)
AuditLog.log(
user_id=user.id,
action='auth.account_locked',
resource_type='user',
resource_id=user.id,
detail=f'Account locked for {LOCKOUT_MINUTES} minutes after {user.failed_login_count} failed attempts',
ip_address=client_ip(),
)
else:
AuditLog.log(
user_id=user.id,
action='auth.login_failed',
resource_type='user',
resource_id=user.id,
detail=f'Failed login attempt — invalid password ({user.failed_login_count}/{MAX_FAILED_LOGINS})',
ip_address=client_ip(),
)
db.session.commit()
except OperationalError:
db.session.rollback() db.session.rollback()
triggered = False
# users.failed_login_count / locked_until are kept up to date purely
# as an aggregate signal for the audit log and security dashboard.
# They no longer gate authentication.
try:
user.failed_login_count = (user.failed_login_count or 0) + 1
except (OperationalError, ProgrammingError):
db.session.rollback()
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
action='auth.login_failed', action='auth.account_locked' if triggered else 'auth.login_failed',
resource_type='user', resource_type='user',
resource_id=user.id, resource_id=user.id,
detail='Failed login attempt — invalid password', detail=(
ip_address=client_ip(), f'This IP locked out for {LoginAttempt.LOCKOUT_MINUTES} minutes '
f'after {LoginAttempt.MAX_FAILED} failed attempts'
if triggered else
'Failed login attempt — invalid password'
),
ip_address=ip,
) )
db.session.commit() db.session.commit()
return jsonify({'error': 'Invalid email or password'}), 401 return _reject()
# Successful authentication — clear this IP's failure history.
try:
LoginAttempt.clear(user.id, ip)
except (OperationalError, ProgrammingError):
db.session.rollback()
# Successful authentication — reset lockout state. # Successful authentication — reset lockout state.
try: try:
@@ -175,16 +327,20 @@ def login():
) )
db.session.commit() db.session.commit()
# MFA gate: if enabled, issue a short-lived mfa_token instead of full tokens # MFA gate: if enabled, issue a short-lived mfa_token instead of full tokens.
#
# enc_key_salt is deliberately NOT returned here. It is the PBKDF2 salt for
# the vault key, and releasing it to a caller that has only cleared the
# password factor is a partial authentication result. The client receives it
# from /mfa/verify once the second factor is satisfied.
if user.totp_enabled: if user.totp_enabled:
mfa_token = generate_mfa_token(user.id) mfa_token = generate_mfa_token(user.id)
return jsonify({ return jsonify({
'mfa_required': True, 'mfa_required': True,
'mfa_token': mfa_token, 'mfa_token': mfa_token,
'enc_key_salt': user.enc_key_salt,
}), 200 }), 200
tokens = generate_tokens(user.id) tokens = generate_tokens(user.id, user.token_epoch)
return jsonify({ return jsonify({
'access_token': tokens['access_token'], 'access_token': tokens['access_token'],
'refresh_token': tokens['refresh_token'], 'refresh_token': tokens['refresh_token'],
@@ -221,9 +377,17 @@ def refresh():
except Exception: except Exception:
return jsonify({'error': 'Invalid or expired refresh token'}), 401 return jsonify({'error': 'Invalid or expired refresh token'}), 401
# Same gate as require_jwt: the account must still exist and the token's
# epoch must still match. Without this a refresh token captured before a
# password change could keep minting fresh access tokens for its full
# 7-day lifetime, defeating the revocation entirely.
user = load_user_for_token(payload)
if user is None:
return jsonify({'error': 'Session is no longer valid. Please log in again.'}), 401
# Rotate: blacklist old refresh token and issue fresh pair # Rotate: blacklist old refresh token and issue fresh pair
blacklist_token(refresh_token, 'refresh') blacklist_token(refresh_token, 'refresh')
tokens = generate_tokens(int(payload['sub'])) tokens = generate_tokens(user.id, user.token_epoch)
return jsonify({ return jsonify({
'access_token': tokens['access_token'], 'access_token': tokens['access_token'],
'refresh_token': tokens['refresh_token'], 'refresh_token': tokens['refresh_token'],
@@ -433,10 +597,12 @@ def mfa_verify():
) )
db.session.commit() db.session.commit()
tokens = generate_tokens(user.id) tokens = generate_tokens(user.id, user.token_epoch)
return jsonify({ return jsonify({
'access_token': tokens['access_token'], 'access_token': tokens['access_token'],
'refresh_token': tokens['refresh_token'], 'refresh_token': tokens['refresh_token'],
# Released here rather than at /login — both factors are now proven.
'enc_key_salt': user.enc_key_salt,
}), 200 }), 200
@@ -581,6 +747,12 @@ def change_password():
items = data.get('items', []) # [{id, enc_data, iv, enc_name?, iv_name?}, ...] items = data.get('items', []) # [{id, enc_data, iv, enc_name?, iv_name?}, ...]
sharing_private_key_enc = data.get('sharing_private_key_enc', '') sharing_private_key_enc = data.get('sharing_private_key_enc', '')
sharing_private_key_iv = data.get('sharing_private_key_iv', '') sharing_private_key_iv = data.get('sharing_private_key_iv', '')
# NOTE: there is deliberately no allow_partial opt-in here.
#
# Recovery needs one, because refusing outright leaves a locked-out user with
# no way into their account. Changing the password has no such pressure — the
# current password keeps working — so accepting data loss is never the right
# answer, and the server refuses regardless of what the client asks for.
if not current_auth_hash or not new_auth_hash or not new_enc_key_salt: if not current_auth_hash or not new_auth_hash or not new_enc_key_salt:
return jsonify({'error': 'current_auth_hash, new_auth_hash, and new_enc_key_salt are required'}), 400 return jsonify({'error': 'current_auth_hash, new_auth_hash, and new_enc_key_salt are required'}), 400
@@ -600,33 +772,9 @@ def change_password():
return jsonify({'error': 'Current password is incorrect'}), 401 return jsonify({'error': 'Current password is incorrect'}), 401
try: try:
from app.models.vault_item import VaultItem # Refuse the rotation outright unless every item was re-encrypted —
# see _apply_reencrypted_items. Raises IncompleteReencryption otherwise.
# Bulk-update all vault item ciphertexts with new vault key encryption updated, total = _apply_reencrypted_items(user.id, items)
item_ids = [i.get('id') for i in items if i.get('id')]
existing = {
v.id: v
for v in VaultItem.query.filter(
VaultItem.user_id == user.id,
VaultItem.id.in_(item_ids),
).all()
} if item_ids else {}
for item_data in items:
item_id = item_data.get('id')
enc_data = item_data.get('enc_data', '')
iv = item_data.get('iv', '')
if not item_id or not enc_data or not iv:
continue
vault_item = existing.get(item_id)
if vault_item:
vault_item.enc_data = enc_data
vault_item.iv = iv
# Re-encrypt the name ciphertext if the client sent updated enc_name/iv_name.
if item_data.get('enc_name'):
vault_item.enc_name = item_data['enc_name']
if item_data.get('iv_name'):
vault_item.iv_name = item_data['iv_name']
# Update credentials # Update credentials
user.master_hash = hash_auth_token(new_auth_hash) user.master_hash = hash_auth_token(new_auth_hash)
@@ -634,6 +782,11 @@ def change_password():
# Clear recovery data — it was encrypted with the old vault key and is now invalid # Clear recovery data — it was encrypted with the old vault key and is now invalid
user.recovery_enc_salt = None user.recovery_enc_salt = None
user.recovery_iv = None user.recovery_iv = None
user.recovery_verifier = None
# Revoke every token issued under the old password. Without this the
# "Please log in again" message below is advisory only — outstanding
# refresh tokens would stay valid for their full 7-day lifetime.
user.token_epoch = (user.token_epoch or 0) + 1
# Re-encrypt sharing private key with new vault key if the client sent it. # Re-encrypt sharing private key with new vault key if the client sent it.
# Without this update, the old ciphertext would be undecryptable after key rotation. # Without this update, the old ciphertext would be undecryptable after key rotation.
if sharing_private_key_enc and sharing_private_key_iv: if sharing_private_key_enc and sharing_private_key_iv:
@@ -645,10 +798,38 @@ def change_password():
action='auth.change_password', action='auth.change_password',
resource_type='user', resource_type='user',
resource_id=user.id, resource_id=user.id,
detail=f'Master password changed; {len(existing)} vault item(s) re-encrypted; recovery code cleared', detail=(
f'Master password changed; {updated}/{total} vault item(s) '
're-encrypted; recovery code cleared'
),
ip_address=client_ip(), ip_address=client_ip(),
) )
db.session.commit() db.session.commit()
except IncompleteReencryption as exc:
db.session.rollback()
AuditLog.log(
user_id=g.current_user_id,
action='auth.change_password_failed',
resource_type='user',
resource_id=g.current_user_id,
detail=(
f'Password change refused — re-encryption payload covered '
f'{exc.received} of {exc.expected} vault item(s)'
),
ip_address=client_ip(),
)
db.session.commit()
return jsonify({
'error': (
f'Password not changed: the re-encryption payload covered only '
f'{exc.received} of your {exc.expected} vault item(s). Completing '
'this would permanently lock the missing items. Reload the vault '
'and try again.'
),
'code': 'incomplete_reencryption',
'expected': exc.expected,
'received': exc.received,
}), 409
except Exception: except Exception:
db.session.rollback() db.session.rollback()
_log.exception('change_password failed for user %s', g.current_user_id) _log.exception('change_password failed for user %s', g.current_user_id)
@@ -727,13 +908,20 @@ def recovery_setup():
data = request.get_json(silent=True) or {} data = request.get_json(silent=True) or {}
recovery_enc_salt = data.get('recovery_enc_salt', '').strip() recovery_enc_salt = data.get('recovery_enc_salt', '').strip()
recovery_iv = data.get('recovery_iv', '').strip() recovery_iv = data.get('recovery_iv', '').strip()
# 64 lowercase hex chars, derived client-side from the recovery code alone.
recovery_verifier = (data.get('recovery_verifier') or '').strip().lower()
if not recovery_enc_salt or not recovery_iv: if not recovery_enc_salt or not recovery_iv:
return jsonify({'error': 'recovery_enc_salt and recovery_iv are required'}), 400 return jsonify({'error': 'recovery_enc_salt and recovery_iv are required'}), 400
if not recovery_verifier or not _HEX64_RE.match(recovery_verifier):
return jsonify({
'error': 'recovery_verifier must be 64 hexadecimal characters'
}), 400
user = db.session.get(User, g.current_user_id) user = db.session.get(User, g.current_user_id)
user.recovery_enc_salt = recovery_enc_salt user.recovery_enc_salt = recovery_enc_salt
user.recovery_iv = recovery_iv user.recovery_iv = recovery_iv
user.recovery_verifier = recovery_verifier
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
@@ -754,7 +942,13 @@ def recovery_setup():
def recovery_status(): def recovery_status():
"""Return whether the user has a recovery code configured.""" """Return whether the user has a recovery code configured."""
user = db.session.get(User, g.current_user_id) user = db.session.get(User, g.current_user_id)
return jsonify({'recovery_configured': bool(user.recovery_enc_salt)}), 200 return jsonify({
'recovery_configured': bool(user.recovery_enc_salt),
# True when the stored recovery code predates recovery_verifier and so
# still relies on the weaker enc_key_salt-keyed proof. The settings UI
# surfaces this as a prompt to regenerate.
'recovery_is_legacy': bool(user.recovery_enc_salt and not user.recovery_verifier),
}), 200
@auth_bp.route('/recover', methods=['POST']) @auth_bp.route('/recover', methods=['POST'])
@@ -771,7 +965,12 @@ def recover_account():
5. Client POSTs everything here in one atomic payload. 5. Client POSTs everything here in one atomic payload.
The server validates recovery_proof against the value stored in the DB The server validates recovery_proof against the value stored in the DB
during /recovery/data — enc_key_salt is never sent in plaintext. during /recovery/data — neither the recovery code nor the verifier is ever
sent in plaintext.
The re-encrypted `items` array must cover every vault item the user owns;
otherwise the rotation is refused with 409. Pass allow_partial=true to
override once the user has confirmed the resulting data loss.
The challenge row is consumed (deleted) on first use to prevent replay. The challenge row is consumed (deleted) on first use to prevent replay.
Challenge state is stored in the database, not the Flask session, so the Challenge state is stored in the database, not the Flask session, so the
flow works correctly across all Gunicorn workers. flow works correctly across all Gunicorn workers.
@@ -783,6 +982,11 @@ def recover_account():
new_enc_key_salt = data.get('new_enc_key_salt', '') new_enc_key_salt = data.get('new_enc_key_salt', '')
client_proof = data.get('recovery_proof', '') client_proof = data.get('recovery_proof', '')
items = data.get('items', []) items = data.get('items', [])
# Explicit opt-in to recovering with some items left un-re-encrypted.
# The client sets this only after telling the user how many items it could
# not decrypt and getting confirmation — without the escape hatch a single
# corrupt row would block recovery entirely.
allow_partial = bool(data.get('allow_partial'))
if not all([email, new_auth_hash, new_enc_key_salt, client_proof]): if not all([email, new_auth_hash, new_enc_key_salt, client_proof]):
return jsonify({'error': 'email, new_auth_hash, new_enc_key_salt, and recovery_proof are required'}), 400 return jsonify({'error': 'email, new_auth_hash, new_enc_key_salt, and recovery_proof are required'}), 400
@@ -813,53 +1017,69 @@ def recover_account():
try: try:
from app.models.vault_item import VaultItem # Refuse to rotate the key while items remain under the old one —
# see _apply_reencrypted_items. Raises IncompleteReencryption otherwise.
item_ids = [i.get('id') for i in items if i.get('id')] updated, total = _apply_reencrypted_items(
existing = { user.id, items, allow_partial=allow_partial
v.id: v )
for v in VaultItem.query.filter(
VaultItem.user_id == user.id,
VaultItem.id.in_(item_ids),
).all()
} if item_ids else {}
for item_data in items:
item_id = item_data.get('id')
enc_data = item_data.get('enc_data', '')
iv = item_data.get('iv', '')
if not item_id or not enc_data or not iv:
continue
vault_item = existing.get(item_id)
if vault_item:
vault_item.enc_data = enc_data
vault_item.iv = iv
if item_data.get('enc_name'):
vault_item.enc_name = item_data['enc_name']
if item_data.get('iv_name'):
vault_item.iv_name = item_data['iv_name']
user.master_hash = hash_auth_token(new_auth_hash) user.master_hash = hash_auth_token(new_auth_hash)
user.enc_key_salt = new_enc_key_salt user.enc_key_salt = new_enc_key_salt
# Recovery code is consumed — clear it so it cannot be reused. # Recovery code is consumed — clear it so it cannot be reused.
user.recovery_enc_salt = None user.recovery_enc_salt = None
user.recovery_iv = None user.recovery_iv = None
user.recovery_verifier = None
# Recovery resets the master password, so revoke prior sessions too —
# an attacker holding a stolen token must not survive the victim
# recovering their account.
user.token_epoch = (user.token_epoch or 0) + 1
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
action='auth.recovery_success', action='auth.recovery_success',
resource_type='user', resource_type='user',
resource_id=user.id, resource_id=user.id,
detail=f'Account recovered; {len(existing)} vault item(s) re-encrypted; recovery code consumed', detail=(
f'Account recovered; {updated}/{total} vault item(s) re-encrypted'
f'{" (PARTIAL — user confirmed data loss)" if updated != total else ""}; '
'recovery code consumed'
),
ip_address=client_ip(), ip_address=client_ip(),
) )
db.session.commit() db.session.commit()
except IncompleteReencryption as exc:
# The challenge was already consumed above, so the client must restart
# from /recovery/data. That is the correct trade-off: better to repeat
# the flow than to commit a rotation that orphans ciphertext.
db.session.rollback()
AuditLog.log(
user_id=user.id,
action='auth.recovery_failed',
resource_type='user',
resource_id=user.id,
detail=(
f'Recovery refused — re-encryption payload covered '
f'{exc.received} of {exc.expected} vault item(s)'
),
ip_address=client_ip(),
)
db.session.commit()
return jsonify({
'error': (
f'Recovery stopped: only {exc.received} of your {exc.expected} '
'vault item(s) could be re-encrypted. Continuing would permanently '
'lock the rest.'
),
'code': 'incomplete_reencryption',
'expected': exc.expected,
'received': exc.received,
}), 409
except Exception: except Exception:
db.session.rollback() db.session.rollback()
_log.exception('recover_account failed for user %s', user.id) _log.exception('recover_account failed for user %s', user.id)
return jsonify({'error': 'Account recovery failed. Please try again.'}), 500 return jsonify({'error': 'Account recovery failed. Please try again.'}), 500
tokens = generate_tokens(user.id) tokens = generate_tokens(user.id, user.token_epoch)
return jsonify({ return jsonify({
'message': 'Account recovered successfully', 'message': 'Account recovered successfully',
'access_token': tokens['access_token'], 'access_token': tokens['access_token'],
@@ -876,11 +1096,15 @@ def recovery_data():
Exposes: enc_key_salt, recovery_enc_salt, recovery_iv, and a one-time nonce. Exposes: enc_key_salt, recovery_enc_salt, recovery_iv, and a one-time nonce.
The nonce is used for the HMAC-SHA256 challenge-response proof: The nonce is used for the HMAC-SHA256 challenge-response proof:
- Client decrypts recovery_enc_salt → gets enc_key_salt bytes. - Client derives the recovery verifier from the recovery code:
- Client computes: proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce) PBKDF2(recovery_code, "passkeeper-recovery-verifier:" + email, 200k)
- Client computes: proof = HMAC-SHA256(key=verifier, msg=nonce)
- Server stores expected proof in the DB (recovery_challenges table), - Server stores expected proof in the DB (recovery_challenges table),
verifying it on /recover and /recovery/items without ever receiving verifying it on /recover and /recovery/items.
enc_key_salt in plaintext.
The response's `proof_scheme` field tells the client which key to use.
Accounts whose recovery code predates recovery_verifier get 'legacy' and
key the proof on the enc_key_salt decrypted out of the recovery blob.
Returns 404 if no recovery code is configured (prevents user enumeration). Returns 404 if no recovery code is configured (prevents user enumeration).
The challenge is stored in the database (not the Flask session cookie) so The challenge is stored in the database (not the Flask session cookie) so
@@ -903,7 +1127,7 @@ def recovery_data():
# enc_key_salt in plaintext. # enc_key_salt in plaintext.
nonce = generate_recovery_nonce() nonce = generate_recovery_nonce()
expected_proof = _hmac.new( expected_proof = _hmac.new(
user.enc_key_salt.encode(), _recovery_proof_key(user),
nonce.encode(), nonce.encode(),
hashlib.sha256, hashlib.sha256,
).hexdigest() ).hexdigest()
@@ -922,6 +1146,10 @@ def recovery_data():
'recovery_enc_salt': user.recovery_enc_salt, 'recovery_enc_salt': user.recovery_enc_salt,
'recovery_iv': user.recovery_iv, 'recovery_iv': user.recovery_iv,
'nonce': nonce, 'nonce': nonce,
# Tells the client which value to key the HMAC proof with:
# 'verifier' → PBKDF2(recovery_code, "passkeeper-recovery-verifier:" + email)
# 'legacy' → the enc_key_salt decrypted out of the recovery blob
'proof_scheme': 'verifier' if user.recovery_verifier else 'legacy',
}), 200 }), 200
@@ -932,11 +1160,14 @@ def recovery_items():
Return encrypted vault items for recovery re-encryption (unauthenticated). Return encrypted vault items for recovery re-encryption (unauthenticated).
Requires X-Recovery-Proof header containing the HMAC-SHA256 proof: Requires X-Recovery-Proof header containing the HMAC-SHA256 proof:
proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce_from_recovery_data) proof = HMAC-SHA256(key=recovery_verifier, msg=nonce_from_recovery_data)
The enc_key_salt used as the HMAC key is NOT returned by /recovery/data; The verifier is derived client-side from the recovery code alone and is
the client must derive it by decrypting the recovery blob with the recovery never returned by any endpoint, so only the holder of the recovery code can
code. This ensures only the holder of the recovery code can compute the proof. compute the proof. It is deliberately not enc_key_salt: that value is also
the vault-key PBKDF2 salt and is released to the client on login, so keying
the proof with it allowed anyone holding the master password to forge a
proof and pull the whole vault here — bypassing MFA.
Replay prevention: the challenge is consumed (deleted) on success, then Replay prevention: the challenge is consumed (deleted) on success, then
immediately re-issued with the same expected_proof but a new nonce and a immediately re-issued with the same expected_proof but a new nonce and a
+72 -21
View File
@@ -41,6 +41,39 @@ def list_emergency():
}), 200 }), 200
def _log_for_both(ea: EmergencyAccess, action: str, grantor_detail: str,
grantee_detail: str) -> None:
"""
Write the audit entry twice — once under each party's user_id.
/api/auth/audit-log filters by user_id, so an entry written only under the
acting user is invisible to the other party. That meant a grantee could
request access and retrieve the vault snapshot without a single line of it
appearing in the grantor's own audit log or security dashboard — the person
whose vault it was had no way to see it had happened.
Until email notifications exist (roadmap item 4), the grantor's audit log is
the only channel that reaches them, so it must carry these events.
"""
AuditLog.log(
user_id=ea.grantor_id,
action=action,
resource_type='emergency_access',
resource_id=ea.id,
detail=grantor_detail,
ip_address=client_ip(),
)
if ea.grantee_id and ea.grantee_id != ea.grantor_id:
AuditLog.log(
user_id=ea.grantee_id,
action=action,
resource_type='emergency_access',
resource_id=ea.id,
detail=grantee_detail,
ip_address=client_ip(),
)
def _ea_as_grantee(ea: EmergencyAccess, grantor: 'User | None' = None) -> dict: def _ea_as_grantee(ea: EmergencyAccess, grantor: 'User | None' = None) -> dict:
if grantor is None: if grantor is None:
grantor = db.session.get(User, ea.grantor_id) grantor = db.session.get(User, ea.grantor_id)
@@ -150,14 +183,13 @@ def accept_emergency(ea_id):
ea.status = 'accepted' ea.status = 'accepted'
ea.grantee_id = user.id ea.grantee_id = user.id
db.session.flush()
AuditLog.log( _log_for_both(
user_id=g.current_user_id, ea,
action='emergency_access.accept', 'emergency_access.accept',
resource_type='emergency_access', grantor_detail=f'{ea.grantee_email} accepted your emergency access invitation',
resource_id=ea.id, grantee_detail=f'Accepted emergency access invitation from grantor_id={ea.grantor_id}',
detail=f'Accepted emergency access invitation from grantor_id={ea.grantor_id}',
ip_address=client_ip(),
) )
db.session.commit() db.session.commit()
@@ -221,14 +253,21 @@ def request_access(ea_id):
ea.status = 'pending' ea.status = 'pending'
ea.request_initiated_at = datetime.now(timezone.utc).replace(tzinfo=None) ea.request_initiated_at = datetime.now(timezone.utc).replace(tzinfo=None)
db.session.flush()
AuditLog.log( # The grantor has `wait_days` to notice and deny this. If it only appeared in
user_id=g.current_user_id, # the grantee's audit log they would never see it in time.
action='emergency_access.request', _log_for_both(
resource_type='emergency_access', ea,
resource_id=ea.id, 'emergency_access.request',
detail=f'Requested emergency vault access from grantor_id={ea.grantor_id} (wait: {ea.wait_days}d)', grantor_detail=(
ip_address=client_ip(), f'ACTION REQUIRED: {ea.grantee_email} requested emergency access to '
f'your vault. It unlocks in {ea.wait_days} day(s) unless you deny it.'
),
grantee_detail=(
f'Requested emergency vault access from grantor_id={ea.grantor_id} '
f'(wait: {ea.wait_days}d)'
),
) )
db.session.commit() db.session.commit()
@@ -291,13 +330,25 @@ def get_emergency_vault(ea_id):
'error': f'Wait period not yet elapsed ({days_left:.1f} day(s) remaining)' 'error': f'Wait period not yet elapsed ({days_left:.1f} day(s) remaining)'
}), 403 }), 403
AuditLog.log( # Record the retrieval. Access is deliberately not revoked afterwards — the
user_id=g.current_user_id, # grantor may be unable to re-provision, and a failed import must not strand
action='emergency_access.vault_retrieved', # the grantee — but every retrieval is counted and shown to the grantor, who
resource_type='emergency_access', # can revoke the grant outright.
resource_id=ea.id, now = datetime.now(timezone.utc).replace(tzinfo=None)
detail=f'Retrieved emergency vault from grantor_id={ea.grantor_id}', is_first = ea.vault_retrieved_at is None
ip_address=client_ip(), if is_first:
ea.vault_retrieved_at = now
ea.vault_retrieval_count = (ea.vault_retrieval_count or 0) + 1
_log_for_both(
ea,
'emergency_access.vault_retrieved',
grantor_detail=(
f'{ea.grantee_email} retrieved your emergency vault snapshot '
f'({"first" if is_first else f"retrieval #{ea.vault_retrieval_count}"}). '
'Remove the grant if this was not expected.'
),
grantee_detail=f'Retrieved emergency vault from grantor_id={ea.grantor_id}',
) )
db.session.commit() db.session.commit()
+11 -2
View File
@@ -131,19 +131,28 @@ def create_share():
iv_name = data.get('iv_name') or None iv_name = data.get('iv_name') or None
# Optional expiry: number of days until the share expires (None = never). # Optional expiry: number of days until the share expires (None = never).
# Accepted values: 1, 7, 30, 90, None. # Accepted values: 1, 7, 30, 90, None.
# Fail closed: a value we cannot parse must be an error, not "never
# expires". The previous `except: pass` meant a typo or a client bug
# silently produced a permanent share — the opposite of what was asked for.
expires_days = data.get('expires_days') expires_days = data.get('expires_days')
expires_at = None expires_at = None
if expires_days is not None: if expires_days is not None:
try: try:
expires_days = int(expires_days) expires_days = int(expires_days)
except (TypeError, ValueError):
return jsonify({
'error': 'expires_days must be an integer number of days, or null for no expiry'
}), 400
if expires_days < 0 or expires_days > 3650:
return jsonify({
'error': 'expires_days must be between 0 and 3650'
}), 400
if expires_days > 0: if expires_days > 0:
from datetime import timedelta from datetime import timedelta
expires_at = ( expires_at = (
datetime.now(timezone.utc).replace(tzinfo=None) datetime.now(timezone.utc).replace(tzinfo=None)
+ timedelta(days=expires_days) + timedelta(days=expires_days)
) )
except (TypeError, ValueError):
pass
if not all([item_id, recipient_email, enc_data, iv, item_name]): if not all([item_id, recipient_email, enc_data, iv, item_name]):
return jsonify({'error': 'item_id, recipient_email, enc_data, iv, item_name are required'}), 400 return jsonify({'error': 'item_id, recipient_email, enc_data, iv, item_name are required'}), 400
+2 -2
View File
@@ -2,13 +2,13 @@ import logging
from flask import Blueprint, request, jsonify, g from flask import Blueprint, request, jsonify, g
from app import db, limiter, client_ip from app import db, limiter, client_ip
_log = logging.getLogger(__name__)
from app.models.vault_item import VaultItem, ItemType from app.models.vault_item import VaultItem, ItemType
from app.models.folder import Folder from app.models.folder import Folder
from app.models.audit_log import AuditLog from app.models.audit_log import AuditLog
from app.services.auth_service import require_jwt from app.services.auth_service import require_jwt
_log = logging.getLogger(__name__)
vault_bp = Blueprint('vault', __name__) vault_bp = Blueprint('vault', __name__)
VALID_TYPES = {t.value for t in ItemType} VALID_TYPES = {t.value for t in ItemType}
+32 -9
View File
@@ -27,6 +27,7 @@ Challenge storage:
is stored client-side (signed, not encrypted the challenge is not secret). is stored client-side (signed, not encrypted the challenge is not secret).
""" """
import json import json
import logging
from datetime import datetime, timezone from datetime import datetime, timezone
import webauthn import webauthn
@@ -50,6 +51,8 @@ from app.services.auth_service import require_jwt, generate_tokens
webauthn_bp = Blueprint('webauthn', __name__) webauthn_bp = Blueprint('webauthn', __name__)
_log = logging.getLogger(__name__)
# Session key for the pending challenge bytes. # Session key for the pending challenge bytes.
_REG_CHALLENGE_KEY = 'webauthn_reg_challenge' _REG_CHALLENGE_KEY = 'webauthn_reg_challenge'
_AUTH_CHALLENGE_KEY = 'webauthn_auth_challenge' _AUTH_CHALLENGE_KEY = 'webauthn_auth_challenge'
@@ -113,7 +116,12 @@ def register_begin():
user_display_name=user.email, user_display_name=user.email,
authenticator_selection=AuthenticatorSelectionCriteria( authenticator_selection=AuthenticatorSelectionCriteria(
resident_key=ResidentKeyRequirement.PREFERRED, resident_key=ResidentKeyRequirement.PREFERRED,
user_verification=UserVerificationRequirement.PREFERRED, # REQUIRED, not PREFERRED. A passkey here replaces BOTH the password
# and the TOTP second factor, so the authenticator must actually
# verify the human (biometric or PIN) rather than merely prove it is
# present. Under PREFERRED an authenticator is free to skip that,
# which reduced a full login to possession of an unlocked device.
user_verification=UserVerificationRequirement.REQUIRED,
authenticator_attachment=authenticator_attachment, authenticator_attachment=authenticator_attachment,
), ),
exclude_credentials=exclude_credentials, exclude_credentials=exclude_credentials,
@@ -156,10 +164,18 @@ def register_complete():
expected_challenge=expected_challenge, expected_challenge=expected_challenge,
expected_rp_id=_rp_id(), expected_rp_id=_rp_id(),
expected_origin=_origins(), expected_origin=_origins(),
require_user_verification=False, # Reject a credential created without user verification — otherwise
# the REQUIRED hint above is only a request, not a guarantee.
require_user_verification=True,
) )
except (InvalidCBORData, InvalidRegistrationResponse, Exception) as e: except (InvalidCBORData, InvalidRegistrationResponse) as e:
return jsonify({'error': f'Registration verification failed: {str(e)}'}), 400 # Never echo str(e) to the client: py-webauthn messages quote raw
# attestation internals. Log the detail, return a generic message.
_log.warning('[PassKeeper] passkey registration rejected: %s', e)
return jsonify({'error': 'Could not verify this passkey. Please try again.'}), 400
except Exception:
_log.exception('[PassKeeper] passkey registration failed unexpectedly')
return jsonify({'error': 'Could not verify this passkey. Please try again.'}), 400
# Persist the new credential. # Persist the new credential.
import base64 import base64
@@ -231,7 +247,8 @@ def authenticate_begin():
options = webauthn.generate_authentication_options( options = webauthn.generate_authentication_options(
rp_id=_rp_id(), rp_id=_rp_id(),
allow_credentials=allow_credentials, allow_credentials=allow_credentials,
user_verification=UserVerificationRequirement.PREFERRED, # See register_begin — this assertion stands in for password + MFA.
user_verification=UserVerificationRequirement.REQUIRED,
) )
session[_AUTH_CHALLENGE_KEY] = webauthn.options_to_json(options) session[_AUTH_CHALLENGE_KEY] = webauthn.options_to_json(options)
@@ -296,15 +313,21 @@ def authenticate_complete():
expected_origin=_origins(), expected_origin=_origins(),
credential_public_key=webauthn.base64url_to_bytes(credential.public_key), credential_public_key=webauthn.base64url_to_bytes(credential.public_key),
credential_current_sign_count=credential.sign_count, credential_current_sign_count=credential.sign_count,
require_user_verification=False, # Enforced, not merely requested: the assertion must carry the UV
# flag or it is not sufficient to stand in for two factors.
require_user_verification=True,
)
except Exception:
_log.warning(
'[PassKeeper] passkey assertion rejected for credential id=%s',
credential.id, exc_info=True,
) )
except Exception as e:
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
action='webauthn.auth_failed', action='webauthn.auth_failed',
resource_type='webauthn_credential', resource_type='webauthn_credential',
resource_id=credential.id, resource_id=credential.id,
detail=f'Passkey authentication failed: {str(e)[:200]}', detail='Passkey authentication failed (assertion rejected)',
ip_address=client_ip(), ip_address=client_ip(),
) )
db.session.commit() db.session.commit()
@@ -315,7 +338,7 @@ def authenticate_complete():
credential.last_used_at = datetime.now(timezone.utc).replace(tzinfo=None) credential.last_used_at = datetime.now(timezone.utc).replace(tzinfo=None)
# Issue tokens. # Issue tokens.
tokens = generate_tokens(user.id) tokens = generate_tokens(user.id, user.token_epoch)
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
+63 -7
View File
@@ -84,14 +84,23 @@ def decrypt_totp_secret(ciphertext_b64: str, iv_b64: str) -> str:
return aesgcm.decrypt(iv, ciphertext, None).decode() return aesgcm.decrypt(iv, ciphertext, None).decode()
def generate_tokens(user_id: int) -> dict: def generate_tokens(user_id: int, token_epoch: int = 0) -> dict:
"""Return access_token and refresh_token JWTs, each with a unique jti.""" """
Return access_token and refresh_token JWTs, each with a unique jti.
token_epoch stamps the user's current session generation into both tokens.
require_jwt and /refresh compare it against users.token_epoch and reject on
mismatch, so incrementing that column revokes every outstanding token.
Always pass user.token_epoch the 0 default exists only so old call sites
fail visibly in tests rather than silently minting unrevokable tokens.
"""
now = datetime.now(timezone.utc).replace(tzinfo=None) now = datetime.now(timezone.utc).replace(tzinfo=None)
secret = current_app.config['JWT_SECRET_KEY'] secret = current_app.config['JWT_SECRET_KEY']
access_payload = { access_payload = {
'sub': str(user_id), 'sub': str(user_id),
'type': 'access', 'type': 'access',
'jti': str(uuid.uuid4()), 'jti': str(uuid.uuid4()),
'epoch': int(token_epoch or 0),
'iat': now, 'iat': now,
'exp': now + current_app.config['JWT_ACCESS_TOKEN_EXPIRES'], 'exp': now + current_app.config['JWT_ACCESS_TOKEN_EXPIRES'],
} }
@@ -99,6 +108,7 @@ def generate_tokens(user_id: int) -> dict:
'sub': str(user_id), 'sub': str(user_id),
'type': 'refresh', 'type': 'refresh',
'jti': str(uuid.uuid4()), 'jti': str(uuid.uuid4()),
'epoch': int(token_epoch or 0),
'iat': now, 'iat': now,
'exp': now + current_app.config['JWT_REFRESH_TOKEN_EXPIRES'], 'exp': now + current_app.config['JWT_REFRESH_TOKEN_EXPIRES'],
} }
@@ -172,8 +182,45 @@ def blacklist_token(token: str, token_type: str) -> None:
pass # Never let blacklisting errors break the logout flow pass # Never let blacklisting errors break the logout flow
def load_user_for_token(payload) -> 'object | None':
"""
Resolve the User a validated token refers to, or None if the token must be
rejected.
Two checks beyond signature validity:
1. The user still exists. Routes immediately dereference the result of
db.session.get(User, ...); without this a valid token for a deleted
account produced an AttributeError on None and a 500.
2. The token's epoch claim still matches users.token_epoch. A master-password
change increments that column, which revokes every token minted before it.
Tokens issued before the claim existed decode as 0 and match the column
default, so an upgrade does not sign existing sessions out.
"""
from app.models.user import User
from app import db
try:
user_id = int(payload['sub'])
except (KeyError, TypeError, ValueError):
return None
user = db.session.get(User, user_id)
if user is None:
return None
if int(payload.get('epoch', 0) or 0) != int(user.token_epoch or 0):
return None
return user
def require_jwt(f): def require_jwt(f):
"""Decorator: validates Bearer token and sets g.current_user_id.""" """
Decorator: validates the Bearer token and sets g.current_user_id.
Also sets g.current_user to the resolved User so handlers can reuse it
instead of issuing a second lookup (SQLAlchemy's identity map makes the
repeat cheap, but reusing it is clearer).
"""
@wraps(f) @wraps(f)
def decorated(*args, **kwargs): def decorated(*args, **kwargs):
auth_header = request.headers.get('Authorization', '') auth_header = request.headers.get('Authorization', '')
@@ -186,7 +233,14 @@ def require_jwt(f):
return jsonify({'error': 'Token expired'}), 401 return jsonify({'error': 'Token expired'}), 401
except jwt.PyJWTError: except jwt.PyJWTError:
return jsonify({'error': 'Invalid token'}), 401 return jsonify({'error': 'Invalid token'}), 401
g.current_user_id = int(payload['sub'])
user = load_user_for_token(payload)
if user is None:
# Deleted account, or a token predating a credential change.
return jsonify({'error': 'Session is no longer valid. Please log in again.'}), 401
g.current_user = user
g.current_user_id = user.id
return f(*args, **kwargs) return f(*args, **kwargs)
return decorated return decorated
@@ -220,9 +274,11 @@ def generate_recovery_nonce() -> str:
# NOTE: compute_recovery_proof() is intentionally absent. # NOTE: compute_recovery_proof() is intentionally absent.
# The server cannot decrypt the recovery blob (it was encrypted client-side with # The server cannot decrypt the recovery blob (it was encrypted client-side with
# the user's recovery key). Instead, the expected HMAC is computed inline in # the user's recovery key). Instead, the expected HMAC is computed inline in the
# the /recovery/data route using user.enc_key_salt as the HMAC key, stored in # /recovery/data route using the key returned by _recovery_proof_key(user) —
# flask.session, and compared on submission via verify_recovery_proof() below. # user.recovery_verifier, or user.enc_key_salt for legacy codes — persisted in
# the recovery_challenges table, and compared on submission via
# verify_recovery_proof() below.
def verify_recovery_proof(expected_hmac: str, client_hmac: str) -> bool: def verify_recovery_proof(expected_hmac: str, client_hmac: str) -> bool:
+19
View File
@@ -953,6 +953,25 @@ ul {
border: 1px solid #ffb74d; border: 1px solid #ffb74d;
} }
/* Emergency access: a pending request or a retrieved snapshot is the one thing
in this list the grantor must not scroll past, so it gets the strongest
treatment available rather than the amber used for ordinary warnings. */
.badge-danger {
background: #ffebee;
color: #b71c1c;
border: 1px solid #ef9a9a;
}
.share-item.em-alert {
border-left: 3px solid #c62828;
background: #fff5f5;
}
.share-meta.em-retrieved {
color: #b71c1c;
font-weight: 500;
}
.badge-info { .badge-info {
background: #e3f2fd; background: #e3f2fd;
color: #1565c0; color: #1565c0;
+4 -6
View File
@@ -83,7 +83,6 @@ const Auth = (() => {
// Temporarily held between Step 1 and Step 2 // Temporarily held between Step 1 and Step 2
let _pendingMfaToken = null; let _pendingMfaToken = null;
let _pendingEncKeySalt = null;
let _pendingPassword = null; let _pendingPassword = null;
async function handleLogin(e) { async function handleLogin(e) {
@@ -114,11 +113,11 @@ const Auth = (() => {
} }
if (data.mfa_required) { if (data.mfa_required) {
// Step 2: collect TOTP code // Step 2: collect TOTP code.
// enc_key_salt is no longer part of this response — the server withholds
// it until the second factor is verified, so it arrives from /mfa/verify.
_pendingMfaToken = data.mfa_token; _pendingMfaToken = data.mfa_token;
_pendingEncKeySalt = data.enc_key_salt;
_pendingPassword = password; _pendingPassword = password;
sessionStorage.setItem("enc_key_salt", data.enc_key_salt);
showMfaStep(); showMfaStep();
return; return;
} }
@@ -183,7 +182,7 @@ const Auth = (() => {
await completeLogin(_pendingPassword, { await completeLogin(_pendingPassword, {
access_token: data.access_token, access_token: data.access_token,
refresh_token: data.refresh_token, refresh_token: data.refresh_token,
enc_key_salt: _pendingEncKeySalt, enc_key_salt: data.enc_key_salt,
}); });
} catch (err) { } catch (err) {
errEl.textContent = "An unexpected error occurred. Please try again."; errEl.textContent = "An unexpected error occurred. Please try again.";
@@ -228,7 +227,6 @@ const Auth = (() => {
document.getElementById("mfa-code").value = ""; document.getElementById("mfa-code").value = "";
document.getElementById("mfa-error").classList.add("hidden"); document.getElementById("mfa-error").classList.add("hidden");
_pendingMfaToken = null; _pendingMfaToken = null;
_pendingEncKeySalt = null;
_pendingPassword = null; _pendingPassword = null;
} }
+98 -12
View File
@@ -159,17 +159,54 @@ const Recover = (() => {
} }
/** /**
* Compute the HMAC-SHA256 recovery proof. * Derive the recovery verifier: 256 bits of PBKDF2 over the recovery code,
* proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce_bytes) * salted with a domain-separated label plus the user's email, as 64 hex chars.
* *
* This proves to the server that the client correctly decrypted the recovery * This is the HMAC key for the recovery challenge under the current scheme.
* blob (and therefore holds the right recovery code) without transmitting * It depends on the recovery code alone and is used for nothing else, so it
* enc_key_salt in plaintext. * cannot be derived from enc_key_salt (which the server hands to any client
* that clears the password factor).
*
* Must stay byte-identical to _deriveRecoveryVerifier() in vault.js.
*/ */
async function computeRecoveryProof(encKeySalt, nonce) { async function deriveRecoveryVerifier(recoveryCode, email) {
const baseKey = await subtle.importKey(
"raw",
strToBytes(recoveryCode),
"PBKDF2",
false,
["deriveBits"],
);
const bits = await subtle.deriveBits(
{
name: "PBKDF2",
salt: strToBytes(
"passkeeper-recovery-verifier:" + email.toLowerCase(),
),
iterations: 200_000,
hash: "SHA-256",
},
baseKey,
256,
);
return Array.from(new Uint8Array(bits))
.map((b) => b.toString(16).padStart(2, "0"))
.join("");
}
/**
* Compute the HMAC-SHA256 recovery proof: HMAC(key=proofKey, msg=nonce).
*
* proofKey is the recovery verifier for codes generated under the current
* scheme, or for codes predating it the enc_key_salt decrypted out of the
* recovery blob. The server tells us which via `proof_scheme` on
* /recovery/data. Either way the proof demonstrates possession of the
* recovery code without transmitting anything reusable.
*/
async function computeRecoveryProof(proofKey, nonce) {
const keyMaterial = await subtle.importKey( const keyMaterial = await subtle.importKey(
"raw", "raw",
strToBytes(encKeySalt), strToBytes(proofKey),
{ name: "HMAC", hash: "SHA-256" }, { name: "HMAC", hash: "SHA-256" },
false, false,
["sign"], ["sign"],
@@ -245,9 +282,15 @@ const Recover = (() => {
return; return;
} }
// Compute HMAC-SHA256 proof: proves we correctly decrypted the blob // Key the proof on the recovery verifier when the account has one.
// without sending enc_key_salt in plaintext. // Legacy accounts (recovery code created before recovery_verifier existed)
const proof = await computeRecoveryProof(decryptedEncKeySalt, data.nonce); // still key it on the decrypted enc_key_salt; regenerating the code from
// Settings migrates them.
const proofKey =
data.proof_scheme === "verifier"
? await deriveRecoveryVerifier(rawCode, email)
: decryptedEncKeySalt;
const proof = await computeRecoveryProof(proofKey, data.nonce);
// Derive the old vault key using the recovery code as master password proxy // Derive the old vault key using the recovery code as master password proxy
_oldVaultKey = await Crypto.deriveVaultKey(rawCode, decryptedEncKeySalt); _oldVaultKey = await Crypto.deriveVaultKey(rawCode, decryptedEncKeySalt);
@@ -340,9 +383,20 @@ const Recover = (() => {
}, },
); );
if (!itemsRes.ok) {
showError(
"recover-error-2",
"Could not load your vault items. Please restart the recovery process.",
);
return;
}
let reEncryptedItems = []; let reEncryptedItems = [];
if (itemsRes.ok) { let totalItems = 0;
const failedItemIds = [];
{
const itemsData = await itemsRes.json(); const itemsData = await itemsRes.json();
totalItems = itemsData.items.length;
// Re-encrypt each item: old vault key → new vault key // Re-encrypt each item: old vault key → new vault key
for (const item of itemsData.items) { for (const item of itemsData.items) {
try { try {
@@ -369,12 +423,43 @@ const Recover = (() => {
} }
reEncryptedItems.push({ id: item.id, enc_data, iv, ...encNamePayload }); reEncryptedItems.push({ id: item.id, enc_data, iv, ...encNamePayload });
} catch { } catch {
// Item decryption failed — skip (shouldn't happen if recovery code is correct) // Item decryption failed — record it. The server refuses to rotate
// the key unless every item is covered, so we must either resolve
// this or have the user explicitly accept losing these items.
failedItemIds.push(item.id);
console.warn(`Could not re-encrypt item ${item.id}`); console.warn(`Could not re-encrypt item ${item.id}`);
} }
} }
} }
// Completing recovery rotates enc_key_salt, which permanently orphans any
// item still encrypted under the old key. Unlike the change-password flow
// we cannot simply refuse — the user is locked out of their account and
// has no other way in — so we surface the exact cost and let them decide.
let allowPartial = false;
if (reEncryptedItems.length !== totalItems) {
const lost = totalItems - reEncryptedItems.length;
const proceed = confirm(
`${lost} of your ${totalItems} vault item(s) could not be decrypted ` +
`with this recovery code and cannot be carried over.
` +
`Continuing will recover your account and the other ` +
`${reEncryptedItems.length} item(s), but those ${lost} item(s) will ` +
`be permanently unreadable.
Continue with recovery?`,
);
if (!proceed) {
showError(
"recover-error-2",
"Recovery cancelled. Your account is unchanged.",
);
return;
}
allowPartial = true;
}
// Submit recovery // Submit recovery
const recoverRes = await fetch("/api/auth/recover", { const recoverRes = await fetch("/api/auth/recover", {
method: "POST", method: "POST",
@@ -385,6 +470,7 @@ const Recover = (() => {
new_enc_key_salt: newEncKeySalt, new_enc_key_salt: newEncKeySalt,
recovery_proof: _recoveryProof, recovery_proof: _recoveryProof,
items: reEncryptedItems, items: reEncryptedItems,
allow_partial: allowPartial,
}), }),
}); });
+126 -15
View File
@@ -2445,6 +2445,16 @@ const Vault = (() => {
} }
} }
/**
* Whole days left before a pending emergency request unlocks. Mirrors the
* server's wait_elapsed calculation in EmergencyAccess.wait_elapsed.
*/
function _emDaysRemaining(g) {
if (!g.request_initiated_at) return g.wait_days;
const elapsedMs = Date.now() - new Date(g.request_initiated_at).getTime();
return Math.max(0, Math.ceil(g.wait_days - elapsedMs / 86400000));
}
function renderEmergencyGrants(grants) { function renderEmergencyGrants(grants) {
const ul = document.getElementById("em-grants-list"); const ul = document.getElementById("em-grants-list");
if (!ul) return; if (!ul) return;
@@ -2471,17 +2481,32 @@ const Vault = (() => {
} }
} }
if (g.status === "pending") { if (g.status === "pending") {
// The wait period is the only thing standing between a request and
// the grantee reading the vault, so make the countdown explicit
// rather than showing a bare status word.
const waitInfo = g.wait_elapsed const waitInfo = g.wait_elapsed
? "Wait period elapsed" ? "Wait elapsed — access is available now"
: "Access requested"; : `⏳ Unlocks in ${_emDaysRemaining(g)} day(s)`;
actions = `<span class="badge badge-warn">${waitInfo}</span> actions = `<span class="badge badge-danger">${waitInfo}</span>
<button class="btn-secondary btn-sm" data-deny="${g.id}">Deny</button>`; <button class="btn-primary btn-sm" data-deny="${g.id}">Deny</button>`;
} }
return `<li class="share-item">
// Retrieval is not blocked after the wait elapses, so the grantor's
// signal that it happened is this badge plus their audit log.
const retrieved = g.vault_retrieval_count
? `<span class="share-meta em-retrieved">⚠ Vault retrieved ${
g.vault_retrieval_count
}&times; · first on ${new Date(
g.vault_retrieved_at,
).toLocaleDateString()} remove this grant if unexpected</span>`
: "";
return `<li class="share-item${g.status === "pending" || g.vault_retrieval_count ? " em-alert" : ""}">
<div class="share-icon">🚨</div> <div class="share-icon">🚨</div>
<div class="share-info"> <div class="share-info">
<span class="share-name">${escHtml(g.grantee_email)}</span> <span class="share-name">${escHtml(g.grantee_email)}</span>
<span class="share-meta">Status: ${escHtml(g.status)} · Wait: ${g.wait_days} day(s)</span> <span class="share-meta">Status: ${escHtml(g.status)} · Wait: ${g.wait_days} day(s)</span>
${retrieved}
</div> </div>
<div class="share-actions"> <div class="share-actions">
${actions} ${actions}
@@ -3554,12 +3579,17 @@ const Vault = (() => {
btn.textContent = `Re-encrypting ${items.length} item(s)…`; btn.textContent = `Re-encrypting ${items.length} item(s)…`;
const reEncrypted = []; const reEncrypted = [];
const failedIds = [];
for (const item of items) { for (const item of items) {
const plain = await Crypto.decryptItem( let plain;
vaultKey, try {
item.enc_data, plain = await Crypto.decryptItem(vaultKey, item.enc_data, item.iv);
item.iv, } catch {
); // Cannot re-encrypt what we cannot read. Collect and abort below —
// rotating the key regardless would orphan this item permanently.
failedIds.push(item.id);
continue;
}
const { enc_data, iv } = await Crypto.encryptItem(newVaultKey, plain); const { enc_data, iv } = await Crypto.encryptItem(newVaultKey, plain);
// Re-encrypt the name if it was previously encrypted. // Re-encrypt the name if it was previously encrypted.
let encNamePayload = {}; let encNamePayload = {};
@@ -3580,6 +3610,19 @@ const Vault = (() => {
reEncrypted.push({ id: item.id, enc_data, iv, ...encNamePayload }); reEncrypted.push({ id: item.id, enc_data, iv, ...encNamePayload });
} }
// Refuse to rotate unless every item was re-encrypted. Unlike recovery
// there is no lockout risk in stopping here — the current password keeps
// working — so this fails closed with no partial-completion escape hatch.
if (failedIds.length || reEncrypted.length !== items.length) {
cpError.textContent =
`Password not changed: ${failedIds.length || items.length - reEncrypted.length} of ` +
`${items.length} item(s) could not be re-encrypted. Continuing would ` +
`permanently lock them. Reload the vault and try again — if this ` +
`persists, export your vault before retrying.`;
cpError.classList.remove("hidden");
return;
}
// Re-encrypt sharing private key with new vault key so sharing stays functional. // Re-encrypt sharing private key with new vault key so sharing stays functional.
// The private key is stored as AES-GCM ciphertext on the server; rotating the // The private key is stored as AES-GCM ciphertext on the server; rotating the
// vault key without re-encrypting it would leave it permanently unreadable. // vault key without re-encrypting it would leave it permanently unreadable.
@@ -3644,7 +3687,10 @@ const Vault = (() => {
showToast("Password changed. Please log in again."); showToast("Password changed. Please log in again.");
setTimeout(() => redirectToLogin(), 1500); setTimeout(() => redirectToLogin(), 1500);
} catch (err) { } catch (err) {
cpError.textContent = "An error occurred: " + err.message; // The server runs the same completeness check and answers 409 if the
// payload was short; show its message rather than burying it.
cpError.textContent =
err.status === 409 ? err.message : "An error occurred: " + err.message;
cpError.classList.remove("hidden"); cpError.classList.remove("hidden");
console.error(err); console.error(err);
} finally { } finally {
@@ -3663,7 +3709,16 @@ const Vault = (() => {
const statusEl = document.getElementById("recovery-status-text"); const statusEl = document.getElementById("recovery-status-text");
const actionsEl = document.getElementById("recovery-actions"); const actionsEl = document.getElementById("recovery-actions");
if (data.recovery_configured) { if (data.recovery_configured && data.recovery_is_legacy) {
// Pre-verifier recovery code: its challenge proof is still keyed on
// enc_key_salt, which the server discloses at login. Regenerating
// rebinds the proof to a value derived from the recovery code alone.
statusEl.textContent =
"⚠ Your recovery code uses an outdated verification method. " +
"Generate a new one to secure it — your current code keeps working until you do.";
actionsEl.innerHTML =
'<button class="btn-primary" id="btn-regen-recovery">Generate new recovery code</button>';
} else if (data.recovery_configured) {
statusEl.textContent = statusEl.textContent =
"✅ A recovery code is configured for your account."; "✅ A recovery code is configured for your account.";
actionsEl.innerHTML = actionsEl.innerHTML =
@@ -3753,10 +3808,23 @@ const Vault = (() => {
const recovery_enc_salt = bytesToBase64(ciphertext); const recovery_enc_salt = bytesToBase64(ciphertext);
const recovery_iv = bytesToBase64(iv); const recovery_iv = bytesToBase64(iv);
// Derive the recovery verifier — the HMAC key the server uses for the
// recovery challenge-response. It comes from the recovery code alone and
// is used for nothing else, so it never doubles as key material.
// Must stay byte-identical to deriveRecoveryVerifier() in recover.js.
const recovery_verifier = await _deriveRecoveryVerifier(
recoveryCode,
userEmail,
);
// Store on server // Store on server
const res = await apiFetch("/api/auth/recovery/setup", { const res = await apiFetch("/api/auth/recovery/setup", {
method: "POST", method: "POST",
body: JSON.stringify({ recovery_enc_salt, recovery_iv }), body: JSON.stringify({
recovery_enc_salt,
recovery_iv,
recovery_verifier,
}),
}); });
if (!res) return; if (!res) return;
@@ -4532,12 +4600,55 @@ const Vault = (() => {
.catch(() => {}); .catch(() => {});
} }
/**
* Derive the recovery verifier: 256 bits of PBKDF2 over the recovery code,
* salted with a domain-separated label plus the user's email, returned as 64
* lowercase hex characters.
*
* The server stores this and keys the recovery challenge HMAC with it. It is
* deliberately independent of enc_key_salt enc_key_salt is the vault-key
* PBKDF2 salt and is disclosed to the client at login, so using it as the
* proof key let anyone with the master password forge a proof and pull the
* whole vault from /recovery/items without a second factor.
*
* recover.js has a byte-identical copy. Changing the label or iteration count
* in one place without the other invalidates every existing recovery code.
*/
async function _deriveRecoveryVerifier(recoveryCode, email) {
const baseKey = await window.crypto.subtle.importKey(
"raw",
new TextEncoder().encode(recoveryCode),
"PBKDF2",
false,
["deriveBits"],
);
const bits = await window.crypto.subtle.deriveBits(
{
name: "PBKDF2",
salt: new TextEncoder().encode(
"passkeeper-recovery-verifier:" + email.toLowerCase(),
),
iterations: 200_000,
hash: "SHA-256",
},
baseKey,
256,
);
return Array.from(new Uint8Array(bits))
.map((b) => b.toString(16).padStart(2, "0"))
.join("");
}
function escHtml(str) { function escHtml(str) {
return String(str) // &quot; and &#39; are both required: templates in this file use a mix
// of double- and single-quoted attributes, and an unescaped quote of
// either kind lets injected text break out of an attribute.
return String(str ?? "")
.replace(/&/g, "&amp;") .replace(/&/g, "&amp;")
.replace(/</g, "&lt;") .replace(/</g, "&lt;")
.replace(/>/g, "&gt;") .replace(/>/g, "&gt;")
.replace(/"/g, "&quot;"); .replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
} }
function itemIcon(type) { function itemIcon(type) {
+6 -1
View File
@@ -7,8 +7,13 @@ block body_class %}auth-page{% endblock %} {% block body %}
<span class="logo-text">PassKeeper</span> <span class="logo-text">PassKeeper</span>
</div> </div>
<!-- Deliberately non-committal: the server returns the same response whether
or not the address was already registered, so that registration cannot
be used to probe which addresses have PassKeeper accounts. Asserting
"account created" here would leak what the API withholds. -->
<p id="register-notice" class="notice-success hidden"> <p id="register-notice" class="notice-success hidden">
Account created! Please sign in. If that email address was available, your account has been created — please sign in below.
Already had an account? Sign in with your existing master password.
</p> </p>
<!-- Step 1: Email + Master Password --> <!-- Step 1: Email + Master Password -->
+10 -3
View File
@@ -113,14 +113,21 @@ async function updateBadgeForTab(tabId, url) {
return; return;
} }
let hostname; let hostname;
try { hostname = new URL(url).hostname.replace(/^www\./, ''); } catch { try { hostname = new URL(url).hostname; } catch {
chrome.browserAction.setBadgeText({ text: '', tabId }); return; chrome.browserAction.setBadgeText({ text: '', tabId }); return;
} }
// Registrable-domain comparison via the vendored PSL (loaded ahead of this
// file by manifest.firefox.json background.scripts), matching content.js
// and popup.js. Falls back to exact equality so a load failure undercounts
// rather than counting an attacker's neighbouring subdomain.
const sameSite =
typeof PkPsl !== 'undefined' && PkPsl?.isSameSite
? PkPsl.isSameSite
: (a, b) => String(a).toLowerCase() === String(b).toLowerCase();
const matches = vault_items.filter((item) => { const matches = vault_items.filter((item) => {
if (item.item_type !== 'password' || !item.plain?.url) return false; if (item.item_type !== 'password' || !item.plain?.url) return false;
try { try {
const h = new URL(item.plain.url).hostname.replace(/^www\./, ''); return sameSite(new URL(item.plain.url).hostname, hostname);
return h === hostname || h.endsWith(`.${hostname}`) || hostname.endsWith(`.${h}`);
} catch { return false; } } catch { return false; }
}); });
if (matches.length > 0) { if (matches.length > 0) {
+16 -7
View File
@@ -8,6 +8,12 @@
* - Lock the vault automatically after IDLE_LOCK_SECONDS of system inactivity. * - Lock the vault automatically after IDLE_LOCK_SECONDS of system inactivity.
*/ */
// Public Suffix List — the badge counts matching items, and must use the same
// same-site rule as content.js and popup.js. Counting a match on an attacker's
// neighbouring subdomain is itself a misleading signal, even though the badge
// alone does not disclose a credential.
importScripts("shared/psl.js");
// ── Idle lock ───────────────────────────────────────────────────────────────── // ── Idle lock ─────────────────────────────────────────────────────────────────
// Default: never lock (session clears naturally on browser close via chrome.storage.session). // Default: never lock (session clears naturally on browser close via chrome.storage.session).
@@ -119,21 +125,24 @@ async function updateBadgeForTab(tabId, url) {
let hostname; let hostname;
try { try {
hostname = new URL(url).hostname.replace(/^www\./, ""); hostname = new URL(url).hostname;
} catch { } catch {
chrome.action.setBadgeText({ text: "", tabId }); chrome.action.setBadgeText({ text: "", tabId });
return; return;
} }
// Registrable-domain comparison, matching content.js and popup.js. Falls
// back to exact equality if psl.js is unavailable — strict, so a load
// failure undercounts rather than counting an attacker's subdomain.
const sameSite =
typeof PkPsl !== "undefined" && PkPsl?.isSameSite
? PkPsl.isSameSite
: (a, b) => String(a).toLowerCase() === String(b).toLowerCase();
const matches = vault_items.filter((item) => { const matches = vault_items.filter((item) => {
if (item.item_type !== "password" || !item.plain?.url) return false; if (item.item_type !== "password" || !item.plain?.url) return false;
try { try {
const h = new URL(item.plain.url).hostname.replace(/^www\./, ""); return sameSite(new URL(item.plain.url).hostname, hostname);
return (
h === hostname ||
h.endsWith(`.${hostname}`) ||
hostname.endsWith(`.${h}`)
);
} catch { } catch {
return false; return false;
} }
+259 -29
View File
@@ -32,10 +32,17 @@
// ── Helpers ────────────────────────────────────────────────────────────────── // ── Helpers ──────────────────────────────────────────────────────────────────
function escHtml(str) { function escHtml(str) {
// &quot; and &#39; are both required: templates in this file use a mix
// of double- and single-quoted attributes, and an unescaped quote of
// either kind lets injected text break out of an attribute.
// This copy previously escaped neither, while injecting attacker-influenced
// values (site hostname, stored item names) into attributes.
return String(str ?? "") return String(str ?? "")
.replace(/&/g, "&amp;") .replace(/&/g, "&amp;")
.replace(/</g, "&lt;") .replace(/</g, "&lt;")
.replace(/>/g, "&gt;"); .replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
} }
/** /**
@@ -74,6 +81,69 @@
}; };
} }
/**
* True when this page is safe enough to put a stored credential into.
*
* The extension holds http://*\/* permission on purpose: a great many devices
* that genuinely need a password manager routers, NAS boxes, printers,
* self-hosted admin panels are only reachable over plain HTTP on the local
* network, and dropping the permission would make PassKeeper useless exactly
* where people reuse weak passwords most.
*
* What is NOT acceptable is filling a credential into a plaintext page on the
* public internet, where anyone on the path can read it. Loopback and RFC1918
* / RFC4193 / link-local addresses and .local names are treated as acceptable;
* every other http:// origin gets a warning in the dropdown before the user
* chooses an item.
*/
function _isTrustworthyOrigin() {
if (location.protocol === "https:" || location.protocol === "file:") return true;
var h = (location.hostname || "").toLowerCase().replace(/^\[|\]$/g, "");
if (h === "localhost" || h.endsWith(".localhost")) return true;
// Reserved TLDs that cannot be registered publicly.
if (/\.(local|lan|home|internal)$/.test(h)) return true;
// RFC4193 unique-local / RFC4291 link-local IPv6.
if (h === "::1") return true;
if (/^f[cd][0-9a-f]{2}:/i.test(h) || /^fe80:/i.test(h)) return true;
// IPv4 must match in FULL. Prefix checks like h.startsWith("127.") also
// accept attacker-registrable names such as "127.0.0.1.evil.com", which
// would silently suppress the insecure-page warning on a hostile site.
var m = h.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
if (!m) return false;
var o = m.slice(1).map(Number);
if (o.some(function (n) { return n > 255; })) return false;
if (o[0] === 127) return true; // loopback
if (o[0] === 10) return true; // RFC1918
if (o[0] === 192 && o[1] === 168) return true; // RFC1918
if (o[0] === 172 && o[1] >= 16 && o[1] <= 31) return true; // RFC1918
if (o[0] === 169 && o[1] === 254) return true; // link-local
return false;
}
/**
* Prepend an unmissable warning to the dropdown on plaintext public pages.
* Deliberately a warning and not a block: filling is always user-initiated,
* and silently offering nothing would look like a broken extension.
*/
function _insecureWarningRow() {
if (_isTrustworthyOrigin()) return null;
var row = document.createElement("div");
Object.assign(row.style, {
padding: "8px 12px",
background: "#fdecea",
color: "#b71c1c",
borderBottom: "1px solid #f5c6cb",
fontSize: "12px",
lineHeight: "1.35",
});
row.textContent =
"⚠ This page is not encrypted (http://). A credential filled here " +
"can be read by anyone on the network.";
return row;
}
function visiblePasswordFields() { function visiblePasswordFields() {
return Array.from( return Array.from(
document.querySelectorAll('input[type="password"]'), document.querySelectorAll('input[type="password"]'),
@@ -99,8 +169,21 @@
if (["username", "email", "tel"].includes(ac)) return true; if (["username", "email", "tel"].includes(ac)) return true;
// Definite negative signals (Chrome's autocomplete token set). // Definite negative signals (Chrome's autocomplete token set).
//
// "off" is deliberately NOT in this list. It says nothing about whether a
// field holds a credential — routers, banks and admin panels set it on
// login inputs precisely to discourage password managers, and every major
// password manager (and Chrome itself, for password fields) ignores it.
// Treating it as a negative signal meant a field as obvious as
// <input type="text" id="login_username" placeholder="Username"
// autocomplete="off">
// was rejected before the keyword check below ever ran.
//
// Letting "off" fall through is safe: the field still has to carry a
// credential keyword AND sit near a password input (_hasPasswordSibling)
// before it is decorated.
const NON_CRED_AC = const NON_CRED_AC =
/^(name|given-name|family-name|additional-name|honorific-prefix|honorific-suffix|organization|street-address|address-line[123]|address-level[1234]|country|country-name|postal-code|cc-|transaction-|language|bday|sex|url|photo|search|new-password|current-password|one-time-code|off)$/i; /^(name|given-name|family-name|additional-name|honorific-prefix|honorific-suffix|organization|street-address|address-line[123]|address-level[1234]|country|country-name|postal-code|cc-|transaction-|language|bday|sex|url|photo|search|new-password|current-password|one-time-code)$/i;
if (ac && NON_CRED_AC.test(ac)) return false; if (ac && NON_CRED_AC.test(ac)) return false;
// Check name, id, placeholder, and aria-label for credential keywords. // Check name, id, placeholder, and aria-label for credential keywords.
@@ -218,24 +301,65 @@
el.style.outline = ""; el.style.outline = "";
}, 1500); }, 1500);
}); });
// Autologin: submit the form automatically after filling. // Autologin: submit automatically after filling.
if (autologin) { if (autologin) {
const form = pwField.closest("form");
if (form) {
setTimeout(function () { setTimeout(function () {
// Prefer clicking a visible submit button so site-specific submit var form = pwField.closest("form");
// handlers (React, Vue, etc.) fire correctly. // Prefer clicking a real control so site-specific handlers (React, Vue,
var submitBtn = form.querySelector( // inline onclick) fire — form.submit() bypasses them entirely.
'[type="submit"]:not([disabled])', var control = _findSubmitControl(pwField);
); if (control) {
if (submitBtn) { control.click();
submitBtn.click(); } else if (form) {
} else {
form.submit(); form.submit();
} }
}, 400); }, 400);
} }
} }
// Controls that look like submits but would discard the login instead.
var _NEGATIVE_CONTROL = /cancel|reset|back|close|forgot|register|sign\s*up|create/i;
/**
* Find the control that submits the login containing `pwField`.
*
* Autologin previously required a <form> and did nothing without one, so it
* silently never worked on the many login UIs built from plain divs (the ASUS
* router admin page submits with
* <div class="button" onclick="preLogin();">Sign In</div>).
*
* Returns null when nothing convincing is found better to leave the filled
* form for the user than to click the wrong thing.
*/
function _findSubmitControl(pwField) {
var scope = pwField.closest("form") || pwField.closest('[role="form"]');
if (scope) {
var explicit = scope.querySelector('[type="submit"]:not([disabled])');
if (explicit && isVisible(explicit)) return explicit;
}
// No form (or no explicit submit in it): search progressively wider
// ancestors so the nearest plausible control wins.
var node = scope || pwField.parentElement;
for (var depth = 0; depth < 5 && node; depth++, node = node.parentElement) {
var candidates = node.querySelectorAll(
'button, [role="button"], [onclick], input[type="submit"], ' +
'input[type="button"], div, a',
);
for (var i = 0; i < candidates.length; i++) {
var el = candidates[i];
if (el === pwField || el.disabled) continue;
if (!_looksLikeSubmitControl(el)) continue;
if (!isVisible(el)) continue;
// Only leaf-ish controls — a wrapping div can carry a button class.
if (el.querySelector("input, button")) continue;
var label = (el.textContent || el.value || "").trim();
if (label.length > 40) continue;
if (_NEGATIVE_CONTROL.test(label)) continue;
return el;
}
}
return null;
} }
// ── Icon button (fixed-position, outside the DOM tree of the field) ─────────── // ── Icon button (fixed-position, outside the DOM tree of the field) ───────────
@@ -305,6 +429,9 @@
overflow: "hidden", overflow: "hidden",
}); });
var warning = _insecureWarningRow();
if (warning) dropdown.appendChild(warning);
if (panel === "more") { if (panel === "more") {
buildMorePanel(dropdown, anchorField, pwField, freshItems, filterText); buildMorePanel(dropdown, anchorField, pwField, freshItems, filterText);
} else { } else {
@@ -922,17 +1049,39 @@
return "https://" + s; // bare domain or path return "https://" + s; // bare domain or path
} }
/**
* Select the stored items whose URL belongs to the page we are on.
*
* Matching is by registrable domain (PkPsl.isSameSite), NOT by suffix
* comparison. The previous test was:
*
* h === host || h.endsWith("." + host) || host.endsWith("." + h)
*
* which had no notion of a public suffix, so a credential saved for
* victim.github.io was offered on evil.github.io, and one saved for a bare
* TLD was offered everywhere under it. Surfacing a match on an attacker's
* neighbouring subdomain defeats the phishing resistance that is most of the
* point of a password manager.
*
* If psl.js somehow failed to load we fall back to exact hostname equality
* strict, so a load failure loses matches rather than leaking credentials.
*/
function _filterForHost(items) { function _filterForHost(items) {
var host = location.hostname.replace(/^www\./, ""); var host = location.hostname;
var sameSite =
typeof PkPsl !== "undefined" && PkPsl && PkPsl.isSameSite
? PkPsl.isSameSite
: function (a, b) {
return String(a).toLowerCase() === String(b).toLowerCase();
};
return (items || []).filter(function (item) { return (items || []).filter(function (item) {
if (item.item_type !== "password" || !(item.plain && item.plain.url)) if (item.item_type !== "password" || !(item.plain && item.plain.url))
return false; return false;
try { try {
var normalised = _normaliseUrl(item.plain.url); var normalised = _normaliseUrl(item.plain.url);
if (!normalised) return false; if (!normalised) return false;
var h = new URL(normalised).hostname.replace(/^www\./, ""); return sameSite(new URL(normalised).hostname, host);
// Match exact domain or any subdomain relationship.
return h === host || h.endsWith("." + host) || host.endsWith("." + h);
} catch (e) { } catch (e) {
console.warn( console.warn(
"[PassKeeper] _filterForHost: could not parse URL:", "[PassKeeper] _filterForHost: could not parse URL:",
@@ -1112,28 +1261,44 @@
}); });
} }
// ── Form submission watch ───────────────────────────────────────────────────── // ── Credential capture / save prompt ─────────────────────────────
function watchSubmissions() { // Guards against two triggers firing for the same login (e.g. a click handler
document.addEventListener( // that also submits a form). Cleared after a short window.
"submit", var _captureCooldown = false;
async function (e) {
var form = e.target; /**
var pwField = form.querySelector( * Collect the credentials currently entered within `scope` and offer to save
'input[type="password"]:not([disabled])', * them. `scope` is the <form> for a real submit, or the document for the
* fallback triggers below.
*/
async function maybeCaptureCredentials(scope) {
if (_captureCooldown) return;
var root = scope || document;
var pwField = Array.prototype.find.call(
root.querySelectorAll('input[type="password"]:not([disabled])'),
function (el) {
return isVisible(el) && el.value;
},
); );
if (!pwField || !pwField.value) return; if (!pwField) return;
var userField = var userField =
findUsernameField(pwField) || findUsernameField(pwField) ||
form.querySelector('input[type="email"]:not([disabled])') || root.querySelector('input[type="email"]:not([disabled])') ||
form.querySelector('input[type="text"]:not([disabled])'); root.querySelector('input[type="text"]:not([disabled])');
var username = var username =
(userField && userField.value && userField.value.trim()) || ""; (userField && userField.value && userField.value.trim()) || "";
var password = pwField.value; var password = pwField.value;
if (!username || !password) return; if (!username || !password) return;
_captureCooldown = true;
setTimeout(function () {
_captureCooldown = false;
}, 2000);
removeDropdown(); removeDropdown();
// Check blocklist before doing anything else. // Check blocklist before doing anything else.
@@ -1149,7 +1314,7 @@
console.log( console.log(
"[PassKeeper] Credential state for", "[PassKeeper] Credential state for",
location.hostname, location.hostname,
"\u2192", "→",
credentialState, credentialState,
); );
if (credentialState === "same") return; if (credentialState === "same") return;
@@ -1157,6 +1322,71 @@
setTimeout(function () { setTimeout(function () {
showSaveBanner(username, password, credentialState); showSaveBanner(username, password, credentialState);
}, 500); }, 500);
}
/**
* Does this element look like the control that submits a login?
*
* Needed because many login UIs never use a <form> at all the ASUS router
* admin page, for example, submits with
* <div class="button" onclick="preLogin();">Sign In</div>
* so no "submit" event is ever dispatched and the save prompt never appeared.
*/
function _looksLikeSubmitControl(el) {
if (!el || !el.tagName) return false;
var tag = el.tagName.toUpperCase();
if (tag === "BUTTON") return true;
if (tag === "INPUT" && /^(submit|button|image)$/i.test(el.type)) return true;
if (el.getAttribute("role") === "button") return true;
// Non-semantic controls: an inline click handler, or a button-ish class.
if (el.hasAttribute("onclick")) return true;
var cls = (el.getAttribute("class") || "").toLowerCase();
return /(^|[\s_-])(btn|button|submit|login|signin|sign-in)([\s_-]|$)/.test(cls);
}
function watchSubmissions() {
// 1. Real form submits.
document.addEventListener(
"submit",
function (e) {
maybeCaptureCredentials(e.target);
},
true,
);
// 2. Clicks on anything that looks like a submit control. Required for the
// common case of a login UI built without a <form>, where no submit
// event fires and the save prompt would otherwise never appear.
document.addEventListener(
"click",
function (e) {
var node = e.target;
for (var i = 0; i < 5 && node && node !== document; i++) {
if (_looksLikeSubmitControl(node)) {
// Let the page's own handler run first.
setTimeout(function () {
maybeCaptureCredentials(document);
}, 0);
return;
}
node = node.parentElement;
}
},
true,
);
// 3. Enter pressed inside a credential field — the other way such forms
// get submitted without a <form>.
document.addEventListener(
"keydown",
function (e) {
if (e.key !== "Enter") return;
var el = e.target;
if (!el || el.tagName !== "INPUT") return;
if (el.type !== "password" && !_isLikelyUsernameField(el)) return;
setTimeout(function () {
maybeCaptureCredentials(document);
}, 0);
}, },
true, true,
); );
+20 -5
View File
@@ -21,18 +21,33 @@
} }
}, },
"background": { "background": {
"scripts": ["background.firefox.js"], "scripts": [
"shared/psl.js",
"background.firefox.js"
],
"persistent": false "persistent": false
}, },
"content_scripts": [ "content_scripts": [
{ {
"matches": ["http://*/*", "https://*/*"], "matches": [
"js": ["shared/browser-polyfill.js", "content/content.js"], "http://*/*",
"https://*/*"
],
"js": [
"shared/browser-polyfill.js",
"shared/psl.js",
"content/content.js"
],
"run_at": "document_idle" "run_at": "document_idle"
}, },
{ {
"matches": ["https://pwkeeper.ngodanguyen.tech/*"], "matches": [
"js": ["shared/browser-polyfill.js", "bridge/bridge.js"], "https://pwkeeper.ngodanguyen.tech/*"
],
"js": [
"shared/browser-polyfill.js",
"bridge/bridge.js"
],
"run_at": "document_idle" "run_at": "document_idle"
} }
], ],
+4 -1
View File
@@ -32,6 +32,7 @@
"https://*/*" "https://*/*"
], ],
"js": [ "js": [
"shared/psl.js",
"content/content.js" "content/content.js"
], ],
"run_at": "document_idle" "run_at": "document_idle"
@@ -49,7 +50,9 @@
"web_accessible_resources": [ "web_accessible_resources": [
{ {
"resources": [], "resources": [],
"matches": ["<all_urls>"] "matches": [
"<all_urls>"
]
} }
], ],
"icons": { "icons": {
+3
View File
@@ -621,6 +621,9 @@
</nav> </nav>
</div> </div>
<!-- /app --> <!-- /app -->
<!-- Public Suffix List — must load before popup.js, which calls PkPsl
from isMatch() to decide which stored items belong to the active tab. -->
<script src="../shared/psl.js"></script>
<script src="../shared/crypto.js"></script> <script src="../shared/crypto.js"></script>
<script src="../shared/sharing-crypto.js"></script> <script src="../shared/sharing-crypto.js"></script>
<script src="popup.js"></script> <script src="popup.js"></script>
+24 -14
View File
@@ -60,11 +60,15 @@ function hideError(elId) {
} }
function escHtml(str) { function escHtml(str) {
// &quot; and &#39; are both required: templates in this file use a mix of
// double- and single-quoted attributes, and an unescaped quote of either
// kind lets injected text break out of an attribute.
return String(str ?? "") return String(str ?? "")
.replace(/&/g, "&amp;") .replace(/&/g, "&amp;")
.replace(/</g, "&lt;") .replace(/</g, "&lt;")
.replace(/>/g, "&gt;") .replace(/>/g, "&gt;")
.replace(/"/g, "&quot;"); .replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
} }
// Auto-clear clipboard 30 s after a sensitive copy. // Auto-clear clipboard 30 s after a sensitive copy.
@@ -220,15 +224,28 @@ function _normaliseUrl(raw) {
return "https://" + s; return "https://" + s;
} }
/**
* Does this stored item belong to the site in the active tab?
*
* Compared by registrable domain (PkPsl.isSameSite), not by suffix. The old
* test had no notion of a public suffix, so victim.github.io matched
* evil.github.io and an item saved for a bare TLD matched every site under it.
* Must stay in step with _filterForHost() in content/content.js.
*
* Falls back to exact hostname equality if psl.js failed to load strict, so a
* load failure loses matches rather than leaking credentials.
*/
function isMatch(item) { function isMatch(item) {
const host = currentHostname(); const host = currentHostname();
if (!host || item.item_type !== "password" || !item.plain?.url) return false; if (!host || item.item_type !== "password" || !item.plain?.url) return false;
try { try {
const normalised = _normaliseUrl(item.plain.url); const normalised = _normaliseUrl(item.plain.url);
if (!normalised) return false; if (!normalised) return false;
const h = new URL(normalised).hostname.replace(/^www\./, ""); const itemHost = new URL(normalised).hostname;
// Match exact domain or any subdomain relationship. if (typeof PkPsl !== "undefined" && PkPsl?.isSameSite) {
return h === host || h.endsWith(`.${host}`) || host.endsWith(`.${h}`); return PkPsl.isSameSite(itemHost, host);
}
return itemHost.toLowerCase() === String(host).toLowerCase();
} catch { } catch {
return false; return false;
} }
@@ -321,10 +338,9 @@ async function handleLogin() {
} }
if (data.mfa_required) { if (data.mfa_required) {
// enc_key_salt is withheld by the server until the second factor is
// verified — it now arrives with the /mfa/verify response instead.
_mfaToken = data.mfa_token; _mfaToken = data.mfa_token;
await chrome.storage.session.set({
_pending_enc_key_salt: data.enc_key_salt,
});
handleMfaStage(password); handleMfaStage(password);
// Reset MFA view to TOTP mode each time it's shown // Reset MFA view to TOTP mode each time it's shown
$("mfa-totp-section").classList.remove("hidden"); $("mfa-totp-section").classList.remove("hidden");
@@ -385,13 +401,7 @@ function handleMfaStage(password) {
return; return;
} }
_mfaToken = null; _mfaToken = null;
const { _pending_enc_key_salt } = await chrome.storage.session.get( await completeLogin(data, password);
"_pending_enc_key_salt",
);
await completeLogin(
{ ...data, enc_key_salt: _pending_enc_key_salt },
password,
);
} catch (err) { } catch (err) {
showError("mfa-error", "Error: " + err.message); showError("mfa-error", "Error: " + err.message);
} finally { } finally {
File diff suppressed because it is too large Load Diff
+148
View File
@@ -0,0 +1,148 @@
"""
gunicorn.conf.py Gunicorn runtime configuration for PassKeeper.
Loaded explicitly by the systemd unit:
ExecStart=/home/spuser/.venv/bin/gunicorn -c /home/spuser/PassKeeper/gunicorn.conf.py wsgi:app
Every value can be overridden from the environment (systemd reads
EnvironmentFile=/home/spuser/PassKeeper/.env), so tuning a production box does
not require editing this file or the unit.
Timeout ordering (this is what causes 502s)
Nginx must give up BEFORE Gunicorn kills the worker:
nginx proxy_read_timeout < gunicorn timeout
If Gunicorn kills first, the connection is severed mid-response and Nginx
reports 502 Bad Gateway. If Nginx gives up first, the client gets a clean
504 Gateway Timeout instead. `timeout` below is therefore set comfortably
above the proxy_read_timeout in scripts/passkeeper-nginx.conf raise this
value first if you ever raise that one.
"""
import multiprocessing
import os
def _env_int(name: str, default: int) -> int:
"""Read an int from the environment, falling back on unset/garbage values."""
try:
return int(os.environ.get(name, default))
except (TypeError, ValueError):
return default
# ── Socket ───────────────────────────────────────────────────────────────────
# Loopback only — Nginx is the sole public entry point and terminates TLS.
bind = os.environ.get('GUNICORN_BIND', '127.0.0.1:5000')
backlog = 2048
# ── Worker processes ─────────────────────────────────────────────────────────
# gthread rather than the default sync worker.
#
# Login is the expensive path: Argon2id at ARGON2_MEMORY_COST=65536 (64 MB) with
# parallelism=4 blocks a sync worker completely for the duration of the hash, and
# a sync worker serves exactly one request at a time. A handful of concurrent
# logins was enough to saturate all workers, queue everything behind them, and
# push requests past the worker timeout — which surfaces as 502.
#
# Threads let a worker keep serving while another request is blocked, and make
# the `keepalive` setting below meaningful (sync workers ignore keep-alive
# entirely, so Nginx's `proxy_http_version 1.1` was a no-op against them).
worker_class = 'gthread'
# Memory, not CPU, is the binding constraint: each in-flight Argon2id hash claims
# 64 MB. Keep workers modest and add concurrency with threads instead.
workers = _env_int('GUNICORN_WORKERS', min(4, multiprocessing.cpu_count() + 1))
threads = _env_int('GUNICORN_THREADS', 4)
# ── Timeouts ─────────────────────────────────────────────────────────────────
# Must exceed nginx proxy_read_timeout — see the module docstring.
# Long enough to cover the genuinely slow flows: bulk vault import/export and
# the atomic re-encryption of every item during a password change or recovery.
timeout = _env_int('GUNICORN_TIMEOUT', 90)
# Let in-flight requests finish on reload/restart rather than cutting them off.
graceful_timeout = _env_int('GUNICORN_GRACEFUL_TIMEOUT', 30)
# Idle keep-alive window for connections from Nginx. Slightly above Nginx's
# keepalive_timeout (15s) so Gunicorn is never the side that closes first —
# a connection closed underneath Nginx as it reuses it is a classic 502.
keepalive = _env_int('GUNICORN_KEEPALIVE', 20)
# ── Worker recycling ─────────────────────────────────────────────────────────
# Recycle workers periodically to bound the impact of any slow memory growth.
# The jitter staggers restarts so workers never all recycle at once (which would
# briefly leave nothing to serve — another source of intermittent 502s).
max_requests = _env_int('GUNICORN_MAX_REQUESTS', 1000)
max_requests_jitter = _env_int('GUNICORN_MAX_REQUESTS_JITTER', 100)
# Heartbeat file location. The default (/tmp) is disk-backed on many systems,
# and a slow disk makes the arbiter believe healthy workers have died and kill
# them. /dev/shm is always tmpfs. The unit sets PrivateTmp=true, which does not
# cover /dev/shm, so this stays writable.
if os.path.isdir('/dev/shm'):
worker_tmp_dir = '/dev/shm'
# ── Application loading ──────────────────────────────────────────────────────
# preload_app MUST stay False.
#
# create_app() starts an APScheduler BackgroundScheduler for the hourly cleanup
# of token_blacklist / recovery_challenges / totp_used_codes / expired shares.
# With preload_app=True the app is built once in the arbiter and then forked —
# and threads do not survive fork(), so the scheduler thread would exist only in
# the arbiter, which serves no requests. The cleanup job would silently never
# run and those tables would grow without bound.
#
# Note: Deloy.md's "test Gunicorn manually" step suggests --preload. That is fine
# for a one-off smoke test, but must never reach the service definition.
#
# The cost of preload_app=False is one scheduler per worker, so the cleanup runs
# `workers` times an hour instead of once. The job only DELETEs already-expired
# rows, so it is idempotent and the redundancy is harmless.
preload_app = False
# ── Proxy ────────────────────────────────────────────────────────────────────
# Only trust X-Forwarded-* from the local Nginx. ProxyFix(x_for=1) in the app
# factory does the actual header parsing; this stops Gunicorn honouring
# forwarded headers from anything else.
forwarded_allow_ips = os.environ.get('GUNICORN_FORWARDED_ALLOW_IPS', '127.0.0.1')
# ── Request limits ───────────────────────────────────────────────────────────
# Defence in depth behind Nginx's client_max_body_size 1m.
limit_request_line = 8190
limit_request_fields = 100
limit_request_field_size = 8190
# ── Logging ──────────────────────────────────────────────────────────────────
# Paths must be inside the unit's ReadWritePaths= or Gunicorn cannot start.
accesslog = os.environ.get('GUNICORN_ACCESS_LOG', '/home/spuser/logs/access.log')
errorlog = os.environ.get('GUNICORN_ERROR_LOG', '/home/spuser/logs/error.log')
loglevel = os.environ.get('GUNICORN_LOG_LEVEL', 'warning')
# %({X-Forwarded-For}i)s rather than %(h)s — %(h)s would log Nginx's loopback
# address for every request. Never log Authorization headers or request bodies:
# they carry bearer tokens and auth_hash values.
access_log_format = (
'%({X-Forwarded-For}i)s %(l)s %(u)s %(t)s "%(r)s" %(s)s %(b)s "%(f)s" "%(a)s" %(D)sus'
)
proc_name = 'passkeeper'
# ── Hooks ────────────────────────────────────────────────────────────────────
def on_starting(server):
server.log.info(
'[PassKeeper] starting: %s worker(s) x %s thread(s), class=%s, timeout=%ss',
workers, threads, worker_class, timeout,
)
def worker_abort(worker):
"""Fires when a worker is killed for exceeding `timeout`."""
worker.log.error(
'[PassKeeper] worker %s aborted after %ss — a request exceeded the '
'timeout. Nginx will have reported 502 to the client.',
worker.pid, timeout,
)
@@ -0,0 +1,47 @@
"""add recovery_verifier to users
Revision ID: j0k1l2m3n4o5
Revises: i9j0k1l2m3n4
Create Date: 2026-08-26 00:00:00.000000
Decouples the account-recovery challenge-response proof from enc_key_salt.
Before this change the recovery proof was HMAC-SHA256(key=enc_key_salt, msg=nonce).
Because enc_key_salt is also the PBKDF2 salt for the vault key and is handed to
the client at login, anyone who learned enc_key_salt could forge a recovery proof
and pull the entire encrypted vault from the unauthenticated /recovery/items
endpoint bypassing MFA entirely.
recovery_verifier is an independent 256-bit value derived client-side from the
recovery code alone:
verifier = PBKDF2(recovery_code, "passkeeper-recovery-verifier:" + email,
200_000 iter, SHA-256) 64 hex chars
It is stored server-side purely as the HMAC key for the recovery challenge, and
is never used for any encryption. Knowing enc_key_salt no longer grants the
ability to forge a proof.
NULL = legacy recovery code (created before this migration). Those accounts fall
back to the old enc_key_salt-keyed proof so existing recovery codes keep working;
the settings UI prompts the user to regenerate.
"""
from alembic import op
import sqlalchemy as sa
revision = 'j0k1l2m3n4o5'
down_revision = 'i9j0k1l2m3n4'
branch_labels = None
depends_on = None
def upgrade():
op.add_column(
'users',
sa.Column('recovery_verifier', sa.String(64), nullable=True),
)
def downgrade():
op.drop_column('users', 'recovery_verifier')
@@ -0,0 +1,50 @@
"""add token_epoch to users
Revision ID: k1l2m3n4o5p6
Revises: j0k1l2m3n4o5
Create Date: 2026-08-26 00:00:00.000000
Adds a monotonic session-generation counter so credential changes can revoke
every token issued before them.
Previously, changing the master password left all outstanding access and refresh
tokens valid a stolen refresh token kept working for its full 7-day lifetime
after the victim changed their password. The response said "Please log in again"
but nothing enforced it.
Every JWT now carries an `epoch` claim. require_jwt (and /refresh) compare it
against users.token_epoch and reject on mismatch. change_password and /recover
increment the column, which invalidates every previously issued token at once.
A counter rather than a timestamp: JWT `iat` has one-second granularity, so a
token minted in the same second as the password change could otherwise slip
through the comparison.
Existing tokens predate the claim and decode with epoch 0, which matches the
server_default so deploying this does not sign everyone out. The first
password change moves them to 1 and invalidates them as intended.
"""
from alembic import op
import sqlalchemy as sa
revision = 'k1l2m3n4o5p6'
down_revision = 'j0k1l2m3n4o5'
branch_labels = None
depends_on = None
def upgrade():
op.add_column(
'users',
sa.Column(
'token_epoch',
sa.Integer,
nullable=False,
server_default='0',
),
)
def downgrade():
op.drop_column('users', 'token_epoch')
@@ -0,0 +1,49 @@
"""add vault retrieval tracking to emergency_access
Revision ID: l2m3n4o5p6q7
Revises: k1l2m3n4o5p6
Create Date: 2026-08-26 00:00:00.000000
Makes emergency-vault retrieval visible to the grantor.
Previously GET /api/emergency/<id>/vault neither changed the record nor recorded
anything the grantor could see: the audit entry was written under the *grantee's*
user_id, and /api/auth/audit-log filters by user_id, so it never appeared in the
grantor's own log. Combined with the status never advancing past 'pending', a
grantee could re-fetch the snapshot indefinitely with nothing surfacing to the
person whose vault it was.
vault_retrieved_at when the snapshot was FIRST retrieved (NULL = never)
vault_retrieval_count how many times, so repeated access is visible
Retrieval is deliberately NOT blocked after the first time: the whole premise of
emergency access is that the grantor may be unable to re-provision, and a browser
crash mid-import must not permanently strand the grantee. The wait period remains
the gate; these columns plus the dual audit entries make use of that access
auditable, and the grantor can still revoke with DELETE at any point.
"""
from alembic import op
import sqlalchemy as sa
revision = 'l2m3n4o5p6q7'
down_revision = 'k1l2m3n4o5p6'
branch_labels = None
depends_on = None
def upgrade():
op.add_column(
'emergency_access',
sa.Column('vault_retrieved_at', sa.DateTime, nullable=True),
)
op.add_column(
'emergency_access',
sa.Column('vault_retrieval_count', sa.Integer,
nullable=False, server_default='0'),
)
def downgrade():
op.drop_column('emergency_access', 'vault_retrieval_count')
op.drop_column('emergency_access', 'vault_retrieved_at')
@@ -0,0 +1,52 @@
"""add login_attempts table (per-IP lockout)
Revision ID: m3n4o5p6q7r8
Revises: l2m3n4o5p6q7
Create Date: 2026-08-26 00:00:00.000000
Moves failed-login lockout from a global per-account counter to per (account, IP).
The old design was a denial-of-service primitive: anyone who knew an email
address could send five wrong passwords and lock the real owner out for 15
minutes, repeatedly and indefinitely, at near-zero cost. Locking someone out of
their password manager is a serious harm in itself.
Scoping by IP means an attacker locks out only their own address. The legitimate
owner signing in from their own IP is unaffected, and a distributed attacker
still faces Flask-Limiter (10/min per IP on /login) plus the Nginx auth_limit
zone on every address they rotate through.
users.failed_login_count / users.locked_until are left in place and still
maintained as an aggregate signal for the audit log, but no longer gate
authentication.
"""
from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects.mysql import INTEGER
revision = 'm3n4o5p6q7r8'
down_revision = 'l2m3n4o5p6q7'
branch_labels = None
depends_on = None
def upgrade():
op.create_table(
'login_attempts',
sa.Column('id', INTEGER(unsigned=True), autoincrement=True, primary_key=True),
sa.Column('user_id', INTEGER(unsigned=True), nullable=False),
sa.Column('ip_address', sa.String(45), nullable=False, server_default=''),
sa.Column('failed_count', sa.Integer, nullable=False, server_default='0'),
sa.Column('locked_until', sa.DateTime, nullable=True),
sa.Column('updated_at', sa.DateTime, nullable=False),
sa.ForeignKeyConstraint(['user_id'], ['users.id'], ondelete='CASCADE'),
sa.UniqueConstraint('user_id', 'ip_address', name='uq_login_attempt_user_ip'),
)
# The cleanup job sweeps by updated_at.
op.create_index('ix_login_attempts_updated_at', 'login_attempts', ['updated_at'])
def downgrade():
op.drop_index('ix_login_attempts_updated_at', table_name='login_attempts')
op.drop_table('login_attempts')
+7
View File
@@ -0,0 +1,7 @@
[pytest]
testpaths = tests
python_files = test_*.py
# Fail on unraised warnings that indicate real problems, but keep the
# deliberately-bad signing key in test_forged_epoch_claim_is_rejected quiet.
filterwarnings =
ignore::UserWarning:jwt.api_jwt
+4
View File
@@ -0,0 +1,4 @@
# Development / CI-only dependencies.
# Install alongside requirements.txt: pip install -r requirements.txt -r requirements-dev.txt
pytest>=8.0
flake8>=7.0
+55 -10
View File
@@ -10,12 +10,37 @@
# - Content-Security-Policy (HTTP header level — authoritative over meta tag) # - Content-Security-Policy (HTTP header level — authoritative over meta tag)
# - Connection-level rate limiting zones for auth endpoints # - Connection-level rate limiting zones for auth endpoints
# - Buffer and timeout hardening # - Buffer and timeout hardening
#
# Phase 6 (502 fixes):
# - proxy_connect_timeout / proxy_read_timeout / proxy_send_timeout made
# explicit. proxy_read_timeout MUST stay BELOW the `timeout` value in
# gunicorn.conf.py (90s): if Gunicorn kills the worker first the connection
# is severed mid-response and Nginx reports 502; if Nginx gives up first the
# client gets a clean 504 instead.
# - Security headers repeated inside the /static/ location block. Nginx drops
# ALL inherited add_header directives in any block that declares one of its
# own, so /static/'s Cache-Control header was silently stripping CSP, HSTS,
# X-Frame-Options and nosniff from every JS and CSS asset.
# ── Rate limiting zones ──────────────────────────────────────────────────────── # ── Rate limiting zones ────────────────────────────────────────────────────────
# auth_limit: 10 req/s per IP for auth endpoints (login, register, MFA verify) # MIND THE UNITS — r/s and r/m are easy to confuse, and getting it wrong is
# api_limit: 60 req/s per IP for all other API endpoints # invisible until users start seeing 429s.
limit_req_zone $binary_remote_addr zone=auth_limit:10m rate=10r/m; #
limit_req_zone $binary_remote_addr zone=api_limit:10m rate=60r/m; # api_limit was 60r/m, i.e. 1 req/s sustained for the ENTIRE API. A single vault
# page load fires several /api/* calls (vault, folders, sharing inbox, me), and
# any active session drains the burst bucket quickly, so normal use produced
# spurious 429s. Now 10r/s, which is generous for a human and still bounds
# scripted abuse.
#
# auth_limit stays deliberately tight — it is the brute-force surface. 20r/m
# with burst=5 is well above what a human retyping a password needs, and
# Flask-Limiter (Redis-backed, 10/min on /login) remains the primary guard;
# this zone exists to shed load before it reaches Gunicorn.
#
# auth_limit: 20 req/MINUTE per IP — login, register, MFA verify
# api_limit: 10 req/SECOND per IP — everything else
limit_req_zone $binary_remote_addr zone=auth_limit:10m rate=20r/m;
limit_req_zone $binary_remote_addr zone=api_limit:10m rate=10r/s;
server { server {
server_name pwkeeper.ngodanguyen.tech passkeeper.ngodanguyen.tech; server_name pwkeeper.ngodanguyen.tech passkeeper.ngodanguyen.tech;
@@ -27,14 +52,14 @@ server {
include /etc/letsencrypt/options-ssl-nginx.conf; include /etc/letsencrypt/options-ssl-nginx.conf;
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem;
# Modern TLS only # options-ssl-nginx.conf (managed by Certbot) already sets ssl_protocols,
ssl_protocols TLSv1.2 TLSv1.3; # ssl_prefer_server_ciphers, ssl_session_timeout and ssl_session_tickets.
ssl_prefer_server_ciphers on; # Repeating them here conflicts whenever Certbot updates its managed file.
ssl_session_cache shared:SSL:10m; ssl_session_cache shared:SSL:10m;
ssl_session_timeout 1d;
ssl_session_tickets off;
# ── Security headers ────────────────────────────────────────────────────── # ── Security headers ──────────────────────────────────────────────────────
# IMPORTANT: these are repeated verbatim inside the /static/ block below.
# Keep the two copies in sync whenever either is modified.
# HSTS: enforce HTTPS for 1 year; include subdomains; allow preload submission # HSTS: enforce HTTPS for 1 year; include subdomains; allow preload submission
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
@@ -68,11 +93,31 @@ server {
keepalive_timeout 15s; keepalive_timeout 15s;
send_timeout 10s; send_timeout 10s;
# ── Proxy timeouts ────────────────────────────────────────────────────────
# proxy_read_timeout must stay BELOW gunicorn.conf.py `timeout` (90s) so
# Nginx is the side that gives up first and the client sees 504, not 502.
# proxy_connect_timeout is short — Gunicorn is on loopback.
proxy_connect_timeout 5s;
proxy_read_timeout 60s;
proxy_send_timeout 10s;
# ── Static files ────────────────────────────────────────────────────────── # ── Static files ──────────────────────────────────────────────────────────
# Every add_header from the server block MUST be repeated here. Nginx drops
# all inherited add_header directives in any location that declares one of
# its own, so without this the Cache-Control below silently strips CSP,
# HSTS, X-Frame-Options and nosniff from every static asset.
location /static/ { location /static/ {
alias /home/spuser/PassKeeper/app/static/; alias /home/spuser/PassKeeper/app/static/;
expires 30d; expires 30d;
add_header Cache-Control "public, immutable"; add_header Cache-Control "public, immutable" always;
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
add_header X-Frame-Options "DENY" always;
add_header X-Content-Type-Options "nosniff" always;
add_header Referrer-Policy "strict-origin-when-cross-origin" always;
add_header Permissions-Policy "geolocation=(), camera=(), microphone=()" always;
add_header Content-Security-Policy
"default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; font-src 'self'; connect-src 'self' https://api.pwnedpasswords.com; frame-ancestors 'none';"
always;
} }
# ── Auth endpoints — stricter Nginx-level rate limit ────────────────────── # ── Auth endpoints — stricter Nginx-level rate limit ──────────────────────
+38 -18
View File
@@ -8,9 +8,9 @@
# journalctl -xeu passkeeper.service # journalctl -xeu passkeeper.service
# #
# Phase 5 additions vs original: # Phase 5 additions vs original:
# - WatchdogSec: systemd kills and restarts a hung Gunicorn within 30 s # - Restart=on-failure: systemd restarts Gunicorn if the master exits non-zero
# - Gunicorn --timeout: workers that don't respond within 25 s are replaced # - Gunicorn settings live in gunicorn.conf.py (gthread workers, timeout
# - Gunicorn --graceful-timeout: allows in-flight requests to finish on reload # ordered above nginx proxy_read_timeout, /dev/shm heartbeat dir)
# - PrivateTmp, NoNewPrivileges, ProtectSystem: basic systemd sandboxing # - PrivateTmp, NoNewPrivileges, ProtectSystem: basic systemd sandboxing
# - StartLimitIntervalSec / StartLimitBurst: caps restart storm # - StartLimitIntervalSec / StartLimitBurst: caps restart storm
@@ -29,25 +29,45 @@ Group=www-data
WorkingDirectory=/home/spuser/PassKeeper WorkingDirectory=/home/spuser/PassKeeper
EnvironmentFile=/home/spuser/PassKeeper/.env EnvironmentFile=/home/spuser/PassKeeper/.env
# All tuning lives in gunicorn.conf.py (worker class, counts, timeouts,
# logging) so it is versioned with the code and documented in one place.
# Override any of it with GUNICORN_* variables in the EnvironmentFile above
# rather than editing this line.
# Absolute path — do not rely on WorkingDirectory for config lookup.
ExecStart=/home/spuser/.venv/bin/gunicorn \ ExecStart=/home/spuser/.venv/bin/gunicorn \
--workers 4 \ -c /home/spuser/PassKeeper/gunicorn.conf.py \
--bind 127.0.0.1:5000 \
--timeout 25 \
--graceful-timeout 20 \
--keep-alive 5 \
--access-logfile /home/spuser/logs/access.log \
--error-logfile /home/spuser/logs/error.log \
--log-level warning \
wsgi:app wsgi:app
# Reload (zero-downtime): send USR2 to Gunicorn master # Reload: HUP re-reads config and restarts workers in place under the
ExecReload=/bin/kill -s USR2 $MAINPID # existing master.
#
# NOT USR2. USR2 forks a *second* master that inherits the listening socket,
# and retiring the old one needs a follow-up WINCH + QUIT that this unit never
# sent. The result was two masters competing for the port with systemd $MAINPID
# tracking the stale one — a later stop/restart then signalled the wrong
# process and the socket vanished, which Nginx reports as 502.
#
# HUP does not pick up changed Python source: use `systemctl restart` for code
# deploys, reload only for config-only changes.
ExecReload=/bin/kill -s HUP $MAINPID
# Watchdog: systemd sends SIGKILL if Gunicorn doesn't send keepalives within 30 s. # NO WatchdogSec here — deliberately.
# Requires gunicorn to be started with --preload OR the watchdog plugin; here we #
# rely on the worker timeout (25 s) to recycle hung workers before the 30 s # WatchdogSec requires the service to send WATCHDOG=1 keepalives over the sd_notify
# watchdog fires, which restarts the entire service. # socket. Gunicorn only does that when systemd exports NOTIFY_SOCKET, which happens
WatchdogSec=30s # only under Type=notify (+ NotifyAccess=main). This unit is Type=simple (the
# default), so no keepalive was ever sent, systemd treated the service as hung, and
# SIGKILLed it every ~30 s. Restart=on-failure then brought it back after RestartSec,
# producing a repeating window of 502s from Nginx.
#
# Hung *workers* are already handled by Gunicorn's own `timeout` in gunicorn.conf.py; a crashed
# *master* is already handled by Restart=on-failure below. The watchdog added no
# coverage, only outages.
#
# To re-enable it properly (optional), all three lines are required:
# Type=notify
# NotifyAccess=main
# WatchdogSec=30s
Restart=on-failure Restart=on-failure
RestartSec=5s RestartSec=5s
+212
View File
@@ -0,0 +1,212 @@
#!/usr/bin/env python3
"""
Regenerate extension/shared/psl.js from the Public Suffix List.
python scripts/update_psl.py
The extension has no build step it is zipped as-is so the list is vendored
as a plain classic script rather than pulled in via npm/bundler.
Why the extension needs this at all: autofill decides whether a stored
credential belongs to the page you are on. Plain suffix comparison treats
`evil.github.io` and `victim.github.io` as the same site, because `github.io`
looks like an ordinary domain. The PSL is the only way to know that it is a
public suffix and that those are different sites.
BOTH sections are included, deliberately:
ICANN real TLDs (co.uk, com.au, ...)
PRIVATE github.io, vercel.app, herokuapp.com, ...
The PRIVATE section is the one that matters most here: those are the hosts where
an attacker can actually obtain a neighbouring subdomain.
Re-run when the list goes stale (it changes a few times a month). The generated
file records the upstream VERSION header so staleness is visible in a diff.
"""
import pathlib
import re
import sys
import urllib.request
PSL_URL = 'https://publicsuffix.org/list/public_suffix_list.dat'
OUT = pathlib.Path(__file__).resolve().parent.parent / 'extension' / 'shared' / 'psl.js'
HEADER = '''/**
* extension/shared/psl.js GENERATED FILE, DO NOT EDIT BY HAND.
*
* Regenerate with: python scripts/update_psl.py
*
* Vendored Public Suffix List (https://publicsuffix.org/), used to decide
* whether two hostnames belong to the same site before offering a stored
* credential for autofill.
*
* Source list version: {version}
* Rules: {n_rules} exact, {n_wild} wildcard, {n_exc} exception
*
* The list is MPL-2.0 licensed; see https://mozilla.org/MPL/2.0/.
*
* Exposes a single global, PkPsl, with:
* getRegistrableDomain(host) -> "example.co.uk" | null
* isSameSite(hostA, hostB) -> boolean
*/
'''
BODY = r'''
const PkPsl = (() => {
"use strict";
// Split from single strings rather than array literals same data, far less
// punctuation, and the parse cost is a one-off at script load.
const RULES = new Set(EXACT_BLOB.split("\n"));
const WILDCARDS = new Set(WILD_BLOB ? WILD_BLOB.split("\n") : []);
const EXCEPTIONS = new Set(EXC_BLOB ? EXC_BLOB.split("\n") : []);
const IPV4_RE = /^\d{1,3}(\.\d{1,3}){3}$/;
function _normalise(host) {
if (!host) return null;
let h = String(host).trim().toLowerCase();
if (h.endsWith(".")) h = h.slice(0, -1); // trailing root dot
return h || null;
}
/**
* Number of trailing labels that form the public suffix of `labels`.
* Implements the matching rules from https://publicsuffix.org/list/:
* exception rules win outright, otherwise the longest match wins, and an
* unmatched host falls back to the implicit "*" rule.
*/
function _publicSuffixLength(labels) {
// Exception rules (!foo.bar) take priority over everything else.
for (let i = 0; i < labels.length; i++) {
if (EXCEPTIONS.has(labels.slice(i).join("."))) {
return labels.length - i - 1;
}
}
let best = 0;
for (let i = 0; i < labels.length; i++) {
const len = labels.length - i;
if (len <= best) continue;
if (RULES.has(labels.slice(i).join("."))) {
best = len;
continue;
}
// A wildcard rule "*.x.y" matches when labels[i] is any single label and
// the remainder equals "x.y".
if (i + 1 <= labels.length - 1 &&
WILDCARDS.has(labels.slice(i + 1).join("."))) {
best = len;
}
}
// No rule matched: the implicit "*" rule makes the rightmost label the
// public suffix (so "example.invalidtld" is still a registrable domain).
return best || 1;
}
/**
* The registrable domain ("example.co.uk") for a hostname, or null when the
* host has none an IP address, a single label like "localhost", or a host
* that IS a public suffix ("github.io" itself).
*
* Callers must treat null as "no site identity": fall back to exact hostname
* equality rather than assuming a match.
*/
function getRegistrableDomain(host) {
const h = _normalise(host);
if (!h) return null;
if (IPV4_RE.test(h) || h.includes(":")) return null; // IPv4 / IPv6
const labels = h.split(".");
if (labels.length < 2) return null; // "localhost"
const suffixLen = _publicSuffixLength(labels);
if (labels.length <= suffixLen) return null; // host is itself a suffix
return labels.slice(labels.length - suffixLen - 1).join(".");
}
/**
* True when two hostnames belong to the same registrable site.
*
* When either host has no registrable domain (IP, localhost, or a bare public
* suffix) this falls back to exact hostname equality never to a suffix
* comparison, which is what allowed evil.github.io to match victim.github.io.
*/
function isSameSite(a, b) {
const ha = _normalise(a);
const hb = _normalise(b);
if (!ha || !hb) return false;
if (ha === hb) return true;
const da = getRegistrableDomain(ha);
const db = getRegistrableDomain(hb);
if (!da || !db) return false;
return da === db;
}
return { getRegistrableDomain, isSameSite };
})();
// Content scripts and the popup load this as a classic script; the service
// worker imports it via importScripts. Export only where a module system exists.
if (typeof module !== "undefined" && module.exports) {
module.exports = PkPsl;
}
'''
def main():
print(f'Fetching {PSL_URL} ...')
with urllib.request.urlopen(PSL_URL, timeout=60) as resp:
text = resp.read().decode('utf-8')
version = 'unknown'
m = re.search(r'^// VERSION:\s*(.+)$', text, re.M)
if m:
version = m.group(1).strip()
exact, wildcards, exceptions = [], [], []
for line in text.splitlines():
rule = line.strip()
if not rule or rule.startswith('//'):
continue
if rule.startswith('!'):
exceptions.append(rule[1:])
elif rule.startswith('*.'):
wildcards.append(rule[2:])
elif '*' in rule:
# No such rules exist today (wildcards are always leftmost). Skip
# loudly rather than silently mis-parsing if that ever changes.
print(f' WARNING: skipping unsupported rule {rule!r}', file=sys.stderr)
else:
exact.append(rule)
if len(exact) < 5000:
sys.exit(f'FAIL: only {len(exact)} exact rules parsed — the list looks truncated')
for expected in ('github.io', 'vercel.app', 'co.uk'):
if expected not in exact:
sys.exit(f'FAIL: expected rule {expected!r} missing — parse is wrong')
def blob(name, values):
return f' const {name} = `' + '\n'.join(sorted(set(values))) + '`;\n'
out = HEADER.format(version=version, n_rules=len(exact),
n_wild=len(wildcards), n_exc=len(exceptions))
out += '\n// eslint-disable-next-line no-unused-vars\n'
out += 'const _PSL_DATA = (() => {\n'
out += blob('EXACT_BLOB', exact)
out += blob('WILD_BLOB', wildcards)
out += blob('EXC_BLOB', exceptions)
out += ' return { EXACT_BLOB, WILD_BLOB, EXC_BLOB };\n})();\n'
out += '\nconst { EXACT_BLOB, WILD_BLOB, EXC_BLOB } = _PSL_DATA;\n'
out += BODY
OUT.write_text(out, encoding='utf-8', newline='\n')
size_kb = OUT.stat().st_size / 1024
print(f'Wrote {OUT.relative_to(OUT.parent.parent.parent)} '
f'({size_kb:.0f} KB) — version {version}')
print(f' {len(exact)} exact, {len(wildcards)} wildcard, {len(exceptions)} exception rules')
if __name__ == '__main__':
main()
+80
View File
@@ -0,0 +1,80 @@
"""
Shared pytest fixtures.
Runs the real app factory against in-memory SQLite. The server treats all
client-side crypto as opaque strings (auth_hash, enc_data, iv, enc_name), so
these tests can pass arbitrary values for them no Web Crypto needed. The one
place real crypto matters is the recovery proof, which is plain HMAC-SHA256 and
is computed here exactly as recover.js does.
"""
import os
import sys
import pytest
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
from app import create_app, db as _db # noqa: E402
@pytest.fixture
def app():
application = create_app('testing')
with application.app_context():
_db.create_all()
yield application
_db.session.remove()
_db.drop_all()
@pytest.fixture
def client(app):
return app.test_client()
@pytest.fixture
def db(app):
return _db
# ── Helpers ──────────────────────────────────────────────────────────────────
def register(client, email='user@example.com', auth_hash='AUTH-HASH-V1',
enc_key_salt='SALT-V1'):
return client.post('/api/auth/register', json={
'email': email, 'auth_hash': auth_hash, 'enc_key_salt': enc_key_salt,
})
def login(client, email='user@example.com', auth_hash='AUTH-HASH-V1'):
return client.post('/api/auth/login', json={'email': email, 'auth_hash': auth_hash})
def auth_headers(token):
return {'Authorization': f'Bearer {token}'}
def make_user(client, email='user@example.com', auth_hash='AUTH-HASH-V1',
enc_key_salt='SALT-V1'):
"""
Register + log in. Returns (access_token, refresh_token).
Registration answers 202 for both new and duplicate addresses so it cannot
be used to probe account existence see test_registration_privacy.py.
"""
assert register(client, email, auth_hash, enc_key_salt).status_code == 202
res = login(client, email, auth_hash)
assert res.status_code == 200, res.get_json()
body = res.get_json()
return body['access_token'], body['refresh_token']
def add_item(client, token, name='password', enc_data='CT', iv='IV'):
"""Create a vault item. `name` is the server-side type label."""
res = client.post('/api/vault', headers=auth_headers(token), json={
'name': name, 'item_type': 'password',
'enc_data': enc_data, 'iv': iv,
'enc_name': 'ENCNAME', 'iv_name': 'IVNAME',
})
assert res.status_code == 201, res.get_json()
return res.get_json()['id']
+177
View File
@@ -0,0 +1,177 @@
/**
* tests/js/test_field_heuristics.js run with:
* node tests/js/test_field_heuristics.js
*
* Pins the field-detection predicates in extension/content/content.js against
* the ASUS RT-AX88U router login page, which the extension could not detect.
*
* The username input there is about as obvious as they come:
*
* <input type="text" id="login_username" name="login_username"
* class="form_input" placeholder="Username" autocomplete="off">
*
* but "off" was in the NON_CRED_AC deny-list, so _isLikelyUsernameField()
* returned false at that check and never reached the name/id/placeholder
* keyword test. `autocomplete="off"` says nothing about whether a field holds a
* credential routers and admin panels set it specifically to discourage
* password managers.
*
* SCOPE: this exercises the regex predicates extracted from the real source
* file, not the DOM traversal in _hasPasswordSibling() (which needs a browser).
* Those paths are covered by the manual check in the accompanying notes.
*/
'use strict';
const fs = require('fs');
const path = require('path');
const SRC = fs.readFileSync(
path.join(__dirname, '..', '..', 'extension', 'content', 'content.js'),
'utf8',
);
let failures = 0;
function check(label, got, want) {
if (got !== want) {
failures++;
console.log(`FAIL ${label}\n got=${got} want=${want}`);
}
}
/** Pull a named regex literal out of the real source rather than copying it. */
function extractRegex(name) {
const m = SRC.match(new RegExp(`const ${name} =\\s*(/[^\\n]+/[a-z]*);`));
if (!m) throw new Error(`could not find regex ${name} in content.js`);
// eslint-disable-next-line no-eval
return eval(m[1]);
}
const CRED_HINTS = extractRegex('CRED_HINTS');
const NON_CRED_AC = extractRegex('NON_CRED_AC');
// ── The bug: autocomplete="off" must not be a negative signal ───────────────
check('NON_CRED_AC rejects "off"', NON_CRED_AC.test('off'), false);
check('NON_CRED_AC rejects "OFF" (case-insensitive)', NON_CRED_AC.test('OFF'), false);
// Anti-autofill junk values sites use must also fall through to the keywords.
for (const junk of ['nope', 'no', 'false', 'disabled', 'new-user', 'chrome-off']) {
check(`NON_CRED_AC lets junk value "${junk}" fall through`,
NON_CRED_AC.test(junk), false);
}
// ── But genuine non-credential tokens must still be rejected ───────────────
for (const token of ['name', 'given-name', 'family-name', 'organization',
'street-address', 'country', 'postal-code', 'search',
'url', 'bday', 'sex', 'photo', 'language',
'new-password', 'current-password', 'one-time-code']) {
check(`NON_CRED_AC still rejects "${token}"`, NON_CRED_AC.test(token), true);
}
// ── The ASUS field's identifying attributes must read as a credential ───────
const ASUS_USERNAME_ATTRS = ['login_username', 'login_username', 'Username', ''].join(' ');
check('ASUS username attrs match CRED_HINTS',
CRED_HINTS.test(ASUS_USERNAME_ATTRS), true);
// Other real-world username fields.
for (const attrs of ['user_name', 'j_username', 'email', 'userEmail',
'account', 'Login ID', 'mobile', 'tel']) {
check(`CRED_HINTS matches "${attrs}"`, CRED_HINTS.test(attrs), true);
}
// Fields that must NOT be treated as credential inputs on keywords alone.
for (const attrs of ['q', 'search-query', 'first_name', 'zipcode',
'street', 'company', 'comment']) {
check(`CRED_HINTS does not match "${attrs}"`, CRED_HINTS.test(attrs), false);
}
// ── Submit-control detection for pages with no <form> ──────────────────────
// The ASUS page submits via <div class="button" onclick="preLogin();">, so no
// "submit" event ever fires and the save prompt never appeared.
const clsMatch = SRC.match(
/return (\/\(\^\|\[\\s_-\]\)\(btn\|button[^\n]+\/)\.test\(cls\);/,
);
if (!clsMatch) {
failures++;
console.log('FAIL could not find the submit-control class regex in content.js');
} else {
// eslint-disable-next-line no-eval
const BTN_CLS = eval(clsMatch[1]);
check('recognises class="button" (ASUS)', BTN_CLS.test('button'), true);
for (const cls of ['btn', 'btn btn-primary', 'submit', 'login-button',
'signin', 'sign-in', 'form_btn button']) {
check(`recognises class="${cls}"`, BTN_CLS.test(cls), true);
}
for (const cls of ['form_input', 'container', 'buttonish', 'rebutton']) {
check(`ignores class="${cls}"`, BTN_CLS.test(cls), false);
}
}
// The click and keydown fallbacks must actually be registered.
check('click fallback registered', /addEventListener\(\s*"click"/.test(SRC), true);
check('keydown fallback registered', /addEventListener\(\s*"keydown"/.test(SRC), true);
check('submit listener retained', /addEventListener\(\s*"submit"/.test(SRC), true);
// ── _isTrustworthyOrigin: where credentials may be filled ──────────────────
// Extracted from the real source and evaluated against a stubbed `location`,
// so this exercises the shipped function rather than a copy of its rules.
{
const fnSrc = SRC.match(
/function _isTrustworthyOrigin\(\) \{[\s\S]*?\n \}/,
);
if (!fnSrc) {
failures++;
console.log('FAIL could not extract _isTrustworthyOrigin from content.js');
} else {
const make = new Function(
'location',
`${fnSrc[0]}; return _isTrustworthyOrigin();`,
);
const at = (protocol, hostname) => make({ protocol, hostname });
// HTTPS is always fine.
check('https is trustworthy', at('https:', 'example.com'), true);
// Local devices over plain HTTP — routers, NAS, printers. Dropping
// http://*/* entirely would break exactly these.
for (const host of ['localhost', '127.0.0.1', '::1', 'router.local',
'10.0.0.1', '192.168.1.1', '172.16.5.4', '172.31.0.1',
'169.254.1.1', 'nas.lan', 'box.home']) {
check(`http://${host} is treated as local`, at('http:', host), true);
}
// Plaintext on the public internet must warn.
for (const host of ['example.com', 'bank.co.uk', '8.8.8.8',
'172.15.0.1', '172.32.0.1', '11.0.0.1',
'192.169.1.1', 'evil-localhost.com',
'localhost.evil.com', '127.0.0.1.evil.com']) {
check(`http://${host} is NOT trusted`, at('http:', host), false);
}
}
}
// ── Autologin must not click a control that discards the login ────────────
{
const m = SRC.match(/var _NEGATIVE_CONTROL = (\/[^\n]+\/[a-z]*);/);
if (!m) {
failures++;
console.log('FAIL could not find _NEGATIVE_CONTROL in content.js');
} else {
// eslint-disable-next-line no-eval
const NEG = eval(m[1]);
for (const label of ['Cancel', 'Reset', 'Go back', 'Forgot password?',
'Register', 'Sign up', 'Create account']) {
check(`autologin skips "${label}"`, NEG.test(label), true);
}
for (const label of ['Sign In', 'Log in', 'Submit', 'Continue', 'OK']) {
check(`autologin allows "${label}"`, NEG.test(label), false);
}
}
}
// ── Summary (must stay last so every block above is counted) ───────────────
if (failures) {
console.log(`\n${failures} failure(s)`);
process.exit(1);
}
console.log('OK: field heuristics assertions passed');
+127
View File
@@ -0,0 +1,127 @@
/**
* tests/js/test_psl.js run with: node tests/js/test_psl.js
*
* Covers extension/shared/psl.js, which decides whether a stored credential
* belongs to the page being viewed (review finding #5).
*
* The old matcher in content.js compared hostnames by plain suffix:
*
* h === host || h.endsWith("." + host) || host.endsWith("." + h)
*
* which treated evil.github.io and victim.github.io as the same site, and let
* an item saved for a bare TLD match every site under it. The attack cases
* below pin that shut; the Mozilla vectors verify the PSL algorithm itself.
*/
'use strict';
const fs = require('fs');
const path = require('path');
const vm = require('vm');
const PSL_PATH = path.join(__dirname, '..', '..', 'extension', 'shared', 'psl.js');
// Load as a classic script in a bare context, the way a content script sees it.
const ctx = vm.createContext({});
vm.runInContext(fs.readFileSync(PSL_PATH, 'utf8'), ctx, { filename: 'psl.js' });
const PkPsl = vm.runInContext('PkPsl', ctx);
let failures = 0;
function check(label, got, want) {
if (got !== want) {
failures++;
console.log(`FAIL ${label}\n got=${JSON.stringify(got)} want=${JSON.stringify(want)}`);
}
}
// ── Canonical vectors from Mozilla's PSL test suite ─────────────────────────
// https://github.com/publicsuffix/list/blob/master/tests/test_psl.txt
const MOZILLA_VECTORS = [
['example.COM', 'example.com'], ['WwW.example.COM', 'example.com'],
['example', null], ['b.example', 'b.example'], ['a.b.example', 'b.example'],
['biz', null], ['domain.biz', 'domain.biz'], ['b.domain.biz', 'domain.biz'],
['a.b.domain.biz', 'domain.biz'],
['com', null], ['example.com', 'example.com'], ['b.example.com', 'example.com'],
['a.b.example.com', 'example.com'], ['uk.com', null],
['example.uk.com', 'example.uk.com'], ['b.example.uk.com', 'example.uk.com'],
['a.b.example.uk.com', 'example.uk.com'], ['test.ac', 'test.ac'],
// TLD with only a wildcard rule
['mm', null], ['c.mm', null], ['b.c.mm', 'b.c.mm'], ['a.b.c.mm', 'b.c.mm'],
// More complex TLD
['jp', null], ['test.jp', 'test.jp'], ['www.test.jp', 'test.jp'],
['ac.jp', null], ['test.ac.jp', 'test.ac.jp'], ['www.test.ac.jp', 'test.ac.jp'],
['kyoto.jp', null], ['test.kyoto.jp', 'test.kyoto.jp'],
['ide.kyoto.jp', null], ['b.ide.kyoto.jp', 'b.ide.kyoto.jp'],
['a.b.ide.kyoto.jp', 'b.ide.kyoto.jp'],
['c.kobe.jp', null], ['b.c.kobe.jp', 'b.c.kobe.jp'], ['a.b.c.kobe.jp', 'b.c.kobe.jp'],
['city.kobe.jp', 'city.kobe.jp'], ['www.city.kobe.jp', 'city.kobe.jp'],
// Wildcard rule plus exceptions
['ck', null], ['test.ck', null], ['b.test.ck', 'b.test.ck'],
['a.b.test.ck', 'b.test.ck'], ['www.ck', 'www.ck'], ['www.www.ck', 'www.ck'],
['us', null], ['test.us', 'test.us'], ['www.test.us', 'test.us'],
['ak.us', null], ['test.ak.us', 'test.ak.us'], ['www.test.ak.us', 'test.ak.us'],
['k12.ak.us', null], ['test.k12.ak.us', 'test.k12.ak.us'],
['www.test.k12.ak.us', 'test.k12.ak.us'],
];
for (const [input, want] of MOZILLA_VECTORS) {
check(`getRegistrableDomain(${input})`, PkPsl.getRegistrableDomain(input), want);
}
// ── The PRIVATE section must be present ─────────────────────────────────────
// These are the suffixes where an attacker can actually register a neighbouring
// subdomain, so dropping the PRIVATE section would silently reopen the hole.
check('github.io is a public suffix', PkPsl.getRegistrableDomain('github.io'), null);
check('vercel.app is a public suffix', PkPsl.getRegistrableDomain('vercel.app'), null);
check('user.github.io is registrable',
PkPsl.getRegistrableDomain('victim.github.io'), 'victim.github.io');
// ── Attack cases: these must NOT be treated as the same site ────────────────
const MUST_NOT_MATCH = [
['evil.github.io', 'victim.github.io', 'siblings on github.io'],
['attacker.vercel.app', 'realapp.vercel.app', 'siblings on vercel.app'],
['evil.herokuapp.com', 'real.herokuapp.com', 'siblings on herokuapp.com'],
['evil.co.uk', 'bank.co.uk', 'siblings under co.uk'],
['anything.com', 'com', 'item saved for a bare TLD'],
['login.evil.com', 'evil.com.attacker.net', 'suffix confusion'],
['example.com.evil.net', 'example.com', 'apex embedded in an attacker host'],
['192.168.1.10', '192.168.1.11', 'different IPs'],
['github.io', 'victim.github.io', 'bare suffix vs a site under it'],
];
for (const [a, b, label] of MUST_NOT_MATCH) {
check(`isSameSite(${a}, ${b}) [${label}]`, PkPsl.isSameSite(a, b), false);
}
// ── Legitimate matches must keep working ────────────────────────────────────
const MUST_MATCH = [
['login.example.com', 'example.com', 'subdomain to apex'],
['www.example.com', 'accounts.example.com', 'sibling subdomains'],
['a.b.c.example.co.uk', 'example.co.uk', 'deep subdomain under an ICANN suffix'],
['example.com', 'example.com', 'identical'],
['WWW.Example.COM', 'example.com', 'case-insensitive'],
['example.com.', 'example.com', 'trailing root dot'],
['localhost', 'localhost', 'localhost falls back to exact equality'],
['192.168.1.10', '192.168.1.10', 'IP falls back to exact equality'],
['github.io', 'github.io', 'bare suffix matches itself exactly'],
];
for (const [a, b, label] of MUST_MATCH) {
check(`isSameSite(${a}, ${b}) [${label}]`, PkPsl.isSameSite(a, b), true);
}
// ── Malformed input must not throw ──────────────────────────────────────────
for (const bad of [null, undefined, '', '.', '..', ' ', 'a..b']) {
try {
PkPsl.getRegistrableDomain(bad);
PkPsl.isSameSite(bad, 'example.com');
} catch (e) {
failures++;
console.log(`FAIL threw on input ${JSON.stringify(bad)}: ${e.message}`);
}
}
const total = MOZILLA_VECTORS.length + MUST_NOT_MATCH.length + MUST_MATCH.length + 3;
if (failures) {
console.log(`\n${failures} failure(s)`);
process.exit(1);
}
console.log(`OK: ${total} PSL assertions passed`);
+206
View File
@@ -0,0 +1,206 @@
"""
Guards on deployment configuration that only bites in production.
These are the settings whose failure mode is an intermittent 502 rather than a
stack trace, so nothing else catches them drifting apart.
"""
import re
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parent.parent
NGINX = (ROOT / 'scripts' / 'passkeeper-nginx.conf').read_text(encoding='utf-8')
UNIT = (ROOT / 'scripts' / 'passkeeper.service').read_text(encoding='utf-8')
@pytest.fixture(scope='module')
def gunicorn_conf():
ns = {}
exec(compile((ROOT / 'gunicorn.conf.py').read_text(encoding='utf-8'),
'gunicorn.conf.py', 'exec'), ns)
return ns
def _nginx_seconds(directive):
m = re.search(rf'^\s*{directive}\s+(\d+)s;', NGINX, re.M)
assert m, f'{directive} not found in passkeeper-nginx.conf'
return int(m.group(1))
def test_nginx_gives_up_before_gunicorn_kills_the_worker(gunicorn_conf):
"""
The 502 invariant. If Gunicorn's timeout fires first the connection is
severed mid-response and Nginx reports 502; if Nginx times out first the
client gets a clean 504 instead.
"""
assert _nginx_seconds('proxy_read_timeout') < gunicorn_conf['timeout']
def test_gunicorn_holds_keepalive_longer_than_nginx(gunicorn_conf):
"""
Nginx must be the side that closes an idle upstream connection. If Gunicorn
closes one as Nginx is reusing it, that request fails as a 502.
"""
assert gunicorn_conf['keepalive'] > _nginx_seconds('keepalive_timeout')
def test_preload_app_is_disabled(gunicorn_conf):
"""
create_app() starts an APScheduler thread, and threads do not survive
fork(). Under preload_app the scheduler would exist only in the arbiter,
which serves no requests, so the cleanup job would silently never run.
"""
assert gunicorn_conf['preload_app'] is False
def test_static_location_repeats_every_security_header():
"""
Nginx drops ALL inherited add_header directives in any location that
declares one of its own. /static/ sets Cache-Control, so without explicit
copies every JS and CSS asset ships with no CSP, HSTS or X-Frame-Options.
"""
static = re.search(r'location /static/ \{(.*?)\n \}', NGINX, re.S)
assert static, 'no /static/ location block found'
body = static.group(1)
for header in ('Strict-Transport-Security', 'X-Frame-Options',
'X-Content-Type-Options', 'Referrer-Policy',
'Permissions-Policy', 'Content-Security-Policy'):
assert header in body, f'/static/ is missing {header}'
def test_hibp_origin_is_allowed_in_every_csp():
"""The security dashboard's breach check needs this origin in connect-src."""
policies = re.findall(r'connect-src[^;"]*', NGINX)
assert policies, 'no connect-src directive found'
for p in policies:
assert 'https://api.pwnedpasswords.com' in p, p
def test_unit_has_no_watchdog():
"""
WatchdogSec without Type=notify meant systemd never received a keepalive,
declared the service hung, and SIGKILLed it on a loop a repeating window
of 502s. Re-enabling it requires Type=notify AND NotifyAccess=main.
"""
active = [ln for ln in UNIT.splitlines()
if ln.strip().startswith('WatchdogSec')]
if active:
assert 'Type=notify' in UNIT and 'NotifyAccess=main' in UNIT, (
'WatchdogSec requires Type=notify + NotifyAccess=main or systemd '
'will kill the service on a loop'
)
def test_unit_reload_does_not_use_usr2():
"""
USR2 forks a second master without retiring the first, leaving systemd's
$MAINPID tracking a stale process.
"""
reload_line = next((ln for ln in UNIT.splitlines()
if ln.strip().startswith('ExecReload=')), '')
assert 'USR2' not in reload_line, reload_line
def test_unit_loads_the_gunicorn_config_file():
assert 'gunicorn.conf.py' in UNIT, (
'the unit no longer references gunicorn.conf.py, so its tuning is dead code'
)
def test_api_rate_limit_is_not_absurdly_tight():
"""
api_limit was 60r/m 1 req/s for the entire API. A vault page load fires
several /api/* calls, so normal use produced spurious 429s. The units are
easy to misread, which is exactly why this is pinned.
"""
m = re.search(r'zone=api_limit:\S+\s+rate=(\d+)r/([sm]);', NGINX)
assert m, 'api_limit zone not found'
per_second = int(m.group(1)) / (1 if m.group(2) == 's' else 60)
assert per_second >= 5, (
f'api_limit is {per_second:.2f} req/s — too tight for normal vault use'
)
def test_auth_rate_limit_stays_tight():
"""The brute-force surface must NOT be widened along with api_limit."""
m = re.search(r'zone=auth_limit:\S+\s+rate=(\d+)r/([sm]);', NGINX)
assert m, 'auth_limit zone not found'
per_minute = int(m.group(1)) * (60 if m.group(2) == 's' else 1)
assert per_minute <= 60, f'auth_limit is {per_minute} req/min — too permissive'
# -- Extension packaging -----------------------------------------------------
EXT = ROOT / 'extension'
@pytest.mark.parametrize('manifest_name',
['manifest.json', 'manifest.firefox.json'])
def test_manifest_loads_psl_before_content_script(manifest_name):
"""
content.js calls PkPsl at match time. If psl.js is missing from the manifest
the matcher silently falls back to exact-hostname equality, quietly losing
every subdomain match.
"""
import json
manifest = json.loads((EXT / manifest_name).read_text(encoding='utf-8'))
blocks = [cs for cs in manifest.get('content_scripts', [])
if any('content/content.js' in f for f in cs['js'])]
assert blocks, f'{manifest_name} has no content.js block'
for cs in blocks:
assert 'shared/psl.js' in cs['js'], f'{manifest_name}: psl.js not loaded'
assert cs['js'].index('shared/psl.js') < cs['js'].index('content/content.js'), \
f'{manifest_name}: psl.js must load BEFORE content.js'
def test_popup_html_loads_psl():
# Compare the parsed <script src> order, not raw string positions — the
# surrounding comments mention these filenames too.
html = (EXT / 'popup' / 'popup.html').read_text(encoding='utf-8')
srcs = re.findall(r'<script src="([^"]+)"', html)
assert '../shared/psl.js' in srcs, 'popup.html does not load psl.js'
assert srcs.index('../shared/psl.js') < srcs.index('popup.js'), \
f'psl.js must load before popup.js, got {srcs}'
def test_psl_includes_the_private_section():
"""
The PRIVATE section (github.io, vercel.app, herokuapp.com) is where an
attacker can actually register a neighbouring subdomain. Regenerating the
list with only the ICANN section would silently reopen finding #5.
"""
psl = (EXT / 'shared' / 'psl.js').read_text(encoding='utf-8')
for suffix in ('github.io', 'vercel.app', 'herokuapp.com'):
assert f'\n{suffix}\n' in psl, f'PSL is missing the private suffix {suffix}'
def test_no_suffix_matching_remains_in_the_extension():
"""
The endsWith("." + host) pattern is the finding-#5 bug. If it reappears,
credentials are being offered across public-suffix boundaries again.
"""
for rel in ('content/content.js', 'popup/popup.js',
'background.js', 'background.firefox.js'):
src = (EXT / rel).read_text(encoding='utf-8')
code = '\n'.join(ln for ln in src.splitlines()
if not ln.strip().startswith(('*', '//', '/*')))
for pattern in ('endsWith("." + host', 'endsWith(`.${host',
'endsWith(`.${h}`)', "endsWith('.' + host"):
assert pattern not in code, f'{rel}: suffix matching is back ({pattern})'
def test_background_scripts_load_psl():
"""
The badge counts matching items and must use the same same-site rule.
Chrome pulls psl.js in via importScripts; Firefox via background.scripts.
"""
import json
mv3 = (EXT / 'background.js').read_text(encoding='utf-8')
assert 'importScripts("shared/psl.js")' in mv3, 'background.js does not importScripts psl.js'
ff = json.loads((EXT / 'manifest.firefox.json').read_text(encoding='utf-8'))
scripts = ff['background']['scripts']
assert 'shared/psl.js' in scripts, 'firefox background does not load psl.js'
assert scripts.index('shared/psl.js') < scripts.index('background.firefox.js'), 'psl.js must load before background.firefox.js'
+173
View File
@@ -0,0 +1,173 @@
"""
Regression tests for emergency-access visibility (finding #9).
Two problems, both leaving the grantor blind to activity on their own vault:
1. Audit entries were written only under the acting user's id, and
/api/auth/audit-log filters by user_id so a grantee could request access
and retrieve the snapshot without a single line appearing in the grantor's
log or security dashboard.
2. Retrieval left no trace on the record at all: status stayed 'pending' and
nothing counted, so repeated fetches were indistinguishable from none.
Retrieval is deliberately still permitted after the first time the grantor may
be unable to re-provision, which is the whole premise so the fix is visibility
and revocability, not blocking.
"""
from datetime import datetime, timedelta, timezone
from app import db
from app.models.audit_log import AuditLog
from app.models.emergency_access import EmergencyAccess
from tests.conftest import auth_headers, make_user
GRANTOR = 'owner@example.com'
GRANTEE = 'trusted@example.com'
def _pair(client):
"""Create grantor + grantee, returns (grantor_token, grantee_token)."""
g_token, _ = make_user(client, GRANTOR, 'HASH-O', 'SALT-O')
t_token, _ = make_user(client, GRANTEE, 'HASH-T', 'SALT-T')
return g_token, t_token
def _grant(client, grantor_token, grantee_token, wait_days=7):
res = client.post('/api/emergency', headers=auth_headers(grantor_token),
json={'grantee_email': GRANTEE, 'wait_days': wait_days})
assert res.status_code == 201, res.get_json()
ea_id = res.get_json()['id']
assert client.post(f'/api/emergency/{ea_id}/accept',
headers=auth_headers(grantee_token)).status_code == 200
assert client.post(f'/api/emergency/{ea_id}/provide',
headers=auth_headers(grantor_token),
json={'enc_vault': '[{"id":1,"enc_data":"X","iv":"Y","enc_name":"N","iv_name":"I"}]'}
).status_code == 200
return ea_id
def _grantor_log(client, token):
res = client.get('/api/auth/audit-log?limit=200', headers=auth_headers(token))
assert res.status_code == 200
return res.get_json()['entries']
def _elapse_wait(ea_id):
"""Backdate the request so the wait period has passed."""
ea = db.session.get(EmergencyAccess, ea_id)
ea.request_initiated_at = (
datetime.now(timezone.utc).replace(tzinfo=None)
- timedelta(days=ea.wait_days + 1)
)
db.session.commit()
def test_access_request_appears_in_the_grantors_audit_log(client, app):
"""The grantor has wait_days to notice and deny — they must be able to see it."""
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
assert client.post(f'/api/emergency/{ea_id}/request',
headers=auth_headers(t_token)).status_code == 200
entries = _grantor_log(client, g_token)
requests = [e for e in entries if e['action'] == 'emergency_access.request']
assert requests, 'the access request is invisible in the grantor audit log'
assert 'ACTION REQUIRED' in requests[0]['detail']
assert GRANTEE in requests[0]['detail']
def test_vault_retrieval_appears_in_the_grantors_audit_log(client, app):
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
client.post(f'/api/emergency/{ea_id}/request', headers=auth_headers(t_token))
_elapse_wait(ea_id)
assert client.get(f'/api/emergency/{ea_id}/vault',
headers=auth_headers(t_token)).status_code == 200
entries = _grantor_log(client, g_token)
retrievals = [e for e in entries
if e['action'] == 'emergency_access.vault_retrieved']
assert retrievals, 'vault retrieval is invisible in the grantor audit log'
assert GRANTEE in retrievals[0]['detail']
def test_retrieval_is_counted_and_exposed_to_the_grantor(client, app):
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
client.post(f'/api/emergency/{ea_id}/request', headers=auth_headers(t_token))
_elapse_wait(ea_id)
grants = client.get('/api/emergency', headers=auth_headers(g_token)).get_json()['grants']
assert grants[0]['vault_retrieval_count'] == 0
assert grants[0]['vault_retrieved_at'] is None
for _ in range(3):
assert client.get(f'/api/emergency/{ea_id}/vault',
headers=auth_headers(t_token)).status_code == 200
grants = client.get('/api/emergency', headers=auth_headers(g_token)).get_json()['grants']
assert grants[0]['vault_retrieval_count'] == 3, 'repeated retrieval not counted'
assert grants[0]['vault_retrieved_at'] is not None, 'first retrieval not timestamped'
def test_first_retrieval_timestamp_does_not_move(client, app):
"""vault_retrieved_at records FIRST access, so it cannot be reset by re-fetching."""
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
client.post(f'/api/emergency/{ea_id}/request', headers=auth_headers(t_token))
_elapse_wait(ea_id)
client.get(f'/api/emergency/{ea_id}/vault', headers=auth_headers(t_token))
first = db.session.get(EmergencyAccess, ea_id).vault_retrieved_at
client.get(f'/api/emergency/{ea_id}/vault', headers=auth_headers(t_token))
assert db.session.get(EmergencyAccess, ea_id).vault_retrieved_at == first
def test_grantor_can_still_revoke_after_retrieval(client, app):
"""Visibility is only useful if the grantor can act on it."""
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
client.post(f'/api/emergency/{ea_id}/request', headers=auth_headers(t_token))
_elapse_wait(ea_id)
client.get(f'/api/emergency/{ea_id}/vault', headers=auth_headers(t_token))
assert client.delete(f'/api/emergency/{ea_id}',
headers=auth_headers(g_token)).status_code == 200
assert client.get(f'/api/emergency/{ea_id}/vault',
headers=auth_headers(t_token)).status_code == 404
def test_acceptance_is_visible_to_the_grantor(client, app):
g_token, t_token = _pair(client)
_grant(client, g_token, t_token)
accepts = [e for e in _grantor_log(client, g_token)
if e['action'] == 'emergency_access.accept']
assert accepts, 'grantee acceptance is invisible to the grantor'
def test_retrieval_before_the_wait_elapses_is_still_refused(client, app):
"""The wait period remains the gate — none of this loosens it."""
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
client.post(f'/api/emergency/{ea_id}/request', headers=auth_headers(t_token))
res = client.get(f'/api/emergency/{ea_id}/vault', headers=auth_headers(t_token))
assert res.status_code == 403
assert db.session.get(EmergencyAccess, ea_id).vault_retrieval_count == 0
def test_denying_a_request_stops_retrieval(client, app):
g_token, t_token = _pair(client)
ea_id = _grant(client, g_token, t_token)
client.post(f'/api/emergency/{ea_id}/request', headers=auth_headers(t_token))
assert client.post(f'/api/emergency/{ea_id}/deny',
headers=auth_headers(g_token)).status_code == 200
_elapse_wait(ea_id) # even with time passed, the request was cancelled
res = client.get(f'/api/emergency/{ea_id}/vault', headers=auth_headers(t_token))
assert res.status_code == 403
+208
View File
@@ -0,0 +1,208 @@
"""
Regression tests for silent vault destruction (review finding #2).
change_password and /recover both rotate enc_key_salt, which invalidates every
ciphertext under the previous vault key. The client re-encrypts each item and
sends it back but nothing checked that the payload actually covered every
item. A short payload rotated the key anyway and left the missing items
permanently undecryptable, with no error and a success entry in the audit log.
"""
import hashlib
import hmac
from app import db
from app.models.audit_log import AuditLog
from app.models.user import User
from app.models.vault_item import VaultItem
from tests.conftest import add_item, auth_headers, login, make_user
VERIFIER = 'c' * 64
def _payload(ids):
return [{'id': i, 'enc_data': f'NEW-{i}', 'iv': f'NEWIV-{i}'} for i in ids]
def _change_password(client, token, items, allow_partial=None):
body = {
'current_auth_hash': 'AUTH-HASH-V1',
'new_auth_hash': 'AUTH-HASH-V2',
'new_enc_key_salt': 'SALT-V2',
'items': items,
}
if allow_partial is not None:
body['allow_partial'] = allow_partial
return client.post('/api/auth/change-password', headers=auth_headers(token), json=body)
# -- change_password ---------------------------------------------------------
def test_full_payload_rotates_everything(client, app):
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(3)]
res = _change_password(client, token, _payload(ids))
assert res.status_code == 200, res.get_json()
for i in ids:
assert db.session.get(VaultItem, i).enc_data == f'NEW-{i}'
user = User.query.filter_by(email='user@example.com').first()
assert user.enc_key_salt == 'SALT-V2'
def test_short_payload_is_refused_and_nothing_changes(client, app):
"""The exact data-loss bug: one item omitted from the re-encryption."""
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(3)]
res = _change_password(client, token, _payload(ids[:2])) # third omitted
assert res.status_code == 409, res.get_json()
body = res.get_json()
assert body['code'] == 'incomplete_reencryption'
assert (body['expected'], body['received']) == (3, 2)
# Nothing may have been committed: salt unchanged, ciphertext untouched.
user = User.query.filter_by(email='user@example.com').first()
assert user.enc_key_salt == 'SALT-V1', 'key rotated despite refusal'
assert user.token_epoch == 0
for i in ids:
assert db.session.get(VaultItem, i).enc_data == 'CT'
# The old password must still work.
assert login(client).status_code == 200
def test_empty_payload_against_populated_vault_is_refused(client, app):
token, _ = make_user(client)
for _ in range(4):
add_item(client, token)
res = _change_password(client, token, [])
assert res.status_code == 409
assert res.get_json()['received'] == 0
assert User.query.filter_by(email='user@example.com').first().enc_key_salt == 'SALT-V1'
def test_refusal_is_audited(client, app):
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(2)]
_change_password(client, token, _payload(ids[:1]))
entry = (AuditLog.query
.filter_by(action='auth.change_password_failed')
.order_by(AuditLog.id.desc()).first())
assert entry is not None
assert '1 of 2' in entry.detail
def test_another_users_item_does_not_count_toward_coverage(client, app):
"""A foreign id must not pad the payload up to the expected count."""
token_a, _ = make_user(client, 'a@example.com', 'HASH-A', 'SALT-A')
token_b, _ = make_user(client, 'b@example.com', 'HASH-B', 'SALT-B')
a_ids = [add_item(client, token_a) for _ in range(2)]
b_id = add_item(client, token_b)
res = client.post('/api/auth/change-password', headers=auth_headers(token_a), json={
'current_auth_hash': 'HASH-A', 'new_auth_hash': 'HASH-A2',
'new_enc_key_salt': 'SALT-A2',
'items': _payload([a_ids[0], b_id]), # b_id does not belong to user A
})
assert res.status_code == 409
assert (res.get_json()['expected'], res.get_json()['received']) == (2, 1)
assert db.session.get(VaultItem, b_id).enc_data == 'CT'
# -- /recover ----------------------------------------------------------------
def _setup_recovery(client, token):
assert client.post('/api/auth/recovery/setup', headers=auth_headers(token), json={
'recovery_enc_salt': 'BLOB', 'recovery_iv': 'IV',
'recovery_verifier': VERIFIER,
}).status_code == 200
def _proof(client, email='user@example.com'):
nonce = client.get(f'/api/auth/recovery/data?email={email}').get_json()['nonce']
return hmac.new(VERIFIER.encode(), nonce.encode(), hashlib.sha256).hexdigest()
def _consume_challenge(client, proof):
"""Fetch items exactly as recover.js does, which rotates the challenge."""
return client.get('/api/auth/recovery/items?email=user@example.com',
headers={'X-Recovery-Proof': proof})
def test_recover_refuses_short_payload(client, app):
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(3)]
_setup_recovery(client, token)
proof = _proof(client)
_consume_challenge(client, proof)
res = client.post('/api/auth/recover', json={
'email': 'user@example.com', 'new_auth_hash': 'AUTH-HASH-V2',
'new_enc_key_salt': 'SALT-V2', 'recovery_proof': proof,
'items': _payload(ids[:2]),
})
assert res.status_code == 409, res.get_json()
user = User.query.filter_by(email='user@example.com').first()
assert user.enc_key_salt == 'SALT-V1'
assert user.recovery_enc_salt == 'BLOB', 'recovery code consumed despite refusal'
def test_recover_allows_partial_when_explicitly_confirmed(client, app):
"""
The escape hatch exists because refusing outright would leave a locked-out
user with no way into their account at all.
"""
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(3)]
_setup_recovery(client, token)
proof = _proof(client)
_consume_challenge(client, proof)
res = client.post('/api/auth/recover', json={
'email': 'user@example.com', 'new_auth_hash': 'AUTH-HASH-V2',
'new_enc_key_salt': 'SALT-V2', 'recovery_proof': proof,
'items': _payload(ids[:2]), 'allow_partial': True,
})
assert res.status_code == 200, res.get_json()
entry = (AuditLog.query.filter_by(action='auth.recovery_success')
.order_by(AuditLog.id.desc()).first())
assert 'PARTIAL' in entry.detail, 'partial recovery not flagged in the audit log'
def test_recover_full_payload_succeeds_and_consumes_the_code(client, app):
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(2)]
_setup_recovery(client, token)
proof = _proof(client)
_consume_challenge(client, proof)
res = client.post('/api/auth/recover', json={
'email': 'user@example.com', 'new_auth_hash': 'AUTH-HASH-V2',
'new_enc_key_salt': 'SALT-V2', 'recovery_proof': proof,
'items': _payload(ids),
})
assert res.status_code == 200, res.get_json()
user = User.query.filter_by(email='user@example.com').first()
assert user.enc_key_salt == 'SALT-V2'
assert user.recovery_enc_salt is None
assert user.recovery_verifier is None
assert login(client, auth_hash='AUTH-HASH-V2').status_code == 200
def test_change_password_ignores_allow_partial(client, app):
"""
Recovery has a partial-completion escape hatch; changing the password must
not. The current password keeps working, so there is never a reason to
accept permanent data loss here the server refuses even if a client asks.
"""
token, _ = make_user(client)
ids = [add_item(client, token) for _ in range(3)]
res = _change_password(client, token, _payload(ids[:1]), allow_partial=True)
assert res.status_code == 409, 'server honoured allow_partial on change-password'
assert User.query.filter_by(email='user@example.com').first().enc_key_salt == 'SALT-V1'
+168
View File
@@ -0,0 +1,168 @@
"""
Regression tests for the account lockout (two related problems).
1. Disclosure a locked account answered 429 "Account temporarily locked. Try
again in N minute(s)", confirming the address had an account. That is the same
leak /register was fixed for, reachable by anyone willing to send five wrong
passwords.
2. Denial of service the lockout was global to the account, so anyone who knew
an address could lock the real owner out for 15 minutes, repeatedly and
indefinitely. Locking someone out of their password manager is a serious harm
on its own.
Lockout is now scoped to (account, source IP): an attacker locks out only
themselves, and every failure mode returns one identical response.
"""
from app import db
from app.models.audit_log import AuditLog
from app.models.login_attempt import LoginAttempt
from app.models.user import User
from tests.conftest import make_user, register
ATTACKER = '203.0.113.9'
OWNER = '198.51.100.4'
def _login(client, ip, email='user@example.com', auth_hash='AUTH-HASH-V1'):
# ProxyFix(x_for=1) makes the rightmost XFF hop the client address, which is
# what client_ip() and the lockout key off.
return client.post('/api/auth/login',
json={'email': email, 'auth_hash': auth_hash},
headers={'X-Forwarded-For': ip})
def _lock_out(client, ip, email='user@example.com'):
for _ in range(LoginAttempt.MAX_FAILED):
_login(client, ip, email, 'WRONG-HASH')
# ── Disclosure ──────────────────────────────────────────────────────────────
def test_locked_response_matches_wrong_password(client, app):
make_user(client)
_lock_out(client, ATTACKER)
locked = _login(client, ATTACKER, auth_hash='WRONG-HASH')
wrong_on_fresh_ip = _login(client, '203.0.113.77', auth_hash='WRONG-HASH')
assert locked.status_code == wrong_on_fresh_ip.status_code == 401
assert locked.get_json() == wrong_on_fresh_ip.get_json(), (
'the locked response is distinguishable, so it discloses the account'
)
def test_locked_response_matches_unknown_account(client, app):
make_user(client)
_lock_out(client, ATTACKER)
locked = _login(client, ATTACKER, auth_hash='WRONG-HASH')
unknown = _login(client, ATTACKER, 'nobody@example.com', 'WRONG-HASH')
assert locked.status_code == unknown.status_code == 401
assert locked.get_json() == unknown.get_json()
def test_response_never_names_the_lockout(client, app):
make_user(client)
_lock_out(client, ATTACKER)
body = _login(client, ATTACKER, auth_hash='WRONG-HASH').get_json()
text = ' '.join(str(v) for v in body.values()).lower()
for leak in ('locked', 'lockout', 'minute(s) remaining', 'too many'):
assert leak not in text, f'response leaks lockout state via {leak!r}: {body}'
# ── Denial of service ───────────────────────────────────────────────────────
def test_attacker_cannot_lock_the_owner_out(client, app):
"""The core DoS fix: the victim's own IP is untouched."""
make_user(client)
_lock_out(client, ATTACKER)
assert _login(client, ATTACKER, auth_hash='WRONG-HASH').status_code == 401
res = _login(client, OWNER)
assert res.status_code == 200, 'the owner was locked out by someone else'
assert 'access_token' in res.get_json()
def test_lockout_actually_applies_to_the_offending_ip(client, app):
"""The DoS fix must not have removed brute-force protection."""
make_user(client)
_lock_out(client, ATTACKER)
# Even the CORRECT password is refused from a locked-out IP.
assert _login(client, ATTACKER).status_code == 401
row = LoginAttempt.query.filter_by(ip_address=ATTACKER).first()
assert row is not None and row.locked_until is not None
def test_each_ip_gets_its_own_budget(client, app):
make_user(client)
_lock_out(client, ATTACKER)
# A second IP is still four failures away from its own lockout.
for _ in range(LoginAttempt.MAX_FAILED - 1):
_login(client, '203.0.113.55', auth_hash='WRONG-HASH')
assert _login(client, '203.0.113.55').status_code == 200
assert LoginAttempt.query.filter_by(ip_address=ATTACKER).first().locked_until
def test_successful_login_clears_that_ips_history(client, app):
make_user(client)
for _ in range(LoginAttempt.MAX_FAILED - 1):
_login(client, OWNER, auth_hash='WRONG-HASH')
assert _login(client, OWNER).status_code == 200
assert LoginAttempt.query.filter_by(ip_address=OWNER).first() is None, (
'failure history survived a successful login, so the next typo locks out'
)
# ── Bookkeeping ─────────────────────────────────────────────────────────────
def test_lockout_is_audited(client, app):
make_user(client)
_lock_out(client, ATTACKER)
entry = (AuditLog.query.filter_by(action='auth.account_locked')
.order_by(AuditLog.id.desc()).first())
assert entry is not None
assert entry.ip_address == ATTACKER
_login(client, ATTACKER, auth_hash='WRONG-HASH')
blocked = (AuditLog.query.filter_by(action='auth.login_blocked')
.order_by(AuditLog.id.desc()).first())
assert blocked is not None, 'blocked attempts are not recorded'
def test_aggregate_counter_still_tracks_all_ips(client, app):
"""users.failed_login_count no longer gates login but stays informative."""
make_user(client)
_login(client, ATTACKER, auth_hash='WRONG-HASH')
_login(client, '203.0.113.55', auth_hash='WRONG-HASH')
assert User.query.filter_by(email='user@example.com').first().failed_login_count == 2
def test_cleanup_prunes_stale_rows(client, app):
from datetime import datetime, timedelta, timezone
make_user(client)
_login(client, ATTACKER, auth_hash='WRONG-HASH')
row = LoginAttempt.query.filter_by(ip_address=ATTACKER).first()
row.updated_at = (datetime.now(timezone.utc).replace(tzinfo=None)
- timedelta(hours=LoginAttempt.RETENTION_HOURS + 1))
db.session.commit()
assert LoginAttempt.cleanup_expired() == 1
db.session.commit()
assert LoginAttempt.query.filter_by(ip_address=ATTACKER).first() is None
def test_unknown_account_creates_no_rows(client, app):
"""No user, nothing to track — must not be a way to grow the table."""
_login(client, ATTACKER, 'nobody@example.com', 'WRONG-HASH')
assert LoginAttempt.query.count() == 0
+143
View File
@@ -0,0 +1,143 @@
"""
Regression tests for the MFA bypass (review finding #1).
The bug had two halves:
a) /login returned enc_key_salt in the MFA-pending response, before the second
factor was verified.
b) The recovery challenge was keyed on enc_key_salt, so anyone holding it
could forge a proof and pull the whole encrypted vault from the
unauthenticated /recovery/items bypassing MFA entirely.
Chained, an attacker with only the master password could exfiltrate or take over
the account. These tests pin both halves shut.
"""
import hashlib
import hmac
import json
import pyotp
from app import db
from app.models.user import User
from app.services.auth_service import encrypt_totp_secret
from tests.conftest import add_item, auth_headers, login, make_user
def _enable_mfa(email='user@example.com'):
"""Turn on TOTP directly in the DB and return the plaintext secret."""
user = User.query.filter_by(email=email).first()
secret = pyotp.random_base32()
enc, iv = encrypt_totp_secret(secret)
user.totp_secret, user.totp_iv, user.totp_enabled = enc, iv, True
db.session.commit()
return secret
def test_login_withholds_enc_key_salt_until_mfa_is_verified(client, app):
make_user(client)
secret = _enable_mfa()
res = login(client)
body = res.get_json()
assert res.status_code == 200
assert body['mfa_required'] is True
# The heart of finding #1a.
assert 'enc_key_salt' not in body, (
'enc_key_salt leaked before the second factor was verified'
)
res = client.post('/api/auth/mfa/verify', json={
'mfa_token': body['mfa_token'], 'totp_code': pyotp.TOTP(secret).now(),
})
verified = res.get_json()
assert res.status_code == 200
# Released only now, once both factors are proven.
assert verified['enc_key_salt'] == 'SALT-V1'
def test_login_without_mfa_still_returns_enc_key_salt(client):
"""The withholding must apply only to the MFA-pending path."""
make_user(client)
body = login(client).get_json()
assert 'mfa_required' not in body
assert body['enc_key_salt'] == 'SALT-V1'
# ── Finding #1b: the recovery proof must not be forgeable from enc_key_salt ──
def _setup_recovery(client, token, verifier):
res = client.post('/api/auth/recovery/setup', headers=auth_headers(token), json={
'recovery_enc_salt': 'RECOVERY-BLOB', 'recovery_iv': 'RECOVERY-IV',
'recovery_verifier': verifier,
})
assert res.status_code == 200, res.get_json()
def test_recovery_proof_cannot_be_forged_from_enc_key_salt(client, app):
"""
The attack: password known, second factor not. Previously the attacker could
key the HMAC with enc_key_salt and walk away with every encrypted item.
"""
token, _ = make_user(client)
add_item(client, token)
verifier = 'a' * 64
_setup_recovery(client, token, verifier)
nonce = client.get('/api/auth/recovery/data?email=user@example.com').get_json()['nonce']
forged = hmac.new(b'SALT-V1', nonce.encode(), hashlib.sha256).hexdigest()
res = client.get('/api/auth/recovery/items?email=user@example.com',
headers={'X-Recovery-Proof': forged})
assert res.status_code == 401, 'enc_key_salt still forges a valid recovery proof'
def test_recovery_proof_from_verifier_is_accepted(client, app):
"""The legitimate holder of the recovery code must still get through."""
token, _ = make_user(client)
add_item(client, token)
verifier = 'b' * 64
_setup_recovery(client, token, verifier)
data = client.get('/api/auth/recovery/data?email=user@example.com').get_json()
assert data['proof_scheme'] == 'verifier'
proof = hmac.new(verifier.encode(), data['nonce'].encode(), hashlib.sha256).hexdigest()
res = client.get('/api/auth/recovery/items?email=user@example.com',
headers={'X-Recovery-Proof': proof})
assert res.status_code == 200, res.get_json()
assert len(res.get_json()['items']) == 1
def test_legacy_account_falls_back_to_enc_key_salt_proof(client, app):
"""
Recovery codes created before recovery_verifier must keep working, and be
reported as legacy so the UI can prompt a regeneration.
"""
token, _ = make_user(client)
add_item(client, token)
user = User.query.filter_by(email='user@example.com').first()
user.recovery_enc_salt, user.recovery_iv = 'BLOB', 'IV'
user.recovery_verifier = None # pre-migration state
db.session.commit()
status = client.get('/api/auth/recovery/status', headers=auth_headers(token)).get_json()
assert status['recovery_configured'] is True
assert status['recovery_is_legacy'] is True
data = client.get('/api/auth/recovery/data?email=user@example.com').get_json()
assert data['proof_scheme'] == 'legacy'
proof = hmac.new(b'SALT-V1', data['nonce'].encode(), hashlib.sha256).hexdigest()
res = client.get('/api/auth/recovery/items?email=user@example.com',
headers={'X-Recovery-Proof': proof})
assert res.status_code == 200
def test_recovery_setup_rejects_malformed_verifier(client):
token, _ = make_user(client)
for bad in ('', 'short', 'g' * 64, 'A' * 63):
res = client.post('/api/auth/recovery/setup', headers=auth_headers(token), json={
'recovery_enc_salt': 'B', 'recovery_iv': 'IV', 'recovery_verifier': bad,
})
assert res.status_code == 400, f'accepted malformed verifier {bad!r}'
+111
View File
@@ -0,0 +1,111 @@
"""
Regression tests for registration account-existence disclosure (finding #8).
/register answered 409 "Email already registered", which let anyone probe
whether a given address has a PassKeeper account a ready-made target list for
phishing, and precisely what /login goes out of its way not to reveal.
Both branches must now be indistinguishable in status, body, and timing.
"""
import time
from app import db
from app.models.audit_log import AuditLog
from app.models.user import User
from tests.conftest import login, register
def test_duplicate_registration_is_indistinguishable(client, app):
first = register(client, 'user@example.com', 'HASH-1', 'SALT-1')
second = register(client, 'user@example.com', 'HASH-2', 'SALT-2')
assert first.status_code == second.status_code == 202
assert first.get_json() == second.get_json(), (
'the response differs for an existing address, so it can be probed'
)
def test_duplicate_registration_does_not_touch_the_existing_account(client, app):
"""The generic response must not come at the cost of overwriting credentials."""
register(client, 'user@example.com', 'HASH-1', 'SALT-1')
original = User.query.filter_by(email='user@example.com').first()
original_hash, original_salt = original.master_hash, original.enc_key_salt
register(client, 'user@example.com', 'ATTACKER-HASH', 'ATTACKER-SALT')
user = User.query.filter_by(email='user@example.com').first()
assert user.master_hash == original_hash, 'existing credentials overwritten'
assert user.enc_key_salt == original_salt
assert User.query.filter_by(email='user@example.com').count() == 1
# The original password must still be the one that works.
assert login(client, 'user@example.com', 'HASH-1').status_code == 200
assert login(client, 'user@example.com', 'ATTACKER-HASH').status_code == 401
def test_registration_response_does_not_name_the_cause(client, app):
register(client, 'user@example.com')
body = register(client, 'user@example.com').get_json()
text = ' '.join(str(v) for v in body.values()).lower()
for leak in ('already', 'exists', 'taken', 'registered account', 'duplicate'):
assert leak not in text, f'response body leaks existence via {leak!r}: {body}'
def test_timing_does_not_disclose_existence(client, app):
"""
Creating an account runs Argon2id, which is deliberately slow. If the
duplicate branch returned early it would be measurably faster and the oracle
would survive in the timing even though the body is identical.
Uses a loose bound: this asserts the expensive work happens on both paths,
not that timing is cryptographically uniform.
"""
register(client, 'taken@example.com')
def elapsed(email):
start = time.perf_counter()
register(client, email)
return time.perf_counter() - start
new_times = [elapsed(f'fresh{i}@example.com') for i in range(3)]
dup_times = [elapsed('taken@example.com') for _ in range(3)]
new_avg = sum(new_times) / len(new_times)
dup_avg = sum(dup_times) / len(dup_times)
slower, faster = max(new_avg, dup_avg), min(new_avg, dup_avg)
assert slower < faster * 4, (
f'timing distinguishes the branches: new={new_avg:.4f}s dup={dup_avg:.4f}s'
)
def test_duplicate_attempt_is_audited(client, app):
"""Invisible to the prober, but the operator should still see the attempts."""
register(client, 'user@example.com')
register(client, 'user@example.com')
entry = (AuditLog.query.filter_by(action='auth.register_duplicate')
.order_by(AuditLog.id.desc()).first())
assert entry is not None, 'duplicate registration attempt was not audited'
assert 'user@example.com' not in (entry.detail or ''), (
'audit detail should not need the probed address to be useful'
)
def test_validation_errors_still_reported(client, app):
"""Input validation does not reveal existence, so it stays specific."""
assert register(client, 'not-an-email').status_code == 400
assert client.post('/api/auth/register', json={'email': 'a@b.co'}).status_code == 400
assert client.post('/api/auth/register',
json={'email': 'a@b.co', 'auth_hash': 'h'}).status_code == 400
def test_new_registration_still_creates_a_usable_account(client, app):
"""The privacy fix must not break the happy path."""
assert register(client, 'fresh@example.com', 'HASH', 'SALT').status_code == 202
assert User.query.filter_by(email='fresh@example.com').first() is not None
res = login(client, 'fresh@example.com', 'HASH')
assert res.status_code == 200
assert res.get_json()['enc_key_salt'] == 'SALT'
+148
View File
@@ -0,0 +1,148 @@
"""
Regression tests for session revocation (review finding #3).
Changing the master password used to leave every outstanding access and refresh
token valid. The response said "Please log in again" but nothing enforced it, so
a stolen refresh token kept working for its full 7-day lifetime after the victim
changed the password it was obtained under.
Every JWT now carries an `epoch` claim checked against users.token_epoch.
Also covers the sub-finding that require_jwt never confirmed the user still
existed: a valid token for a deleted account dereferenced None and returned 500.
"""
import jwt as pyjwt
from app import db
from app.models.user import User
from tests.conftest import add_item, auth_headers, login, make_user
def _rotate_password(client, token, ids):
"""Complete a full, valid password change."""
res = client.post('/api/auth/change-password', headers=auth_headers(token), json={
'current_auth_hash': 'AUTH-HASH-V1',
'new_auth_hash': 'AUTH-HASH-V2',
'new_enc_key_salt': 'SALT-V2',
'items': [{'id': i, 'enc_data': f'NEW-{i}', 'iv': f'IV-{i}'} for i in ids],
})
assert res.status_code == 200, res.get_json()
return res
def test_access_token_is_revoked_by_password_change(client, app):
token, _ = make_user(client)
ids = [add_item(client, token)]
assert client.get('/api/vault', headers=auth_headers(token)).status_code == 200
_rotate_password(client, token, ids)
res = client.get('/api/vault', headers=auth_headers(token))
assert res.status_code == 401, 'access token survived the password change'
def test_refresh_token_is_revoked_by_password_change(client, app):
"""
The more damaging half: a refresh token is valid for 7 days and can mint
fresh access tokens indefinitely.
"""
token, refresh = make_user(client)
ids = [add_item(client, token)]
_rotate_password(client, token, ids)
res = client.post('/api/auth/refresh', json={'refresh_token': refresh})
assert res.status_code == 401, 'refresh token survived the password change'
def test_epoch_increments_and_new_login_works(client, app):
token, _ = make_user(client)
ids = [add_item(client, token)]
_rotate_password(client, token, ids)
user = User.query.filter_by(email='user@example.com').first()
assert user.token_epoch == 1
res = login(client, auth_hash='AUTH-HASH-V2')
assert res.status_code == 200
new_token = res.get_json()['access_token']
assert client.get('/api/vault', headers=auth_headers(new_token)).status_code == 200
def test_recovery_also_revokes_prior_sessions(client, app):
"""An attacker holding a token must not survive the victim recovering."""
import hashlib
import hmac
verifier = 'd' * 64
token, _ = make_user(client)
ids = [add_item(client, token)]
assert client.post('/api/auth/recovery/setup', headers=auth_headers(token), json={
'recovery_enc_salt': 'BLOB', 'recovery_iv': 'IV',
'recovery_verifier': verifier,
}).status_code == 200
nonce = client.get('/api/auth/recovery/data?email=user@example.com').get_json()['nonce']
proof = hmac.new(verifier.encode(), nonce.encode(), hashlib.sha256).hexdigest()
client.get('/api/auth/recovery/items?email=user@example.com',
headers={'X-Recovery-Proof': proof})
res = client.post('/api/auth/recover', json={
'email': 'user@example.com', 'new_auth_hash': 'AUTH-HASH-V2',
'new_enc_key_salt': 'SALT-V2', 'recovery_proof': proof,
'items': [{'id': i, 'enc_data': f'NEW-{i}', 'iv': f'IV-{i}'} for i in ids],
})
assert res.status_code == 200, res.get_json()
assert client.get('/api/vault', headers=auth_headers(token)).status_code == 401
# The tokens handed back by /recover must carry the NEW epoch and work.
fresh = res.get_json()['access_token']
assert client.get('/api/vault', headers=auth_headers(fresh)).status_code == 200
def test_token_for_deleted_account_is_401_not_500(client, app):
"""Previously this dereferenced None inside the handler and returned 500."""
token, _ = make_user(client)
user = User.query.filter_by(email='user@example.com').first()
db.session.delete(user)
db.session.commit()
for path in ('/api/vault', '/api/auth/me', '/api/sharing/keys', '/api/emergency'):
res = client.get(path, headers=auth_headers(token))
assert res.status_code == 401, f'{path} returned {res.status_code}'
def test_forged_epoch_claim_is_rejected(client, app):
"""
The epoch is inside the signed payload, so tampering invalidates the
signature. Re-signing with the wrong key must also fail.
"""
token, _ = make_user(client)
payload = pyjwt.decode(token, options={'verify_signature': False})
payload['epoch'] = 99
forged = pyjwt.encode(payload, 'not-the-real-signing-key', algorithm='HS256')
assert client.get('/api/vault', headers=auth_headers(forged)).status_code == 401
def test_tokens_predating_the_epoch_claim_still_work(client, app):
"""
Deploying this must not sign existing sessions out: tokens minted before the
claim existed decode with epoch 0, matching the column default.
"""
token, _ = make_user(client)
payload = pyjwt.decode(token, options={'verify_signature': False})
del payload['epoch'] # simulate a pre-upgrade token
legacy = pyjwt.encode(payload, app.config['JWT_SECRET_KEY'], algorithm='HS256')
assert client.get('/api/vault', headers=auth_headers(legacy)).status_code == 200
def test_logout_still_revokes_via_blacklist(client, app):
"""Epoch checking must not have displaced the existing jti blacklist."""
token, refresh = make_user(client)
assert client.post('/api/auth/logout', headers=auth_headers(token),
json={'refresh_token': refresh}).status_code == 200
assert client.get('/api/vault', headers=auth_headers(token)).status_code == 401
assert client.post('/api/auth/refresh',
json={'refresh_token': refresh}).status_code == 401
+87
View File
@@ -0,0 +1,87 @@
"""
Regression tests for share expiry failing open (review finding #10).
create_share parsed expires_days inside `try: ... except: pass`, so any value it
could not parse silently became "never expires" the opposite of what the user
asked for, with no error to notice.
"""
from datetime import datetime, timedelta, timezone
from tests.conftest import add_item, auth_headers, make_user
def _share(client, token, item_id, recipient='friend@example.com', **extra):
body = {
'item_id': item_id,
'recipient_email': recipient,
'enc_data': 'ECDH-CT',
'iv': 'ECDH-IV',
'item_name': 'password',
'item_type': 'password',
}
body.update(extra)
return client.post('/api/sharing', headers=auth_headers(token), json=body)
def test_valid_expiry_is_applied(client, app):
token, _ = make_user(client)
item_id = add_item(client, token)
res = _share(client, token, item_id, expires_days=7)
assert res.status_code == 201, res.get_json()
expires_at = res.get_json()['expires_at']
assert expires_at is not None
parsed = datetime.fromisoformat(expires_at)
expected = datetime.now(timezone.utc).replace(tzinfo=None) + timedelta(days=7)
assert abs((parsed - expected).total_seconds()) < 60
def test_null_expiry_means_never(client, app):
token, _ = make_user(client)
item_id = add_item(client, token)
res = _share(client, token, item_id, expires_days=None)
assert res.status_code == 201
assert res.get_json()['expires_at'] is None
def test_omitted_expiry_means_never(client, app):
token, _ = make_user(client)
item_id = add_item(client, token)
res = _share(client, token, item_id)
assert res.status_code == 201
assert res.get_json()['expires_at'] is None
def test_unparseable_expiry_is_rejected_not_silently_dropped(client, app):
"""The bug: 'seven' used to yield a share that never expires."""
token, _ = make_user(client)
for bad in ('seven', '7 days', {}, [], 'NaN', ''):
item_id = add_item(client, token)
res = _share(client, token, item_id, expires_days=bad)
assert res.status_code == 400, (
f'expires_days={bad!r} accepted; share would never expire'
)
assert 'expires_days' in res.get_json()['error']
def test_out_of_range_expiry_is_rejected(client, app):
token, _ = make_user(client)
for bad in (-1, -30, 4000):
item_id = add_item(client, token)
res = _share(client, token, item_id, expires_days=bad)
assert res.status_code == 400, f'expires_days={bad!r} accepted'
def test_zero_expiry_means_never(client, app):
"""0 is 'no expiry', consistent with null — not 'expires immediately'."""
token, _ = make_user(client)
item_id = add_item(client, token)
res = _share(client, token, item_id, expires_days=0)
assert res.status_code == 201
assert res.get_json()['expires_at'] is None
+83
View File
@@ -0,0 +1,83 @@
"""
Regression tests for passkey user verification (review finding #4).
Both ceremonies used UserVerificationRequirement.PREFERRED with
require_user_verification=False, so an authenticator was free to skip the
biometric/PIN check. Since a passkey assertion here replaces BOTH the password
and the TOTP second factor, that reduced a full login to possession of an
unlocked device enough to enumerate and delete vault items.
A full ceremony needs a real authenticator, so these tests pin the negotiated
options (what the server asks the browser for) and the rejection path. The
enforcement half require_user_verification=True passed to py-webauthn is
asserted directly against the source.
"""
import inspect
import re
from tests.conftest import auth_headers, make_user
import app.routes.webauthn as webauthn_routes
def test_registration_options_require_user_verification(client, app):
token, _ = make_user(client)
res = client.post('/api/webauthn/register/begin',
headers=auth_headers(token), json={})
assert res.status_code == 200, res.get_json()
body = res.get_json()
assert body['authenticatorSelection']['userVerification'] == 'required'
def test_authentication_options_require_user_verification(client, app):
make_user(client)
res = client.post('/api/webauthn/authenticate/begin',
json={'email': 'user@example.com'})
assert res.status_code == 200, res.get_json()
assert res.get_json()['userVerification'] == 'required'
def test_verification_calls_enforce_user_verification():
"""
Negotiating 'required' is only a request to the browser. The server must
also refuse an assertion that comes back without the UV flag set, or the
hint is decorative.
"""
src = inspect.getsource(webauthn_routes)
calls = re.findall(r'require_user_verification=(\w+)', src)
assert calls, 'no require_user_verification argument found'
assert all(v == 'True' for v in calls), (
f'require_user_verification must be True everywhere, found: {calls}'
)
def test_unknown_credential_is_rejected(client, app):
make_user(client)
client.post('/api/webauthn/authenticate/begin', json={'email': 'user@example.com'})
res = client.post('/api/webauthn/authenticate/complete',
json={'id': 'bm9wZQ', 'rawId': 'bm9wZQ'})
assert res.status_code == 401
assert 'not recognised' in res.get_json()['error']
def test_registration_failure_does_not_leak_exception_text(client, app):
"""
CLAUDE.md forbids returning str(e) to clients; this handler used to embed
the raw py-webauthn message, which quotes attestation internals.
"""
token, _ = make_user(client)
client.post('/api/webauthn/register/begin', headers=auth_headers(token), json={})
res = client.post('/api/webauthn/register/complete',
headers=auth_headers(token), json={'id': 'garbage'})
assert res.status_code == 400
error = res.get_json()['error']
assert error == 'Could not verify this passkey. Please try again.', error
def test_register_complete_requires_a_pending_challenge(client, app):
token, _ = make_user(client)
res = client.post('/api/webauthn/register/complete',
headers=auth_headers(token), json={'id': 'x'})
assert res.status_code == 400
assert 'No pending registration challenge' in res.get_json()['error']