Changeset d6834e0 in mod_gnutls

Timestamp:

Jun 10, 2016, 9:34:08 AM (6 years ago)

Author:

Thomas Klute <thomas2.klute@…>

Branches:

asyncio, debian/master, debian/stretch-backports, master, proxy-ticket, upstream

Children:

15b22cb

Parents:

aa68232

git-author:

Thomas Klute <thomas2.klute@…> (06/10/16 09:29:57)

git-committer:

Thomas Klute <thomas2.klute@…> (06/10/16 09:34:08)

Message:

OCSP refresh mutex: Prevent parallel requests

Add a global mutex which a thread must hold before updating a cached
OCSP response. This avoids two threads updating the same response in
parallel. The impact of parallel updates may be small with the
experimental file-based mechanism, but an extra OCSP request over HTTP
would add a lot of overhead.

Note that the new 'gnutls-ocsp' mutex is a global mutex, not one per
virtual host, because a mutex must be registered in pre_config for the
Mutex directive to work.

Files:

• include/mod_gnutls.h.in (modified) (1 diff)
• src/gnutls_config.c (modified) (1 diff)
• src/gnutls_hooks.c (modified) (5 diffs)
• src/gnutls_ocsp.c (modified) (1 diff)
• src/gnutls_ocsp.h (modified) (1 diff)

include/mod_gnutls.h.in

@@ -215 +215 @@
      * unset. */
     apr_uri_t *ocsp_uri;
+    /* Mutex to prevent parallel OCSP requests */
+    apr_global_mutex_t *ocsp_mutex;
     /* Trust list to verify OCSP responses for stapling. Should
      * usually only contain the CA that signed the server

src/gnutls_config.c

@@ -981 +981 @@
     sc->ocsp_response_file = NULL;
     sc->ocsp_uri = NULL;
+    sc->ocsp_mutex = NULL;
     sc->ocsp_trust = NULL;
     sc->ocsp_grace_time = apr_time_from_sec(MGS_GRACE_TIME);

src/gnutls_hooks.c

@@ -127 +127 @@
 
     ap_mutex_register(pconf, MGS_CACHE_MUTEX_NAME, NULL, APR_LOCK_DEFAULT, 0);
+    ap_mutex_register(pconf, MGS_OCSP_MUTEX_NAME, NULL, APR_LOCK_DEFAULT, 0);
 
     /* Register a pool clean-up function */
@@ -342 +343 @@
     }
 
+    if (sc_base->ocsp_mutex == NULL)
+    {
+        rv = ap_global_mutex_create(&sc_base->ocsp_mutex, NULL,
+                                    MGS_OCSP_MUTEX_NAME, NULL,
+                                    base_server, pconf, 0);
+        if (rv != APR_SUCCESS)
+        {
+            ap_log_error(APLOG_MARK, APLOG_STARTUP, rv, base_server,
+                         "Failed to create mutex '" MGS_OCSP_MUTEX_NAME
+                         "'.");
+            return HTTP_INTERNAL_SERVER_ERROR;
+        }
+    }
+
     /* If GnuTLSP11Module is set, load the listed PKCS #11
      * modules. Otherwise system defaults will be used. */
@@ -370 +385 @@
     }
 
-    for (s = base_server; s; s = s->next) {
+    for (s = base_server; s; s = s->next)
+    {
         sc = (mgs_srvconf_rec *) ap_get_module_config(s->module_config, &gnutls_module);
         sc->cache_type = sc_base->cache_type;
@@ -385 +401 @@
         }
 
-        /* init OCSP trust list if OCSP is enabled */
+        sc->ocsp_mutex = sc_base->ocsp_mutex;
+        /* init OCSP trust list if OCSP is enabled for this host */
         if (sc->ocsp_response_file != NULL)
         {
@@ -522 +539 @@
         }
     }
+
+    /* reinit OCSP mutex */
+    const char *lockfile = apr_global_mutex_lockfile(sc->ocsp_mutex);
+    rv = apr_global_mutex_child_init(&sc->ocsp_mutex, lockfile, p);
+    if (rv != APR_SUCCESS)
+        ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
+                     "Failed to reinit mutex '" MGS_OCSP_MUTEX_NAME "'.");
 
     /* Block SIGPIPE Signals */

src/gnutls_ocsp.c

@@ -450 +450 @@
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, ctxt->c,
                   "No valid OCSP response in cache, trying to update.");
-    apr_status_t rv = mgs_cache_ocsp_response(ctxt->c->base_server);
+
+    apr_status_t rv = apr_global_mutex_trylock(ctxt->sc->ocsp_mutex);
+    if (APR_STATUS_IS_EBUSY(rv))
+    {
+        /* Another thread is currently holding the mutex, wait. */
+        apr_global_mutex_lock(ctxt->sc->ocsp_mutex);
+        /* Check if this other thread updated the response we need. It
+         * would be better to have a vhost specific mutex, but at the
+         * moment there's no good way to integrate that with the
+         * Apache Mutex directive. */
+        *ocsp_response = ctxt->sc->cache->fetch(ctxt, fingerprint);
+        if (ocsp_response->size > 0
+            && check_ocsp_response_expiry(ctxt, ocsp_response)
+            == GNUTLS_E_SUCCESS)
+        {
+            /* Got a valid response now, unlock mutex and return. */
+            apr_global_mutex_unlock(ctxt->sc->ocsp_mutex);
+            return GNUTLS_E_SUCCESS;
+        }
+        else
+        {
+            gnutls_free(ocsp_response->data);
+            ocsp_response->data = NULL;
+        }
+    }
+
+    rv = mgs_cache_ocsp_response(ctxt->c->base_server);
     if (rv != APR_SUCCESS)
     {
         ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, ctxt->c,
                       "Updating OCSP response cache failed");
+        apr_global_mutex_unlock(ctxt->sc->ocsp_mutex);
         goto fail_cleanup;
     }
+    apr_global_mutex_unlock(ctxt->sc->ocsp_mutex);
 
     /* retry reading from cache */

src/gnutls_ocsp.h

@@ -22 +22 @@
 #include "httpd.h"
 #include "http_config.h"
+
+#define MGS_OCSP_MUTEX_NAME "gnutls-ocsp"
 
 const char *mgs_store_ocsp_response_path(cmd_parms * parms,