SSL connection hangs open with firefox, can't reverseproxy things on a HTTPS server.

[Tv]

[see http://divmod.org/svn/Nevow/sandbox/tv/https-proxy-hang for the code]
There's a bug somewhere that makes firefox hang in some situations
when talking to https URLs.
First of all, run "./createkey" to set up the SSL keys.
There are two ways to reproduce it:
The original way
================
This is how I discovered this bug.
A "public" server proxies some requests over to "private",
in effect
  browser --HTTP--> public --HTTP--> private
now, when browser uses HTTPS to talk to public, the page
never finishes loading.
  browser --HTTPS--> public --HTTP--> private
twistd --pidfile public.pid -noy public.tac
twistd --pidfile private.pid -noy private.tac
Use firefox to browse to http://localhost:8080/
Click on the plaintext link, see how the page completes loading.
Click on the HTTPS link, see how the spinner keeps spinning and the
mouse pointer has a clock. The connection never closes down.
The minimal way
===============
I _hope_ this is the same bug.
twistd --pidfile minimal.pid -noy minimal.tac
Browse to http://localhost:8084/ and see how the browser quickly
aborts loading the page.
Browse to https://localhost:8083/ and see how it hangs.
A Clue
======
I've traced the bug far enough to know the patch in makes-it-work.diff
eliminates this symptom.

[Tv]

<foom> Tv: using ssltap, I see that firefox never sends back an SSL alert
<Tv> foom: I know that.
<foom> Tv: okay. So I guess the real question is, what does apache do. :)
<Tv> foom: Also, what's different from the case with no proxy -- because then
there is no hang.
<Tv> just normal apache2 does so much keep-alive that seeing the same thing
happen is quite hard.
<Tv> So is this about no content-length?
<Tv> That would explain why firefox shows busy -- it doesn't know it received
everything already.
<Tv> Hmm.
<foom> Tv: the difference with proxy is that the proxy forces connection close,
I think
<Tv> foom: Yeah, it seems to do that, but I thought that was just because it was
giving a non-chunked streaming result.
<foom> Tv: when firefox talks to apache it sends back a close alert just fine
<Tv> I bet the apache content had a content-length in it.
<foom> er actually I lie.
<foom> Apache sends a close alert and immediately shuts the socket
<Tv> foom: A-ha!
<foom> and *then* firefox responds with a close alert. :P
<Tv> foom: I think it does a shutdown and not a real close, then.
<Tv> I don't see why you couldn't half-close the socket after saying you will
never say anything again
<Tv> Is there something against that in the SSL spec?
<foom> Tv: i'm using a proxy, so, I don't know if it ever gets back to apache or
not.
<foom> Tv: but I don't think a shutdown works because ssl needs two way
communications even for one way sending, IIRC
<Tv> Even for the final alert?
<Tv> I don't see any two-way for that in ethereal.
<Tv> Way too long keep-alive, this is horrible to debug with apache2<->firefox.
<Tv> The connection doesn't close for atleast a minute.
<foom> Tv: you can turn off keepalive in apache config somehow
<foom> okay, here we go:
<foom>     else if (ap_ctx_get(conn->client->ctx,
"ssl::flag::accurate-shutdown") == PTRUE) {
<foom>         /* send close notify and wait for clients close notify
<foom>            (standard compliant, but usually causes connection hangs) */
<foom>     else {
<foom>         /* send close notify, but don't wait for clients close notify
<foom>            (standard compliant and safe, so it's the DEFAULT!) */
<foom> so basically, firefox (and likely all mozillas before it) is a piece of
crap, so apache works around
<Tv> foom: I just talked to https://sourceforge.net/ with firefox, and I see
e.g. alert and FIN in the same package.
<Tv> foom: Hmm, that says that both modes of operation are standards compliant.
<foom> aha.
<foom> according to the TLS RFC, it is acceptable to only send the ``close
notify'' alert but to not wait for the peer's answer, when the underlying
connection is closed
<foom> (according to openssl docs)
<foom> I believe that more than I believe mod_ssl source.
<foom> In that case, I think we can remove the wait-for-other-side-to-send-close
bit from twisted.
<foom> but, pyopenssl is missing the function we need
<foom> :(
<foom> twisted should call SSL_set_shutdown(SSL_RECEIVED_SHUTDOWN) and then call
SSL_shutdown
<foom> but it can't do the first
<foom> perhaps you'd like to lobby for that function to be added to pyopenssl
<foom> actually, there's a .6
<foom> lemme see if that has it
<foom> nope.
<Tv> foom: Err, what does that mean? That the functionality cannot be
implemented without adding a wrapper for one more C function?
<foom> Tv: yes./
<Tv> foom: Err, surely it _can_. I mean, I already kludged it :) You mean
without that extra function the session state is lost and must be renegotiated?
<Tv> foom: (so you can detect whether pyopenssl has it, and use it if not, and
just break SSL sessions if pyopenssl is too old to have it)
<Tv> s/not/it has/
<foom> Tv: i'm not entirely sure what breaks if you don't set that, but mod_ssl
does it that way, so I'm assuming it's for a reason.
<foom> Tv: I think because SSL_shutdown uses that flag to determine when it's done
<Tv> The worst you can loose is speedy startup of next TCP connection.
<foom> Tv: you could also lose data
<Tv> Hmm.
<foom> Tv: if you send a bunch of data and then immediately close the
connection, you could lose some of it
<Tv> No, wait.
<Tv> What is the part that makes SSL wait for the incoming shutdown?
<Tv> Does it do that always if SSL_shutdown is called and it so far hasn't seen one?
<Tv> <foom> twisted should call SSL_set_shutdown(SSL_RECEIVED_SHUTDOWN) and then
call SSL_shutdown
<Tv> and that basically lies to it?
<foom> Tv: yeah, SSL_shutdown returns 0 which means not done yet
<Tv> foom: I think what your saying is not 100% in agreement with man SSL_shutdown
<foom> Tv: maybe mod_ssl is being overprotective. let me go read the ssl docs again.
<Tv> I don't see why you'd need to set SSL_RECEIVED_SHUTDOWN manually.
<Tv> foom: I mean, the man page says one call for unidirectional (what we want),
two calls for bidirectional (what twisted does now).
<foom> Tv: yes, i concur.
<Tv> foom: But I don't know how one ensures SSL has flushed data, in general.
That's still needed after SSL_shutdown
<foom> Tv: I think you can assume it has after shutdown returns 0 or 1
<Tv> But it seems there's nothing special in that, it's the same way as with any
normal data..
<foom> Tv: i notice we don't properly handle the error conditino
<Tv> Heh.
<foom> Tv: SSL_ERROR_WANT_WRITE/READ from SSL_shutdown is unhandled
<foom> Tv: i think that means if you send a lot of data, and then lose the
connection, the shutdown will not occur and furthermore the socket will be
closed without finishing sending the data.

[Tv]

<foom> Tv: okay, I've got it now. :) _postLoseConnection should send the
shutdown (and if it fails, retry when socket becomes readable/writable), and
lose the socket. _closeWriteConnection (from halfCloseConnection write=True)
should call shutdown (and retry etc) but not lose the socket.

[jknight]

Working on it. Turns out SSL_shutdown is a piece of crap, so some workaround is necessary. Attached
patch may do the right thing. Also need the patch for pyOpenSSL .6 to have get/set_shutdown. I
submitted it to martin strakt, hopefully it'll get in next pyOpenSSL.
I don't think there's a way to do the right thing without that call.

[jknight]

Tv: Please verify fixed in trunk and resolve if it's good now.

[exarkun]

Ping. Earth to Tv? Any comments on this?

[jknight]

I don't believe above patch ever got into pyOpenSSL, although I admit to not having checked very recently. :(

[exarkun]

I think Strakt is maintaining PyOpenSSL now. Maybe we should toss the patch over to someone there?

[exarkun]

The PyOpenSSL patch has been applied, at least. It's in the PyOpenSSL repository now hosted on Launchpad. As soon as I get a release together, it will be part of it.

[collab]

Replying to exarkun:

The PyOpenSSL patch has been applied, at least. It's in the PyOpenSSL repository now hosted on Launchpad. As soon as I get a release together, it will be part of it.

Guess this one can be closed, the patch was included in PyOpenSSL 0.7, released on 2008-04-11.

[exarkun]

I think Twisted is missing test coverage for this feature. Also, it's broken. See #3218