feat: reduce read syscalls to improve performance (#4485)

Author: lrstewart

api/s2n.h

@@ -1812,6 +1812,65 @@ S2N_API extern int s2n_connection_prefer_throughput(struct s2n_connection *conn)
  */
 S2N_API extern int s2n_connection_prefer_low_latency(struct s2n_connection *conn);
 
+/**
+ * Configure the connection to reduce potentially expensive calls to recv.
+ *
+ * If this setting is disabled, s2n-tls will call read twice for every TLS record,
+ * which can be expensive but ensures that s2n-tls will always attempt to read the
+ * exact number of bytes it requires. If this setting is enabled, s2n-tls will
+ * instead reduce the number of calls to read by attempting to read as much data
+ * as possible in each read call, storing the extra in the existing IO buffers.
+ * This may cause it to request more data than will ever actually be available.
+ *
+ * There is no additional memory cost of enabling this setting. It reuses the
+ * existing IO buffers.
+ *
+ * This setting is disabled by default. Depending on how your application detects
+ * data available for reading, buffering reads may break your event loop.
+ * In particular, note that:
+ *
+ * 1. File descriptor reads or calls to your custom s2n_recv_cb may request more
+ *    data than is available. Reads must return partial data when available rather
+ *    than blocking until all requested data is available.
+ *
+ * 2. s2n_negotiate may read and buffer application data records.
+ *    You must call s2n_recv at least once after negotiation to ensure that you
+ *    handle any buffered data.
+ *
+ * 3. s2n_recv may read and buffer more records than it parses and decrypts.
+ *    You must call s2n_recv until it reports S2N_ERR_T_BLOCKED, rather than just
+ *    until it reports S2N_SUCCESS.
+ *
+ * 4. s2n_peek reports available decrypted data. It does not report any data
+ *    buffered by this feature.
+ *
+ * 5. s2n_connection_release_buffers will not release the input buffer if it
+ *    contains buffered data.
+ *
+ * For example: if your event loop uses `poll`, you will receive a POLLIN event
+ * for your read file descriptor when new data is available. When you call s2n_recv
+ * to read that data, s2n-tls reads one or more TLS records from the file descriptor.
+ * If you stop calling s2n_recv before it reports S2N_ERR_T_BLOCKED, some of those
+ * records may remain in s2n-tls's read buffer. If you read part of a record,
+ * s2n_peek will report the remainder of that record as available. But if you don't
+ * read any of a record, it remains encrypted and is not reported by s2n_peek.
+ * And because the data is buffered in s2n-tls instead of in the file descriptor,
+ * another call to `poll` will NOT report any more data available. Your application
+ * may hang waiting for more data.
+ *
+ * @warning This feature cannot be enabled for a connection that will enable kTLS for receiving.
+ *
+ * @warning This feature may work with blocking IO, if used carefully. Your blocking
+ * IO must support partial reads (so MSG_WAITALL cannot be used). You will need
+ * to know how much data will eventually be available rather than relying on
+ * S2N_ERR_T_BLOCKED as noted in #3 above.
+ *
+ * @param conn The connection object being updated
+ * @param enabled Set to `true` to enable, `false` to disable.
+ * @returns S2N_SUCCESS on success. S2N_FAILURE on failure
+ */
+S2N_API extern int s2n_connection_set_recv_buffering(struct s2n_connection *conn, bool enabled);
+
 /**
  * Configure the connection to free IO buffers when they are not currently in use.
  *

api/unstable/ktls.h

@@ -38,6 +38,7 @@
  *   The TLS kernel module currently doesn't support renegotiation.
  * - By default, you must negotiate TLS1.2. See s2n_config_ktls_enable_tls13
  *   for the requirements to also support TLS1.3.
+ * - You must not use s2n_connection_set_recv_buffering
  */
 
 /**

stuffer/s2n_stuffer.c

@@ -215,7 +215,7 @@ int s2n_stuffer_wipe_n(struct s2n_stuffer *stuffer, const uint32_t size)
 
 bool s2n_stuffer_is_consumed(struct s2n_stuffer *stuffer)
 {
-    return stuffer && (stuffer->read_cursor == stuffer->write_cursor);
+    return stuffer && (stuffer->read_cursor == stuffer->write_cursor) && !stuffer->tainted;
 }
 
 int s2n_stuffer_wipe(struct s2n_stuffer *stuffer)

tests/cbmc/proofs/s2n_stuffer_is_consumed/s2n_stuffer_is_consumed_harness.c

@@ -33,10 +33,13 @@ void s2n_stuffer_is_consumed_harness()
     save_byte_from_blob(&stuffer->blob, &old_byte_from_stuffer);
 
     /* Operation under verification. */
-    if (s2n_stuffer_is_consumed(stuffer)) {
-        assert(stuffer->read_cursor == old_stuffer.write_cursor);
+    bool result = s2n_stuffer_is_consumed(stuffer);
+    if (old_stuffer.read_cursor != old_stuffer.write_cursor) {
+        assert(result == false);
+    } else if (old_stuffer.tainted) {
+        assert(result == false);
     } else {
-        assert(stuffer->read_cursor != old_stuffer.write_cursor);
+        assert(result == true);
     }
 
     /* Post-conditions. */

tests/unit/s2n_connection_size_test.c

@@ -45,7 +45,7 @@ int main(int argc, char **argv)
     }
 
     /* Carefully consider any increases to this number. */
-    const uint16_t max_connection_size = 4290;
+    const uint16_t max_connection_size = 4350;
     const uint16_t min_connection_size = max_connection_size * 0.9;
 
     size_t connection_size = sizeof(struct s2n_connection);

tests/unit/s2n_ktls_io_test.c

@@ -1118,7 +1118,8 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_ktls_read_full_record(conn, &record_type));
             EXPECT_EQUAL(record_type, TLS_ALERT);
 
