Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (nike.apache.org: local policy)
Message-ID: <484778CA.3020800@velox.ch>
Date: Thu, 05 Jun 2008 07:25:30 +0200
From: Kaspar Brand <httpd-dev.2008@velox.ch>
User-Agent: Thunderbird 2.0.0.14 (Windows/20080421)
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: SNI in 2.2.9? (Re: 2.2.9 status)
References: <019E26AF-FE2A-4B14-B058-FCE1E7FACD4A@jaguNET.com>
 <56E13120-955E-4027-8682-A8168684B348@jaguNET.com>
 <72E8FB73-F2E8-4A3B-86E7-DB19B12ADE13@jaguNET.com>
 <4845583F.5070808@velox.ch> <20080604140111.GA12050@redhat.com>
In-Reply-To: <20080604140111.GA12050@redhat.com>
Content-Type: multipart/mixed;
 boundary="------------060005040104010901000203"

This is a multi-part message in MIME format.
--------------060005040104010901000203
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

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

--------------060005040104010901000203
Content-Type: text/plain;
 name="sni_sslverifyclient-v3.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="sni_sslverifyclient-v3.diff"

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

--------------060005040104010901000203--