Hello Request breaks pyOpenSSL

[mhils]

Hi there,

I tried to hunt down mitmproxy/mitmproxy#472 today and encountered a strange pyOpenSSL bug. If the server

1. requests renegotiation (Hello Request),
2. (unsuccessfully) asks for a client certificate during renegotation and then
3. sends some encrypted data, I cannot read that data using pyOpenSSL.

Here's my testcase - I think I'm doing nothing special:

from OpenSSL import SSL
import socket

ctx = SSL.Context(SSL.SSLv23_METHOD)
sock = SSL.Connection(ctx, socket.socket(socket.AF_INET, socket.SOCK_STREAM))
sock.connect(("exchangedev.taskbox.co", 443))
sock.do_handshake()

def dump_creds():
    crand = sock.client_random().encode("hex")
    masterkey = sock.master_key().encode("hex")
    with open("keys","ab") as f:
        f.write("CLIENT_RANDOM {} {}\r\n".format(crand, masterkey))

dump_creds()

sock.send("GET /Microsoft-Server-ActiveSync HTTP/1.1\r\n"
          "Host: exchangedev.taskbox.co\r\n"
          "\r\n"
          "\r\n")

try:
    print(sock.recv(1))
finally:
    dump_creds() # renegotiated master key

Here's the output:

C:\Users\user\git\mitmproxy\test\>python test.py
Traceback (most recent call last):
  File "test.py", line 23, in <module>
    print(sock.recv(1))
  File "C:\Python27\lib\site-packages\OpenSSL\SSL.py", line 995, in recv
    self._raise_ssl_error(self._ssl, result)
  File "C:\Python27\lib\site-packages\OpenSSL\SSL.py", line 847, in _raise_ssl_error
    raise WantReadError()
OpenSSL.SSL.WantReadError

Now, that clearly looks like I'm not getting any data back. However, looking at Wireshark, I see this:

Similarly, using openssl s_client -connect exchangedev.taskbox.co:443, I get the response.

Here's the pcap dump with the corresponding SSL keys for Wireshark (Protocol Options -> SSL -> Master-Secret log file). For the lazy, here's also a screenshot:

The server mentioned above is on the public internet, and according to mitmproxy/mitmproxy#472 (comment) you're welcome to use it for testing. If there's anything else I can help with, please let me know! 😃

Thanks!
Max

[iragsdale]

As the administrator of the server mentioned, I can confirm you're welcome to test against it, and if there's anything I can provide to make that simpler, please let me know.

[mhils]

Ok, it is actually mentioned in the OpenSSL docs that a blocking socket may raise a SSL_ERROR_WANT_READ...

If the underlying BIO is blocking, SSL_read() will only return, once the read operation has been finished or an error occurred, except when a renegotiation take place, in which case a SSL_ERROR_WANT_READ may occur. This behaviour can be controlled with the SSL_MODE_AUTO_RETRY flag of the SSL_CTX_set_mode call.

Using sock.recv(1, SSL._lib.SSL_MODE_AUTO_RETRY) seems to fix the issue. I'll push a PR that moves the constant to the SSL namespace in a minute.

Looking at SSL.py#L298-L301, you apparently set this flag by default in a previous version or intended to set it in a future version? Was this left out on purpose?

[exarkun]

Ah. Thanks for tracking that down. The flag wasn't really dropped
entirely on purpose but the fact that it didn't get re-added means that
there was never any test coverage demonstrating the feature it provides.
I'd be entirely happy to re-add the setting-by-default behavior if you also
want to add a unit test for it. Thanks again.

On Fri, Feb 27, 2015 at 10:06 PM, Maximilian Hils notifications@github.com
wrote:

Ok, it is actually mentioned in the OpenSSL docs that a blocking socket
may raise a SSL_ERROR_WANT_READ:

If the underlying BIO is blocking, SSL_read() will only return, once the
read operation has been finished or an error occurred, except when a
renegotiation take place, in which case a SSL_ERROR_WANT_READ may occur.
This behaviour can be controlled with the SSL_MODE_AUTO_RETRY flag of the
SSL_CTX_set_mode call.

Using sock.recv(1, SSL._lib.SSL_MODE_AUTO_RETRY) seems to fix the issue.
I'll push a PR that moves the constant to the SSL namespace in a minute.

Looking at SSL.py#L298-L301

pyopenssl/OpenSSL/SSL.py

Lines 298 to 301 in 496f40d

	# SSL_CTX_set_mode(self->ctx, SSL_MODE_ENABLE_PARTIAL_WRITE |
	# SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER |
	# SSL_MODE_AUTO_RETRY);
	self.set_mode(_lib.SSL_MODE_ENABLE_PARTIAL_WRITE)

,
you apparently set this flag by default in a previous version? Was this
left out on purpose?

—
Reply to this email directly or view it on GitHub
#190 (comment).

[mhils]

I'd be entirely happy to re-add the setting-by-default behavior if you also
want to add a unit test for it. Thanks again.

Any hints how could get that going?
I added SSL_renegotiate to cryptography, wrote a test case that renegotiates, but apparently due to loopback that's happening too fast to trigger the WantReadError. I can't really come up with a way how to add latency to the renegotiation here, which would be required to fail the test.

Alternatively, I'd be happy to send a PR that re-adds the setting with an explanatory comment instead.

[exarkun]

Any hints how could get that going?

If you use a memory BIO connection then you'll have full control over when bytes get delivered. It seems like this should let you deterministically trigger the WantReadError case: just be sure not to deliver the bytes OpenSSL wants to read before you do the relevant app-level recv. It'll probably be a bit more verbose than the _loopback-based test because you become responsible for moving bytes around instead of letting the kernel do it. But it seems like what's necessary to exercise this case.

[GordonMcKinney]

Any progress on this? [i'm not a python dev]

[mhils]

We have a workaround for pyOpenSSL 0.14 in the latest mitmproxy/netlib master - I haven't gotten around writing a proper test case yet and fix this upstream, sorry.

[GordonMcKinney]

Should I do a local build or is there an OSX binary I can test with? (bin preferred)

[mhils]

I tried implementing this using a memory BIO, but I'm stuck at this point. No matter whether I specify SSL_MODE_AUTO_RETRY, I always get a WantReadError if I try to recv() - the recv() call just never blocks. Is that due to the memory BIO? Any way how I can circumvent that?

Here's my current status:
https://gist.github.com/mhils/ddf740119c93c721d577