upload.py compares node API keys with != instead of secrets.compare_digest #12

Closed
opened 2026-08-19 22:06:55 -04:00 by logan · 1 comment
Owner

routers/upload.py compares the node API key with a plain != (key_doc.get("api_key") != credentials.credentials), which is not constant-time.

A minor timing side-channel on /upload. Worth fixing mainly because it is the exact class of issue the newer MQTT auth code (enrollment.py, dynsec.py) was explicit about avoiding with secrets.compare_digest — so the codebase currently contradicts itself. One-line fix.

`routers/upload.py` compares the node API key with a plain `!=` (`key_doc.get("api_key") != credentials.credentials`), which is not constant-time. A minor timing side-channel on `/upload`. Worth fixing mainly because it is the exact class of issue the newer MQTT auth code (`enrollment.py`, `dynsec.py`) was explicit about avoiding with `secrets.compare_digest` — so the codebase currently contradicts itself. One-line fix.
logan closed this issue 2026-08-20 03:08:22 -04:00
Author
Owner

Fixed in 5355095. /upload now uses secrets.compare_digest, matching what enrollment.py and dynsec.py already did for the same class of credential.

One thing the fix added beyond the one-liner: a missing api_key field is coalesced to "" before the comparison. compare_digest(None, ...) raises TypeError, which would have turned a malformed node_keys document into a 500 instead of a 401.

Fixed in `5355095`. `/upload` now uses `secrets.compare_digest`, matching what `enrollment.py` and `dynsec.py` already did for the same class of credential. One thing the fix added beyond the one-liner: a missing `api_key` field is coalesced to `""` before the comparison. `compare_digest(None, ...)` raises `TypeError`, which would have turned a malformed `node_keys` document into a 500 instead of a 401.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: logan/server-26#12