> -----Ursprüngliche Nachricht-----
> Von: Kaspar Brand
> Gesendet: Mittwoch, 22. April 2009 09:12
> An: dev@httpd.apache.org
> Betreff: Re: SNI in 2.2.x (Re: Time for 2.2.10?)
>
> 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.
IMHO this is a *huge* difference. If its optional I can decide whether to
present one or not, but if I present one it should be ensured that
the feedback whether it is valid or not is based on the correct CA.
>
> > 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).
As I said further down below I see also good and valid use cases for the
combination
SSLVerifyClient optional
and
%{SSL_CLIENT_VERIFY}
And this combination should be safe even if this comes at the price that
some configuration are not possible without SNI. But yes, we may disagree
on this :-).
IMHO the largest consumers of name based virtual hosts without SNI would be
people that simply want to setup a SSL based side without client certs.
> 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.)
This is exactly the intended configuration here:
Have a bunch of users that auth via cert and thus have the dummy password
setup and have other users with a real password that use basic auth and simply
do a "normal" authn and authz configuration
>
> > 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)?
Agreed. I would only love to see that the parts in your patch were you
used r->connection->aborted are adjusted accordingly.
The other locations of r->connection->aborted will be fixed in a separate
patch.
Hope to get to your patch until after the weekend.
Regards
Rüdiger