-            EXPECT_EQUAL(conn->in.blob.allocated, max_frag_len);
+            EXPECT_EQUAL(conn->buffer_in.blob.allocated, max_frag_len);
+            EXPECT_EQUAL(conn->in.blob.size, max_frag_len);
             EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), max_frag_len);
             uint8_t *read = s2n_stuffer_raw_read(&conn->in, max_frag_len);
             EXPECT_BYTEARRAY_EQUAL(read, test_data, max_frag_len);
@@ -1152,7 +1153,8 @@ int main(int argc, char **argv)
             /* Verify that conn->in reflects the correct size of the "record"
              * read and doesn't just assume the maximum read size.
              */
-            EXPECT_EQUAL(conn->in.blob.allocated, max_frag_len);
+            EXPECT_EQUAL(conn->buffer_in.blob.allocated, max_frag_len);
+            EXPECT_EQUAL(conn->in.blob.size, small_frag_len);
             EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), small_frag_len);
             uint8_t *read = s2n_stuffer_raw_read(&conn->in, small_frag_len);
             EXPECT_BYTEARRAY_EQUAL(read, test_data, small_frag_len);
@@ -1172,6 +1174,8 @@ int main(int argc, char **argv)
             /* Write half the test data into conn->in */
             const size_t offset = sizeof(test_data) / 2;
             EXPECT_SUCCESS(s2n_stuffer_write_bytes(&conn->in, test_data, offset));
+            /* Resize conn->buffer_in to match conn->in */
+            EXPECT_SUCCESS(s2n_stuffer_resize(&conn->buffer_in, offset));
 
             /* Write the other half into a new record */
             size_t written = 0;
@@ -1201,6 +1205,41 @@ int main(int argc, char **argv)
             read = s2n_stuffer_raw_read(&conn->in, offset_iovec.iov_len);
             EXPECT_BYTEARRAY_EQUAL(read, offset_iovec.iov_base, offset_iovec.iov_len);
         };
+
+        /* Test: Receive multiple records */
+        {
+            const size_t small_frag_len = 10;
+            EXPECT_TRUE(small_frag_len < max_frag_len);
+            EXPECT_TRUE(small_frag_len < sizeof(test_data));
+            struct iovec small_test_iovec = test_iovec;
+            small_test_iovec.iov_len = small_frag_len;
+
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+
+            DEFER_CLEANUP(struct s2n_test_ktls_io_stuffer_pair pair = { 0 },
+                    s2n_ktls_io_stuffer_pair_free);
+            EXPECT_OK(s2n_test_init_ktls_io_stuffer(conn, conn, &pair));
+            struct s2n_test_ktls_io_stuffer *ctx = &pair.client_in;
+
+            for (size_t i = 0; i < 100; i++) {
+                size_t written = 0;
+                EXPECT_OK(s2n_ktls_sendmsg(ctx, TLS_ALERT, &small_test_iovec, 1, &blocked, &written));
+                EXPECT_EQUAL(written, small_frag_len);
+
+                uint8_t record_type = 0;
+                EXPECT_SUCCESS(s2n_ktls_read_full_record(conn, &record_type));
+                EXPECT_EQUAL(record_type, TLS_ALERT);
+                EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), written);
+                uint8_t *read = s2n_stuffer_raw_read(&conn->in, small_frag_len);
+                EXPECT_BYTEARRAY_EQUAL(read, test_data, small_frag_len);
+
+                EXPECT_OK(s2n_record_wipe(conn));
+                size_t space_remaining = s2n_stuffer_space_remaining(&conn->buffer_in);
+                EXPECT_EQUAL(space_remaining, max_frag_len);
+            }
+        };
     };
 
     /* Test: key encryption limit tracked */

tests/unit/s2n_ktls_test.c

@@ -387,6 +387,22 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn));
         };
 
+        /* Fail if buffer_in contains any data.
+         * A connection that will enable ktls needs to disable recv_greedy
+         */
+        {
+            DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
+                    s2n_connection_ptr_free);
+            EXPECT_OK(s2n_test_configure_connection_for_ktls(server_conn));
+
+            EXPECT_SUCCESS(s2n_stuffer_write_uint8(&server_conn->buffer_in, 1));
+            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_ktls_enable_recv(server_conn),
+                    S2N_ERR_KTLS_UNSUPPORTED_CONN);
+
+            EXPECT_SUCCESS(s2n_stuffer_skip_read(&server_conn->buffer_in, 1));
+            EXPECT_SUCCESS(s2n_connection_ktls_enable_recv(server_conn));
+        };
+
         /* Fail if not using managed IO for send */
         {
             DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),

