Hello!
On Thu, Sep 01, 2016 at 02:16:37PM -0700, Piotr Sikora wrote:
> 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.
You are misreading the BUGS section. It doesn't say that
SSL_get_peer_certificate() must be always called when
SSL_get_verify_result() is called. It says that SSL_get_verify_result() is
only useful in connection with SSL_get_peer_certificate(). More
specifically, this is needed to distinguish two of the different
meanings of X509_V_OK. And this is what nginx does.
> > 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.
The difference between ngx_ssl_error() and what you've suggested
is that ngx_ssl_error() doesn't try to cast errors to an nginx rc
value. Instead, it uses the error stack saved in the relevant
connection object.
> 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.
As previously suggested, it might be a good solution to use "peer", as
already used in serveral error messages in ngx_event_openssl.c
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
