Skip to content

Conversation

Tagadda
Copy link
Contributor

@Tagadda Tagadda commented May 5, 2025

This is yolocommit :) Not really tested yet
This PR should allow YunoHost to accept proxy protocol requests on port 444 from allowed IPs, while accepting regular HTTPS requests on port 443.
FIXME:

  • Locales
  • Check nftables syntax

@Tagadda Tagadda force-pushed the nginx-behind-proxy branch 4 times, most recently from 1862e13 to 06d1bcc Compare May 6, 2025 16:57
@Tagadda Tagadda force-pushed the nginx-behind-proxy branch from 06d1bcc to f1383c6 Compare August 30, 2025 11:36
@Tagadda Tagadda marked this pull request as ready for review August 30, 2025 13:44
"global_settings_setting_postfix_compatibility_help": "Compatibility vs. security tradeoff for the Postfix server. Affects the ciphers (and other security-related aspects)",
"global_settings_setting_postfix_name": "Postfix (SMTP email server)",
"global_settings_setting_trusted_proxies_list": "Trusted proxies list",
"global_settings_setting_trusted_proxies_list_help": "",
Copy link
Member

Choose a reason for hiding this comment

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

Ideally it's better to add "trusted_proxies_list" to the "settings_without_help_key" here : https://github.com/YunoHost/yunohost/blob/dev/maintenance/missing_i18n_keys.py#L217 (it's a bit hackish but that's what we have for now)

On the other hand, maybe we do need an explication here ? If I understand correctly, this is the list of the "front" servers when being the back / reverse-proxied server ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, the IP the proxy server is using to communicate with your YunoHost server. That IP would be different from the public IP the proxy is using to be reached.
IP addresses allowed to act as a proxy for your server.

"global_settings_setting_tls_passthrough_enabled": "Enable TLS-passthrough / SNI-based forwarding",
"global_settings_setting_tls_passthrough_enabled_help": "This is an advanced feature to reverse-proxy an entire domain to another machine *without* decrypting the traffic. Useful when you want to expose several machines behind the same IP but still allow each machine to handle the SSL termination.",
"global_settings_setting_tls_passthrough_explain": "This feature is ADVANCED and EXPERIMENTAL and will trigger major changes in the nginx configuration of this server. Please DO NOT use it if you don't know what you are doing! In particular, you must be aware that fail2ban cannot be implemented on the proxied server (nftables cannot ban malicious traffic as all IP packets appear as coming from the front server). In addition, for now the proxied server's nginx configuration needs to manually tweaked to accept the `proxy_protocol`.",
"global_settings_setting_tls_passthrough_explain": "This feature is ADVANCED and EXPERIMENTAL. Please DO NOT use it if you don't know what you are doing! In particular, you must be aware that fail2ban cannot be implemented on the proxied server (nftables cannot ban malicious traffic as all IP packets appear as coming from the front server). In addition, for now the proxied server's nginx configuration needs to manually tweaked to accept the `proxy_protocol`.",
Copy link
Member

Choose a reason for hiding this comment

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

It sounds important to keep an explanation somewhere of what this feature is about ?

Suggested change
"global_settings_setting_tls_passthrough_explain": "This feature is ADVANCED and EXPERIMENTAL. Please DO NOT use it if you don't know what you are doing! In particular, you must be aware that fail2ban cannot be implemented on the proxied server (nftables cannot ban malicious traffic as all IP packets appear as coming from the front server). In addition, for now the proxied server's nginx configuration needs to manually tweaked to accept the `proxy_protocol`.",
"global_settings_setting_tls_passthrough_explain": "This is an advanced feature to reverse-proxy an entire domain to another machine *without* decrypting the traffic. Useful when you want to expose several machines behind the same IP but still allow each machine to handle the SSL termination. This feature is ADVANCED and EXPERIMENTAL. Please DO NOT use it if you don't know what you are doing! In particular, you must be aware that fail2ban cannot be implemented on the proxied server (nftables cannot ban malicious traffic as all IP packets appear as coming from the front server). In addition, for now the proxied server's nginx configuration needs to manually tweaked to accept the `proxy_protocol`.",

Also maybe we want to update the last sentence to refer to the "trusted proxies list" ?

listen [::]:443 ssl http2;
{%- else %}
listen 127.0.0.1:444 ssl proxy_protocol;
listen 444 ssl http2 proxy_protocol;
Copy link
Member

@zamentur zamentur Sep 3, 2025

Choose a reason for hiding this comment

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

SO if i understand well, server.tpl.conf use 444 by default the proxy_protocol and http2 are enabled on it.

Warning on address IP spoofing

Spoofing address ip is prevent because only some ips are allowed to by nftable. However if nftable is down or accept too much traffic, an attacker could spoof the IP. In more, i don't know if it's secure to use proxy_protocol outside of a local network...

Note: that 127.0.0.1 is always allowed so someone with a non root ssh account might use this account to spoof an ip by doing a request with proxy protocol. (i don't know if curl is able to use proxy_protocol).

Regenerate nginx conf and old customized change

I think by default the regen conf don't remove old customize conf. So we could be in a situation where tls_passthrough.conf is added but the old server.tpl.conf is maintained with 443 port

else
touch "${tls_passthrough_module}"
fi
ynh_render_template "tls_passthrough.conf" "${tls_passthrough_module}"
Copy link
Member

Choose a reason for hiding this comment

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

Have we an idea on performance impact of making this by default ?

[misc.trusted_proxies]
[misc.trusted_proxies.trusted_proxies_list]
type = "tags"
default = ""
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should add a pattern to check element are in IP format ?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe local ip only...

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants