Welcome! Log In Create A New Profile


Re: [PATCH] SSL: consistent certificate verification depth

Maxim Dounin
July 23, 2022 05:54PM

On Thu, Jul 21, 2022 at 06:18:24PM +0400, Sergey Kandaurov wrote:

> > On 20 Jul 2022, at 22:04, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > On Wed, Jul 20, 2022 at 05:58:26PM +0400, Sergey Kandaurov wrote:
> >
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet@nginx.com>
> >> # Date 1658325446 -14400
> >> # Wed Jul 20 17:57:26 2022 +0400
> >> # Node ID 5a9cc2a846c9ea4c0af03109ab186af1ac28e222
> >> # Parent 069a4813e8d6d7ec662d282a10f5f7062ebd817f
> >> SSL: consistent certificate verification depth.
> >>
> >> Originally, certificate verification depth was used to limit the number
> >> of signatures to validate, that is, to limit chains with intermediate
> >> certificates one less. The semantics was changed in OpenSSL 1.1.0, and
> >> instead it limits now the number of intermediate certificates allowed.
> >> This makes it not possible to limit certificate checking to self-signed
> >> certificates with verify depth 0 in OpenSSL 1.1.0+, and is inconsistent
> >> compared with former behaviour in BoringSSL and older OpenSSL versions.
> >>
> >> This change restores former verification logic when using OpenSSL 1.1.0+.
> >> The verify callback is adjusted to emit the "certificate chain too long"
> >> error when the certificate chain exceeds the verification depth. It has
> >> no effect to other SSL libraries, where this is limited by other means.
> >> Also, this fixes verification checks when using LibreSSL 3.4.0+, where
> >> a chain depth is not limited except by X509_VERIFY_MAX_CHAIN_CERTS (32).
> >
> > This (highly questionable) OpenSSL behaviour seems to be status
> > quo for a while, also recorded in tests (see ssl_verify_depth.t).
> > Any specific reasons for the nginx-side workaround?
> As explained in the commit message, main motivation is to eliminate
> annoying difference in behaviour among various SSL libraries
> (aside from working around the arguably broken LibreSSL verifier).
> Nothing specific behind that.
> Disambiguating ssl_verify_depth.t is a good demo of net result.
> The downside is that this can potentially break previously working
> configurations when using with modern OpenSSL versions.
> So the patch is to seek feedback on whether this makes sense.

Apart from potentially breaking some existing configurations
(especially with LibreSSL), I have several basic concerns here:

1. As of now, it's OpenSSL responsibility to apply verify depth
limit, and "openssl verify -verify_depth <num>" can be used to
test the particular verification. Switching to our implementation
of the depth limit means that it will be our responsibility to
apply and document the limit.

2. The verify callback behaviour is complex enough, and I expect
portability issues.

3. Further, the main reason why the depth limit exists is to limit
maximum resource consumption of the whole certificate verification
process. Raising an error within the verify callback without
returning failure won't stop OpenSSL from checking additional
certificates, and hence is not going to limit the resource usage.
That is, such a limit would be misleading: it will prevent
certificates from being accepted, but won't stop DoS attacks.

I don't think that there is a way to implement the limit via
verify callback in a way which will prevent excessive signature
checks yet preserve nginx existing behaviour with not closing the
connection in case of verification failures.

Summing the above, I tend to think that it's not worth the effort.


Maxim Dounin
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH] SSL: consistent certificate verification depth

Sergey Kandaurov 467 July 20, 2022 10:00AM

Re: [PATCH] SSL: consistent certificate verification depth

Maxim Dounin 113 July 20, 2022 02:06PM

Re: [PATCH] SSL: consistent certificate verification depth

Sergey Kandaurov 134 July 21, 2022 10:20AM

Re: [PATCH] SSL: consistent certificate verification depth

Maxim Dounin 149 July 23, 2022 05:54PM

Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 262
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready