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

set_real_ip_from (ngx_http_realip_module) should be a list #415

Open
glbyers opened this issue Mar 19, 2024 · 1 comment
Open

set_real_ip_from (ngx_http_realip_module) should be a list #415

glbyers opened this issue Mar 19, 2024 · 1 comment

Comments

@glbyers
Copy link

glbyers commented Mar 19, 2024

Describe the bug

In the http/modules.j2 template, the realip macro assumes that set_real_ip_from can only ever be a single value. In reality, set_real_ip_from can (and likely often is) be defined multiple times.

To reproduce

Steps to reproduce the behavior:

  1. Define a nginx_config_http_template_enable some like;
nginx_config_http_template_enable:
  - template_file: http/default.conf.j2
    deployment_location: /etc/nginx/conf.d/default.conf
    backup: false
    config:
      realip:
        ## BUG: set_real_ip_from in http/modules.j2 only allows a single value.
        set_real_ip_from:
          - 127.127.127.127/32
          - 10.10.10.10/32
          - 192.192.192.192/32
        real_ip_header: X-Forwarded-For
  1. Deploy the Ansible NGINX configuration role using playbook.yml
  2. Inspect set_real_ip_from value in /etc/nginx/conf.d/default.conf

Expected behavior

set_real_ip_from should allow multiple values like many other config items do.

Your environment

  • nginxinc.nginx_core 0.8.0
  • ansible-core 2.14
  • Jinja 3.1.3
  • Any deployment platform

Additional context

The macro realip in http/modules.j2 is defined as follows;

{# NGINX HTTP RealIP -- ngx_http_realip_module #}
{% macro realip(realip) %}
{% if realip['set_real_ip_from'] is defined %}
set_real_ip_from {{ realip['set_real_ip_from'] }};
{% endif %}
{% if realip['real_ip_header'] is defined %}
real_ip_header {{ realip['real_ip_header'] }};
{% endif %}
{% if realip['real_ip_recursive'] is defined and realip['real_ip_recursive'] is boolean %}
real_ip_recursive {{ realip['real_ip_recursive'] | ternary('on', 'off') }};
{% endif %}

I think this should be;

{# NGINX HTTP RealIP -- ngx_http_realip_module #}
{% macro realip(realip) %}
{% if realip['set_real_ip_from'] is defined %}
{% for set_real_ip_from in realip['set_real_ip_from'] if realip['set_real_ip_from'] is not string %}
set_real_ip_from {{ set_real_ip_from }};
{% else %}
set_real_ip_from {{ realip['set_real_ip_from'] }};
{% endfor %}
{% endif %}
{% if realip['real_ip_header'] is defined %}
real_ip_header {{ realip['real_ip_header'] }};
{% endif %}
{% if realip['real_ip_recursive'] is defined and realip['real_ip_recursive'] is boolean %}
real_ip_recursive {{ realip['real_ip_recursive'] | ternary('on', 'off') }};
{% endif %}
@alessfg
Copy link
Collaborator

alessfg commented Mar 20, 2024

You are absolutely right! Could you please open a PR?

glbyers pushed a commit to glbyers/ansible-role-nginx-config that referenced this issue Mar 21, 2024
glbyers pushed a commit to glbyers/ansible-role-nginx-config that referenced this issue Mar 21, 2024
The set_real_ip_from paremeter from the ngx_http_realip_module may be specified
multiple times within the http, server & location contexts. This commit adds
list support whilst preserving original behaviour (single-valued string).
Resolves issue nginxinc#415.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants