Appearance
ADR 0005 — Protection at rest of the Auth module's reversible secrets
- Status: Accepted
- Date: 2026-07-24
- Context: GrydAuth stores two secrets it must be able to read back — the user's TOTP seed and the enterprise IdP client secret. Both were protected by one class named as if it belonged to MFA, under an AEAD whose associated data bound the ciphertext to nothing but the format version and the key id. Companion documentation:
docs/modules/auth/secret-protection.md.
Context
Two columns hold reversible secrets:
| Secret | Column | Needs the plaintext at runtime |
|---|---|---|
| TOTP seed | MfaFactors.EncryptedSecret | to compute/verify the code |
| IdP client secret | IdentityProviderConnections.ClientSecretEncrypted | to complete the token exchange |
Both went through SecretProtector (AES-256-GCM), configured under GrydAuth:Mfa:SecretProtection. The cipher choice was correct. Everything around it was not:
- The name lied. A federation write failed with "MFA secret protection active key is not configured", sending operators to investigate the wrong subsystem.
- No domain separation. The associated data covered only
version|keyId, so a TOTP seed planted in the client-secret column decrypted cleanly, and so did tenant A's client secret in tenant B's connection row. - The interface could not be fixed.
Protect(string)had nowhere to receive context. - One key for everything — every tenant, both domains.
- Half a rotation. Retired keys still decrypted, but nothing re-encrypted the backlog, so removing a key failed at a customer's next federated login.
- BCL exceptions crossed the layer boundary and fell through to a generic
InvalidOperationException → 409 Conflictmapping. - Validation was relaxed in Development, so a missing key surfaced on the first write instead of at boot. This is how the defect reached production configuration in the first place.
- No KMS seam — the master key lived in the config store and in process memory.
Decision
1. Keep AES-256-GCM; do not reuse the password hasher
Argon2id is one-way by design; the TOTP seed and the client secret must come back. These are different cryptographic categories, not a style preference. Hashing them would end TOTP and enterprise SSO.
2. Bind purpose and row identity into the associated data
AAD = magic | scheme | keyRef | purpose | context, where context is derived from the identities that own the secret (user+factor, tenant+connection).
Costs nothing at rest — the AAD is never stored, it is rebuilt on read from context the caller already holds. If the reconstruction differs, the tag fails. A ciphertext therefore only opens in the exact place it was written.
Rejected: a separate column naming the purpose. It would not be authenticated, so an attacker who can rewrite the ciphertext can rewrite that column with it.
3. Derive a subkey per purpose with HKDF-SHA256
The configured key is a master key; the encryption key is HKDF(masterKey, salt = keyId, info = "gryd.secret.v2|" + purpose). The operator still manages one key per keyId, but leaking the MFA subkey exposes no client secret.
Rejected: one configuration section per domain — double the operational surface, double the chance of a rotation mistake.
4. Keep a purpose-built protector; do not adopt IDataProtection
ASP.NET Core Data Protection targets transient payloads (cookies, short-lived tokens) with a 90-day key ring and revocation semantics; Microsoft's own guidance advises against indefinite persistence. An Entra client secret lives for years. The key ring would still need its own persistence and encryption (Blob/DB + KMS), so the key-management problem does not disappear — it moves — and explicit control of keyId, format and AAD is lost. The repository uses IDataProtection nowhere today.
5. A versioned envelope with a scheme discriminator (Strategy)
g2.<scheme>.<keyRef>.<nonce>.<tag>.<ciphertext>, scheme ∈ {kr, kms}. Reading follows the scheme named in the ciphertext; writing follows configuration. That asymmetry is what makes a scheme migration additive and downtime-free instead of another destructive refactor.
6. An asynchronous contract from the start
ValueTask<string> ProtectAsync/UnprotectAsync. A KMS-backed scheme calls the network, and decryption sits on the federated-login hot path — a synchronous contract would force sync-over-async there. Every call site was already inside an async method, so the cost now is zero and one refactor later is avoided.
7. Purpose as data, not as type
One ISecretProtector taking a SecretProtectionScope, not IMfaSecretProtector + IClientSecretProtector. Two interfaces with identical signatures and one implementation would be ceremony without isolation; the isolation that matters is cryptographic (subkey + AAD), not a C# type. ISP is applied where the clients genuinely differ: envelope inspection (IProtectedSecretInspector) is separate, so the bulk re-wrap can ask "is this stale?" over the whole database without holding the ability to decrypt it.
8. Encrypt after constructing the aggregate
The context includes the row's id, which exists only once the aggregate does. Factories therefore no longer take ciphertext; SetSecret / SetClientSecret do, and RewrapSecret / RewrapClientSecret exist alongside them so rotation never has to reach around the aggregate. Re-wrap raises no domain event — the configuration did not change, and evicting every login-path cache entry during a routine rotation would be a self-inflicted stampede.
9. Fail fast at boot, in every environment
The Development exemption is gone. Mandatory configuration that fails at runtime is a trap; the startup failure carries the command that generates a key.
10. No format backward compatibility
Only g2 is readable. The ResetSecretProtectionForV2 migration clears the old values, disabling MFA factors (owners re-enrol) and returning connections to Draft (admins re-enter the secret). Leaving them in place would be worse: an Enabled factor with an unreadable seed locks its owner out with a 500 at the challenge, and an Active connection fails at the token exchange.
Consequences
Gained
- A ciphertext is worthless outside its own row, purpose and tenant; the AEAD now fails loudly on a bad write, a partial restore or an insider copy, where it used to succeed silently.
- Failures map to 503/500 with codes an operator can act on, never to a 409 naming the wrong subsystem.
- Rotation is completable and provable: status, idempotent re-wrap, then key removal.
- The re-wrap doubles as a tamper sweep — it cannot re-bless a mis-bound ciphertext, only report it.
- KMS custody is an added strategy, not a rewrite.
Paid
- A one-way data migration: mass MFA re-enrolment and re-entry of every client secret, which has to be scheduled with whoever operates the environment.
- Every call site passes a scope, so a new kind of protected secret must define what it is bound to — deliberate friction, since that decision is the security property.
- The purpose tokens and context formats are now wire values, frozen like any other stored format.