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

Reply via email to