Bringing it up again. Thanks for contributing client certificate validation
with OCSP
<https://mailman.nginx.org/pipermail/nginx-devel/2024-August/KAELLGI7MJSRYFCYINB7DZRFSNOM777K.html>.
We were waiting for this feature.
Kindly merge below fix as well. Also, let me know if we need to keep
check simple
as this:
*if (conf->client_certificate.len == 0 && conf->trusted_certificate.len ==
0 && conf->verify != 3) {*
*PATCH:*
# 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 Wed, Aug 21, 2024 at 9:36 AM Praveen Chaudhary <pclico...@gmail.com>
wrote:
> @a.bavs...@nginx.com <a.bavs...@nginx.com>
>
> Gentle Reminder for review. This feature to make ssl_client_certificate
> optional may help us here at Nvidia.
> Thanks in advance. Kindly let me know if any more modification is needed
> in fix.
>
> Note: AFAIK, mTLS was not supported with SSLv2. I kept the NGX_SSL_SSLv2
> flag in fix, if Nginx today supports SSLv2.
>
> On Mon, Aug 19, 2024 at 4:22 PM Praveen Chaudhary <pclico...@gmail.com>
> wrote:
>
>> 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
