certbot
[Bug]: Could not parse file error (at char 0), (line:1, col:1) because there's a "{" in a commented line of an NGINX server block config file
OS
Ubuntu 22.04.5 LTS
Installation method
snap
Certbot Version
4.0.0
What happened?
I ran this command:
sudo certbot renew --dry-run --dns-cloudflare-propagation-seconds 30
It produced this output repeatedly for every server block configured in NGINX :
Could not parse file: /etc/nginx/sites-enabled/adamlein.com due to Expected string_end, found 'server' (at char 0), (line:1, col:1)
The same error also occurs when running:
sudo certbot --installer nginx --dns-cloudflare --dns-cloudflare-credentials /etc/cloudflare/credential.ini --dns-cloudflare-propagation-seconds 30
Along with help in the community forum, I found that the issue was not with the actual character 0 or line 1 column 1 (the word "server"). It was with a comment on line 68 in one of my server block config files.
Changing this:
location = /resume
# { rewrite .* /resume.php redirect; }
{ rewrite .* /Files/Adam_Lein_resume.pdf redirect; }
to this:
location = /resume
{ rewrite .* /Files/Adam_Lein_resume.pdf redirect; }
... removes the error and certbot is able to function properly. Upon further investigation, simply adding a comment with the { character will cause the parsing error on the first character of the NGINX config file even if the parsing error is on a different line. For example, adding "# {" between two lines of a rewrite will cause the problem:
location = /resume
# {
{ rewrite .* /Files/Adam_Lein_resume.pdf redirect; }
Expected behavior
- The error message should indicate the actual line number, character, and column where the parsing error occurs.
- Certbot should ignore commented-out lines.
Relevant log output
Thanks for finding this. We use pyparsing to read nginx config files. This unfortunately means that it's basically impossible to say where the parsing error actually occurred; it's basically a giant regex. And then comments in particular tend to give us trouble -- the usually parsing method would be to first delete all comments, then parse. But since we want to write the file back out, we can't delete comments, and basically need to special-case all the places where they might occur. We recently did that for server names here, for example. I'll look into adding this one though.
Unfortunately there will probably always be more, and the solution will be to move the comment or delete it. For example, I'm pretty sure that this would parse correctly, if you want to keep the comment around:
location = /resume
{ rewrite .* /Files/Adam_Lein_resume.pdf redirect; }
# { rewrite .* /resume.php redirect; }
We just don't handle the comment between the declaration of the start of the location block and the braces defining its contents.
Thanks for the further explanation. I personally don't really need that comment anymore now that I know it's the problem, but the real problem was:
- Certbot works perfectly fine for some years
- One day I edit a redirect in an NGINX server block config file
- Everything works fine
- Some weeks or months later, the website's HTTPS stops working because the SSL certificate was not automatically renewed
- Certbot's error message points to the word "server" which was always there and should be valid
- There's no easy way to tell what went wrong
It seems that the parser can tell which file the error is in at least. Since it's impossible to tell what line or character the error occurs on, can the error message be changed to something more useful like what you wrote above? We don't need the error message providing wrong information saying the error is (at char 0), (line:1, col:1) when it isn't. Maybe:
Could not parse file: /etc/nginx/sites-enabled/adamlein.com. Please see https://certbot.eff.org/docs/BLAHBLAHBLAH for information about NGINX config options that may cause Certbot to fail.
Then make a web page in the docs that lists troubleshooting ideas and known limitations like the ones you mentioned:
We just don't handle the comment between the declaration of the start of the location block and the braces defining its contents.
Would something like that be easier/better?
Until recently, I was under the optimistic assumption that we'd be able to find and fix any such issues. Given that that's no longer the case, I like your proposal of doing our best to explain and improve the error message. I think a general explanation of the type of things that are hard (lua, comments) might be easier, because some errors are fixed in certain versions and we'd then have to note which version various errors were fixed in, which seems like a lot of upkeep for our small project.
As for this specific error, I did take a stab at it, but the various things I tried haven't helped. My initial thought was to just explicitly define this case by adding a comment to the definition of a block (block << block_begin + ZeroOrMore(comment) + left_bracket + block_innards + right_bracket), but that isn't working. Which makes me think that to fix this, I would need a deeper understanding of the inner workings of pyparsing and how it actually works...which is all to say, I think I'm going to go the "improve the error message route."
I think I'm going to learn to run certbot renew --dry-run AND nginx -t after every NGINX config file change now that I know!
Maybe after a successful certbot --installer nginx is run, a warning should appear to educate the user as something like:
Warning: Certain NGINX configuration changes may cause certbot renewals to fail. We recommend running "certbot renew --dry-run" after future NGINX configuration edits to make sure certbot can process them.
Maybe that's something for a different issue thread.
Thanks again!
Hm, how to convey that is quite an interesting question. I don't know that adding that text at first certificate issuance will help; it's just text that people will skim past since it doesn't apply immediately.
Ideally it would be presented when the user has changed the config -- perhaps placed as a comment along with the lines we add, though that would only help users who have certbot install the certificate into the config as well as acquire the certificate. Might still be worth adding a note.
Otherwise, there's the general mechanism where we start attempting certbot renewals prior to expiry (2/3 or 1/2 the way through depending on the certificate lifetime), and making sure that error messages from that are sufficiently clear, which I do think the PR I just added will help with. But that method relies on people having set up sufficient monitoring in some way; get notices of automated certificate renewal failure from the machine, or sign up for certificate monitoring that warns when certs are about to expire, which if that date is after it's supposed to have renewed will indicate that the cert didn't renew. At which point they'll run it manually and again encounter the new error message.
how often that plugin need to change config in renewal? Could we calculate and record webroot for that domain and use that for renewal?
based on the comment at https://github.com/certbot/certbot/pull/10265#issue-2989816621, i'm not sure if we wanted to close this issue or not so i'm reopening for now. ohemorange can close if they wish
