fix(openai-frontend): use hmac.compare_digest for restriction header value - #8788
Open
ibondarenko1 wants to merge 1 commit into
Open
Conversation
…value APIRestrictionMiddleware._check_authentication compares the incoming header value against the operator-configured expected value with !=, which short-circuits at the first byte mismatch. A caller that can issue many requests against a restricted endpoint can in principle infer the configured header value byte-by-byte via response-time differences. Switch the compare to hmac.compare_digest, the standard Python idiom for constant-time string equality. Signed-off-by: Ievgen Bondarenko <sactransport2000@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
APIRestrictionMiddleware._check_authenticationatpython/openai/openai_frontend/frontend/fastapi/middleware/api_restriction.py:223validates the incoming header value with:The
!=on twostrvalues runs in time proportional to the matching-prefix length. A caller who can submit many requests against a restricted endpoint can, in principle, recover the configured header value byte-by-byte by observing response time. The operator-chosen header value is the only secret material guarding the endpoint, so a leak on this compare reduces the effective key strength of the restriction.Change
Switch the compare to
hmac.compare_digest, the Python standard library's constant-time string equality function. Oneimport hmacat the top of the file, one line at the compare site, and an inline comment explaining the reason so a future contributor does not regress to==/!=.The short-circuit on
not actual_valuekeepsNoneand empty-string headers out ofcompare_digest, which would raiseTypeErroronNone. The length ofactual_valueis still potentially leaked (compare_digestwarns about this), but length is not the secret part of an operator-configured header value; the byte content is.Impact
Backwards compatibility
RestrictedFeatures,APIRestrictionMiddleware.__init__, andAPIRestrictionMiddleware.dispatchsignatures unchanged.--openai-restricted-api) parses and behaves identically.python/openai/tests/test_openai_restricted_apis.pyexercises the pass / fail semantics and does not assert on timing; behavior under that suite is unchanged.Tests
python3 -c "import ast; ast.parse(open(path).read())"): pass.git apply --checkagainstmainHEAD1e69d88b0fccd77c5782c316f6f32a7c2175622f: clean.python/openai/tests/test_openai_restricted_apis.py(467 lines, 12+ test cases) not run locally: this suite spins up a Triton server with the OpenAI frontend and a loaded engine, which needs a GPU host and a builttritonserver. The change is one-line semantics-preserving; the suite should remain green on CI.I would happily extend the test file with one fixture that asserts on
hmac.compare_digestbeing the compare path (e.g., mocking the compare to count call sites) if the reviewer wants the regression covered explicitly.Why this is a hardening contribution
I was reading the OpenAI frontend's request pipeline and noticed the compare site. The same hardening pattern (constant-time compare on operator-configured header values) appears in many similar HTTP middleware projects; it is a standard idiom rather than a novel claim. The change converts a theoretical timing oracle into a no-oracle implementation at minimal review cost.
Related code paths (for reviewer context)
The C++ HTTP server (
src/http_server.ccHTTPAPIServer::RespondIfRestricted) and the C++ gRPC server (src/grpc/grpc_server.ccCommonCallData::ExecutePrecondition) implement the same restriction check in their respective protocols and use the same non-constant-time compare pattern. I am happy to follow up with parallel hardening PRs against those sinks once this Python change is reviewed; keeping the changes one-per-PR per CONTRIBUTING.md's "single concern" guidance.