[patch] v2 Proof of concept patch to address the issue

0001-v2-Remove-OCSP-stapling-info-from-X509-ex_data.patch (text/plain), 6.45 KB, created by Alex Bligh on 2014-09-14 20:15:19 UTC

(hide)

Description: v2 Proof of concept patch to address the issue

Filename:

MIME Type:

Creator: Alex Bligh

Created: 2014-09-14 20:15:19 UTC

Size: 6.45 KB

patch

obsolete

>From 42caa01a6de09bdc3507ec481da330ed5b1d419f Mon Sep 17 00:00:00 2001 >From: Alex Bligh <alex@alex.org.uk> >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 <alex@alex.org.uk> >--- > 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 > View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]