feedback

tests/testlib/s2n_ktls_test_utils.c

@@ -22,20 +22,17 @@ S2N_RESULT s2n_test_ktls_update_prev_header_len(struct s2n_test_ktls_io_stuffer
     RESULT_ENSURE_REF(io_ctx);
     RESULT_ENSURE(remaining_len > 0, S2N_ERR_IO);
 
-    /* rewind the read ptr */
-    RESULT_GUARD_POSIX(s2n_stuffer_rewind_read(&io_ctx->ancillary_buffer, S2N_TEST_KTLS_MOCK_HEADER_LENGTH_SIZE));
-
-    /* rewrite the length */
-    uint8_t *ptr = s2n_stuffer_raw_read(&io_ctx->ancillary_buffer, S2N_TEST_KTLS_MOCK_HEADER_LENGTH_SIZE);
-    struct s2n_blob ancillary_blob = { 0 };
-    RESULT_GUARD_POSIX(s2n_blob_init(&ancillary_blob, ptr, S2N_TEST_KTLS_MOCK_HEADER_LENGTH_SIZE));
-    struct s2n_stuffer ancillary_stuffer = { 0 };
-    RESULT_GUARD_POSIX(s2n_stuffer_init(&ancillary_stuffer, &ancillary_blob));
-    RESULT_GUARD_POSIX(s2n_stuffer_write_uint16(&ancillary_stuffer, remaining_len));
-
-    /* rewind to re-read the record with the remaining length */
+    /* rewind the last header */
     RESULT_GUARD_POSIX(s2n_stuffer_rewind_read(&io_ctx->ancillary_buffer, S2N_TEST_KTLS_MOCK_HEADER_SIZE));
 
+    /* get position for the last header's length */
+    uint32_t rewrite_len_ptr = io_ctx->ancillary_buffer.read_cursor + S2N_TEST_KTLS_MOCK_HEADER_TAG_SIZE;
+    /* create a new stuffer pointing to len data and rewrite it */
+    struct s2n_stuffer rewrite_len_stuffer = io_ctx->ancillary_buffer;
+    RESULT_GUARD_POSIX(s2n_stuffer_rewrite(&rewrite_len_stuffer));
+    RESULT_GUARD_POSIX(s2n_stuffer_skip_write(&rewrite_len_stuffer, rewrite_len_ptr));
+    RESULT_GUARD_POSIX(s2n_stuffer_write_uint16(&rewrite_len_stuffer, remaining_len));
+
     return S2N_RESULT_OK;
 }
 
