fix(bindings): correct poll_flush implementation (#4859)

bindings/rust/s2n-tls/src/connection.rs

@@ -650,8 +650,18 @@ impl Connection {
     }
 
     /// Attempts to flush any data previously buffered by a call to [send](`Self::poll_send`).
+    ///
+    /// poll_flush can only flush data that s2n-tls has already encrypted and
+    /// buffered for sending. poll_send may need to be called again to fully send
+    /// all data. See the [Usage Guide](https://github.com/aws/s2n-tls/blob/main/docs/usage-guide/topics/ch07-io.md)
+    /// for more details.
     pub fn poll_flush(&mut self) -> Poll<Result<&mut Self, Error>> {
-        self.poll_send(&[0; 0]).map_ok(|_| self)
+        let mut blocked = s2n_blocked_status::NOT_BLOCKED;
+        unsafe {
+            s2n_flush(self.connection.as_ptr(), &mut blocked)
+                .into_poll()
+                .map_ok(|_| self)
+        }
     }
 
     /// Gets the number of bytes that are currently available in the buffer to be read.

docs/usage-guide/topics/ch07-io.md

@@ -103,7 +103,7 @@ connections aborted while active.
 
 A single call to `s2n_send()` may involve multiple system calls to write the
 provided application data. s2n-tls breaks the application data into fixed-sized
-records before encryption, and calls write for each record.
+records before encryption, then calls write for each record.
 [See the record size documentation for how record size may impact performance](./ch08-record-sizes.md).
 
 In non-blocking mode, `s2n_send()` will send data from the provided buffer and return the number of
@@ -117,7 +117,9 @@ able to send more data. This can be determined by using methods like
 [`poll`](https://linux.die.net/man/2/poll) or [`select`](https://linux.die.net/man/2/select).
 
 Unlike OpenSSL, repeated calls to `s2n_send()` should not duplicate the original
-parameters, but should update the inputs per the indication of size written.
+parameters, but should update the inputs per the indication of size written. s2n-tls
+will attempt to sanity check that the inputs are properly updated between calls.
+Because of those sanity checks, a zero-length send call cannot be used as a flushing mechanism.
 
 `s2n_sendv_with_offset()` behaves like `s2n_send()`, but supports vectorized buffers.
 The offset input should be updated between calls to reflect the data already written.

tests/unit/s2n_send_test.c

@@ -750,5 +750,60 @@ int main(int argc, char **argv)
         }
     };
 
+    /* Test: s2n_flush is necessary and not achievable with s2n_send */
+    {
+        bool use_send[] = { true, false };
+
+        /* To reproduce the problematic scenario, we need to block on
+         * sending a record during a multi-record write.
+         */
+        struct s2n_send_result results[] = {
+            BLOCK_SEND_RESULT,
+            OK_SEND_RESULT,
+            OK_SEND_RESULT,
+            OK_SEND_RESULT,
+        };
+
+        for (size_t i = 0; i < s2n_array_len(use_send); i++) {
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+            EXPECT_OK(s2n_connection_set_secrets(conn));
+
+            struct s2n_send_context context = {
+                .results = results,
+                .results_len = s2n_array_len(results)
+            };
+            EXPECT_SUCCESS(s2n_connection_set_send_cb(conn, s2n_test_send_cb));
+            EXPECT_SUCCESS(s2n_connection_set_send_ctx(conn, (void *) &context));
+
+            s2n_blocked_status blocked = 0;
+
+            /* First attempt blocks */
+            EXPECT_FAILURE_WITH_ERRNO(s2n_send(conn, large_test_data, sizeof(large_test_data), &blocked),
+                    S2N_ERR_IO_BLOCKED);
+            EXPECT_EQUAL(blocked, S2N_BLOCKED_ON_WRITE);
+            EXPECT_EQUAL(context.bytes_sent, 0);
+
+            /* For our control case, we attempt to use a zero-length send as flush */
+            if (use_send[i]) {
+                EXPECT_FAILURE_WITH_ERRNO(
+                        s2n_send(conn, large_test_data, 0, &blocked),
+                        S2N_ERR_SEND_SIZE);
+                continue;
+            }
+
+            /* Unlike the zero-length send, s2n_flush succeeds */
+            EXPECT_SUCCESS(s2n_flush(conn, &blocked));
+            EXPECT_EQUAL(context.bytes_sent, max_frag_bytes_sent[S2N_MFL_DEFAULT]);
+
+            /* We can also successfully finish sending */
+            EXPECT_EQUAL(
+                    s2n_send(conn, large_test_data, sizeof(large_test_data), &blocked),
+                    sizeof(large_test_data));
+            EXPECT_EQUAL(context.bytes_sent, large_test_data_bytes_sent);
+        }
+    };
+
     END_TEST();
 }

tls/s2n_internal.h

@@ -15,24 +15,21 @@
 
 #pragma once
 
+#include <s2n.h>
+
 #if ((__GNUC__ >= 4) || defined(__clang__)) && defined(S2N_EXPORTS)
     #define S2N_PRIVATE_API __attribute__((visibility("default")))
 #else
     #define S2N_PRIVATE_API
 #endif /* __GNUC__ >= 4 || defined(__clang__) */
 
-#include <stdint.h>
-
 /*
  * Internal APIs.
  *
  * These APIs change the behavior of S2N in potentially dangerous ways and should only be
  * used for testing purposes. All Internal APIs are subject to change without notice.
  */
 
-struct s2n_config;
-struct s2n_connection;
-
 /*
  * Gets the config set on the connection.
  *
@@ -53,3 +50,12 @@ S2N_PRIVATE_API int s2n_connection_get_config(struct s2n_connection *conn, struc
  */
 S2N_PRIVATE_API int s2n_config_add_cert_chain(struct s2n_config *config,
         uint8_t *cert_chain_pem, uint32_t cert_chain_pem_size);
+
+/*
+ * Attempts to flush any data buffered for sending.
+ *
+ * This method is not sufficient to complete a previous partial send. It can only
+ * attempt to flush data that has been encrypted and buffered, not data that
+ * is still waiting for encryption.
+ */
+S2N_PRIVATE_API int s2n_flush(struct s2n_connection *conn, s2n_blocked_status *blocked);

tls/s2n_send.c

@@ -24,6 +24,7 @@
 #include "tls/s2n_cipher_suites.h"
 #include "tls/s2n_connection.h"
 #include "tls/s2n_handshake.h"
+#include "tls/s2n_internal.h"
 #include "tls/s2n_ktls.h"
 #include "tls/s2n_post_handshake.h"
 #include "tls/s2n_record.h"
@@ -82,6 +83,8 @@ bool s2n_should_flush(struct s2n_connection *conn, ssize_t total_message_size)
 
 int s2n_flush(struct s2n_connection *conn, s2n_blocked_status *blocked)
 {
+    POSIX_ENSURE_REF(conn);
+    POSIX_ENSURE_REF(blocked);
     *blocked = S2N_BLOCKED_ON_WRITE;
 
     /* Write any data that's already pending */