ASF Bugzilla – Attachment 32053 Details for
Bug 54357
Crash during restart or at startup in mod_ssl, in certinfo_free() function registered by ssl_stapling_ex_init()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch v7 - store stapling certinfo in a global hash, based on work by Alex Bligh
PR54357-v7.patch (text/plain), 9.58 KB, created by
Kaspar Brand
on 2014-09-24 12:29:20 UTC
(
hide
)
Description:
Patch v7 - store stapling certinfo in a global hash, based on work by Alex Bligh
Filename:
MIME Type:
Creator:
Kaspar Brand
Created:
2014-09-24 12:29:20 UTC
Size:
9.58 KB
patch
obsolete
>Index: modules/ssl/ssl_util_stapling.c >=================================================================== >--- modules/ssl/ssl_util_stapling.c (revision 1626667) >+++ modules/ssl/ssl_util_stapling.c (working copy) >@@ -43,38 +43,16 @@ > > #define MAX_STAPLING_DER 10240 > >-/* Cached info stored in certificate ex_info. */ >+/* Cached info stored in the global stapling_cert_info hash. */ > 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 */ >+ /* Index in session cache (SHA-1 digest of DER encoded certificate) */ >+ UCHAR idx[SHA_DIGEST_LENGTH]; >+ /* Certificate ID for OCSP request */ > OCSP_CERTID *cid; >- /* Responder details */ >+ /* URI of the OCSP responder */ > 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,54 +84,83 @@ static X509 *stapling_get_issuer(modssl_ctx_t *mct > > } > >-int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x) >+int ssl_stapling_init_cert(server_rec *s, apr_pool_t *ptemp, modssl_ctx_t *mctx, X509 *x) > { >- certinfo *cinf; >+ SSLModConfigRec *mc = myModConfig(s); >+ UCHAR idx[SHA_DIGEST_LENGTH]; >+ certinfo *cinf = NULL; > X509 *issuer = NULL; >+ OCSP_CERTID *cid = NULL; > STACK_OF(OPENSSL_STRING) *aia = NULL; > >- if (x == NULL) >+ if ((x == NULL) || (X509_digest(x, EVP_sha1(), idx, NULL) != 1)) > return 0; >- cinf = X509_get_ex_data(x, stapling_ex_idx); >+ >+ cinf = apr_hash_get(mc->stapling_cert_info, idx, sizeof(idx)); > if (cinf) { >- ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02215) >- "ssl_stapling_init_cert: certificate already initialized!"); >- return 0; >+ /* >+ * We already parsed the certificate, and no OCSP URI was found. >+ * The certificate might be used for multiple vhosts, though, >+ * so we check for a ForceURL for this vhost. >+ */ >+ if (!cinf->uri && !mctx->stapling_force_url) { >+ ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x, >+ APLOGNO() "ssl_stapling_init_cert: no OCSP URI " >+ "in certificate and no SSLStaplingForceURL " >+ "configured for server %s", mctx->sc->vhost_id); >+ return 0; >+ } >+ return 1; > } >- cinf = OPENSSL_malloc(sizeof(certinfo)); >- if (!cinf) { >- ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02216) >- "ssl_stapling_init_cert: error allocating memory!"); >+ >+ if (!(issuer = stapling_get_issuer(mctx, x))) { >+ ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x, APLOGNO(02217) >+ "ssl_stapling_init_cert: can't retrieve issuer " >+ "certificate!"); > return 0; > } >- cinf->cid = NULL; >- cinf->uri = NULL; >- X509_set_ex_data(x, stapling_ex_idx, cinf); > >- issuer = stapling_get_issuer(mctx, x); >- >- if (issuer == NULL) { >- ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02217) >- "ssl_stapling_init_cert: Can't retrieve issuer certificate!"); >+ cid = OCSP_cert_to_id(NULL, x, issuer); >+ X509_free(issuer); >+ if (!cid) { >+ ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x, APLOGNO() >+ "ssl_stapling_init_cert: can't create CertID " >+ "for OCSP request"); > return 0; > } > >- cinf->cid = OCSP_cert_to_id(NULL, x, issuer); >- X509_free(issuer); >- if (!cinf->cid) >+ aia = X509_get1_ocsp(x); >+ if (!aia && !mctx->stapling_force_url) { >+ OCSP_CERTID_free(cid); >+ ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x, >+ APLOGNO(02218) "ssl_stapling_init_cert: no OCSP URI " >+ "in certificate and no SSLStaplingForceURL set"); > return 0; >- X509_digest(x, EVP_sha1(), cinf->idx, NULL); >+ } > >- aia = X509_get1_ocsp(x); >+ /* >+ * At this point, we definitely know that we do have information we want >+ * to store permanently (i.e., for the lifetime of the process). There >+ * isn't really an option to manage the memory for the OCSP_CERTID struct >+ * by APR, so we just store pointers to OPENSSL_malloc'ed stuff here, >+ * based on the reasoning that mc->pPool will stay for the process lifetime >+ * anyway (and we're careful in not allocating any cinf again when >+ * restarting, see the check at the beginning of this function). >+ */ >+ cinf = apr_pcalloc(mc->pPool, sizeof(certinfo)); >+ memcpy (cinf->idx, idx, sizeof(idx)); >+ cinf->cid = cid; > if (aia) { >- cinf->uri = sk_OPENSSL_STRING_pop(aia); >- X509_email_free(aia); >+ cinf->uri = sk_OPENSSL_STRING_pop(aia); >+ X509_email_free(aia); > } >- 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; >- } >+ >+ ssl_log_xerror(SSLLOG_MARK, APLOG_TRACE1, 0, ptemp, s, x, >+ "ssl_stapling_init_cert: storing certinfo for server %s", >+ mctx->sc->vhost_id); >+ >+ apr_hash_set(mc->stapling_cert_info, cinf->idx, sizeof(cinf->idx), cinf); >+ > return 1; > } > >@@ -162,10 +169,12 @@ static certinfo *stapling_get_cert_info(server_rec > { > certinfo *cinf; > X509 *x; >+ UCHAR idx[SHA_DIGEST_LENGTH]; >+ SSLModConfigRec *mc = myModConfig(s); > x = SSL_get_certificate(ssl); >- if (x == NULL) >+ if ((x == NULL) || (X509_digest(x, EVP_sha1(), idx, NULL) != 1)) > return NULL; >- cinf = X509_get_ex_data(x, stapling_ex_idx); >+ cinf = apr_hash_get(mc->stapling_cert_info, idx, sizeof(idx)); > if (cinf && cinf->cid) > return cinf; > ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01926) >Index: modules/ssl/ssl_private.h >=================================================================== >--- modules/ssl/ssl_private.h (revision 1626667) >+++ modules/ssl/ssl_private.h (working copy) >@@ -499,6 +499,9 @@ typedef struct { > const ap_socache_provider_t *stapling_cache; > ap_socache_instance_t *stapling_cache_context; > apr_global_mutex_t *stapling_mutex; >+ >+ /* Hash of stapling information by SHA1 of certificate */ >+ apr_hash_t *stapling_cert_info; > #endif > } SSLModConfigRec; > >@@ -812,8 +815,7 @@ const char *ssl_cmd_SSLStaplingFakeTryLater(cmd_pa > 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, apr_pool_t *, modssl_ctx_t *mctx, X509 *x); > #endif > #ifdef HAVE_SRP > int ssl_callback_SRPServerParams(SSL *, int *, void *); >Index: modules/ssl/ssl_engine_init.c >=================================================================== >--- modules/ssl/ssl_engine_init.c (revision 1626667) >+++ modules/ssl/ssl_engine_init.c (working copy) >@@ -277,9 +277,6 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po > 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_r > * 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, ptemp, mctx, cert)) { > ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02567) > "Unable to configure certificate %s for stapling", > key_id); >@@ -1450,7 +1447,7 @@ static apr_status_t ssl_init_server_ctx(server_rec > SSL_CERT_SET_FIRST); > while (ret) { > cert = SSL_CTX_get0_certificate(sc->server->ssl_ctx); >- if (!cert || !ssl_stapling_init_cert(s, sc->server, cert)) { >+ if (!cert || !ssl_stapling_init_cert(s, ptemp, sc->server, cert)) { > ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02604) > "Unable to configure certificate %s:%d " > "for stapling", sc->vhost_id, i); >Index: modules/ssl/ssl_engine_config.c >=================================================================== >--- modules/ssl/ssl_engine_config.c (revision 1626667) >+++ modules/ssl/ssl_engine_config.c (working copy) >@@ -72,6 +72,7 @@ SSLModConfigRec *ssl_config_global_create(server_r > #ifdef HAVE_OCSP_STAPLING > mc->stapling_cache = NULL; > mc->stapling_mutex = NULL; >+ mc->stapling_cert_info = apr_hash_make(pool); > #endif > > apr_pool_userdata_set(mc, SSL_MOD_CONFIG_KEY,
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 54357
:
32010
|
32021
|
32035
|
32036
|
32037
|
32039
|
32043
|
32053
|
32073