Bug 25361 - Support X-Forwarded-* headers
Summary: Support X-Forwarded-* headers
Status: CLOSED WORKSFORME
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-04 00:10 UTC by David Cook
Modified: 2021-06-14 21:29 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-05-04 00:10:12 UTC Comment hidden (obsolete)
Comment 1 David Cook 2020-05-04 00:19:13 UTC Comment hidden (obsolete)
Comment 2 David Cook 2020-05-04 00:20:25 UTC Comment hidden (obsolete)
Comment 3 David Cook 2020-05-04 00:38:42 UTC
Thanks to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21267, the X-Forwarded-Proto header gets set in Apache when using Plack, so Plack::Middleware::ReverseProxy will set the $ENV->{HTTPS} to true. 
(With CGI, you'd need to add the headers in your reverse proxy, but Koha doesn't have a mechanism for using X-Forwarded-Proto for CGI Koha.)

Thanks to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23068, there is support for X-Forwarded-For for both Plack and CGI Koha. (mod_proxy_http in Apache should automatically add headers for X-Forwarded-For, X-Forwarded-Host, and X-Forwarded-Server). I guess Plack::Middleware::ReverseProxy runs too and takes care of the first reverse proxy which will be Apache on the local machine...
Comment 4 David Cook 2020-05-04 02:37:09 UTC
Plack:
- Apache should set X-Forwarded-* headers so that they can be consumed by Plack::Middleware::ReverseProxy

CGI:
- Apache should set HTTP headers using X-Forwarded-* headers from a separate reverse proxy?

--

It makes sense that the community maintain the Apache configuration in the Plack context I suppose, but perhaps it doesn't make sense for CGI, since 1) we don't recommend CGI, and 2) you don't have to use a reverse proxy out of the box.

--

But then Bug 23068 has set a precedent by adding Plack/CGI support for the X-Forwarded-For header. Support which is really useful. 

There are so many different levels where these things can be handled with their own pros and cons...
Comment 5 Marcel de Rooy 2020-08-05 13:09:47 UTC
I do not understand what this report is about. We already have support for X-Forwarded headers. So what is the point ?
Comment 6 David Cook 2020-08-06 01:00:17 UTC
(In reply to Marcel de Rooy from comment #5)
> I do not understand what this report is about. We already have support for
> X-Forwarded headers. So what is the point ?

It's been a while since I reported this one, so I'm not 100% sure of my original intent. I didn't word it very well.

However, in Koha, we actually only use X-Forwarded-For and X-Forwarded-Proto. Bug 23068 explicitly uses X-Forwarded-For, while X-Forwarded-Proto is used by Plack::Middleware::ReverseProxy to set $ENV->{HTTPS} to true.

Apache's mod_proxy_http automatically sets X-Forwarded-For, X-Forwarded-Host, and X-Forwarded-Server, I believe, but doesn't do others like X-Forwarded-Proto (which is why Tomas added it in Bug 21267). 

AWS load balancers pass X-Forwarded-For, X-Forwarded-Proto, and X-Forwarded-Port. 

I mostly see utility for X-Forwarded-For and X-Forwarded-Proto. Not really sure why a person would care about X-Forwarded-Port 

I think I'll close this one for now.