Due to the way nginx configuration works, any failure in certificate generation can lead to massive breakage as nginx refuses to reload configuration globally if there are any missing certificate files.

So, the configuration generation must be careful to ensure certificates are in place BEFORE uploading ssl enabled vhost configuration.

CommentFileSizeAuthor
#5 2955062-nginx-safe-cert-copying_3.patch3.91 KBbdragon
#3 2955062-nginx-safe-cert-copying_2.patch3.88 KBbdragon
#2 2955062-nginx-safe-cert-copying.patch3.88 KBbdragon

Comments

bdragon created an issue. See original summary.

bdragon’s picture

bdragon commented
Status: Active » Needs work
StatusFileSize
new 2955062-nginx-safe-cert-copying.patch3.88 KB

Here's a first stab at this. Will test it shortly to see if it works as intended.

bdragon’s picture

bdragon commented
StatusFileSize
new 2955062-nginx-safe-cert-copying_2.patch3.88 KB

Fix typo.

bdragon’s picture

bdragon commented
Status: Needs work » Needs review

OK, after fixing that typo, I was unable to break it again.

bdragon’s picture

bdragon commented
StatusFileSize
new 2955062-nginx-safe-cert-copying_3.patch3.91 KB

Forgot to convert the return value to a status check.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented

We found ourselves in this situation after running a remote_import, but prior to DNS being cut-over. So the LE script was triggering a challenge against the old IP, failing, then generating vhosts pointing to non-existent certs.

We had to manually edit the vhosts to comment-out the SSL lines, so that NGINX would reload.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Status: Needs review » Reviewed & tested by the community

Code looks good to me, and applied to one of my Dev servers without any problems. Will report back if anything strange comes up.

  • helmo committed faa5b02 on 7.x-3.x authored by bdragon 
    Issue #2955062 by bdragon: Do not let nginx configuration reference...
helmo’s picture

helmo commented
Status: Reviewed & tested by the community » Fixed

committed, thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jon pugh’s picture

jon pugh
he/him
Primary language English
Location Newburgh, NY
commented

I really wish this logic was applied to Apache at this time.

I did a lot of work on the apache side of this only just recently, caused a major regression in LetsEncrypt, despite being marked as RTBC. I wasn't even aware of this issue until just now, after debugging #3020747: Don't add SSL config to configuration files if the crt files aren't there/aren't readable. (especially redirects).

Please make sure to apply changes like this to both NGINX and Apache templates, and please don't forget Hosting SSL is still present, so it should receive these enhancements too.

jon pugh’s picture

jon pugh
he/him
Primary language English
Location Newburgh, NY
commented
Related issues: +#3020747: Don't add SSL config to configuration files if the crt files aren't there/aren't readable. (especially redirects)