| Message ID | 20230403133241.14760-1-saku@laesvuori.fi |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#62642] services: certbot: Fix nginx crash when certbot is used without domains | expand |
Hi Saku, On 2023-04-03 14:32, Saku Laesvuori via Guix-patches via wrote: > * gnu/services/certbot.scm (certbot-nginx-server-configurations): > Don't return a broken nginx-server-configuration when no certificate > domains are configured. Is there a use-case for certbot without any certificate configurations provided? IMO it looks to me that the 'certificates' field shouldn't have a default value configured instead? Cheers, Bruno
Hi, > Is there a use-case for certbot without any certificate configurations provided? I was writing a service that extends certbot if a configuration option for it is set to #t. To me it seems that it is currently impossible to view the configuration in the service type definition, so I worked around it by extending certbot-service-type with an empty list if the option is set to #f. > IMO it looks to me that the 'certificates' field shouldn't have a default value > configured instead? Wouldn't that mean that users who use certbot only via services that extend it would have to configure 'certificates' to () manually and have their nginx configuration crash if they remove the extending services and forget to remove the certbot service? - Saku
On 2023-04-03 19:06, Saku Laesvuori wrote: > Hi, > >> Is there a use-case for certbot without any certificate configurations provided? > > I was writing a service that extends certbot if a configuration option > for it is set to #t. To me it seems that it is currently impossible to > view the configuration in the service type definition, so I worked > around it by extending certbot-service-type with an empty list if the > option is set to #f. Right, that's a valid use case. > >> IMO it looks to me that the 'certificates' field shouldn't have a default value >> configured instead? > > Wouldn't that mean that users who use certbot only via services that > extend it would have to configure 'certificates' to () manually and have > their nginx configuration crash if they remove the extending services > and forget to remove the certbot service? You're correct, having the default value set is not a problem here. IMO, certbot should be extending the nginx service only when the 'challenge' field is #f (ideally this should be made into a “enumerated” type, where the values range from 'http-01, 'dns-01, 'custom (as an escape hatch), ...) Perhaps you could partition 'certificates' by whether 'challenge' is #f or not and use the results to craft the nginx extension value instead? Cheers, Bruno
> IMO, certbot should be extending the nginx service only when the 'challenge' field > is #f (ideally this should be made into a “enumerated” type, where the values range from > 'http-01, 'dns-01, 'custom (as an escape hatch), ...) > > Perhaps you could partition 'certificates' by whether 'challenge' is #f or not and use the > results to craft the nginx extension value instead? Certbot extends nginx for two reasons: 1. serving the challenge files 2. enforcing HTTPS by redirecting requests to domains with a certificate The v2 patch adds a separate nginx server block for each certificate and only servers challenge files if 'challenge' is #f. This also causes an empty list of certificates to return an empty list of nginx server blocks and thus fixes the original issue. - Saku Laesvuori
Is there something blocking this patch or is it just waiting for someone to get around to applying it? - Saku
diff --git a/gnu/services/certbot.scm b/gnu/services/certbot.scm index 8e6784df2b..3d9d207f8a 100644 --- a/gnu/services/certbot.scm +++ b/gnu/services/certbot.scm @@ -173,20 +173,21 @@ (define certbot-nginx-server-configurations (match-lambda (($ <certbot-configuration> package webroot certificates email server rsa-key-size default-location) - (list - (nginx-server-configuration - (listen '("80" "[::]:80")) - (ssl-certificate #f) - (ssl-certificate-key #f) - (server-name - (apply append (map certificate-configuration-domains certificates))) - (locations - (filter identity - (list - (nginx-location-configuration - (uri "/.well-known") - (body (list (list "root " webroot ";")))) - default-location)))))))) + (if (null? certificates) '() + (list + (nginx-server-configuration + (listen '("80" "[::]:80")) + (ssl-certificate #f) + (ssl-certificate-key #f) + (server-name + (apply append (map certificate-configuration-domains certificates))) + (locations + (filter identity + (list + (nginx-location-configuration + (uri "/.well-known") + (body (list (list "root " webroot ";")))) + default-location))))))))) (define certbot-service-type (service-type (name 'certbot)