[PATCH] (re-post) Add "optional_no_ca" option to ssl_verify_client to enable app-only CA chain validation

[Eric O'Connor]

It appears to be technically correct, but not semantically correct. I
would describe it as a nasty kludge.

I'm not sure why that would be preferable to a small ~3 line + a macro
patch that solves the issue and provides a nice interface. Error page
handling should be used for handling errors, not supporting
workarounds for a new set of technologies.

On Thu, Sep 27, 2012 at 10:05 AM, Mike Kazantsev <mk.fraggod at gmail.com> wrote:
> On Tue, 25 Sep 2012 08:39:28 +0400
> Maxim Dounin <mdounin at mdounin.ru> wrote:
>
>> Hello!
>>
>> On Sat, Sep 22, 2012 at 05:09:53PM +0600, Mike Kazantsev wrote:
>>
>> [...]
>>
>> > > 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.
>>
>> This actually counts as a separate problem.  I think it should be
>> possible to specify empty certificate authorities list to be sent
>> to clients even if nginx does actual verification.  Part of the
>> problem is addressed by the ssl_trusted_certificate directive which is
>> going to be introduced along with OCSP stapling patches.
>>
>
> Ok.
>
> Don't think finding any cert to put there should be a problem anyway.
>
>
>> >  - 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:
>> >
>
> ...
>
>> >
>> >    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.
>>
>> Works fine here:
>>
>>     ssl_verify_client on;
>>     ssl_client_certificate ca.crt;
>>
>>     error_page 495 = /foo;
>>
>>     location = /foo {
>>         return 200 "dn: $ssl_client_s_dn, verify: $ssl_client_verify\n";
>>     }
>>
>> $ openssl s_client -connect ... -cert ... -key ...
>> ...
>> GET / HTTP/1.0
>>
>> HTTP/1.1 200 OK
>> Server: nginx/1.3.6
>> Date: Tue, 25 Sep 2012 02:37:45 GMT
>> Content-Type: text/plain
>> Content-Length: 35
>> Connection: close
>>
>> dn: /CN=mdounin.ru, verify: FAILED
>>
>
> With a bit more universal catch-all config like this:
>
>   error_page 495 = @cert_err;
>   location @cert_err { ...
>
> It seem to work pretty much as desired, was some misconfiguration on my
> part before, I guess.
>
> Since it's possible to use and rewrite $uri in named locations, I don't
> see any obvious flaws in this approach, so I think "error_page" might
> make this patch redundant indeed.
>
> I'll try it out (and ping other users) and re-raise the issue with
> modified patch if there'll be any less obvious issues with it.
>
> Eric, does it work for your use-case as well?
>
>
>> >  - "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.
>> >
>> >
>
> ...
>
>> See below for some comments.
>
> I'll look into addressing these, thanks, though as noted, the patch
> itself doesn't seem to be necessary, since "error_page 495 ..." works.
>
>
> --
> Mike Kazantsev // fraggod.net