[Twisted-Python] HTTP Agent persistent connections not closed for some HTTPS sites

[Adi Roiban]

Hi,

It looks like when connected to some HTTPS servers, the TLS
connection is not successfully closed.

With sites like google.com, the connection is successfully closed.

But with Office365 sharepoint.com sites the TLS shutdown is not completed.

Has anyone else observed this behaviour ?

I have observed this while running some end to end tests in which the
pool.closeCachedConnections() deferred was not called, even after a
generous amount of seconds :)

--------

the TLSMemoryBIOProtocol._tlsConnection.shutdown() will return False
and the connection is left open.

Future calls to tlsConnection will result in WantReadError

From my understanding WantReadError means that more data needs to be
read and passed to the protocol.

And If I try to read from the raw socket I get this

data = TLSMemoryBIOProtocol.transport.socket.recv(1000)
*** error: [Errno 11] Resource temporarily unavailable

and the socket is never made available and in the end it is closed with

data = TLSMemoryBIOProtocol.transport.socket.recv(100)
*** error: [Errno 104] Connection reset by peer

-----------

Here is a gits with the example code and some example output
https://gist.github.com/adiroiban/346dd455094d1762f0e69e9812309ad6

The gist also contains a patched TLSMemoryBIOProtocol._shutdownTLS
which will close the connection after some time.

PS: TLSMemoryBIOProtocol._tlsConnection.shutdown() make reference to
this issue https://github.com/pyca/pyopenssl/issues/91... but the
issue is now closed

Thanks!

[Tristan Seligmann]

Post by Adi Roiban
I have observed this while running some end to end tests in which the
pool.closeCachedConnections() deferred was not called, even after a
generous amount of seconds :)

The code to abort an HTTP client connection is here:

https://github.com/twisted/twisted/blob/twisted-17.1.0/src/twisted/web/_newclient.py#L1657

This calls loseConnection which for a TLS connection will try to do a TLS
shutdown under most circumstances (in some cases it can't, and will
abortConnection on the underlying transport instead). If the remote end has
stopped responding to the connection, I think this may end up hanging
forever.

I think this code should either call abortConnection directly, or set a
timer which will abort the connection after a little while if a clean
shutdown from loseConnection has not completed yet. I'

[Glyph Lefkowitz]

I think this code should either call abortConnection directly, or set a timer which will abort the connection after a little while if a clean shutdown from loseConnection has not completed yet. I'

Agreed - did someone file a bug? :)

-g

[Paweł Miech]

Post by Adi Roiban
It looks like when connected to some HTTPS servers, the TLS

connection is not successfully closed.

Post by Adi Roiban
But with Office365 sharepoint.com sites the TLS shutdown is not completed.
Has anyone else observed this behaviour ?

I observed something simillar a while ago, there was bug report created for
it in Scrapy. I'm not 100% sure this was same issue as you're discussing
here. Issue in Scrapy: https://github.com/scrapy/scrapy/issues/985 fix in
Scrapy https://github.com/scrapy/scrapy/pull/999 Author of this PR to
Scrapy also created Twisted ticket
https://twistedmatrix.com/trac/ticket/7738

Hope it helps

Post by Adi Roiban
I think this code should either call abortConnection directly, or set a
timer which will abort the connection after a little while if a clean
shutdown from loseConnection has not completed yet. I'
Agreed - did someone file a bug? :)
-g
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Cory Benfield]

Post by Adi Roiban
I have observed this while running some end to end tests in which the
pool.closeCachedConnections() deferred was not called, even after a
generous amount of seconds :)
https://github.com/twisted/twisted/blob/twisted-17.1.0/src/twisted/web/_newclient.py#L1657 <https://github.com/twisted/twisted/blob/twisted-17.1.0/src/twisted/web/_newclient.py#L1657>
This calls loseConnection which for a TLS connection will try to do a TLS shutdown under most circumstances (in some cases it can't, and will abortConnection on the underlying transport instead). If the remote end has stopped responding to the connection, I think this may end up hanging forever.
I think this code should either call abortConnection directly, or set a timer which will abort the connection after a little while if a clean shutdown from loseConnection has not completed yet. I’

Yeah, this looks right. We hit it in the server side code too: it’s a very unintuitive API in that sense.

Cory

[Adi Roiban]

Post by Tristan Seligmann

Post by Adi Roiban
I have observed this while running some end to end tests in which the
pool.closeCachedConnections() deferred was not called, even after a
generous amount of seconds :)

