Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloudflare real visitor IP support in Nginx config #198

Closed
jessuppi opened this issue Apr 20, 2023 · 18 comments
Closed

Cloudflare real visitor IP support in Nginx config #198

jessuppi opened this issue Apr 20, 2023 · 18 comments

Comments

@jessuppi
Copy link
Member

Some users have mentioned that SlickStack doesn't support the "real IPs" feature from Cloudflare, which will send the visitor's real IP address to the origin server for diagnostics, or for determining proper country, etc.

Personally I have several WooCommerce clients using country detection plugins who I think never had any problem with SlickStack's default config, I guess because these plugins use third party tracking javascript and such (based on my memory only) so they don't require Nginx to get involved.

But some users are saying they still would like to be able to see real IPs for security/logging reasons.

I'm not sure the privacy/GDPR/etc concerns with this stuff, but it's still probably a feature we should get working properly, but perhaps keep it disabled by default?

@jessuppi
Copy link
Member Author

jessuppi commented Apr 20, 2023

Here's an overview of the problem from Cloudflare:

https://developers.cloudflare.com/support/troubleshooting/restoring-visitor-ips/restoring-original-visitor-ips/

...and their Nginx config example:

set_real_ip_from 192.0.2.1 (example IP address)
(repeat for all Cloudflare IPs listed at https://www.cloudflare.com/ips/)

#use any of the following two

real_ip_header CF-Connecting-IP;
#real_ip_header X-Forwarded-For;

The docs are messy but it also contains this note, possibly for Apache:

To Include the original visitor IP in your logs, add the variables $http_cf_connecting_ip and $http_x_forwarded_for in the log_format directive.

They also warn:

That list of prefixes needs to be updated regularly, and we publish the full list in Cloudflare IP addresses.

@jessuppi
Copy link
Member Author

And here's the sort of de facto semi-official Nginx config that I've seen shared:

https://github.com/ergin/nginx-cloudflare-real-ip

@NathanAdhitya
Copy link

NathanAdhitya commented Apr 20, 2023

To Include the original visitor IP in your logs, add the variables $http_cf_connecting_ip and $http_x_forwarded_for in the log_format directive.

is indeed for Apache. There are no other modifications needed to be done in vanilla NGINX that's prepackaged by most distros except adding the real_ip_header and set_real_ip. NGINX's logs will use the actual client IP when that's used.

I really wish for this to be pushed forward, as many features rely on PHP's $_SERVER["REMOTE_ADDR"], or certain features e.g. rate limiting rely on $binary_remote_addr, which is incorrectly defined when connecting through a reverse proxy and not using set_real_ip.

@jessuppi
Copy link
Member Author

@jessuppi
Copy link
Member Author

jessuppi commented May 7, 2023

Without testing this entirely, I think we can simply swap out:

include /etc/nginx/conf.d/*.conf;

For this:

include /var/www/sites/includes/cloudflare[.]conf;
include /var/www/sites/includes/featurepolicy[.]conf;

....in the nginx.conf and it should be fine... it would be up to each sysadmin to ensure those work (for now).

@jessuppi
Copy link
Member Author

jessuppi commented May 7, 2023

7ecdc6f

@jessuppi
Copy link
Member Author

jessuppi commented May 17, 2023

We've copied over /etc/nginx/conf.d/cloudflare.conf to /var/www/sites/includes/cloudflare.conf if that file exists on all SlickStack servers.

The following line is now added to nginx.conf in SlickStack:

include /var/www/sites/includes/cloudflare[.]conf;

@jessuppi
Copy link
Member Author

We force deleted the old cloudflare.conf files if exist and reinstalled nginx.conf via ss-worker command.

In the future we should validate the cloudflare.conf files to ensure that the syntax looks correct perhaps, otherwise force delete that file if it doesn't pass the test... we could also possibly generate cloudflare.conf automatically in the future as a nifty SlickStack feature?

@vivianedias
Copy link

vivianedias commented May 17, 2023

Hey @jessuppi I'd like to enable a custom featurepolicy for my nginx config, but have no idea how that config should look like 😅 I know i need this header to look like:

add_header Permissions-Policy "camera=*, encrypted-media=(), geolocation=(), microphone=*, midi=()";

I've edited on the nginx.conf file for now, but I'm looking at a more long-term solution, so the featurepolicy.conf file would be it, but I'm not sure how I should declare this header in there to overwrite the default.

@jessuppi
Copy link
Member Author

@vivianedias Okay we will add that shortly after testing, please open a different Issue if any further feedback --

@jessuppi
Copy link
Member Author

If anyone wants to contribute toward example cloudflare.conf file:

https://github.com/littlebizzy/slickstack/blob/master/modules/nginx/includes/cloudflare-conf.txt

@NathanAdhitya
Copy link

If anyone wants to contribute toward example cloudflare.conf file:

https://github.com/littlebizzy/slickstack/blob/master/modules/nginx/includes/cloudflare-conf.txt

I say automatically generate contents via a script that fetches Cloudflare's IPv6 and IPv4 list.

@jessuppi
Copy link
Member Author

jessuppi commented Aug 7, 2023

@jessuppi
Copy link
Member Author

jessuppi commented Aug 7, 2023

@jessuppi
Copy link
Member Author

jessuppi commented Aug 7, 2023

An interesting Issue on @ergin repo shows how to pass real IPs while blocking non-Cloudflare, which is quite interesting and worth exploring later:

ergin/nginx-cloudflare-real-ip#3

@NathanAdhitya
Copy link

NathanAdhitya commented Aug 7, 2023

My pragmatic way is to literally block non-cloudflare IPs to port 443 with ufw/iptables.

In short, implicit deny, allow from cloudflare IPs.

@jessuppi
Copy link
Member Author

jessuppi commented Aug 7, 2023

Okay this is now supported as of build version AUG2023D ... simply ensure the new NGINX_CLOUDFLARE_IPS variable is set to a value of true in your ss-config. Then, when SlickStack runs ss-install-nginx-config script it will install the new cloudflare.conf includes file under /var/www/sites/includes/ automatically.

You can also run ss install nginx cloudflare if you want to manually reinstall/update the Cloudflare IP ranges.

Next, we will add this task to the cron jobs interval...

@jessuppi
Copy link
Member Author

jessuppi commented Aug 8, 2023

Done!

INTERVAL_SS_INSTALL_NGINX_CLOUDFLARE_IPS controls the schedule now in ss-config and half-weekly is the default value which should be more than often enough since Cloudflare's IP ranges rarely change.

Also a point of clarification, that ss-install-nginx-config in fact calls a different script ss-install-nginx-cloudflare-ips in order to do this... so you can run either script and cloudflare.conf will be installed, or you can run only the script for cloudflare instead if you want too.

Future comments welcome, for now this is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants