Changeset 641d11b in mod_gnutls for src

Timestamp:

Jun 5, 2020, 2:18:27 PM (8 months ago)

Author:

Fiona Klute <fiona.klute@…>

Branches:

asyncio, master

Children:

e6d9e47

Parents:

2b1b52f

Message:

Delete session tickets for proxy connections when using them

Tickets should not be reused because an attacker could correlate
connections using the same ticket. Cache deletion code has been
extracted from socache_delete_session() into a generic function.

Location:

src

Files:

• gnutls_cache.c (modified) (2 diffs)
• gnutls_cache.h (modified) (1 diff)
• gnutls_proxy.c (modified) (1 diff)

src/gnutls_cache.c

@@ -269 +269 @@
 
 
+apr_status_t mgs_cache_delete(mgs_cache_t cache, server_rec *server,
+                              gnutls_datum_t key, apr_pool_t *pool)
+{
+    apr_pool_t *spool;
+    apr_pool_create(&spool, pool);
+
+    if (cache->prov->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
+        apr_global_mutex_lock(cache->mutex);
+    apr_status_t rv = cache->prov->remove(cache->socache, server,
+                                          key.data, key.size,
+                                          spool);
+    if (cache->prov->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
+        apr_global_mutex_unlock(cache->mutex);
+
+    if (rv != APR_SUCCESS)
+        ap_log_error(APLOG_MARK, APLOG_NOTICE, rv, server,
+                     "error deleting from cache '%s:%s'",
+                     cache->prov->name, cache->config);
+    else
+        ap_log_error(APLOG_MARK, APLOG_TRACE1, rv, server,
+                     "deleted entry from cache '%s:%s'",
+                     cache->prov->name, cache->config);
+    apr_pool_destroy(spool);
+    return rv;
+}
+
+
+
 /**
  * Remove function for the GnuTLS session cache, see
@@ -288 +316 @@
         return -1;
 
-    if (ctxt->sc->cache->prov->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
-        apr_global_mutex_lock(ctxt->sc->cache->mutex);
-    apr_status_t rv = ctxt->sc->cache->prov->remove(ctxt->sc->cache->socache,
-                                                    ctxt->c->base_server,
-                                                    dbmkey.data, dbmkey.size,
-                                                    ctxt->c->pool);
-    if (ctxt->sc->cache->prov->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
-        apr_global_mutex_unlock(ctxt->sc->cache->mutex);
-
-    if (rv != APR_SUCCESS) {
-        ap_log_error(APLOG_MARK, APLOG_NOTICE, rv,
-                     ctxt->c->base_server,
-                     "error deleting from cache '%s:%s'",
-                     ctxt->sc->cache->prov->name, ctxt->sc->cache->config);
+    apr_status_t rv = mgs_cache_delete(ctxt->sc->cache, ctxt->c->base_server,
+                                       dbmkey, ctxt->c->pool);
+    if (rv != APR_SUCCESS)
         return -1;
-    }
-    return 0;
+    else
+        return 0;
 }
 

src/gnutls_cache.h

@@ -154 +154 @@
                              gnutls_datum_t key, gnutls_datum_t *output,
                              apr_pool_t *pool);
+
+/**
+ * Delete an item from the mod_gnutls object caches.
+ *
+ * @param cache the cache to delete from
+ *
+ * @param server server context for the request
+ *
+ * @param key key for the cache entry to be deleted
+ *
+ * @param pool pool to allocate temporary memory from
+ *
+ * @return APR status or error value
+ */
+apr_status_t mgs_cache_delete(mgs_cache_t cache, server_rec *server,
+                              gnutls_datum_t key, apr_pool_t *pool);
 
 /**

src/gnutls_proxy.c

@@ -427 +427 @@
     }
 
-    // TODO: delete the cache entry
+    /* Best effort attempt to avoid ticket reuse. Unfortunately
+     * another thread or process could update (or remove) the cache in
+     * between, but that can't be avoided without forcing use of a
+     * global mutex even with a multiprocess-safe socache provider. */
+    mgs_cache_delete(ctxt->sc->cache, ctxt->c->base_server,
+                     ctxt->proxy_ticket_key, ctxt->c->pool);
 
     int ret = gnutls_session_set_data(ctxt->session, data.data, data.size);