tests/unit/s2n_quic_support_io_test.c

@@ -244,6 +244,87 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_stuffer_free(&stuffer));
             EXPECT_SUCCESS(s2n_connection_free(conn));
         };
+
+        /* Succeeds for a handshake message larger than the input buffer */
+        {
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+
+            DEFER_CLEANUP(struct s2n_stuffer stuffer = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));
+            EXPECT_SUCCESS(s2n_connection_set_io_stuffers(&stuffer, &stuffer, conn));
+
+            uint8_t actual_message_type = 0;
+
+            /* Read a small message to initialize the input buffer */
+            const size_t small_message_size = 10;
+            EXPECT_SUCCESS(s2n_stuffer_write_uint8(&stuffer, 7));
+            EXPECT_SUCCESS(s2n_stuffer_write_uint24(&stuffer, small_message_size));
+            EXPECT_SUCCESS(s2n_stuffer_skip_write(&stuffer, small_message_size));
+            EXPECT_OK(s2n_quic_read_handshake_message(conn, &actual_message_type));
+            EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), small_message_size);
+
+            EXPECT_SUCCESS(s2n_stuffer_wipe(&conn->handshake.io));
+            EXPECT_OK(s2n_record_wipe(conn));
+            const size_t max_buffer_size = s2n_stuffer_space_remaining(&conn->buffer_in);
+            EXPECT_TRUE(max_buffer_size > small_message_size);
+
+            /* Read a large message to force the input buffer to resize */
+            const size_t large_message_size = max_buffer_size + 10;
+            EXPECT_SUCCESS(s2n_stuffer_write_uint8(&stuffer, 7));
+            EXPECT_SUCCESS(s2n_stuffer_write_uint24(&stuffer, large_message_size));
+            EXPECT_SUCCESS(s2n_stuffer_skip_write(&stuffer, large_message_size));
+            EXPECT_OK(s2n_quic_read_handshake_message(conn, &actual_message_type));
+            EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), large_message_size);
+
+            EXPECT_SUCCESS(s2n_stuffer_wipe(&conn->handshake.io));
+            EXPECT_OK(s2n_record_wipe(conn));
+            const size_t resized_buffer_size = s2n_stuffer_space_remaining(&conn->buffer_in);
+            EXPECT_TRUE(resized_buffer_size >= large_message_size);
+
+            /* Read another message to check that the resize doesn't prevent future reads */
+            EXPECT_SUCCESS(s2n_stuffer_write_uint8(&stuffer, 7));
+            EXPECT_SUCCESS(s2n_stuffer_write_uint24(&stuffer, TEST_DATA_SIZE));
+            EXPECT_SUCCESS(s2n_stuffer_write_bytes(&stuffer, TEST_DATA, TEST_DATA_SIZE));
+            EXPECT_OK(s2n_quic_read_handshake_message(conn, &actual_message_type));
+            EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), TEST_DATA_SIZE);
+            EXPECT_BYTEARRAY_EQUAL(s2n_stuffer_raw_read(&conn->in, TEST_DATA_SIZE),
+                    TEST_DATA, sizeof(TEST_DATA));
+        };
+
+        /* Succeeds for multiple messages */
+        {
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+
+            DEFER_CLEANUP(struct s2n_stuffer stuffer = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));
+            EXPECT_SUCCESS(s2n_connection_set_io_stuffers(&stuffer, &stuffer, conn));
+
+            uint8_t actual_message_type = 0;
+            size_t expected_buffer_size = 0;
+            for (size_t i = 0; i < 100; i++) {
+                EXPECT_SUCCESS(s2n_stuffer_write_uint8(&stuffer, 7));
+                EXPECT_SUCCESS(s2n_stuffer_write_uint24(&stuffer, TEST_DATA_SIZE));
+                EXPECT_SUCCESS(s2n_stuffer_write_bytes(&stuffer, TEST_DATA, TEST_DATA_SIZE));
+                EXPECT_OK(s2n_quic_read_handshake_message(conn, &actual_message_type));
+                EXPECT_EQUAL(s2n_stuffer_data_available(&conn->in), TEST_DATA_SIZE);
+                EXPECT_BYTEARRAY_EQUAL(s2n_stuffer_raw_read(&conn->in, TEST_DATA_SIZE),
+                        TEST_DATA, sizeof(TEST_DATA));
+
+                EXPECT_SUCCESS(s2n_stuffer_wipe(&conn->handshake.io));
+                EXPECT_OK(s2n_record_wipe(conn));
+
+                /* Ensure buffer size stays constant */
+                const size_t buffer_size = s2n_stuffer_space_remaining(&conn->buffer_in);
+                if (i == 0) {
+                    expected_buffer_size = buffer_size;
+                }
+                EXPECT_EQUAL(expected_buffer_size, buffer_size);
+            }
+        };
     };
 
     /* Functional Tests */