-
Notifications
You must be signed in to change notification settings - Fork 274
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
X-Forwarded headers not handled with Nginx + FPM #675
Comments
Don't turn cors on for this. In addition you need to ensure ingress-nginx is deployed with the following settings:
|
Ah no - this is a bit nuanced, because of FastCGI. Looks like we need something like this
However there are caveats when you have a full forwarded for chain as I have in my config because while I have other apps in my cluster that need this, fastcgi cannot handle a chain and needs to get one ip only. The comments on this SO post have context https://stackoverflow.com/a/36451158/2262288 - I am trying to see if the mentioned module is available in the NGINX container being used for this chart. You do get close with this - but I'm not sure if the module you are trying to use is in the container. EDIT: It appears my suspicion is correct, the module is not installed/enabled in the container: /etc/nginx/modules # ls
ngx_http_geoip_module-debug.so ngx_http_xslt_filter_module-debug.so
ngx_http_geoip_module.so ngx_http_xslt_filter_module.so
ngx_http_image_filter_module-debug.so ngx_stream_geoip_module-debug.so
ngx_http_image_filter_module.so ngx_stream_geoip_module.so
ngx_http_js_module-debug.so ngx_stream_js_module-debug.so
ngx_http_js_module.so ngx_stream_js_module.so It also looks like you forgot to set the following to disable the default config: config:
default: false Digging into options here. Nope, I was wrong it actually is built into this image: /etc/nginx/modules # nginx -V
nginx version: nginx/1.27.3
built by gcc 13.2.1 20240309 (Alpine 13.2.1_git20240309)
built with OpenSSL 3.3.0 9 Apr 2024 (running with OpenSSL 3.3.2 3 Sep 2024)
TLS SNI support enabled
configure arguments: --prefix=/etc/nginx --sbin-path=/usr/sbin/nginx --modules-path=/usr/lib/nginx/modules --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --with-perl_modules_path=/usr/lib/perl5/vendor_perl --user=nginx --group=nginx --with-compat --with-file-aio --with-threads --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_flv_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-http_v3_module --with-mail --with-mail_ssl_module --with-stream --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module --with-cc-opt='-Os -fstack-clash-protection -Wformat -Werror=format-security -fno-plt -g' --with-ld-opt='-Wl,--as-needed,-O1,--sort-common -Wl,-z,pack-relative-relocs' |
Ok.. this is working for me - on my external facing ingress controller - but not my internal controller. The external controller receives traffic port forwarded from the router and is seeing the true IP, while the internal one is seeing the ingress-nginx controller IP for the internal controller - I am combing through the configs because this is either a "spot the difference" issue - or something different about port forwarded traffic vs local: nginx:
enabled: true
config:
default: false
custom: |-
upstream php-handler {
server 127.0.0.1:9000;
}
map $http_x_forwarded_proto $fastcgi_https {
default "";
https "on";
}
server {
# Default, IPv4 only
listen 80;
# HSTS settings
# WARNING: Only add the preload option once you read about
# the consequences in https://hstspreload.org/. This option
# will add the domain to a hardcoded list that is shipped
# in all major browsers and getting removed from this list
# could take several months.
add_header Referrer-Policy "no-referrer" always;
add_header X-Content-Type-Options "nosniff" always;
add_header X-Download-Options "noopen" always;
add_header X-Frame-Options "SAMEORIGIN" always;
add_header X-Permitted-Cross-Domain-Policies "none" always;
add_header X-Robots-Tag "noindex, nofollow" always;
add_header X-XSS-Protection "1; mode=block" always;
# set max upload size
client_max_body_size 10G;
fastcgi_buffers 64 4K;
# Enable gzip but do not remove ETag headers
gzip on;
gzip_vary on;
gzip_comp_level 4;
gzip_min_length 256;
gzip_proxied expired no-cache no-store private no_last_modified no_etag auth;
gzip_types application/atom+xml application/javascript application/json application/ld+json application/manifest+json application/rss+xml application/vnd.geo+json application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/bmp image/svg+xml image/x-icon text/cache-manifest text/css text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy;
# Pagespeed is not supported by Nextcloud, so if your server is built
# with the `ngx_pagespeed` module, uncomment this line to disable it.
#pagespeed off;
# Remove X-Powered-By, which is an information leak
fastcgi_hide_header X-Powered-By;
# Add .mjs as a file extension for javascript
# Either include it in the default mime.types list
# or include you can include that list explicitly and add the file extension
# only for Nextcloud like below:
include mime.types;
types {
text/javascript js mjs;
}
set_real_ip_from 10.2.0.0/16; # My pod IPv4 subnet
real_ip_header X-Forwarded-For;
real_ip_recursive on;
# Path to the root of your installation
root /var/www/html;
# Specify how to handle directories -- specifying `/index.php$request_uri`
# here as the fallback means that Nginx always exhibits the desired behaviour
# when a client requests a path that corresponds to a directory that exists
# on the server. In particular, if that directory contains an index.php file,
# that file is correctly served; if it doesn't, then the request is passed to
# the front-end controller. This consistent behaviour means that we don't need
# to specify custom rules for certain paths (e.g. images and other assets,
# `/updater`, `/ocm-provider`, `/ocs-provider`), and thus
# `try_files $uri $uri/ /index.php$request_uri`
# always provides the desired behaviour.
index index.php index.html /index.php$request_uri;
# Rule borrowed from `.htaccess` to handle Microsoft DAV clients
location = / {
if ( $http_user_agent ~ ^DavClnt ) {
return 302 /remote.php/webdav/$is_args$args;
}
}
location = /robots.txt {
allow all;
log_not_found off;
access_log off;
}
# Make a regex exception for `/.well-known` so that clients can still
# access it despite the existence of the regex rule
# `location ~ /(\.|autotest|...)` which would otherwise handle requests
# for `/.well-known`.
location ^~ /.well-known {
# The following 6 rules are borrowed from `.htaccess`
location = /.well-known/carddav { return 301 /remote.php/dav/; }
location = /.well-known/caldav { return 301 /remote.php/dav/; }
# Anything else is dynamically handled by Nextcloud
location ^~ /.well-known { return 301 /index.php$uri; }
try_files $uri $uri/ =404;
}
# Rules borrowed from `.htaccess` to hide certain paths from clients
location ~ ^/(?:build|tests|config|lib|3rdparty|templates|data)(?:$|/) { return 404; }
location ~ ^/(?:\.|autotest|occ|issue|indie|db_|console) { return 404; }
# Ensure this block, which passes PHP files to the PHP process, is above the blocks
# which handle static assets (as seen below). If this block is not declared first,
# then Nginx will encounter an infinite rewriting loop when it prepends `/index.php`
# to the URI, resulting in a HTTP 500 error response.
location ~ \.php(?:$|/) {
# Required for legacy support
rewrite ^/(?!index|remote|public|cron|core\/ajax\/update|status|ocs\/v[12]|updater\/.+|oc[ms]-provider\/.+|.+\/richdocumentscode(_arm64)?\/proxy) /index.php$request_uri;
fastcgi_split_path_info ^(.+?\.php)(/.*)$;
set $path_info $fastcgi_path_info;
try_files $fastcgi_script_name =404;
include fastcgi_params;
fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
fastcgi_param PATH_INFO $path_info;
#fastcgi_param HTTPS on;
fastcgi_param modHeadersAvailable true; # Avoid sending the security headers twice
fastcgi_param front_controller_active true; # Enable pretty urls
fastcgi_pass php-handler;
fastcgi_intercept_errors on;
fastcgi_request_buffering off;
fastcgi_param HTTPS $fastcgi_https;
fastcgi_param SERVER_PORT $http_x_forwarded_port;
}
location ~ \.(?:css|js|svg|gif)$ {
try_files $uri /index.php$request_uri;
expires 6M; # Cache-Control policy borrowed from `.htaccess`
access_log off; # Optional: Don't log access to assets
}
location ~ \.woff2?$ {
try_files $uri /index.php$request_uri;
expires 7d; # Cache-Control policy borrowed from `.htaccess`
access_log off; # Optional: Don't log access to assets
}
location / {
try_files $uri $uri/ /index.php$request_uri;
}
} |
OK, i've crawled the LoadBalancers from Metal LB, through the ingress-nginx and external-ingress-nginx setups, to the two ingress objects - there are zero in-cluster differences. So something that gets sent by the router during port-forwarding is making the above work |
Ok, the internal ingress-nginx needed |
Hello,
Exactly The only best-practice as described by the official Nginx documentation is using ngx_http_realip_module. This is also the way the ingress controller uses to provide the client's source IP. I investigated a lot on this subject before opening this case, and didn't found better "cleaner" ways, it seems clean for me.
I did't set compute-full-forwarded-for to
You're right, this block is in my config but forgot to include it..
I also forgot to mention that I use Merry Christmas and happy new year 🎄 |
Just added |
This ensures that the upstream IPs are preserved through the setup. I've never had any apps not able to handle this, and if you do not have it, then ingress-nginx will only keep the IP that forwarded the traffic to it, and lose any client context in the forwarding chain. Try it and see if it resolves the problem for you. Ideally once we both have it solved then a PR should be opened so these custom configs are not needed. |
Ah okay, after checking - nginx-fpm got the correct ip (and is viewable in the So we should change: with
nice founding (and debugging) |
Describe your Issue
I run Nextcloud behind a Openstack Octavia Loadbalancer (with HTTPS termination), that send
X-Forwarded-For
,X-Forwarded-Port
andX-Forwarded-Proto
headers to the Nginx ingress controller.The LB pool is configured to use the Proxy Protocol in order to preserve client source IP.
Natively, the chart does not allow me to pass these headers from Nginx to PHP-FPM, so I had to create a custom configuration.
Also, the documentation is wrong about
trusted_proxies
Nextcloud parameter. In our case, it should be equal to['127.0.0.1']
, since the Nextcloud instance and the Nginx webserver are running under the same pod, and since the trafic is forwarded to the PHP upstream on 127.0.0.1 too.Logs and Errors
Under Nextcloud Administration Area > Admin Settings > Security, the recognized IP is something like
::ffff:10.42.X.X
this is the address of therke2-nginx-ingress-controller
, with a helm configuration that is supposed to be correct (I think)On the Nginx container logs,
$http_x_forwarded_for
log value is correct, but not transmitted to PHP.Describe your Environment
Kubernetes distribution: RKE2 v1.31
Argo-CD Version : v2.13.1+af54ef8
Helm Version : v3.15.4+gfa9efb0
Helm Chart Version: 6.5.1
values.yaml
: (issue non relative content is removed)Additional context, if any
With this solution, you can pass all the headers that I mentionned. Working fine :)
Creating conditions on the Helm chart, it would be great to implement these fixes.
The text was updated successfully, but these errors were encountered: