Merge pull request #769 from sfackler/want-read

Handle local retries
This commit is contained in:
Steven Fackler 2017-11-04 15:50:35 -07:00 committed by GitHub
commit 8e0b69aa73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 100 additions and 94 deletions

View File

@ -66,6 +66,33 @@ impl From<ErrorStack> for Error {
} }
} }
/// An error indicating that the operation can be immediately retried.
///
/// OpenSSL's [`SSL_read`] and [`SSL_write`] functions can return `SSL_ERROR_WANT_READ` even when
/// the underlying socket is performing blocking IO in certain cases. When this happens, the
/// the operation can be immediately retried.
///
/// To signal this event, the `io::Error` inside of [`Error::WantRead`] will be constructed around
/// a `RetryError`.
///
/// [`SSL_read`]: https://www.openssl.org/docs/manmaster/man3/SSL_read.html
/// [`SSL_write`]: https://www.openssl.org/docs/manmaster/man3/SSL_write.html
/// [`Error::WantRead`]: enum.Error.html#variant.WantRead
#[derive(Debug)]
pub struct RetryError;
impl fmt::Display for RetryError {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
fmt.write_str(error::Error::description(self))
}
}
impl error::Error for RetryError {
fn description(&self) -> &str {
"operation must be retried"
}
}
/// An error or intermediate state after a TLS handshake attempt. /// An error or intermediate state after a TLS handshake attempt.
#[derive(Debug)] #[derive(Debug)]
pub enum HandshakeError<S> { pub enum HandshakeError<S> {

View File

@ -72,7 +72,7 @@
//! ``` //! ```
use ffi; use ffi;
use foreign_types::{ForeignType, ForeignTypeRef, Opaque}; use foreign_types::{ForeignType, ForeignTypeRef, Opaque};
use libc::{c_int, c_void, c_long, c_ulong}; use libc::{c_int, c_long, c_ulong, c_void};
use libc::{c_uchar, c_uint}; use libc::{c_uchar, c_uint};
use std::any::Any; use std::any::Any;
use std::any::TypeId; use std::any::TypeId;
@ -93,12 +93,12 @@ use std::slice;
use std::str; use std::str;
use std::sync::Mutex; use std::sync::Mutex;
use {init, cvt, cvt_p, cvt_n}; use {cvt, cvt_n, cvt_p, init};
use dh::{Dh, DhRef}; use dh::{Dh, DhRef};
use ec::EcKeyRef; use ec::EcKeyRef;
#[cfg(any(all(feature = "v101", ossl101), all(feature = "v102", ossl102)))] #[cfg(any(all(feature = "v101", ossl101), all(feature = "v102", ossl102)))]
use ec::EcKey; use ec::EcKey;
use x509::{X509StoreContextRef, X509FileType, X509, X509Ref, X509VerifyError, X509Name}; use x509::{X509, X509FileType, X509Name, X509Ref, X509StoreContextRef, X509VerifyError};
use x509::store::{X509StoreBuilderRef, X509StoreRef}; use x509::store::{X509StoreBuilderRef, X509StoreRef};
#[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))] #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]
use x509::store::X509Store; use x509::store::X509Store;
@ -111,9 +111,9 @@ use stack::{Stack, StackRef};
use ssl::bio::BioMethod; use ssl::bio::BioMethod;
use ssl::callbacks::*; use ssl::callbacks::*;
pub use ssl::connector::{SslConnectorBuilder, SslConnector, SslAcceptorBuilder, SslAcceptor, pub use ssl::connector::{ConnectConfiguration, SslAcceptor, SslAcceptorBuilder, SslConnector,
ConnectConfiguration}; SslConnectorBuilder};
pub use ssl::error::{Error, HandshakeError}; pub use ssl::error::{Error, HandshakeError, RetryError};
mod error; mod error;
mod callbacks; mod callbacks;
@ -416,10 +416,8 @@ impl SslContextBuilder {
pub fn set_verify_cert_store(&mut self, cert_store: X509Store) -> Result<(), ErrorStack> { pub fn set_verify_cert_store(&mut self, cert_store: X509Store) -> Result<(), ErrorStack> {
unsafe { unsafe {
let ptr = cert_store.as_ptr(); let ptr = cert_store.as_ptr();
cvt( cvt(ffi::SSL_CTX_set0_verify_cert_store(self.as_ptr(), ptr)
ffi::SSL_CTX_set0_verify_cert_store(self.as_ptr(), ptr) as as c_int)?;
c_int,
)?;
mem::forget(cert_store); mem::forget(cert_store);
Ok(()) Ok(())
@ -461,8 +459,9 @@ impl SslContextBuilder {
pub fn set_tmp_ecdh(&mut self, key: &EcKeyRef) -> Result<(), ErrorStack> { pub fn set_tmp_ecdh(&mut self, key: &EcKeyRef) -> Result<(), ErrorStack> {
unsafe { unsafe {
cvt(ffi::SSL_CTX_set_tmp_ecdh(self.as_ptr(), key.as_ptr()) as cvt(ffi::SSL_CTX_set_tmp_ecdh(self.as_ptr(), key.as_ptr())
c_int).map(|_| ()) as c_int)
.map(|_| ())
} }
} }
@ -579,10 +578,7 @@ impl SslContextBuilder {
/// `set_certificate` to a trusted root. /// `set_certificate` to a trusted root.
pub fn add_extra_chain_cert(&mut self, cert: X509) -> Result<(), ErrorStack> { pub fn add_extra_chain_cert(&mut self, cert: X509) -> Result<(), ErrorStack> {
unsafe { unsafe {
cvt(ffi::SSL_CTX_add_extra_chain_cert( cvt(ffi::SSL_CTX_add_extra_chain_cert(self.as_ptr(), cert.as_ptr()) as c_int)?;
self.as_ptr(),
cert.as_ptr(),
) as c_int)?;
mem::forget(cert); mem::forget(cert);
Ok(()) Ok(())
} }
@ -766,10 +762,9 @@ impl SslContextBuilder {
Box::into_raw(callback) as *mut c_void, Box::into_raw(callback) as *mut c_void,
); );
let f: unsafe extern "C" fn(_, _) -> _ = raw_tlsext_status::<F>; let f: unsafe extern "C" fn(_, _) -> _ = raw_tlsext_status::<F>;
cvt(ffi::SSL_CTX_set_tlsext_status_cb( cvt(ffi::SSL_CTX_set_tlsext_status_cb(self.as_ptr(), Some(f))
self.as_ptr(), as c_int)
Some(f), .map(|_| ())
) as c_int).map(|_| ())
} }
} }
@ -781,7 +776,8 @@ impl SslContextBuilder {
#[cfg(not(osslconf = "OPENSSL_NO_PSK"))] #[cfg(not(osslconf = "OPENSSL_NO_PSK"))]
pub fn set_psk_callback<F>(&mut self, callback: F) pub fn set_psk_callback<F>(&mut self, callback: F)
where where
F: Fn(&mut SslRef, Option<&[u8]>, &mut [u8], &mut [u8]) -> Result<usize, ErrorStack> F: Fn(&mut SslRef, Option<&[u8]>, &mut [u8], &mut [u8])
-> Result<usize, ErrorStack>
+ Any + Any
+ 'static + 'static
+ Sync + Sync
@ -1239,10 +1235,8 @@ impl SslRef {
pub fn set_hostname(&mut self, hostname: &str) -> Result<(), ErrorStack> { pub fn set_hostname(&mut self, hostname: &str) -> Result<(), ErrorStack> {
let cstr = CString::new(hostname).unwrap(); let cstr = CString::new(hostname).unwrap();
unsafe { unsafe {
cvt(ffi::SSL_set_tlsext_host_name( cvt(ffi::SSL_set_tlsext_host_name(self.as_ptr(), cstr.as_ptr() as *mut _) as c_int)
self.as_ptr(), .map(|_| ())
cstr.as_ptr() as *mut _,
) as c_int).map(|_| ())
} }
} }
@ -1373,9 +1367,7 @@ impl SslRef {
return None; return None;
} }
let meth = ffi::SSL_COMP_get_name(ptr); let meth = ffi::SSL_COMP_get_name(ptr);
Some( Some(str::from_utf8(CStr::from_ptr(meth as *const _).to_bytes()).unwrap())
str::from_utf8(CStr::from_ptr(meth as *const _).to_bytes()).unwrap(),
)
} }
} }
@ -1392,9 +1384,7 @@ impl SslRef {
return None; return None;
} }
Some( Some(str::from_utf8(CStr::from_ptr(name as *const _).to_bytes()).unwrap())
str::from_utf8(CStr::from_ptr(name as *const _).to_bytes()).unwrap(),
)
} }
} }
@ -1459,10 +1449,7 @@ impl SslRef {
/// Sets the status response a client wishes the server to reply with. /// Sets the status response a client wishes the server to reply with.
pub fn set_status_type(&mut self, type_: StatusType) -> Result<(), ErrorStack> { pub fn set_status_type(&mut self, type_: StatusType) -> Result<(), ErrorStack> {
unsafe { unsafe {
cvt(ffi::SSL_set_tlsext_status_type( cvt(ffi::SSL_set_tlsext_status_type(self.as_ptr(), type_.as_raw()) as c_int).map(|_| ())
self.as_ptr(),
type_.as_raw(),
) as c_int).map(|_| ())
} }
} }
@ -1494,7 +1481,8 @@ impl SslRef {
self.as_ptr(), self.as_ptr(),
p as *mut c_uchar, p as *mut c_uchar,
response.len() as c_long, response.len() as c_long,
) as c_int).map(|_| ()) ) as c_int)
.map(|_| ())
} }
} }
@ -1561,19 +1549,16 @@ impl Ssl {
Ok(stream) Ok(stream)
} else { } else {
match stream.make_error(ret) { match stream.make_error(ret) {
e @ Error::WantWrite(_) | e @ Error::WantWrite(_) | e @ Error::WantRead(_) => {
e @ Error::WantRead(_) => {
Err(HandshakeError::Interrupted(MidHandshakeSslStream { Err(HandshakeError::Interrupted(MidHandshakeSslStream {
stream: stream, stream: stream,
error: e, error: e,
})) }))
} }
err => { err => Err(HandshakeError::Failure(MidHandshakeSslStream {
Err(HandshakeError::Failure(MidHandshakeSslStream {
stream: stream, stream: stream,
error: err, error: err,
})) })),
}
} }
} }
} }
@ -1594,19 +1579,16 @@ impl Ssl {
Ok(stream) Ok(stream)
} else { } else {
match stream.make_error(ret) { match stream.make_error(ret) {
e @ Error::WantWrite(_) | e @ Error::WantWrite(_) | e @ Error::WantRead(_) => {
e @ Error::WantRead(_) => {
Err(HandshakeError::Interrupted(MidHandshakeSslStream { Err(HandshakeError::Interrupted(MidHandshakeSslStream {
stream: stream, stream: stream,
error: e, error: e,
})) }))
} }
err => { err => Err(HandshakeError::Failure(MidHandshakeSslStream {
Err(HandshakeError::Failure(MidHandshakeSslStream {
stream: stream, stream: stream,
error: err, error: err,
})) })),
}
} }
} }
} }
@ -1652,8 +1634,7 @@ impl<S> MidHandshakeSslStream<S> {
Ok(self.stream) Ok(self.stream)
} else { } else {
match self.stream.make_error(ret) { match self.stream.make_error(ret) {
e @ Error::WantWrite(_) | e @ Error::WantWrite(_) | e @ Error::WantRead(_) => {
e @ Error::WantRead(_) => {
self.error = e; self.error = e;
Err(HandshakeError::Interrupted(self)) Err(HandshakeError::Interrupted(self))
} }
@ -1775,12 +1756,10 @@ impl<S> SslStream<S> {
if errs.errors().is_empty() { if errs.errors().is_empty() {
match self.get_bio_error() { match self.get_bio_error() {
Some(err) => Error::Stream(err), Some(err) => Error::Stream(err),
None => { None => Error::Stream(io::Error::new(
Error::Stream(io::Error::new(
io::ErrorKind::ConnectionAborted, io::ErrorKind::ConnectionAborted,
"unexpected EOF observed", "unexpected EOF observed",
)) )),
}
} }
} else { } else {
Error::Ssl(errs) Error::Ssl(errs)
@ -1790,33 +1769,24 @@ impl<S> SslStream<S> {
ffi::SSL_ERROR_WANT_WRITE => { ffi::SSL_ERROR_WANT_WRITE => {
let err = match self.get_bio_error() { let err = match self.get_bio_error() {
Some(err) => err, Some(err) => err,
None => { None => io::Error::new(
io::Error::new(
io::ErrorKind::Other, io::ErrorKind::Other,
"BUG: got an SSL_ERROR_WANT_WRITE with no error in the BIO", "BUG: got an SSL_ERROR_WANT_WRITE with no error in the BIO",
) ),
}
}; };
Error::WantWrite(err) Error::WantWrite(err)
} }
ffi::SSL_ERROR_WANT_READ => { ffi::SSL_ERROR_WANT_READ => {
let err = match self.get_bio_error() { let err = match self.get_bio_error() {
Some(err) => err, Some(err) => err,
None => { None => io::Error::new(io::ErrorKind::Other, RetryError),
io::Error::new(
io::ErrorKind::Other,
"BUG: got an SSL_ERROR_WANT_READ with no error in the BIO",
)
}
}; };
Error::WantRead(err) Error::WantRead(err)
} }
err => { err => Error::Stream(io::Error::new(
Error::Stream(io::Error::new(
io::ErrorKind::InvalidData, io::ErrorKind::InvalidData,
format!("unexpected error {}", err), format!("unexpected error {}", err),
)) )),
}
} }
} }
@ -1859,25 +1829,34 @@ impl<S> SslStream<S> {
impl<S: Read + Write> Read for SslStream<S> { impl<S: Read + Write> Read for SslStream<S> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
loop {
match self.ssl_read(buf) { match self.ssl_read(buf) {
Ok(n) => Ok(n), Ok(n) => return Ok(n),
Err(Error::ZeroReturn) => Ok(0), Err(Error::ZeroReturn) => return Ok(0),
Err(Error::Stream(e)) => Err(e), Err(Error::WantRead(ref e))
Err(Error::WantRead(e)) => Err(e), if e.get_ref().map_or(false, |e| e.is::<RetryError>()) => {}
Err(Error::WantWrite(e)) => Err(e), Err(Error::Stream(e)) | Err(Error::WantRead(e)) | Err(Error::WantWrite(e)) => {
Err(e) => Err(io::Error::new(io::ErrorKind::Other, e)), return Err(e);
}
Err(e) => return Err(io::Error::new(io::ErrorKind::Other, e)),
}
} }
} }
} }
impl<S: Read + Write> Write for SslStream<S> { impl<S: Read + Write> Write for SslStream<S> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> { fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
self.ssl_write(buf).map_err(|e| match e { loop {
Error::Stream(e) => e, match self.ssl_write(buf) {
Error::WantRead(e) => e, Ok(n) => return Ok(n),
Error::WantWrite(e) => e, Err(Error::WantRead(ref e))
e => io::Error::new(io::ErrorKind::Other, e), if e.get_ref().map_or(false, |e| e.is::<RetryError>()) => {}
}) Err(Error::Stream(e)) | Err(Error::WantRead(e)) | Err(Error::WantWrite(e)) => {
return Err(e);
}
Err(e) => return Err(io::Error::new(io::ErrorKind::Other, e)),
}
}
} }
fn flush(&mut self) -> io::Result<()> { fn flush(&mut self) -> io::Result<()> {
@ -1902,8 +1881,8 @@ mod compat {
use ffi; use ffi;
use libc::c_int; use libc::c_int;
pub use ffi::{SSL_CTX_get_options, SSL_CTX_set_options, SSL_CTX_clear_options, SSL_CTX_up_ref, pub use ffi::{SSL_CTX_clear_options, SSL_CTX_get_options, SSL_CTX_set_options, SSL_CTX_up_ref,
SSL_SESSION_get_master_key, SSL_is_server, SSL_SESSION_up_ref}; SSL_SESSION_get_master_key, SSL_SESSION_up_ref, SSL_is_server};
pub unsafe fn get_new_idx(f: ffi::CRYPTO_EX_free) -> c_int { pub unsafe fn get_new_idx(f: ffi::CRYPTO_EX_free) -> c_int {
ffi::CRYPTO_get_ex_new_index( ffi::CRYPTO_get_ex_new_index(
@ -1942,7 +1921,7 @@ mod compat {
use std::ptr; use std::ptr;
use ffi; use ffi;
use libc::{self, c_long, c_ulong, c_int, size_t, c_uchar}; use libc::{self, c_int, c_long, c_uchar, c_ulong, size_t};
pub unsafe fn SSL_CTX_get_options(ctx: *const ffi::SSL_CTX) -> c_ulong { pub unsafe fn SSL_CTX_get_options(ctx: *const ffi::SSL_CTX) -> c_ulong {
ffi::SSL_CTX_ctrl(ctx as *mut _, ffi::SSL_CTRL_OPTIONS, 0, ptr::null_mut()) as c_ulong ffi::SSL_CTX_ctrl(ctx as *mut _, ffi::SSL_CTRL_OPTIONS, 0, ptr::null_mut()) as c_ulong