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

[Plüm , Rüdiger , Vodafone Group]

> -Ursprüngliche Nachricht-
> Von: Graham Leggett [mailto:minf...@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  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

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

[Plüm , Rüdiger , Vodafone Group]

> -Ursprüngliche Nachricht-
> Von: Yann Ylavic [mailto:ylavic@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 
> 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

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

[Plüm , Rüdiger , Vodafone Group]

> -Ursprüngliche Nachricht-
> Von: Yann Ylavic [mailto:ylavic@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 
> wrote:
> > On Tue, Oct 6, 2015 at 5:44 PM, Joe Orton  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