virtualmin / virtualmin-gpl

Virtualmin web hosting control panel for Webmin
https://www.virtualmin.com
GNU General Public License v3.0
330 stars 102 forks source link

disabled sites have letsencrypt asking new certificates and fail #956

Closed aqueos closed 2 weeks ago

aqueos commented 2 weeks ago

hi,

i have a disabled website since august with domains that do not point to the server as they moved away. The check on domain resolution is up but :+1: Screenshot from 2024-11-08 09-10-07

the letsencrypt renewall still runs....

Should it continue to run for a website that is disabled ? The error threshold can rise quickly and the server be blocked by letsencrypt if too many disabled domains do that.

If you disable a website it does not allow any web request and i cannot renew by hand as it tells me its disabled but virtualmin continue to renew it ? I think there is an issue here.

regards, Ghislain.

aqueos commented 2 weeks ago

need a little testing between certbot time and virtualmin, will reopen if test show that virtualmin is doing it

iliajie commented 2 weeks ago

We don't auto-renew domains that are disabled https://github.com/virtualmin/virtualmin-gpl/commit/e91b6402d6fa990af2be6834fc60bdbce10d8318.

aqueos commented 2 weeks ago

hi,

you were quicker than me :)

so i testedd and the issue was we upgraded the systems from initv to systemd that enabled the certbot timer.

Perhaps "virtualmin check" could warn about this ? its a corner case i know :)

best regards, Ghislain.

iliajie commented 2 weeks ago

Perhaps "virtualmin check" could warn about this ? its a corner case i know :)

Jamie, what are your thoughts on this?

jcameron commented 2 weeks ago

Yeah we could check that certbot's own renewal action isn't enabled. What is it called though?

iliajie commented 2 weeks ago

It’s certbot.timer. You can check the code in Virtualmin-Config repo, we already do it there by default.

jcameron commented 2 weeks ago

Is there a more generic way to disable certbot auto-renewal? Checking for a specific systemd action seems unreliable, because it might be run from cron.

iliajie commented 2 weeks ago

No, it must never run from cron on systemd systems.

jcameron commented 2 weeks ago

Looks like the cleaner solution for each virtualmin-managed LE cert is to delete or disable the file in /etc/letsencrypt/renewal

iliajie commented 2 weeks ago

I don't think we should handle it this way in our case. We definitely shouldn’t delete anything in the /etc/letsencrypt/renewal folder. Instead, we could manage renewals by using the renew_before_expiry option in each domain’s Certbot config file, but that’s an overhead in my view, since we don’t want it to run at all. I think turning off the timer service is the better and simpler option.

Why did you first think we should handle it by adjusting the configs in the /etc/letsencrypt/renewal directory?

jcameron commented 2 weeks ago

I like doing it per-cert via changes in /etc/letsencrypt/renewal for two reasons :

1 - This covers all possible ways certbot might be run, such as cron jobs, systemd or manual runs by the user.

2 - Users might have certs that were manually issued by certbot and unmanaged by Virtualmin that they do want automatic renewals continue for.

iliajie commented 2 weeks ago

Users might have certs that were manually issued by certbot and unmanaged by Virtualmin that they do want automatic renewals continue for.

This one is very reasonable!

Okay, so, how do we set it up in the Certbot domain config file, i.e. in /etc/letsencrypt/renewal?

jcameron commented 2 weeks ago

Maybe there's some line we can remove, like renew_before_expiry ? The format of that file doesn't seem to be well documented..

iliajie commented 2 weeks ago

Why not simply add the --no-autorenew flag to the certbot command in the letsencrypt-lib.pl file?

This will add the line autorenew = False in the [renewalparams] section of the domain Certbot config file.

jcameron commented 2 weeks ago

Oh that's perfect! I will add that flag