From 9e78eea79687dbef71a4e1083b1d42980aba5360 Mon Sep 17 00:00:00 2001 From: Alex Bligh Date: Fri, 12 Sep 2014 08:59:22 +0000 Subject: [PATCH] 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. 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 | 14 +++++----- modules/ssl/ssl_private.h | 19 ++++++++++++- modules/ssl/ssl_util_stapling.c | 59 +++++++++-------------------------------- 3 files changed, 37 insertions(+), 55 deletions(-) diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index f6e010d..8648384 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -199,9 +199,6 @@ int 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 @@ -820,7 +817,8 @@ static void ssl_init_ctx(server_rec *s, static int ssl_server_import_cert(server_rec *s, modssl_ctx_t *mctx, const char *id, - int idx) + int idx, + apr_pool_t *p) { SSLModConfigRec *mc = myModConfig(s); ssl_asn1_t *asn1; @@ -852,7 +850,7 @@ static int ssl_server_import_cert(server_rec *s, #ifdef HAVE_OCSP_STAPLING if ((mctx->pkp == FALSE) && (mctx->stapling_enabled == TRUE)) { - if (!ssl_stapling_init_cert(s, mctx, cert)) { + if (!ssl_stapling_init_cert(s, mctx, cert, p)) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02235) "Unable to configure server certificate for stapling"); } @@ -1000,10 +998,10 @@ static void ssl_init_server_certs(server_rec *s, ecc_id = ssl_asn1_table_keyfmt(ptemp, vhost_id, SSL_AIDX_ECC); #endif - have_rsa = ssl_server_import_cert(s, mctx, rsa_id, SSL_AIDX_RSA); - have_dsa = ssl_server_import_cert(s, mctx, dsa_id, SSL_AIDX_DSA); + have_rsa = ssl_server_import_cert(s, mctx, rsa_id, SSL_AIDX_RSA, p); + have_dsa = ssl_server_import_cert(s, mctx, dsa_id, SSL_AIDX_DSA, p); #ifdef HAVE_ECC - have_ecc = ssl_server_import_cert(s, mctx, ecc_id, SSL_AIDX_ECC); + have_ecc = ssl_server_import_cert(s, mctx, ecc_id, SSL_AIDX_ECC, p); #endif if (!(have_rsa || have_dsa diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 4ea924f..7fb950d 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -522,6 +522,18 @@ typedef struct { } rCtx; } SSLModConfigRec; +#ifdef HAVE_OCSP_STAPLING +/* Cached OCSP stapling info stored in modssl_pk_server_t */ +typedef struct { + /* Index in session cache SHA1 hash of certificate */ + UCHAR idx[20]; + /* Certificate ID for OCSP requests or NULL if ID cannot be determined */ + OCSP_CERTID *cid; + /* Responder details */ + char *uri; +} certinfo; +#endif + /** Structure representing configured filenames for certs and keys for * a given vhost, and the corresponding in-memory structures once the * files are parsed. */ @@ -538,6 +550,11 @@ typedef struct { X509 *certs[SSL_AIDX_MAX]; EVP_PKEY *keys[SSL_AIDX_MAX]; +#ifdef HAVE_OCSP_STAPLING + /* OCSP stapling information */ + certinfo *stapling_cert_info; +#endif + /** Certificates which specify the set of CA names which should be * sent in the CertificateRequest message: */ const char *ca_name_path; @@ -829,7 +846,7 @@ const char *ssl_cmd_SSLStaplingResponderTimeout(cmd_parms *, void *, const char const char *ssl_cmd_SSLStaplingForceURL(cmd_parms *, void *, const char *); void 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 0387cf9..d40d7a1 100644 --- a/modules/ssl/ssl_util_stapling.c +++ b/modules/ssl/ssl_util_stapling.c @@ -43,38 +43,6 @@ #define MAX_STAPLING_DER 10240 -/* Cached info stored in certificate ex_info. */ -typedef struct { - /* Index in session cache SHA1 hash of certificate */ - UCHAR idx[20]; - /* Certificate ID for OCSP requests or NULL if ID cannot be determined */ - OCSP_CERTID *cid; - /* Responder details */ - 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 +74,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 +82,19 @@ 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) { + + if (mctx->pks->stapling_cert_info) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02215) "ssl_stapling_init_cert: certificate already initialized!"); 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); + mctx->pks->stapling_cert_info = cinf; issuer = stapling_get_issuer(mctx, x); @@ -145,8 +111,13 @@ int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x) X509_digest(x, EVP_sha1(), cinf->idx, NULL); aia = X509_get1_ocsp(x); - if (aia) - cinf->uri = sk_OPENSSL_STRING_pop(aia); + if (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"); @@ -160,11 +131,7 @@ static certinfo *stapling_get_cert_info(server_rec *s, modssl_ctx_t *mctx, SSL *ssl) { certinfo *cinf; - X509 *x; - x = SSL_get_certificate(ssl); - if (x == NULL) - return NULL; - cinf = X509_get_ex_data(x, stapling_ex_idx); + cinf = mctx->pks->stapling_cert_info; if (cinf && cinf->cid) return cinf; ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01926) -- 1.9.1