feat: HMAC-verified account cache (#80) #85

Merged

maximus merged 1 commit from issue-80-subscription-integrity into main

2026-04-14 12:12:17 +00:00

maximus commented

2026-04-14 12:08:53 +00:00

Owner

Fixes #80
Refs #66

Summary

Closes the subscription_status tampering trap. Before this PR, writing {"subscription_status": "active"} to account.json granted Premium features without touching the Logto session.

New account_cache module: HMAC-SHA256 signs the serialised AccountInfo with a 32-byte keychain-stored key.
load_verified fails closed on legacy/tampered/missing-key payloads.
license_commands::check_account_edition now uses load_verified — Premium stays locked until the next token refresh re-signs the cache.
handle_auth_callback, refresh_auth_token, logout, get_account_info refactored through account_cache.

Choice vs issue options

The issue listed three options. Picked Option B (HMAC signed cache) because:

Sync verification — no async ripple through check_entitlement / current_edition.
No network on gating decisions (Option A adds 100-500ms latency per call).
Trust anchor is the keychain, which is the same boundary as OAuth tokens (#78).
Transparent to the UI — get_account_info still returns the profile info for display.

Test plan

cargo test 23/23 pass (6 new tests for account_cache: sign/verify, tamper detection, wrong key, envelope serde, key encode roundtrip, key length validation)
npm run build clean
Manual: tamper with account.json after login, verify Premium features lock again (deferred to #82)
Manual: logout clears both account.json and keychain HMAC key

Fixes #80 Refs #66 ## Summary Closes the `subscription_status` tampering trap. Before this PR, writing `{"subscription_status": "active"}` to `account.json` granted Premium features without touching the Logto session. - New `account_cache` module: HMAC-SHA256 signs the serialised AccountInfo with a 32-byte keychain-stored key. - `load_verified` fails closed on legacy/tampered/missing-key payloads. - `license_commands::check_account_edition` now uses `load_verified` — Premium stays locked until the next token refresh re-signs the cache. - `handle_auth_callback`, `refresh_auth_token`, `logout`, `get_account_info` refactored through `account_cache`. ## Choice vs issue options The issue listed three options. Picked **Option B (HMAC signed cache)** because: - Sync verification — no async ripple through `check_entitlement` / `current_edition`. - No network on gating decisions (Option A adds 100-500ms latency per call). - Trust anchor is the keychain, which is the same boundary as OAuth tokens (#78). - Transparent to the UI — `get_account_info` still returns the profile info for display. ## Test plan - [x] `cargo test` 23/23 pass (6 new tests for account_cache: sign/verify, tamper detection, wrong key, envelope serde, key encode roundtrip, key length validation) - [x] `npm run build` clean - [ ] Manual: tamper with account.json after login, verify Premium features lock again (deferred to #82) - [ ] Manual: logout clears both account.json and keychain HMAC key

maximus added 1 commit 2026-04-14 12:08:54 +00:00

feat: HMAC-sign cached account info to close subscription tampering (#80 )

PR Check / rust (push) Successful in 26m11s

Details

PR Check / frontend (push) Successful in 2m20s

Details

PR Check / rust (pull_request) Successful in 22m22s

Details

PR Check / frontend (pull_request) Successful in 2m18s

Details

2d7d1e05d2

Before this change, `license_commands::check_account_edition` read
`account.json` directly and granted Premium when `subscription_status`
was `"active"`. Any local process could write that JSON and bypass
the paywall without ever touching the Logto session.

Introduce `account_cache` with:
- `save(app, &AccountInfo)` — signs the serialised AccountInfo with
  HMAC-SHA256 and writes a `{"data", "sig"}` envelope. The 32-byte
  key lives in the OS keychain (service `com.simpl.resultat`, user
  `account-hmac-key`) alongside the OAuth tokens from #78.
- `load_unverified` — accepts both signed and legacy payloads for UI
  display (name, email, picture). The license path must never use
  this.
- `load_verified` — requires a valid HMAC signature; returns None for
  legacy payloads, missing keychain, tampered data. Used by
  `check_account_edition` so Premium stays locked until the next
  token refresh re-signs the cache.
- `delete` — wipes both the file and the keychain key on logout so
  the next session generates a fresh cryptographic anchor.

`auth_commands::handle_auth_callback` and `refresh_auth_token` now
call `account_cache::save` instead of writing the file directly.
`logout` clears both stores. `get_account_info` delegates to
`load_unverified` so upgraded users see their profile immediately.

Trust boundary: the HMAC key lives in the keychain and shares its
security model with the OAuth tokens. If the keychain is unreachable,
the gating path refuses to grant Premium (fail-closed), which matches
the store_mode policy introduced in #78.

Refs #66, CWE-345

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

maximus commented

2026-04-14 12:10:39 +00:00

Author

Owner

Review — APPROVE ✓

Security

✓ HMAC-SHA256 via hmac crate — primitive correcte
✓ verify_slice est constant-time (pas de timing leak)
✓ Clé 32 bytes générée via rand::thread_rng().fill_bytes (OsRng-backed)
✓ Fail-closed partout : legacy cache, tampering, keychain indispo → load_verified retourne None → Premium reste gated
✓ logout supprime bien la clé HMAC du keychain pour que la prochaine session ait un anchor frais

Correctness

✓ Round-trip serde déterministe : save() signe serde_json::to_vec(&account), load_verified re-sérialise envelope.data → mêmes bytes (serde produit dans l'ordre des champs du struct)
✓ license_commands::check_account_edition utilise bien load_verified (pas load_unverified)
✓ get_account_info utilise load_unverified pour que l'UI affiche l'email d'un utilisateur upgradé depuis v0.7.x sans re-login
✓ Tous les fs::read/write directs d'account.json sont supprimés (grep confirme)

Quality

✓ 6 tests unitaires : sign/verify, tamper detection, wrong key, envelope serde, encode roundtrip, length validation
✓ Séparation claire load_verified (gating) / load_unverified (display)
✓ Commentaires expliquent le rationale fail-closed et la compat legacy

Observations mineures

Le test manuel de tampering sur account.json (écriture d'un JSON valide mais non-signé) est reporté à #82 — la logique est couverte par les tests unitaires (verify_rejects_tampered_payload, verify_rejects_wrong_key), mais un test d'intégration bout-à-bout serait valuable.
Si un attaquant écrit un envelope {data, sig} bien formé mais avec une sig invalide, load_verified retourne None. C'est bien géré.

Verdict : APPROVE

## Review — APPROVE ✓ ### Security - ✓ HMAC-SHA256 via `hmac` crate — primitive correcte - ✓ `verify_slice` est constant-time (pas de timing leak) - ✓ Clé 32 bytes générée via `rand::thread_rng().fill_bytes` (OsRng-backed) - ✓ Fail-closed partout : legacy cache, tampering, keychain indispo → `load_verified` retourne None → Premium reste gated - ✓ `logout` supprime bien la clé HMAC du keychain pour que la prochaine session ait un anchor frais ### Correctness - ✓ Round-trip serde déterministe : `save()` signe `serde_json::to_vec(&account)`, `load_verified` re-sérialise `envelope.data` → mêmes bytes (serde produit dans l'ordre des champs du struct) - ✓ `license_commands::check_account_edition` utilise bien `load_verified` (pas `load_unverified`) - ✓ `get_account_info` utilise `load_unverified` pour que l'UI affiche l'email d'un utilisateur upgradé depuis v0.7.x sans re-login - ✓ Tous les fs::read/write directs d'account.json sont supprimés (grep confirme) ### Quality - ✓ 6 tests unitaires : sign/verify, tamper detection, wrong key, envelope serde, encode roundtrip, length validation - ✓ Séparation claire `load_verified` (gating) / `load_unverified` (display) - ✓ Commentaires expliquent le rationale fail-closed et la compat legacy ### Observations mineures - Le test manuel de tampering sur account.json (écriture d'un JSON valide mais non-signé) est reporté à #82 — la logique est couverte par les tests unitaires (`verify_rejects_tampered_payload`, `verify_rejects_wrong_key`), mais un test d'intégration bout-à-bout serait valuable. - Si un attaquant écrit un envelope `{data, sig}` bien formé mais avec une sig invalide, `load_verified` retourne None. C'est bien géré. **Verdict : APPROVE**