Store API keys hashed at rest instead of plaintext #18

Closed
opened 2026-08-07 10:17:38 +00:00 by james.oates · 1 comment
Owner

Problem

API key values are stored plaintext in the api_keys table. db.validate_api_key does a literal WHERE key_value = ? match, and create_api_key inserts the raw value — nothing is hashed. So a leak of just the SQLite file hands an attacker every live API key, including the master key.

This is inconsistent with the rest of the secret-handling posture: Cloudflare API tokens and ACME private keys are AES-256-GCM encrypted at rest (ENCRYPTION_KEY, crypto.rs), but the API keys — equally sensitive bearer credentials — are not protected at all. The "Save this key securely! It cannot be retrieved again" message printed at first boot is also misleading: SELECT key_value FROM api_keys WHERE key_type='master' recovers it.

Suggested fix

  • Store a hash of each key (SHA-256 is adequate for a high-entropy random token; a slow KDF is overkill here since keys aren't low-entropy passwords). Validate by hashing the presented key and matching the hash.
  • Show the raw key only once, at creation. Stop returning key_value from list_api_keys (it would only be a hash anyway).
  • Migration: existing values are currently plaintext, so a one-time startup rehash can hash them in place (SQLite has no built-in SHA-256, so do it in app code, guarded by a "already hashed?" marker — e.g. a schema column or a stored-format sentinel). No operator reissue needed — presented raw keys still validate after the rehash.

Coordination

Depends on / coordinates with the MASTER_API_KEY env var work (#B below): that path must hash the provided value on ingestion too. The env var always carries the raw key; storage is the implementation detail that changes here.

Security hardening — not urgent, but a real gap for an off-the-shelf product. Surfaced 2026-08-07.

## Problem API key values are stored **plaintext** in the `api_keys` table. `db.validate_api_key` does a literal `WHERE key_value = ?` match, and `create_api_key` inserts the raw value — nothing is hashed. So a leak of just the SQLite file hands an attacker every live API key, including the master key. This is inconsistent with the rest of the secret-handling posture: Cloudflare API tokens and ACME private keys **are** AES-256-GCM encrypted at rest (`ENCRYPTION_KEY`, `crypto.rs`), but the API keys — equally sensitive bearer credentials — are not protected at all. The "Save this key securely! It cannot be retrieved again" message printed at first boot is also misleading: `SELECT key_value FROM api_keys WHERE key_type='master'` recovers it. ## Suggested fix - Store a hash of each key (SHA-256 is adequate for a high-entropy random token; a slow KDF is overkill here since keys aren't low-entropy passwords). Validate by hashing the presented key and matching the hash. - Show the raw key only once, at creation. Stop returning `key_value` from `list_api_keys` (it would only be a hash anyway). - Migration: existing values are currently plaintext, so a one-time startup rehash can hash them in place (SQLite has no built-in SHA-256, so do it in app code, guarded by a "already hashed?" marker — e.g. a schema column or a stored-format sentinel). No operator reissue needed — presented raw keys still validate after the rehash. ## Coordination Depends on / coordinates with the `MASTER_API_KEY` env var work (#B below): that path must hash the provided value on ingestion too. The env var always carries the **raw** key; storage is the implementation detail that changes here. Security hardening — not urgent, but a real gap for an off-the-shelf product. Surfaced 2026-08-07.
Author
Owner

Implemented in 7f68b22.

Approach: SHA-256 hash at rest, stored as sha256:<hex> (plain SHA-256 is sufficient for high-entropy random tokens — a slow KDF adds nothing). Hashing happens at the DB boundary — create_api_key, set_api_key_value, validate_api_key all hash their raw input, so every caller keeps passing raw. rehash_plaintext_api_keys() upgrades any pre-existing plaintext rows in place (idempotent; skips already-hashed), run at startup before master-key seeding so both sides of the hash(env) == stored_hash comparison are hashes.

Coordination with MASTER_API_KEY (the #B path): decide_master_key now compares hash(env) == stored_hash while Seed/Rotate still carry the raw value (storage hashes on write). Startup rejects a MASTER_API_KEY beginning with the sha256: sentinel, so a raw key can't be mistaken for already-hashed and silently locked out.

No operator reissue — presented raw keys still validate after the rehash. list_api_keys no longer returns key_value (#[serde(skip_serializing)]); the raw key is shown once at creation via the separate CreateKeyResponse. The first-boot "Save this key securely! It cannot be retrieved again" message is now actually true (it was previously recoverable via SELECT key_value).

Known properties to be aware of:

  • One-way door. Once the rehash runs, rolling back to a pre-#18 binary (raw WHERE key_value = <raw> match) can't validate stored keys — service keys stay hashed; the master key only self-heals if MASTER_API_KEY is set. Reverting needs a pre-rehash DB snapshot, not just an image-tag change. (Noted for devinfra, which runs :develop, in the deployment-topology memory.)
  • ApiKey.key_value is now write-only over serde (Deserialize still present but unused) — a future endpoint accepting an ApiKey body would be asymmetric. Inert today; flagged so it isn't a surprise.

Tests: hash determinism/format/detection; create stores hash not raw + raw still validates; rehash one-shot + idempotent + heals a simulated legacy plaintext row; rotation stores a hash; decide_master_key hash-comparison branches; session-login e2e unchanged. Full suite green (196), clippy + fmt clean.

Unblocks #24 (passkeys), whose ticket noted it sensibly lands after this.

Implemented in `7f68b22`. **Approach:** SHA-256 hash at rest, stored as `sha256:<hex>` (plain SHA-256 is sufficient for high-entropy random tokens — a slow KDF adds nothing). Hashing happens at the DB boundary — `create_api_key`, `set_api_key_value`, `validate_api_key` all hash their raw input, so every caller keeps passing raw. `rehash_plaintext_api_keys()` upgrades any pre-existing plaintext rows in place (idempotent; skips already-hashed), run at startup **before** master-key seeding so both sides of the `hash(env) == stored_hash` comparison are hashes. **Coordination with `MASTER_API_KEY` (the #B path):** `decide_master_key` now compares `hash(env) == stored_hash` while `Seed`/`Rotate` still carry the **raw** value (storage hashes on write). Startup rejects a `MASTER_API_KEY` beginning with the `sha256:` sentinel, so a raw key can't be mistaken for already-hashed and silently locked out. **No operator reissue** — presented raw keys still validate after the rehash. `list_api_keys` no longer returns `key_value` (`#[serde(skip_serializing)]`); the raw key is shown once at creation via the separate `CreateKeyResponse`. The first-boot "Save this key securely! It cannot be retrieved again" message is now **actually true** (it was previously recoverable via `SELECT key_value`). **Known properties to be aware of:** - **One-way door.** Once the rehash runs, rolling back to a pre-#18 binary (raw `WHERE key_value = <raw>` match) can't validate stored keys — service keys stay hashed; the master key only self-heals if `MASTER_API_KEY` is set. Reverting needs a **pre-rehash DB snapshot**, not just an image-tag change. (Noted for devinfra, which runs `:develop`, in the deployment-topology memory.) - `ApiKey.key_value` is now write-only over serde (Deserialize still present but unused) — a future endpoint accepting an `ApiKey` body would be asymmetric. Inert today; flagged so it isn't a surprise. Tests: hash determinism/format/detection; create stores hash not raw + raw still validates; rehash one-shot + idempotent + heals a simulated legacy plaintext row; rotation stores a hash; `decide_master_key` hash-comparison branches; session-login e2e unchanged. Full suite green (196), clippy + fmt clean. Unblocks #24 (passkeys), whose ticket noted it sensibly lands after this.
Sign in to join this conversation.
No labels
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
IsoHex/edge-router#18
No description provided.