Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c

[Yann Ylavic]

On Thu, Oct 1, 2015 at 8:22 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>
> The issue is that openssl during the connect handshake to a clieent does not 
> tell httpd to flush. Hence the CLIENT_HELLO
> remains in the core output filter buffer and openssl waits for the 
> SERVER_HELLO from the remote server which of course
> does not happen without the CLIENT_HELLO having been sent there.
>

I also tried the following patch which also passes the test framework
and is maybe more straightforward since it flushes on write (during
handshake only), thus avoiding any flush (and round-trip) on read.

WDYT?

Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c    (revision 1706668)
+++ modules/ssl/ssl_engine_io.c    (working copy)
@@ -214,6 +214,21 @@ static int bio_filter_out_write(BIO *bio, const ch
     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 (!SSL_is_init_finished(outctx->filter_ctx->pssl)) {
+        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;
     }
@@ -452,7 +467,6 @@ static int bio_filter_in_read(BIO *bio, char *in,
     apr_size_t inl = inlen;
     bio_filter_in_ctx_t *inctx = (bio_filter_in_ctx_t *)(bio->ptr);
     apr_read_type_e block = inctx->block;
-    int need_flush;

     inctx->rc = APR_SUCCESS;

@@ -466,27 +480,6 @@ static int bio_filter_in_read(BIO *bio, char *in,
         return -1;
     }

-    /* 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 = 1;
-#else
-    need_flush = !SSL_is_init_finished(inctx->ssl);
-#endif
-    if (need_flush && 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) {
--

Regards,
Yann.