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

python-certbot-nginx: certbot does not add "proxy_protocol" in "listen" line #8057

Open
simos opened this issue Jun 5, 2020 · 9 comments
Open
Labels
area: nginx feature request needs-update priority: significant Issues with higher than average priority that do not need to be in the current milestone.

Comments

@simos
Copy link

simos commented Jun 5, 2020

My operating system is (include version):

Ubuntu 20.04 LTS

I installed Certbot with (certbot-auto, OS package manager, pip, etc):

"certbot 0.40.0-1", as provided from the focal/universe repository.

How to replicate

You are using nginx as a reverse proxy.
You have configured a few http websites.
You are using the proxy_protocol directive in the listen 80 proxy_protocol line.

You now run sudo certbot --nginx to convert them to http/https.
certbot does not parse the listen 80 ... line to detect any directives like proxy_protocol in this case. Therefore, the new configuration line for listen 443 ... does not have the proxy_protocol directive, and it has to be added manually.

Configuration before running certbot

server {
        listen 80 proxy_protocol;
        listen [::]:80 proxy_protocol;

        server_name nginx1.example.com;

        location / {
                include /etc/nginx/proxy_params;

                proxy_pass http://nginx1.lxd;
        }

        real_ip_header proxy_protocol;
        set_real_ip_from 127.0.0.1;
}

Configuration after running certbot

Note the missing proxy_protocol in the two listen lines.
certbot should copy any directives from the listen 80 lines to the listen 443 lines.

server {

        server_name nginx1.example.com;

        location / {
                include /etc/nginx/proxy_params;

                proxy_pass http://nginx1.lxd;
        }

        real_ip_header proxy_protocol;
        set_real_ip_from 127.0.0.1;

    listen [::]:443 ssl; # managed by Certbot
    listen 443 ssl; # managed by Certbot
    ssl_certificate /etc/letsencrypt/live/nginx1.example.com/fullchain.pem; # managed by Certbot
    ssl_certificate_key /etc/letsencrypt/live/nginx1.example.com/privkey.pem; # managed by Certbot
    include /etc/letsencrypt/options-ssl-nginx.conf; # managed by Certbot
    ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; # managed by Certbot

}
server {
    if ($host = nginx1.example.com) {
        return 301 https://$host$request_uri;
    } # managed by Certbot


        listen 80 proxy_protocol;
        listen [::]:80 proxy_protocol;

        server_name nginx1.example.com;
    return 404; # managed by Certbot


}

Corrected configuration file

server {

        server_name nginx1.example.com;

        location / {
                include /etc/nginx/proxy_params;

                proxy_pass http://nginx1.lxd;
        }

        real_ip_header proxy_protocol;
        set_real_ip_from 127.0.0.1;

    listen [::]:443 proxy_protocol ssl; # managed by Certbot
    listen 443 proxy_protocol  ssl; # managed by Certbot
    ssl_certificate /etc/letsencrypt/live/nginx1.example.com/fullchain.pem; # managed by Certbot
    ssl_certificate_key /etc/letsencrypt/live/nginx1.example.com/privkey.pem; # managed by Certbot
    include /etc/letsencrypt/options-ssl-nginx.conf; # managed by Certbot
    ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; # managed by Certbot

}
server {
    if ($host = nginx1.example.com) {
        return 301 https://$host$request_uri;
    } # managed by Certbot


        listen 80 proxy_protocol;
        listen [::]:80 proxy_protocol;

        server_name nginx1.example.com;
    return 404; # managed by Certbot


}
@alexzorin
Copy link
Collaborator

alexzorin commented Jun 6, 2020

