[openssl-users] Confirmation of what I believe to be true from docs and observation

[Benjamin Kaduk]

On 01/10/2018 02:37 PM, Karl Denninger wrote:
> On 1/10/2018 14:07, Benjamin Kaduk wrote:
>> On 01/10/2018 08:41 AM, Karl Denninger wrote:
>>> We start with a context that I load a dhparam file to (so I can take a
>>> DH connection) along with an edh curve, then set an acceptable cipher
>>> list for it to use.
>>>
>> Why not just use AUTO_DH (the only option for 1.1.0, IIRC)?
> That's a reasonable change (and I'll go ahead and make it); the
> dhparam was only there in the first place for those browsers and such
> that can't negotiate EC (which my cipher selection set prefers.)
>>> Assume I next manually load both the CA store (using
>>> X509_STORE_add_cert as many times as necessary to load the
>>> intermediate components and the root of trust) and then load the
>>> cert/key pair (using SSL_CTX_use_certificate/SSL_CTX_use_PrivateKey)
>>>
>>> I then create some number of SSLs from that context to perform
>>> communication with and all is well.
>>>
>>> Now I want to rekey that context for some reason.  It appears that
>>> while I can add things to
>>>
>> Why do you need to rekey the context as opposed to making a new one?
> I could make a new one (or more-specifically, destroy the existing one
> and re-initialize it), but that is more-complicated as the application
> in question is multi-threaded -- and it's not at all clear from the
> documentation if I destroy a context that has SSLs that have been
> generated from it will cause "bad things" to happen (like a deference
> on a freed object!)
>

Each SSL holds a reference on its parent SSL_CTX (and another reference
on its associated session_ctx, which starts out as the same SSL_CTX
object); SSL_CTX_free() does not do any deallocation until the refcount
drops to zero.

So, no "bad things" will happen if you SSL_CTX_free() the handle you
have in your application while SSL objects are still using that object.

> The reason I may want to rekey is that the cert/key pair used for that
> context may have changed.  The user's certificate may have expired for
> example (or been revoked) and I wish to reload it (and the matching
> key) without having to shut down the software and restart it.
>
>> In general, making configuration changes to an SSL_CTX after it has been
>> used to generate SSL objects is a risky proposition -- the locking model
>> does not really account for doing synchronization properly, and there
>> might be some latent race conditions.  In practice, some operations are
>> currently safe, but there is no authoritative list of which ones, and at
>> least my personal recommendation is to not try to rely on it.
> Assuming that there are SSL objects that are in use and I destroy and
> re-generate the CTX from which it was generated, is *THAT* safe?  Or
> do I need to flag all the in-use descendants and wait for them to be
> closed (or force them closed and release them) before I can safely
> destroy the context? 

You do not need to flag and wait, due to the reference counting on the
SSL and SSL_CTX objects.

>>> the CA chain trying to load the same component that is already in
>>> there returns a failure (somewhat-expected; that is, it does not
>>> overwrite but rather adds, and if you try to add what's already there
>>> you get an error back) and there's no call to CLEAR the certificate
>>> validation chain -- if I want to *replace* the validation chain I have
>>> to destroy the context and initialize a new one from scratch.
>>>
>> N.B. that the X509_STORE_add_cert behavior when presented with a
>> duplicate certificate changed in commit
>> c0452248ea1a59a41023a4765ef7d9825e80a62b (from returning an error to
>> doing nothing and returning success); this will be in 1.1.1.
>>
>> As to the desired behavior, there does not seem to be an API to remove
>> an entry from an X509_STORE.  With the above caveat about thread-safety
>> in mind, couldn't you just make a call to SSL_CTX_set{1}_cert_store() to
>> replace the X509_STORE without tearing down the whole SSL_CTX?
> Yeah, I didn't see one either.  I'm not particularly concerned about
> the verification chain being able to be modified while "in-use"; that
> ought not happen except in an extreme circumstance (e.g. the
> intermediate cert's key is compromised and thus both it and the cert
> have to be regenerated and re-distributed.)  If it DOES happen when
> the end-entity cert and key are reloaded (as they've been signed from
> the new intermediate) they'll fail to validate against the old chain
> and the user will get plenty of notice that there's trouble.
>>> It appears, however, that I *can* load over the top of a certificate
>>> and private key of the same type and that's acceptable.  In other
>>> words, if I have an RSA key/cert pair in the context and I load
>>> another one, the first one is replaced.  This *looks* to be working ok
>>> as far as I can tell and it doesn't appear to leak memory doing that
>>> but it's not explicitly stated that this is considered acceptable
>>> (rather than destroying and re-creating the context.)
>>>
>> The leaf certificate and private key are stored as arrays (for different
>> types of certificates) of pointers in the associated CERT structure, and
>> the "set" routines do not currently check for and error out if there is
>> already one set.
> Yes, but do they replace the pointer and, if they do, do they
> decrement the reference counter on the existing one before replacing
> it so it will get freed?   If yes then all is ok but if not then I
> need to destroy the context otherwise I'm going to be leaking memory. 

The underlying implementation (ssl_set_cert()) does call X509_free() and
EVP_PKEY_free() before doing the update, so there is no leak.

>>> Is my understanding correct?
>>>
>>>
>> Mostly ... but I am not sure that your desired workflow is wise.
>>
>> -Ben
> If it's safe to destroy a context with potential SSL options from it
> still in existence then it's pretty easy to do it the other way
> otherwise I have some work to do in order to make sure all the
> potential objects created from the parent have gone away before the
> old context is destroyed.

Yup, it's safe.

Good luck!

-Ben