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


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1634529 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Jim Jagielski
2014-10-27 12:50:05 +00:00
parent 1dbdeb51d1
commit 8ac9c67bcf
5 changed files with 96 additions and 67 deletions

View File

@ -2,6 +2,10 @@
Changes with Apache 2.4.11 Changes with Apache 2.4.11
*) mod_ssl: Move OCSP stapling information from a per-certificate store to
a per-server hash. PR 54357, PR 56919. [Alex Bligh <alex alex.org.uk>,
Yann Ylavic, Kaspar Brand]
*) mod_cache_socache: Change average object size hint from 32 bytes to *) mod_cache_socache: Change average object size hint from 32 bytes to
2048 bytes. [Rainer Jung] 2048 bytes. [Rainer Jung]

6
STATUS
View File

@ -102,12 +102,6 @@ RELEASE SHOWSTOPPERS:
PATCHES ACCEPTED TO BACKPORT FROM TRUNK: PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ] [ start all new proposals below, under PATCHES PROPOSED. ]
* mod_ssl: Move OCSP stapling information to a per-server hash. PR 54357.
trunk patches: https://svn.apache.org/r1629372
https://svn.apache.org/r1629485
https://svn.apache.org/r1629519
2.4.x patch: https://people.apache.org/~kbrand/mod_ssl-2.4.x-PR54357.diff
+1: kbrand, ylavic, jim
PATCHES PROPOSED TO BACKPORT FROM TRUNK: PATCHES PROPOSED TO BACKPORT FROM TRUNK:

View File

@ -272,7 +272,7 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog,
return HTTP_INTERNAL_SERVER_ERROR; return HTTP_INTERNAL_SERVER_ERROR;
} }
#ifdef HAVE_OCSP_STAPLING #ifdef HAVE_OCSP_STAPLING
ssl_stapling_ex_init(); ssl_stapling_certinfo_hash_init(p);
#endif #endif
/* /*
@ -1067,7 +1067,7 @@ static apr_status_t ssl_init_server_certs(server_rec *s,
* later, we defer to the code in ssl_init_server_ctx. * later, we defer to the code in ssl_init_server_ctx.
*/ */
if ((mctx->stapling_enabled == TRUE) && 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) ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02567)
"Unable to configure certificate %s for stapling", "Unable to configure certificate %s for stapling",
key_id); key_id);
@ -1425,7 +1425,8 @@ static apr_status_t ssl_init_server_ctx(server_rec *s,
SSL_CERT_SET_FIRST); SSL_CERT_SET_FIRST);
while (ret) { while (ret) {
cert = SSL_CTX_get0_certificate(sc->server->ssl_ctx); 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) ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02604)
"Unable to configure certificate %s:%d " "Unable to configure certificate %s:%d "
"for stapling", sc->vhost_id, i); "for stapling", sc->vhost_id, i);

View File

@ -151,6 +151,13 @@
/* OCSP stapling */ /* OCSP stapling */
#if !defined(OPENSSL_NO_OCSP) && defined(SSL_CTX_set_tlsext_status_cb) #if !defined(OPENSSL_NO_OCSP) && defined(SSL_CTX_set_tlsext_status_cb)
#define HAVE_OCSP_STAPLING #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 #ifndef sk_OPENSSL_STRING_pop
#define sk_OPENSSL_STRING_pop sk_pop #define sk_OPENSSL_STRING_pop sk_pop
#endif #endif
@ -812,10 +819,11 @@ const char *ssl_cmd_SSLStaplingErrorCacheTimeout(cmd_parms *, void *, const char
const char *ssl_cmd_SSLStaplingReturnResponderErrors(cmd_parms *, void *, int); const char *ssl_cmd_SSLStaplingReturnResponderErrors(cmd_parms *, void *, int);
const char *ssl_cmd_SSLStaplingFakeTryLater(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_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 *); apr_status_t modssl_init_stapling(server_rec *, apr_pool_t *, apr_pool_t *, modssl_ctx_t *);
void ssl_stapling_ex_init(void); void ssl_stapling_certinfo_hash_init(apr_pool_t *);
int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x); int ssl_stapling_init_cert(server_rec *, apr_pool_t *, apr_pool_t *,
modssl_ctx_t *, X509 *);
#endif #endif
#ifdef HAVE_SRP #ifdef HAVE_SRP
int ssl_callback_SRPServerParams(SSL *, int *, void *); int ssl_callback_SRPServerParams(SSL *, int *, void *);

View File

