Commit b8885db0 authored by Jim Jagielski's avatar Jim Jagielski
Browse files

Merge r1705194, r1705823, r1705826, r1705828, r1705833, r1706275, r1707230, r1707231 from trunk: 

mod_ssl: forward EOR (only) brigades to the core_output_filter().

mod_ssl: don't FLUSH output (blocking) on read.
This defeats deferred write (and pipelining), eg. check_pipeline() is not
expecting the pipe to be flushed under it.
So let OpenSSL >= 0.9.8m issue the flush when necessary (earlier versions
are known to not handle all the cases, so we keep flushing with those).


mod_ssl: follow up to r1705823.
Oups, every #if needs a #endif...

mod_ssl: pass through metadata buckets untouched in ssl_io_filter_output(),
the core output filter needs them.

Proposed by: jorton


mod_ssl: follow up to r1705194, r1705823, r1705826 and r1705828.
Add CHANGES entry, and restore ap_process_request_after_handler()'s comment
as prior to r1705194 (the change makes no sense now).


mod_ssl: follow up to r1705823.
We still need to flush in the middle of a SSL/TLS handshake.


mod_ssl: follow up to r1705823.
Flush SSL/TLS handshake data when writing (instead of before reading),
and only when necessary (openssl < 0.9.8m or proxy/client side).


mod_ssl: follow up to r1707230: fix (inverted) logic for SSL_in_connect_init().

Submitted by: ylavic
Reviewed/backported by: jim


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1715014 13f79535-47bb-0310-9956-ffa450edef68
parent c7538222

CHANGES

+3 −0
Original line number Diff line number Diff line
@@ -24,6 +24,9 @@ Changes with Apache 2.4.17 
     to avoid reusing it should the close be effective after some new request

     is ready to be sent.  [Yann Ylavic]



  *) mod_ssl: Make the output filter more friendly with deferred write and

     response pipelining. [Yann Ylavic, Joe Orton]



  *) mod_substitute: Allow to configure the patterns merge order with the new

     SubstituteInheritBefore on|off directive.  PR 57641

     [Marc.Stern <Marc.Stern approach.be>, Yann Ylavic, William Rowe]

STATUS

+0 −12
Original line number Diff line number Diff line
@@ -111,18 +111,6 @@ RELEASE SHOWSTOPPERS: 
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:

  [ start all new proposals below, under PATCHES PROPOSED. ]



   * mod_ssl: Make the output filter more friendly with deferred write and

              response pipelining.

     trunk patch: http://svn.apache.org/r1705194

                  http://svn.apache.org/r1705823

                  http://svn.apache.org/r1705826

                  http://svn.apache.org/r1705828

                  http://svn.apache.org/r1705833

                  http://svn.apache.org/r1706275

                  http://svn.apache.org/r1707230

                  http://svn.apache.org/r1707231

     2.4.x patch: http://people.apache.org/~ylavic/httpd-2.4.x-mod_ssl-deferred_friendly-v3.patch

     +1: ylavic, icing, jim



  *) synch 2.4.x with trunk

        mod_authn_anon: Constify + save a few bytes in conf pool

modules/ssl/ssl_engine_io.c

