<div dir="ltr"><div dir="ltr"><a class="gmail_plusreply" id="gmail-plusReplyChip-2" href="mailto:a.bavshin@nginx.com" tabindex="-1">@a.bavshin@nginx.com</a> <br></div><div dir="ltr"><br></div><div>Gentle Reminder for review. This feature to make ssl_client_certificate optional may help us here at Nvidia.</div><div>Thanks in advance. Kindly let me know if any more modification is needed in fix.</div><div><br></div><div>Note: AFAIK, mTLS was not supported with SSLv2. I kept the NGX_SSL_SSLv2 flag in fix, if Nginx today supports SSLv2.</div><div dir="ltr"></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 19, 2024 at 4:22 PM Praveen Chaudhary <<a href="mailto:pclicoder@gmail.com" target="_blank">pclicoder@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Thanks Aleksei for the review.</div><div><br></div><div>Agree, It makes sense to have explicit error message to require either</div><div>ssl_client_certificate or ssl_trusted_certificate. Because: <br></div><div>Nginx prints error number from SSL to identify SSL error\routine,</div><div>but for a client or admin, it may be still hard to find why "SSL certificate error"</div><div>is seen.</div><div><br></div><div>V2 Patch.</div><div>[Keeping same Subject]</div><div><br></div># HG changeset patch<br># User Praveen Chaudhary <<a href="mailto:praveen5582@gmail.com" target="_blank">praveen5582@gmail.com</a>><br># Date 1723406727 25200<br>#      Sun Aug 11 13:05:27 2024 -0700<br># Node ID a5525b8eac0e1f10da42b7367cd5296fb29f4787<br># Parent  8796dfbe7177cb0be2a53bcdb4d25cc64a58d2a7<br>Make ssl_client_certificate directive optional with TLSv1.1+.<br><br>- With TLS 1.1+, Certificate Authorities(CAs) are optional<br>  in the Certificate Request packet. This makes directive<br>  ssl_client_certificate also optional for mutual TLS<br>  configurations.<br><br>- For TLS 1.1, check either ssl_client_certificate or<br>  ssl_trusted_certificate is non empty.<br><br>diff -r 8796dfbe7177 -r a5525b8eac0e src/http/modules/ngx_http_ssl_module.c<br>--- a/src/http/modules/ngx_http_ssl_module.c    Mon Aug 12 18:21:01 2024 +0400<br>+++ b/src/http/modules/ngx_http_ssl_module.c    Sun Aug 11 13:05:27 2024 -0700<br>@@ -788,9 +788,20 @@<br>     if (conf->verify) {<br> <br>         if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>-            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>-                          "no ssl_client_certificate for ssl_verify_client");<br>-            return NGX_CONF_ERROR;<br>+<br>+            if (conf->protocols &<br>+                (NGX_SSL_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1)) {<br>+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>+                            "no ssl_client_certificate for ssl_verify_client");<br>+                return NGX_CONF_ERROR;<br>+            }<br>+            /* For TLS 1.1+. */<br>+            if (conf->trusted_certificate.len == 0) {<br>+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>+                            "no ssl_client_certificate or "<br>+                            "ssl_trusted_certificate for ssl_verify_client");<br>+                return NGX_CONF_ERROR;<br>+            }<br>         }<br> <br>         if (ngx_ssl_client_certificate(cf, &conf->ssl,<br>diff -r 8796dfbe7177 -r a5525b8eac0e src/mail/ngx_mail_ssl_module.c<br>--- a/src/mail/ngx_mail_ssl_module.c      Mon Aug 12 18:21:01 2024 +0400<br>+++ b/src/mail/ngx_mail_ssl_module.c    Sun Aug 11 13:05:27 2024 -0700<br>@@ -451,9 +451,20 @@<br>     if (conf->verify) {<br> <br>         if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>-            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>-                          "no ssl_client_certificate for ssl_verify_client");<br>-            return NGX_CONF_ERROR;<br>+<br>+            if (conf->protocols &<br>+                (NGX_SSL_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1)) {<br>+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>+                            "no ssl_client_certificate for ssl_verify_client");<br>+                return NGX_CONF_ERROR;<br>+            }<br>+            /* For TLS 1.1+. */<br>+            if (conf->trusted_certificate.len == 0) {<br>+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>+                            "no ssl_client_certificate or "<br>+                            "ssl_trusted_certificate for ssl_verify_client");<br>+                return NGX_CONF_ERROR;<br>+            }<br>         }<br> <br>         if (ngx_ssl_client_certificate(cf, &conf->ssl,<br>diff -r 8796dfbe7177 -r a5525b8eac0e src/stream/ngx_stream_ssl_module.c<br>--- a/src/stream/ngx_stream_ssl_module.c      Mon Aug 12 18:21:01 2024 +0400<br>+++ b/src/stream/ngx_stream_ssl_module.c        Sun Aug 11 13:05:27 2024 -0700<br>@@ -933,9 +933,20 @@<br>     if (conf->verify) {<br> <br>         if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>-            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>-                          "no ssl_client_certificate for ssl_verify_client");<br>-            return NGX_CONF_ERROR;<br>+<br>+            if (conf->protocols &<br>+                (NGX_SSL_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1)) {<br>+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>+                            "no ssl_client_certificate for ssl_verify_client");<br>+                return NGX_CONF_ERROR;<br>+            }<br>+            /* For TLS 1.1+. */<br>+            if (conf->trusted_certificate.len == 0) {<br>+                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>+                            "no ssl_client_certificate or "<br>+                            "ssl_trusted_certificate for ssl_verify_client");<br>+                return NGX_CONF_ERROR;<br>+            }<br>         }<br> <br>         if (ngx_ssl_client_certificate(cf, &conf->ssl,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 19, 2024 at 11:41 AM Aleksei Bavshin <<a href="mailto:a.bavshin@nginx.com" target="_blank">a.bavshin@nginx.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">On 8/16/2024 8:02 AM, Praveen Chaudhary wrote:<br>
> Hi Nginx Devs<br>
> <br>
> Bumping patch to the top for review.<br>
> <br>
> CC: @Sergey Kandaurov<br>
> Thanks for contributing client certificate validation with OSCP.  It is <br>
> a long awaited feature.<br>
> In this patch, I am trying to fix another lingering concern. It will be <br>
> great, if you can have a look.<br>
<br>
Hello,<br>
<br>
Sending an empty list of CAs is explicitly mentioned starting from TLS <br>
1.1; RFC 4346 Section 7.4.4:<br>
<br>
     If the certificate_authorities list is empty then the client MAY<br>
     send any certificate of the appropriate ClientCertificateType,<br>
     unless there is some external arrangement to the contrary.<br>
