feedback

error/s2n_errno.c

@@ -290,9 +290,9 @@ 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_KTLS_ALREADY_ENABLED, "kTLS is already enabled on the socket")  \
     ERR_ENTRY(S2N_ERR_KTLS_MANAGED_IO, "kTLS cannot be enabled while custom I/O is configured for the connection")  \
-    ERR_ENTRY(S2N_ERR_KTLS_HANDSHAKE_NOT_COMPLETE, "kTLS can only be enabled once the handshake is completed") \
+    ERR_ENTRY(S2N_ERR_KTLS_ALREADY_ENABLED, "kTLS is already enabled on the socket")  \
+    ERR_ENTRY(S2N_ERR_HANDSHAKE_NOT_COMPLETE, "Operation is only allowed after the handshake is complete") \
     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.")  \

error/s2n_errno.h

@@ -306,9 +306,9 @@ typedef enum {
     S2N_ERR_SECRET_SCHEDULE_STATE,
     S2N_ERR_CERT_OWNERSHIP,
     S2N_ERR_INTERNAL_LIBCRYPTO_ERROR,
+    S2N_ERR_HANDSHAKE_NOT_COMPLETE,
     S2N_ERR_KTLS_ALREADY_ENABLED,
     S2N_ERR_KTLS_MANAGED_IO,
-    S2N_ERR_KTLS_HANDSHAKE_NOT_COMPLETE,
     S2N_ERR_KTLS_UNSUPPORTED_PLATFORM,
     S2N_ERR_KTLS_UNSUPPORTED_CONN,
     S2N_ERR_KTLS_ULP,

tests/unit/s2n_ktls_test.c

@@ -94,8 +94,8 @@ int main(int argc, char **argv)
             int fd = 1;
             EXPECT_OK(s2n_test_configure_mock_ktls_connection(server_conn, fd, false));
 
-            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_HANDSHAKE_NOT_COMPLETE);
-            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_HANDSHAKE_NOT_COMPLETE);
+            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);
         };
 
         /* s2n_connection_ktls_enable */
@@ -110,18 +110,18 @@ int main(int argc, char **argv)
             EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_DISABLED_FOR_TEST);
         };
 
-        /* ktls already enabled */
+        /* ktls already enabled is a noop and returns success */
         {
             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));
 
             server_conn->ktls_send_enabled = true;
-            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_send(server_conn), S2N_ERR_KTLS_ALREADY_ENABLED);
+            EXPECT_SUCCESS(s2n_connection_ktls_enable_send(server_conn));
 
             server_conn->ktls_recv_enabled = true;
-            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn), S2N_ERR_KTLS_ALREADY_ENABLED);
+            EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn));
         };
 
         /* unsupported protocols */

tls/s2n_ktls.c

@@ -42,7 +42,7 @@ S2N_RESULT s2n_ktls_validate(struct s2n_connection *conn, s2n_ktls_mode ktls_mod
 
     /* kTLS enable should only be called once the handshake has completed. */
     if (!is_handshake_complete(conn)) {
-        RESULT_BAIL(S2N_ERR_KTLS_HANDSHAKE_NOT_COMPLETE);
+        RESULT_BAIL(S2N_ERR_HANDSHAKE_NOT_COMPLETE);
     }
 
     /* TODO support TLS 1.3
@@ -76,6 +76,9 @@ S2N_RESULT s2n_ktls_validate(struct s2n_connection *conn, s2n_ktls_mode ktls_mod
             /* 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);
             break;
     }
 
@@ -88,15 +91,9 @@ S2N_RESULT s2n_ktls_retrieve_file_descriptor(struct s2n_connection *conn, s2n_kt
     RESULT_ENSURE_REF(fd);
 
     if (ktls_mode == S2N_KTLS_MODE_RECV) {
-        /* retrieve the receive fd */
-        RESULT_ENSURE_REF(conn->recv_io_context);
-        const struct s2n_socket_read_io_context *peer_socket_ctx = conn->recv_io_context;
-        *fd = peer_socket_ctx->fd;
+        RESULT_GUARD_POSIX(s2n_connection_get_read_fd(conn, fd));
     } else if (ktls_mode == S2N_KTLS_MODE_SEND) {
-        /* retrieve the send fd */
-        RESULT_ENSURE_REF(conn->send_io_context);
-        const struct s2n_socket_write_io_context *peer_socket_ctx = conn->send_io_context;
-        *fd = peer_socket_ctx->fd;
+        RESULT_GUARD_POSIX(s2n_connection_get_write_fd(conn, fd));
     }
 
     return S2N_RESULT_OK;
