On Thu, Jun 05, 2008 at 07:25:30AM +0200, Kaspar Brand wrote:
> Joe Orton wrote:
> > http://svn.apache.org/viewvc?rev=662815&view=rev
> >
> > Changing the dirconf structure fields in-place seems ugly and may even
> > be thread-unsafe (not sure).
>
> Thanks for pointing this out, I wasn't aware of the danger of doing so.
> The same effect can be achieved with the attached, hopefully more
> acceptable patch, however (diff against the current code in trunk).
>
> > From a quick look I can't see how a reneg would be forced for any of:
> >
> > 1) SSLCipherSuite changed since original vhost
> > 2) SSLCACeritificate* changed since original vhost (where both
> > 3) SSLOCSP* changed since original vhost
>
> To better understand what your primary concern is - can we agree what
> specific case we're considering in this case? I see four of them, actually:
>
> (1) SNI client connecting to
> (a) the first, access-restricted vhost
> (b) one of the other, also access-restricted vhosts (two or above)
>
> (2) non-SNI client connecting to
> (a) the first, access-restricted vhost
> (b) one of the other, also access-restricted vhosts (two or above)
>
> The issues you raised all belong to (2b), is that correct? From my
> perspective, the patch is working correctly for (1a), (1b) and (2a), but
> the question is mainly how to handle (2b), i.e. properly enforcing
> access restrictions for "legacy" clients for vhosts two and above.
I'm concerned mostly about 1 (b) but I guess we'd have to consider an
access control bypass in any of these four cases to be a security issue
once we start claiming this as a supported configuration.
> > I would go through each of the config directive which
> > supports vhost context in turn. What about SSLCertificateChainFile?
> > What about CRLs? etc etc.
>
> If we agree that the remaining problem is about enforcing access
> control, then I would consider these directives relevant:
Access control is certainly the most important issue, but e.g. if
SSLCertificateChainFile is not supported properly for the named vhost
that's also a bug. Many configs depend on supplying the intermediate
certs.
> Directive accessed as... (assuming that mctx is
> pointing to current modssl_ctx_t)
>
> SSLCipherSuite mctx->auth.cipher_suite
> SSLVerifyDepth mctx->auth.verify_depth
> SSLVerifyClient mctx->auth.verify_mode
> SSLCACertificateFile mctx->auth.ca_cert_file
> SSLCACertificatePath mctx->auth.ca_cert_path
> SSLCARevocationFile mctx->crl_file
> SSLCARevocationPath mctx->crl_path
> SSLOCSDefaultResponder mctx->ocsp_responder
> SSLOCSPEnable mctx->ocsp_enabled
> SSLOCSPOverrideResponder mctx->ocsp_force_default
>
> Do you see others that should be added to this list?
>
> (SSLCertificateChain is not relevant when verifying peer certs, and
> apart from this, I didn't see any additional parameters in
> modssl_ctx_t/modssl_auth_ctx_t which are relevant for access control.)
I'm think it is relevant. AFAICT the SSL handshake which takes place in
the SNI case is going to be done using the settings from the SSL_CTX
from the original vhost not the named vhost, plus those explicitly
copied over after the SSL_set_SSL_CTX() call.
So *all* vhost-specific config settings which affect the handshake are
going to have to be set up manually in the SSL_CTX, otherwise the
configuration is not going to be applied correctly. That means in
addition:
SSLCertificateChainFile
SSLCADNRequest{File,Path}
SSLHonorCipherOrder
SSLProtocol
which I think is an exhaustive list. SSLProtocol and
SSLHonorCipherOrder may be "interesting" because the handshake has
already begun at the point it needs to be applied.
joe
