You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2009/12/16 16:59:49 UTC
svn commit: r891282 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_io.c
ssl_engine_kernel.c
Author: jorton
Date: Wed Dec 16 15:59:49 2009
New Revision: 891282
URL: http://svn.apache.org/viewvc?rev=891282&view=rev
Log:
Further mitigation for the TLS renegotation attack, CVE-2009-3555:
* modules/ssl/ssl_engine_kernel.c (has_buffered_data): New function.
(ssl_hook_Access): Forcibly disable keepalive for the connection if
there is any buffered data readable from the input filter stack.
* modules/ssl/ssl_engine_io.c (ssl_io_filter_input): Ensure that the
BIO uses blocking operations when invoked outside direct control of
the httpd filter stack.
Thanks to Hartmut Keil <Hartmut.Keil adnovum.ch> for proposing this
technique.
Modified:
httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=891282&r1=891281&r2=891282&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Wed Dec 16 15:59:49 2009
@@ -1344,9 +1344,17 @@
}
else {
/* We have no idea what you are talking about, so return an error. */
- return APR_ENOTIMPL;
+ status = APR_ENOTIMPL;
}
+ /* It is possible for mod_ssl's BIO to be used outside of the
+ * direct control of mod_ssl's input or output filter -- notably,
+ * when mod_ssl initiates a renegotiation. Switching the BIO mode
+ * back to "blocking" here ensures such operations don't fail with
+ * SSL_ERROR_WANT_READ. */
+ inctx->block = APR_BLOCK_READ;
+
+ /* Handle custom errors. */
if (status != APR_SUCCESS) {
return ssl_io_filter_error(f, bb, status);
}
Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=891282&r1=891281&r2=891282&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Wed Dec 16 15:59:49 2009
@@ -87,6 +87,29 @@
return APR_SUCCESS;
}
+/* Perform a speculative (and non-blocking) read from the connection
+ * filters for the given request, to determine whether there is any
+ * pending data to read. Return non-zero if there is, else zero. */
+static int has_buffered_data(request_rec *r)
+{
+ apr_bucket_brigade *bb;
+ apr_off_t len;
+ apr_status_t rv;
+ int result;
+
+ bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+
+ rv = ap_get_brigade(r->connection->input_filters, bb, AP_MODE_SPECULATIVE,
+ APR_NONBLOCK_READ, 1);
+ result = rv == APR_SUCCESS
+ && apr_brigade_length(bb, 1, &len) == APR_SUCCESS
+ && len > 0;
+
+ apr_brigade_destroy(bb);
+
+ return result;
+}
+
/*
* Post Read Request Handler
*/
@@ -724,6 +747,23 @@
else {
request_rec *id = r->main ? r->main : r;
+ /* Additional mitigation for CVE-2009-3555: At this point,
+ * before renegotiating, an (entire) request has been read
+ * from the connection. An attacker may have sent further
+ * data to "prefix" any subsequent request by the victim's
+ * client after the renegotiation; this data may already
+ * have been read and buffered. Forcing a connection
+ * closure after the response ensures such data will be
+ * discarded. Legimately pipelined HTTP requests will be
+ * retried anyway with this approach. */
+ if (has_buffered_data(r)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "insecure SSL re-negotiation required, but "
+ "a pipelined request is present; keepalive "
+ "disabled");
+ r->connection->keepalive = AP_CONN_CLOSE;
+ }
+
/* do a full renegotiation */
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"Performing full renegotiation: "