Fix TSAN s2n_shutdown failures

CMakeLists.txt

@@ -222,7 +222,7 @@ if(S2N_UNSAFE_FUZZING_MODE)
 endif()
 
 if(TSAN)
-    target_compile_options(${PROJECT_NAME} PUBLIC -fsanitize=thread)
+    target_compile_options(${PROJECT_NAME} PUBLIC -fsanitize=thread -DS2N_THREAD_SANITIZER=1)
     target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=thread)
 endif()
 
@@ -510,7 +510,11 @@ if (BUILD_TESTING)
             message(FATAL_ERROR "TSAN suppression file ${TSAN_SUPPRESSIONS_FILE} missing")
         endif()
         set(UNIT_TEST_ENVS ${UNIT_TEST_ENVS} S2N_ADDRESS_SANITIZER=1)
-        set(UNIT_TEST_ENVS ${UNIT_TEST_ENVS} TSAN_OPTIONS=suppressions=${TSAN_SUPPRESSIONS_FILE})
+        set(TSAN_OPTIONS suppressions=${TSAN_SUPPRESSIONS_FILE})
+        if(DEFINED ENV{TSAN_OPTIONS})
+            set(TSAN_OPTIONS "${TSAN_OPTIONS} $ENV{TSAN_OPTIONS}")
+        endif()
+        set(UNIT_TEST_ENVS ${UNIT_TEST_ENVS} TSAN_OPTIONS=${TSAN_OPTIONS})
     endif()
     message(STATUS "Running tests with environment: ${UNIT_TEST_ENVS}")
 

error/s2n_errno.c

@@ -290,6 +290,7 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_INTERNAL_LIBCRYPTO_ERROR, "An internal error has occurred in the libcrypto API") \
     ERR_ENTRY(S2N_ERR_NO_RENEGOTIATION, "Only secure, server-initiated renegotiation is supported") \
     ERR_ENTRY(S2N_ERR_APP_DATA_BLOCKED, "Blocked on application data during handshake") \
+    ERR_ENTRY(S2N_ERR_ATOMIC, "Atomic operations in this environment would require locking") \
     /* clang-format on */
 
 #define ERR_STR_CASE(ERR, str) \

error/s2n_errno.h

@@ -306,6 +306,7 @@ typedef enum {
     S2N_ERR_SECRET_SCHEDULE_STATE,
     S2N_ERR_CERT_OWNERSHIP,
     S2N_ERR_INTERNAL_LIBCRYPTO_ERROR,
+    S2N_ERR_ATOMIC,
     S2N_ERR_T_USAGE_END,
 } s2n_error;
 

tests/features/S2N_ATOMIC_SUPPORTED.c

@@ -0,0 +1,30 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+#include <signal.h>
+#include <stddef.h>
+
+int main() {
+    /* Atomic builtins are supported by gcc 4.7.3 and later. */
+    sig_atomic_t atomic = 0, value = 1;
+    __atomic_store(&atomic, &value, __ATOMIC_RELAXED);
+    __atomic_load(&atomic, &value, __ATOMIC_RELAXED);
+
+    /* _Static_assert is supported for C99 by gcc 4.6 and later,
+     * so using it here shouldn't limit use of the atomic builtins. */
+    _Static_assert(__atomic_always_lock_free(sizeof(sig_atomic_t), NULL),
+            "Atomic operations in this environment would require locking");
+    return 0;
+}

tests/features/S2N_ATOMIC_SUPPORTED.flags

@@ -0,0 +1 @@
+-Werror

tests/unit/s2n_alerts_test.c

@@ -106,7 +106,7 @@ int main(int argc, char **argv)
             EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_CLIENT));
 
             /* Verify state prior to alert */
-            EXPECT_FALSE(conn->close_notify_received);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
 
             /* Write and process the alert */
             EXPECT_SUCCESS(s2n_stuffer_write_bytes(&conn->in, not_close_notify_alert, sizeof(not_close_notify_alert)));
@@ -115,7 +115,7 @@ int main(int argc, char **argv)
             EXPECT_FAILURE_WITH_ERRNO(s2n_process_alert_fragment(conn), S2N_ERR_ALERT);
 
             /* Verify state after alert */
-            EXPECT_FALSE(conn->close_notify_received);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
 
             EXPECT_SUCCESS(s2n_connection_free(conn));
         }
@@ -126,14 +126,14 @@ int main(int argc, char **argv)
             EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_CLIENT));
 
             /* Verify state prior to alert */