<br>
TLS 1.0 (RFC 2246 Section 7.4.4) does not specify any behavior. While <br>
it's known that some 1.0 or SSL 3.0 clients can accept an empty list, it <br>
could be safer to limit the ability to the TLS 1.1+ configurations.<br>
<br>
As for the means of doing so, simply skipping the <br>
conf->client_certificate check is not correct. For any ssl_client_verify <br>
mode other than 'optional_no_ca' nginx must have a list of known trusted <br>
CAs, and the configuration without any is not valid.<br>
A better approach is to require either 'ssl_client_certificate' or <br>
'ssl_trusted_certificate' to be set when the client cert verification is <br>
enabled.<br>
<br>
> <br>
> # HG changeset patch<br>
> # User Praveen Chaudhary <<a href="mailto:praveen5582@gmail.com" target="_blank">praveen5582@gmail.com</a> <br>
> <mailto:<a href="mailto:praveen5582@gmail.com" target="_blank">praveen5582@gmail.com</a>>><br>
> # Date 1723406727 25200<br>
> #      Sun Aug 11 13:05:27 2024 -0700<br>
> # Node ID 199a35c74b60437da9d22a70d257507b4afb1878<br>
> # Parent  b5550a7f16c795f394f9d1ac87132dd2b7ef0e41<br>
> Make ssl_client_certificate directive optional with TLSv1.3.<br>
> <br>
> - As per RFC 8446 Section 4.2.4, server MAY (not SHOULD or MUST)<br>
>    send Certificate Authorities (CAs) in the Certificate Request<br>
>    packet. This makes ssl_client_certificate directive optional<br>
>    when only TLS 1.3 is used for mutual TLS configurations.<br>
 > > - Today, Nginx requires ssl_client_certificate directive to<br>
