Bug 61355 - DirectorySlash directive should use protocol in X-Forwarded-Proto header when available
Summary: DirectorySlash directive should use protocol in X-Forwarded-Proto header when...
Status: REOPENED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_dir (show other bugs)
Version: 2.4.25
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-28 00:39 UTC by keith.w.shaw
Modified: 2019-01-17 07:54 UTC (History)
1 user (show)



Attachments
Bug 61355 - make http_scheme use X-Forwarded-Proto header (929 bytes, patch)
2018-12-06 06:31 UTC, Axel Reinhold
Details | Diff
get the proxied protocol from header with mod_remoteip (5.33 KB, patch)
2018-12-19 11:08 UTC, Axel Reinhold
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description keith.w.shaw 2017-07-28 00:39:48 UTC
The DirectorySlash directive in mod_dir does not respect the X-Forwarded-Proto header. This results in Apache HTTPD generating redirects with the wrong protocol when the DirectorySlash directive is enabled and the Apache HTTPD service is running behind a reverse proxy.
Comment 1 Axel Reinhold 2018-12-06 06:31:40 UTC
Created attachment 36295 [details]
Bug 61355 - make http_scheme use X-Forwarded-Proto header

As DirectorySlash honors the scheme given in ServerName the scheme in http_scheme() from http_core.c should respect X-Forwarded-Proto header instead of fixing mod_dir.c for this bug. This has been implemented in the attached patch. Should also be mentioned in the doc. Add respect of X-Forwarded-Proto to http_scheme() instead of fixing mod_dir
Comment 2 William A. Rowe Jr. 2018-12-14 18:24:38 UTC
It should be noted that this introduces a monstrous security hole.

mod_remoteip uses explicit lists of trusted peers to pass valid X-F-F data for
interpretation. This hack is no different in trust requirements in order for the
project to consider this submission.
Comment 3 Axel Reinhold 2018-12-18 13:49:35 UTC
Ok - but then following config is the same threat:

SetEnvIf X-Forwarded-Proto https HTTPS=on
SetEnvIf X-Forwarded-Proto https REQUEST_SCHEME=https

And this is recommended everywhere to do!

Anyways i will try to create a patch for mod_remoteip
which uses the list of trusted peers.
Comment 4 Axel Reinhold 2018-12-19 11:08:22 UTC
Created attachment 36337 [details]
get the proxied protocol from header with mod_remoteip

This patch adds information about the proxied protocol into the server-request using a new header in mod_remoteip. The trust-requirements are the same as with the remote ip. The path includes also a patch for the documentation of mod_remoteip.
Comment 5 William A. Rowe Jr. 2019-01-16 17:38:29 UTC
> [The] following config is the same threat:
> 
> SetEnvIf X-Forwarded-Proto https HTTPS=on
> SetEnvIf X-Forwarded-Proto https REQUEST_SCHEME=https
> 
> And this is recommended everywhere to do!

Yes. That is a threat, unless the internally-trusted front end ahead of all
external routes to that server unilaterally clears and then forces the true
value of the X-F-P header. When you do see that recommended, you would be
doing a great service to comment on the potential hazard of those directives.

Thank you for your patch submission. Entirely returned from holiday schedules,
so I'll examine your patch shortly.
Comment 6 William A. Rowe Jr. 2019-01-16 18:06:58 UTC
In general I like the patch very much.

But there is a problem with the proposal;
+    <example><title>Proxy Example</title>
+    <highlight language="config">
+        RemoteIPProtoHeader X-Forwarded-Proto
+        </highlight>
+    </example>

Can you suggest any case where it would be legitimate to accept a different protocol
other than the true protocol used to deliver the request across the internet?

This seems like an entirely-intranet convention, should use only the trusted Internal
proxy list, and accept any protocol (not only HTTPS) presented by that internal
gateway agent. WDYT?
Comment 7 Axel Reinhold 2019-01-16 20:12:26 UTC
sorry - i do not understand your question. In apaches http_core.c the schemas apache uses are fixed to http and https - see this comment in http_scheme():

    /*
     * The http module shouldn't return anything other than
     * "http" (the default) or "https".
     */

so i reduced the patch also to these protocols.

I needed this behaviour not in an intranet environment but in a real-live website which is running behind haproxy which also does SSL-offloading. This page uses an iframe in which links to directories are generated - these were redirected to trailing-slash versions by mod_dir with the wrong protocol. So i needed this solution and did not like the ServerName solution, because this does not work when both http and https are available. In apache the config is only available once, because of the haproxy loadbalancer in front of apache and apache sees no difference in the requests other than the x-forwarded-proto header.
Comment 8 William A. Rowe Jr. 2019-01-16 23:22:49 UTC
(In reply to Axel Reinhold from comment #7)
> sorry - i do not understand your question. In apaches http_core.c the
> schemas apache uses are fixed to http and https - see this comment in
> http_scheme():
> 
>     /*
>      * The http module shouldn't return anything other than
>      * "http" (the default) or "https".
>      */
> 
> so i reduced the patch also to these protocols.

That makes sense!

> I needed this behaviour not in an intranet environment but in a real-live
> website which is running behind haproxy which also does SSL-offloading. This
> page uses an iframe in which links to directories are generated - these were
> redirected to trailing-slash versions by mod_dir with the wrong protocol. So
> i needed this solution and did not like the ServerName solution, because
> this does not work when both http and https are available. In apache the
> config is only available once, because of the haproxy loadbalancer in front
> of apache and apache sees no difference in the requests other than the
> x-forwarded-proto header.

So I just want to clarify, both PROXY protocol and RemoteIPInternalProxy
lists represent the intranet, absolutely safe routes which can be trusted,
so they should toggle the decoding of the designated RemoteIPProtoHeader.

If the route comes instead only through RemoteIPTrustedProxy or through
unrecognized proxies, the protocol should not be overridden, IMO. Would 
you concur?

Looks like we are close to having a patch to commit to trunk for further
feedback and potential backporting.
Comment 9 Axel Reinhold 2019-01-17 07:54:50 UTC
In my configuration the proxy is in my local virtual network which has no routing outside the virtual machines running on the host:

# grep -i remoteip *
modules.conf:LoadModule remoteip_module modules/mod_remoteip.so
httpd.conf:<IfModule mod_remoteip.c>
httpd.conf:RemoteIPHeader X-Forwarded-For
httpd.conf:RemoteIPProtoHeader X-Forwarded-Proto
httpd.conf:RemoteIPInternalProxy 192.168.37.37

The patch is running in production since two weeks without any issue.