[PATCH] SSL: fix order of checks during SSL certificate verification
mdounin at mdounin.ru
Sun Aug 21 14:02:11 UTC 2016
On Thu, Aug 18, 2016 at 07:46:29PM -0700, Piotr Sikora wrote:
> Hey Maxim,
> > Calling SSL_get_peer_certificate() is only needed if
> > SSL_get_verify_result() returns X509_V_OK, to distinguish two of
> > its different meanings:
> > X509_V_OK
> > The verification succeeded or no peer certificate was presented.
> > And that's what nginx does.
> That's not what the documentation says, though...
The above quote is from the documentation.
> At this point, NGINX is just relying on a known bug, nothing else.
At this point nginx just uses the interface provided by the
OpenSSL library, exactly as documented. The fact that the
interface is flawed (and documented to be flawed to make sure
people will use it properly) has nothing to do with nginx use of
> > Ok, this looks like the real reason for the patch. This looks
> > like an API change in BoringSSL, and should be threated
> > accordingly.
> Nope, that's not the real reason... BoringSSL actually added a
> workaround for this , just to play nicely with NGINX and
> netty-tcnative, because virtually everything else is using OpenSSL API
> correctly, i.e. checking if peer sent the certificate before checking
> if that certificate passed verification.
> The real reason for this patch is that existing code is misusing OpenSSL API.
>  https://boringssl.googlesource.com/boringssl/+/37646838e9bb62a0d9d506b117193611c4c46012
So the change was inspired by an attempt to change the API in
BoringSSL. As for "misusing", see above.
> > (2) looks hardly portable (what if another
> > library will have different rc values? or will have more than one
> > error string to print?),
> Well, it's portable assuming that the library also converts the "rc"
> value back to the error string (like in the patchset I sent
Your patches assume that there are no conflicts between nginx
error codes (NGX_OK, NGX_DECLINED) and SSL_get_verify_result()
error codes. While this is currently true, this is not something
I would rely on, even assuming OpenSSL only.
More information about the nginx-devel