Remove all connect and accept functions

Those functions were blurring the line between setup failures
(which are caused by the developer misusing the API) and actual
failures encountered on the stream while trying to achieve the
TLS handshake, especially on SslConnector and SslAcceptor.

Removing them allows for the removal of HandshakeError, as
the HandshakeError::SetupFailure variant becomes useless,
and there is no real need to distinguish in that error type
between Failure and WouldBlock when we can just check
the error stored in MidHandshakeSslStream.

This then allow us to simplify tokio_boring's own entry points,
also making them distinguish between setup failures and failures
on the stream.

Author: nox

boring/src/ssl/connector.rs

@@ -4,8 +4,8 @@ use std::ops::{Deref, DerefMut};
 use crate::dh::Dh;
 use crate::error::ErrorStack;
 use crate::ssl::{
-    HandshakeError, Ssl, SslContext, SslContextBuilder, SslContextRef, SslMethod, SslMode,
-    SslOptions, SslRef, SslStream, SslVerifyMode,
+    Ssl, SslContext, SslContextBuilder, SslContextRef, SslMethod, SslMode, SslOptions, SslRef,
+    SslVerifyMode,
 };
 use crate::version;
 
@@ -111,21 +111,6 @@ impl SslConnector {
         self.configure()?.setup_connect(domain, stream)
     }
 
