Message ID | 9a18d0c03940cfe0d8ab01964f12d08fcc972e30.1669507155.git.mirai@makinata.eu |
---|---|
State | New |
Headers | show |
Series | [bug#59621] services: nginx: Add support for ssl-stapling in server blocks. | expand |
mirai@makinata.eu writes: > From: Bruno Victal <mirai@makinata.eu> > > * gnu/services/web.scm (<nginx-server-configuration>): Add > ssl-stapling? and ssl-stapling-verify?. > * doc/guix.texi (NGINX): Document this. > --- > doc/guix.texi | 7 +++++ > gnu/services/web.scm | 69 +++++++++++++++++++++++++------------------- > 2 files changed, 46 insertions(+), 30 deletions(-) Hi Bruno, Thanks for the patch, and sorry it's taken so long to reply. > @@ -647,6 +654,8 @@ (define-syntax-rule (and/l x tail ...) > " server_name " (config-domain-strings server-name) ";\n" > (and/l ssl-certificate " ssl_certificate " <> ";\n") > (and/l ssl-certificate-key " ssl_certificate_key " <> ";\n") > + " ssl_stapling " (if ssl-stapling? "on" "off") ";\n" > + " ssl_stapling_verify " (if ssl-stapling-verify? "on" "off") ";\n" > (if (not (equal? "" root)) > (list " root " root ";\n") > "") > > base-commit: 68925b5ee7e0d96b0c84ae98a633eea5097bf511 Generally this looks good to me. There's some unnecessary indentation changes that should probably go in another commit if they're made, but I did spot something in the above diff. I'm no expert in NGinx configs, but I do wonder if this change will break using nginx if it's built without the ngx_http_ssl_module? With the other module specific configuration (e.g. ssl_certificate), it's possible to specify a value in the <nginx-server-configuration> that means the line won't be included in the configuration. I think it would be good to continue that here. I'm not sure how to enable not including these config lines. Maybe a symbol value like 'noval could be used (this should also be the default, rather than #f), or maybe 'on and 'off could be used as the values with #f meaning the line isn't included. Does that make sense? Thanks, Chris
Hi On 2023-01-07 17:21, Christopher Baines wrote: > > mirai@makinata.eu writes: > >> From: Bruno Victal <mirai@makinata.eu> >> >> * gnu/services/web.scm (<nginx-server-configuration>): Add >> ssl-stapling? and ssl-stapling-verify?. >> * doc/guix.texi (NGINX): Document this. >> --- >> doc/guix.texi | 7 +++++ >> gnu/services/web.scm | 69 +++++++++++++++++++++++++------------------- >> 2 files changed, 46 insertions(+), 30 deletions(-) > > Hi Bruno, > > Thanks for the patch, and sorry it's taken so long to reply. > >> @@ -647,6 +654,8 @@ (define-syntax-rule (and/l x tail ...) >> " server_name " (config-domain-strings server-name) ";\n" >> (and/l ssl-certificate " ssl_certificate " <> ";\n") >> (and/l ssl-certificate-key " ssl_certificate_key " <> ";\n") >> + " ssl_stapling " (if ssl-stapling? "on" "off") ";\n" >> + " ssl_stapling_verify " (if ssl-stapling-verify? "on" "off") ";\n" >> (if (not (equal? "" root)) >> (list " root " root ";\n") >> "") >> >> base-commit: 68925b5ee7e0d96b0c84ae98a633eea5097bf511 > > Generally this looks good to me. There's some unnecessary indentation > changes that should probably go in another commit if they're made, but I > did spot something in the above diff. I was afraid that doing it in a separate commit would have made it less clearer as it would have looked like a trivial cosmetic change without any purpose. > > I'm no expert in NGinx configs, but I do wonder if this change will > break using nginx if it's built without the ngx_http_ssl_module? With > the other module specific configuration (e.g. ssl_certificate), it's > possible to specify a value in the <nginx-server-configuration> that > means the line won't be included in the configuration. I think it would > be good to continue that here. I haven't tested this with a nginx that is built without ngx_http_ssl_module, it would be a rather esoteric nginx build as TLS support presence is a common expectation of web servers. > I'm not sure how to enable not including these config lines. Maybe a > symbol value like 'noval could be used (this should also be the default, > rather than #f), or maybe 'on and 'off could be used as the values with > #f meaning the line isn't included. > > Does that make sense? I'm not a fan of this approach as there's define-configuration and define-maybe value types that should be used here rather than making up a custom value, though I'm afraid reworking nginx-configuration and writing the serialize- procedures to use the gnu/services/configuration.scm facilities is a much bigger effort than what's done in this patch. Before such effort is to be considered, a plan to solve [1] is required as I don't think define-configuration is enough to represent the structure of nginx.conf (nested locations, if branches, configuration for custom modules, etc.) [1]: https://issues.guix.gnu.org/37388 Cheers, Bruno
Hi Bruno, Chris, Bruno Victal <mirai@makinata.eu> writes: > Hi > > On 2023-01-07 17:21, Christopher Baines wrote: >> >> mirai@makinata.eu writes: >> >>> From: Bruno Victal <mirai@makinata.eu> >>> >>> * gnu/services/web.scm (<nginx-server-configuration>): Add >>> ssl-stapling? and ssl-stapling-verify?. >>> * doc/guix.texi (NGINX): Document this. >>> --- >>> doc/guix.texi | 7 +++++ >>> gnu/services/web.scm | 69 +++++++++++++++++++++++++------------------- >>> 2 files changed, 46 insertions(+), 30 deletions(-) >> >> Hi Bruno, >> >> Thanks for the patch, and sorry it's taken so long to reply. >> >>> @@ -647,6 +654,8 @@ (define-syntax-rule (and/l x tail ...) >>> " server_name " (config-domain-strings server-name) ";\n" >>> (and/l ssl-certificate " ssl_certificate " <> ";\n") >>> (and/l ssl-certificate-key " ssl_certificate_key " <> ";\n") >>> + " ssl_stapling " (if ssl-stapling? "on" "off") ";\n" >>> + " ssl_stapling_verify " (if ssl-stapling-verify? "on" "off") ";\n" >>> (if (not (equal? "" root)) >>> (list " root " root ";\n") >>> "") >>> >>> base-commit: 68925b5ee7e0d96b0c84ae98a633eea5097bf511 >> >> Generally this looks good to me. There's some unnecessary indentation >> changes that should probably go in another commit if they're made, but I >> did spot something in the above diff. > > I was afraid that doing it in a separate commit would have > made it less clearer as it would have looked like a trivial cosmetic > change without any purpose. > >> >> I'm no expert in NGinx configs, but I do wonder if this change will >> break using nginx if it's built without the ngx_http_ssl_module? With >> the other module specific configuration (e.g. ssl_certificate), it's >> possible to specify a value in the <nginx-server-configuration> that >> means the line won't be included in the configuration. I think it would >> be good to continue that here. > > I haven't tested this with a nginx that is built without ngx_http_ssl_module, > it would be a rather esoteric nginx build as TLS support presence is a > common expectation of web servers. The only nginx package in Guix has TLS support; I wouldn't expect people will go out of the way to define TLS-less variants just to run a local HTTP-only web server; perhaps it's OK to not give to much importance to that for now?
diff --git a/doc/guix.texi b/doc/guix.texi index e547d469f4..f116798dba 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -29339,6 +29339,13 @@ you don't have a certificate or you don't want to use HTTPS. Where to find the private key for secure connections. Set it to @code{#f} if you don't have a key or you don't want to use HTTPS. +@item @code{ssl-stapling?} (default: @code{#f}) +Whether the server should @uref{https://datatracker.ietf.org/doc/html/rfc6066#section-8,staple OCSP responses}. +Requires at least one @samp{resolver} directive in @code{raw-content}. + +@item @code{ssl-stapling-verify?} (default: @code{#f}) +Whether the server should verify the OCSP responses. + @item @code{server-tokens?} (default: @code{#f}) Whether the server should add its configuration to response. diff --git a/gnu/services/web.scm b/gnu/services/web.scm index 83aa97055f..8ab4050d47 100644 --- a/gnu/services/web.scm +++ b/gnu/services/web.scm @@ -510,48 +510,52 @@ (define httpd-service-type (define-record-type* <nginx-server-configuration> nginx-server-configuration make-nginx-server-configuration nginx-server-configuration? - (listen nginx-server-configuration-listen - (default '("80" "443 ssl"))) - (server-name nginx-server-configuration-server-name - (default (list 'default))) - (root nginx-server-configuration-root - (default "/srv/http")) - (locations nginx-server-configuration-locations - (default '())) - (index nginx-server-configuration-index - (default (list "index.html"))) - (try-files nginx-server-configuration-try-files - (default '())) - (ssl-certificate nginx-server-configuration-ssl-certificate - (default #f)) - (ssl-certificate-key nginx-server-configuration-ssl-certificate-key - (default #f)) - (server-tokens? nginx-server-configuration-server-tokens? - (default #f)) - (raw-content nginx-server-configuration-raw-content - (default '()))) + (listen nginx-server-configuration-listen + (default '("80" "443 ssl"))) + (server-name nginx-server-configuration-server-name + (default (list 'default))) + (root nginx-server-configuration-root + (default "/srv/http")) + (locations nginx-server-configuration-locations + (default '())) + (index nginx-server-configuration-index + (default (list "index.html"))) + (try-files nginx-server-configuration-try-files + (default '())) + (ssl-certificate nginx-server-configuration-ssl-certificate + (default #f)) + (ssl-certificate-key nginx-server-configuration-ssl-certificate-key + (default #f)) + (ssl-stapling? nginx-server-configuration-ssl-stapling? + (default #f)) + (ssl-stapling-verify? nginx-server-configuration-ssl-stapling-verify? + (default #f)) + (server-tokens? nginx-server-configuration-server-tokens? + (default #f)) + (raw-content nginx-server-configuration-raw-content + (default '()))) (define-record-type* <nginx-upstream-configuration> nginx-upstream-configuration make-nginx-upstream-configuration nginx-upstream-configuration? - (name nginx-upstream-configuration-name) - (servers nginx-upstream-configuration-servers) - (extra-content nginx-upstream-configuration-extra-content - (default '()))) + (name nginx-upstream-configuration-name) + (servers nginx-upstream-configuration-servers) + (extra-content nginx-upstream-configuration-extra-content + (default '()))) (define-record-type* <nginx-location-configuration> nginx-location-configuration make-nginx-location-configuration nginx-location-configuration? - (uri nginx-location-configuration-uri - (default #f)) - (body nginx-location-configuration-body)) + (uri nginx-location-configuration-uri + (default #f)) + (body nginx-location-configuration-body)) (define-record-type* <nginx-named-location-configuration> nginx-named-location-configuration make-nginx-named-location-configuration nginx-named-location-configuration? - (name nginx-named-location-configuration-name - (default #f)) - (body nginx-named-location-configuration-body)) + (name nginx-named-location-configuration-name + (default #f)) + (body nginx-named-location-configuration-body)) (define-record-type* <nginx-configuration> nginx-configuration make-nginx-configuration @@ -628,6 +632,9 @@ (define (emit-nginx-server-config server) (ssl-certificate (nginx-server-configuration-ssl-certificate server)) (ssl-certificate-key (nginx-server-configuration-ssl-certificate-key server)) + (ssl-stapling? (nginx-server-configuration-ssl-stapling? server)) + (ssl-stapling-verify? + (nginx-server-configuration-ssl-stapling-verify? server)) (root (nginx-server-configuration-root server)) (index (nginx-server-configuration-index server)) (try-files (nginx-server-configuration-try-files server)) @@ -647,6 +654,8 @@ (define-syntax-rule (and/l x tail ...) " server_name " (config-domain-strings server-name) ";\n" (and/l ssl-certificate " ssl_certificate " <> ";\n") (and/l ssl-certificate-key " ssl_certificate_key " <> ";\n") + " ssl_stapling " (if ssl-stapling? "on" "off") ";\n" + " ssl_stapling_verify " (if ssl-stapling-verify? "on" "off") ";\n" (if (not (equal? "" root)) (list " root " root ";\n") "")
From: Bruno Victal <mirai@makinata.eu> * gnu/services/web.scm (<nginx-server-configuration>): Add ssl-stapling? and ssl-stapling-verify?. * doc/guix.texi (NGINX): Document this. --- doc/guix.texi | 7 +++++ gnu/services/web.scm | 69 +++++++++++++++++++++++++------------------- 2 files changed, 46 insertions(+), 30 deletions(-) base-commit: 68925b5ee7e0d96b0c84ae98a633eea5097bf511