@@ -49,9 +46,6 @@ ssize_t s2n_test_ktls_sendmsg_stuffer_io(struct s2n_connection *conn, struct msg
     struct s2n_test_ktls_io_stuffer *io_ctx = (struct s2n_test_ktls_io_stuffer *) conn->send_io_context;
     POSIX_ENSURE_REF(io_ctx);
     io_ctx->send_recv_msg_invoked_count++;
-    /* Assert ancillary_buffer is growable, which simplifies IO logic. Blocking IO can
-     * be mocked by restricting the data_buffer. */
-    POSIX_ENSURE(io_ctx->ancillary_buffer.growable, S2N_ERR_IO);
 
     size_t total_len = 0;
     for (size_t count = 0; count < msg->msg_iovlen; count++) {
@@ -61,14 +55,23 @@ ssize_t s2n_test_ktls_sendmsg_stuffer_io(struct s2n_connection *conn, struct msg
 
         /* If we fail to write to stuffer then return blocked */
         if (s2n_stuffer_write_bytes(&io_ctx->data_buffer, buf, len) < 0) {
+            if (count > 0) {
+                /* This mock implementation doesn't handle partial writes for msg_iovlen > 1.
+                 *
+                 * This simplifies the implementation and importantly doesn't limit our test
+                 * coverage because partial write are handled the same regardless of
+                 * msg_iovlen. */
+                POSIX_BAIL(S2N_ERR_SAFETY);
+            }
+
             errno = EAGAIN;
             return -1;
         }
 
         total_len += len;
     }
     if (total_len) {
-        /* write record_type and len after data was written successfully */
+        /* write record_type and len after some data was written successfully */
         POSIX_GUARD(s2n_stuffer_write_uint8(&io_ctx->ancillary_buffer, record_type));
         POSIX_GUARD(s2n_stuffer_write_uint16(&io_ctx->ancillary_buffer, total_len));
     }
@@ -87,17 +90,13 @@ ssize_t s2n_test_ktls_recvmsg_stuffer_io(struct s2n_connection *conn, struct msg
     POSIX_ENSURE_REF(conn->recv_io_context);
     POSIX_ENSURE_REF(msg);
     POSIX_ENSURE_REF(msg->msg_iov);
+    POSIX_ENSURE_REF(record_type);
 
     struct s2n_test_ktls_io_stuffer *io_ctx = (struct s2n_test_ktls_io_stuffer *) conn->recv_io_context;
     POSIX_ENSURE_REF(io_ctx);
     io_ctx->send_recv_msg_invoked_count++;
-    /* Assert ancillary_buffer is growable, which simplifies IO logic. Blocking IO can
-     * be mocked by restricting the data_buffer. */
-    POSIX_ENSURE(io_ctx->ancillary_buffer.growable, S2N_ERR_IO);
-
     /* s2n only receives using msg_iovlen of 1 */
     POSIX_ENSURE_EQ(msg->msg_iovlen, 1);
-
     uint8_t *buf = msg->msg_iov->iov_base;
     POSIX_ENSURE_REF(buf);
 
@@ -122,7 +121,7 @@ ssize_t s2n_test_ktls_recvmsg_stuffer_io(struct s2n_connection *conn, struct msg
 
         /* read minimul of requested_len and bytes_available */
         size_t n_read = MIN(updated_requested_len, n_avail);
-        POSIX_ENSURE(n_read > 0, S2N_ERR_SAFETY);
+        POSIX_ENSURE_GT(n_read, 0);
 
         int ret = s2n_stuffer_read_bytes(&io_ctx->data_buffer, buf + total_read, n_read);
         if (ret < 0) {
@@ -150,7 +149,6 @@ ssize_t s2n_test_ktls_recvmsg_stuffer_io(struct s2n_connection *conn, struct msg
 
             bool no_more_records = ret < 0;
             bool next_record_different_type = next_record_type != *record_type;
-
             if (no_more_records || next_record_different_type) {
                 break;
             }
@@ -165,7 +163,6 @@ S2N_RESULT s2n_test_validate_data(struct s2n_test_ktls_io_stuffer *ktls_io, uint
     RESULT_ENSURE_REF(ktls_io);
     RESULT_ENSURE_REF(expected_data);
 
-    /* verify data */
     struct s2n_stuffer *stuffer = &ktls_io->data_buffer;
     RESULT_ENSURE_EQ(s2n_stuffer_data_available(stuffer), expected_len);
     uint8_t *data_ptr = s2n_stuffer_raw_read(stuffer, expected_len);
@@ -179,7 +176,6 @@ S2N_RESULT s2n_test_validate_ancillary(struct s2n_test_ktls_io_stuffer *ktls_io,
 {
     RESULT_ENSURE_REF(ktls_io);
 
-    /* verify ancillary data */
     uint8_t tag;
     RESULT_GUARD_POSIX(s2n_stuffer_read_uint8(&ktls_io->ancillary_buffer, &tag));
     RESULT_ENSURE_EQ(tag, expected_record_type);
@@ -207,6 +203,7 @@ S2N_RESULT s2n_test_init_ktls_stuffer_io(struct s2n_connection *server, struct s
     RESULT_GUARD(s2n_ktls_set_recvmsg_cb(server, s2n_test_ktls_recvmsg_stuffer_io, &io_pair->server_in));
     RESULT_GUARD(s2n_ktls_set_sendmsg_cb(client, s2n_test_ktls_sendmsg_stuffer_io, &io_pair->server_in));
     RESULT_GUARD(s2n_ktls_set_recvmsg_cb(client, s2n_test_ktls_recvmsg_stuffer_io, &io_pair->client_in));
+
     return S2N_RESULT_OK;
 }
 
@@ -217,5 +214,6 @@ S2N_CLEANUP_RESULT s2n_ktls_io_pair_free(struct s2n_test_ktls_io_pair *ctx)
     RESULT_GUARD_POSIX(s2n_stuffer_free(&ctx->client_in.ancillary_buffer));
     RESULT_GUARD_POSIX(s2n_stuffer_free(&ctx->server_in.data_buffer));
     RESULT_GUARD_POSIX(s2n_stuffer_free(&ctx->server_in.ancillary_buffer));
+
     return S2N_RESULT_OK;
 }

tests/testlib/s2n_ktls_test_utils.h

@@ -16,8 +16,8 @@
 #include "tls/s2n_connection.h"
 #include "tls/s2n_ktls.h"
 
-#define S2N_TEST_KTLS_MOCK_HEADER_SIZE        3
-#define S2N_TEST_KTLS_MOCK_HEADER_LENGTH_SIZE 2
+#define S2N_TEST_KTLS_MOCK_HEADER_SIZE     3
+#define S2N_TEST_KTLS_MOCK_HEADER_TAG_SIZE 1
 
 /* The record_type is communicated via ancillary data when using kTLS. For this
  * reason s2n must use `send/recvmsg` syscalls rather than `send/read`. To mimic
@@ -41,7 +41,7 @@
  *            ancillary_buffer
  *    [ [record] [record] [record] ]
  *    [ [u8|u16] [u8|u16] [u8|u16] ]
- *    [  [21|5]   [22|7]    [21|2] ]
+ *    [  [23|5]   [23|7]    [21|2] ]
  *           |        |         |
  *     v-------v v-----------v v-v
  *    [1 2 3 4 5 1 2 3 4 5 6 7 1 2]

tests/testlib/s2n_testlib.h

@@ -17,8 +17,8 @@
 
 #include <stdint.h>
 
+#include "stuffer/s2n_stuffer.h"
 #include "tls/s2n_connection.h"
-#include "tls/s2n_ktls.h"
 
 extern const struct s2n_ecc_preferences ecc_preferences_for_retry;
 extern const struct s2n_security_policy security_policy_test_tls13_retry;
@@ -245,4 +245,3 @@ extern const s2n_parsed_extension EMPTY_PARSED_EXTENSIONS[S2N_PARSED_EXTENSIONS_
 int s2n_kem_recv_public_key_fuzz_test(const uint8_t *buf, size_t len, struct s2n_kem_params *kem_params);
 int s2n_kem_recv_ciphertext_fuzz_test(const uint8_t *buf, size_t len, struct s2n_kem_params *kem_params);
 int s2n_kem_recv_ciphertext_fuzz_test_init(const char *kat_file_path, struct s2n_kem_params *kem_params);
-

tests/unit/s2n_ktls_test.c

@@ -13,6 +13,8 @@
  * permissions and limitations under the License.
  */
 
+#include "tls/s2n_ktls.h"
+
 #include "s2n_test.h"
 #include "testlib/s2n_testlib.h"
 #include "utils/s2n_random.h"

tests/unit/s2n_ktls_test_utils_test.c

@@ -43,13 +43,13 @@ int main(int argc, char **argv)
                     s2n_ktls_io_pair_free);
             EXPECT_OK(s2n_test_init_ktls_stuffer_io(server, client, &io_pair));
 
-            size_t to_send = 0;
+            size_t send_zero = 0;
             /* Init send msghdr */
-            struct iovec send_msg_iov = { .iov_base = test_data, .iov_len = to_send };
+            struct iovec send_msg_iov = { .iov_base = test_data, .iov_len = send_zero };
             struct msghdr send_msg = { .msg_iov = &send_msg_iov, .msg_iovlen = 1 };
             /* sendmsg */
             ssize_t bytes_written = s2n_test_ktls_sendmsg_stuffer_io(server, &send_msg, S2N_TEST_RECORD_TYPE);
-            EXPECT_EQUAL(bytes_written, to_send);
+            EXPECT_EQUAL(bytes_written, send_zero);
 
             EXPECT_EQUAL(io_pair.client_in.send_recv_msg_invoked_count, 1);
             EXPECT_EQUAL(io_pair.server_in.send_recv_msg_invoked_count, 0);
@@ -79,11 +79,13 @@ int main(int argc, char **argv)
             /* rewind the read so that the recvmsg can retrieve it */
             EXPECT_SUCCESS(s2n_stuffer_rewind_read(&io_pair.client_in.data_buffer, to_send));
             EXPECT_SUCCESS(s2n_stuffer_rewind_read(&io_pair.client_in.ancillary_buffer, S2N_TEST_KTLS_MOCK_HEADER_SIZE));
+            EXPECT_EQUAL(io_pair.client_in.send_recv_msg_invoked_count, 1);
 
             /* Init recv msghdr */
             uint8_t recv_buffer[S2N_TLS_MAXIMUM_FRAGMENT_LENGTH] = { 0 };
             struct iovec recv_msg_iov = { .iov_base = recv_buffer, .iov_len = to_send };
             struct msghdr recv_msg = { .msg_iov = &recv_msg_iov, .msg_iovlen = 1 };
+            /* recvmsg */
             uint8_t recv_record_type = 0;
             ssize_t bytes_read = s2n_test_ktls_recvmsg_stuffer_io(client, &recv_msg, &recv_record_type);
             EXPECT_EQUAL(bytes_read, to_send);

tls/s2n_ktls.c

@@ -99,7 +99,7 @@ static S2N_RESULT s2n_ktls_validate(struct s2n_connection *conn, s2n_ktls_mode k
  *
  * Retrieving fd assumes that the connection is using socket IO and has the
  * send_io_context set. While kTLS overrides IO and essentially disables
- * the socket conn->send function callback, it doesnt modify the
+ * the socket conn->send function callback, it doesn't modify the
  * send_io_context. */
 S2N_RESULT s2n_ktls_get_file_descriptor(struct s2n_connection *conn, s2n_ktls_mode ktls_mode, int *fd)
 {

tls/s2n_ktls_io.c

@@ -26,7 +26,6 @@ S2N_RESULT s2n_ktls_set_sendmsg_cb(struct s2n_connection *conn, s2n_ktls_sendmsg
     RESULT_ENSURE_REF(conn);
     RESULT_ENSURE_REF(send_ctx);
     RESULT_ENSURE(s2n_in_test(), S2N_ERR_NOT_IN_TEST);
-
     conn->send_io_context = send_ctx;
     s2n_sendmsg_fn = send_cb;
     return S2N_RESULT_OK;
@@ -37,7 +36,6 @@ S2N_RESULT s2n_ktls_set_recvmsg_cb(struct s2n_connection *conn, s2n_ktls_recvmsg
     RESULT_ENSURE_REF(conn);
     RESULT_ENSURE_REF(recv_ctx);
     RESULT_ENSURE(s2n_in_test(), S2N_ERR_NOT_IN_TEST);
-
     conn->recv_io_context = recv_ctx;
     s2n_recvmsg_fn = recv_cb;
     return S2N_RESULT_OK;