Private
Public Access
1
0
Files
linux_patch_manager/tasks/secret-encryption-spec.md
Draco-Lunaris-Echo b9fb3427e0
All checks were successful
CI Pipeline / Rust Format Check (push) Successful in 8s
CI Pipeline / Clippy Lints (push) Successful in 50s
CI Pipeline / Rust Unit Tests (push) Successful in 1m8s
CI Pipeline / Security Audit (push) Successful in 5s
CI Pipeline / Frontend Lint & Type Check (push) Successful in 15s
CI Pipeline / Build .deb & Release (push) Has been skipped
fix(security): encrypt app secrets at rest with AES-256-GCM (#6)
Encrypt three sensitive secrets that were stored in plaintext: OIDC client_secret, SMTP smtp_password, TOTP totp_secret. AES-256-GCM via pm-core::crypto helper. New per-install key at /etc/patch-manager/keys/secret-encryption.key, separate from health-check.key for blast-radius isolation. MASKED placeholder behavior in API responses is preserved.

23 files changed, +1248 / -28. Closes #6.
2026-06-03 15:08:25 -05:00

16 KiB
Raw Permalink Blame History

Secret Encryption at Rest — Issue #6 Spec

Spec version: v0.1.0 Issue: #6 — Plaintext storage of secrets in database Severity: Medium Author: Draco-Lunaris-Echo Status: Awaiting sign-off


1. Goal

Encrypt three sensitive secrets that are currently stored in plaintext in the database, using the existing AES-256-GCM crypto helper (crates/pm-core/src/crypto.rs) with a new dedicated encryption key.

Secrets to encrypt:

Secret Table Current column Current type
OIDC client_secret oidc_config client_secret TEXT NOT NULL DEFAULT ''
SMTP smtp_password system_config (key-value) value WHERE key = 'smtp_password' TEXT
TOTP totp_secret users totp_secret TEXT (nullable)

Why: Database exfiltration (via SQL injection, backup theft, insider threat) would expose the client_secret to the IdP, SMTP credentials, and persistent TOTP code generation capability for all MFA-enabled users.


2. Non-Goals

  • NOT adding a new KMS / Vault integration. AES-256-GCM with a file-based key is sufficient for our threat model and matches the existing health check credential pattern.
  • NOT rotating the encryption key. This PR establishes the encryption infrastructure; key rotation is a follow-up issue.
  • NOT encrypting health check credentials (already done in a previous PR).
  • NOT adding a new master key derivation step. The key file is the only secret to protect at the OS level.
  • NOT changing the MASKED placeholder behavior in API responses. That defense-in-depth pattern continues to apply on top of DB encryption.

3. Design Decisions (Kelly-approved Q1Q4)

Q Decision Rationale
Q1 — Key management A. New dedicated key at /etc/patch-manager/keys/secret-encryption.key Blast-radius isolation: if health-check key is compromised (least critical), secrets remain protected. Single-responsibility principle.
Q2 — totp_secret scope A. Encrypt it DB exfiltration = persistent TOTP code generation for all MFA-enabled users. Risk is real.
Q3 — Migration path Hard cutover (development stage) No dual-read window. The deploy MUST run a one-shot migration that encrypts existing plaintext values before dropping old columns.
Q4 — Key derivation A. Reuse load_or_create_key() Random 32-byte file, auto-generates on first start, 0600 perms. Same pattern as the health-check key, proven reliable.

4. Design

4.1 Crypto helper extension (crates/pm-core/src/crypto.rs)

Add a new constant alongside the existing KEY_PATH:

/// Path to the encryption key for sensitive app secrets
/// (OIDC client_secret, SMTP password, TOTP secret).
/// Separate from `KEY_PATH` (health-check credentials) for blast-radius isolation.
pub const SECRET_ENCRYPTION_KEY_PATH: &str =
    "/etc/patch-manager/keys/secret-encryption.key";

Re-export from crates/pm-core/src/lib.rs:

pub use crypto::{decrypt, encrypt, load_or_create_key, CryptoError, KEY_PATH, SECRET_ENCRYPTION_KEY_PATH};

4.2 Migration: migrations/020_encrypt_secrets_at_rest.sql

Schema changes (3 tables):

-- 1. oidc_config: replace client_secret TEXT with BYTEA columns
ALTER TABLE oidc_config
    ADD COLUMN IF NOT EXISTS client_secret_encrypted BYTEA,
    ADD COLUMN IF NOT EXISTS client_secret_nonce BYTEA;

-- One-shot encryption: read old plaintext, encrypt, write to new columns.
-- Requires the application to be running to provide the key (see §4.6).

ALTER TABLE oidc_config
    DROP COLUMN client_secret;

-- 2. system_config: replace smtp_password row with new key + encrypted+nonce columns
-- Approach: add new keys 'smtp_password_encrypted' and 'smtp_password_nonce';
-- remove the old 'smtp_password' row after migration script encrypts it.
-- (We don't change the system_config schema — we add new keys.)

-- 3. users: replace totp_secret TEXT with BYTEA columns
ALTER TABLE users
    ADD COLUMN IF NOT EXISTS totp_secret_encrypted BYTEA,
    ADD COLUMN IF NOT EXISTS totp_secret_nonce BYTEA;

ALTER TABLE users
    DROP COLUMN totp_secret;

Hard cutover requirement: The deploy must execute a one-shot Rust helper (see §4.6) BEFORE the DROP COLUMN statements run. The migration order is:

  1. ADD new BYTEA columns (idempotent, no data loss)
  2. Run one-shot encrypt helper (reads old plaintext, writes to new columns)
  3. DROP old TEXT columns

In development, we'll combine steps 1+2+3 into a single migration script that the operator runs manually before restarting the service.

4.3 Code changes (6 read/write sites)

A. crates/pm-web/src/routes/sso.rs — OIDC client_secret READ

Location: load_oidc_config function, line 802 Before:

sqlx::query_as(
    "SELECT enabled, provider_type, display_name, discovery_url, client_id, client_secret, redirect_uri, scopes FROM oidc_config WHERE id = 1",
)

After:

sqlx::query_as(
    "SELECT enabled, provider_type, display_name, discovery_url, client_id, \
     client_secret_encrypted, client_secret_nonce, redirect_uri, scopes \
     FROM oidc_config WHERE id = 1",
)
// ... then decrypt the secret in the OidcConfig struct construction

OidcConfig struct (line 216) change:

  • pub client_secret: Stringpub client_secret_encrypted: Vec<u8> + pub client_secret_nonce: Vec<u8>
  • Add a pub fn decrypt_client_secret(&self, key: &[u8; 32]) -> Result<String, CryptoError> method

B. crates/pm-web/src/routes/settings.rs — OIDC client_secret READ+WRITE+MASK

Read (line 280): Same query change as A above, then decrypt. Write (line 360400): Replace plaintext bind with encrypted+nonce binds. MASK (line 295315): No change — the API still returns MASKED if the secret is set.

C. crates/pm-web/src/routes/settings.rs — SMTP password READ+WRITE

Read (line 793, smtp_password key in system_config):

  • Before: cfg.get("smtp_password").cloned().unwrap_or_default()
  • After: read smtp_password_encrypted + smtp_password_nonce keys, decrypt with the same key

Write (line 453):

  • Before: update_config_key(&state.db, "smtp_password", v).await?;
  • After: let (enc, nonce) = crypto::encrypt(v, &key)?; then write to smtp_password_encrypted and smtp_password_nonce keys

D. crates/pm-auth/src/session.rs — TOTP secret READ

Location: line 197, let secret = user.totp_secret.as_deref().unwrap_or("");

Before:

let secret = user.totp_secret.as_deref().unwrap_or("");

After:

let secret = user.totp_secret_encrypted.as_ref()
    .zip(user.totp_secret_nonce.as_ref())
    .map(|(enc, nonce)| crypto::decrypt(enc, nonce, &key))
    .transpose()?
    .unwrap_or_default();

User struct (line 80) change:

  • totp_secret: Option<String>totp_secret_encrypted: Option<Vec<u8>> + totp_secret_nonce: Option<Vec<u8>>

E. crates/pm-web/src/routes/auth.rs — TOTP secret WRITE (MFA enrollment)

Location: line 363, sqlx::query("UPDATE users SET totp_secret = $1, mfa_enabled = TRUE WHERE id = $2")

Before:

.bind(&req.secret_base32)

After:

let (enc, nonce) = crypto::encrypt(&req.secret_base32, &key)?;
// ... bind enc and nonce, drop the plaintext

F. crates/pm-web/src/routes/users.rs — TOTP secret NULL write (disable MFA)

Location: line 537, sqlx::query("UPDATE users SET totp_secret = NULL, ... WHERE id = $1")

Before: Sets totp_secret = NULL. After: Sets totp_secret_encrypted = NULL, totp_secret_nonce = NULL.

G. crates/pm-worker/src/email.rs — SMTP password READ in worker

Location: line 58, password: get("smtp_password")

Before: Reads plaintext key from system_config. After: Reads smtp_password_encrypted + smtp_password_nonce, decrypts.

4.4 Key loading in pm-web (one-time setup)

The secret-encryption key must be loaded at startup and accessible to all routes that decrypt secrets. Pattern: load at request time, cache per process.

Implementation: Add a helper module crates/pm-web/src/secret_key.rs:

use once_cell::sync::OnceCell;
use pm_core::crypto;
use std::path::Path;

static SECRET_KEY: OnceCell<[u8; 32]> = OnceCell::new();

/// Load the secret-encryption key at first call. Subsequent calls return the cached value.
/// Returns CryptoError if the key file is missing or invalid.
pub fn get() -> Result<&'static [u8; 32], crypto::CryptoError> {
    SECRET_KEY.get_or_try_init(|| {
        crypto::load_or_create_key(Path::new(crypto::SECRET_ENCRYPTION_KEY_PATH))
    })
}

Note: once_cell is already a workspace dependency. Each route that needs to decrypt calls secret_key::get()? and uses the key.

For the worker crate (pm-worker), the same pattern is needed in crates/pm-worker/src/secret_key.rs.

4.5 Migration helper: migrations/020_migrate_secrets.rs (one-shot, dev only)

A standalone Rust binary (or an #[ignore] integration test) that:

  1. Connects to the database using the existing DATABASE_URL
  2. Reads the plaintext secrets from the old columns/rows
  3. Encrypts each one with the secret-encryption key
  4. Writes to the new BYTEA columns
  5. Verifies the encrypted values match the plaintext (round-trip check)
  6. Reports success and recommends running migration 020 to drop the old columns

For development: This helper is run manually before deploying the new code. The migration file 020_encrypt_secrets_at_rest.sql drops the old columns after the helper completes.

4.6 Key generation on first start

On first start of the new code:

  1. If /etc/patch-manager/keys/secret-encryption.key doesn't exist, the load_or_create_key() function generates a new 32-byte key and writes it with 0600 permissions.
  2. The new code looks for encrypted columns. If they're NULL and the old plaintext columns are gone, the application will fail with a clear error message ("Secret not initialized — run the migration helper").
  3. The migration helper from §4.5 must be run BEFORE the new code's first start, OR the deployment must be ordered: run helper → deploy new code.

5. Acceptance Criteria

  • migrations/020_encrypt_secrets_at_rest.sql adds BYTEA columns for all 3 secrets, then drops the old TEXT columns.
  • crypto::SECRET_ENCRYPTION_KEY_PATH constant added; re-exported from pm-core/lib.rs.
  • pm-web and pm-worker have a secret_key::get() helper using OnceCell.
  • All 6 read sites (sso.rs:802, settings.rs:280, settings.rs:793, session.rs:197, pm-worker/email.rs:58, plus the write site at auth.rs:363) use crypto::encrypt/decrypt with the secret-encryption key.
  • All 3 write sites (settings.rs:375 for OIDC, settings.rs:453 for SMTP, auth.rs:363 for TOTP, users.rs:537 for TOTP disable) bind encrypted+nonce instead of plaintext.
  • The MASKED placeholder behavior in API responses is preserved.
  • A one-shot migration helper (020_migrate_secrets.rs or equivalent) is provided and documented.
  • cargo fmt --check --all clean.
  • cargo clippy --all-targets -- -D warnings clean.
  • cargo test -p pm-web --bins --tests passes (43 existing + 2 new = 45 tests).
  • cargo test -p pm-worker --bins --tests passes (existing + 1 new = at least 1 test).
  • No new entries in the audit log (encryption is a data migration, not a user action).
  • The new key file /etc/patch-manager/keys/secret-encryption.key is documented in the install/runbook.

6. Test Plan

Unit tests (3 new):

  • crypto::encrypt_decrypt_round_trip — encrypt a known plaintext, decrypt it, assert equality
  • secret_key::get_returns_same_key — call get() twice, assert pointer equality (caching works)
  • secret_key::get_creates_key_on_first_call — delete the key file, call get(), assert the key file is recreated

Migration helper test (1 new):

  • 020_migrate_secrets::test_round_trip_oidc — seed DB with known plaintext, run helper, assert encrypted column matches the expected ciphertext (computed independently)

Existing tests to verify still pass:

  • cargo test -p pm-web --bins --tests — 43 existing tests
  • cargo test -p pm-auth --bins --tests — session tests for TOTP verification
  • cargo test -p pm-worker --bins --tests — email tests (if any)

Manual verification:

  • Start the service, log in as admin, navigate to Settings → OIDC, verify the API response shows MASKED (no plaintext leak)
  • psql -c "SELECT client_secret_encrypted FROM oidc_config" — verify the value is binary (BYTEA), not readable text
  • psql -c "SELECT value FROM system_config WHERE key = 'smtp_password'" — verify the row is gone (replaced by encrypted+nonce rows)
  • psql -c "SELECT totp_secret FROM users WHERE mfa_enabled = TRUE LIMIT 1" — verify the column is gone

7. Risk Analysis

Risk Likelihood Impact Mitigation
Deploy order: new code starts before migration helper runs → service fails to read secrets Medium High Document the deploy order in the runbook. Add a startup check that detects missing encrypted columns and returns a clear error.
Key file lost (deleted, disk failure) → all secrets unreadable Low Critical Document the key file in the backup runbook. Add a backup.sh hook to include the key file in backups. Follow-up issue for key recovery / rotation.
Worker doesn't share key with web Low Medium Both use the same load_or_create_key() with the same path. Key file is filesystem-shared.
TOTP secret encryption breaks existing MFA sessions Low Medium The one-shot migration helper decrypts old plaintext, re-encrypts, and writes. Existing TOTP seeds remain valid.
Migration helper crashes mid-migration → partial state Low Medium The helper is idempotent (uses UPSERT). On retry, it re-encrypts and overwrites.
Key file permissions wrong → OS-level exposure Very low Medium load_or_create_key() sets 0600 on creation. chmod enforcement in the install script.
Audit log entries leak the secret value Very low N/A We don't log the plaintext or ciphertext. Only the fact that the column was updated.

8. Documentation Updates

8.1 docs/security-review.md §4.1 (Encryption at Rest)

Add a new evidence row:

Control Status Evidence
Secrets encrypted at rest (issue #6 fix) Verified OIDC client_secret, SMTP smtp_password, and TOTP totp_secret are encrypted with AES-256-GCM using a dedicated per-install key at /etc/patch-manager/keys/secret-encryption.key. Encryption/decryption via pm-core::crypto::encrypt/decrypt (same helper as health check credentials, but with a separate key for blast-radius isolation). Schema migration 020_encrypt_secrets_at_rest.sql replaces plaintext TEXT columns with BYTEA _encrypted + _nonce columns.

8.2 docs/runbooks/restore.md (or new docs/runbooks/key-management.md)

Add a section on the new key file:

## Encryption Keys

Two per-install AES-256-GCM keys are auto-generated on first start:

| Key | Path | Protects |
|-----|------|----------|
| `health-check.key` | `/etc/patch-manager/keys/health-check.key` | HTTP basic auth passwords for health check endpoints |
| `secret-encryption.key` | `/etc/patch-manager/keys/secret-encryption.key` | OIDC client_secret, SMTP password, TOTP secrets |

**Backup:** Both key files MUST be included in `/etc/patch-manager` backups. Without them, the encrypted data is unrecoverable.

**Rotation:** Key rotation is not yet supported (follow-up issue). If a key is compromised, generate a new key and re-encrypt all secrets.

8.3 docs/REST_API.md (no changes needed)

The API surface is unchanged — the MASKED placeholder behavior is preserved.


9. Follow-ups

  • Key rotation — add support for rotating the secret-encryption key without service downtime. Requires wrapping the key in a versioned envelope (e.g., {key_id, ciphertext, nonce}).
  • Integration tests — covered by issue #15. The migration helper has its own unit test.
  • Audit logging — log the fact that secret-encryption key was loaded at startup (NOT the key itself).
  • Backup verification — automated test that verifies a fresh install can restore from a backup by decrypting the secrets.

10. Sign-off

Approve to proceed to Phase 1 (crypto helper extension + one-shot migration helper + 3 new unit tests). Per project rules, I will not commit or push anything until Phase 7.