Created attachment 163104 [details] [review] Patch CF::Auth in_iprange to allow for CloudFlare proxying When Koha is run behind a cloudflare proxy, cloudflare obfuscates the REMOTE_ADDR that is provided to koha. It does, however, add another header that we can use to check the actual origin of the client - HTTP_CF_CONNECTING_IP. This patch simply checks to see if this new header is present, indicating koha is behind the cloudflare poroxy, and uses it instead of REMOTE_ADDR to check if a client's IP address is in a valid range.
Cloudflare is just a reverse proxy like any other, so it will pass the client's IP in the X-Forwarded-For header. This means that you should be able to use "koha_trusted_proxies" in koha-conf.xml to handle this scenario. This also means REMOTE_ADDR will be the client's IP for other IP-based scenarios like ILS-DI, logging, etc. Cloudflare provides a list of their public IP addresses at https://www.cloudflare.com/en-gb/ips/ -- If you did want to use HTTP_CF_CONNECTING_IP specifically, you could look at modifying Koha/Middleware/RealIP.pm But since CF_CONNECTING_IP is user-supplied you would still want to validate the REMOTE_ADDR against the list of Cloudflare's IP addresses. Otherwise, anyone could stick their own reverse proxy in front of your Koha and pretend to be someone else by inserting their own header.
Created attachment 163515 [details] [review] Preference and plumbing to trust cloudflare proxies automatically This patch adds in a new preference, and updates Koha::Middleware::RealIP to automatically add the current list of CloudFlare proxies as trusted proxies. This relieves administrators of having to constantly check if the cloudflare proxies are correctly configured on koha, since cloudflare does occassionally add or remove proxies. Rather than individual IPs being added to the trusted_proxies list, this also uses the CIDRs that cloudflare provides, and that koha uses internally. In the future, it might be nice to use the CF_CONNECTING_IP header, but this shouldn't be necessary right now.
Thanks for your work on this, Nicholas. I don't mean to be a stickler, but since koha_trusted_proxies is a koha-conf.xml config, it probably makes sense to have this one be one too. I like the idea of using the Cloudflare API to keep an up-to-date list, but the list rarely changes. According to https://www.cloudflare.com/en-gb/ips/ it's only changed 4 times in about 7 years. So I'm not sure that we should have each worker call the API on startup. Personally, I find the startup super slow already. It might make more sense for the list to be updated by a cronjob. Perhaps the "TrustCloudFlare" config option could be a path to the list. Another thought I had might be to add a timeout to LWP::UserAgent, because I think it defaults to 180 seconds. If there were an issue with the CloudFlare API, I don't think you'd want each worker to wait 3 minutes before starting up.
Created attachment 163973 [details] [review] Support for externally updated proxy lists Thanks for the feedback, David! It makes sense to move it into the koha-conf.xml file, and this updated patch has done so. It includes a sample bash script that can be used to keep an external list file up to date. This can also easily be extended to automatically update other providers, hence I have taken away the cloudflare name, although cloudflare is referenced in a comment in koha-conf.xml Currently, RealIP was re-generating the trusted_proxies list on every call to get_real_ip. This patch also uses Koha's caching system to prevent reloading from the config file and from the external list on every call, which should speed things up just a little bit. The sample bash script will restart koha-common if it is running, and also clear the caches of all koha-instances.
(In reply to Nicholas van Oudtshoorn from comment #4) > Created attachment 163973 [details] [review] [review] > Support for externally updated proxy lists > > Thanks for the feedback, David! Thanks for being receptive! > It makes sense to move it into the koha-conf.xml file, and this updated > patch has done so. It includes a sample bash script that can be used to keep > an external list file up to date. This can also easily be extended to > automatically update other providers, hence I have taken away the cloudflare > name, although cloudflare is referenced in a comment in koha-conf.xml You'll want to add the config block to "debian/templates/koha-conf-site.xml.in" as well. (I think it could be worthwhile to say in the comment that the list is a plain-text with an IP (range) per line, but not a blocker.) > Currently, RealIP was re-generating the trusted_proxies list on every call > to get_real_ip. This patch also uses Koha's caching system to prevent > reloading from the config file and from the external list on every call, > which should speed things up just a little bit. Oh yikes. I remember reading through this recently, and I saw "$self->trusted_proxy()" in prepare_app(), and that list is passed to get_real_ip(), so I figured it was just determining the list at startup time, but it looks like it's unused currently and it really is fetching it on every call! Double yikes... I think we'd want to split the changes here. I think a new ticket to fix RealIP.pm to use the $self->trusted_proxy() set up in "prepare_app", and we could also set that up to use the cache to further improve performance. The change to RealIP.pm looks like it has some copy/pasted code. I'd suggest adding the list IPs to @trusted_proxies() rather than having 2 separate blocks calling Net::Netmask->new2(). > The sample bash script will restart koha-common if it is running, and also > clear the caches of all koha-instances. Since this one is Cloudflare specific, I think it would be worthwhile including that in the script name (e.g. "update_cloudflare_proxies_list.sh). I think it might be overkill restarting the whole koha-common service, since we really just need to restart the Plack in this case. You'd want to clear the cache first before running koha-plack --reload/--restart as well I believe. -- Overall, I think it's looking pretty good though.