httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From traw...@apache.org
Subject svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c
Date Tue, 12 May 2015 18:59:30 GMT
Author: trawick
Date: Tue May 12 18:59:29 2015
New Revision: 1679032

URL: http://svn.apache.org/r1679032
Log:
mod_ssl OCSP Stapling: Don't block initial handshakes while refreshing
the OCSP response for a different certificate.  mod_ssl has an additional
global mutex, "ssl-stapling-refresh".

Not mentioned in CHANGES:

Stapling no longer uses a mutex when using a stapling cache
implementation which doesn't require it.  (A further, unrelated
code change to mod_ssl is required to allow the use of memcache 
as a stapling cachek, and I haven't tested with distcache; thus
it isn't clear if this helps in practice yet.)

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml
    httpd/httpd/trunk/modules/ssl/mod_ssl.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
    httpd/httpd/trunk/modules/ssl/ssl_private.h
    httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1679032&r1=1679031&r2=1679032&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue May 12 18:59:29 2015
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_ssl OCSP Stapling: Don't block initial handshakes while refreshing
+     the OCSP response for a different certificate.  mod_ssl has an additional
+     global mutex, "ssl-stapling-refresh".
+     [Jeff Trawick]
+
   *) http: Don't remove the Content-Length of zero from a HEAD response if
      it comes from an origin server, module or script.  [Yann Ylavic]
 

Modified: httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml?rev=1679032&r1=1679031&r2=1679032&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml Tue May 12 18:59:29 2015
@@ -2371,6 +2371,14 @@ stated goal of "saving roundtrips and re
 <a href="http://www.ietf.org/rfc/rfc6961.txt">RFC 6961</a>
 (TLS Multiple Certificate Status Extension).
 </p>
+
+<p>When OCSP stapling is enabled, the <code>ssl-stapling</code> mutex is
used
+to control access to the OCSP stapling cache in order to prevent corruption,
+and the <code>sss-stapling-refresh</code> mutex is used to control refreshes
+of OCSP responses.  These mutexes can be configured using the
+<directive module="core">Mutex</directive> directive.
+</p>
+
 </usage>
 </directivesynopsis>
 
@@ -2388,10 +2396,6 @@ is enabled. Configuration of a cache is
 With the exception of <code>none</code> and <code>nonenotnull</code>,
 the same storage types are supported as with
 <directive module="mod_ssl">SSLSessionCache</directive>.</p>
