[apache2] 04/05: mod_ssl: Avoid crashes due to stapling callback
Stefan Fritsch
sf at moszumanska.debian.org
Tue Nov 18 14:31:05 UTC 2014
This is an automated email from the git hooks/post-receive script.
sf pushed a commit to branch master
in repository apache2.
commit 8d29fbc6663c7dc55d6dbe36d84950a71ea99822
Author: Stefan Fritsch <sf at sfritsch.de>
Date: Tue Nov 18 15:16:04 2014 +0100
mod_ssl: Avoid crashes due to stapling callback
---
debian/changelog | 2 +
debian/patches/mod_ssl-oscp_stapling_crash.diff | 298 ++++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 301 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 5fbce09..87789ea 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,8 @@ apache2 (2.4.10-8) UNRELEASED; urgency=medium
though it does not seem to be exploitable.
* mpm_event: Fix use-after-free that may lead to a server crash.
* mod_ssl: Fix memory leak on graceful restart. Closes: #754492
+ * mod_ssl: Avoid crashes during startup or graceful restart due to
+ openssl using a callback to invalid memory. LP: #1366174
-- Stefan Fritsch <sf at debian.org> Mon, 17 Nov 2014 00:38:07 +0100
diff --git a/debian/patches/mod_ssl-oscp_stapling_crash.diff b/debian/patches/mod_ssl-oscp_stapling_crash.diff
new file mode 100644
index 0000000..c2de4eb
--- /dev/null
+++ b/debian/patches/mod_ssl-oscp_stapling_crash.diff
@@ -0,0 +1,298 @@
+# http://svn.apache.org/viewvc?view=revision&revision=1634529
+#
+# Avoid registering a cleanup function with openssl for the OCSP
+# stapling information. During startup or graceful restart mod_ssl
+# may be reloaded without libssl being reloaded. In this case
+# libssl may still use the cleanup that now points to invalid memory,
+# causing a crash.
+#
+# =================================
+# commit 1046d0c01b67d50af9d4570e975ba418dff3fa96
+# Author: Jim Jagielski <jim at apache.org>
+# Date: Mon Oct 27 12:50:05 2014 +0000
+#
+# Merge r1629372, r1629485, r1629519 from trunk:
+#
+# Move OCSP stapling information from a per-certificate store
+# (ex_data attached to an X509 *) to a per-server hash which is
+# allocated from the pconf pool. Fixes PR 54357, PR 56919 and
+# a leak with the certinfo_free cleanup function (missing
+# OCSP_CERTID_free).
+#
+# * modules/ssl/ssl_util_stapling.c: drop certinfo_free, and add
+# ssl_stapling_certid_free (used with apr_pool_cleanup_register).
+# Switch to a stapling_certinfo hash which is keyed by the SHA-1
+# digest of the certificate's DER encoding, rework ssl_stapling_init_cert
+# to only store info once per certificate (allocated from the pconf
+# to the extent possible) and extend the logging.
+#
+# * modules/ssl/ssl_private.h: adjust prototype for
+# ssl_stapling_init_cert, replace ssl_stapling_ex_init with
+# ssl_stapling_certinfo_hash_init
+#
+# * modules/ssl/ssl_engine_init.c: adjust ssl_stapling_* calls
+#
+# Based on initial work by Alex Bligh <alex alex.org.uk>
+#
+# Follow up to r1629372: ensure compatibily with OpenSSL < 1.0 (sk_OPENSSL_STRING_value).
+#
+# Follow up to r1629372 and r1629485: ensure compatibily with OpenSSL < 1.0 (sk_OPENSSL_STRING_[num|value|pop] macros).
+# Submitted by: kbrand, ylavic, ylavic
+# Reviewed/backported by: jim
+--- apache2.orig/modules/ssl/ssl_util_stapling.c
++++ apache2/modules/ssl/ssl_util_stapling.c
+@@ -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_
+
+ }
+
+-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;
+- }
+- 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!");
+- 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(02814) "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->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!");
++
++ 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 = OCSP_cert_to_id(NULL, x, issuer);
++ cid = OCSP_cert_to_id(NULL, x, issuer);
+ X509_free(issuer);
+- if (!cinf->cid)
++ if (!cid) {
++ ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x, APLOGNO(02815)
++ "ssl_stapling_init_cert: can't create CertID "
++ "for OCSP request");
+ return 0;
+- X509_digest(x, EVP_sha1(), cinf->idx, NULL);
++ }
+
+ aia = X509_get1_ocsp(x);
+- if (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");
++ 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;
+ }
++
++ /* 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) {
++ /* allocate uri from the pconf pool */
++ cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));
++ X509_email_free(aia);
++ }
++
++ 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 *a
+ 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;
+ }
+--- apache2.orig/modules/ssl/ssl_private.h
++++ apache2/modules/ssl/ssl_private.h
+@@ -151,6 +151,13 @@
+ /* OCSP stapling */
+ #if !defined(OPENSSL_NO_OCSP) && defined(SSL_CTX_set_tlsext_status_cb)
+ #define HAVE_OCSP_STAPLING
++/* backward compatibility with OpenSSL < 1.0 */
++#ifndef sk_OPENSSL_STRING_num
++#define sk_OPENSSL_STRING_num sk_num
++#endif
++#ifndef sk_OPENSSL_STRING_value
++#define sk_OPENSSL_STRING_value sk_value
++#endif
+ #ifndef sk_OPENSSL_STRING_pop
+ #define sk_OPENSSL_STRING_pop sk_pop
+ #endif
+@@ -812,10 +819,11 @@ const char *ssl_cmd_SSLStaplingErrorCach
+ const char *ssl_cmd_SSLStaplingReturnResponderErrors(cmd_parms *, void *, int);
+ const char *ssl_cmd_SSLStaplingFakeTryLater(cmd_parms *, void *, int);
+ const char *ssl_cmd_SSLStaplingResponderTimeout(cmd_parms *, void *, const char *);
+-const char *ssl_cmd_SSLStaplingForceURL(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 *, apr_pool_t *, apr_pool_t *,
++ modssl_ctx_t *, X509 *);
+ #endif
+ #ifdef HAVE_SRP
+ int ssl_callback_SRPServerParams(SSL *, int *, void *);
+--- apache2.orig/modules/ssl/ssl_engine_init.c
++++ apache2/modules/ssl/ssl_engine_init.c
+@@ -272,7 +272,7 @@ apr_status_t ssl_init_Module(apr_pool_t
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
+ #ifdef HAVE_OCSP_STAPLING
+- ssl_stapling_ex_init();
++ ssl_stapling_certinfo_hash_init(p);
+ #endif
+
+ /*
+@@ -1067,7 +1067,7 @@ static apr_status_t ssl_init_server_cert
+ * 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);
+@@ -1425,7 +1425,8 @@ static apr_status_t ssl_init_server_ctx(
+ 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);
diff --git a/debian/patches/series b/debian/patches/series
index eaab7ca..3be72f8 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,3 +9,4 @@ pull_upstream_2.4.x_branch.patch
CVE-2014-3583_mod_proxy_fcgi.diff
mpm_event_use_after_free.diff
mod_ssl_memleak.diff
+mod_ssl-oscp_stapling_crash.diff
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-apache/apache2.git
More information about the Pkg-apache-commits
mailing list