https://github.com/twisted/twisted/blob/twisted-17.1.0/src/twisted/web/_newclient.py#L1657
This calls loseConnection which for a TLS connection will try to do a TLS
shutdown under most circumstances (in some cases it can't, and will
abortConnection on the underlying transport instead). If the remote end has
stopped responding to the connection, I think this may end up hanging
forever.
I think this code should either call abortConnection directly, or set a
timer which will abort the connection after a little while if a clean
shutdown from loseConnection has not completed yet. I’
Yeah, this looks right. We hit it in the server side code too: it’s a very
unintuitive API in that sense.

I am not sure about which code are we talking here.
The specific HTTP11ClientProtocol which will fix only the HTTP client
part or the generic TLSMemoryBIOProtocol code which might fix any TLS
connection?

I have observed this issue on the server side for the case in which a
TLS connection is opened and then closed without transferring any
data ... and then the handshake will not have the chance to complete.

I think that this is the same issue which was reported for Scrapy

[Cory Benfield]

Post by Adi Roiban
I am not sure about which code are we talking here.
The specific HTTP11ClientProtocol which will fix only the HTTP client
part or the generic TLSMemoryBIOProtocol code which might fix any TLS
connection?

I’m talking about the fact that calling TLSMemoryBIOProtocol.loseConnection may not ever actually drop the underlying FD is the surprising part of the code. We had to work around it in the server side by adding a timeout after loseConnection is called to end up calling abortConnection.

Cory

[Adi Roiban]

Post by Adi Roiban
I am not sure about which code are we talking here.
The specific HTTP11ClientProtocol which will fix only the HTTP client
part or the generic TLSMemoryBIOProtocol code which might fix any TLS
connection?
I’m talking about the fact that calling TLSMemoryBIOProtocol.loseConnection
may not ever actually drop the underlying FD is the surprising part of the
code. We had to work around it in the server side by adding a timeout after
loseConnection is called to end up calling abortConnection.
Cory

In order to submit a patch, I am trying to write an automated tests for this.
Were you able to reproduce this issue in a controlled environment.

I am still not fully understanding why I get this behaviour with
sharepoint.com sites.

I am thinking at having a rigged TLS server which will keep the socket
open but will not
respond to the shutdown request.... but I still don't know how :)

[Jean-Paul Calderone]

Post by Tristan Seligmann

Post by Adi Roiban
I am not sure about which code are we talking here.
The specific HTTP11ClientProtocol which will fix only the HTTP client
part or the generic TLSMemoryBIOProtocol code which might fix any TLS
connection?
I’m talking about the fact that calling TLSMemoryBIOProtocol.

loseConnection

Post by Adi Roiban
may not ever actually drop the underlying FD is the surprising part of

the

Post by Adi Roiban
code. We had to work around it in the server side by adding a timeout

after

Post by Adi Roiban
loseConnection is called to end up calling abortConnection.
Cory

In order to submit a patch, I am trying to write an automated tests for this.
Were you able to reproduce this issue in a controlled environment.
I am still not fully understanding why I get this behaviour with
sharepoint.com sites.
I am thinking at having a rigged TLS server which will keep the socket
open but will not
respond to the shutdown request.... but I still don't know how :)

Run the server with TLSMemoryBIOProtocol and use the transport's
pauseProducing method before you attempt the shutdown?

Jean-Paul

Post by Tristan Seligmann
--
Adi Roiban
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Cory Benfield]

Run the server with TLSMemoryBIOProtocol and use the transport's pauseProducing method before you attempt the shutdown?
Jean-Paul

Yup, that will work. Similarly, having two TLSMemoryBIOProtocols communicating together and pausing one before calling loseConnection on the other should also work. Essentially, any situation you can conjure where the closing connection will manage not to see either a TLS CloseNotify or a TCP FIN or RST will do the job.

Cory

[Jean-Paul Calderone]

Post by Jean-Paul Calderone
Run the server with TLSMemoryBIOProtocol and use the transport's
pauseProducing method before you attempt the shutdown?
Jean-Paul
Yup, that will work. Similarly, having two TLSMemoryBIOProtocols
communicating together and pausing one before calling loseConnection on the
other should also work. Essentially, any situation you can conjure where
the closing connection will manage not to see either a TLS CloseNotify or a
TCP FIN or RST will do the job.

Indeed. For that matter, you can have them interact in-memory (maybe
StringTransport is good enough as-is, maybe not, I'm not sure) and then
just stop shuttling the bytes in one direction. (I/O-free tests for the
win)

Jean-Paul