Commit 3ecd6d7f authored by Rainer Jung's avatar Rainer Jung
Browse files

Merge r891282 from trunk resp. 896900 from 2.2.x: 

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.

Submitted by: jorton
Backport by: rjung
Reviewed by: pgollucci, wrowe


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@1002227 13f79535-47bb-0310-9956-ffa450edef68
parent 26ddd114

CHANGES

+5 −4
Changes for CHANGES: 5 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -18,10 +18,11 @@ 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]

     renegotiations. Forcibly disable keepalive for the connection if there

     is any buffered data readable. 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, Hartmut Keil <Hartmut.Keil adnovum.ch>, Rainer Jung]



  *) SECURITY: CVE-2010-0434 (cve.mitre.org)

     Ensure each subrequest has a shallow copy of headers_in so that the

STATUS

+0 −9
Changes for STATUS: 0 added lines, 9 removed lines.
Original line number Diff line number Diff line
@@ -137,15 +137,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: 
      http://people.apache.org/~rjung/patches/SSLInsecureRenegotiation_httpd_2_0_x-backport-r917044.patch 

    +1: rjung, pgollucci (+1 2.0.64 w/ this), wrowe



  * mod_ssl: Further mitigation for the TLS renegotation attack, CVE-2009-3555

    Trunk version of patch:

      http://svn.apache.org/viewvc?rev=891282&view=rev

    Patch in 2.2.x branch:

      http://svn.apache.org/viewvc?rev=896900&view=rev

    Backport:

      http://people.apache.org/~rjung/patches/cve-2009-3555_httpd_2_0_x-backport-r891282.patch

    +1: rjung, pgollucci (+1 2.0.64 w/ this), wrowe



PATCHES PROPOSED TO BACKPORT FROM TRUNK:

  [ please place SVN revisions from trunk here, so it is easy to

    identify exactly what the proposed changes are!  Add all new

modules/ssl/ssl_engine_io.c

+9 −1
Changes for modules/ssl/ssl_engine_io.c: 9 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -1261,9 +1261,17 @@ static apr_status_t ssl_io_filter_input(ap_filter_t *f, 
    }

    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);

    }

modules/ssl/ssl_engine_kernel.c

+40 −0
Changes for modules/ssl/ssl_engine_kernel.c: 40 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -32,6 +32,29 @@ 


static void ssl_configure_env(request_rec *r, SSLConnRec *sslconn);

 

/* 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

 */
@@ -602,6 +625,23 @@ int ssl_hook_Access(request_rec *r) 
        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_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,

                         "Performing full renegotiation: "