From: Jeff Trawick Date: Tue, 12 May 2015 18:59:29 +0000 (+0000) Subject: mod_ssl OCSP Stapling: Don't block initial handshakes while refreshing X-Git-Tag: 2.5.0-alpha~3156 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9db0d19d21a5b3ad87173b3df8bec04c2ac854e6;p=apache 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 cache, and I haven't tested with distcache; thus it isn't clear if this helps in practice yet.) git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1679032 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 8de6225846..6c1529dbb6 100644 --- a/CHANGES +++ b/CHANGES @@ -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] diff --git a/docs/manual/mod/mod_ssl.xml b/docs/manual/mod/mod_ssl.xml index a213759a86..a087267fed 100644 --- a/docs/manual/mod/mod_ssl.xml +++ b/docs/manual/mod/mod_ssl.xml @@ -2371,6 +2371,14 @@ stated goal of "saving roundtrips and resources" - see also RFC 6961 (TLS Multiple Certificate Status Extension).

+ +

When OCSP stapling is enabled, the ssl-stapling mutex is used +to control access to the OCSP stapling cache in order to prevent corruption, +and the sss-stapling-refresh mutex is used to control refreshes +of OCSP responses. These mutexes can be configured using the +Mutex directive. +

+ @@ -2388,10 +2396,6 @@ is enabled. Configuration of a cache is mandatory for OCSP stapling. With the exception of none and nonenotnull, the same storage types are supported as with SSLSessionCache.

- -

The ssl-stapling mutex is used to serialize access to the -OCSP stapling cache to prevent corruption. This mutex can be configured -using the Mutex directive.

diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c index 188d3cb7b6..d53b99392f 100644 --- a/modules/ssl/mod_ssl.c +++ b/modules/ssl/mod_ssl.c @@ -380,7 +380,10 @@ static int ssl_hook_pre_config(apr_pool_t *pconf, /* 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; diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c index 7eb62d1ffe..f85a8a0fb9 100644 --- a/modules/ssl/ssl_engine_config.c +++ b/modules/ssl/ssl_engine_config.c @@ -71,7 +71,8 @@ SSLModConfigRec *ssl_config_global_create(server_rec *s) #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, diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 8a441f1c05..c4085f52c1 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -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(server_rec *, apr_pool_t *); /* 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 *); diff --git a/modules/ssl/ssl_util_stapling.c b/modules/ssl/ssl_util_stapling.c index 0e83baf3ea..ac4859f27d 100644 --- a/modules/ssl/ssl_util_stapling.c +++ b/modules/ssl/ssl_util_stapling.c @@ -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(server_rec *s, modssl_ctx_t *mctx, 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(server_rec *s, OCSP_RESPONSE **prsp, 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(server_rec *s, apr_pool_t *p) 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(server_rec *s, apr_pool_t *p) 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 *arg) 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)