Repository navigation
fix: throttle before auth, plugin request fields, bounded metrics label, host parsing - #271
Merged
Merged
Conversation
…trics server label rate limits ran after global and per-route basic auth, so requests with wrong credentials never counted and password guessing was unlimited. The limiters now run right after the IP allowlist. plugins got the client's raw path spelling, an empty Host for ordinary origin-form requests and the X-Real-IP header as the client sent it. A plugin rejecting /fail let /%66ail through to the upstream as /fail, and could be fed any client address. Plugins now get the canonical path, the request host, and X-Real-IP resolved through --trusted-proxy. with --mgmt.low-cardinality the server label still took the raw Host, so each new Host value added a series. It now uses the matched route's server, or [unmatched]. the host port was cut with a split on ":", which turned [::1]:8080 into "[" in route matching and metrics. It is now parsed with url.URL.
url.URL.Hostname strips a port only when it is all digits, so a Host like ADMIN.example.com:x or a.example.com:x kept the suffix and missed its exact and wildcard servers. The request fell through to a catch-all route and skipped the narrower rule's auth and allowlist. Route matching, the plugin Host field and the metrics server label now share discovery.HostName, which cuts at the first colon as before and only unwraps bracketed IPv6. the README plugin intro no longer calls the plugin URL the original one.
route matching preferred r.URL.Hostname for absolute-form targets. For a non-http scheme Go splits that host at the last colon, so GET foo://a.wild.example:1:2/x gave "a.wild.example:1", missed its wildcard server and reached the catch-all without that rule's auth. Go's server already copies the target's host into r.Host, so route matching, the health-check forwarding lookup, the plugin Host and the metrics server label now all use discovery.HostName(r.Host). The health-check lookup also stops turning [::1] into "[".
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.
the rest of the sweep after #270: request handling where reproxy layers read the same request differently, each reproduced on a live build before the fix.
--throttle.user=2, six rapid bad-password requests got six 401s; now the third one gets 429Hostfor ordinary requests andX-Real-IPas the client sent it. A plugin rejecting/faillet/%66ailthrough, and the request reached the upstream despite the plugin restriction. Plugins now get the canonical path, the request host, andX-Real-IPresolved through--trusted-proxy.RemoteAddrstays the TCP peer--mgmt.low-cardinalitytheserverlabel ofhttp_requests_totalstill took the raw Host, so each new Host value added a series. It is now the matched route's server, or[unmatched]:, so[::1]:8080became[and missed a::1rule. All server name lookups now go throughdiscovery.HostName(r.Host), which unwraps bracketed IPv6 and otherwise cuts at the first colon. That also closes an absolute-form bypass on master:GET foo://a.wild.example:1:2/xkepta.wild.example:1, missed its wildcard server and reached the catch-all route without its authREADME notes cover the plugin request fields, limits counting failed logins, and the low-cardinality server label.