Thanks Aleksei for the review.
Agree, It makes sense to have explicit error message to require either
ssl_client_certificate or ssl_trusted_certificate. Because:
Nginx prints error number from SSL to identify SSL error\routine,
but for a client or admin, it may be still hard to find why "SSL
certificate error"
is seen.
V2 Patch.
[Keeping same Subject]
# HG changeset patch
# User Praveen Chaudhary <praveen5...@gmail.com>
# Date 1723406727 25200
# Sun Aug 11 13:05:27 2024 -0700
# Node ID a5525b8eac0e1f10da42b7367cd5296fb29f4787
# Parent 8796dfbe7177cb0be2a53bcdb4d25cc64a58d2a7
Make ssl_client_certificate directive optional with TLSv1.1+.
- With TLS 1.1+, Certificate Authorities(CAs) are optional
in the Certificate Request packet. This makes directive
ssl_client_certificate also optional for mutual TLS
configurations.
- For TLS 1.1, check either ssl_client_certificate or
ssl_trusted_certificate is non empty.
diff -r 8796dfbe7177 -r a5525b8eac0e src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c Mon Aug 12 18:21:01 2024 +0400
+++ b/src/http/modules/ngx_http_ssl_module.c Sun Aug 11 13:05:27 2024 -0700
@@ -788,9 +788,20 @@
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_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1)) {
+ ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+ "no ssl_client_certificate for
ssl_verify_client");
+ return NGX_CONF_ERROR;
+ }
+ /* For TLS 1.1+. */
+ if (conf->trusted_certificate.len == 0) {
+ ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+ "no ssl_client_certificate or "
+ "ssl_trusted_certificate for
ssl_verify_client");
+ return NGX_CONF_ERROR;
+ }
}
if (ngx_ssl_client_certificate(cf, &conf->ssl,
diff -r 8796dfbe7177 -r a5525b8eac0e src/mail/ngx_mail_ssl_module.c
--- a/src/mail/ngx_mail_ssl_module.c Mon Aug 12 18:21:01 2024 +0400
+++ b/src/mail/ngx_mail_ssl_module.c Sun Aug 11 13:05:27 2024 -0700
@@ -451,9 +451,20 @@
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_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1)) {
+ ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+ "no ssl_client_certificate for
ssl_verify_client");
+ return NGX_CONF_ERROR;
+ }
+ /* For TLS 1.1+. */
+ if (conf->trusted_certificate.len == 0) {
+ ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+ "no ssl_client_certificate or "
+ "ssl_trusted_certificate for
ssl_verify_client");
+ return NGX_CONF_ERROR;
+ }
}
if (ngx_ssl_client_certificate(cf, &conf->ssl,
diff -r 8796dfbe7177 -r a5525b8eac0e src/stream/ngx_stream_ssl_module.c
--- a/src/stream/ngx_stream_ssl_module.c Mon Aug 12 18:21:01 2024 +0400
+++ b/src/stream/ngx_stream_ssl_module.c Sun Aug 11 13:05:27 2024 -0700
@@ -933,9 +933,20 @@
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_SSLv2|NGX_SSL_SSLv3|NGX_SSL_TLSv1)) {
+ ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+ "no ssl_client_certificate for
ssl_verify_client");
+ return NGX_CONF_ERROR;
+ }
+ /* For TLS 1.1+. */
+ if (conf->trusted_certificate.len == 0) {
+ ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+ "no ssl_client_certificate or "
+ "ssl_trusted_certificate for
ssl_verify_client");
+ return NGX_CONF_ERROR;
+ }
}
if (ngx_ssl_client_certificate(cf, &conf->ssl,
On Mon, Aug 19, 2024 at 11:41 AM Aleksei Bavshin <a.bavs...@nginx.com>
wrote:
> 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
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
