Introduce setup_accept and setup_connect

These two new kinds of methods immediately return a MidHandshakeSslStream
instead of actually initiating a handshake. This greatly simplifies
loops around MidHandshakeSslStream::WouldBlock.

boring/src/ssl/connector.rs

@@ -10,6 +10,8 @@ use crate::ssl::{
 use crate::version;
 use std::net::IpAddr;
 
+use super::MidHandshakeSslStream;
+
 const FFDHE_2048: &str = "
 -----BEGIN DH PARAMETERS-----
 MIIBCAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz
@@ -99,11 +101,30 @@ impl SslConnector {
     /// Initiates a client-side TLS session on a stream.
     ///
     /// The domain is used for SNI and hostname verification.
+    pub fn setup_connect<S>(
+        &self,
+        domain: &str,
+        stream: S,
+    ) -> Result<MidHandshakeSslStream<S>, ErrorStack>
+    where
+        S: Read + Write,
+    {
+        self.configure()?.setup_connect(domain, stream)
+    }
+
+    /// Attempts a client-side TLS session on a stream.
+    ///
+    /// The domain is used for SNI (if it is not an IP address) and hostname verification if enabled.
+    ///
+    /// This is a convenience method which combines [`Self::setup_connect`] and
+    /// [`MidHandshakeSslStream::handshake`].
     pub fn connect<S>(&self, domain: &str, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
     where
         S: Read + Write,
     {
-        self.configure()?.connect(domain, stream)
+        self.setup_connect(domain, stream)
+            .map_err(HandshakeError::SetupFailure)?
+            .handshake()
     }
 
     /// Returns a structure allowing for configuration of a single TLS session before connection.
@@ -190,7 +211,7 @@ impl ConnectConfiguration {
         self.verify_hostname = verify_hostname;
     }
 
-    /// Returns an `Ssl` configured to connect to the provided domain.
+    /// Returns an [`Ssl`] configured to connect to the provided domain.
     ///
     /// The domain is used for SNI (if it is not an IP address) and hostname verification if enabled.
     pub fn into_ssl(mut self, domain: &str) -> Result<Ssl, ErrorStack> {
@@ -214,11 +235,33 @@ impl ConnectConfiguration {
     /// Initiates a client-side TLS session on a stream.
     ///
     /// The domain is used for SNI (if it is not an IP address) and hostname verification if enabled.
+    ///
+    /// This is a convenience method which combines [`Self::into_ssl`] and
+    /// [`Ssl::setup_connect`].
+    pub fn setup_connect<S>(
+        self,
+        domain: &str,
+        stream: S,
+    ) -> Result<MidHandshakeSslStream<S>, ErrorStack>
+    where
+        S: Read + Write,
+    {
+        Ok(self.into_ssl(domain)?.setup_connect(stream))
+    }
+
+    /// Attempts a client-side TLS session on a stream.
+    ///
+    /// The domain is used for SNI (if it is not an IP address) and hostname verification if enabled.
+    ///
+    /// This is a convenience method which combines [`Self::setup_connect`] and
+    /// [`MidHandshakeSslStream::handshake`].
     pub fn connect<S>(self, domain: &str, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
     where
         S: Read + Write,
     {
-        self.into_ssl(domain)?.connect(stream)
+        self.setup_connect(domain, stream)
+            .map_err(HandshakeError::SetupFailure)?
+            .handshake()
     }
 }
 
@@ -327,13 +370,29 @@ impl SslAcceptor {
         Ok(SslAcceptorBuilder(ctx))
     }
 
-    /// Initiates a server-side TLS session on a stream.
+    /// Initiates a server-side TLS handshake on a stream.
-    pub fn accept<S>(&self, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
+    ///
+    /// See [`Ssl::setup_accept`] for more details.
+    pub fn setup_accept<S>(&self, stream: S) -> Result<MidHandshakeSslStream<S>, ErrorStack>
     where
         S: Read + Write,
     {
         let ssl = Ssl::new(&self.0)?;
-        ssl.accept(stream)
+
+        Ok(ssl.setup_accept(stream))
+    }
+
+    /// Attempts a server-side TLS handshake on a stream.
+    ///
+    /// This is a convenience method which combines [`Self::setup_accept`] and
+    /// [`MidHandshakeSslStream::handshake`].
+    pub fn accept<S>(&self, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
+    where
+        S: Read + Write,
+    {
+        self.setup_accept(stream)
+            .map_err(HandshakeError::SetupFailure)?
+            .handshake()
     }
 
     /// Consumes the `SslAcceptor`, returning the inner raw `SslContext`.

boring/src/ssl/mod.rs

@@ -2320,10 +2320,11 @@ impl Ssl {
         }
     }
 
-    /// Creates a new `Ssl`.
+    /// Creates a new [`Ssl`].
     ///
-    /// This corresponds to [`SSL_new`].
+    /// This corresponds to [`SSL_new`](`ffi::SSL_new`).
-    /// This function does the same as [`Self:new()`] except that it takes &[SslContextRef].
+    ///
+    /// This function does the same as [`Self:new`] except that it takes &[SslContextRef].
     // Both functions exist for backward compatibility (no breaking API).
     pub fn new_from_ref(ctx: &SslContextRef) -> Result<Ssl, ErrorStack> {
         unsafe {
@@ -2337,34 +2338,52 @@ impl Ssl {
         }
     }
 
-    /// Initiates a client-side TLS handshake.
+    /// Initiates a client-side TLS handshake, returning a [`MidHandshakeSslStream`].
     ///
-    /// This corresponds to [`SSL_connect`].
+    /// This method is guaranteed to return without calling any callback defined
+    /// in the internal [`Ssl`] or [`SslContext`].
+    ///
+    /// See [`SslStreamBuilder::setup_connect`] for more details.
+    ///
+    /// # Warning
+    ///
+    /// BoringSSL's default configuration is insecure. It is highly recommended to use
+    /// [`SslConnector`] rather than [`Ssl`] directly, as it manages that configuration.
+    pub fn setup_connect<S>(self, stream: S) -> MidHandshakeSslStream<S>
+    where
+        S: Read + Write,
+    {
+        SslStreamBuilder::new(self, stream).setup_connect()
+    }
+
+    /// Attempts a client-side TLS handshake.
+    ///
+    /// This is a convenience method which combines [`Self::setup_connect`] and
+    /// [`MidHandshakeSslStream::handshake`].
     ///
     /// # Warning
     ///
     /// OpenSSL's default configuration is insecure. It is highly recommended to use
-    /// `SslConnector` rather than `Ssl` directly, as it manages that configuration.
+    /// [`SslConnector`] rather than `Ssl` directly, as it manages that configuration.
-    ///
-    /// [`SSL_connect`]: https://www.openssl.org/docs/manmaster/man3/SSL_connect.html
     pub fn connect<S>(self, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
     where
         S: Read + Write,
     {
-        SslStreamBuilder::new(self, stream).connect()
+        self.setup_connect(stream).handshake()
     }
 
     /// Initiates a server-side TLS handshake.
     ///
-    /// This corresponds to [`SSL_accept`].
+    /// This method is guaranteed to return without calling any callback defined
+    /// in the internal [`Ssl`] or [`SslContext`].
+    ///
+    /// See [`SslStreamBuilder::setup_accept`] for more details.
     ///
     /// # Warning
     ///
-    /// OpenSSL's default configuration is insecure. It is highly recommended to use
+    /// BoringSSL's default configuration is insecure. It is highly recommended to use
-    /// `SslAcceptor` rather than `Ssl` directly, as it manages that configuration.
+    /// [`SslAcceptor`] rather than [`Ssl`] directly, as it manages that configuration.
-    ///
+    pub fn setup_accept<S>(self, stream: S) -> MidHandshakeSslStream<S>
-    /// [`SSL_accept`]: https://www.openssl.org/docs/manmaster/man3/SSL_accept.html
-    pub fn accept<S>(self, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
     where
         S: Read + Write,
     {
@@ -2383,7 +2402,25 @@ impl Ssl {
             }
         }
 
-        SslStreamBuilder::new(self, stream).accept()
+        SslStreamBuilder::new(self, stream).setup_accept()
+    }
+
+    /// Attempts a server-side TLS handshake.
+    ///
+    /// This is a convenience method which combines [`Self::setup_accept`] and
+    /// [`MidHandshakeSslStream::handshake`].
+    ///
+    /// # Warning
+    ///
+    /// OpenSSL's default configuration is insecure. It is highly recommended to use
+    /// `SslAcceptor` rather than `Ssl` directly, as it manages that configuration.
+    ///
+    /// [`SSL_accept`]: https://www.openssl.org/docs/manmaster/man3/SSL_accept.html
+    pub fn accept<S>(self, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
+    where
+        S: Read + Write,
+    {
+        self.setup_accept(stream).handshake()
     }
 }
 
@@ -3594,56 +3631,68 @@ where
         unsafe { ffi::SSL_set_accept_state(self.inner.ssl.as_ptr()) }
     }
 
-    /// See `Ssl::connect`
+    /// Initiates a client-side TLS handshake, returning a [`MidHandshakeSslStream`].
-    pub fn connect(self) -> Result<SslStream<S>, HandshakeError<S>> {
+    ///
-        let mut stream = self.inner;
+    /// This method calls [`Self::set_connect_state`] and returns without actually
+    /// initiating the handshake. The caller is then free to call
+    /// [`MidHandshakeSslStream`] and loop on [`HandshakeError::WouldBlock`].
+    pub fn setup_connect(mut self) -> MidHandshakeSslStream<S> {
+        self.set_connect_state();
 
         #[cfg(feature = "kx-safe-default")]
-        stream.ssl.client_set_default_curves_list();
+        self.inner.ssl.client_set_default_curves_list();
 
-        let ret = unsafe { ffi::SSL_connect(stream.ssl.as_ptr()) };
+        MidHandshakeSslStream {
-        if ret > 0 {
+            stream: self.inner,
-            Ok(stream)
+            error: Error {
-        } else {
+                code: ErrorCode::WANT_WRITE,
-            let error = stream.make_error(ret);
+                cause: Some(InnerError::Io(io::Error::new(
-            match error.would_block() {
+                    io::ErrorKind::WouldBlock,
-                true => Err(HandshakeError::WouldBlock(MidHandshakeSslStream {
+                    "connect handshake has not started yet",
-                    stream,
+                ))),
-                    error,
+            },
-                })),
-                false => Err(HandshakeError::Failure(MidHandshakeSslStream {
-                    stream,
-                    error,
-                })),
-            }
         }
     }
 
-    /// See `Ssl::accept`
+    /// Attempts a client-side TLS handshake.
-    pub fn accept(self) -> Result<SslStream<S>, HandshakeError<S>> {
+    ///
-        let mut stream = self.inner;
+    /// This is a convenience method which combines [`Self::setup_connect`] and
+    /// [`MidHandshakeSslStream::handshake`].
+    pub fn connect(self) -> Result<SslStream<S>, HandshakeError<S>> {
+        self.setup_connect().handshake()
+    }
+
+    /// Initiates a server-side TLS handshake, returning a [`MidHandshakeSslStream`].
+    ///
+    /// This method calls [`Self::set_accept_state`] and returns without actually
+    /// initiating the handshake. The caller is then free to call
+    /// [`MidHandshakeSslStream`] and loop on [`HandshakeError::WouldBlock`].
+    pub fn setup_accept(mut self) -> MidHandshakeSslStream<S> {
+        self.set_accept_state();
 
         #[cfg(feature = "kx-safe-default")]
-        stream.ssl.server_set_default_curves_list();
+        self.inner.ssl.server_set_default_curves_list();
 
-        let ret = unsafe { ffi::SSL_accept(stream.ssl.as_ptr()) };
+        MidHandshakeSslStream {
-        if ret > 0 {
+            stream: self.inner,
-            Ok(stream)
+            error: Error {
-        } else {
+                code: ErrorCode::WANT_READ,
-            let error = stream.make_error(ret);
+                cause: Some(InnerError::Io(io::Error::new(
-            match error.would_block() {
+                    io::ErrorKind::WouldBlock,
-                true => Err(HandshakeError::WouldBlock(MidHandshakeSslStream {
+                    "accept handshake has not started yet",
-                    stream,
+                ))),
-                    error,
+            },
-                })),
-                false => Err(HandshakeError::Failure(MidHandshakeSslStream {
-                    stream,
-                    error,
-                })),
-            }
         }
     }
 
+    /// Attempts a server-side TLS handshake.
+    ///
+    /// This is a convenience method which combines [`Self::setup_accept`] and
+    /// [`MidHandshakeSslStream::handshake`].
+    pub fn accept(self) -> Result<SslStream<S>, HandshakeError<S>> {
+        self.setup_accept().handshake()
+    }
+
     /// Initiates the handshake.
     ///
     /// This will fail if `set_accept_state` or `set_connect_state` was not called first.

tokio-boring/src/lib.rs

@@ -13,6 +13,7 @@
 #![warn(missing_docs)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 
+use boring::error::ErrorStack;
 use boring::ssl::{
     self, ConnectConfiguration, ErrorCode, MidHandshakeSslStream, ShutdownResult, SslAcceptor,
     SslRef,
@@ -35,7 +36,7 @@ pub async fn connect<S>(
 where
     S: AsyncRead + AsyncWrite + Unpin,
 {
-    handshake(|s| config.connect(domain, s), stream).await
+    handshake(|s| config.setup_connect(domain, s), stream).await
 }
 
 /// Asynchronously performs a server-side TLS handshake over the provided stream.
@@ -43,24 +44,22 @@ pub async fn accept<S>(acceptor: &SslAcceptor, stream: S) -> Result<SslStream<S>
 where
     S: AsyncRead + AsyncWrite + Unpin,
 {
-    handshake(|s| acceptor.accept(s), stream).await
+    handshake(|s| acceptor.setup_accept(s), stream).await
 }
 
-async fn handshake<F, S>(f: F, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
+async fn handshake<S>(
+    f: impl FnOnce(StreamWrapper<S>) -> Result<MidHandshakeSslStream<StreamWrapper<S>>, ErrorStack>,
+    stream: S,
+) -> Result<SslStream<S>, HandshakeError<S>>
 where
-    F: FnOnce(
-            StreamWrapper<S>,
-        )
-            -> Result<ssl::SslStream<StreamWrapper<S>>, ssl::HandshakeError<StreamWrapper<S>>>
-        + Unpin,
     S: AsyncRead + AsyncWrite + Unpin,
 {
-    let start = StartHandshakeFuture(Some(StartHandshakeFutureInner { f, stream }));
+    let ongoing_handshake = Some(
+        f(StreamWrapper { stream, context: 0 })
+            .map_err(|err| HandshakeError(ssl::HandshakeError::SetupFailure(err)))?,
+    );
 
-    match start.await? {
+    HandshakeFuture(ongoing_handshake).await
-        StartedHandshake::Done(s) => Ok(s),
-        StartedHandshake::Mid(s) => HandshakeFuture(Some(s)).await,
-    }
 }
 
 struct StreamWrapper<S> {
@@ -334,53 +333,6 @@ where
     }
 }
 
-enum StartedHandshake<S> {
-    Done(SslStream<S>),
-    Mid(MidHandshakeSslStream<StreamWrapper<S>>),
-}
-
-struct StartHandshakeFuture<F, S>(Option<StartHandshakeFutureInner<F, S>>);
-
-struct StartHandshakeFutureInner<F, S> {
-    f: F,
-    stream: S,
-}
-
-impl<F, S> Future for StartHandshakeFuture<F, S>
-where
-    F: FnOnce(
-            StreamWrapper<S>,
-        )
-            -> Result<ssl::SslStream<StreamWrapper<S>>, ssl::HandshakeError<StreamWrapper<S>>>
-        + Unpin,
-    S: Unpin,
-{
-    type Output = Result<StartedHandshake<S>, HandshakeError<S>>;
-
-    fn poll(
-        mut self: Pin<&mut Self>,
-        ctx: &mut Context<'_>,
-    ) -> Poll<Result<StartedHandshake<S>, HandshakeError<S>>> {
-        let inner = self.0.take().expect("future polled after completion");
-
-        let stream = StreamWrapper {
-            stream: inner.stream,
-            context: ctx as *mut _ as usize,
-        };
-        match (inner.f)(stream) {
-            Ok(mut s) => {
-                s.get_mut().context = 0;
-                Poll::Ready(Ok(StartedHandshake::Done(SslStream(s))))
-            }
-            Err(ssl::HandshakeError::WouldBlock(mut s)) => {
-                s.get_mut().context = 0;
-                Poll::Ready(Ok(StartedHandshake::Mid(s)))
-            }
-            Err(e) => Poll::Ready(Err(HandshakeError(e))),
-        }
-    }
-}
-
 struct HandshakeFuture<S>(Option<MidHandshakeSslStream<StreamWrapper<S>>>);
 
 impl<S> Future for HandshakeFuture<S>
@@ -389,21 +341,22 @@ where
 {
     type Output = Result<SslStream<S>, HandshakeError<S>>;
 
-    fn poll(
+    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-        mut self: Pin<&mut Self>,
-        ctx: &mut Context<'_>,
-    ) -> Poll<Result<SslStream<S>, HandshakeError<S>>> {
         let mut s = self.0.take().expect("future polled after completion");
 
-        s.get_mut().context = ctx as *mut _ as usize;
+        s.get_mut().context = cx as *mut _ as usize;
+
         match s.handshake() {
             Ok(mut s) => {
                 s.get_mut().context = 0;
+
                 Poll::Ready(Ok(SslStream(s)))
             }
             Err(ssl::HandshakeError::WouldBlock(mut s)) => {
                 s.get_mut().context = 0;
+
                 self.0 = Some(s);
+
                 Poll::Pending
             }
             Err(e) => Poll::Ready(Err(HandshakeError(e))),