993674 - relengweb: Configure Zeus to send X-Forwarded-For, and use it in Apache configs

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: WebOps: Other, task)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

The cluster currently uses the Zeus-default X-Cluster-Client-Ip, but everyone else seems to have standardized on the non-default X-Forwarded-For.  So we should use those too.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

Webops: this isn't a huge priority, but may help in keeping the releng cluster in line with other clusters.

Comment 2

[C. Liang [:cyliang]]

I've added the rule that adds the X-Forwarded-For header to the releng-stage-http and releng-stage-https virtual servers on both the internal and external VIPs.  Does it make sense to see how it works in staging for a week before applying to production?  Or is this one of the situations where it is either safe or desirable to have this applied to production now?

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

The headers are only used by Apache for access control, so I think the next step is to change those ACLs in puppet and see that staging is still access-controlling properly.  As a test case, http://buildapi.pvt.build.mozilla.org/ should only be available from the listed IPs (it might be easiest for you to verify from nagios1) and not from other 10.x addresses.

If that's OK, then it's fine to move on to production.

Comment 4

[C. Liang [:cyliang]]

Apologies for my brain-deadness, but I'm not what is the safest way to alter the allow-build-net-zlb.conf.erb template for just staging.  (If there is a clear path for doing so, I'm not seeing it.)

Does it make sense to, in both of the loops in the template, to put in an if statement to use X-Cluster-Client-IP versus X-Forwarded-For, based on the environment variable?  That is, something like:

<% environment = scope.lookupvar("environment") -%>
<% vpn_internal_regexps.each do |subnet| -%>
  <%- if environment == "stage" -%>
    RewriteCond %{HTTP:X-Forwarded-For} !^<%= subnet %>$
  <%- else -%>
    RewriteCond %{HTTP:X-Cluster-Client-Ip} !^<%= subnet %>$
  <%- end -%>
<% end -%>

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Oh, good point.  No, I think that's more likely to introduce breakage than to avoid it.  We might be overthinking this!

I think something as simple as a tcpdump to verify that the request headers seen on each port (80..82, since 83 appears not to be in use) contain identical values in the X-{Forwarded-For,Cluster-Client-Ip} headers should be enough to be confident that the change will have no impact.

Comment 6

[C. Liang [:cyliang]]

Attachment: forward_dump_80.pcap

Comment 7

[C. Liang [:cyliang]]

Attachment: forward_dump_81.pcap

Comment 8

[C. Liang [:cyliang]]

Attachment: forward_dump_82.pcap

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

lgtm!

Comment 10

[C. Liang [:cyliang]]

Changes applied to the releng and releng-https virutal servers on both the internal and external VIPs.  

Please reopen this bug if I've missed something.

Comment 11

[Jake Maul [:jakem]]

This is resolved now, but for future reference, Zeus always adds a header "X-Cluster-Client-IP" whose value is the external IP that made the request to Zeus.

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

That's what we were using, but after atoll pointed out that their foreman configuration expects X-Forwarded-For, I noticed that *all* other webapps use X-Forwarded-For, so I filed this bug to make releng look more like everything else.