On 8/16/2024 8:02 AM, Praveen Chaudhary wrote:
Hi Nginx Devs

Bumping patch to the top for review.

CC: @Sergey Kandaurov
Thanks for contributing client certificate validation with OSCP.  It is a long awaited feature. In this patch, I am trying to fix another lingering concern. It will be great, if you can have a look.

Hello,

Sending an empty list of CAs is explicitly mentioned starting from TLS 1.1; RFC 4346 Section 7.4.4:

    If the certificate_authorities list is empty then the client MAY
    send any certificate of the appropriate ClientCertificateType,
    unless there is some external arrangement to the contrary.

TLS 1.0 (RFC 2246 Section 7.4.4) does not specify any behavior. While it's known that some 1.0 or SSL 3.0 clients can accept an empty list, it could be safer to limit the ability to the TLS 1.1+ configurations.

As for the means of doing so, simply skipping the conf->client_certificate check is not correct. For any ssl_client_verify mode other than 'optional_no_ca' nginx must have a list of known trusted CAs, and the configuration without any is not valid. A better approach is to require either 'ssl_client_certificate' or 'ssl_trusted_certificate' to be set when the client cert verification is enabled.


# HG changeset patch
# User Praveen Chaudhary <praveen5...@gmail.com <mailto:praveen5...@gmail.com>>
# Date 1723406727 25200
#      Sun Aug 11 13:05:27 2024 -0700
# Node ID 199a35c74b60437da9d22a70d257507b4afb1878
# Parent  b5550a7f16c795f394f9d1ac87132dd2b7ef0e41
Make ssl_client_certificate directive optional with TLSv1.3.

- As per RFC 8446 Section 4.2.4, server MAY (not SHOULD or MUST)
   send Certificate Authorities (CAs) in the Certificate Request
   packet. This makes ssl_client_certificate directive optional
   when only TLS 1.3 is used for mutual TLS configurations.
> > - Today, Nginx requires ssl_client_certificate directive to
   be set to CA Certificates file, if ssl_verify_client is
   enabled, even when using only TLS 1.3. Else Nginx does not
   reload or restart.

diff -r b5550a7f16c7 -r 199a35c74b60 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c Fri Aug 09 19:12:26 2024 +0400
+++ b/src/http/modules/ngx_http_ssl_module.c Sun Aug 11 13:05:27 2024 -0700
@@ -787,10 +787,16 @@

      if (conf->verify) {

-        if (conf->client_certificate.len == 0 && conf->verify != 3) {
-            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                          "no ssl_client_certificate for ssl_verify_client");
-            return NGX_CONF_ERROR;
+        if (conf->protocols & (NGX_SSL_TLSv1|NGX_SSL_TLSv1_1| NGX_SSL_TLSv1_2)) {
+            /*
+            For TLS 1.3, It is optional to send Certificate Authorities in
+            Certificate Request Packet. RFC 8446#section-4.2.4
+            */
+            if (conf->client_certificate.len == 0 && conf->verify != 3) {
+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+                            "no ssl_client_certificate for ssl_verify_client");
+                return NGX_CONF_ERROR;
+            }
          }

          if (ngx_ssl_client_certificate(cf, &conf->ssl,
diff -r b5550a7f16c7 -r 199a35c74b60 src/mail/ngx_mail_ssl_module.c
--- a/src/mail/ngx_mail_ssl_module.c Fri Aug 09 19:12:26 2024 +0400
+++ b/src/mail/ngx_mail_ssl_module.c Sun Aug 11 13:05:27 2024 -0700
@@ -450,12 +450,19 @@

      if (conf->verify) {

-        if (conf->client_certificate.len == 0 && conf->verify != 3) {
-            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                          "no ssl_client_certificate for ssl_verify_client");
-            return NGX_CONF_ERROR;
+        if (conf->protocols & (NGX_SSL_TLSv1|NGX_SSL_TLSv1_1| NGX_SSL_TLSv1_2)) {
+            /*
+            For TLS 1.3, It is optional to send Certificate Authorities in
+            Certificate Request Packet. RFC 8446#section-4.2.4
+            */
+            if (conf->client_certificate.len == 0 && conf->verify != 3) {
+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+                            "no ssl_client_certificate for ssl_verify_client");
+                return NGX_CONF_ERROR;
+            }
          }

+
          if (ngx_ssl_client_certificate(cf, &conf->ssl,
                                         &conf->client_certificate,
                                         conf->verify_depth)
diff -r b5550a7f16c7 -r 199a35c74b60 src/stream/ngx_stream_ssl_module.c
--- a/src/stream/ngx_stream_ssl_module.c Fri Aug 09 19:12:26 2024 +0400
+++ b/src/stream/ngx_stream_ssl_module.c Sun Aug 11 13:05:27 2024 -0700
@@ -932,10 +932,16 @@

      if (conf->verify) {

-        if (conf->client_certificate.len == 0 && conf->verify != 3) {
-            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                          "no ssl_client_certificate for ssl_verify_client");
-            return NGX_CONF_ERROR;
+        if (conf->protocols & (NGX_SSL_TLSv1|NGX_SSL_TLSv1_1| NGX_SSL_TLSv1_2)) {
+            /*
+            For TLS 1.3, It is optional to send Certificate Authorities in
+            Certificate Request Packet. RFC 8446#section-4.2.4
+            */
+            if (conf->client_certificate.len == 0 && conf->verify != 3) {
+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+                            "no ssl_client_certificate for ssl_verify_client");
+                return NGX_CONF_ERROR;
+            }
          }

          if (ngx_ssl_client_certificate(cf, &conf->ssl,

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Reply via email to