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 <praveen5582@gmail.com
> <mailto:praveen5582@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