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

PROXY Protocol support #1882

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Conversation

SBado
Copy link

@SBado SBado commented Feb 23, 2022

This PR add basic support for PROXY protocol to NPM. I needed NPM to support PROXY protocol becasue I'm using two instances of HAProxy as a point of access to my services, as described here. It's been working fine for me so far, so I thought to share.

Note: to allow coexistence of "regular" and "PROXY protocol enabled" hosts, the latter ones will listen on port 88 and 444.

Related issue: #1114.

immagine

@jc21
Copy link
Member

jc21 commented Feb 23, 2022

This is an automated message from CI:

Docker Image for build 1 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1882

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

@ylx2016
Copy link

ylx2016 commented Mar 7, 2022

Good News.
I can't wait it.

@jc21
Copy link
Member

jc21 commented Mar 14, 2022

Thanks for the work. I might hit you up about supporting this better in v3. I'm not familiar with this directive entirely.

Why does this need a different port? Reading some documentation it doesn't suggest that I can't mix "normal" hosts with proxy_protocol ones.

Also would you expect this to apply to Streams too?

For reference I was reading this

@SBado
Copy link
Author

SBado commented Mar 22, 2022

First question:

Why does this need a different port? Reading some documentation it doesn't suggest that I can't mix "normal" hosts with proxy_protocol ones.

Empirical evidence :)
Also, this:

The receiver MUST be configured to only receive the protocol described in this
specification and MUST not try to guess whether the protocol header is present
or not. This means that the protocol explicitly prevents port sharing between
public and private access. Otherwise it would open a major security breach by
allowing untrusted parties to spoof their connection addresses. The receiver
SHOULD ensure proper access filtering so that only trusted proxies are allowed
to use this protocol.

Second question:

Also would you expect this to apply to Streams too?

Yes:

Syntax:		proxy_protocol on | off;
Default:	proxy_protocol off;
Context:	stream, server

This PR does not cover streams because I don't use them, but I can try to add support for them if necessary.

@paaland
Copy link

paaland commented Apr 22, 2022

Can't wait. I'm tempted to fork and build my own image just to get this PR in.

@openncomp
Copy link

I'm looking forward to adding this option!

@christiaangoossens
Copy link

I would like to see an addition where you can select if you want the normal ports enabled or only the proxy protocol ports, or both sets.

If you enable both, you could use the normal ports within the local network for instance, with direct access, where you use the proxy protocol ports for outside access through a proxy. Otherwise, all local access would need to go through that outside proxy too.

@christiaangoossens
Copy link

@jc21 Any status on if this will be merged or if changes are required?

@christiaangoossens
Copy link

For those trying out this image instead of the default one, a few notes:

  • You cannot use redirection hosts, 404 hosts because they have no PROXY protocol setting yet
  • The default site is broken as it's not using proxy protocol, so some random proxy protocol site will be set as default instead. Alternative to this is using a proxy host with a wildcard domain name instead.

@baudneo
Copy link

baudneo commented Oct 10, 2022

I added this to my modsec npm image and also added PROXY protocol in stream hosts.

I am adding it to 404 and redirection hosts as well.

#1867

baudneo/nginx-proxy-manager:latest

@MatthiasLohr
Copy link

Any update on this?

@baudneo
Copy link

baudneo commented Nov 21, 2022

Any update on this?

I added PROXY protocol for proxy hosts and stream hosts in my image -> baudneo/Nginx-Proxy-Manager:bullseye

Also includes crowdsec openresty bouncer and modsec.

@nginxproxymanagerci
Copy link

Docker Image for build 7 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1882

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

@Kohbrax
Copy link

Kohbrax commented Mar 27, 2023

@jc21 any chances, to merge this? I would really like to use the proxy protocol.

@philipreichert
Copy link

I would like to use proxy protocol in productive as well :)

@christiaangoossens
Copy link

@SBado Could you update it based on the upstream changes?
Currently the latest Docker image (jc21/nginx-proxy-manager:github-pr-1882) errors out with:

