feature(bindings): scheduled renegotiation via poll_recv (#4764)

Author: lrstewart

.github/workflows/ci_rust.yml

@@ -44,16 +44,20 @@ jobs:
       - name: Generate
         run: ${{env.ROOT_PATH}}/generate.sh
 
-      - name: Tests
-        working-directory: ${{env.ROOT_PATH}}
-        # Test all features except for FIPS, which is tested separately.
-        run: cargo test --features unstable-renegotiate,unstable-fingerprint,unstable-ktls,quic,pq
-
       # Ensure that all tests pass with the default feature set
       - name: Default Tests
         working-directory: ${{env.ROOT_PATH}}
         run: cargo test
 
+      - name: "Feature Tests: Fingerprint, kTLS, QUIC, and PQ"
+        working-directory: ${{env.ROOT_PATH}}
+        # Test all features except for FIPS, which is tested separately.
+        run: cargo test --features unstable-fingerprint,unstable-ktls,quic,pq
+
+      - name: "Feature Test: Renegotiate"
+        working-directory: ${{env.ROOT_PATH}}
+        run: cargo test --features unstable-renegotiate
+
       - name: Test external build
         # if this test is failing, make sure that api headers are appropriately
         # included. For a symbol to be visible in a shared lib, the

api/unstable/renegotiate.h

@@ -29,6 +29,31 @@
  * s2n-tls clients support secure (compliant with RFC5746) renegotiation for compatibility reasons,
  * but s2n-tls does NOT recommend its use. While s2n-tls addresses all currently known security concerns,
  * renegotiation has appeared in many CVEs and was completely removed from TLS1.3.
+ *
+ * A basic renegotiation integration with s2n-tls would look like:
+ *  1. The application calls s2n_recv as part of normal IO.
+ *  2. s2n_recv receives a request for renegotiation (a HelloRequest message)
+ *     instead of application data.
+ *  3. s2n_recv calls the configured s2n_renegotiate_request_cb.
+ *  4. The application's implementation of the s2n_renegotiate_request_cb should:
+ *     1. Set the `response` parameter to S2N_RENEGOTIATE_ACCEPT
+ *     2. Set some application state to indicate that renegotiation is required.
+ *        s2n_connection_set_ctx can be used to associate application state with
+ *        a specific connection.
+ *     3. Return success.
+ *  5. s2n_recv returns as part of normal IO.
+ *  6. The application should check the application state set in 4.2 to determine
+ *     whether or not renegotiation is required.
+ *  7. The application should complete any in-progress IO. Failing to do this will
+ *     cause s2n_renegotiate_wipe to fail.
+ *     1. For sending, the application must retry any blocked calls to s2n_send
+ *        until they return success.
+ *     2. For receiving, the application must call s2n_recv to handle any buffered
+ *        decrypted application data. s2n_peek indicates how much data is buffered.
+ *  8. The application should call s2n_renegotiate_wipe.
+ *  9. The application should reconfigure the connection, if necessary.
+ * 10. The application should call s2n_renegotiate until it indicates success,
+ *     while handling any application data encountered.
  */
 
 /**
@@ -92,7 +117,7 @@ S2N_API int s2n_config_set_renegotiate_request_cb(struct s2n_config *config, s2n
  *
  * The application MUST handle any incomplete IO before calling this method. The last call to `s2n_send` must
  * have succeeded, and `s2n_peek` must return zero. If there is any data in the send or receive buffers,
- * this method will fail.
+ * this method will fail. That means that this method cannot be called from inside s2n_renegotiate_request_cb.
  *
  * The application MUST repeat any connection-specific setup after calling this method. This method
  * cannot distinguish between internal connection state and configuration state set by the application,
@@ -130,6 +155,10 @@ S2N_API int s2n_renegotiate_wipe(struct s2n_connection *conn);
  * copy the data to `app_data_buf`, and set `app_data_size` to the size of the data.
  * The application should handle the data in `app_data_buf` before calling s2n_renegotiate again.
  *
+ * This method cannot be called from inside s2n_renegotiate_request_cb. The receive
+ * call that triggered s2n_renegotiate_request_cb must complete before either
+ * s2n_renegotiate_wipe or s2n_renegotiate can be called.
+ *
  * @note s2n_renegotiate_wipe MUST be called before this method.
  * @note Calling this method on a server connection will fail. s2n-tls servers do not support renegotiation.
  *

bindings/rust/s2n-tls-tokio/src/lib.rs

@@ -171,8 +171,7 @@ where
         &mut self.stream
     }
 
-    async fn open(mut conn: C, stream: S) -> Result<Self, Error> {
-        conn.as_mut().set_blinding(Blinding::SelfService)?;
+    async fn open(conn: C, stream: S) -> Result<Self, Error> {
         let mut tls = TlsStream {
             conn,
             stream,
@@ -203,6 +202,7 @@ where
             self.as_mut().set_receive_context(context)?;
             self.as_mut().set_send_context(context)?;
             self.as_mut().set_waker(Some(ctx.waker()))?;
+            self.as_mut().set_blinding(Blinding::SelfService)?;
 
             let result = action(Pin::new(self));
 

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

@@ -3,6 +3,8 @@
 
 #![allow(clippy::missing_safety_doc)] // TODO add safety docs
 
+#[cfg(feature = "unstable-renegotiate")]
+use crate::renegotiate::RenegotiateState;
 use crate::{
     callbacks::*,
     cert_chain::CertificateChain,
@@ -580,23 +582,33 @@ impl Connection {
     /// [negotiate](`Self::poll_negotiate`) has succeeded.
     ///
     /// Returns the number of bytes written, and may indicate a partial write.
+    #[cfg(not(feature = "unstable-renegotiate"))]
     pub fn poll_send(&mut self, buf: &[u8]) -> Poll<Result<usize, Error>> {
         let mut blocked = s2n_blocked_status::NOT_BLOCKED;
         let buf_len: isize = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?;
         let buf_ptr = buf.as_ptr() as *const ::libc::c_void;
         unsafe { s2n_send(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
     }
 
+    #[cfg(not(feature = "unstable-renegotiate"))]
+    pub(crate) fn poll_recv_raw(
+        &mut self,
+        buf_ptr: *mut ::libc::c_void,
+        buf_len: isize,
+    ) -> Poll<Result<usize, Error>> {
+        let mut blocked = s2n_blocked_status::NOT_BLOCKED;
+        unsafe { s2n_recv(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
+    }
+
     /// Reads and decrypts data from a connection where
     /// [negotiate](`Self::poll_negotiate`) has succeeded.
     ///
     /// Returns the number of bytes read, and may indicate a partial read.
     /// 0 bytes returned indicates EOF due to connection closure.
     pub fn poll_recv(&mut self, buf: &mut [u8]) -> Poll<Result<usize, Error>> {
-        let mut blocked = s2n_blocked_status::NOT_BLOCKED;
         let buf_len: isize = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?;
         let buf_ptr = buf.as_ptr() as *mut ::libc::c_void;
-        unsafe { s2n_recv(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
+        self.poll_recv_raw(buf_ptr, buf_len)
     }
 
     /// Reads and decrypts data from a connection where
@@ -614,7 +626,6 @@ impl Connection {
         &mut self,
         buf: &mut [MaybeUninit<u8>],
     ) -> Poll<Result<usize, Error>> {
-        let mut blocked = s2n_blocked_status::NOT_BLOCKED;
         let buf_len: isize = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?;
         let buf_ptr = buf.as_ptr() as *mut ::libc::c_void;
 
@@ -623,7 +634,7 @@ impl Connection {
         // 2. if s2n_recv returns `+n`, it guarantees that the first
         // `n` bytes of `buf` have been initialized, which allows this
         // function to return `Ok(n)`
-        unsafe { s2n_recv(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
+        self.poll_recv_raw(buf_ptr, buf_len)
     }
 
     /// Attempts to flush any data previously buffered by a call to [send](`Self::poll_send`).
@@ -1168,6 +1179,16 @@ impl Connection {
             Some(app_context) => app_context.downcast_mut::<T>(),
         }
     }
+
+    #[cfg(feature = "unstable-renegotiate")]
+    pub(crate) fn renegotiate_state_mut(&mut self) -> &mut RenegotiateState {
+        &mut self.context_mut().renegotiate_state
+    }
+
+    #[cfg(feature = "unstable-renegotiate")]
+    pub(crate) fn renegotiate_state(&self) -> &RenegotiateState {
+        &self.context().renegotiate_state
+    }
 }
 
 struct Context {
@@ -1177,6 +1198,8 @@ struct Context {
     verify_host_callback: Option<Box<dyn VerifyHostNameCallback>>,
     connection_initialized: bool,
     app_context: Option<Box<dyn Any + Send + Sync>>,
+    #[cfg(feature = "unstable-renegotiate")]
+    pub(crate) renegotiate_state: RenegotiateState,
 }
 
 impl Context {
@@ -1188,6 +1211,8 @@ impl Context {
             verify_host_callback: None,
             connection_initialized: false,
             app_context: None,
+            #[cfg(feature = "unstable-renegotiate")]
+            renegotiate_state: RenegotiateState::default(),
         }
     }
 }