It looks like ipv6only is currently the only listen parameter which (potentially) survives in _make_server_ssl. We could copy over other non-blacklisted (#6118) parameters.

@alexzorin alexzorin added area: nginx feature request priority: significant Issues with higher than average priority that do not need to be in the current milestone. labels Jun 6, 2020
@osirisinferi
Copy link
Collaborator

I'm wondering if issue #6118 was actually an issue? If I understand it correctly, the virtualhost is only copied to make a HTTPS server block, correct? So that would mean a *different "address:port pair" for the new server block. And those "blacklisted" listen options were blacklisted only for identical address:port pairs, right? So wasn't necessary to blacklist it in the first place.. Or am I missing something?

@alexzorin
Copy link
Collaborator

alexzorin commented Sep 6, 2020

That makes sense to me, but (speculation alert) I believe the scenario in #6118 is slightly more complicated.

If you don't have an existing virtualhost for example.com, and run:

certbot --nginx -d example.com

Certbot then finds the existing default port 80 virtualhost and duplicates it (including the listen line), changing its server_name.

If there are any unique parameters in that listen and they are copied over, it will result in an nginx configuration error.

@audouts
Copy link

audouts commented Mar 2, 2021

This is still an issue in 1.11. I don't see it on the list of fixes for 1.12.

My configuration starts with two entries:

server {
   server_name nginx.proxy.example

   location / {
      proxy_pass http://localhost:4430;
      proxy_redirect off;
      proxy_set_header Host $host;
      proxy_set_header X-Real-IP $remote_addr;
      proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
      proxy_connect_timeout 90;
      ...
   }

    # Certbot inserts lines here - listen 443 ssl, etc.
}

server {
   # Certbot doesn't touch this section

   server_name nginx.proxy.example
   listen 80 proxy_protocol;

   location / {
      root /var/www/nginx;

      proxy_redirect off;
      proxy_set_header Host $host;
      proxy_set_header X-Real-IP $remote_addr;
      proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
   }
}

# Certbot adds a new server section here with listen 80, etc.

Certbot adds the SSL entries to the first section (which I want) but doesn't add proxy_protocol. It then creates a new, third section with port 80 listening but that's ignored because of the existing section with port 80 (which is untouched).

I'm open to suggestions to improve the configuration but it seems like Certbot needs to check for (and update) proxy_protocol correctly.

@stale
Copy link

stale bot commented Apr 18, 2022

We've made a lot of changes to Certbot since this issue was opened. If you still have this issue with an up-to-date version of Certbot, can you please add a comment letting us know? This helps us to better see what issues are still affecting our users. If there is no activity in the next 30 days, this issue will be automatically closed.

@dionysius
Copy link

dionysius commented May 13, 2022

How about certbot just keeps/remembers any suffixed keywords after listen? They were there before for a reason ignore me, would fail #6118

@github-actions
Copy link

We've made a lot of changes to Certbot since this issue was opened. If you still have this issue with an up-to-date version of Certbot, can you please add a comment letting us know? This helps us to better see what issues are still affecting our users. If there is no activity in the next 30 days, this issue will be automatically closed.

@simos
Copy link
Author

simos commented May 14, 2023

I just (today) verified that the issue is not resolved. The test case still applies.

I used Ubuntu 22.04 LTS and the snap package of certbot (version 2.6.0).

In short, if the original nginx configuration file has the following configuration

server {
        listen 80 proxy_protocol;
        listen [::]:80 proxy_protocol;

        server_name example.com www.example.com;
...

then certbot does not include the proxy_protocol directive in the generated listen 443... lines.

Certbot produces

    listen [::]:443 ssl; # managed by Certbot
    listen 443 ssl; # managed by Certbot

while the correct should be

    listen [::]:443 ssl proxy_protocol; # managed by Certbot
    listen 443 ssl proxy_protocol; # managed by Certbot

Copy link

We've made a lot of changes to Certbot since this issue was opened. If you still have this issue with an up-to-date version of Certbot, can you please add a comment letting us know? This helps us to better see what issues are still affecting our users. If there is no activity in the next 30 days, this issue will be automatically closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nginx feature request needs-update priority: significant Issues with higher than average priority that do not need to be in the current milestone.
Projects
None yet
Development

No branches or pull requests

5 participants