BBlack has submitted this change and it was merged. Change subject: VCL: do not use illegal "trusted" XFF values for XCIP ......................................................................
VCL: do not use illegal "trusted" XFF values for XCIP As noted in the linked bug, our XFF-processing needs to get a lot more advanced in the long run. For now this closes the functional issue where we set X-Client-IP to a private or loopback IP when that's the first XFF entry set by a "trusted" proxy such as OperaMini. In these cases we'll fall back to acting as if there was no trusted proxy and set XCIP to the OperaMini proxy IP for now (because that's better than logging 127.0.0.1 or whatever as XCIP). Bug: T120121 Change-Id: Ib7d46cc1e527265c3f8169d483929acf44278fef --- M modules/varnish/templates/vcl/wikimedia.vcl.erb 1 file changed, 12 insertions(+), 3 deletions(-) Approvals: BBlack: Verified; Looks good to me, approved diff --git a/modules/varnish/templates/vcl/wikimedia.vcl.erb b/modules/varnish/templates/vcl/wikimedia.vcl.erb index c802dc7..9e25e10 100644 --- a/modules/varnish/templates/vcl/wikimedia.vcl.erb +++ b/modules/varnish/templates/vcl/wikimedia.vcl.erb @@ -10,6 +10,7 @@ <% if @vcl_config.fetch("layer", "") == "frontend" -%> # only used in recv_fe_ip_processing on frontends import netmapper; +import ipcast; <% end %> <% @@ -319,7 +320,11 @@ } if (req.http.X-Trusted-Proxy && req.http.X-Forwarded-For) { // get last from trusted-proxy-supplied XFF - set req.http.X-Client-IP = regsub(req.http.X-Forwarded-For, "^([^,]+, )+", ""); + set req.http.maybe-xcip = regsub(req.http.X-Forwarded-For, "^([^,]+, )+", ""); + if(ipcast.ip(req.http.maybe-xcip, "127.0.0.1") !~ wikimedia_nets) { + set req.http.X-Client-IP = req.http.maybe-xcip; + } + unset req.http.maybe-xcip; } } else { // XCIP from nginx, XFF set/appended by nginx and contains at @@ -336,8 +341,12 @@ // regsub's below if there was only one (which would // alias XCIP by definition), there would be no commas // to match and XCIP gets reset to its original value. - set req.http.X-Client-IP = regsub(req.http.X-Forwarded-For, ", [^,]+$", ""); - set req.http.X-Client-IP = regsub(req.http.X-Client-IP, "^([^,]+, )+", ""); + set req.http.maybe-xcip = regsub(req.http.X-Forwarded-For, ", [^,]+$", ""); + set req.http.maybe-xcip = regsub(req.http.maybe-xcip, "^([^,]+, )+", ""); + if(ipcast.ip(req.http.maybe-xcip, "127.0.0.1") !~ wikimedia_nets) { + set req.http.X-Client-IP = req.http.maybe-xcip; + } + unset req.http.maybe-xcip; } } -- To view, visit https://gerrit.wikimedia.org/r/266486 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib7d46cc1e527265c3f8169d483929acf44278fef Gerrit-PatchSet: 2 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: BBlack <bbl...@wikimedia.org> Gerrit-Reviewer: BBlack <bbl...@wikimedia.org> Gerrit-Reviewer: Ema <e...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits