> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic [mailto:[email protected]]
> Gesendet: Donnerstag, 1. Oktober 2015 16:47
> An: [email protected]
> Betreff: Re: svn commit: r1705823 -
> /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> 
> On Thu, Oct 1, 2015 at 3:54 PM, Plüm, Rüdiger, Vodafone Group
> <[email protected]> wrote:
> >>
> >> Looks like as neither ssl23_client_hello nor ssl23_write_bytes cause
> >> BIO_flush to be called.
> >
> > Dumped the wrong memory section for a HEAP bucket. The contents of the
> heap bucket is actually (now a different session):
> >
> > gdb) x/70x 0x7f03a801c0e8
> > 0x7f03a801c0e8: 0x01010316      0x0100010c      0x56030308
> 0xcbee2e0d
> > 0x7f03a801c0f8: 0x08b833ba      0xfc96ae2b      0xba2b9a9b
> 0xa67ea8d2
> > 0x7f03a801c108: 0x0ef75ce9      0xb398fba7      0x004fe092
> 0x30c08a00
> > 0x7f03a801c118: 0x28c02cc0      0x14c024c0      0xa3000ac0
> 0x6b009f00
> > 0x7f03a801c128: 0x39006a00      0x88003800      0x32c08700
> 0x2ac02ec0
> > 0x7f03a801c138: 0x0fc026c0      0x9d0005c0      0x35003d00
> 0x2fc08400
> > 0x7f03a801c148: 0x27c02bc0      0x13c023c0      0xa20009c0
> 0x67009e00
> > 0x7f03a801c158: 0x33004000      0x12c03200      0x450008c0
> 0x16004400
> > 0x7f03a801c168: 0x31c01300      0x29c02dc0      0x0ec025c0
> 0x0dc004c0
> > 0x7f03a801c178: 0x9c0003c0      0x2f003c00      0x0a004100
> 0x99009a00
> > 0x7f03a801c188: 0x07009600      0x07c011c0      0x02c00cc0
> 0x04000500
> > 0x7f03a801c198: 0x12001500      0xff000900      0x55000001
> 0x0e000000
> > 0x7f03a801c1a8: 0x00000c00      0x636f6c09      0x6f686c61
> 0x0b007473
> > 0x7f03a801c1b8: 0x00030400      0x0a000201      0x06000800
> 0x18001900
> > 0x7f03a801c1c8: 0x23001700      0x0d000000      0x20002200
> 0x02060106
> > 0x7f03a801c1d8: 0x01050306      0x03050205      0x02040104
> 0x01030304
> > 0x7f03a801c1e8: 0x03030203      0x02020102      0x01010302
> 0x01000f00
> > 0x7f03a801c1f8: 0x00000001      0x00000000
> >
> > And this is indeed a client hello.
> 
> Yes, I checked openssl's ssl23_client_hello() and indeed there is no
> flush from there...
> 
> Actually I don't see any flush in any SSL_method*() during handshake
> but at the end of it...
> 
> So we probably should go for something like:
> 
> Index: modules/ssl/ssl_engine_io.c
> ===================================================================
> --- modules/ssl/ssl_engine_io.c    (revision 1706228)
> +++ modules/ssl/ssl_engine_io.c    (working copy)
> @@ -452,6 +452,7 @@ 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 do_flush;
> 
>      inctx->rc = APR_SUCCESS;
> 
> @@ -466,21 +467,25 @@ static int bio_filter_in_read(BIO *bio, char *in,
>      }
> 
>      /* 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.
> +     * not to do so correctly in some cases (< 0.9.8m; see PR 46952),
> +     * or on the proxy/client side (after ssl23_client_hello(), eg.
> +     * 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
> -    /* 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) {
> +    do_flush = 1;
> +#else
> +    do_flush = !SSL_is_init_finished(inctx->ssl);
> +#endif
> +    if (do_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;
>      }
> -#endif
> 
>      BIO_clear_retry_flags(bio);
> 
> --
> 
> This makes the tests work for me...


Looks sensible. +1.

Regards

Rüdiger

Reply via email to