Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. We can use Plack::Middleware::RealIP to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable so no Koha code need be modified. A future enhancement could be to take advantage of the module's trusted_proxy setting to allow Koha to use these feature even after going through *multiple* proxies. This would require being able to read configuration from the plack.psgi file and is beyond the scope of this enhancement.
Actually, we can use an environment variable so I'm increasing the scope to include setting the list of proxies!
Created attachment 90395 [details] [review] Bug 23068: [DO NOT PUSH] Add debugging to C4::Context
Created attachment 90396 [details] [review] Bug 23068 - Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. We can use Plack::Middleware::RealIP to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable so no Koha code need be modified. We can take advantage of the module's trusted_proxy setting to allow Koha to use these feature even after going through *multiple* proxies. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs is what you put in the environment variable: KOHA_TRUSTED_PROXIES="172.22.0.1 1.1.1.1" koha-plack --restart kohadev 9) Repeat step 6 7) You should now see 32.32.32.32 as the remote address in your logs!
*** Bug 21477 has been marked as a duplicate of this bug. ***
Created attachment 90582 [details] [review] Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file
QA Questions 1) Does this work with the Mojo derived API's too? 2) What if Koha is running under CGI and not plack - yes.. there are still some out there :P
Created attachment 91173 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. We can use Plack::Middleware::RealIP to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable so no Koha code need be modified. We can take advantage of the module's trusted_proxy setting to allow Koha to use these feature even after going through *multiple* proxies. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs is what you put in the environment variable: KOHA_TRUSTED_PROXIES="172.22.0.1 1.1.1.1" koha-plack --restart kohadev 9) Repeat step 6 7) You should now see 32.32.32.32 as the remote address in your logs! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 91174 [details] [review] Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Fixed up the commit line to get the QA scripts happy and signing off.. I'll leave my two QA questions for a QA person to follow up but I didn't see any point in holding up SO.
Created attachment 91447 [details] [review] Bug 23068: Allow the list of proxy IPs to be set in the koha-conf.xml file Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Found a problem. The middleware is working via plack so we can the real ip address when we are on a script that uses plack. When we visit a script that doesn't use plack, this code doesn't get executed and so Koha sees the proxy ip instead. And Koha does what it always does when a session changes ip addresses, it forces a re-login
Created attachment 92461 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf: <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies> 9) Restart all the things! 10) Repeat step 6 11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs! 12) Disable plack so you are running in cgi mode, repeat step 6 again 13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!
Created attachment 92462 [details] [review] Bug 23068: [DO NOT PUSH] Debugging code for test plan
Created attachment 92463 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf: <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies> 9) Restart all the things! 10) Repeat step 6 11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs! 12) Disable plack so you are running in cgi mode, repeat step 6 again 13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs!
Created attachment 92464 [details] [review] Bug 23068: [DO NOT PUSH] Debugging code for test plan
Created attachment 94807 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf: <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies> 9) Restart all the things! 10) Repeat step 6 11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs! 12) Disable plack so you are running in cgi mode, repeat step 6 again 13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs! Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Created attachment 94808 [details] [review] Bug 23068: [DO NOT PUSH] Debugging code for test plan Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
(In reply to Martin Renvoize from comment #6) > QA Questions > 1) Does this work with the Mojo derived API's too? > 2) What if Koha is running under CGI and not plack - yes.. there are still > some out there :P I have these questions too, although I've been using CGI behind (sometimes multiple) reverse proxies for a long time, so I've already hacked around where necessary to support X-Forwarded-For. It's not really shareable code though as it's very specific.
Comment on attachment 94807 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Review of attachment 94807 [details] [review]: ----------------------------------------------------------------- ::: C4/Context.pm @@ +1107,5 @@ > + > +=cut > + > +sub set_remote_address { > + if ( C4::Context->config('koha_trusted_proxies') ) { Wouldn't this still be called under Plack? Couldn't we just look to see if it's Plack via environmental variables and only do this for CGI?
(In reply to David Cook from comment #19) > Comment on attachment 94807 [details] [review] [review] > Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so > REMOTE_ADDR features work behind a proxy > > Review of attachment 94807 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Context.pm > @@ +1107,5 @@ > > + > > +=cut > > + > > +sub set_remote_address { > > + if ( C4::Context->config('koha_trusted_proxies') ) { > > Wouldn't this still be called under Plack? > > Couldn't we just look to see if it's Plack via environmental variables and > only do this for CGI? That's a good thought. Is there an environment variable that is set for scripts running under plack?
(In reply to David Cook from comment #19) > Comment on attachment 94807 [details] [review] [review] > Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so > REMOTE_ADDR features work behind a proxy > > Review of attachment 94807 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Context.pm > @@ +1107,5 @@ > > + > > +=cut > > + > > +sub set_remote_address { > > + if ( C4::Context->config('koha_trusted_proxies') ) { > > Wouldn't this still be called under Plack? > > Couldn't we just look to see if it's Plack via environmental variables and > only do this for CGI? Actually, it only gets called under plack at startup, since C4::Auth is loaded at that time. So, yes, we could do that, but it doesn't actually hurt performance by any meaningful metric!
(In reply to Martin Renvoize from comment #6) > QA Questions > 1) Does this work with the Mojo derived API's too? Yes it does! > 2) What if Koha is running under CGI and not plack - yes.. there are still > some out there :P It does indeed! I had to engineer it specifically to work under CGI as well as we still have a handful of scripts that we still run under CGI instead of plack.
Created attachment 94838 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf: <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies> 9) Restart all the things! 10) Repeat step 6 11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs! 12) Disable plack so you are running in cgi mode, repeat step 6 again 13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs! Signed-off-by: Ed Veal <eveal@mckinneytexas.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Sorry dude.. lots of QA script failures.. and I think this also introduces a new dependancy that will need to be listed: FAIL C4/Auth.pm FAIL valid Can't locate object method "set_remote_address" via package "C4::Context" OK C4/Context.pm FAIL Koha/Middleware/RealIP.pm FAIL critic Variable declared in conditional statement at line 53, column 9. Declare variables outside of the condition. FAIL pod *** WARNING: =head3 without preceding higher level in file Koha/Middleware/RealIP.pm *** WARNING: =head3 without preceding higher level in file Koha/Middleware/RealIP.pm *** WARNING: =head3 without preceding higher level in file Koha/Middleware/RealIP.pm *** WARNING: =head3 without preceding higher level in file Koha/Middleware/RealIP.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL valid Can't locate Net/Netmask.pm in @INC (you may need to install the Net::Netmask module) (@INC contains: /home/martin/kohaclone /home/martin/kohaclone/C4/SIP /home/martin/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) OK etc/koha-conf.xml Processing additional checks OK!
Created attachment 94839 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf: <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies> 9) Restart all the things! 10) Repeat step 6 11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs! 12) Disable plack so you are running in cgi mode, repeat step 6 again 13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs! Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
(In reply to Kyle M Hall from comment #21) > Actually, it only gets called under plack at startup, since C4::Auth is > loaded at that time. So, yes, we could do that, but it doesn't actually hurt > performance by any meaningful metric! I was wondering about that a bit. That's a good point hehe. In any case, there should be a number of "psgi.*" environmental variables: https://metacpan.org/pod/PSGI#The-Environment.
Created attachment 94919 [details] [review] Bug 23068: Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy Koha has a number of features that rely on knowing the IP address of the connecting client. If that server is behind a proxy these features do not work. This patch adds a module to automatically convert the X-Forwarded-For header into the REMOTE_ADDR environment variable for both CGI and Plack processes. TEST PLAN: 1) Apply this patch set 2) Install Plack::Middleware::RealIP via cpanm or your favorite utility 3) Update your plack.psgi with the changes you find in this patch set ( this process differs based on your testing environment ) 4) Restart plack 5) Tail the plack error log for your instance 6) Use curl to access the OPAC, adding an X-Forwarded-For header: curl --header "X-Forwarded-For: 32.32.32.32" http://127.0.0.1:8080 7) Note the logs output this address if you are unproxied 8) If you are proxied, restart plack using a command like below, where the ip you see in the logs ("REAL IP) is what you put in the koha conf: <koha_trusted_proxies>172.22.0.1 1.1.1.1</koha_trusted_proxies> 9) Restart all the things! 10) Repeat step 6 11) You should now see "REAL IP: 32.32.32.32" in the plack logs as the remote address in your plack-error.log logs! 12) Disable plack so you are running in cgi mode, repeat step 6 again 13) You should see "REAL IP: 32.32.32.32" as the remove address in your opac-error.log logs! Signed-off-by: Ed Veal <eveal@mckinneytexas.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice work Kyle, all QA issues fixed. Works well, dependancies is available in D8 by default so no need to worry about packaging. Passing QA :)
Nice work! Pushed to master for 19.11.00
Yay! Nice one, Kyle!
Great job Kyle.
KohaDevBoxes (even new ones) and other development environments based on Debian or Ubuntu need to install the package "libnet-netmask-perl"
(In reply to Victor Grousset/tuxayo from comment #32) > KohaDevBoxes (even new ones) and other development environments based on > Debian or Ubuntu need to install the package "libnet-netmask-perl" I've often wondered when the debian control files get updated with the dependencies...
(In reply to Victor Grousset/tuxayo from comment #32) > KohaDevBoxes (even new ones) and other development environments based on > Debian or Ubuntu need to install the package "libnet-netmask-perl" Correct, it has been escalated to a 'required' dependancy. This is because for plack we need to load it before we load C4::Context and as such we can't use it optionally as initially thought.
(In reply to David Cook from comment #33) > (In reply to Victor Grousset/tuxayo from comment #32) > > KohaDevBoxes (even new ones) and other development environments based on > > Debian or Ubuntu need to install the package "libnet-netmask-perl" > > I've often wondered when the debian control files get updated with the > dependencies... The control files were updated almost immediately.. The process is to add a dependency to C4/Installer/PerlDependancies.pm and then push that up to master. The evening the nightly build bot will spot the new dependency and add it to the control file and send the patch to bug 17084 which the release manager should be watching. The next day, the release manager will do a quick QA on the resultant file and push the patch as necessary. So, in this case this took a sum total of a day to get the new dependency into the debian control files (there was indeed a day of Jenkins getting upset about the missing dependency too as the packages were failing to build). I can't entirely remember how the biblibre sandboxes work (koha-testing-docker, which koha-sandboxes-docker depends upon for it's base koha image, has already been updated). Do you now re-run apt install koha-perldeps or anything regularly? That should have caught the debian control change.. also, I presume if you are running such a command then you're also tracking the nightly repository to pickup the very latest changes. Hope that helps, Martin
(In reply to Martin Renvoize from comment #34) > Correct, it has been escalated to a 'required' dependancy. This is because > for plack we need to load it before we load C4::Context and as such we can't > use it optionally as initially thought. I was thinking we could actually make good use of Net::Netmask in other parts of Koha as well. I'd love to use it for processing ILS-DI:AuthorizedIPs (could also use it for OpacSuppressionByIPRange, RestrictedPageLocalIPs, and surely other places that don't come to mind yet...). So it being a required dependency could be a motivator for updating those too. *puts it on his infinite todo list*.
> Do you now re-run apt install koha-perldeps or anything regularly? That should have caught the debian control change.. also, I presume if you are running such a command then you're also tracking the nightly repository to pickup the very latest changes. It was on a KohaDevBox, I guess the nightly repo wasn't up to date yet.
I just want to say that this is working beautifully in 19.11. Thanks a lot, Kyle!
I've been thinking about this one more, and I just want to say again well done, Kyle! I like how it can handle multiple proxies, and the trust model so that end users can't hack REMOTE_USER using their own user-supplied headers. It's too bad there are no unit tests for Koha::Middleware::RealIP. I ended up writing a very basic one here just to test that it worked for multiple proxies.
One last note for people interested in this feature, Net::Netmask lets you define IP addresses in a number of ways. If you wanted to trust a whole local subnet, you could do 192.168.0.0/16, or something like that. (If you're using a proxy like an AWS Elastic Load Balancer, which is dynamically assigned an IP address chosen by AWS, you could specify the subnets for your ELBs, and then wouldn't have to worry about IP addresses changing and keeping your koha-conf.xml up to date.)
Ok found a little problem and I'll open up a separate bug report for it.
(In reply to David Cook from comment #39) > It's too bad there are no unit tests for Koha::Middleware::RealIP. I ended > up writing a very basic one here just to test that it worked for multiple > proxies. Agreed, but I didn't want to hold up this vital development and we weren't at all sure how to write such a test.. as it sounds like you've got the foundations for a test written, please submit it upstream ;)
(In reply to Martin Renvoize from comment #42) > (In reply to David Cook from comment #39) > > It's too bad there are no unit tests for Koha::Middleware::RealIP. I ended > > up writing a very basic one here just to test that it worked for multiple > > proxies. > > Agreed, but I didn't want to hold up this vital development and we weren't > at all sure how to write such a test.. as it sounds like you've got the > foundations for a test written, please submit it upstream ;) That makes sense. I really am loving this development overall! I'll do a patch for https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24538 and write a test for it. It won't be total coverage I suppose, but it should be enough to test the change.
Thanks David, that's great news.
(In reply to Martin Renvoize from comment #44) > Thanks David, that's great news. Finally getting around to this... Have to leave for the night but the following isn't working for some reason: t::lib::Mocks::mock_preference('koha_trusted_proxies', '1.1.1.1'); Will have to look at this more tomorrow...
koha_trusted_proxies is a config setting from the Koha conf xml file, so mock_preference won't work on that. You'll want to use mock_config instead. (In reply to David Cook from comment #45) > (In reply to Martin Renvoize from comment #44) > > Thanks David, that's great news. > > Finally getting around to this... > > Have to leave for the night but the following isn't working for some reason: > > t::lib::Mocks::mock_preference('koha_trusted_proxies', '1.1.1.1'); > > Will have to look at this more tomorrow...
(In reply to Kyle M Hall from comment #46) > koha_trusted_proxies is a config setting from the Koha conf xml file, so > mock_preference won't work on that. You'll want to use mock_config instead. > So obvious in the light of day hehe. Thanks, Kyle. Yesterday was a loooong day.
(In reply to Martin Renvoize from comment #44) > Thanks David, that's great news. It only took me nearly 3 weeks but I finally got there heh. Unit test and bug fix available at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24538.
Unfortunately, it looks like this feature breaks file upload for CGI Koha. I haven't checked Plack Koha yet but might still be relevant. Please see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24719 for more details. I'll include a first go at a patch there, which I've shown to work in CGI Koha.
Trying to remember why we needed this since in theory Plack::Middleware::ReverseProxy should've handled this...
Also wondering how we could update REMOTE_USER so that it appears in plack.log...
(In reply to David Cook from comment #50) > Trying to remember why we needed this since in theory > Plack::Middleware::ReverseProxy should've handled this... Ahh I see now. Plack::Middleware::ReverseProxy chooses the last IP in the list (rather than the first), so if you're using a reverse proxy in front of Koha's Apache reverse proxy, then you'll get the IP address for that reverse proxy instead of the client IP address. I guess I can understand that... since it is the most recent IP address to connect... and then we use Koha::Middleware::RealIP (inspired by Plack::Middleware::RealIP) to validate the proxy addresses and get the real client IP... -- I can get the client IP via $ENV{REMOTE_ADDR} in Koha as set by Koha::Middleware::RealIP, but plack.log is still using the proxy address as set by Plack::Middleware::ReverseProxy. (I've confirmed that by hacking on Plack::Middleware::ReverseProxy.pm in k-t-d.)
(In reply to David Cook from comment #52) > I can get the client IP via $ENV{REMOTE_ADDR} in Koha as set by > Koha::Middleware::RealIP, but plack.log is still using the proxy address as > set by Plack::Middleware::ReverseProxy. (I've confirmed that by hacking on > Plack::Middleware::ReverseProxy.pm in k-t-d.) This one is driving me a little crazy... It looks like /usr/share/perl5/Plack/Runner.pm applies the Plack::Middleware::AccessLog middleware and that's wrapped around the app last... Yet it appears to be getting the REMOTE_ADDR from Plack::Middleware::ReverseProxy and not Koha::Middleware::RealIP. (If you remove Plack::Middleware::ReverseProxy then it'll get it from Koha::Middleware::RealIP.) But it seems more complicated than that because adding Koha::Middleware::RealIP *after* Plack::Middleware::ReverseProxy doesn't seem to make a difference which seems illogical...
The problem is Koha::Middleware::SetEnv. It's a fundamentally flawed middleware. The wrapping of middleware works by passing around a hash reference (ie $env), and Koha::Middleware::SetEnv breaks the referential train. It passes a new hashref to Koha::Middleware::RealIP which then passes that new reference to the Koha app. But when the post-processing aspect of the Plack middleware runs, after leaving the Koha::Middleware::SetEnv wrapper, it goes back to Plack::Middleware::Static which has the *original* reference, which undoes the changes of Koha::Middleware::SetEnv wrapper and Koha::Middleware::RealIP. We don't notice this because a lot of our middleware only "pre-process". But Plack::Middleware::AccessLog" is a "post-process" middleware. This should actually be a fairly easy fix, and it'll fix the IP address in plack.log when running behind a reverse proxy.
Boy that was crazy-making but feels good to figure out a tough problem [U+1F605]
(In reply to David Cook from comment #55) > Boy that was crazy-making but feels good to figure out a tough problem [U+1F605] Thanks not only for figuring this out, but writing it up for the rest of us!
(In reply to Kyle M Hall from comment #56) > Thanks not only for figuring this out, but writing it up for the rest of us! Thanks, Kyle! That really means a lot to me :)