[bug#62642] services: certbot: Fix nginx crash when certbot is used without domains
Commit Message
* gnu/services/certbot.scm (certbot-nginx-server-configurations):
Don't return a broken nginx-server-configuration when no certificate
domains are configured.
---
gnu/services/certbot.scm | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
base-commit: 2cf71e725d55bc5bf1ad663b7c696516299cc8a7
Comments
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
@@ -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)