From: Rainer Jung Date: Thu, 13 May 2010 13:27:03 +0000 (+0000) Subject: Merge r833582, r833593, r881222 from trunk: X-Git-Tag: 2.0.64~52 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=80584a8564ba95b10b370c7d0e01b7726ea867a1;p=thirdparty%2Fapache%2Fhttpd.git Merge r833582, r833593, r881222 from trunk: SECURITY: Partial fix for CVE-2009-3555: Reject client-initiated renegotiations; this is sufficient to prevent the attack for any configuration which does not require renegotiation due to per-directory/per-location access control configuration. Configuration with per-directory/per-location access control requirements (such as "SSLVerifyClient require") are still vulnerable to CVE-2009-3555 with this patch applied (if using OpenSSL != 0.9.8l). * modules/ssl/ssl_private.h (SSLConnRec): Add reneg_state field. (ssl_callback_Info): Renamed from ssl_callback_LogTracingState. * modules/ssl/ssl_engine_init.c (ssl_init_ctx_callbacks): Install the (renamed) info callback unconditionally. * modules/ssl/ssl_engine_io.c (ssl_filter_ctx_t): Add config pointer to SSLConnRec. (bio_filter_out_write, bio_filter_in_read): Fail with APR_ECONNABORTED if the reneg state is set to RENEG_ABORT. * modules/ssl/ssl_engine_kernel.c (log_tracing_state): Factored out of ssl_callback_LogTracingState. (ssl_callback_Info): New function. Submitted by: jorton, rpluem, rjung Reviewed by: rjung, rpluem, pgollucci git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@943879 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 0f486965fad..599d594a2af 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,14 @@ -*- coding: utf-8 -*- Changes with Apache 2.0.64 + *) SECURITY: CVE-2009-3555 (cve.mitre.org) + mod_ssl: A partial fix for the TLS renegotiation prefix injection attack + for OpenSSL versions prior to 0.9.8l; reject any client-initiated + renegotiations. Any configuration which requires renegotiation for + per-directory/location access control is still vulnerable, unless using + OpenSSL 0.9.8l or later. + [Joe Orton, Ruediger Pluem, Rainer Jung] + *) SECURITY: CVE-2010-0434 (cve.mitre.org) Ensure each subrequest has a shallow copy of headers_in so that the parent request headers are not corrupted. Elimiates a problematic diff --git a/STATUS b/STATUS index 1f32953dda9..524e5d4684b 100644 --- a/STATUS +++ b/STATUS @@ -124,16 +124,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: Backport version for 2.0.x of patch: http://people.apache.org/~fuankg/diffs/httpd-2.0.x-ap_vhost_iterate_given_conn.diff +1: fuankg, wrowe, pgollucci - * mod_ssl: Partial fix for CVE-2009-3555 - Trunk version of patch: - http://svn.apache.org/viewvc?rev=833582&view=rev - http://svn.apache.org/viewvc?rev=833593&view=rev - http://svn.apache.org/viewvc?rev=881222&view=rev - Patch in 2.2.x branch: - http://svn.apache.org/viewvc?rev=833622&view=rev - Backport version for 2.0.x of patch (Updated with backport of r881222): - http://people.apache.org/~rjung/patches/cve-2009-3555_httpd_2_0_x-v2.patch - +1: rjung, rpluem, pgollucci (+1 2.0.64 w/ this) PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ please place SVN revisions from trunk here, so it is easy to diff --git a/modules/ssl/mod_ssl.h b/modules/ssl/mod_ssl.h index 66a60374ca0..7ce2d62e011 100644 --- a/modules/ssl/mod_ssl.h +++ b/modules/ssl/mod_ssl.h @@ -389,6 +389,19 @@ typedef struct { int is_proxy; int disabled; int non_ssl_request; + + /* Track the handshake/renegotiation state for the connection so + * that all client-initiated renegotiations can be rejected, as a + * partial fix for CVE-2009-3555. */ + enum { + RENEG_INIT = 0, /* Before initial handshake */ + RENEG_REJECT, /* After initial handshake; any client-initiated + * renegotiation should be rejected */ + RENEG_ALLOW, /* A server-initated renegotiation is taking + * place (as dictated by configuration) */ + RENEG_ABORT /* Renegotiation initiated by client, abort the + * connection */ + } reneg_state; } SSLConnRec; typedef struct { @@ -585,7 +598,7 @@ int ssl_callback_proxy_cert(SSL *ssl, MODSSL_CLIENT_CERT_CB_ARG_TYPE ** int ssl_callback_NewSessionCacheEntry(SSL *, SSL_SESSION *); SSL_SESSION *ssl_callback_GetSessionCacheEntry(SSL *, unsigned char *, int, int *); void ssl_callback_DelSessionCacheEntry(SSL_CTX *, SSL_SESSION *); -void ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE, int, int); +void ssl_callback_Info(MODSSL_INFO_CB_ARG_TYPE, int, int); /* Session Cache Support */ void ssl_scache_init(server_rec *, apr_pool_t *); diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 439f7c0e513..b82d02801d0 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -464,10 +464,7 @@ static void ssl_init_ctx_callbacks(server_rec *s, SSL_CTX_set_tmp_rsa_callback(ctx, ssl_callback_TmpRSA); SSL_CTX_set_tmp_dh_callback(ctx, ssl_callback_TmpDH); - if (s->loglevel >= APLOG_DEBUG) { - /* this callback only logs if LogLevel >= info */ - SSL_CTX_set_info_callback(ctx, ssl_callback_LogTracingState); - } + SSL_CTX_set_info_callback(ctx, ssl_callback_Info); } static void ssl_init_ctx_verify(server_rec *s, diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c index 721fba5eacc..f68ca1ab911 100644 --- a/modules/ssl/ssl_engine_io.c +++ b/modules/ssl/ssl_engine_io.c @@ -102,6 +102,7 @@ typedef struct { ap_filter_t *pInputFilter; ap_filter_t *pOutputFilter; int nobuffer; /* non-zero to prevent buffering */ + SSLConnRec *config; } ssl_filter_ctx_t; typedef struct { @@ -192,7 +193,13 @@ static int bio_filter_out_read(BIO *bio, char *out, int outl) static int bio_filter_out_write(BIO *bio, const char *in, int inl) { bio_filter_out_ctx_t *outctx = (bio_filter_out_ctx_t *)(bio->ptr); - + + /* Abort early if the client has initiated a renegotiation. */ + if (outctx->filter_ctx->config->reneg_state == RENEG_ABORT) { + outctx->rc = APR_ECONNABORTED; + return -1; + } + /* when handshaking we'll have a small number of bytes. * max size SSL will pass us here is about 16k. * (16413 bytes to be exact) @@ -465,6 +472,12 @@ static int bio_filter_in_read(BIO *bio, char *in, int inlen) if (!in) return 0; + /* Abort early if the client has initiated a renegotiation. */ + if (inctx->filter_ctx->config->reneg_state == RENEG_ABORT) { + inctx->rc = APR_ECONNABORTED; + return -1; + } + /* XXX: flush here only required for SSLv2; * OpenSSL calls BIO_flush() at the appropriate times for * the other protocols. @@ -1585,6 +1598,8 @@ void ssl_io_filter_init(conn_rec *c, SSL *ssl) filter_ctx = apr_palloc(c->pool, sizeof(ssl_filter_ctx_t)); + filter_ctx->config = myConnConfig(c); + filter_ctx->nobuffer = 0; filter_ctx->pOutputFilter = ap_add_output_filter(ssl_io_filter, filter_ctx, NULL, c); diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c index 46367e34dc0..ecf254ebe43 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/modules/ssl/ssl_engine_kernel.c @@ -611,6 +611,10 @@ int ssl_hook_Access(request_rec *r) (unsigned char *)&id, sizeof(id)); + /* Toggle the renegotiation state to allow the new + * handshake to proceed. */ + sslconn->reneg_state = RENEG_ALLOW; + SSL_renegotiate(ssl); SSL_do_handshake(ssl); @@ -628,6 +632,8 @@ int ssl_hook_Access(request_rec *r) SSL_set_state(ssl, SSL_ST_ACCEPT); SSL_do_handshake(ssl); + sslconn->reneg_state = RENEG_REJECT; + if (SSL_get_state(ssl) != SSL_ST_OK) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, "Re-negotiation handshake failed: " @@ -1700,76 +1706,55 @@ void ssl_callback_DelSessionCacheEntry(SSL_CTX *ctx, return; } -/* - * This callback function is executed while OpenSSL processes the - * SSL handshake and does SSL record layer stuff. We use it to - * trace OpenSSL's processing in out SSL logfile. - */ -void ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc) +/* Dump debugginfo trace to the log file. */ +static void log_tracing_state(MODSSL_INFO_CB_ARG_TYPE ssl, conn_rec *c, + server_rec *s, int where, int rc) { - conn_rec *c; - server_rec *s; - SSLSrvConfigRec *sc; - /* - * find corresponding server + * create the various trace messages */ - if (!(c = (conn_rec *)SSL_get_app_data((SSL *)ssl))) { - return; + if (where & SSL_CB_HANDSHAKE_START) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Handshake: start", SSL_LIBRARY_NAME); } - - s = c->base_server; - if (!(sc = mySrvConfig(s))) { - return; + else if (where & SSL_CB_HANDSHAKE_DONE) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Handshake: done", SSL_LIBRARY_NAME); } - - /* - * create the various trace messages - */ - if (s->loglevel >= APLOG_DEBUG) { - if (where & SSL_CB_HANDSHAKE_START) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Handshake: start", SSL_LIBRARY_NAME); - } - else if (where & SSL_CB_HANDSHAKE_DONE) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Handshake: done", SSL_LIBRARY_NAME); - } - else if (where & SSL_CB_LOOP) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Loop: %s", - SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); - } - else if (where & SSL_CB_READ) { + else if (where & SSL_CB_LOOP) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Loop: %s", + SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); + } + else if (where & SSL_CB_READ) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Read: %s", + SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); + } + else if (where & SSL_CB_WRITE) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Write: %s", + SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); + } + else if (where & SSL_CB_ALERT) { + char *str = (where & SSL_CB_READ) ? "read" : "write"; + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Alert: %s:%s:%s", + SSL_LIBRARY_NAME, str, + SSL_alert_type_string_long(rc), + SSL_alert_desc_string_long(rc)); + } + else if (where & SSL_CB_EXIT) { + if (rc == 0) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Read: %s", + "%s: Exit: failed in %s", SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); } - else if (where & SSL_CB_WRITE) { + else if (rc < 0) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Write: %s", + "%s: Exit: error in %s", SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); } - else if (where & SSL_CB_ALERT) { - char *str = (where & SSL_CB_READ) ? "read" : "write"; - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Alert: %s:%s:%s", - SSL_LIBRARY_NAME, str, - SSL_alert_type_string_long(rc), - SSL_alert_desc_string_long(rc)); - } - else if (where & SSL_CB_EXIT) { - if (rc == 0) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Exit: failed in %s", - SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); - } - else if (rc < 0) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Exit: error in %s", - SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); - } - } } /* @@ -1789,3 +1774,48 @@ void ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc } } +/* + * This callback function is executed while OpenSSL processes the SSL + * handshake and does SSL record layer stuff. It's used to trap + * client-initiated renegotiations, and for dumping everything to the + * log. + */ +void ssl_callback_Info(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc) +{ + conn_rec *c; + server_rec *s; + SSLConnRec *scr; + + /* Retrieve the conn_rec and the associated SSLConnRec. */ + if ((c = (conn_rec *)SSL_get_app_data((SSL *)ssl)) == NULL) { + return; + } + + if ((scr = myConnConfig(c)) == NULL) { + return; + } + + /* If the reneg state is to reject renegotiations, check the SSL + * state machine and move to ABORT if a Client Hello is being + * read. */ + if ((where & SSL_CB_ACCEPT_LOOP) && scr->reneg_state == RENEG_REJECT) { + int state = SSL_get_state((SSL *)ssl); + + if (state == SSL3_ST_SR_CLNT_HELLO_A + || state == SSL23_ST_SR_CLNT_HELLO_A) { + scr->reneg_state = RENEG_ABORT; + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, + "rejecting client initiated renegotiation"); + } + } + /* If the first handshake is complete, change state to reject any + * subsequent client-initated renegotiation. */ + else if ((where & SSL_CB_HANDSHAKE_DONE) && scr->reneg_state == RENEG_INIT) { + scr->reneg_state = RENEG_REJECT; + } + + s = c->base_server; + if (s && s->loglevel >= APLOG_DEBUG) { + log_tracing_state(ssl, c, s, where, rc); + } +}