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

www/nginx: add proxy protocol support and improve whitelist handling of WAF #1051

Merged
merged 4 commits into from
Dec 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions www/nginx/pkg-descr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ Plugin Changelog
* Add proxy options for ignore client abort and disabling buffering
* Add logviewer support for streams
* Fix charset is not defined bug (contributed by ccesario [1])
* Add an existence check for locations
* Add PROXY protocol for HTTP and Streams frontend
* Add PROXY backend support for Streams
* Add support for better whitelist rules in the WAF

[1] https://github.com/opnsense/plugins/pull/1035

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,30 @@
<label>HTTPS Listen Port</label>
<type>text</type>
</field>
<field>
<id>httpserver.proxy_protocol</id>
<label>PROXY Protocol</label>
<type>checkbox</type>
<advanced>true</advanced>
<help>If you enable the proxy protocol, a downstream proxy can send the client IP and port before the real traffic is set.</help>
</field>
<field>
<id>httpserver.trusted_proxies</id>
<label>Trusted Proxies</label>
<allownew>true</allownew>
<style>tokenize</style>
<type>select_multiple</type>
<advanced>true</advanced>
<help>Enter a list of IP addresses or CIDR networks which are allowed to override the source IP address using the specified header.</help>
</field>
<field>
<id>httpserver.real_ip_source</id>
<label>Real IP Source</label>
<style>selectpicker</style>
<advanced>true</advanced>
<help>X-Real-IP and X-Forwarded-For are HTTP headers, while PROXY protocol is a protocol which needs to be enabled.</help>
<type>dropdown</type>
</field>
<field>
<id>httpserver.servername</id>
<label>Server Name</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,21 @@
<label>UDP Port</label>
<type>checkbox</type>
</field>
<field>
<id>streamserver.proxy_protocol</id>
<label>PROXY Protocol</label>
<type>checkbox</type>
<help>If you enable the proxy protocol, a downstream proxy can send the client IP and port before the real traffic is set.</help>
</field>
<field>
<id>httpserver.trusted_proxies</id>
<label>Trusted Proxies</label>
<allownew>true</allownew>
<style>tokenize</style>
<type>select_multiple</type>
<advanced>true</advanced>
<help>Enter a list of IP addresses or CIDR networks which are allowed to override the source IP address using the specified header.</help>
</field>
<field>
<id>streamserver.certificate</id>
<label>TLS Certificate</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@
<style>selectpicker</style>
<type>select_multiple</type>
</field>
<field>
<id>upstream.proxy_protocol</id>
<label>PROXY Protocol</label>
<type>checkbox</type>
<advanced>true</advanced>
<help>If you enable the proxy protocol, an upstream proxy or server will get the client IP and the server port before the real traffic is sent.</help>
</field>
<field>
<id>upstream.tls_enable</id>
<label>Enable TLS (HTTPS)</label>
Expand Down
66 changes: 63 additions & 3 deletions www/nginx/src/opnsense/mvc/app/models/OPNsense/Nginx/Nginx.xml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@
<Required>Y</Required>
<multiple>Y</multiple>
</serverentries>
<proxy_protocol type="BooleanField">
<default>0</default>
<Required>Y</Required>
</proxy_protocol>
<store type="BooleanField">
<Required>Y</Required>
<default>0</default>
Expand Down Expand Up @@ -439,8 +443,16 @@
<Required>Y</Required>
</ruletype>
<message type="TextField">
<Required>Y</Required>
<Required>N</Required>
<pattern>/^[^"]+$/</pattern>
<Constraints>
<check001>
<ValidationMessage>This field must be set.</ValidationMessage>
<type>SetIfConstraint</type>
<field>match_type</field>
<check>id</check>
</check001>
</Constraints>
</message>
<identifier type="IntegerField">
<Required>Y</Required>
Expand All @@ -455,8 +467,16 @@
<pattern>/^[^"]+$/</pattern>
</dollar_url>
<match_value type="TextField">
<Required>Y</Required>
<Required>N</Required>
<pattern>/^[^"]+$/</pattern>
<Constraints>
<check001>
<ValidationMessage>This field must be set.</ValidationMessage>
<type>SetIfConstraint</type>
<field>match_type</field>
<check>id</check>
</check001>
</Constraints>
</match_value>
<match_type type="OptionField">
<Required>Y</Required>
Expand All @@ -470,8 +490,16 @@
<Required>Y</Required>
</negate>
<score type="IntegerField">
<Required>Y</Required>
<Required>N</Required>
<default>8</default>
<Constraints>
<check001>
<ValidationMessage>This field must be set.</ValidationMessage>
<type>SetIfConstraint</type>
<field>match_type</field>
<check>id</check>
</check001>
</Constraints>
</score>
<regex type="BooleanField">
<Required>Y</Required>
Expand Down Expand Up @@ -514,6 +542,23 @@
<Required>N</Required>
<default>443</default>
</listen_https_port>
<proxy_protocol type="BooleanField">
<default>0</default>
<Required>Y</Required>
</proxy_protocol>
<trusted_proxies type="CSVListField">
<Required>N</Required>
<mask>/^((?:\d+\.){3,3}\d+|[a-f0-9\:]+)(?:\/\d+)?(,?(?:(?:(\d+\.){3,3}\d+|[a-f0-9\:]+)(?:\/\d+)?))*$/i</mask>
<multiple>Y</multiple>
</trusted_proxies>
<real_ip_source type="OptionField">
<OptionValues>
<X-Real-IP>X-Real-IP (default)</X-Real-IP>
Copy link
Member

@mimugmail mimugmail Dec 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be a <default>?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment is incomplete…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed the comment ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the default is configuring nothing. If nothing is configured, nginx will fall back to its default, which currently is X-Real-IP and may change in the future. If you choose the option it will be set explicitly in the configuration and will probably be future safe against changes (regarding unexpected changes).

<X-Forwarded-For>X-Forwarded-For</X-Forwarded-For>
<proxy_protocol>PROXY Protocol</proxy_protocol>
</OptionValues>
<Required>N</Required>
</real_ip_source>
<locations type="ModelRelationField">
<Model>
<template>
Expand Down Expand Up @@ -658,11 +703,26 @@
<listen_port type="PortField">
<Required>N</Required>
<default>80</default>
<Constraints>
<check001>
<ValidationMessage>You can only use one server at this port.</ValidationMessage>
<type>UniqueConstraint</type>
</check001>
</Constraints>
</listen_port>
<udp type="BooleanField">
<Required>Y</Required>
<default>0</default>
</udp>
<trusted_proxies type="CSVListField">
<Required>N</Required>
<mask>/^((?:\d+\.){3,3}\d+|[a-f0-9\:]+)(?:\/\d+)?(,?(?:(?:(\d+\.){3,3}\d+|[a-f0-9\:]+)(?:\/\d+)?))*$/i</mask>
<multiple>Y</multiple>
</trusted_proxies>
<proxy_protocol type="BooleanField">
<default>0</default>
<Required>Y</Required>
</proxy_protocol>
<certificate type="CertificateField">
<Type>cert</Type>
<Required>N</Required>
Expand Down
14 changes: 12 additions & 2 deletions www/nginx/src/opnsense/service/templates/OPNsense/Nginx/http.conf
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ server {
{% set our_headers = [] %}
{% do our_headers.append('X-Powered-By') %}
{% if server.listen_http_port is defined %}
listen {{ server.listen_http_port }};
listen [::]:{{ server.listen_http_port }};
listen {{ server.listen_http_port }}{% if server.proxy_protocol is defined and server.proxy_protocol == '1' %} proxy_protocol{% endif %};
listen [::]:{{ server.listen_http_port }}{% if server.proxy_protocol is defined and server.proxy_protocol == '1' %} proxy_protocol{% endif %};
{% do listen_list.append(server.listen_http_port) %}
{% endif %}
{% if server.listen_https_port is defined and server.certificate is defined %}
Expand All @@ -86,6 +86,14 @@ server {
sendfile {% if server.sendfile is defined and server.sendfile == '1' %}On{% else %}Off{% endif %};
{% endif %}
server_name {{ server.servername.replace(',', ' ') }};
{% if server.real_ip_source is defined and server.real_ip_source != '' %}
real_ip_header {{ server.real_ip_source }};
{% if server.trusted_proxies is defined and server.trusted_proxies != '' %}
{% for trusted_proxy in server.trusted_proxies.split(',') %}
set_real_ip_from {{ trusted_proxy }};
{% endfor %}
{% endif %}
{% endif %}
{% if server.charset is defined %}
charset {{ server.charset }};
{% endif %}
Expand Down Expand Up @@ -220,7 +228,9 @@ server {
{% if server.locations is defined %}
{% for location_uuid in server.locations.split(',') %}
{% set location = helpers.getUUID(location_uuid) %}
{% if location.urlpattern is defined %}
{% include "OPNsense/Nginx/location.conf" ignore missing with context %}
{% endif %}
{% endfor %}
{% endif %}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,14 @@
{{ mz_matches|join('|') }}
{%- endmacro %}
{% macro naxsi_rule(uuid, rule, ruletype) -%}
{% if rule.message is defined and rule.match_value is defined %}
{{ ruletype }}{% if rule.negate is defined and rule.negate == '1' %} negative{% endif
%} {{ rule.match_type }}:{{ rule.identifier }} "{% if rule.regex == '1' %}rx{% else %}str{% endif
%}:{{ rule.match_value }}" "msg:{{ rule.message }}" "mz:{{ naxsi_mzhelper(rule)
}}" "s:$policy{{ uuid.replace('-', '') }}:{{ rule.score }}";
{% else %}
{{ ruletype }} {{ rule.match_type }}:{{ rule.identifier }};
{% endif %}
{%- endmacro %}

{% if naxsi_ruletype == 'basic' %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
server {
{% set tls_enabled = server.certificate is defined %}
{% if server.listen_port is defined %}
listen {{ server.listen_port }}{% if server.udp is defined and server.udp == '1' %} udp{% endif %}{% if tls_enabled %} ssl{% endif %};
listen [::]:{{ server.listen_port }}{% if server.udp is defined and server.udp == '1' %} udp{% endif %}{% if tls_enabled %} ssl{% endif %};
listen {{ server.listen_port }}{% if server.udp is defined and server.udp == '1' %} udp{% endif %}{% if tls_enabled %} ssl{% endif %}{% if server.proxy_protocol is defined and server.proxy_protocol == '1' %} proxy_protocol{% endif %};
listen [::]:{{ server.listen_port }}{% if server.udp is defined and server.udp == '1' %} udp{% endif %}{% if tls_enabled %} ssl{% endif %}{% if server.proxy_protocol is defined and server.proxy_protocol == '1' %} proxy_protocol{% endif %};
{% endif %}

access_log /var/log/nginx/stream_{{ server['@uuid'] }}.access.log main;
Expand Down Expand Up @@ -84,6 +84,12 @@
{% elif server.route_field == 'sni_upstream_map' %}
proxy_pass $hostmap{{ server.sni_upstream_map.replace('-','') }};
{% endif %}
proxy_protocol {% if upstream.proxy_protocol == '1' %}on{% else %}off{% endif %};
{% if server.trusted_proxies is defined and server.trusted_proxies != '' %}
{% for trusted_proxy in server.trusted_proxies.split(',') %}
set_real_ip_from {{ trusted_proxy }};
{% endfor %}
{% endif%}

}
{% endfor %}