Taking ownership of OpenSSL randomness backend doesn't always work

[WesleyRosenblum]

Problem:

Issue first reported by @torben-hansen:

Building s2n-tls with OpenSSL 3.0 shows that generating some randomness using RAND_bytes() before calling s2n_init() blocks s2n-tls from overriding the OpenSSL randomness backend using the s2n-tls rand engine. From s2nc, modified to exit from s2n's custom random function:

OpenSSL randomness backend is overwritten by s2n-tls:

$ ./bin/s2nc aws.amazon.com
Aborted (core dumped)

Generating randomness before s2n-tls init, means OpenSSL randomness backend is not overwritten by s2n-tls:

$ ./bin/s2nc --rand-bytes-before-init aws.amazon.com
CONNECTED:
[...]

Patch to reproduce

diff --git a/bin/s2nc.c b/bin/s2nc.c
index 8203a1c4..3885d197 100644
--- a/bin/s2nc.c
+++ b/bin/s2nc.c
@@ -27,6 +27,9 @@
 #ifndef S2N_INTERN_LIBCRYPTO
     #include <openssl/crypto.h>
     #include <openssl/err.h>
+    #include <openssl/rand.h>
+#else
+    #error "Don't want this configuration..."
 #endif
 
 #include "api/s2n.h"
@@ -282,6 +285,7 @@ int main(int argc, char *const *argv)
     int r, sockfd = 0;
     bool session_ticket_recv = 0;
     /* Optional args */
+    bool generate_rand_bytes_before_init = false;
     const char *alpn_protocols = NULL;
     const char *server_name = NULL;
     const char *ca_file = NULL;
@@ -353,6 +357,7 @@ int main(int argc, char *const *argv)
         { "buffered-send", required_argument, 0, OPT_BUFFERED_SEND },
         { "prefer-low-latency", no_argument, NULL, OPT_PREFER_LOW_LATENCY },
         { "prefer-throughput", no_argument, NULL, OPT_PREFER_THROUGHPUT },
+        { "rand-bytes-before-init", no_argument, NULL, 'R'},
         { 0 },
     };
 
@@ -363,6 +368,9 @@ int main(int argc, char *const *argv)
             break;
         }
         switch (c) {
+            case 'R':
+                generate_rand_bytes_before_init = true;
+                break;
             case 'a':
                 alpn_protocols = optarg;
                 break;
@@ -540,6 +548,14 @@ int main(int argc, char *const *argv)
         exit(1);
     }
 
+    if (generate_rand_bytes_before_init) {
+        uint8_t some_randomness[8];
+        if (RAND_bytes(some_randomness, 8) != 1) {
+            fprintf(stderr, "RAND_bytes failed somehow...\n");
+            exit(1);
+        }
+    }
+
     GUARD_EXIT(s2n_init(), "Error running s2n_init()");
 
     if ((r = getaddrinfo(host, port, &hints, &ai_list)) != 0) {

diff --git a/utils/s2n_random.c b/utils/s2n_random.c
index 485229df..5d47ce48 100644
--- a/utils/s2n_random.c
+++ b/utils/s2n_random.c
@@ -411,6 +411,10 @@ S2N_RESULT s2n_public_random(int64_t bound, uint64_t *output)
     }
 }
 
+#ifndef S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND
+#error "engines not supported"
+#endif
+
 #if S2N_LIBCRYPTO_SUPPORTS_CUSTOM_RAND
 
     #define S2N_RAND_ENGINE_ID "s2n_rand"
@@ -420,6 +424,8 @@ int s2n_openssl_compat_rand(unsigned char *buf, int num)
     struct s2n_blob out = { 0 };
     POSIX_GUARD(s2n_blob_init(&out, buf, num));
 
+    abort();
+
     if (s2n_result_is_error(s2n_get_private_random_data(&out))) {
         return 0;
     }

The reason is probably that the default engine callback is globally cached: https://github.com/openssl/openssl/blob/openssl-3.0/crypto/rand/rand_lib.c#L184-L213

And therefore the rand engine backend is never used. In fact, totally ignored: https://github.com/openssl/openssl/blob/openssl-3.0/crypto/rand/rand_lib.c#L361

Solution:

Maybe should not override when using OpenSSL +3 (randomness backend is OK) or create a provider and fetch the correct implementation.

• Does this change what S2N sends over the wire? If yes, explain.
• Does this change any public APIs? If yes, explain.
• Which versions of TLS will this impact?

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

• RFC links: Links to relevant RFC(s)
• Related Issues: Link any relevant issues
• Will the Usage Guide or other documentation need to be updated?
• Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
  • Will this change trigger SAW changes? Changes to the state machine, the s2n_handshake_io code that controls state transitions, the DRBG, or the corking/uncorking logic could trigger SAW failures.
  • Should this change be fuzz tested? Will it handle untrusted input? Create a separate issue to track the fuzzing work.

Out of scope:

Is there anything the solution will intentionally NOT address?