From 42caa01a6de09bdc3507ec481da330ed5b1d419f Mon Sep 17 00:00:00 2001 From: Alex Bligh Date: Sun, 14 Sep 2014 20:08:45 +0000 Subject: [PATCH] (v2) Remove OCSP stapling info from X509 ex_data Remove OSCP stapling info from X509 ex_data, and manage it within normal APR pools with server lifecyle. This is to address BZ 54357 and BZ 56919. Introduce a hash of stapling info indexed by the SHA1 hash of the certificate content. Note this code as been compile tested only at this stage and is submitted as a proof of concept. Signed-off-by: Alex Bligh --- modules/ssl/ssl_engine_init.c | 5 +--- modules/ssl/ssl_private.h | 5 +++- modules/ssl/ssl_util_stapling.c | 61 ++++++++++++++++++----------------------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 0cdf766..7967d4c 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -277,9 +277,6 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, if (!ssl_mutex_init(base_server, p)) { return HTTP_INTERNAL_SERVER_ERROR; } -#ifdef HAVE_OCSP_STAPLING - ssl_stapling_ex_init(); -#endif /* * initialize session caching @@ -1093,7 +1090,7 @@ static apr_status_t ssl_init_server_certs(server_rec *s, * later, we defer to the code in ssl_init_server_ctx. */ if ((mctx->stapling_enabled == TRUE) && - !ssl_stapling_init_cert(s, mctx, cert)) { + !ssl_stapling_init_cert(s, mctx, cert, p)) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02567) "Unable to configure certificate %s for stapling", key_id); diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index a0100d0..214530d 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -513,6 +513,9 @@ typedef struct { * sent in the CertificateRequest message: */ const char *ca_name_path; const char *ca_name_file; + + /* Hash of stapling information by SHA1 of certificate */ + apr_hash_t *stapling_info; } modssl_pk_server_t; typedef struct { @@ -813,7 +816,7 @@ const char *ssl_cmd_SSLStaplingResponderTimeout(cmd_parms *, void *, const char const char *ssl_cmd_SSLStaplingForceURL(cmd_parms *, void *, const char *); apr_status_t modssl_init_stapling(server_rec *, apr_pool_t *, apr_pool_t *, modssl_ctx_t *); void ssl_stapling_ex_init(void); -int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x); +int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x, apr_pool_t *p); #endif #ifdef HAVE_SRP int ssl_callback_SRPServerParams(SSL *, int *, void *); diff --git a/modules/ssl/ssl_util_stapling.c b/modules/ssl/ssl_util_stapling.c index 2dc8fce..f0bd985 100644 --- a/modules/ssl/ssl_util_stapling.c +++ b/modules/ssl/ssl_util_stapling.c @@ -53,28 +53,6 @@ typedef struct { char *uri; } certinfo; -static void certinfo_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad, - int idx, long argl, void *argp) -{ - certinfo *cinf = ptr; - - if (!cinf) - return; - if (cinf->uri) - OPENSSL_free(cinf->uri); - OPENSSL_free(cinf); -} - -static int stapling_ex_idx = -1; - -void ssl_stapling_ex_init(void) -{ - if (stapling_ex_idx != -1) - return; - stapling_ex_idx = X509_get_ex_new_index(0, "X509 cached OCSP info", 0, 0, - certinfo_free); -} - static X509 *stapling_get_issuer(modssl_ctx_t *mctx, X509 *x) { X509 *issuer = NULL; @@ -106,7 +84,7 @@ static X509 *stapling_get_issuer(modssl_ctx_t *mctx, X509 *x) } -int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x) +int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x, apr_pool_t *p) { certinfo *cinf; X509 *issuer = NULL; @@ -114,21 +92,22 @@ int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x) if (x == NULL) return 0; - cinf = X509_get_ex_data(x, stapling_ex_idx); - if (cinf) { - ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02215) - "ssl_stapling_init_cert: certificate already initialized!"); + + if (mctx->pks->stapling_info == NULL) + mctx->pks->stapling_info = apr_hash_make(p); + + if (mctx->pks->stapling_info == NULL) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02216) + "ssl_stapling_init_cert: error allocating memory!"); return 0; } - cinf = OPENSSL_malloc(sizeof(certinfo)); + + cinf = apr_pcalloc(p, sizeof(certinfo)); if (!cinf) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02216) "ssl_stapling_init_cert: error allocating memory!"); return 0; } - cinf->cid = NULL; - cinf->uri = NULL; - X509_set_ex_data(x, stapling_ex_idx, cinf); issuer = stapling_get_issuer(mctx, x); @@ -146,14 +125,26 @@ int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x) aia = X509_get1_ocsp(x); if (aia) { - cinf->uri = sk_OPENSSL_STRING_pop(aia); - X509_email_free(aia); + /* Ugly: ensure memory managed by apr */ + char *uri; + uri = sk_OPENSSL_STRING_pop(aia); + cinf->uri = apr_pstrdup(p, uri); + OPENSSL_free(uri); } if (!cinf->uri && !mctx->stapling_force_url) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02218) "ssl_stapling_init_cert: no responder URL"); return 0; } + + if (apr_hash_get(mctx->pks->stapling_info, cinf->idx, sizeof(cinf->idx))) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02215) + "ssl_stapling_init_cert: certificate already initialized!"); + return 0; + } + + apr_hash_set(mctx->pks->stapling_info, cinf->idx, sizeof(cinf->idx), cinf); + return 1; } @@ -162,10 +153,12 @@ static certinfo *stapling_get_cert_info(server_rec *s, modssl_ctx_t *mctx, { certinfo *cinf; X509 *x; + UCHAR idx[20]; x = SSL_get_certificate(ssl); if (x == NULL) return NULL; - cinf = X509_get_ex_data(x, stapling_ex_idx); + X509_digest(x, EVP_sha1(), idx, NULL); + cinf = apr_hash_get(mctx->pks->stapling_info, idx, sizeof(idx)); if (cinf && cinf->cid) return cinf; ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01926) -- 1.9.1