diff --git a/tests/unit/s2n_mem_allocator_test.c b/tests/unit/s2n_mem_allocator_test.c index bd4bdf0f294..eea84b58809 100644 --- a/tests/unit/s2n_mem_allocator_test.c +++ b/tests/unit/s2n_mem_allocator_test.c @@ -82,8 +82,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) struct s2n_config *config = NULL; s2n_blocked_status blocked; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair is valid from fork; the kernel buffers writes until the + * parent reads, so no pre-negotiate sleep is needed. */ conn = s2n_connection_new(S2N_CLIENT); config = s2n_config_new(); @@ -138,8 +138,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_connection_free(conn); s2n_config_free(config); - /* Give the server a chance to a void a sigpipe */ - sleep(1); + /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so + * no pre-cleanup sleep is needed. */ s2n_io_pair_close_one_end(io_pair, S2N_CLIENT); diff --git a/tests/unit/s2n_self_talk_alerts_test.c b/tests/unit/s2n_self_talk_alerts_test.c index cf596ba5d5c..c6edfda4501 100644 --- a/tests/unit/s2n_self_talk_alerts_test.c +++ b/tests/unit/s2n_self_talk_alerts_test.c @@ -50,8 +50,8 @@ int mock_client(struct s2n_test_io_pair *io_pair, s2n_alert_behavior alert_behav int result = 0; int rc = 0; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair is valid from fork; the kernel buffers writes until the + * parent reads, so no pre-negotiate sleep is needed. */ conn = s2n_connection_new(S2N_CLIENT); config = s2n_config_new(); diff --git a/tests/unit/s2n_self_talk_broken_pipe_test.c b/tests/unit/s2n_self_talk_broken_pipe_test.c index 87b237d11d2..cdb942089c5 100644 --- a/tests/unit/s2n_self_talk_broken_pipe_test.c +++ b/tests/unit/s2n_self_talk_broken_pipe_test.c @@ -30,14 +30,15 @@ static const char *certificate_paths[SUPPORTED_CERTIFICATE_FORMATS] = { S2N_RSA_2048_PKCS1_CERT_CHAIN, S2N_RSA_2048_PKCS8_CERT_CHAIN }; static const char *private_key_paths[SUPPORTED_CERTIFICATE_FORMATS] = { S2N_RSA_2048_PKCS1_KEY, S2N_RSA_2048_PKCS8_KEY }; -void mock_client(struct s2n_test_io_pair *io_pair) +void mock_client(struct s2n_test_io_pair *io_pair, int ready_pipe_w, int done_pipe_r) { struct s2n_connection *conn = NULL; struct s2n_config *config = NULL; s2n_blocked_status blocked; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair underlying io_pair is valid from the moment of fork, + * so the child can start the handshake immediately. The kernel buffers + * writes until the parent is ready to read. */ conn = s2n_connection_new(S2N_CLIENT); config = s2n_config_new(); @@ -64,8 +65,23 @@ void mock_client(struct s2n_test_io_pair *io_pair) /* Close client read fd to mock half closed pipe at server side */ s2n_io_pair_shutdown_one_end(io_pair, S2N_CLIENT, SHUT_RD); #endif - /* Give server a chance to send data on broken pipe */ - sleep(2); + + /* Signal the parent: the read end of the socketpair is now shut down, + * so the parent's next s2n_send is guaranteed to observe EPIPE. */ + uint8_t ready = 1; + if (write(ready_pipe_w, &ready, 1) != 1) { + exit(1); + } + close(ready_pipe_w); + + /* Wait for the parent to finish its broken-pipe assertions before + * continuing teardown. Without this, the child could race ahead and + * close the socket entirely while the parent is mid-assertion. */ + uint8_t done = 0; + if (read(done_pipe_r, &done, 1) != 1) { + exit(1); + } + close(done_pipe_r); s2n_shutdown(conn, &blocked); @@ -79,8 +95,9 @@ void mock_client(struct s2n_test_io_pair *io_pair) exit(1); } - /* Give the server a chance to avoid a sigpipe */ - sleep(1); + /* The parent has SIGPIPE ignored (via s2n_io_pair_init), and the child + * inherits that signal disposition through fork(), so there's no need to + * sleep before tearing down the other end of the socketpair. */ s2n_io_pair_shutdown_one_end(io_pair, S2N_CLIENT, SHUT_WR); s2n_io_pair_close_one_end(io_pair, S2N_CLIENT); @@ -107,16 +124,35 @@ int main(int argc, char **argv) DEFER_CLEANUP(struct s2n_test_io_pair io_pair = { 0 }, s2n_io_pair_close); EXPECT_SUCCESS(s2n_io_pair_init(&io_pair)); + /* Create control pipes for child<->parent synchronization. The child + * writes to ready_pipe after shutting down its read side, so the + * parent can deterministically observe EPIPE on its next s2n_send. + * The parent writes to done_pipe after completing its assertions, so + * the child knows when it's safe to tear down the socket. This + * replaces two timing-dependent sleep() calls. */ + int ready_pipe[2] = { -1, -1 }; + int done_pipe[2] = { -1, -1 }; + EXPECT_EQUAL(pipe(ready_pipe), 0); + EXPECT_EQUAL(pipe(done_pipe), 0); + /* Create a child process */ pid_t pid = fork(); if (pid == 0) { /* This is the client process, close the server end of the pipe */ EXPECT_SUCCESS(s2n_io_pair_close_one_end(&io_pair, S2N_SERVER)); + /* Close the pipe ends the child doesn't use */ + close(ready_pipe[0]); + close(done_pipe[1]); + /* Write the fragmented hello message */ - mock_client(&io_pair); + mock_client(&io_pair, ready_pipe[1], done_pipe[0]); } + /* Close the pipe ends the parent doesn't use */ + close(ready_pipe[1]); + close(done_pipe[0]); + /* This is the server process, close the client end of the pipe */ EXPECT_SUCCESS(s2n_io_pair_close_one_end(&io_pair, S2N_CLIENT)); @@ -146,8 +182,12 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_negotiate(conn, &blocked)); EXPECT_EQUAL(conn->actual_protocol_version, S2N_TLS12); - /* Give client a chance to close pipe at the receiving end */ - sleep(1); + /* Wait for the child to shut down its read side of the socketpair. + * Once this read returns, the next s2n_send on the server conn is + * guaranteed to observe EPIPE. */ + uint8_t ready = 0; + EXPECT_EQUAL(read(ready_pipe[0], &ready, 1), 1); + EXPECT_SUCCESS(close(ready_pipe[0])); char buffer[1]; /* Fist flush on half closed pipe should get EPIPE */ ssize_t w = s2n_send(conn, buffer, 1, &blocked); @@ -161,6 +201,12 @@ int main(int argc, char **argv) EXPECT_EQUAL(s2n_errno, S2N_ERR_IO); EXPECT_EQUAL(errno, 0); + /* Signal the child that the assertions are complete so it can tear + * down its socket and exit. */ + uint8_t done = 1; + EXPECT_EQUAL(write(done_pipe[1], &done, 1), 1); + EXPECT_SUCCESS(close(done_pipe[1])); + EXPECT_SUCCESS(s2n_connection_free(conn)); for (int cert = 0; cert < SUPPORTED_CERTIFICATE_FORMATS; cert++) { EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_keys[cert])); diff --git a/tests/unit/s2n_self_talk_client_hello_cb_test.c b/tests/unit/s2n_self_talk_client_hello_cb_test.c index 84ef83f33a5..21c5f2b1096 100644 --- a/tests/unit/s2n_self_talk_client_hello_cb_test.c +++ b/tests/unit/s2n_self_talk_client_hello_cb_test.c @@ -48,8 +48,9 @@ int mock_client(struct s2n_test_io_pair *io_pair, int expect_failure, int expect int rc = 0; const char *protocols[] = { "h2", "http/1.1" }; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair underlying io_pair is valid from the moment of fork, so + * the child can start the handshake immediately. The kernel will buffer + * writes until the parent starts reading. No pre-negotiate sleep needed. */ conn = s2n_connection_new(S2N_CLIENT); config = s2n_config_new(); @@ -96,8 +97,9 @@ int mock_client(struct s2n_test_io_pair *io_pair, int expect_failure, int expect s2n_connection_free(conn); s2n_config_free(config); - /* Give the server a chance to a void a sigpipe */ - sleep(1); + /* SIGPIPE is handled at the test's signal level (or ignored via + * SIGPIPE policy), so no post-shutdown sleep is needed to avoid a race + * with the server trying to send while we tear down. */ s2n_cleanup(); s2n_io_pair_close_one_end(io_pair, S2N_CLIENT); diff --git a/tests/unit/s2n_self_talk_nonblocking_test.c b/tests/unit/s2n_self_talk_nonblocking_test.c index 427d03e46eb..85d0981ae9f 100644 --- a/tests/unit/s2n_self_talk_nonblocking_test.c +++ b/tests/unit/s2n_self_talk_nonblocking_test.c @@ -44,8 +44,9 @@ int mock_client(struct s2n_test_io_pair *io_pair, uint8_t *expected_data, uint32 * CI/CD pipelines might never complete */ int should_block = 1; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair underlying io_pair is valid from the moment of fork, + * so the child can start the handshake immediately. The kernel buffers + * writes until the parent is ready to read. */ client_conn = s2n_connection_new(S2N_CLIENT); client_config = s2n_config_new(); @@ -110,8 +111,9 @@ int mock_client_iov(struct s2n_test_io_pair *io_pair, struct iovec *iov, uint32_ uint8_t *buffer = malloc(total_size + iov[0].iov_len); int buffer_offs = 0; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair underlying io_pair is valid from the moment of fork, + * so the child can start the handshake immediately. The kernel buffers + * writes until the parent is ready to read. */ client_conn = s2n_connection_new(S2N_CLIENT); client_config = s2n_config_new(); @@ -336,8 +338,14 @@ int test_send(int use_tls13, int use_iov, int prefer_throughput) EXPECT_TRUE(remaining < data_size); EXPECT_TRUE(remaining > 0); - /* Wait for the child process to read some bytes and block itself*/ - sleep(1); + /* Wait for the child to actually be stopped before sending SIGCONT. + * WUNTRACED causes waitpid to return when the child changes state to + * stopped, not just when it exits. This removes the timing race that + * would otherwise require a sleep() here. */ + int stop_status = 0; + pid_t wait_rc = waitpid(pid, &stop_status, WUNTRACED); + EXPECT_EQUAL(wait_rc, pid); + EXPECT_TRUE(WIFSTOPPED(stop_status)); /* Wake the child process by sending it SIGCONT */ EXPECT_SUCCESS(kill(pid, SIGCONT)); diff --git a/tests/unit/s2n_self_talk_session_id_test.c b/tests/unit/s2n_self_talk_session_id_test.c index dbef1031b01..9621c157027 100644 --- a/tests/unit/s2n_self_talk_session_id_test.c +++ b/tests/unit/s2n_self_talk_session_id_test.c @@ -147,8 +147,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_blocked_status blocked; int result = 0; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair is valid from fork; the kernel buffers writes until the + * parent reads, so no pre-negotiate sleep is needed. */ /* Initial handshake */ conn = s2n_connection_new(S2N_CLIENT); @@ -208,8 +208,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_connection_free(conn); - /* Give the server a chance to avoid sigpipe */ - sleep(1); + /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so + * no pre-cleanup sleep is needed. */ /* Session resumption */ conn = s2n_connection_new(S2N_CLIENT); @@ -241,8 +241,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_connection_free(conn); - /* Give the server a chance to avoid sigpipe */ - sleep(1); + /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so + * no pre-cleanup sleep is needed. */ /* Session resumption with bad session state */ conn = s2n_connection_new(S2N_CLIENT); @@ -282,8 +282,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_connection_free(conn); s2n_config_free(config); - /* Give the server a chance to avoid sigpipe */ - sleep(1); + /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so + * no pre-cleanup sleep is needed. */ s2n_io_pair_close_one_end(io_pair, S2N_CLIENT); exit(result); diff --git a/tests/unit/s2n_self_talk_tls12_test.c b/tests/unit/s2n_self_talk_tls12_test.c index e3713e30fad..51e25711a89 100644 --- a/tests/unit/s2n_self_talk_tls12_test.c +++ b/tests/unit/s2n_self_talk_tls12_test.c @@ -36,8 +36,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) struct s2n_config *config = NULL; s2n_blocked_status blocked; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair is valid from fork; the kernel buffers writes until the + * parent reads, so no pre-negotiate sleep is needed. */ conn = s2n_connection_new(S2N_CLIENT); config = s2n_config_new(); @@ -89,8 +89,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_connection_free(conn); s2n_config_free(config); - /* Give the server a chance to a void a sigpipe */ - sleep(1); + /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so + * no pre-cleanup sleep is needed. */ s2n_io_pair_close_one_end(io_pair, S2N_CLIENT); diff --git a/tests/unit/s2n_self_talk_tls13_test.c b/tests/unit/s2n_self_talk_tls13_test.c index ded3a95c8ae..938f38ddd3a 100644 --- a/tests/unit/s2n_self_talk_tls13_test.c +++ b/tests/unit/s2n_self_talk_tls13_test.c @@ -32,8 +32,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) struct s2n_config *config = NULL; s2n_blocked_status blocked; - /* Give the server a chance to listen */ - sleep(1); + /* The socketpair is valid from fork; the kernel buffers writes until the + * parent reads, so no pre-negotiate sleep is needed. */ conn = s2n_connection_new(S2N_CLIENT); config = s2n_config_new(); @@ -86,8 +86,8 @@ void mock_client(struct s2n_test_io_pair *io_pair) s2n_connection_free(conn); s2n_config_free(config); - /* Give the server a chance to avoid a sigpipe */ - sleep(1); + /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so + * no pre-cleanup sleep is needed. */ s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);