Update force-ssl.conf to allow for letsencrypt directories over http

[the1ts]

Since we have moved force-ssl.conf into the server section, it overrides the location based letsencrypt allowed over http

• Make force-ssl only work if both http traffic and outside the letsencrypt directories.
• Fixes #1625

[jc21]

This is an automated message from CI:

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

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

[the1ts]

Not the best candidate to test this since I'm using a DNS based wildcard so letsencrypt not hitting Nginx. However, I'm not seeing issues for http->https on my domains. And curl to the /.well-known/acme-challenge/1234 gives the expected 404 rather than 301 redirect and I have made no changes so have both old and new style config with force-ssl.conf in both server section and location / section.

[the1ts]

Just a bump, we are blocking renews.

[arsors]

Another bump. I have the same problem and have to manually unforce ssl and then renew the ssl certificate.

[the1ts]

@jc21 I appreciate you're looking hard at 3.0, but please take a look at this pr, this is blocking renewals of newly created proxies (from those made after the force_ssl.conf move to server section) which isn't a good look for us to have in the version 2 branch.

[tfmm]

Bumping this, renewals are still broken as far as I can tell.

[maltokyo]

Bump, why is this not priority, I can only assume that nobody maintains this anymore

[the1ts]

@maltokyo all I would ask is if you can move to using the docker image mentioned in comment 2 here. That has the fix and it would be good to see some confirmation here. You must remember to move back to :latest when this project starts moving again so you don't get stuck on an old version.

[maltokyo]

Thank you @the1ts - I am on the latest version, so hesitate to move back to a version from May. Can I apply the fix myself, build and test this way?

[the1ts]

@maltokyo latest is from March, so older than the fixed one in this PR.

[maltokyo]

Ah apologies! Will try this, thank you @the1ts

[maltokyo]

@the1ts do I need to worry about the warning: "Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes"

I dont see db changes in this PR itself, but just wanted to check. Backing up will take some time for me.

[the1ts]

@maltokyo the only changes are to nginx file creation not DB. So change the docker tag and try and get npm to make a change to the nginx files by perhaps turning off and on the force SSL. Then renewal should be back working next time it's needed.

[maltokyo]

ok, running it now, will leave for some time and see what happens. Unfortunately, next renewals are on 1st Jan, so might need to wait.

[EDIflyer]

@the1ts @jc21 OK I tried the version in jc21/nginx-proxy-manager:github-pr-2038 and confirm it correctly allows manual renewal via the UI (/nginx/certificates page) for proxies that have 'Force SSL' enabled.

(I did have an issue at first where I was getting an internal error but clearing the certbot lock files as per this post did the trick!)

[EDIflyer]

Great to see a new release out but would be great if this PR could be merged in so I could pick up those updates too :)

[Schlumpf9]

It's really a pain right now to manually renew all certificates that have "force ssl" enabled. That's a key function of nginx proxy manager and I am considering to unuse it just because of that bug... I have 15 domains and to update all manually is a real pain! That bug is existing since a year @jc21

[fdzaebel]

As a fix seems to be available now...when will this be added to the official release?

Thank you very much. :)

[maltokyo]

ok, running it now, will leave for some time and see what happens. Unfortunately, next renewals are on 1st Jan, so might need to wait.

@the1ts - I have been running the version specified as above, and my certificates expire on Jan 1st 2023, and have NOT been auto-updated while running this version. They should have been by now, correct? If that is the case, I suppose this fix is not working out.

[the1ts]

Sorry @maltokyo I don't use NPM anymore as its not really been supported for most of a year, but I would check out issue #918 others have seen issues with lock files being left behind that stop certbot from actually running even after this fix for force ssl. You can force an update in the UI and look at both certbot logs and NPM for more info.

[Whoopsadaisy]

IMHO the regexp in the commit needs additional escaping:

- if ($request_uri !~ "^/.well-known/acme-challenge/(.*)") {
+ if ($request_uri !~ "^/\.well-known/acme-challenge/(.*)") {

Also an alternative implementation of force-ssl.conf could be:

set $url "${scheme}:${request_uri}";
if ($url ~ "^http:(?!/\.well-known/acme-challenge/(.*))") {
        return 301 https://$host$request_uri;
}

[maltokyo]

thanks @the1ts - what do you use now? maybe time to switch, cant be chasing basic stuff like this for months and months.

[nginxproxymanagerci[bot]]

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

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

[EDIflyer]

Any chance this PR could be rebased onto the current version?

[sanderlv]

Sorry, but I need to say my enthusiasm for NPM...

[KaeTuuN]

Bump! Still an issue in 2.10.4! @jc21 PLEASE MERGE THIS PR!

Forget it... Merge #3121

[sunsreddit]

+1, please merge #3121 @jc21