>    be set to CA Certificates file, if ssl_verify_client is<br>
>    enabled, even when using only TLS 1.3. Else Nginx does not<br>
>    reload or restart.<br>
> <br>
> diff -r b5550a7f16c7 -r 199a35c74b60 src/http/modules/ngx_http_ssl_module.c<br>
> --- a/src/http/modules/ngx_http_ssl_module.c Fri Aug 09 19:12:26 2024 +0400<br>
> +++ b/src/http/modules/ngx_http_ssl_module.c Sun Aug 11 13:05:27 2024 -0700<br>
> @@ -787,10 +787,16 @@<br>
> <br>
>       if (conf->verify) {<br>
> <br>
> -        if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>
> -            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>
> -                          "no ssl_client_certificate for <br>
> ssl_verify_client");<br>
> -            return NGX_CONF_ERROR;<br>
> +        if (conf->protocols & (NGX_SSL_TLSv1|NGX_SSL_TLSv1_1| <br>
> NGX_SSL_TLSv1_2)) {<br>
> +            /*<br>
> +            For TLS 1.3, It is optional to send Certificate Authorities in<br>
> +            Certificate Request Packet. RFC 8446#section-4.2.4<br>
> +            */<br>
> +            if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>
> +                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>
> +                            "no ssl_client_certificate for <br>
> ssl_verify_client");<br>
> +                return NGX_CONF_ERROR;<br>
> +            }<br>
>           }<br>
> <br>
>           if (ngx_ssl_client_certificate(cf, &conf->ssl,<br>
> diff -r b5550a7f16c7 -r 199a35c74b60 src/mail/ngx_mail_ssl_module.c<br>
> --- a/src/mail/ngx_mail_ssl_module.c Fri Aug 09 19:12:26 2024 +0400<br>
> +++ b/src/mail/ngx_mail_ssl_module.c Sun Aug 11 13:05:27 2024 -0700<br>
> @@ -450,12 +450,19 @@<br>
> <br>
>       if (conf->verify) {<br>
> <br>
> -        if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>
> -            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>
> -                          "no ssl_client_certificate for <br>
> ssl_verify_client");<br>
> -            return NGX_CONF_ERROR;<br>
> +        if (conf->protocols & (NGX_SSL_TLSv1|NGX_SSL_TLSv1_1| <br>
> NGX_SSL_TLSv1_2)) {<br>
> +            /*<br>
> +            For TLS 1.3, It is optional to send Certificate Authorities in<br>
> +            Certificate Request Packet. RFC 8446#section-4.2.4<br>
> +            */<br>
> +            if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>
> +                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>
> +                            "no ssl_client_certificate for <br>
> ssl_verify_client");<br>
> +                return NGX_CONF_ERROR;<br>
> +            }<br>
>           }<br>
> <br>
> +<br>
>           if (ngx_ssl_client_certificate(cf, &conf->ssl,<br>
>                                          &conf->client_certificate,<br>
>                                          conf->verify_depth)<br>
> diff -r b5550a7f16c7 -r 199a35c74b60 src/stream/ngx_stream_ssl_module.c<br>
> --- a/src/stream/ngx_stream_ssl_module.c Fri Aug 09 19:12:26 2024 +0400<br>
> +++ b/src/stream/ngx_stream_ssl_module.c Sun Aug 11 13:05:27 2024 -0700<br>
> @@ -932,10 +932,16 @@<br>
> <br>
>       if (conf->verify) {<br>
> <br>
> -        if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>
> -            ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>
> -                          "no ssl_client_certificate for <br>
> ssl_verify_client");<br>
> -            return NGX_CONF_ERROR;<br>
> +        if (conf->protocols & (NGX_SSL_TLSv1|NGX_SSL_TLSv1_1| <br>
> NGX_SSL_TLSv1_2)) {<br>
> +            /*<br>
> +            For TLS 1.3, It is optional to send Certificate Authorities in<br>
> +            Certificate Request Packet. RFC 8446#section-4.2.4<br>
> +            */<br>
> +            if (conf->client_certificate.len == 0 && conf->verify != 3) {<br>
> +                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,<br>
> +                            "no ssl_client_certificate for <br>
> ssl_verify_client");<br>
> +                return NGX_CONF_ERROR;<br>
> +            }<br>
>           }<br>
> <br>
>           if (ngx_ssl_client_certificate(cf, &conf->ssl,<br>
> <br>
> _______________________________________________<br>
> nginx-devel mailing list<br>
> <a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a><br>
> <a href="https://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">https://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
_______________________________________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a><br>
<a href="https://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">https://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
</blockquote></div>
</blockquote></div>
</div>