httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Plüm, Rüdiger, VF-Group" <ruediger.pl...@vodafone.com>
Subject Re: SNI in 2.2.x (Re: Time for 2.2.10?)
Date Thu, 23 Apr 2009 14:55:04 GMT
 

> -----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

Mime
View raw message