[7/24/2023] [2:32:45 PM] [Global   ] › ✖  error     Command failed: . /opt/certbot/bin/activate && pip install certbot-dns-cloudflare==$(certbot --version | grep -Eo '[0-9](\.[0-9]+)+') cloudflare && deactivate
Traceback (most recent call last):
  File "/opt/certbot/bin/certbot", line 5, in <module>
    from certbot.main import main
  File "/opt/certbot/lib/python3.7/site-packages/certbot/main.py", line 6, in <module>
    from certbot._internal import main as internal_main
  File "/opt/certbot/lib/python3.7/site-packages/certbot/_internal/main.py", line 21, in <module>
    import josepy as jose
  File "/opt/certbot/lib/python3.7/site-packages/josepy/__init__.py", line 40, in <module>
    from josepy.json_util import (
  File "/opt/certbot/lib/python3.7/site-packages/josepy/json_util.py", line 14, in <module>
    from OpenSSL import crypto
  File "/opt/certbot/lib/python3.7/site-packages/OpenSSL/__init__.py", line 8, in <module>
    from OpenSSL import crypto, SSL
  File "/opt/certbot/lib/python3.7/site-packages/OpenSSL/crypto.py", line 1517, in <module>
    class X509StoreFlags(object):
  File "/opt/certbot/lib/python3.7/site-packages/OpenSSL/crypto.py", line 1537, in X509StoreFlags
    CB_ISSUER_CHECK = _lib.X509_V_FLAG_CB_ISSUER_CHECK
AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK'
ERROR: Could not find a version that satisfies the requirement certbot-dns-cloudflare== (from versions: 0.14.0.dev0, 0.15.0, 0.16.0, 0.17.0, 0.18.0, 0.18.1, 0.18.2, 0.19.0, 0.20.0, 0.21.0, 0.21.1, 0.22.0, 0.22.1, 0.22.2, 0.23.0, 0.24.0, 0.25.0, 0.25.1, 0.26.0, 0.26.1, 0.27.0, 0.27.1, 0.28.0, 0.29.0, 0.29.1, 0.30.0, 0.30.1, 0.30.2, 0.31.0, 0.32.0, 0.33.0, 0.33.1, 0.34.0, 0.34.1, 0.34.2, 0.35.0, 0.35.1, 0.36.0, 0.37.0, 0.37.1, 0.37.2, 0.38.0, 0.39.0, 0.40.0, 0.40.1, 1.0.0, 1.1.0, 1.2.0, 1.3.0, 1.4.0, 1.5.0, 1.6.0, 1.7.0, 1.8.0, 1.9.0, 1.10.0, 1.10.1, 1.11.0, 1.12.0, 1.13.0, 1.14.0, 1.15.0, 1.16.0, 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0, 1.22.0, 1.23.0, 1.24.0, 1.25.0, 1.26.0, 1.27.0, 1.28.0, 1.29.0, 1.30.0, 1.31.0, 1.32.0, 2.0.0, 2.1.0, 2.2.0, 2.3.0, 2.4.0, 2.5.0, 2.6.0)
ERROR: No matching distribution found for certbot-dns-cloudflare==
[notice] A new release of pip is available: 23.0.1 -> 23.2.1
[notice] To update, run: pip install --upgrade pip

@Luxbit
Copy link

Luxbit commented Aug 8, 2023

Any timeline on when this could be merged?
Having proxy_protocol control would be very useful

@davideciarmiello
Copy link

Hi. why not include this feature on main package?

@jwklijnsma
Copy link

@jc21 are @SBado is this optie is go make to final build ?

@Morriz
Copy link

Morriz commented Feb 2, 2024

bumping @SBado. Is it much work to finish?

@jwklijnsma
Copy link

update it in new pull request #3537

Copy link

PR is now considered stale. If you want to keep it open, please comment 👍

@github-actions github-actions bot added the stale label Aug 12, 2024
@Morriz
Copy link

Morriz commented Sep 24, 2024

Bump

@github-actions github-actions bot removed the stale label Oct 4, 2024
@xenadmin
Copy link

So do I understand correctly, that Nginx Proxy Manger doesn't support PROXY Protocol until today?

@dlc-letelier
Copy link

Please, UP!

@lastsamurai26
Copy link

lastsamurai26 commented Oct 18, 2024

Would be really nice. Because without i can't use stalwart and i need to migrate to traefik

And what about version 3 can't find any thread :(

snordmann pushed a commit to snordmann/nginx-proxy-manager that referenced this pull request Oct 23, 2024
snordmann added a commit to snordmann/nginx-proxy-manager that referenced this pull request Oct 23, 2024
snordmann added a commit to snordmann/nginx-proxy-manager that referenced this pull request Oct 23, 2024
snordmann added a commit to snordmann/nginx-proxy-manager that referenced this pull request Oct 23, 2024
snordmann added a commit to snordmann/nginx-proxy-manager that referenced this pull request Oct 26, 2024
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

Successfully merging this pull request may close these issues.