Enforce init and cleanup calling rules (aws#3446)

tests/unit/s2n_init_test.c

@@ -23,11 +23,20 @@ int main(int argc, char **argv)
     /* this includes a call to s2n_init */
     BEGIN_TEST();
 
+    /* test call idempotency: see https://github.com/aws/s2n-tls/issues/3446 */
+    EXPECT_FAILURE_WITH_ERRNO(s2n_init(), S2N_ERR_INITIALIZED);
+
+    /* cleanup can only be called once when atexit is disabled, since mem_cleanup is not idempotent */
+    EXPECT_SUCCESS(s2n_cleanup());
+    EXPECT_FAILURE_WITH_ERRNO(s2n_cleanup(), S2N_ERR_NOT_INITIALIZED);
+
     /* clean up and init multiple times */
+    EXPECT_SUCCESS(s2n_init());
     for (size_t i = 0; i < 10; i++) {
-        s2n_cleanup();
+        EXPECT_SUCCESS(s2n_cleanup());
         EXPECT_SUCCESS(s2n_init());
     }
 
+    /* this includes a call to s2n_cleanup */
     END_TEST();
 }

utils/s2n_init.c

@@ -48,6 +48,11 @@ int s2n_disable_atexit(void) {
 
 int s2n_init(void)
 {
+    /* USAGE-GUIDE says s2n_init MUST NOT be called more than once */
+    /* Public documentation for API states s2n_init should only be called once */
+    /* https://github.com/aws/s2n-tls/issues/3446 is a result of not enforcing this */
+    POSIX_ENSURE(!initialized, S2N_ERR_INITIALIZED);
+
     main_thread = pthread_self();
     /* Should run before any init method that calls libcrypto methods
      * to ensure we don't try to call methods that don't exist.
@@ -89,11 +94,15 @@ static bool s2n_cleanup_atexit_impl(void)
     /* the configs need to be wiped before resetting the memory callbacks */
     s2n_wipe_static_configs();
 
-    return s2n_result_is_ok(s2n_libcrypto_cleanup()) &&
+    bool cleaned_up = \
         s2n_result_is_ok(s2n_rand_cleanup_thread()) &&
-        s2n_result_is_ok(s2n_rand_cleanup()) &&
+        s2n_result_is_ok(s2n_rand_cleanup()) &&         // not idempotent
+        s2n_result_is_ok(s2n_libcrypto_cleanup()) &&
         s2n_result_is_ok(s2n_locking_cleanup()) &&
-        (s2n_mem_cleanup() == S2N_SUCCESS);
+        (s2n_mem_cleanup() == S2N_SUCCESS);             // not idempotent
+
+    initialized = !cleaned_up;
+    return cleaned_up;
 }
 
 int s2n_cleanup(void)
@@ -105,12 +114,15 @@ int s2n_cleanup(void)
     /* If this is the main thread and atexit cleanup is disabled,
      * perform final cleanup now */
     if (pthread_equal(pthread_self(), main_thread) && !atexit_cleanup) {
+        /* some cleanups are not idempotent (rand_cleanup, for example) so protect */
+        POSIX_ENSURE(initialized, S2N_ERR_NOT_INITIALIZED);
         POSIX_ENSURE(s2n_cleanup_atexit_impl(), S2N_ERR_ATEXIT);
     }
+
     return 0;
 }
 
 static void s2n_cleanup_atexit(void)
 {
-    s2n_cleanup_atexit_impl();
+    (void)s2n_cleanup_atexit_impl();
 }