Ruediger Pluem wrote:
> the next configuration *can* do security harm:
>
> <VirtualHost foo.example.com:443>
> SSLVerifyClient optional
> SSLCACertificateFile foo-clientauth-bundle.pem
> </VirtualHost>
>
> <VirtualHost bar.example.com:443>
> SSLVerifyClient optional
> SSLCACertificateFile bar-clientauth-bundle.pem
> </VirtualHost>
>
> This would cause client certificates signed by foo-clientauth-bundle
> accepted by the virtual host bar.example.com.
That's true, but I think it's where we disagree about the meaning of
"SSLVerifyClient optional", actually: if it's optional, then a client is
free to not present any client certificate - which, IMO, is not really
different from presenting a client certificate issued by a CA not
included in the respective CA bundle.
> Even if SSLVerifyClient is optional later processing steps such as
> RewriteRules or applications may evaluate the environment variables set
> by SSL and think that a successful authentication took place where
> in fact it has not.
In my opinion, depending on %{SSL_CLIENT_VERIFY} only makes sense in
combination with "SSLVerifyClient require" (either clientauth is
required, or it's not - this is also what the last paragraph in the
documentation abouut SSLVerifyClient says, effectively).
What's still possible, though (as some sort of "workaround"): checking
both %{SSL_CLIENT_VERIFY} and %{SSL_CLIENT_I_DN*} variables in an
SSLRequire statement, to make sure that even if a non-SNI client
connecting to a non-default vhost with "SSLVerifyClient optional"
presents a certificate of one of the CAs we "require" for this vhost.
> The Fakeauth setting above is a perfect example why an admin might
> set optional:
>
> Either the user has a cert or it has to authenticate via username and
> password.
Basically correct, though browsers sometimes show "surprising" behavior
when encountering a configuration like this. (The FakeBasicAuth option
is of no use when the client does not present a cert, though - you would
need to combine "SSLVerifyClient optional" with one of the Auth*
directives to handle this case.)
> It took me some time, but I think I got it. So we either need to
> end the keepalive request or we need to restore the old verify setting
> via
>
> modssl_set_verify(ssl, verify_old, ssl_callback_SSLVerify);
>
> correct?
Yes, actually I had a similar solution in place, when fixing this issue
first. But then I compared to what is done when one of the renegotiation
steps failed, and changed to setting r->connection->aborted ;-)
Instead of setting ssl_callback_SSLVerify again, you can also use NULL,
btw (this will leave the current callback unchanged).
> r->connection->aborted should *only* be set if the client did a network
> disconnect
> *never* if the server decides to shutdown the connection.
I also found it pretty rude, but as it was used in other places in that
same function later on, I was assuming that it would be the right thing
to do.
> In this case we should set
>
> r->connection->keepalive = AP_CONN_CLOSE
>
> But as said this IMHO needs fixing in the code further down below as well.
May I suggest that this is dealt with in a separate patch (not the one
for enabling access to the non-default vhost[s] for non-SNI clients)?
> Further comments might follow when I have time to review v8. Stay tuned :-).
Thanks for your time, again!
Kaspar
