From ef34e0fcb404e51ebc66a719422d1eb326073430 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Tue, 11 Jul 2023 16:06:36 -0700 Subject: [PATCH] feedback --- tests/unit/s2n_ktls_test.c | 117 +++++++++++++++++-------------------- tls/s2n_ktls.c | 8 +-- 2 files changed, 57 insertions(+), 68 deletions(-) diff --git a/tests/unit/s2n_ktls_test.c b/tests/unit/s2n_ktls_test.c index c869c197150..4cf410a1767 100644 --- a/tests/unit/s2n_ktls_test.c +++ b/tests/unit/s2n_ktls_test.c @@ -31,24 +31,20 @@ struct s2n_cipher_suite ktls_temp_supported_cipher_suite = { .record_alg = &ktls_temp_supported_record_alg, }; -S2N_RESULT s2n_test_configure_mock_ktls_connection(struct s2n_connection *conn, int fd, bool complete_handshake) +S2N_RESULT s2n_test_configure_valid_ktls_connection(struct s2n_connection *conn) { RESULT_ENSURE_REF(conn); /* config I/O */ - RESULT_GUARD_POSIX(s2n_connection_set_write_fd(conn, fd)); - RESULT_GUARD_POSIX(s2n_connection_set_read_fd(conn, fd)); - conn->managed_send_io = true; - conn->managed_recv_io = true; + RESULT_GUARD_POSIX(s2n_connection_set_write_fd(conn, 1)); + RESULT_GUARD_POSIX(s2n_connection_set_read_fd(conn, 1)); conn->ktls_send_enabled = false; conn->ktls_recv_enabled = false; /* configure connection so that the handshake is complete */ conn->secure->cipher_suite = &ktls_temp_supported_cipher_suite; conn->actual_protocol_version = S2N_TLS12; - if (complete_handshake) { - RESULT_GUARD(s2n_skip_handshake(conn)); - } + RESULT_GUARD(s2n_skip_handshake(conn)); return S2N_RESULT_OK; } @@ -57,7 +53,18 @@ int main(int argc, char **argv) { BEGIN_TEST(); - /* ktls_supported ciphers */ + if (!s2n_ktls_is_supported_on_platform()) { + DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), + s2n_connection_ptr_free); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); + + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_UNSUPPORTED_PLATFORM); + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_UNSUPPORTED_PLATFORM); + + END_TEST(); + } + + /* Test ktls_supported ciphers */ { struct s2n_cipher cipher = s2n_aes128_gcm; EXPECT_TRUE(cipher.ktls_supported); @@ -75,32 +82,30 @@ int main(int argc, char **argv) EXPECT_FALSE(cipher.ktls_supported); }; - if (!s2n_ktls_is_supported_on_platform()) { + /* Test s2n_ktls_retrieve_file_descriptor */ + { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + int write_fd_orig = 1; + int read_fd_orig = 2; + int fd_ret = 0; - EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_UNSUPPORTED_PLATFORM); - EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_UNSUPPORTED_PLATFORM); - } else { - /* ktls handshake must be complete */ - { - DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), - s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, false)); + EXPECT_SUCCESS(s2n_connection_set_write_fd(server_conn, write_fd_orig)); + EXPECT_OK(s2n_ktls_retrieve_file_descriptor(server_conn, S2N_KTLS_MODE_SEND, &fd_ret)); + EXPECT_EQUAL(write_fd_orig, fd_ret); - EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_HANDSHAKE_NOT_COMPLETE); - EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_HANDSHAKE_NOT_COMPLETE); - }; + EXPECT_SUCCESS(s2n_connection_set_read_fd(server_conn, read_fd_orig)); + EXPECT_OK(s2n_ktls_retrieve_file_descriptor(server_conn, S2N_KTLS_MODE_RECV, &fd_ret)); + EXPECT_EQUAL(read_fd_orig, fd_ret); + }; - /* s2n_connection_ktls_enable */ + /* Test s2n_connection_ktls_enable_recv/send */ + { + /* Success case */ { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn)); EXPECT_TRUE(server_conn->ktls_send_enabled); @@ -108,12 +113,11 @@ int main(int argc, char **argv) EXPECT_TRUE(server_conn->ktls_recv_enabled); }; - /* ktls already enabled is a noop and returns success */ + /* Noop if kTLS is already enabled */ { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); server_conn->ktls_send_enabled = true; EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn)); @@ -122,12 +126,22 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn)); }; - /* unsupported protocols */ + /* Fail if handshake is not complete */ + { + DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), + s2n_connection_ptr_free); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); + server_conn->handshake.message_number = 0; + + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_HANDSHAKE_NOT_COMPLETE); + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_HANDSHAKE_NOT_COMPLETE); + }; + + /* Fail if unsupported protocols */ { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); server_conn->actual_protocol_version = S2N_TLS13; EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_UNSUPPORTED_CONN); @@ -136,7 +150,7 @@ int main(int argc, char **argv) EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_UNSUPPORTED_CONN); }; - /* unsupported ciphers */ + /* Fail if unsupported ciphers */ { /* set kTLS unsupported cipher */ struct s2n_cipher ktls_temp_unsupported_cipher = { @@ -151,20 +165,18 @@ int main(int argc, char **argv) DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); server_conn->secure->cipher_suite = &ktls_temp_unsupported_cipher_suite; EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_UNSUPPORTED_CONN); EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_UNSUPPORTED_CONN); }; - /* drain buffer prior to enabling kTLS */ + /* Fail if buffers are not drained */ { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); uint8_t write_byte = 8; uint8_t read_byte = 0; @@ -183,12 +195,11 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn)); }; - /* managed_send_io */ + /* Fail if not using managed IO for send */ { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); /* expect failure if connection is using custom IO */ server_conn->managed_send_io = false; @@ -199,12 +210,11 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn)); }; - /* managed_recv_io */ + /* Fail if not using managed IO for recv */ { DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); - int fd = 1; - EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true)); + EXPECT_OK(s2n_test_configure_valid_ktls_connection(server_conn)); /* recv managed io */ server_conn->managed_recv_io = false; @@ -214,23 +224,6 @@ int main(int argc, char **argv) server_conn->managed_recv_io = true; EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn)); }; - - /* s2n_ktls_retrieve_file_descriptor */ - { - DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER), - s2n_connection_ptr_free); - int write_fd_orig = 1; - int read_fd_orig = 2; - int fd_ret = 0; - - EXPECT_SUCCESS(s2n_connection_set_write_fd(server_conn, write_fd_orig)); - EXPECT_OK(s2n_ktls_retrieve_file_descriptor(server_conn, S2N_KTLS_MODE_SEND, &fd_ret)); - EXPECT_EQUAL(write_fd_orig, fd_ret); - - EXPECT_SUCCESS(s2n_connection_set_read_fd(server_conn, read_fd_orig)); - EXPECT_OK(s2n_ktls_retrieve_file_descriptor(server_conn, S2N_KTLS_MODE_RECV, &fd_ret)); - EXPECT_EQUAL(read_fd_orig, fd_ret); - }; } END_TEST(); diff --git a/tls/s2n_ktls.c b/tls/s2n_ktls.c index faa9ea0c03a..db3d6ea8096 100644 --- a/tls/s2n_ktls.c +++ b/tls/s2n_ktls.c @@ -21,14 +21,10 @@ bool s2n_ktls_is_supported_on_platform() { -#if S2N_KTLS_SUPPORTED - return true; -#else - return false; -#endif + return S2N_KTLS_SUPPORTED; } -S2N_RESULT s2n_ktls_validate(struct s2n_connection *conn, s2n_ktls_mode ktls_mode) +static S2N_RESULT s2n_ktls_validate(struct s2n_connection *conn, s2n_ktls_mode ktls_mode) { RESULT_ENSURE_REF(conn); RESULT_ENSURE_REF(conn->secure);