Skip to content

Upload token hashes are keyed on SECRET_KEY, so rotating SECRET_KEY silently invalidates every upload token #22

@erskingardner

Description

@erskingardner

Impact

Upload bearer tokens are stored as HMAC-SHA256(SECRET_KEY, secret):

@classmethod
def hash_secret(cls, secret: str) -> str:
    key = settings.SECRET_KEY.encode("utf-8")
    return hmac.new(key, secret.encode("utf-8"), hashlib.sha256).hexdigest()

Because the HMAC key is Django's SECRET_KEY, the stored token hashes are only meaningful for as long as SECRET_KEY stays constant. Rotating SECRET_KEY — a standard, recommended security action (e.g. on suspected key compromise, or routine rotation) — recomputes a different hash for every presented token, so hmac.compare_digest() in authenticate() will never match again. Every previously issued upload token stops working at once, with no migration path (the raw secrets were shown once and never stored, so they cannot be re-hashed). The README's documented rotation story only covers Django's SECRET_KEY indirectly and never warns that it nukes all upload tokens.

This couples two unrelated rotation lifecycles: rotating the web app's signing key (sessions, CSRF, password reset, etc.) also silently breaks the entire client-upload fleet, which during an incident is exactly when you least want a surprise outage of evidence ingestion.

(Note: token secrets are 256 bits of secrets.token_urlsafe(32), so the use of a fast HMAC for the hash itself is fine — the issue is the key choice, not the hash speed.)

Code pointers

  • forensics/models.py:57-60hash_secret() keys the HMAC on settings.SECRET_KEY.
  • forensics/models.py:74authenticate() compares against hash_secret(secret); any SECRET_KEY change makes this fail for all tokens.

Expected behavior

Rotating Django's SECRET_KEY should not invalidate upload tokens (or, at minimum, the coupling should be deliberate and clearly documented).

Suggested fix

  • Key the token HMAC on a dedicated, independently rotatable secret (e.g. GOGGLES_TOKEN_HASH_KEY) rather than SECRET_KEY, falling back to SECRET_KEY only for backwards compatibility during a migration window.
  • Alternatively, store tokens with a plain salted hash (the secret already has full entropy) so validity does not depend on any rotating signing key.
  • Document the chosen coupling explicitly in the rotation runbook either way.

Metadata

Metadata

Assignees

No one assigned

    Labels

    LOWSeverity: minor correctness, polish, or maintainability issuebugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions