[PATCH] (re-post) Add "optional_no_ca" option to ssl_verify_client to enable app-only CA chain validation
Eric O'Connor
eoconnor at coincident.com
Sun Sep 23 01:38:28 UTC 2012
Ah, I didn't know this was previously suggested on the forums. Also,
thanks for looking into the error 495 issue. I looked at it briefly
before deciding that we would rather maintain a 3 line patch than
attempt to hijack an HTTP error code.
Nice work. I had assumed that anyone using this sort of feature would
use an existing SSL library to fully verify the passed certificate,
but blocking totally erroneous/expired/revoked certificates can't
hurt.
Also, I'm happy that there are at least 3 people who think this is
useful, and a few technologies that rely on it.
~ Eric O'Connor
On Sat, Sep 22, 2012 at 7:09 AM, Mike Kazantsev <mk.fraggod at gmail.com> wrote:
>
> Apologies for a bit late answer, managed to miss (or forgot to check
> option for) thread update notification from a forum, my bad.
>
>
> On Tue, 18 Sep 2012 11:43:55 +0400
> Maxim Dounin <mdounin-ux0u7sevaKovJsYlp49lxw at public.gmane.org> wrote:
>
>> Hello!
>>
>> On Sat, Sep 15, 2012 at 07:52:30AM -0400, mk.fg wrote:
>>
>> > Re-post of patch from
>> > http://forum.nginx.org/read.php?2,228761,229586#msg-229586
> ...
>>
>> You may want to join discussion here, about the similar patch
>> submitted:
>>
>> http://mailman.nginx.org/pipermail/nginx-devel/2012-August/002643.html
>>
>
> I hope you don't mind if I'll continue it in this thread, adding Cc to
> Eric.
>
> After my latest failure to get forum notification, and because I've
> managed to find a gmane archive for this list (don't think I've seen it
> there in the not-so-distant past), I'd prefer to use email, but alas,
> indicated thread was rotated-out from even latest-300 on gmane, so I
> can't easily find original message to reply to.
>
>
>> In particular, I would like someone to actually test if the
>> error_page 495 aproach works instead as suggested here:
>>
>> http://mailman.nginx.org/pipermail/nginx-devel/2012-August/002650.html
>>
>
> It doesn't seem to be of much use in current state, problems:
>
> - Requires "ssl_client_certificate" to be set to some valid
> certificate, which then shouldn't actually never be used in this
> case.
>
> - In case of "ssl_verify_client" set to "on" or "optional", setting
> "error_page 400 495 496 =200 /altpath;" doesn't seem to stop nginx
> from returning "HTTP/1.1 400 Bad Request" response with "400 The SSL
> certificate error" in html body.
>
> This code in ngx_http_request.c is probable cause of that:
>
> if (sscf->verify) {
> rc = SSL_get_verify_result(c->ssl->connection);
>
> if (rc != X509_V_OK) {
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
> "client SSL certificate verify error: (%l:%s)",
> rc, X509_verify_cert_error_string(rc));
>
> ngx_ssl_remove_cached_session(sscf->ssl.ctx,
> (SSL_get0_session(c->ssl->connection)));
>
> ngx_http_finalize_request(r, NGX_HTTPS_CERT_ERROR);
> return;
> }
>
> I'm still unsure if it's a general bug in ssl module or some error
> in my configuration above, because docs clearly state that error
> should be handled by error_page and it doesn't seem to be.
>
> - "ssl_verify_client off;" isn't much useful, because it doesn't return
> clent certificate and doesn't check it in any way.
>
> - "ssl_verify_client on;", still gives all-or-nothing check, though
> I see that it's what might indeed be desirable, as Eric indicated.
>
> - I think it's really non-obvious way to do it.
>
>
>> And a quick comment for your patch: I tend to think that
>> introduction of ngx_http_ssl_variable_get_client_verify() is
>> misleading. We shouldn't try to claim the certificate was
>> verified unless it actually was.
>
> It might be misleading, indeed, I see your point.
> Attached patch doesn't try to alter ssl_client_verify result.
>
> On a completely unrelated note - nginx fails to build from svn here due
> to 'Exception in thread "main" java.lang.NoClassDefFoundError:
> com/pault/StyleSheet' (so I just disabled building changelog for tests).
>
> URL for the revised patch: https://gist.github.com/3319062
>
>
> Inline patch follows.
>
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> index cd6d885..97da051 100644
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -141,6 +141,14 @@ ngx_int_t ngx_ssl_get_client_verify(ngx_connection_t *c, ngx_pool_t *pool,
> ngx_str_t *s);
>
>
> +#define ngx_ssl_verify_error_is_optional(errnum) \
> + ((errnum == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) \
> + || (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN) \
> + || (errnum == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY) \
> + || (errnum == X509_V_ERR_CERT_UNTRUSTED) \
> + || (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))
> +
> +
> ngx_int_t ngx_ssl_handshake(ngx_connection_t *c);
> ssize_t ngx_ssl_recv(ngx_connection_t *c, u_char *buf, size_t size);
> ssize_t ngx_ssl_write(ngx_connection_t *c, u_char *data, size_t size);
> diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
> index d759489..ab91670 100644
> --- a/src/http/modules/ngx_http_ssl_module.c
> +++ b/src/http/modules/ngx_http_ssl_module.c
> @@ -48,6 +48,7 @@ static ngx_conf_enum_t ngx_http_ssl_verify[] = {
> { ngx_string("off"), 0 },
> { ngx_string("on"), 1 },
> { ngx_string("optional"), 2 },
> + { ngx_string("optional_no_ca"), 3 },
> { ngx_null_string, 0 }
> };
>
> @@ -466,7 +467,7 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child)
>
> if (conf->verify) {
>
> - if (conf->client_certificate.len == 0) {
> + if (conf->verify != 3 && conf->client_certificate.len == 0) {
> ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
> "no ssl_client_certificate for ssl_client_verify");
> return NGX_CONF_ERROR;
> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> index cb970c5..96cec55 100644
> --- a/src/http/ngx_http_request.c
> +++ b/src/http/ngx_http_request.c
> @@ -1642,7 +1642,9 @@ ngx_http_process_request(ngx_http_request_t *r)
> if (sscf->verify) {
> rc = SSL_get_verify_result(c->ssl->connection);
>
> - if (rc != X509_V_OK) {
> + if ((sscf->verify != 3 && rc != X509_V_OK)
> + || !(sscf->verify == 3 && ngx_ssl_verify_error_is_optional(rc)))
> + {
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
> "client SSL certificate verify error: (%l:%s)",
> rc, X509_verify_cert_error_string(rc));
> --
> 1.7.12
>
>
> --
> Mike Kazantsev // fraggod.net
More information about the nginx-devel
mailing list