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 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: 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.) Kaspar
Index: httpd-trunk/modules/ssl/ssl_engine_kernel.c =================================================================== --- httpd-trunk/modules/ssl/ssl_engine_kernel.c (revision 663447) +++ httpd-trunk/modules/ssl/ssl_engine_kernel.c (working copy) @@ -432,19 +432,16 @@ int ssl_hook_Access(request_rec *r) * currently active/remembered verify depth (because this means more * restriction on the certificate chain). */ - if ((sc->server->auth.verify_depth != UNSET) && - (dc->nVerifyDepth == UNSET)) { - /* apply per-vhost setting, if per-directory config is not set */ - dc->nVerifyDepth = sc->server->auth.verify_depth; - } - if (dc->nVerifyDepth != UNSET) { + if ((dc->nVerifyDepth != UNSET) || + (sc->server->auth.verify_depth != UNSET)) { /* XXX: doesnt look like sslconn->verify_depth is actually used */ if (!(n = sslconn->verify_depth)) { sslconn->verify_depth = n = sc->server->auth.verify_depth; } /* determine whether a renegotiation has to be forced */ - if (dc->nVerifyDepth < n) { + if ((dc->nVerifyDepth < n) || + (sc->server->auth.verify_depth < n)) { renegotiate = TRUE; ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "Reduced client verification depth will force " @@ -466,23 +463,22 @@ int ssl_hook_Access(request_rec *r) * verification but at least skip the I/O-intensive renegotation * handshake. */ - if ((sc->server->auth.verify_mode != SSL_CVERIFY_UNSET) && - (dc->nVerifyClient == SSL_CVERIFY_UNSET)) { - /* apply per-vhost setting, if per-directory config is not set */ - dc->nVerifyClient = sc->server->auth.verify_mode; - } - if (dc->nVerifyClient != SSL_CVERIFY_UNSET) { + if ((dc->nVerifyClient != SSL_CVERIFY_UNSET) || + (sc->server->auth.verify_mode != SSL_CVERIFY_UNSET)) { /* remember old state */ verify_old = SSL_get_verify_mode(ssl); /* configure new state */ verify = SSL_VERIFY_NONE; - if (dc->nVerifyClient == SSL_CVERIFY_REQUIRE) { + if ((dc->nVerifyClient == SSL_CVERIFY_REQUIRE) || + (sc->server->auth.verify_mode == SSL_CVERIFY_REQUIRE)) { verify |= SSL_VERIFY_PEER_STRICT; } if ((dc->nVerifyClient == SSL_CVERIFY_OPTIONAL) || - (dc->nVerifyClient == SSL_CVERIFY_OPTIONAL_NO_CA)) + (dc->nVerifyClient == SSL_CVERIFY_OPTIONAL_NO_CA) || + (sc->server->auth.verify_mode == SSL_CVERIFY_OPTIONAL) || + (sc->server->auth.verify_mode == SSL_CVERIFY_OPTIONAL_NO_CA)) { verify |= SSL_VERIFY_PEER; }