-            EXPECT_FALSE(conn->close_notify_received);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
 
             /* Write and process the alert */
             EXPECT_SUCCESS(s2n_stuffer_write_bytes(&conn->in, close_notify_alert, sizeof(close_notify_alert)));
             EXPECT_SUCCESS(s2n_process_alert_fragment(conn));
 
             /* Verify state after alert */
-            EXPECT_TRUE(conn->close_notify_received);
+            EXPECT_TRUE(s2n_atomic_flag_test(&conn->close_notify_received));
 
             EXPECT_SUCCESS(s2n_connection_free(conn));
         }

tests/unit/s2n_connection_test.c

@@ -795,14 +795,6 @@ int main(int argc, char **argv)
             EXPECT_FALSE(s2n_connection_check_io_status(NULL, S2N_IO_CLOSED));
             EXPECT_FALSE(s2n_connection_check_io_status(NULL, 10));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, 10));
-
-            EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
-            conn->write_closed = 10;
-            EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
-
-            EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
-            conn->read_closed = 10;
-            EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
         }
 
         /* TLS1.2 */
@@ -819,24 +811,24 @@ int main(int argc, char **argv)
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
 
             /* Close write */
-            conn->write_closed = 1;
+            s2n_atomic_flag_set(&conn->write_closed);
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
             EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
-            conn->write_closed = 0;
+            s2n_atomic_flag_clear(&conn->write_closed);
 
             /* Close read */
-            conn->read_closed = 1;
+            s2n_atomic_flag_set(&conn->read_closed);
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
             EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
-            conn->read_closed = 0;
+            s2n_atomic_flag_clear(&conn->read_closed);
 
             /* Close both */
-            conn->read_closed = 1;
-            conn->write_closed = 1;
+            s2n_atomic_flag_set(&conn->read_closed);
+            s2n_atomic_flag_set(&conn->write_closed);
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
@@ -857,24 +849,24 @@ int main(int argc, char **argv)
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
 
             /* Close write */
-            conn->write_closed = 1;
+            s2n_atomic_flag_set(&conn->write_closed);
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
             EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
-            conn->write_closed = 0;
+            s2n_atomic_flag_clear(&conn->write_closed);
 
             /* Close read */
-            conn->read_closed = 1;
+            s2n_atomic_flag_set(&conn->read_closed);
             EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
-            conn->read_closed = 0;
+            s2n_atomic_flag_clear(&conn->read_closed);
 
             /* Close both */
-            conn->read_closed = 1;
-            conn->write_closed = 1;
+            s2n_atomic_flag_set(&conn->read_closed);
+            s2n_atomic_flag_set(&conn->write_closed);
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
             EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));

tests/unit/s2n_early_data_io_api_test.c

@@ -373,8 +373,8 @@ int main(int argc, char **argv)
 
             /* Pretend we didn't test the above error condition.
              * The S2N_ERR_BAD_MESSAGE error triggered S2N to close the connection. */
-            server_conn->write_closed = false;
-            client_conn->write_closed = false;
+            s2n_atomic_flag_clear(&server_conn->write_closed);
+            s2n_atomic_flag_clear(&client_conn->write_closed);
 
             /* Read the remaining early data properly */
             EXPECT_SUCCESS(s2n_recv_early_data(server_conn, actual_payload, sizeof(actual_payload),

tests/unit/s2n_shutdown_test.c

@@ -66,7 +66,7 @@ int main(int argc, char **argv)
 
         /* Verify state prior to alert */
         EXPECT_TRUE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
 
@@ -78,7 +78,7 @@ int main(int argc, char **argv)
 
         /* Verify state after shutdown attempt */
         EXPECT_TRUE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_TRUE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
 
@@ -109,7 +109,7 @@ int main(int argc, char **argv)
 
         /* Verify state prior to alert */
         EXPECT_TRUE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
 
@@ -123,7 +123,7 @@ int main(int argc, char **argv)
 
         /* Verify state after shutdown attempt */
         EXPECT_TRUE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_TRUE(conn->alert_sent);
         EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
 
@@ -146,7 +146,7 @@ int main(int argc, char **argv)
 
         /* Verify state prior to alert */
         EXPECT_TRUE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
 
@@ -173,7 +173,7 @@ int main(int argc, char **argv)
 
         /* Verify state after shutdown attempt */
         EXPECT_TRUE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
 
@@ -195,7 +195,7 @@ int main(int argc, char **argv)
 
         /* Verify state prior to alert */
         EXPECT_FALSE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
 
@@ -204,7 +204,7 @@ int main(int argc, char **argv)
 
         /* Verify state after shutdown */
         EXPECT_FALSE(s2n_handshake_is_complete(conn));
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_TRUE(conn->alert_sent);
 
         /* Fully closed: we don't worry about truncating data */
@@ -225,7 +225,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_connection_set_io_stuffers(&input, &output, conn));
 
         /* Verify state prior to alert */
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
 
@@ -234,7 +234,7 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(blocked, S2N_BLOCKED_ON_READ);
 
         /* Verify state after shutdown attempt */
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_TRUE(conn->alert_sent);
 
         /* Half-close: only write closed */
@@ -257,7 +257,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_connection_set_io_stuffers(&input, &output, conn));
 
         /* Verify state prior to alert */
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_FULL_DUPLEX));
 
