DTLSv1.2 methods and extensions added

[mcfreis]

Hi Ray,

I splitted my added functionality up in different commits/patches and also added a unit test for the wrapper. Would you please add these in your master branch?

I tested the unit.py and unit_wrapper.py before each commit step under Win7 with Python 2.7.6, under Redhat Linux with Python 2.7.6 and 2.7.12 with success.

The openSSL Lib 1.0.2l-dev is taken from 1.0.2-stable on the day of commit and compiled on my Win7 host.

Regards, Björn

[rbit]

Nice separation; thank you!

I'm able to execute the unit tests on Windows. On Ubuntu (16.04), however, an ImportError occurs:

python -m dtls.test.unit   
/usr/bin/python: cannot import name PROTOCOL_SSLv3

The ssl module does not define that name on that platform.

More generally, we should stick with importing the ssl module only if patch.py's dopatch is actually called, considering that the package's __init_\ imports the patch module. PyDTLS should work in environments where the ssl module does not exist.

[mcfreis]

Hi,

regarding your comment on Ubuntu, I'm not sure if it is an Ubuntu issue. It sounds more like a problem to the library in use (OpenSSL) or the way Python is compiled on Ubuntu. Which library do you use?

I compiled Python 2.7.6 and 2.7.12 on a RedHat system and can use this import without any problems.

For the package's init, I would agree. For that I have to exclude the wrappers from the init.py, correct?

[rbit]

On Ubuntu, libcrypto.so.1.0.0 and libssl.so.1.0.0 are part of package libssl1.0.0. This package is preinstalled on desktop and server Ubuntu configurations, as is Python2. So PyDTLS will run on Ubuntu upon download and installation, or directly out of the git cloned directory, without needing to install anything else. This is why I included OpenSSL dynamic libraries for Windows only. I'm not sure about RedHat, though I recommend testing with a yum package rather than a self-built OpenSSL library. In any case, we should preserve the ability to run on Ubuntu without any additional installation requirements.

You're correct: the wrapper can't be imported by the package __init__, since it in turn imports Python's ssl module.

As an aside, I've noticed that there are now inconsistent line endings in some files (e.g., sslconnection.py, openssl.py, etc.). Some lines you've edited now end in CRLF, while other lines remain with the original LF. Please fix this: we should continue to use LF on all development platforms, without needing to configure git to compensate.

Thank you much!

[mcfreis]

Fixed line endings to LF, removed wrapper import from __init__.py and removed PROTOCOL_SSLv3 import (not needed anymore inside patch.py)

Tests unit.py and unit_wrapper.py run fine on Win7 with Python 2.7.6 and on Redhat with Python 2.7.6 and 2.7.12 (self-compiled). I hope this does the job.

Regards

[rbit]

Copying comment to more visible position in conversation:

I recall you asked about this earlier. I've looked through the OpenSSL code, and the logic around MTU determination sure has changed a lot. I think that just setting this to the 802.3 default won't work, though: there can be many reasons that path MTU might be lower than this. One common scenario is DSL: PPPoE induces an 8-byte overhead, reducing the MTU to 1492 right at the network border. I think that PyDTLS would just fail now with this hardcode in such situations.

In bss_dgram.c I see that on Linux, OpenSSL tries to determine the MTU from sockopt IP_MTU as part of BIO_CTRL_DGRAM_QUERY_MTU. On Windows, this path is not taken, and this query just returns 0. The caller then falls into a different code path, which computes a minimum by subtracting a protocol-dependent overhead from a guess.

So I think we need to trace though this and find out why that's not working on Windows. Would you like to do that with a debug version of the OpenSSL dll's you built?

[mcfreis]

Tracing would be a good idea, but how? Having a debug build of the OpenSSL lib and the Python wrapper, what to do then?

Concerning the MTU ... looking at a wireshark capture and not setting any MTU size on Windows gives/uses a default MTU size of 256 - 28 = 228. This fragments the Hello message and the server wont answer.

From the source file d1_both.c:

/* XDTLS:  figure out the right values */
static const unsigned int g_probable_mtu[] = { 1500, 512, 256 };

and

unsigned int dtls1_link_min_mtu(void)
{
    return (g_probable_mtu[(sizeof(g_probable_mtu) /
                            sizeof(g_probable_mtu[0])) - 1]);
}

unsigned int dtls1_min_mtu(SSL *s)
{
    return dtls1_link_min_mtu() - BIO_dgram_get_mtu_overhead(SSL_get_wbio(s));
}

and

static int dtls1_query_mtu(SSL *s)
{
    if (s->d1->link_mtu) {
        s->d1->mtu =
            s->d1->link_mtu - BIO_dgram_get_mtu_overhead(SSL_get_wbio(s));
        s->d1->link_mtu = 0;
    }

    /* AHA!  Figure out the MTU, and stick to the right size */
    if (s->d1->mtu < dtls1_min_mtu(s)) {
        if (!(SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)) {
            s->d1->mtu =
                BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_QUERY_MTU, 0, NULL);

            /*
             * I've seen the kernel return bogus numbers when it doesn't know
             * (initial write), so just make sure we have a reasonable number
             */
            if (s->d1->mtu < dtls1_min_mtu(s)) {
                /* Set to min mtu */
                s->d1->mtu = dtls1_min_mtu(s);
                BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_SET_MTU,
                         s->d1->mtu, NULL);
            }
        } else
            return 0;
    }
    return 1;
}

Due to Windows not supporting the ioctl to fetch the MTU size, this will always result in 228 unless configured externally. But again only for Windows. Maybe the solution in SSLConnection.__init__() is as follows:

if sys.platform.startswith('win') and not (SSL_get_options(self._ssl.value) & SSL_OP_NO_QUERY_MTU):
    SSL_set_options(self._ssl.value, SSL_OP_NO_QUERY_MTU)
    DTLS_set_link_mtu(self._ssl.value, 1500)

Placed directly after a user configuration to backup the Windows problem in case the user didn't configure it.

Attached is a capture with the different settings:

• First sequence leaving the MTU size to the library -> fails as expected
• Secondly user config to 512 MTU size (link) -> is fine, fragmentation is visible
• Lastly with default of 1500 from SSLConnection.__init__() just in case -> works fine too

Maybe we need to choose the default more wisely.

What do you think?

[mcfreis]

I created a pull request on my fork for a patch of the above. I'll merge it if you like it to be contained in this pull request.

Please have a look at: https://github.com/mcfreis/pydtls/pull/4

[rbit]

I traced through the case where handshaking fails on Windows when you don't indicate MTU, and let OpenSSL select the 228 default. In this case the ClientHello datagram fragments (as you found in your Wireshark trace), and that appears to be the root of the problems that follow.

OpenSSL falls into its record layer fragment reassembly path. This tries to do a recvfrom from the datagram BIO that represents the demuxed socket. The first time this works, and the first ClientHello fragment arrives. Then it tries to assemble the second fragment, but here recvfrom fails with EWOULDBLOCK, as the routing demux hasn't yet forwarded any more datagrams. It looks to me like this code could work if timing were right and the next datagram had already been received and were ready for recvfrom (but see caveats below). But since there's nothing at the demux socket, datagram reassembly unwinds.

The eventual return value to sslconnection.py's DTLSv1_listen invocation is actually SSL_ERROR_WANT_READ, and listen re-invocation happens as expected, forwarding the next fragment to the demuxed socket and calling DTLSv1_listen again. But it looks to me like this function wasn't really intended to be resumable in this case: in d1_srvr.c's dtls1_accept, we run into the SSL_clear call near the top, which appears to erase the prior state, and so the second fragment isn't being assembled together with the first one. And so every re-transmission of ClientHello fails in the same way.

It might be possible to have the routing demux do a recvfrom more datagrams from the same peer, and then forward them before the first invocation of DTLSv1_listen. This probably would port the OpenSSL timing dependency to PyDTLS - not sure if this is good: if the second fragment is delayed enough for Windows to return EWOULDBLOCK, then things will still fail.

Then there's also the matter of interleaving ClientHello fragments from different clients. On a busy server, the sequence of datagrams from connecting peers might be     ... <Client 1, fragment 1> - <Client 2, fragment 1> - <Client 1, fragment 2> ... In this case, it looks to me like neither client 1 nor client 2 would connect, since the OpenSSL UDP read logic just pulls the datagrams in order, fails to reassemble them, and then aborts the accept. So both clients would fall into their retry logic, and hope for a better datagram ordering on the next try. The way to avoid this would be to put the socket into a connected state to client 1 temporarily and during re-assembly, but (a) I don't see this happening in OpenSSL, and (b) I'm not sure whether in that state the third datagram in my example above can be read ahead of the second, and (c) even if it can, I don't know if that will preserve the second datagram's position in the queue.

So my feeling right now is that fragmenting ClientHello is probably a bad thing. I think you're right: we need a better default MTU. One large enough to accommodate all reasonable cases of ClientHello, but small enough not to exceed any realistic path MTU. Not a totally trivial question, since the ClientHello cipher_suites parameter can be up to 2^16-1 in length.

I haven't checked whether the MTU we set affects packets later sent by the application, after the handshake. If it does, then perhaps we should not set any default, but force the application to set it - after all it knows what size datagrams it tries to send. And having those datagrams fragmented without that being expected is probably bad, since it would lower performance without there being a reason the developer could easily pinpoint.

[mcfreis]

Wow, you sure took a deep look into OpenSSL.

A default MTU on Windows saves the application and the developer from the situation "nothig works", and "lets grab another solution". Later on, if basic communication works, then the application can fine tune the MTU. That would be my thought about that.

I'm not sure if we have to solve this within this pull request. What do you think?

In the meanwhile I would stick to 576 as a default value.

[rbit]

Yeah, sounds like a reasonable value. But would you please check that with that value set, datagrams submitted by the application (as opposed to handshake datagrams emitted by OpenSSL) don't get fragmented. In other words, if the application submits a datagram of size 1459 bytes, that datagram doesn't get fragmented by OpenSSL if we set 576 as the MTU.

[rbit]

I checked myself, and the result looks correct: the limit doesn't apply to application datagrams.

I'm almost done with the new release now. I checked over the import tables of the Windows dll's you built, and it looks like you used a recent version of Visual Studio. This is a problem, because it forces users to acquire or have installed the Visual C++ redistributable package corresponding to the version of Visual Studio you used. The way to avoid this is to use the version of the compiler used by the version of Python we support (msvcr90.dll of of VC++9 / Visual Studio 2008). I can just generate new libraries on my machine. I'm thinking of using OpenSSL-1.0.2k, the current stable. Thoughts?

[mcfreis]

Yes, I also checked a few minutes ago, the MTU only affects OpenSSL generated datagrams but does not fragment application datagrams. I'll commit the 576 in my branch.

If you generate the dlls, that would be great.

About 1.0.2k, there is a bug which hangs if the handshake fails: https://github.com/openssl/openssl/issues/2886

That is why I used the HEAD of 1.0.2-stable: 1.0.2l-dev

[rbit]

Merged.

[mcfreis]

Thank you for adding my changes.