Skip to content

Commit

Permalink
feedback2
Browse files Browse the repository at this point in the history
  • Loading branch information
@toidiu
toidiu committed Jul 11, 2023
1 parent 196ed3d commit 5e80fd0
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 59 deletions.
1 change: 0 additions & 1 deletion error/s2n_errno.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ static const char *no_such_error = "Internal s2n error";
ERR_ENTRY(S2N_ERR_KTLS_UNSUPPORTED_PLATFORM, "kTLS is unsupported on this platform") \
ERR_ENTRY(S2N_ERR_KTLS_UNSUPPORTED_CONN, "kTLS is unsupported for this connection") \
ERR_ENTRY(S2N_ERR_KTLS_ULP, "An error occurred when attempting to configure the socket for kTLS. Ensure the 'tls' kernel module is enabled.") \
ERR_ENTRY(S2N_ERR_KTLS_DISABLED_FOR_TEST, "Some kTLS operations have been disabled for testing") \
ERR_ENTRY(S2N_ERR_ATOMIC, "Atomic operations in this environment would require locking") \
/* clang-format on */

Expand Down
1 change: 0 additions & 1 deletion error/s2n_errno.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ typedef enum {
S2N_ERR_KTLS_UNSUPPORTED_PLATFORM,
S2N_ERR_KTLS_UNSUPPORTED_CONN,
S2N_ERR_KTLS_ULP,
S2N_ERR_KTLS_DISABLED_FOR_TEST,
S2N_ERR_ATOMIC,
S2N_ERR_T_USAGE_END,
} s2n_error;
Expand Down
43 changes: 12 additions & 31 deletions tests/unit/s2n_ktls_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "testlib/s2n_testlib.h"

S2N_RESULT s2n_ktls_retrieve_file_descriptor(struct s2n_connection *conn, s2n_ktls_mode ktls_mode, int *fd);
S2N_RESULT s2n_disable_ktls_socket_config_for_testing(void);

/* set kTLS supported cipher */
struct s2n_cipher ktls_temp_supported_cipher = {
Expand Down Expand Up @@ -58,8 +57,6 @@ int main(int argc, char **argv)
{
BEGIN_TEST();

EXPECT_OK(s2n_disable_ktls_socket_config_for_testing());

/* ktls_supported ciphers */
{
struct s2n_cipher cipher = s2n_aes128_gcm;
Expand Down Expand Up @@ -105,9 +102,10 @@ int main(int argc, char **argv)
int fd = 1;
EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true));

EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);

EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn));
EXPECT_TRUE(server_conn->ktls_send_enabled);
EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn));
EXPECT_TRUE(server_conn->ktls_recv_enabled);
};

/* ktls already enabled is a noop and returns success */
Expand Down Expand Up @@ -140,7 +138,7 @@ int main(int argc, char **argv)

/* unsupported ciphers */
{
/* set kTLS un-supported cipher */
/* set kTLS unsupported cipher */
struct s2n_cipher ktls_temp_unsupported_cipher = {
.ktls_supported = false,
};
Expand All @@ -156,12 +154,9 @@ int main(int argc, char **argv)
int fd = 1;
EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true));

/* base case */
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);

server_conn->ktls_send_enabled = false; /* reset ktls enable connection */
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 */
Expand All @@ -171,25 +166,21 @@ int main(int argc, char **argv)
int fd = 1;
EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true));

/* base case */
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);

uint8_t write_byte = 8;
uint8_t read_byte = 0;
/* write to conn->out buffer and assert error */
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&server_conn->out, &write_byte, 1));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_RECORD_STUFFER_NEEDS_DRAINING);
/* drain conn->out buffer and assert base case */
/* drain conn->out buffer and assert success case */
EXPECT_SUCCESS(s2n_stuffer_read_bytes(&server_conn->out, &read_byte, 1));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn));

/* write to conn->in buffer and assert error */
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&server_conn->in, &write_byte, 1));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_RECORD_STUFFER_NEEDS_DRAINING);
/* drain conn->in buffer and assert base case */
/* drain conn->in buffer and assert success case */
EXPECT_SUCCESS(s2n_stuffer_read_bytes(&server_conn->in, &read_byte, 1));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn));
};

/* managed_send_io */
Expand All @@ -199,18 +190,13 @@ int main(int argc, char **argv)
int fd = 1;
EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true));

/* base case */
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);

server_conn->ktls_send_enabled = false; /* reset ktls enable connection */
/* expect failure if connection is using custom IO */
server_conn->managed_send_io = false;
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_MANAGED_IO);

server_conn->ktls_send_enabled = false; /* reset ktls enable connection */
/* expect success if connection is NOT using custom IO */
server_conn->managed_send_io = true;
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn));
};

/* managed_recv_io */
Expand All @@ -220,18 +206,13 @@ int main(int argc, char **argv)
int fd = 1;
EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, true));

