/logs and /metrics: a bearer token with a non-ASCII character answers 500 instead of 401 #372
Labels
No labels
accessibility
authentication
breaking change
bug
documentation
enhancement
interface
internationalisation
observability
performance
security
tier
1
tier
2
tier
3
tier/4
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
Postulo/postulo#372
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found on 2026-09-29 in a read-only review of postulo at
95348908d, reading the two token checks and sending them malformedAuthorizationheaders.Checked again on 2026-10-01 against
mainat0a5cad587: still present.Both endpoints compare the presented token with
hmac.compare_digest(presented.strip(), expected), on twostrvalues. Django decodes request headers as Latin-1, so any header byte above 0x7F becomes a non-ASCII character, andcompare_digestrefuses to compare non-ASCII strings: it raisesTypeError. The request fails with a 500 and a logged traceback, where it should get the same 401 as any other wrong token.core/views_logs.py:49-63(_authorised, called fromcollectat:82) andcore/views_metrics.py:25-34(_authorised, called fromscrapeat:41).0xE9raisesTypeError: comparing strings with non-ASCII characters is not supportedinside_authorised. Checked against the snapshot: 500, with the traceback in the log./logsor/metrics. Each such request writes an ERROR record with a traceback, and because the check comes beforethrottle.endpoint, nothing limits how many./logsitself serves) with noise.hmac.compare_digest(presented.strip().encode(), expected.encode()), which accepts any input and still runs in constant time. One helper for both views would stop the two copies drifting apart again.Worth deciding alongside, not part of the defect: wrong tokens are never counted.
tests/security/test_rate_limits.py:385-395(test_a_wrong_token_is_still_refused_before_anything_is_counted) makes that a deliberate choice, on the grounds that there is no work to protect behind a bad token. Operator tokens are usually long, but nothing checks their strength, so a separate bucket for failed attempts per address (answering 429 once spent, and leaving the good-token budget alone) would bound guessing at/logs, which serves the whole application log.Done when
tests/security/test_logs_endpoint.pyandtests/security/test_metrics.pyeach sendAuthorization: Bearerfollowed by a Latin-1 byte above 0x7F and assert a 401. Both fail today with a 500 and pass after the fix.