feat(security): SSN envelope encryption at rest + scrypt admin-PIN KDF (items 1+2)#217
feat(security): SSN envelope encryption at rest + scrypt admin-PIN KDF (items 1+2)#217arigatoexpress wants to merge 2 commits into
Conversation
…F (items 1+2) Both ship INERT/backward-compatible — no behaviour change until an operator provisions the gated material, the same pattern as the inert inbound-email webhook. SSN-at-rest (item 1): deals persist raw buyer_ssn/co_buyer_ssn to Firestore for doc-gen. Firestore encrypts at rest with Google keys + the API strips SSN from every response, but a Firestore-level compromise could expose plaintext. New tools/field_crypto.py adds Cloud KMS envelope encryption at the deal-DB choke points (create_deal/update_deal encrypt; get_deal decrypts transparently). * Inert until SSN_KMS_KEY is set (identity passthrough). * decrypt passes legacy plaintext through, so a migration can run incrementally. * encrypt is idempotent (never double-encrypts) and fails CLOSED on KMS error. * scripts/migrate_encrypt_deal_ssns.py back-fills existing deals (dry-run default). Admin-PIN (item 2): ADMIN_PIN_HASH was unsalted SHA-256 — a 4-digit PIN is brute-forceable in microseconds if the hash leaks. _verify_admin_pin_value now also accepts a salted scrypt hash (scrypt$n$r$p$salt$dk) while staying backward-compatible with the legacy SHA-256 hex, so migration is downtime-free. scripts/generate_admin_pin_hash.py emits the new format. Verified: 18 new tests + admin_auth + PII suites green, ruff clean. Adds google-cloud-kms==3.13.0 (lazy-imported; only loaded once a key is configured). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
scripts/ is gitignored by default; these are intentional ops utilities (force- added like the other tracked scripts). The PIN-KDF test imports the generator, so it must be tracked for CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1102da8be7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stored = ADMIN_PIN_HASH | ||
| if not stored or not pin: | ||
| return False | ||
| if stored.startswith("scrypt$"): |
There was a problem hiding this comment.
When ADMIN_PIN_HASH is switched to the new generated scrypt value, this branch makes that value valid for PIN login, but _JWT_SECRET is still derived from ADMIN_PIN_HASH[:16] at main.py:920. All hashes produced by the new generator with default params start with the same 16 characters (scrypt$16384$8$1), so the HMAC key for tho_admin_token becomes predictable and an attacker can forge admin bearer tokens without knowing the PIN, salt, or derived key. Derive the token-signing secret from the full hash or another real secret before accepting scrypt hashes.
Useful? React with 👍 / 👎.
|
|
||
| from google.cloud import firestore | ||
|
|
||
| from tools import field_crypto |
There was a problem hiding this comment.
When an operator runs the documented command python scripts/migrate_encrypt_deal_ssns.py, Python puts scripts/ rather than the repository root on sys.path; unlike the existing migration scripts, this file never inserts its parent directory, so from tools import field_crypto raises ModuleNotFoundError unless the operator happens to set PYTHONPATH=. manually. Add the repo root to sys.path before importing repo modules so the dry-run/apply migration actually starts.
Useful? React with 👍 / 👎.
1 participant
Two security hardenings, both inert/backward-compatible until an operator provisions gated material (same pattern as the inert email webhook).
SSN at-rest (item 1): Cloud KMS envelope encryption of deal
buyer_ssn/co_buyer_ssnat the deal-DB choke points. Inert untilSSN_KMS_KEYset; decrypt passes legacy plaintext through (incremental migration); idempotent; fails closed on KMS error.scripts/migrate_encrypt_deal_ssns.pyback-fills (dry-run default).Admin-PIN (item 2):
_verify_admin_pin_valuenow accepts a salted scrypt hash while staying compatible with the legacy SHA-256 hex — downtime-free migration.scripts/generate_admin_pin_hash.pyemits the new format.Adds
google-cloud-kms==3.13.0(lazy-imported). Verified: 18 new tests + admin_auth + PII suites green, ruff clean. Activation runbook → memory.