@@ -266,7 +266,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_process_alert_fragment(conn));
 
         /* Verify state after alert */
-        EXPECT_TRUE(conn->close_notify_received);
+        EXPECT_TRUE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_WRITABLE));
         EXPECT_FALSE(s2n_connection_check_io_status(conn, S2N_IO_READABLE));
@@ -276,7 +276,7 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(blocked, S2N_NOT_BLOCKED);
 
         /* Verify state after shutdown attempt */
-        EXPECT_TRUE(conn->close_notify_received);
+        EXPECT_TRUE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_TRUE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
     };
@@ -309,15 +309,15 @@ int main(int argc, char **argv)
         EXPECT_FAILURE_WITH_ERRNO(s2n_shutdown(conn, &blocked), S2N_ERR_ALERT);
 
         /* Verify state after shutdown attempt */
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_TRUE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
         EXPECT_EQUAL(s2n_stuffer_data_available(&output), alert_record_size);
 
         /* Future calls are no-ops */
         for (size_t i = 0; i < 5; i++) {
             EXPECT_SUCCESS(s2n_shutdown(conn, &blocked));
-            EXPECT_FALSE(conn->close_notify_received);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
             EXPECT_TRUE(conn->alert_sent);
         }
     };
@@ -410,7 +410,7 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(blocked, S2N_NOT_BLOCKED);
 
         /* Verify state after shutdown attempt */
-        EXPECT_FALSE(conn->close_notify_received);
+        EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         EXPECT_FALSE(conn->alert_sent);
         EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
     };
@@ -586,7 +586,7 @@ int main(int argc, char **argv)
             /* Full close is no-op */
             EXPECT_SUCCESS(s2n_shutdown(conn, &blocked));
             EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
-            EXPECT_FALSE(conn->close_notify_received);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         };
 
         /* Test: Half close, peer alert, then full close */
@@ -621,7 +621,7 @@ int main(int argc, char **argv)
             /* Full close is no-op */
             EXPECT_SUCCESS(s2n_shutdown(conn, &blocked));
             EXPECT_TRUE(s2n_connection_check_io_status(conn, S2N_IO_CLOSED));
-            EXPECT_FALSE(conn->close_notify_received);
+            EXPECT_FALSE(s2n_atomic_flag_test(&conn->close_notify_received));
         };
     }
 

tls/s2n_alerts.c

@@ -23,6 +23,7 @@
 #include "tls/s2n_record.h"
 #include "tls/s2n_resume.h"
 #include "tls/s2n_tls_parameters.h"
+#include "utils/s2n_atomic.h"
 #include "utils/s2n_blob.h"
 #include "utils/s2n_safety.h"
 
@@ -203,8 +204,8 @@ int s2n_process_alert_fragment(struct s2n_connection *conn)
         if (s2n_stuffer_data_available(&conn->alert_in) == 2) {
             /* Close notifications are handled as shutdowns */
             if (conn->alert_in_data[1] == S2N_TLS_ALERT_CLOSE_NOTIFY) {
-                conn->read_closed = 1;
-                conn->close_notify_received = true;
+                s2n_atomic_flag_set(&conn->read_closed);
+                s2n_atomic_flag_set(&conn->close_notify_received);
                 return 0;
             }
 
@@ -221,6 +222,7 @@ int s2n_process_alert_fragment(struct s2n_connection *conn)
 
             /* All other alerts are treated as fatal errors */
             POSIX_GUARD_RESULT(s2n_connection_set_closed(conn));
+            s2n_atomic_flag_set(&conn->error_alert_received);
             POSIX_BAIL(S2N_ERR_ALERT);
         }
     }