ASF Bugzilla – Attachment 32073 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 v8 (based on work by Alex Bligh) - store stapling certinfo in a pconf-allocated hash
PR54357-v8.patch (text/plain), 9.53 KB, created by
Kaspar Brand
on 2014-10-01 14:52:47 UTC
(
hide
)
Description:
Patch v8 (based on work by Alex Bligh) - store stapling certinfo in a pconf-allocated hash
Filename:
MIME Type:
Creator:
Kaspar Brand
Created:
2014-10-01 14:52:47 UTC
Size:
9.53 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,36 +43,32 @@ > > #define MAX_STAPLING_DER 10240 > >-/* Cached info stored in certificate ex_info. */ >+/* Cached info stored in the global stapling_certinfo 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) >+static apr_status_t ssl_stapling_certid_free(void *data) > { >- certinfo *cinf = ptr; >+ OCSP_CERTID *cid = data; > >- if (!cinf) >- return; >- if (cinf->uri) >- OPENSSL_free(cinf->uri); >- OPENSSL_free(cinf); >+ if (cid) { >+ OCSP_CERTID_free(cid); >+ } >+ >+ return APR_SUCCESS; > } > >-static int stapling_ex_idx = -1; >+static apr_hash_t *stapling_certinfo; > >-void ssl_stapling_ex_init(void) >+void ssl_stapling_certinfo_hash_init(apr_pool_t *p) > { >- if (stapling_ex_idx != -1) >- return; >- stapling_ex_idx = X509_get_ex_new_index(0, "X509 cached OCSP info", 0, 0, >- certinfo_free); >+ stapling_certinfo = apr_hash_make(p); > } > > static X509 *stapling_get_issuer(modssl_ctx_t *mctx, X509 *x) >@@ -106,70 +102,96 @@ 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 *p, apr_pool_t *ptemp, >+ modssl_ctx_t *mctx, X509 *x) > { >- certinfo *cinf; >+ 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(stapling_certinfo, 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 have determined that there's something to store */ >+ cinf = apr_pcalloc(p, sizeof(certinfo)); >+ memcpy (cinf->idx, idx, sizeof(idx)); >+ cinf->cid = cid; >+ /* make sure cid is also freed at pool cleanup */ >+ apr_pool_cleanup_register(p, cid, ssl_stapling_certid_free, >+ apr_pool_cleanup_null); > if (aia) { >- cinf->uri = sk_OPENSSL_STRING_pop(aia); >- X509_email_free(aia); >+ /* allocate uri from the pconf pool */ >+ cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0)); >+ 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(stapling_certinfo, cinf->idx, sizeof(cinf->idx), cinf); >+ > return 1; > } > >-static certinfo *stapling_get_cert_info(server_rec *s, modssl_ctx_t *mctx, >+static certinfo *stapling_get_certinfo(server_rec *s, modssl_ctx_t *mctx, > SSL *ssl) > { > certinfo *cinf; > X509 *x; >+ UCHAR idx[SHA_DIGEST_LENGTH]; > 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(stapling_certinfo, idx, sizeof(idx)); > if (cinf && cinf->cid) > return cinf; > ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01926) >- "stapling_get_cert_info: stapling not supported for certificate"); >+ "stapling_get_certinfo: stapling not supported for certificate"); > return NULL; > } > >@@ -585,7 +607,7 @@ static int stapling_cb(SSL *ssl, void *arg) > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01951) > "stapling_cb: OCSP Stapling callback called"); > >- cinf = stapling_get_cert_info(s, mctx, ssl); >+ cinf = stapling_get_certinfo(s, mctx, ssl); > if (cinf == NULL) { > return SSL_TLSEXT_ERR_NOACK; > } >Index: modules/ssl/ssl_private.h >=================================================================== >--- modules/ssl/ssl_private.h (revision 1626667) >+++ modules/ssl/ssl_private.h (working copy) >@@ -812,8 +812,9 @@ 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); >+void ssl_stapling_certinfo_hash_init(apr_pool_t *); >+int ssl_stapling_init_cert(server_rec *s, apr_pool_t *, 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) >@@ -278,7 +278,7 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po > return HTTP_INTERNAL_SERVER_ERROR; > } > #ifdef HAVE_OCSP_STAPLING >- ssl_stapling_ex_init(); >+ ssl_stapling_certinfo_hash_init(p); > #endif > > /* >@@ -1093,7 +1093,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, p, 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 +1450,8 @@ 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, p, 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);
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