fix: Unsets global libcrypto rand

[maddeleine]

Resolved issues:

Resolves #4253

Description of changes:

Fixes issue where random is not our custom rand implementation after calling s2n_init().

Call-outs:

Testing:

Includes tests. Also, I am fixing the bug where a bunch of test output gets printed when running the s2n_random_test.c file. This seems like a really obvious fix, so maybe someone can explain to me why we never fixed this?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[lrstewart]

Why do these methods work, but how we set the engine / method below don't work? Should we be using these methods to set our randomness instead?

[maddeleine]

It's due to how Openssl gets a random method to use when it needs random bytes. Basically there is a global variable called default_rand_meth which is used over any random engine. Normally, we are able to set our random engine as the default. However, if you call RAND_bytes before setting up our engine, that default_rand_method gets set to the Openssl randomness function, and therefore our randomness method will never be chosen over Openssl's.
To get around this we have to call a function that alters the default_rand_method variable after it's already been set. So we basically have two options, either clean up the default random method before setting up the engine code(what I did), or calling RAND_set_rand_engine() with our custom rand method. I don't think it really matters which one we choose.

[maddeleine]

Another piece of the puzzle, apparently we used to use the RAND_set_rand_method() before replacing it with ENGINE code. But it's very weird that the ENGINE code doesn't fully replace the RAND code.

[goatgoose]

The behavior of the default_rand_meth global is very confusing. If I'm reading the OpenSSL documentation correctly, this shouldn't be happening:
https://www.openssl.org/docs/man1.0.2/man3/RAND_set_rand_method.html#notes

If a default ENGINE is specified for RAND functionality using an ENGINE API function, that will override any RAND defaults set using the RAND API (ie. RAND_set_rand_method()). For this reason, the ENGINE API is the recommended way to control default implementations for use in RAND and other cryptographic algorithms.

But other than calling RAND_set_rand_method()/RAND_set_rand_engine(), there doesn't seem to be a way to override the default_rand_meth variable after it's set, except for OPENSSL_cleanup().

Given that the RAND methods are deprecated, it seems like it might be best to use Madeleine's current implementation and keep the ENGINE APIs to actually set our random implementation.

[maddeleine]

Both ENGINE APIs and RAND APIs are deprecated. Rather than spending time getting our ENGINE code right, I think we should apply this fix and work towards this solution in the long term: #4348. If this is a bug, I doubt Openssl would care about it since technically the new thing for this functionality is "providers".
Also a note: I don't think there is a way to do what we want with the ENGINE APIs after staring at the codepaths for a bit. Nothing touches that default_rand_meth except RAND APIs.

[goatgoose]

The behavior of the default_rand_meth global is very confusing. If I'm reading the OpenSSL documentation correctly, this shouldn't be happening:
https://www.openssl.org/docs/man1.0.2/man3/RAND_set_rand_method.html#notes

If a default ENGINE is specified for RAND functionality using an ENGINE API function, that will override any RAND defaults set using the RAND API (ie. RAND_set_rand_method()). For this reason, the ENGINE API is the recommended way to control default implementations for use in RAND and other cryptographic algorithms.

But other than calling RAND_set_rand_method()/RAND_set_rand_engine(), there doesn't seem to be a way to override the default_rand_meth variable after it's set, except for OPENSSL_cleanup().

Given that the RAND methods are deprecated, it seems like it might be best to use Madeleine's current implementation and keep the ENGINE APIs to actually set our random implementation.