GNU bug report logs - #59621
[PATCH] services: nginx: Add support for ssl-stapling in server blocks.

Previous Next

Package: guix-patches;

Reported by: mirai <at> makinata.eu

Date: Sun, 27 Nov 2022 00:01:02 UTC

Severity: normal

Tags: patch

To reply to this bug, email your comments to 59621 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#59621; Package guix-patches. (Sun, 27 Nov 2022 00:01:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to mirai <at> makinata.eu:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Sun, 27 Nov 2022 00:01:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: mirai <at> makinata.eu
To: guix-patches <at> gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH] services: nginx: Add support for ssl-stapling in server
 blocks.
Date: Sat, 26 Nov 2022 23:59:50 +0000
From: Bruno Victal <mirai <at> 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(-)

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")
          "")

base-commit: 68925b5ee7e0d96b0c84ae98a633eea5097bf511
-- 
2.38.1





Information forwarded to guix-patches <at> gnu.org:
bug#59621; Package guix-patches. (Sat, 07 Jan 2023 18:56:02 GMT) Full text and rfc822 format available.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: mirai <at> makinata.eu
Cc: 59621 <at> debbugs.gnu.org, guix-patches <at> gnu.org
Subject: Re: [bug#59621] [PATCH] services: nginx: Add support for
 ssl-stapling in server blocks.
Date: Sat, 07 Jan 2023 17:21:08 +0000
[Message part 1 (text/plain, inline)]
mirai <at> makinata.eu writes:

> From: Bruno Victal <mirai <at> 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
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#59621; Package guix-patches. (Sat, 07 Jan 2023 18:56:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#59621; Package guix-patches. (Sat, 07 Jan 2023 20:08:02 GMT) Full text and rfc822 format available.

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 59621 <at> debbugs.gnu.org, guix-patches <at> gnu.org
Subject: Re: [bug#59621] [PATCH] services: nginx: Add support for ssl-stapling
 in server blocks.
Date: Sat, 7 Jan 2023 20:07:11 +0000
Hi

On 2023-01-07 17:21, Christopher Baines wrote:
> 
> mirai <at> makinata.eu writes:
> 
>> From: Bruno Victal <mirai <at> 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




Information forwarded to guix-patches <at> gnu.org:
bug#59621; Package guix-patches. (Sat, 07 Jan 2023 20:08:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#59621; Package guix-patches. (Tue, 21 Mar 2023 13:21:02 GMT) Full text and rfc822 format available.

Message #20 received at 59621 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: Christopher Baines <mail <at> cbaines.net>, 59621 <at> debbugs.gnu.org
Subject: Re: bug#59621: [PATCH] services: nginx: Add support for
 ssl-stapling in server blocks.
Date: Tue, 21 Mar 2023 09:20:49 -0400
Hi Bruno, Chris,

Bruno Victal <mirai <at> makinata.eu> writes:

> Hi
>
> On 2023-01-07 17:21, Christopher Baines wrote:
>> 
>> mirai <at> makinata.eu writes:
>> 
>>> From: Bruno Victal <mirai <at> 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?

-- 
Thanks,
Maxim




This bug report was last modified 1 year and 36 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.