@@ -124,14 +121,11 @@ static S2N_RESULT s2n_ktls_configure_socket(struct s2n_connection *conn, s2n_ktl
     int ret = setsockopt(fd, S2N_SOL_TCP, S2N_TCP_ULP, S2N_TLS_ULP_NAME, S2N_TLS_ULP_NAME_SIZE);
 
     if (ret != 0) {
-        /* EEXIST: https://man7.org/linux/man-pages/man3/errno.3.html
-         *
-         * TCP_ULP has already been enabled on the socket so the operation is a noop.
-         * Since its possible to call this twice, once for TX and once for RX, consider
-         * the noop a success. */
-        if (errno != EEXIST) {
-            RESULT_BAIL(S2N_ERR_KTLS_ULP);
-        }
+        /* https://man7.org/linux/man-pages/man3/errno.3.html
+         * EEXIST indicates that TCP_ULP has already been enabled on the socket.
+         * This is a noop and therefore safe to ignore.
+         */
+        RESULT_ENSURE(errno == EEXIST, S2N_ERR_KTLS_ULP);
     }
 
     return S2N_RESULT_OK;
@@ -154,8 +148,12 @@ int s2n_connection_ktls_enable_send(struct s2n_connection *conn)
     }
 
     POSIX_GUARD_RESULT(s2n_ktls_validate(conn, S2N_KTLS_MODE_SEND));
-
-    POSIX_GUARD_RESULT(s2n_ktls_configure_socket(conn, S2N_KTLS_MODE_SEND));
+    s2n_result res = s2n_ktls_configure_socket(conn, S2N_KTLS_MODE_SEND);
+    if (s2n_errno == S2N_ERR_KTLS_ALREADY_ENABLED) {
+        return S2N_SUCCESS;
+    } else {
+        POSIX_GUARD_RESULT(res);
+    }
 
     return S2N_SUCCESS;
 }
@@ -169,8 +167,12 @@ int s2n_connection_ktls_enable_recv(struct s2n_connection *conn)
     }
 
     POSIX_GUARD_RESULT(s2n_ktls_validate(conn, S2N_KTLS_MODE_RECV));
-
-    POSIX_GUARD_RESULT(s2n_ktls_configure_socket(conn, S2N_KTLS_MODE_RECV));
+    s2n_result res = s2n_ktls_configure_socket(conn, S2N_KTLS_MODE_RECV);
+    if (s2n_errno == S2N_ERR_KTLS_ALREADY_ENABLED) {
+        return S2N_SUCCESS;
+    } else {
+        POSIX_GUARD_RESULT(res);
+    }
 
     return S2N_SUCCESS;
 }

tls/s2n_ktls.h

@@ -18,11 +18,11 @@
 #include "tls/s2n_config.h"
 
 /* Define headers needed to enable and use kTLS.
- *  *
- *   * The inline header definitions are required to compile kTLS specific code.
- *    * kTLS has been tested on linux. For all other platforms, kTLS is marked as
- *     * unsuppored, and will return an un-supported error.
- *      */
+ *
+ * The inline header definitions are required to compile kTLS specific code.
+ * kTLS has been tested on linux. For all other platforms, kTLS is marked as
+ * unsupported, and will return an unsupported error.
+ */
 #if defined(__linux__)
     #define S2N_KTLS_SUPPORTED true
     #include "tls/s2n_ktls_linux.h"