+44 −58
Original line number Diff line number Diff line
@@ -187,6 +187,7 @@ 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);

    apr_bucket *e;

    int need_flush;



    /* Abort early if the client has initiated a renegotiation. */

    if (outctx->filter_ctx->config->reneg_state == RENEG_ABORT) {
@@ -205,6 +206,26 @@ static int bio_filter_out_write(BIO *bio, const char *in, int inl) 
    e = apr_bucket_transient_create(in, inl, outctx->bb->bucket_alloc);

    APR_BRIGADE_INSERT_TAIL(outctx->bb, e);



    /* In theory, OpenSSL should flush as necessary, but it is known

     * not to do so correctly in some cases (< 0.9.8m; see PR 46952),

     * or on the proxy/client side (after ssl23_client_hello(), e.g.

     * ssl/proxy.t test suite).

     *

     * Historically, this flush call was performed only for an SSLv2

     * connection or for a proxy connection.  Calling _out_flush can

     * be expensive in cases where requests/reponses are pipelined,

     * so limit the performance impact to handshake time.

     */

#if OPENSSL_VERSION_NUMBER < 0x0009080df

     need_flush = !SSL_is_init_finished(outctx->filter_ctx->pssl)

#else

     need_flush = SSL_in_connect_init(outctx->filter_ctx->pssl);

#endif

    if (need_flush) {

        e = apr_bucket_flush_create(outctx->bb->bucket_alloc);

        APR_BRIGADE_INSERT_TAIL(outctx->bb, e);

    }



    if (bio_filter_out_pass(outctx) < 0) {

        return -1;

    }
@@ -448,21 +469,6 @@ static int bio_filter_in_read(BIO *bio, char *in, int inlen) 
        return -1;

    }



    /* In theory, OpenSSL should flush as necessary, but it is known

     * not to do so correctly in some cases; see PR 46952.

     *

     * Historically, this flush call was performed only for an SSLv2

     * connection or for a proxy connection.  Calling _out_flush

     * should be very cheap in cases where it is unnecessary (and no

     * output is buffered) so the performance impact of doing it

     * unconditionally should be minimal.

     */

    if (bio_filter_out_flush(inctx->bio_out) < 0) {

        bio_filter_out_ctx_t *outctx = inctx->bio_out->ptr;

        inctx->rc = outctx->rc;

        return -1;

    }



    BIO_clear_retry_flags(bio);



    if (!inctx->bb) {
@@ -1632,49 +1638,30 @@ static apr_status_t ssl_io_filter_output(ap_filter_t *f, 
        return ssl_io_filter_error(f, bb, status);

    }



    while (!APR_BRIGADE_EMPTY(bb)) {

    while (!APR_BRIGADE_EMPTY(bb) && status == APR_SUCCESS) {

        apr_bucket *bucket = APR_BRIGADE_FIRST(bb);



        /* If it is a flush or EOS, we need to pass this down.

         * These types do not require translation by OpenSSL.

         */

        if (APR_BUCKET_IS_EOS(bucket) || APR_BUCKET_IS_FLUSH(bucket)) {

            if (bio_filter_out_flush(filter_ctx->pbioWrite) < 0) {

                status = outctx->rc;

                break;

            }



            if (APR_BUCKET_IS_EOS(bucket)) {

                /*

                 * By definition, nothing can come after EOS.

                 * which also means we can pass the rest of this brigade

                 * without creating a new one since it only contains the

                 * EOS bucket.

                 */



                if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {

                    return status;

                }

                break;

            }

            else {

                /* bio_filter_out_flush() already passed down a flush bucket

                 * if there was any data to be flushed.

                 */

                apr_bucket_delete(bucket);

            }

        }

        else if (AP_BUCKET_IS_EOC(bucket)) {

            /* The EOC bucket indicates connection closure, so SSL

             * shutdown must now be performed.  */

        if (APR_BUCKET_IS_METADATA(bucket)) {

            /* Pass through metadata buckets untouched.  EOC is

             * special; terminate the SSL layer first. */

            if (AP_BUCKET_IS_EOC(bucket)) {

                ssl_filter_io_shutdown(filter_ctx, f->c, 0);

            if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {

                return status;

            }

            break;

            AP_DEBUG_ASSERT(APR_BRIGADE_EMPTY(outctx->bb));



            /* Metadata buckets are passed one per brigade; it might

             * be more efficient (but also more complex) to use

             * outctx->bb as a true buffer and interleave these with

             * data buckets. */

            APR_BUCKET_REMOVE(bucket);

            APR_BRIGADE_INSERT_HEAD(outctx->bb, bucket);

            status = ap_pass_brigade(f->next, outctx->bb);

            if (status == APR_SUCCESS && f->c->aborted)

                status = APR_ECONNRESET;

            apr_brigade_cleanup(outctx->bb);

        }

        else {

            /* filter output */

            /* Filter a data bucket. */

            const char *data;

            apr_size_t len;
@@ -1687,7 +1674,9 @@ static apr_status_t ssl_io_filter_output(ap_filter_t *f, 
                    break;

                }

                rblock = APR_BLOCK_READ;

                continue; /* and try again with a blocking read. */

                /* and try again with a blocking read. */

                status = APR_SUCCESS;

                continue;

            }



            rblock = APR_NONBLOCK_READ;
@@ -1698,11 +1687,8 @@ static apr_status_t ssl_io_filter_output(ap_filter_t *f, 


            status = ssl_filter_write(f, data, len);

            apr_bucket_delete(bucket);



            if (status != APR_SUCCESS) {

                break;

            }

        }



    }



    return status;