-    /// Attempts a client-side TLS session on a stream.
-    ///
-    /// The domain is used for SNI and hostname verification.
-    ///
-    /// 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.setup_connect(domain, stream)
-            .map_err(HandshakeError::SetupFailure)?
-            .handshake()
-    }
-
     /// Returns a structure allowing for configuration of a single TLS session before connection.
     pub fn configure(&self) -> Result<ConnectConfiguration, ErrorStack> {
         Ssl::new(&self.0).map(|ssl| ConnectConfiguration {
@@ -239,21 +224,6 @@ impl ConnectConfiguration {
 
         Ok(self.ssl.setup_connect(stream))
     }
-
-    /// Attempts a client-side TLS session on a stream.
-    ///
-    /// The domain is used for SNI 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.setup_connect(domain, stream)
-            .map_err(HandshakeError::SetupFailure)?
-            .handshake()
-    }
 }
 
 impl Deref for ConnectConfiguration {
@@ -373,19 +343,6 @@ impl SslAcceptor {
         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`.
     pub fn into_context(self) -> SslContext {
         self.0

boring/src/ssl/error.rs

@@ -1,13 +1,10 @@
 use crate::ffi;
 use libc::c_int;
 use std::error;
-use std::error::Error as StdError;
 use std::fmt;
 use std::io;
 
 use crate::error::ErrorStack;
-use crate::ssl::MidHandshakeSslStream;
-use crate::x509::X509VerifyResult;
 
 /// An error code returned from SSL functions.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -130,65 +127,3 @@ impl error::Error for Error {
         }
     }
 }
-
-/// An error or intermediate state after a TLS handshake attempt.
-// FIXME overhaul
-#[derive(Debug)]
-pub enum HandshakeError<S> {
-    /// Setup failed.
-    SetupFailure(ErrorStack),
-    /// The handshake failed.
-    Failure(MidHandshakeSslStream<S>),
-    /// The handshake encountered a `WouldBlock` error midway through.
-    ///
-    /// This error will never be returned for blocking streams.
-    WouldBlock(MidHandshakeSslStream<S>),
-}
-
-impl<S: fmt::Debug> StdError for HandshakeError<S> {
-    fn source(&self) -> Option<&(dyn StdError + 'static)> {
-        match *self {
-            HandshakeError::SetupFailure(ref e) => Some(e),
-            HandshakeError::Failure(ref s) | HandshakeError::WouldBlock(ref s) => Some(s.error()),
-        }
-    }
-}
-
-impl<S> fmt::Display for HandshakeError<S> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            HandshakeError::SetupFailure(ref e) => {
-                write!(f, "TLS stream setup failed {}", e)
-            }
-            HandshakeError::Failure(ref s) => fmt_mid_handshake_error(s, f, "TLS handshake failed"),
-            HandshakeError::WouldBlock(ref s) => {
-                fmt_mid_handshake_error(s, f, "TLS handshake interrupted")
-            }
-        }
-    }
-}
-
-fn fmt_mid_handshake_error(
-    s: &MidHandshakeSslStream<impl Sized>,
-    f: &mut fmt::Formatter,
-    prefix: &str,
-) -> fmt::Result {
-    #[cfg(feature = "rpk")]
-    if s.ssl().ssl_context().is_rpk() {
-        write!(f, "{}", prefix)?;
-        return write!(f, " {}", s.error());
-    }
-
-    match s.ssl().verify_result() {
-        X509VerifyResult::OK => write!(f, "{}", prefix)?,
-        verify => write!(f, "{}: cert verification failed - {}", prefix, verify)?,
-    }
-
-    write!(f, " {}", s.error())
-}
-
-impl<S> From<ErrorStack> for HandshakeError<S> {
-    fn from(e: ErrorStack) -> HandshakeError<S> {
-        HandshakeError::SetupFailure(e)
-    }
-}

boring/src/ssl/mod.rs

@@ -15,7 +15,11 @@
 //! let connector = SslConnector::builder(SslMethod::tls()).unwrap().build();
 //!
 //! let stream = TcpStream::connect("google.com:443").unwrap();
-//! let mut stream = connector.connect("google.com", stream).unwrap();
+//! let mut stream = connector
+//!     .setup_connect("google.com", stream)
+//!     .unwrap()
+//!     .handshake()
+//!     .unwrap();
 //!
 //! stream.write_all(b"GET / HTTP/1.0\r\n\r\n").unwrap();
 //! let mut res = vec![];
@@ -49,7 +53,12 @@
 //!         Ok(stream) => {
 //!             let acceptor = acceptor.clone();
 //!             thread::spawn(move || {
-//!                 let stream = acceptor.accept(stream).unwrap();
+//!                 let stream = acceptor
+//!                     .setup_accept(stream)
+//!                     .unwrap()
+//!                     .handshake()
+//!                     .unwrap();
+//!
 //!                 handle_client(stream);
 //!             });
 //!         }
@@ -98,7 +107,7 @@ use crate::{cvt, cvt_0i, cvt_n, cvt_p, init};
 pub use crate::ssl::connector::{
     ConnectConfiguration, SslAcceptor, SslAcceptorBuilder, SslConnector, SslConnectorBuilder,
 };
-pub use crate::ssl::error::{Error, ErrorCode, HandshakeError};
+pub use crate::ssl::error::{Error, ErrorCode};
 
 mod bio;
 mod callbacks;
@@ -2324,22 +2333,6 @@ impl Ssl {
         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.
-    pub fn connect<S>(self, stream: S) -> Result<SslStream<S>, HandshakeError<S>>
-    where
-        S: Read + Write,
-    {
-        self.setup_connect(stream).handshake()
-    }
-
     /// Initiates a server-side TLS handshake.
     ///
     /// This method is guaranteed to return without calling any callback defined
@@ -2372,24 +2365,6 @@ impl Ssl {
 
         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()
-    }
 }
 
 impl fmt::Debug for SslRef {
@@ -3192,16 +3167,14 @@ impl<S> MidHandshakeSslStream<S> {
     /// This corresponds to [`SSL_do_handshake`].
     ///
     /// [`SSL_do_handshake`]: https://www.openssl.org/docs/manmaster/man3/SSL_do_handshake.html
-    pub fn handshake(mut self) -> Result<SslStream<S>, HandshakeError<S>> {
+    pub fn handshake(mut self) -> Result<SslStream<S>, MidHandshakeSslStream<S>> {
         let ret = unsafe { ffi::SSL_do_handshake(self.stream.ssl.as_ptr()) };
         if ret > 0 {
             Ok(self.stream)
         } else {
             self.error = self.stream.make_error(ret);
-            match self.error.would_block() {
-                true => Err(HandshakeError::WouldBlock(self)),
-                false => Err(HandshakeError::Failure(self)),
-            }
+
+            Err(self)
         }
     }
 }
@@ -3484,7 +3457,7 @@ where
     /// This corresponds to [`SSL_set_connect_state`].
     ///
     /// [`SSL_set_connect_state`]: https://www.openssl.org/docs/manmaster/man3/SSL_set_connect_state.html
-    pub fn set_connect_state(&mut self) {
+    fn set_connect_state(&mut self) {
         unsafe { ffi::SSL_set_connect_state(self.inner.ssl.as_ptr()) }
     }
 
@@ -3493,15 +3466,14 @@ where
     /// This corresponds to [`SSL_set_accept_state`].
     ///
     /// [`SSL_set_accept_state`]: https://www.openssl.org/docs/manmaster/man3/SSL_set_accept_state.html
-    pub fn set_accept_state(&mut self) {
+    fn set_accept_state(&mut self) {
         unsafe { ffi::SSL_set_accept_state(self.inner.ssl.as_ptr()) }
     }
 
     /// Initiates a client-side TLS handshake, returning a [`MidHandshakeSslStream`].
     ///
-    /// 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`].
+    /// The caller is then free to call [`MidHandshakeSslStream::handshake`] and retry
+    /// on blocking errors.
     pub fn setup_connect(mut self) -> MidHandshakeSslStream<S> {
         self.set_connect_state();
 
@@ -3517,19 +3489,10 @@ where
         }
     }
 
-    /// Attempts a client-side TLS handshake.
-    ///
-    /// 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`].
+    /// The caller is then free to call [`MidHandshakeSslStream::handshake`] and retry
+    /// on blocking errors.
     pub fn setup_accept(mut self) -> MidHandshakeSslStream<S> {
         self.set_accept_state();
 
@@ -3544,41 +3507,6 @@ where
             },
         }
     }
-
-    /// 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.
-    ///
-    /// This corresponds to [`SSL_do_handshake`].
-    ///
-    /// [`SSL_do_handshake`]: https://www.openssl.org/docs/manmaster/man3/SSL_do_handshake.html
-    pub fn handshake(self) -> Result<SslStream<S>, HandshakeError<S>> {
-        let mut stream = self.inner;
-        let ret = unsafe { ffi::SSL_do_handshake(stream.ssl.as_ptr()) };
-        if ret > 0 {
-            Ok(stream)
-        } else {
-            let error = stream.make_error(ret);
-            match error.would_block() {
-                true => Err(HandshakeError::WouldBlock(MidHandshakeSslStream {
-                    stream,
-                    error,
-                })),
-                false => Err(HandshakeError::Failure(MidHandshakeSslStream {
-                    stream,
-                    error,
-                })),
-            }
-        }
-    }
 }
 
 impl<S> SslStreamBuilder<S> {