-
-<p>The <code>ssl-stapling</code> mutex is used to serialize access to the
-OCSP stapling cache to prevent corruption.  This mutex can be configured
-using the <directive module="core">Mutex</directive> directive.</p>
 </usage>
 </directivesynopsis>
 

Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=1679032&r1=1679031&r2=1679032&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
+++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Tue May 12 18:59:29 2015
@@ -380,7 +380,10 @@ static int ssl_hook_pre_config(apr_pool_
     /* Register mutex type names so they can be configured with Mutex */
     ap_mutex_register(pconf, SSL_CACHE_MUTEX_TYPE, NULL, APR_LOCK_DEFAULT, 0);
 #ifdef HAVE_OCSP_STAPLING
-    ap_mutex_register(pconf, SSL_STAPLING_MUTEX_TYPE, NULL, APR_LOCK_DEFAULT, 0);
+    ap_mutex_register(pconf, SSL_STAPLING_CACHE_MUTEX_TYPE, NULL,
+                      APR_LOCK_DEFAULT, 0);
+    ap_mutex_register(pconf, SSL_STAPLING_REFRESH_MUTEX_TYPE, NULL,
+                      APR_LOCK_DEFAULT, 0);
 #endif
 
     return OK;

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1679032&r1=1679031&r2=1679032&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Tue May 12 18:59:29 2015
@@ -71,7 +71,8 @@ SSLModConfigRec *ssl_config_global_creat
 #endif
 #ifdef HAVE_OCSP_STAPLING
     mc->stapling_cache         = NULL;
-    mc->stapling_mutex         = NULL;
+    mc->stapling_cache_mutex   = NULL;
+    mc->stapling_refresh_mutex = NULL;
 #endif
 
     apr_pool_userdata_set(mc, SSL_MOD_CONFIG_KEY,

Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1679032&r1=1679031&r2=1679032&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_private.h Tue May 12 18:59:29 2015
@@ -505,7 +505,8 @@ typedef struct {
 #ifdef HAVE_OCSP_STAPLING
     const ap_socache_provider_t *stapling_cache;
     ap_socache_instance_t *stapling_cache_context;
-    apr_global_mutex_t   *stapling_mutex;
+    apr_global_mutex_t   *stapling_cache_mutex;
+    apr_global_mutex_t   *stapling_refresh_mutex;
 #endif
 } SSLModConfigRec;
 
@@ -896,7 +897,8 @@ int          ssl_stapling_mutex_reinit(s
 
 /* mutex type names for Mutex directive */
 #define SSL_CACHE_MUTEX_TYPE    "ssl-cache"
-#define SSL_STAPLING_MUTEX_TYPE "ssl-stapling"
+#define SSL_STAPLING_CACHE_MUTEX_TYPE "ssl-stapling"
+#define SSL_STAPLING_REFRESH_MUTEX_TYPE "ssl-stapling-refresh"
 
 apr_status_t ssl_die(server_rec *);
 

Modified: httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c?rev=1679032&r1=1679031&r2=1679032&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_util_stapling.c Tue May 12 18:59:29 2015
@@ -34,6 +34,9 @@
 
 #ifdef HAVE_OCSP_STAPLING
 
+static int stapling_cache_mutex_on(server_rec *s);
+static int stapling_cache_mutex_off(server_rec *s);
+
 /**
  * Maxiumum OCSP stapling response size. This should be the response for a
  * single certificate and will typically include the responder certificate chain
@@ -247,9 +250,13 @@ static BOOL stapling_cache_response(serv
 
     i2d_OCSP_RESPONSE(rsp, &p);
 
+    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
+        stapling_cache_mutex_on(s);
     rv = mc->stapling_cache->store(mc->stapling_cache_context, s,
                                    cinf->idx, sizeof(cinf->idx),
                                    expiry, resp_der, stored_len, pool);
+    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
+        stapling_cache_mutex_off(s);
     if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01929)
                      "stapling_cache_response: OCSP response session store error!");
@@ -270,9 +277,13 @@ static BOOL stapling_get_cached_response
     const unsigned char *p;
     unsigned int resp_derlen = MAX_STAPLING_DER;
 
+    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
+        stapling_cache_mutex_on(s);
     rv = mc->stapling_cache->retrieve(mc->stapling_cache_context, s,
                                       cinf->idx, sizeof(cinf->idx),
                                       resp_der, &resp_derlen, pool);
+    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
+        stapling_cache_mutex_off(s);
     if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01930)
                      "stapling_get_cached_response: cache miss");
@@ -506,7 +517,7 @@ err:
 }
 
 /*
- * SSLStaplingMutex operations. Similar to SSL mutex except a mutex is
+ * SSL stapling mutex operations. Similar to SSL mutex except mutexes are
  * mandatory if stapling is enabled.
  */
 static int ssl_stapling_mutex_init(server_rec *s, apr_pool_t *p)
@@ -515,12 +526,23 @@ static int ssl_stapling_mutex_init(serve
     SSLSrvConfigRec *sc = mySrvConfig(s);
     apr_status_t rv;
 
-    if (mc->stapling_mutex || sc->server->stapling_enabled != TRUE) {
+    /* already init or stapling not enabled? */
+    if (mc->stapling_refresh_mutex || sc->server->stapling_enabled != TRUE) {
         return TRUE;
     }
 
-    if ((rv = ap_global_mutex_create(&mc->stapling_mutex, NULL,
-                                     SSL_STAPLING_MUTEX_TYPE, NULL, s,
+    /* need a cache mutex? */
+    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE) {
+        if ((rv = ap_global_mutex_create(&mc->stapling_cache_mutex, NULL,
+                                         SSL_STAPLING_CACHE_MUTEX_TYPE, NULL, s,
+                                         s->process->pool, 0)) != APR_SUCCESS) {
+            return FALSE;
+        }
+    }
+
+    /* always need stapling_refresh_mutex */
+    if ((rv = ap_global_mutex_create(&mc->stapling_refresh_mutex, NULL,
+                                     SSL_STAPLING_REFRESH_MUTEX_TYPE, NULL, s,
                                      s->process->pool, 0)) != APR_SUCCESS) {
         return FALSE;
     }
@@ -528,65 +550,157 @@ static int ssl_stapling_mutex_init(serve
     return TRUE;
 }
 
-int ssl_stapling_mutex_reinit(server_rec *s, apr_pool_t *p)
+static int stapling_mutex_reinit_helper(server_rec *s, apr_pool_t *p, 
+                                        apr_global_mutex_t **mutex,
+                                        const char *type)
 {
-    SSLModConfigRec *mc = myModConfig(s);
     apr_status_t rv;
     const char *lockfile;
 
-    if (mc->stapling_mutex == NULL) {
-        return TRUE;
-    }
-
-    lockfile = apr_global_mutex_lockfile(mc->stapling_mutex);
-    if ((rv = apr_global_mutex_child_init(&mc->stapling_mutex,
+    lockfile = apr_global_mutex_lockfile(*mutex);
+    if ((rv = apr_global_mutex_child_init(mutex,
                                           lockfile, p)) != APR_SUCCESS) {
         if (lockfile) {
             ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO(01946)
                          "Cannot reinit %s mutex with file `%s'",
-                         SSL_STAPLING_MUTEX_TYPE, lockfile);
+                         type, lockfile);
         }
         else {
             ap_log_error(APLOG_MARK, APLOG_WARNING, rv, s, APLOGNO(01947)
-                         "Cannot reinit %s mutex", SSL_STAPLING_MUTEX_TYPE);
+                         "Cannot reinit %s mutex", type);
         }
         return FALSE;
     }
     return TRUE;
 }
 
-static int stapling_mutex_on(server_rec *s)
+int ssl_stapling_mutex_reinit(server_rec *s, apr_pool_t *p)
 {
     SSLModConfigRec *mc = myModConfig(s);
+
+    if (mc->stapling_cache_mutex != NULL
+        && stapling_mutex_reinit_helper(s, p, &mc->stapling_cache_mutex,
+                                        SSL_STAPLING_CACHE_MUTEX_TYPE) == FALSE) {
+        return FALSE;
+    }
+
+    if (mc->stapling_refresh_mutex != NULL
+        && stapling_mutex_reinit_helper(s, p, &mc->stapling_refresh_mutex,
+                                        SSL_STAPLING_REFRESH_MUTEX_TYPE) == FALSE) {
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+static int stapling_mutex_on(server_rec *s, apr_global_mutex_t *mutex,
+                             const char *name)
+{
     apr_status_t rv;
 
-    if ((rv = apr_global_mutex_lock(mc->stapling_mutex)) != APR_SUCCESS) {
+    if ((rv = apr_global_mutex_lock(mutex)) != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_WARNING, rv, s, APLOGNO(01948)
-                     "Failed to acquire OCSP stapling lock");
+                     "Failed to acquire OCSP %s lock", name);
         return FALSE;
     }
     return TRUE;
 }
 
-static int stapling_mutex_off(server_rec *s)
+static int stapling_mutex_off(server_rec *s, apr_global_mutex_t *mutex,
+                              const char *name)
 {
-    SSLModConfigRec *mc = myModConfig(s);
     apr_status_t rv;
 
-    if ((rv = apr_global_mutex_unlock(mc->stapling_mutex)) != APR_SUCCESS) {
+    if ((rv = apr_global_mutex_unlock(mutex)) != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_WARNING, rv, s, APLOGNO(01949)
-                     "Failed to release OCSP stapling lock");
+                     "Failed to release OCSP %s lock", name);
         return FALSE;
     }
     return TRUE;
 }
 
+static int stapling_cache_mutex_on(server_rec *s)
+{
+    SSLModConfigRec *mc = myModConfig(s);
+
+    return stapling_mutex_on(s, mc->stapling_cache_mutex,
+                             SSL_STAPLING_CACHE_MUTEX_TYPE);
+}
+
+static int stapling_cache_mutex_off(server_rec *s)
+{
+    SSLModConfigRec *mc = myModConfig(s);
+
+    return stapling_mutex_off(s, mc->stapling_cache_mutex,
+                              SSL_STAPLING_CACHE_MUTEX_TYPE);
+}
+
+static int stapling_refresh_mutex_on(server_rec *s)
+{
+    SSLModConfigRec *mc = myModConfig(s);
+
+    return stapling_mutex_on(s, mc->stapling_refresh_mutex,
+                             SSL_STAPLING_REFRESH_MUTEX_TYPE);
+}
+
+static int stapling_refresh_mutex_off(server_rec *s)
+{
+    SSLModConfigRec *mc = myModConfig(s);
+
+    return stapling_mutex_off(s, mc->stapling_refresh_mutex,
+                              SSL_STAPLING_REFRESH_MUTEX_TYPE);
+}
+
+static int get_and_check_cached_response(server_rec *s, modssl_ctx_t *mctx,
+                                         OCSP_RESPONSE **rsp, BOOL *ok,
+                                         certinfo *cinf, apr_pool_t *p)
+{
+    int rv;
+
+    /* Check to see if we already have a response for this certificate */
+    rv = stapling_get_cached_response(s, rsp, ok, cinf, p);
+    if (rv == FALSE) {
+        return SSL_TLSEXT_ERR_ALERT_FATAL;
+    }
+
+    if (*rsp) {
+        /* see if response is acceptable */
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01953)
+                     "stapling_cb: retrieved cached response");
+        rv = stapling_check_response(s, mctx, cinf, *rsp, NULL);
+        if (rv == SSL_TLSEXT_ERR_ALERT_FATAL) {
+            OCSP_RESPONSE_free(*rsp);
+            return SSL_TLSEXT_ERR_ALERT_FATAL;
+        }
+        else if (rv == SSL_TLSEXT_ERR_NOACK) {
+            /* Error in response. If this error was not present when it was
+             * stored (i.e. response no longer valid) then it can be
+             * renewed straight away.
+             *
+             * If the error *was* present at the time it was stored then we
+             * don't renew the response straight away; we just wait for the
+             * cached response to expire.
+             */
+            if (ok) {
+                OCSP_RESPONSE_free(*rsp);
+                *rsp = NULL;
+            }
+            else if (!mctx->stapling_return_errors) {
+                OCSP_RESPONSE_free(*rsp);
+                return SSL_TLSEXT_ERR_NOACK;
+            }
+        }
+    }
+    return 0;
+}
+
 /* Certificate Status callback. This is called when a client includes a
  * certificate status request extension.
  *
  * Check for cached responses in session cache. If valid send back to
- * client.  If absent or no longer valid query responder and update
- * cache. */
+ * client.  If absent or no longer valid, query responder and update
+ * cache.
+ */
 static int stapling_cb(SSL *ssl, void *arg)
 {
     conn_rec *conn      = (conn_rec *)SSL_get_app_data(ssl);
@@ -616,59 +730,51 @@ static int stapling_cb(SSL *ssl, void *a
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01952)
                  "stapling_cb: retrieved cached certificate data");
 
-    /* Check to see if we already have a response for this certificate */
-    stapling_mutex_on(s);
-
-    rv = stapling_get_cached_response(s, &rsp, &ok, cinf, conn->pool);
-    if (rv == FALSE) {
-        stapling_mutex_off(s);
-        return SSL_TLSEXT_ERR_ALERT_FATAL;
-    }
-
-    if (rsp) {
-        /* see if response is acceptable */
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01953)
-                     "stapling_cb: retrieved cached response");
-        rv = stapling_check_response(s, mctx, cinf, rsp, NULL);
-        if (rv == SSL_TLSEXT_ERR_ALERT_FATAL) {
-            OCSP_RESPONSE_free(rsp);
-            stapling_mutex_off(s);
-            return SSL_TLSEXT_ERR_ALERT_FATAL;
-        }
-        else if (rv == SSL_TLSEXT_ERR_NOACK) {
-            /* Error in response. If this error was not present when it was
-             * stored (i.e. response no longer valid) then it can be
-             * renewed straight away.
-             *
-             * If the error *was* present at the time it was stored then we
-             * don't renew the response straight away we just wait for the
-             * cached response to expire.
-             */
-            if (ok) {
-                OCSP_RESPONSE_free(rsp);
-                rsp = NULL;
-            }
-            else if (!mctx->stapling_return_errors) {
-                OCSP_RESPONSE_free(rsp);
-                stapling_mutex_off(s);
-                return SSL_TLSEXT_ERR_NOACK;
-            }
-        }
+    rv = get_and_check_cached_response(s, mctx, &rsp, &ok, cinf, conn->pool);
+    if (rv != 0) {
+        return rv;
     }
 
     if (rsp == NULL) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01954)
                      "stapling_cb: renewing cached response");
-        rv = stapling_renew_response(s, mctx, ssl, cinf, &rsp, conn->pool);
-
-        if (rv == FALSE) {
-            stapling_mutex_off(s);
-            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01955)
-                         "stapling_cb: fatal error");
-            return SSL_TLSEXT_ERR_ALERT_FATAL;
+        stapling_refresh_mutex_on(s);
+        /* Maybe another request refreshed the OCSP response while this
+         * thread waited for the mutex.  Check again.
+         */
+        rv = get_and_check_cached_response(s, mctx, &rsp, &ok, cinf,
+                                           conn->pool);
+        if (rv != 0) {
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                         "stapling_cb: error checking for cached response "
+                         "after obtaining refresh mutex");
+            stapling_refresh_mutex_off(s);
+            return rv;
+        }
+        else if (rsp) {
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                         "stapling_cb: don't need to refresh cached response "
+                         "after obtaining refresh mutex");
+            stapling_refresh_mutex_off(s);
+        }
+        else {
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                         "stapling_cb: still must refresh cached response "
+                         "after obtaining refresh mutex");
+            rv = stapling_renew_response(s, mctx, ssl, cinf, &rsp, conn->pool);
+            stapling_refresh_mutex_off(s);
+
+            if (rv == TRUE) {
+                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                             "stapling_cb: success renewing response");
+            }
+            else {
+                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01955)
+                             "stapling_cb: fatal error renewing response");
+                return SSL_TLSEXT_ERR_ALERT_FATAL;
+            }
         }
     }
-    stapling_mutex_off(s);
 
     if (rsp) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01956)



Mime
View raw message