@ -43,36 +43,32 @@
#define MAX_STAPLING_DER 10240 #define MAX_STAPLING_DER 10240
/* Cached info stored in certificate ex_info. */ /* Cached info stored in the global stapling_certinfo hash. */
typedef struct { typedef struct {
/* Index in session cache SHA1 hash of certificate */ /* Index in session cache (SHA-1 digest of DER encoded certificate) */
UCHAR idx[20]; UCHAR idx[SHA_DIGEST_LENGTH];
/* Certificate ID for OCSP requests or NULL if ID cannot be determined */ /* Certificate ID for OCSP request */
OCSP_CERTID *cid; OCSP_CERTID *cid;
/* Responder details */ /* URI of the OCSP responder */
char *uri; char *uri;
} certinfo; } certinfo;
static void certinfo_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad, static apr_status_t ssl_stapling_certid_free(void *data)
int idx, long argl, void *argp)
{ {
certinfo *cinf = ptr; OCSP_CERTID *cid = data;
if (!cinf) if (cid) {
return; OCSP_CERTID_free(cid);
if (cinf->uri) }
OPENSSL_free(cinf->uri);
OPENSSL_free(cinf); 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) stapling_certinfo = apr_hash_make(p);
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) static X509 *stapling_get_issuer(modssl_ctx_t *mctx, X509 *x)
@ -106,70 +102,96 @@ 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, 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; X509 *issuer = NULL;
OCSP_CERTID *cid = NULL;
STACK_OF(OPENSSL_STRING) *aia = NULL; STACK_OF(OPENSSL_STRING) *aia = NULL;
if (x == NULL) if ((x == NULL) || (X509_digest(x, EVP_sha1(), idx, NULL) != 1))
return 0; return 0;
cinf = X509_get_ex_data(x, stapling_ex_idx);
cinf = apr_hash_get(stapling_certinfo, idx, sizeof(idx));
if (cinf) { if (cinf) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02215) /*
"ssl_stapling_init_cert: certificate already initialized!"); * We already parsed the certificate, and no OCSP URI was found.
return 0; * 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 = 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;
}
cinf->cid = NULL;
cinf->uri = NULL;
X509_set_ex_data(x, stapling_ex_idx, cinf);
issuer = stapling_get_issuer(mctx, x); if (!(issuer = stapling_get_issuer(mctx, x))) {
ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x, APLOGNO(02217)
if (issuer == NULL) { "ssl_stapling_init_cert: can't retrieve issuer "
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02217) "certificate!");
"ssl_stapling_init_cert: Can't retrieve issuer certificate!");
return 0; return 0;
} }
cinf->cid = OCSP_cert_to_id(NULL, x, issuer); cid = OCSP_cert_to_id(NULL, x, issuer);
X509_free(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; return 0;
X509_digest(x, EVP_sha1(), cinf->idx, NULL); }
aia = X509_get1_ocsp(x); aia = X509_get1_ocsp(x);
if (aia) { if (!aia && !mctx->stapling_force_url) {
cinf->uri = sk_OPENSSL_STRING_pop(aia); OCSP_CERTID_free(cid);
X509_email_free(aia); ssl_log_xerror(SSLLOG_MARK, APLOG_ERR, 0, ptemp, s, x,
} APLOGNO(02218) "ssl_stapling_init_cert: no OCSP URI "
if (!cinf->uri && !mctx->stapling_force_url) { "in certificate and no SSLStaplingForceURL set");
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02218)
"ssl_stapling_init_cert: no responder URL");
return 0; 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; 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) SSL *ssl)
{ {
certinfo *cinf; certinfo *cinf;
X509 *x; X509 *x;
UCHAR idx[SHA_DIGEST_LENGTH];
x = SSL_get_certificate(ssl); x = SSL_get_certificate(ssl);
if (x == NULL) if ((x == NULL) || (X509_digest(x, EVP_sha1(), idx, NULL) != 1))
return NULL; return NULL;
cinf = X509_get_ex_data(x, stapling_ex_idx); cinf = apr_hash_get(stapling_certinfo, idx, sizeof(idx));
if (cinf && cinf->cid) if (cinf && cinf->cid)
return cinf; return cinf;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01926) 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; 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) ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01951)
"stapling_cb: OCSP Stapling callback called"); "stapling_cb: OCSP Stapling callback called");
cinf = stapling_get_cert_info(s, mctx, ssl); cinf = stapling_get_certinfo(s, mctx, ssl);
if (cinf == NULL) { if (cinf == NULL) {
return SSL_TLSEXT_ERR_NOACK; return SSL_TLSEXT_ERR_NOACK;
} }