https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Actually, if your REMOTE_ADDR is 1.1.1.1 and your X-Forwarded-For is 1.1.1.1 and your koha_trusted_proxies is 1.1.1.0/24, then your REMOTE_ADDR will be reset to null. If there is only 1 value in the X-Forwarded-For, we shouldn't be looking for proxies. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23068 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23068 [Bug 23068] Add ability for Koha to handle X-Forwarded-For headers so REMOTE_ADDR features work behind a proxy -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|REMOTE_ADDR if client_ip in |REMOTE_ADDR set to null if |X-Forwarded-For matches a |client_ip in |koha_trusted_proxies value |X-Forwarded-For matches a | |koha_trusted_proxies value -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- I probably am not doing a great job of explaining this one. Without the patch, the "<client>" in the X-Forwarded-For will be tested against koha_trusted_proxies, even though it's not a proxy. So if the "<client>" matches against koha_trusted_proxies, it won't be used to set REMOTE_ADDR, which means it'll be left as null. With the patch, the "<client>" is extracted from the X-Forwarded-For header data, and then the REMOTE_ADDR (the proxy that set the X-Forwarded-For) and any "<proxy>" values from the X-Forwarded-For header are evaluated against koha_trusted_proxies. If REMOTE_ADDR and "<proxy>" are all trusted, then the "<client>" is used to re-write REMOTE_ADDR. If they're not trusted, the last value not trusted will be used to re-write REMOTE_ADDR. So while you might trust your reverse proxy at REMOTE_ADDR, you might not trust the "<proxy>" values in X-Forwarded-For. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- I am pretty keen to get this going, so I'm going to apply it locally rather than wait. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 didier <didier.gautheron@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106647|0 |1 is obsolete| | --- Comment #6 from didier <didier.gautheron@biblibre.com> --- Created attachment 106722 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106722&action=edit Bug 25950: Remove <client> in X-Forwarded-For from proxy tests This patch removes the <client> ip address in the X-Forwarded-For header from being tested against koha_trusted_proxies. Without this patch, REMOTE_ADDR will be set to null, if the <client> ip address matches against koha_trusted_proxies. To Test: 1. Run the unit test t/Koha/Middleware/RealIP.t Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106722|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106763&action=edit Bug 25950: Remove <client> in X-Forwarded-For from proxy tests This patch removes the <client> ip address in the X-Forwarded-For header from being tested against koha_trusted_proxies. Without this patch, REMOTE_ADDR will be set to null, if the <client> ip address matches against koha_trusted_proxies. To Test: 1. Run the unit test t/Koha/Middleware/RealIP.t Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Cheers Martin :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00, 20.05.03 |20.11.00, 20.05.03, released in| |19.11.09 Status|Pushed to stable |Pushed to oldstable CC| |aleisha@catalyst.net.nz --- Comment #13 from Aleisha Amohia <aleisha@catalyst.net.nz> --- backported to 19.11.x for 19.11.09 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Got a random failure, D11 run 265 on Docker_5: 18:04:17 koha_1 | could not parse 2.2.2 at /kohadevbox/koha/Koha/Middleware/RealIP.pm line 117. 18:04:17 koha_1 | 18:04:17 koha_1 | # Failed test 'Trust proxy (2.2.2.2) using abbreviated notation, so use the X-Forwarded-For header for the remote address' 18:04:17 koha_1 | # at t/Koha/Middleware/RealIP.t line 114. 18:04:17 koha_1 | # got: '2.2.2.2' 18:04:17 koha_1 | # expected: '1.1.1.1' 18:04:17 koha_1 | # Looks like you failed 1 test of 3. 18:04:17 koha_1 | 18:04:17 koha_1 | # Failed test 'Test alternative configuration styles' 18:04:17 koha_1 | # at t/Koha/Middleware/RealIP.t line 121. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #16)

Maybe not random, it failed once again on D11 (run 266)

Is it happening every time? If so, what version of Net::Netmask is installed on D11? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #18)

Yes it's failing consistently on D11

Well that's good at least.

% pmvers Net::Netmask 1.9104

It looks like buster uses 1.9104-1 and bullseye uses 1.9104-2... And it looks like 1.9104-2 includes a backport of a security patch that doesn't allow the following formats: '216.240.32' '216.240' '140' '216.240.32/24' '216.240/16' The workaround is to use the "shortnet" option which is specified in the newer versions of the module: https://metacpan.org/pod/release/JMASLAK/Net-Netmask-2.0001/lib/Net/Netmask.... Note that we use Net::Netmask in a few places, so we'll need to either add the shortnet option, or make people update their systems. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25950 --- Comment #21 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #20)

I feel like adding the shortnet => 1 constructor option is the most backwards compatible...

Oh goodie, they've changed the constructor in a way that is not backwards compatible with 1.9104. Awesome. Fortunately there is another workaround. But jeez... See Bug 28200. -- You are receiving this mail because: You are watching all bug changes.