Limit conn, limit req, stream limit conn: reject an overlong key - #1583
Draft
VadimZhestikov wants to merge 2 commits into
Draft
Limit conn, limit req, stream limit conn: reject an overlong key#1583VadimZhestikov wants to merge 2 commits into
VadimZhestikov wants to merge 2 commits into
Conversation
When the evaluated key exceeded the length the modules can store (255 bytes for limit_conn, 65535 for limit_req), the request was logged and the limit was silently skipped with "continue", so a request with an overlong key was not limited (fail-open). Such a request is now rejected with the configured status code, like any other limited request, so an oversized key can no longer be used to escape the limit. Keys within the length limit, including the usual $binary_remote_addr, are unaffected. Reported by Hcamael.
Contributor
|
how about similar fix in stream limit conn module? |
saikrishnakumarreddy
self-requested a review
July 22, 2026 02:43
When the evaluated key exceeded the length the module can store (255 bytes), the connection was logged and the limit was silently skipped with "continue", so a connection with an overlong key was not limited (fail-open). Such a connection is now rejected, like any other limited connection, so an oversized key can no longer be used to escape the limit. Keys within the length limit, including the usual $binary_remote_addr, are unaffected. Reported by Hcamael.
Contributor
Author
Good call -- stream limit_conn had the identical fail-open (if (key.len > 255) { ...; continue; }), so an Added a commit that fails it closed the same way (releases state, honours limit_conn_dry_run, rejects the |
VadimZhestikov
changed the title
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
4 participants
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.
What
When the evaluated key exceeded the storable length (255 bytes for
limit_conn, 65535 forlimit_req), the modules logged an error andcontinued, silently skipping the limit -- so a request/connection with an overlong key was not limited (fail-open). With a single zone in the location it passed unlimited.The overlong-key branch now fails closed: the request/connection is rejected like any other limited one (honouring
dry_run, and releasing already-locked/registered state first). Keys within the length limit -- including the usual$binary_remote_addr-- are unaffected (that branch is never taken for bounded keys).Covers three modules, in separate commits:
ngx_http_limit_conn_moduleandngx_http_limit_req_module-- reject with the configuredlimit_*_statuscode.ngx_stream_limit_conn_module-- reject the connection (NGX_STREAM_SERVICE_UNAVAILABLE). (There is no streamlimit_req-- limit_req is http-only.)Priority / type
Low-priority hardening (fail-open -> fail-closed).
Testing
Built with
-Werror.limit_conn 1, all concurrent connections were accepted before the change (fail-open) and are rejected after it; a bounded key is unaffected.Test PR: nginx/nginx-tests#87 (http
limit_conn.t/limit_req.tand streamstream_limit_conn.t).