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
