You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Graham Leggett <mi...@sharp.fm> on 2015/10/01 18:59:29 UTC
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
On 01 Oct 2015, at 5:43 PM, ylavic@apache.org wrote:
> URL: http://svn.apache.org/viewvc?rev=1706275&view=rev
> Log:
> mod_ssl: follow up to r1705823.
> We still need to flush in the middle of a SSL/TLS handshake.
Can you confirm why the flushing is necessary?
In theory mod_ssl should be switching the sense of any reads/writes as necessary without any need for flushing.
Regards,
Graham
—
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Oct 2015, at 8:22 PM, Ruediger Pluem <rp...@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.
>
> The whole game of reading and writing during the handshake happens inside openssl while SSL_connect is running.
> Apache code only gets back into this via bio_filter_out_write and bio_filter_in_read.
While working on the async write completion patch I was having all sorts of headaches with incomplete requests, and eventually reached the point where I removed THRESHOLD_MIN_WRITE completely and suddenly sanity returned across the board.
In the async code there is no way to assume you can not-write when you’ve been asked to write, because there is no way to tell whether more data is coming after this data. If you skip the write in the hope that more data comes but no data does come, you hang.
I get that THRESHOLD_MIN_WRITE was trying to prevent the sending of small packets, but really that should be solved by the filters themselves.
Regards,
Graham
—
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Oct 7, 2015 at 5:59 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 07 Oct 2015, at 5:53 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
>
>>> As I understand we’re using openssl in non blocking mode, which means that openssl will ask us permission before attempting any read or write.
>>>
>>> The core will then in turn either read or write as requested by openssl based on the “sense” flags CONN_SENSE_WANT_READ or CONN_SENSE_WANT_WRITE.
>>>
>>> If openssl has a bug and reads/writes without first asking permission we’ll block, but by the same token if openssl as asking us permission using SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE and we’re ignoring openssl, we’ll block for the same reason.
>>
>> But certainly these are situations which it's "safer" to block
>> in any case, right? Of course, they could also be vectors for some
>> sort of DDoS, but even then, that would be relying on pretty
>> nasty bugs.
>
> The blocking isn’t really the problem, it’s accidentally waiting for a socket to be readable when openssl asked you to tell it when the socket is writable.
>
> I suspect turning on the “flush” is masking a bug.
Are we talking about this particular commit or more generally about
the handling of SSL_ERROR_WANT_READ/WRITE in mod_ssl?
In the former case, the "flush" is not masking but addressing a bug in
openssl, which does not always flush its (handshake) data where it
should...
Also, AFAICT, SSL_ERROR_WANT_READ/WRITE can only happen during
(re)negotiation, it makes sense here to block rather than scheduling
small packet exchanges through the MPM (and maybe let mod_reqtimeout
handle any DOS attempt).
Regards,
Yann.
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Oct 7, 2015, at 11:59 AM, Graham Leggett <mi...@sharp.fm> wrote:
>
> On 07 Oct 2015, at 5:53 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
>
>>> As I understand we’re using openssl in non blocking mode, which means that openssl will ask us permission before attempting any read or write.
>>>
>>> The core will then in turn either read or write as requested by openssl based on the “sense” flags CONN_SENSE_WANT_READ or CONN_SENSE_WANT_WRITE.
>>>
>>> If openssl has a bug and reads/writes without first asking permission we’ll block, but by the same token if openssl as asking us permission using SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE and we’re ignoring openssl, we’ll block for the same reason.
>>
>> But certainly these are situations which it's "safer" to block
>> in any case, right? Of course, they could also be vectors for some
>> sort of DDoS, but even then, that would be relying on pretty
>> nasty bugs.
>
> The blocking isn’t really the problem, it’s accidentally waiting for a socket to be readable when openssl asked you to tell it when the socket is writable.
>
> I suspect turning on the “flush” is masking a bug.
>
Ah, I see now...
Digging into it a bit, I think your suspicion is correct, but I
can't find it.
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 07 Oct 2015, at 7:46 PM, Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com> wrote:
> I guess we are talking of different things. During the initial handshake (client or server) we never hand back
> control to the event part of the MPM. We never use ssl_filter_write and ssl_io_filter_output as this is only used for data we want to encrypt on the connection. During the handshake we deal directly with the core output filter.
> We are just synchronous here. And I think this is not a big deal as the amount of data that needs to be transferred here is low and buffered by the socket buffer anyway. So we would not really notice the difference between synchronous and asynchronous since the socket is not blocking / says its writable.
This is probably where we’re going wrong.
With mod_ssl non blocking behaviour is an all-or-nothing deal. We must follow all of openssl’s instructions to the letter, otherwise we’ll hang. We can’t sometimes be synchronous and sometimes not. Accidentally waiting to be ready to write when we actually need to be waiting to read is easy to get away with, because almost always we are ready to write. The opposite however isn’t true. If we accidentally wait to be readable when openssl wanted us to tell when it’s writable we can very much hang with the client not sending anything to trigger that read (why would it, it’s expecting to receive something).
Regards,
Graham
—
AW: svn commit: r1706275 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Graham Leggett [mailto:minfrin@sharp.fm]
> Gesendet: Mittwoch, 7. Oktober 2015 17:59
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1706275 -
> /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>
> On 07 Oct 2015, at 5:53 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
>
> >> As I understand we’re using openssl in non blocking mode, which means
> that openssl will ask us permission before attempting any read or write.
> >>
> >> The core will then in turn either read or write as requested by
> openssl based on the “sense” flags CONN_SENSE_WANT_READ or
> CONN_SENSE_WANT_WRITE.
> >>
> >> If openssl has a bug and reads/writes without first asking permission
> we’ll block, but by the same token if openssl as asking us permission
> using SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE and we’re ignoring
> openssl, we’ll block for the same reason.
> >
> > But certainly these are situations which it's "safer" to block
> > in any case, right? Of course, they could also be vectors for some
> > sort of DDoS, but even then, that would be relying on pretty
> > nasty bugs.
>
> The blocking isn’t really the problem, it’s accidentally waiting for a
> socket to be readable when openssl asked you to tell it when the socket
> is writable.
>
> I suspect turning on the “flush” is masking a bug.
I guess we are talking of different things. During the initial handshake (client or server) we never hand back
control to the event part of the MPM. We never use ssl_filter_write and ssl_io_filter_output as this is only used for data we want to encrypt on the connection. During the handshake we deal directly with the core output filter.
We are just synchronous here. And I think this is not a big deal as the amount of data that needs to be transferred here is low and buffered by the socket buffer anyway. So we would not really notice the difference between synchronous and asynchronous since the socket is not blocking / says its writable.
Regards
Rüdiger
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 07 Oct 2015, at 5:53 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
>> As I understand we’re using openssl in non blocking mode, which means that openssl will ask us permission before attempting any read or write.
>>
>> The core will then in turn either read or write as requested by openssl based on the “sense” flags CONN_SENSE_WANT_READ or CONN_SENSE_WANT_WRITE.
>>
>> If openssl has a bug and reads/writes without first asking permission we’ll block, but by the same token if openssl as asking us permission using SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE and we’re ignoring openssl, we’ll block for the same reason.
>
> But certainly these are situations which it's "safer" to block
> in any case, right? Of course, they could also be vectors for some
> sort of DDoS, but even then, that would be relying on pretty
> nasty bugs.
The blocking isn’t really the problem, it’s accidentally waiting for a socket to be readable when openssl asked you to tell it when the socket is writable.
I suspect turning on the “flush” is masking a bug.
Regards,
Graham
—
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Oct 7, 2015, at 5:17 AM, Graham Leggett <mi...@sharp.fm> wrote:
>
> On 07 Oct 2015, at 10:04 AM, Joe Orton <jo...@redhat.com> wrote:
>
>> That's really interesting. That extra buffering BIO makes sense if
>> OpenSSL is writing to the socket descriptor directly, as with pre-2.x
>> mod_ssl, but doesn't really make sense with 2.x mod_ssl, since the core
>> output filter is doing that work in the "right" place.
>>
>> I guess it doesn't impact performance much because it's handshake-time
>> only as you say, but still, it would be interesting to try ripping that
>> out.
>
> As I understand we’re using openssl in non blocking mode, which means that openssl will ask us permission before attempting any read or write.
>
> The core will then in turn either read or write as requested by openssl based on the “sense” flags CONN_SENSE_WANT_READ or CONN_SENSE_WANT_WRITE.
>
> If openssl has a bug and reads/writes without first asking permission we’ll block, but by the same token if openssl as asking us permission using SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE and we’re ignoring openssl, we’ll block for the same reason.
But certainly these are situations which it's "safer" to block
in any case, right? Of course, they could also be vectors for some
sort of DDoS, but even then, that would be relying on pretty
nasty bugs.
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 07 Oct 2015, at 10:04 AM, Joe Orton <jo...@redhat.com> wrote:
> That's really interesting. That extra buffering BIO makes sense if
> OpenSSL is writing to the socket descriptor directly, as with pre-2.x
> mod_ssl, but doesn't really make sense with 2.x mod_ssl, since the core
> output filter is doing that work in the "right" place.
>
> I guess it doesn't impact performance much because it's handshake-time
> only as you say, but still, it would be interesting to try ripping that
> out.
As I understand we’re using openssl in non blocking mode, which means that openssl will ask us permission before attempting any read or write.
The core will then in turn either read or write as requested by openssl based on the “sense” flags CONN_SENSE_WANT_READ or CONN_SENSE_WANT_WRITE.
If openssl has a bug and reads/writes without first asking permission we’ll block, but by the same token if openssl as asking us permission using SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE and we’re ignoring openssl, we’ll block for the same reason.
If there is some kind of buffer in between openssl and httpd, that will probably cause strange behaviour too, agreed that we should take that out.
Regards,
Graham
—
Re: svn commit: r1706275 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Oct 07, 2015 at 01:35:38AM +0200, Yann Ylavic wrote:
> For the server case, openssl will use its own buffering mechanism
> during the handshake "so that the output is sent in a way that TCP
> likes", according to the comment in the code, so we shouldn't be
> flushing small chunks.
> Yet for the server case still, openssl will issue its own flush
> appropriately, so we may introduce a spurious flush by doing this
> (something I didn't think about).
That's really interesting. That extra buffering BIO makes sense if
OpenSSL is writing to the socket descriptor directly, as with pre-2.x
mod_ssl, but doesn't really make sense with 2.x mod_ssl, since the core
output filter is doing that work in the "right" place.
I guess it doesn't impact performance much because it's handshake-time
only as you say, but still, it would be interesting to try ripping that
out.
> Thus I agree that Joe's proposal is better: SSL_in_connect_init() but
> for the buggy case (openssl < 0.9.8m) where we need the generic
> SSL_is_init_finished().
> Will commit this, thanks for the feedbacks.
As ever, looks great Yann, thanks a lot.
Regards, Joe
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 8:41 PM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
>
> I am confused now. I understood that the state machine for the server case is fine. Hence that it flushes automatically where needed. So we only should and need to take care of the client case.
> How does using !SSL_is_init_finished(ssl) simplifies the logic?
Sorry for the confusion.
What I meant is that it simplifies the logic in mod_ssl to use
SSL_is_init_finished() unconditionally, since it (obviously) addresses
all the cases: buggy openssl versions, client and server.
It is required for the former case anyway, and is identical to
SSL_in_connect_init() for the client case.
For the server case, openssl will use its own buffering mechanism
during the handshake "so that the output is sent in a way that TCP
likes", according to the comment in the code, so we shouldn't be
flushing small chunks.
Yet for the server case still, openssl will issue its own flush
appropriately, so we may introduce a spurious flush by doing this
(something I didn't think about).
Thus I agree that Joe's proposal is better: SSL_in_connect_init() but
for the buggy case (openssl < 0.9.8m) where we need the generic
SSL_is_init_finished().
Will commit this, thanks for the feedbacks.
Regards,
Yann.
AW: svn commit: r1706275 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Gesendet: Dienstag, 6. Oktober 2015 18:18
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1706275 -
> /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>
> On Tue, Oct 6, 2015 at 6:00 PM, Yann Ylavic <yl...@gmail.com>
> wrote:
> > On Tue, Oct 6, 2015 at 5:44 PM, Joe Orton <jo...@redhat.com> wrote:
> >>
> >> Hence In the server case, it seems reasonable to rely on BIO_flush()
> >> being called at the "right" times during the handshake. Modulo the
> odd
> >> bug!
> >>
> >> But ssl/s3_clnt.c is not following that coding style at all, and it
> only
> >> does a flush after completing the handshake. So I'd say the right
> thing
> >> here is to FLUSH after every packet which comes through the write BIO
> >> when the SSL state machine is in the middle of a "connect", i.e.
> >> handshake as client.
> >>
> >> tl;dr: I think Yann's patch should be right if the test is switched
> from
> >> "always flush if !SSL_is_init_finished(ssl)" to "always flush if
> >> SSL_in_connect_init(ssl)"???
> >
> > Yes, I came to the same conclusion, but decided to use
> > SSL_is_init_finished(ssl) anyway because for the server case it seems
> > that openssl uses it own buffering mechanism to avoid writing small
> > chunks (after the client-hello is received), so possibly we could rely
> > on it (this also simplifies the logic).
>
> Also it seems that for the SSL_ST_RENEGOTIATE state in ssl3_accept(),
> buffering is disabled by openssl (at least in 1.0.2d).
> SSL_is_init_finished(ssl) should catch this case too...
I am confused now. I understood that the state machine for the server case is fine. Hence that it flushes automatically where needed. So we only should and need to take care of the client case.
How does using !SSL_is_init_finished(ssl) simplifies the logic?
Regards
Rüdiger
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 6:00 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Oct 6, 2015 at 5:44 PM, Joe Orton <jo...@redhat.com> wrote:
>>
>> Hence In the server case, it seems reasonable to rely on BIO_flush()
>> being called at the "right" times during the handshake. Modulo the odd
>> bug!
>>
>> But ssl/s3_clnt.c is not following that coding style at all, and it only
>> does a flush after completing the handshake. So I'd say the right thing
>> here is to FLUSH after every packet which comes through the write BIO
>> when the SSL state machine is in the middle of a "connect", i.e.
>> handshake as client.
>>
>> tl;dr: I think Yann's patch should be right if the test is switched from
>> "always flush if !SSL_is_init_finished(ssl)" to "always flush if
>> SSL_in_connect_init(ssl)"???
>
> Yes, I came to the same conclusion, but decided to use
> SSL_is_init_finished(ssl) anyway because for the server case it seems
> that openssl uses it own buffering mechanism to avoid writing small
> chunks (after the client-hello is received), so possibly we could rely
> on it (this also simplifies the logic).
Also it seems that for the SSL_ST_RENEGOTIATE state in ssl3_accept(),
buffering is disabled by openssl (at least in 1.0.2d).
SSL_is_init_finished(ssl) should catch this case too...
Regards,
Yann.
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 6, 2015 at 5:44 PM, Joe Orton <jo...@redhat.com> wrote:
>
> Hence In the server case, it seems reasonable to rely on BIO_flush()
> being called at the "right" times during the handshake. Modulo the odd
> bug!
>
> But ssl/s3_clnt.c is not following that coding style at all, and it only
> does a flush after completing the handshake. So I'd say the right thing
> here is to FLUSH after every packet which comes through the write BIO
> when the SSL state machine is in the middle of a "connect", i.e.
> handshake as client.
>
> tl;dr: I think Yann's patch should be right if the test is switched from
> "always flush if !SSL_is_init_finished(ssl)" to "always flush if
> SSL_in_connect_init(ssl)"???
Yes, I came to the same conclusion, but decided to use
SSL_is_init_finished(ssl) anyway because for the server case it seems
that openssl uses it own buffering mechanism to avoid writing small
chunks (after the client-hello is received), so possibly we could rely
on it (this also simplifies the logic).
Re: svn commit: r1706275 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Oct 06, 2015 at 02:37:32PM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> The drawback is that it will flush every time the handshake writes.
> The handshake may write multiple times before it wants to read.
> So the current approach probably causes bigger amounts of data sent
> across the wire at once then the approach below and thus is more in line with the standard
> approach in httpd to avoid sending small chunks if not needed.
> So I would keep the current approach.
Looking at 1.0.1e, the state machine loop in ssl3_accept(), the coding
style is pretty clearly:
1. send something
2. flush
3. switch to new state.
e.g. like this....
ret=ssl3_send_hello_request(s);
if (ret <= 0) goto end;
s->s3->tmp.next_state=SSL3_ST_SW_HELLO_REQ_C;
s->state=SSL3_ST_SW_FLUSH;
...and then the SW_FLUSH mode switches to the tmp.next_state.
Hence In the server case, it seems reasonable to rely on BIO_flush()
being called at the "right" times during the handshake. Modulo the odd
bug!
But ssl/s3_clnt.c is not following that coding style at all, and it only
does a flush after completing the handshake. So I'd say the right thing
here is to FLUSH after every packet which comes through the write BIO
when the SSL state machine is in the middle of a "connect", i.e.
handshake as client.
tl;dr: I think Yann's patch should be right if the test is switched from
"always flush if !SSL_is_init_finished(ssl)" to "always flush if
SSL_in_connect_init(ssl)"???
Regards, Joe
AW: svn commit: r1706275 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Gesendet: Dienstag, 6. Oktober 2015 16:06
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1706275 -
> /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>
> On Thu, Oct 1, 2015 at 8:22 PM, Ruediger Pluem <rp...@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?
The drawback is that it will flush every time the handshake writes.
The handshake may write multiple times before it wants to read.
So the current approach probably causes bigger amounts of data sent
across the wire at once then the approach below and thus is more in line with the standard
approach in httpd to avoid sending small chunks if not needed.
So I would keep the current approach.
Regards
Rüdiger
Re: svn commit: r1706275 - /httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Oct 1, 2015 at 8:22 PM, Ruediger Pluem <rp...@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.
Re: svn commit: r1706275 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/01/2015 06:59 PM, Graham Leggett wrote:
> On 01 Oct 2015, at 5:43 PM, ylavic@apache.org wrote:
>
>> URL: http://svn.apache.org/viewvc?rev=1706275&view=rev
>> Log:
>> mod_ssl: follow up to r1705823.
>> We still need to flush in the middle of a SSL/TLS handshake.
>
> Can you confirm why the flushing is necessary?
>
> In theory mod_ssl should be switching the sense of any reads/writes as necessary without any need for flushing.
>
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.
The whole game of reading and writing during the handshake happens inside openssl while SSL_connect is running.
Apache code only gets back into this via bio_filter_out_write and bio_filter_in_read.
Regards
Rüdiger