Re: Authentication over ECDHE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I don't have access to the actual testing environments until Wednesday next year, so I've had to create a private account.

> Which version of OpenSSL is this? (I don't remember if you said this
> already).

I'm not entirely sure, but I *think* it's 1.1.0.

=====================================================================

OK, so I've been reading the mails before going to sleep and spent some time thinking and researching about this, and I've come to a conclusion: OpenSSL is a goddamn mess, SSL_clear() is pretty much superfluous, and as such shouldn't exist.

Why? Well, to quote Viktor here:

> DO NOT reuse the same SSL handle for multiple connections,

And that is fricking bullshit. Not the quote itself or the suggestion - it's unlikely you had anything to do with the actual code - but the way things have been thought through (or rather, have not been thought through) by the library devs. I've written highly scalable libraries in the past before, and one thing you always want to do there is to trim fat. And "object allocation and initialisation" is something that you very much want to trim fat of, not only for obvious reasons such as malloc() and free() (or whatever OpenSSL uses as wrappers) being complexity monsters, but also for cache reasons (loading different cache line hurts performance). That's why you usually have functions like XXX_clear() or XXX_reset(), which do exactly that - prepare an object for another usage. memset() (or the OpenSSL equivalent of a secure memset) your allocated resources. I don't really see the problem here.

Now add to that the fact that OpenSSL has been moving towards making its structures opaque, thus falling into the same trap that Microsoft has with COM and DirectX, and you can kind of see why, if you can't really determine anymore WHERE your object is going to be stored, you at least want to keep reusing it. This is not PHP, where people allocate memory all willy-nilly, or C++, where people don't even have shame anymore to use std::vector<std::strings> str_array instead of good old static const char*const str_array[] while expecting things to be made faster by invisible memory pools (and horribly failing at it), but C, where you want to think about each step quite carefully.

Then OpenSSL even provides an SSL_clear function which is advertised like this:

> SSL_clear - reset SSL object to allow another connection

, and then, only later, in a big warning block, decides to tell the reader that this function only works when the stars align quite correctly and you've sacrificed at least two virgins, because:

> The reset operation however keeps several settings of the last
> sessions

Then, as the documentation suggests, I read the entry for SSL_get_session:

> The ssl session contains all information required to re-establish the
> connection without a full handshake for SSL versions up to and
> including TLSv1.2. In TLSv1.3 the same is true, but sessions are
> established after the main handshake has occurred.

And at this point it all falls apart. From my understanding OpenSSL keeps a session cache for servers so that key exchanges and protocol handshakes can be avoided. Problem is, *we're using ECDHE, where the last E stands for "ephemeral"*. In simple English: throw away the keys after you're done, we want to have forward secrecy. And then OpenSSL keeps a fresh copy of those for everyone who happened to be logged on at this point. Heartbleed apparently wasn't enough of a warning. Oh, but lets move everything to the heap so that it's more secure there now.

I don't want to reuse a session with ephemeral keys; I want to reuse an object that is supposed to already have resources allocated for doing its job, as is indicated by the documentation of this function except for a small note at the end that tells you that the devs didn't really think about what "ephemeral" means.

Creating a new SSL object (EVEN FROM AN EXISTING SSL_CTX object) entails:

- allocating the memory for the object itself on the heap (via OPENSSL_zalloc) - creating and managing a new lock for the object, and who knows for much more subobjects - creating a duplicate of the cipher suite stack (which isn't even a flat copy, but something that can cause the code to call OPENSSL_malloc *twice* in the worst case) - creating a duplicate of the certificates (which I don't even use, but that doesn't stop the code of ssl_cert_dup() to call OPENSSL_zalloc *in its very first line!*)
- setting up a bunch of callbacks
- copying 32 bytes for a sid_ctx
- creating an X509_VERIFY_PARAM object (*which calls OPENSSL_zalloc again*) as well as creating a deep copy of the SSL_CTX's parameter via X509_VERIFY_PARAM_inherit(), with Thor knows how many copies hidden in all those *set* and *deep_copy* routines - copying EC point formats from the context - deep again, of course, at least that's what OPENSSL_memdup() makes me think
- copying supported group informations, and of course deep again!
- deep-copying an ALPN object
- SSL_clear()-ing the object (no, really!)
- deep-copying a CRYPTO_EX_DATA object via CRYPTO_new_ex_data ... at this point, is anyone surprised here that timing attacks against crypto are *still* so successful? Because I'm not. Not at all.

I didn't bother looking up what freeing entails - it's obvious to anyone at this point that OpenSSL is a severe victim of feature creep, that its memory allocation scheme is a mess, and long story short: I will NOT free a perfectly fine object just because of incompetent devs' chutzpah expecting their users to allocate memory dynamically en mass for no goddamn reason whenever a new connection comes in. Fix your goddamn code.

And don't give me any "trust us, we're experienced programmers" bullshit. I've *seen* ssl/record/ssl3_record.c:

> static const unsigned char ssl3_pad_1[48] = {
>     0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
>     0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
>     0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
>     0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
>     0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
>     0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36
> };
> static const unsigned char ssl3_pad_2[48] = {
>     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
>     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
>     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
>     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
>     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
>     0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c
> };

What's wrong with that, you ask? Let me show you how I'd have done that:

> static const unsigned char ssl3_pad_1[] =
> {
>     "66666666"
>     "66666666"
>     "66666666"
>     "66666666"
>     "66666666"
>     "66666666"
> };
>
> static const unsigned char*ssl3_pad_2[] =
> {
>     "\\\\\\\\\\\\\\\\"
>     "\\\\\\\\\\\\\\\\"
>     "\\\\\\\\\\\\\\\\"
>     "\\\\\\\\\\\\\\\\"
>     "\\\\\\\\\\\\\\\\"
>     "\\\\\\\\\\\\\\\\"
> };

So, no. I don't trust anyone. Especially not this mess of a code.
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux