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