[PATCH] SSL: fix order of checks during SSL certificate verification

[Piotr Sikora]

Hey Maxim,

> I don't understand why you think that nginx doesn't use it
> properly.  If you still think nginx "doesn't use it properly",
> please elaborate.

Per BUGS section:

    SSL_get_verify_result() is only useful in connection with
SSL_get_peer_certificate.

But the code you pasted, i.e.:

    if (SSL_get_verify_result(c->ssl->connection) != X509_V_OK) {
        ngx_str_set(s, "FAILED");
        return NGX_OK;
    }

    cert = SSL_get_peer_certificate(c->ssl->connection);

uses result of SSL_get_verify_result() without ever calling
SSL_get_peer_ceritficate(), which is what the BUGS section warns
against.

> The "one of the solutions you suggested" claim isn't really true.
> I never suggested such a solution.  Quoting myself,
> http://mailman.nginx.org/pipermail/nginx-devel/2016-August/008680.html:
>
> : I can't say I like either of the variants.  (1) will require
> : memory allocations, (2) looks hardly portable (what if another
> : library will have different rc values? or will have more than one
> : error string to print?), and (3) looks strange.
>
> The (2) here corresponds to the variant in question you suggested.

I was referring to:

    Alternatively,
    we can consider abstracting printing of verification results
    errors with something similar to ngx_ssl_error().

which is basically (2), unless I've misunderstood you.

But that's not really important... what's important is which approach
would be acceptable for your? Because the only reason for the change
in previous patch was the fact that you didn't like my original
version, which printed "client" and "upstream" in ngx_ssl_openssl.c.

Best regards,
Piotr Sikora