/* base case */
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);

server_conn->ktls_recv_enabled = false; /* reset ktls enable connection */
/* recv managed io */
server_conn->managed_recv_io = false;
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_MANAGED_IO);

server_conn->ktls_recv_enabled = false; /* reset ktls enable connection */
/* expect success if connection is NOT using custom IO */
server_conn->managed_recv_io = true;
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn));
};

/* s2n_ktls_retrieve_file_descriptor */
Expand Down
34 changes: 8 additions & 26 deletions tls/s2n_ktls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@

#include "utils/s2n_socket.h"

/* These variables are used to disable ktls mechanisms during testing. */
static bool disable_ktls_socket_config_for_testing = false;

bool s2n_ktls_is_supported_on_platform()
{
#if S2N_KTLS_SUPPORTED
Expand Down Expand Up @@ -62,20 +59,14 @@ S2N_RESULT s2n_ktls_validate(struct s2n_connection *conn, s2n_ktls_mode ktls_mod
*/
switch (ktls_mode) {
case S2N_KTLS_MODE_SEND:
RESULT_ENSURE_REF(conn->send_io_context);
RESULT_ENSURE(conn->managed_send_io, S2N_ERR_KTLS_MANAGED_IO);

/* The output stuffer should be empty before enabling kTLS. */
RESULT_ENSURE(s2n_stuffer_data_available(&conn->out) == 0, S2N_ERR_RECORD_STUFFER_NEEDS_DRAINING);

break;
case S2N_KTLS_MODE_RECV:
RESULT_ENSURE_REF(conn->recv_io_context);
RESULT_ENSURE(conn->managed_recv_io, S2N_ERR_KTLS_MANAGED_IO);

/* The input stuffer should be empty before enabling kTLS. */
RESULT_ENSURE(s2n_stuffer_data_available(&conn->in) == 0, S2N_ERR_RECORD_STUFFER_NEEDS_DRAINING);

break;
default:
RESULT_BAIL(S2N_ERR_SAFETY);
Expand Down Expand Up @@ -115,7 +106,9 @@ static S2N_RESULT s2n_ktls_configure_socket(struct s2n_connection *conn, s2n_ktl
RESULT_GUARD(s2n_ktls_retrieve_file_descriptor(conn, ktls_mode, &fd));

/* Calls to setsockopt require a real socket, which is not used in unit tests. */
RESULT_ENSURE(!disable_ktls_socket_config_for_testing, S2N_ERR_KTLS_DISABLED_FOR_TEST);
if (s2n_in_unit_test()) {
return S2N_RESULT_OK;
}

/* Enable 'tls' ULP for the socket. https://lwn.net/Articles/730207 */
int ret = setsockopt(fd, S2N_SOL_TCP, S2N_TCP_ULP, S2N_TLS_ULP_NAME, S2N_TLS_ULP_NAME_SIZE);
Expand All @@ -133,8 +126,7 @@ static S2N_RESULT s2n_ktls_configure_socket(struct s2n_connection *conn, s2n_ktl

/*
* Since kTLS is an optimization, it is possible to continue operation
* by using userspace TLS if kTLS is not supported. Upon successfully
* enabling kTLS, we set connection->ktls_send_enabled (and recv) to true.
* by using userspace TLS if kTLS is not supported.
*
* kTLS configuration errors are recoverable since calls to setsockopt are
* non-destructive and its possible to fallback to userspace.
Expand All @@ -155,6 +147,8 @@ int s2n_connection_ktls_enable_send(struct s2n_connection *conn)
POSIX_GUARD_RESULT(res);
}

conn->ktls_send_enabled = true;

return S2N_SUCCESS;
}

Expand All @@ -174,19 +168,7 @@ int s2n_connection_ktls_enable_recv(struct s2n_connection *conn)
POSIX_GUARD_RESULT(res);
}

return S2N_SUCCESS;
}

/* Use for testing only.
*
* This function disables the setsockopt call to enable ULP. Calls to setsockopt
* require a real socket, which is not used in unit tests.
*/
S2N_RESULT s2n_disable_ktls_socket_config_for_testing(void)
{
RESULT_ENSURE(s2n_in_unit_test(), S2N_ERR_NOT_IN_UNIT_TEST);

disable_ktls_socket_config_for_testing = true;
conn->ktls_recv_enabled = true;

return S2N_RESULT_OK;
return S2N_SUCCESS;
}
2 changes: 2 additions & 0 deletions tls/s2n_ktls.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,7 @@ typedef enum {
} s2n_ktls_mode;

bool s2n_ktls_is_supported_on_platform();

/* These functions will be moved to api/s2n.h when the kTLS feature is released. */
int s2n_connection_ktls_enable_send(struct s2n_connection *conn);
int s2n_connection_ktls_enable_recv(struct s2n_connection *conn);

0 comments on commit 5e80fd0

Please sign in to comment.