Poll::Pending from TLS Session impls breaks internal state assumptions

[Mark-Simulacrum]

Security issue notifications

If you discover a potential security issue in s2n-quic we ask that you notify
AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

If a tls::Session impl returns Poll::Pending "spuriously" (self-wake and return Pending), without calling context methods to update the crypto state, s2n-quic won't gracefully defer until the Session impl is ready, causing debug assertions to be hit and possibly other state expectation violations.

Solution:

s2n-quic's internal poll_crypto API(s) need to be modified to support Pending being returned from tls::Session even if a packet should otherwise be enough to move forward.

• Does this change what s2n-quic sends over the wire? no
• Does this change any public APIs? no

Requirements / Acceptance Criteria:

• RFC links: n/a
• Related Issues: n/a
• Will the Usage Guide or other documentation need to be updated? n/a
• Testing: Tests using the full s2n-quic endpoint with s2n-tls PrivateKeyCallbacks etc. should be added, similar to https://github.com/aws/s2n-quic/blob/17b5ff261f5b5a42ea7b21520845d7a4aa40ccd8/quic/s2n-quic-tls/src/tests.rs but using the full endpoint. These are expected to fail today.

This diff should not cause any test failures:

diff --git a/quic/s2n-quic-tls/src/session.rs b/quic/s2n-quic-tls/src/session.rs
index b73f71f7..3d878c6c 100644
--- a/quic/s2n-quic-tls/src/session.rs
+++ b/quic/s2n-quic-tls/src/session.rs
@@ -29,6 +29,8 @@ pub struct Session {
     server_name: Option<ServerName>,
     received_ticket: bool,
     server_params: Vec<u8>,
+
+    defer: usize,
 }

 impl Session {
@@ -76,6 +78,7 @@ impl Session {
             server_name,
             received_ticket: false,
             server_params,
+            defer: 3,
         })
     }
 }
@@ -124,6 +127,16 @@ impl tls::Session for Session {
     where
         W: tls::Context<Self>,
     {
+        // While defer is non-zero, self-wake and return Pending.
+        if let Some(d) = self.defer.checked_sub(1) {
+            self.defer = d;
+            context.waker().wake_by_ref();
+            return Poll::Pending;
+        } else {
+            // we'll poll once and then defer again.
+            self.defer = 3;
+        }
+
         let mut callback: Callback<W, Self> = Callback {
             context,
             endpoint: self.endpoint,

Out of scope: