diff --git a/boring/src/ssl/async_callbacks.rs b/boring/src/ssl/async_callbacks.rs index f584731b..db674e9a 100644 --- a/boring/src/ssl/async_callbacks.rs +++ b/boring/src/ssl/async_callbacks.rs @@ -1,7 +1,8 @@ use super::mut_only::MutOnly; use super::{ ClientHello, GetSessionPendingError, PrivateKeyMethod, PrivateKeyMethodError, SelectCertError, - Ssl, SslContextBuilder, SslRef, SslSession, SslSignatureAlgorithm, + Ssl, SslAlert, SslContextBuilder, SslRef, SslSession, SslSignatureAlgorithm, SslVerifyError, + SslVerifyMode, }; use crate::ex_data::Index; use once_cell::sync::Lazy; @@ -30,6 +31,12 @@ pub type BoxGetSessionFuture = ExDataFuture>; /// The type of callbacks returned by [`BoxSelectCertFuture`] methods. pub type BoxGetSessionFinish = Box Option>; +/// The type of futures to pass to [`SslContextBuilderExt::set_async_custom_verify_callback`]. +pub type BoxCustomVerifyFuture = ExDataFuture>; + +/// The type of callbacks returned by [`BoxCustomVerifyFuture`] methods. +pub type BoxCustomVerifyFinish = Box Result<(), SslAlert>>; + /// Convenience alias for futures stored in [`Ssl`] ex data by [`SslContextBuilderExt`] methods. /// /// Public for documentation purposes. @@ -45,6 +52,9 @@ pub(crate) static SELECT_PRIVATE_KEY_METHOD_FUTURE_INDEX: Lazy< pub(crate) static SELECT_GET_SESSION_FUTURE_INDEX: Lazy< Index>>, > = Lazy::new(|| Ssl::new_ex_index().unwrap()); +pub(crate) static SELECT_CUSTOM_VERIFY_FUTURE_INDEX: Lazy< + Index>>, +> = Lazy::new(|| Ssl::new_ex_index().unwrap()); impl SslContextBuilder { /// Sets a callback that is called before most [`ClientHello`] processing @@ -135,15 +145,68 @@ impl SslContextBuilder { self.set_get_session_callback(async_callback) } + + /// Configures certificate verification. + /// + /// The callback should return `Ok(())` if the certificate is valid. + /// If the certificate is invalid, the callback should return `SslVerifyError::Invalid(alert)`. + /// Some useful alerts include [`SslAlert::CERTIFICATE_EXPIRED`], [`SslAlert::CERTIFICATE_REVOKED`], + /// [`SslAlert::UNKNOWN_CA`], [`SslAlert::BAD_CERTIFICATE`], [`SslAlert::CERTIFICATE_UNKNOWN`], + /// and [`SslAlert::INTERNAL_ERROR`]. See RFC 5246 section 7.2.2 for their precise meanings. + /// + /// A task waker must be set on `Ssl` values associated with the resulting + /// `SslContext` with [`SslRef::set_task_waker`]. + /// + /// See [`SslContextBuilder::set_custom_verify_callback`] for the sync version of this method. + /// + /// # Panics + /// + /// This method panics if this `Ssl` is associated with a RPK context. + pub fn set_async_custom_verify_callback(&mut self, mode: SslVerifyMode, callback: F) + where + F: Fn(&mut SslRef) -> Result + Send + Sync + 'static, + { + self.set_custom_verify_callback(mode, async_custom_verify_callback(callback)) + } } impl SslRef { + pub fn set_async_custom_verify_callback(&mut self, mode: SslVerifyMode, callback: F) + where + F: Fn(&mut SslRef) -> Result + Send + Sync + 'static, + { + self.set_custom_verify_callback(mode, async_custom_verify_callback(callback)) + } + /// Sets the task waker to be used in async callbacks installed on this `Ssl`. pub fn set_task_waker(&mut self, waker: Option) { self.replace_ex_data(*TASK_WAKER_INDEX, waker); } } +fn async_custom_verify_callback( + callback: F, +) -> impl Fn(&mut SslRef) -> Result<(), SslVerifyError> +where + F: Fn(&mut SslRef) -> Result + Send + Sync + 'static, +{ + move |ssl| { + let fut_poll_result = with_ex_data_future( + &mut *ssl, + *SELECT_CUSTOM_VERIFY_FUTURE_INDEX, + |ssl| ssl, + &callback, + identity, + ); + + match fut_poll_result { + Poll::Ready(Err(alert)) => Err(SslVerifyError::Invalid(alert)), + Poll::Ready(Ok(finish)) => Ok(finish(ssl).map_err(SslVerifyError::Invalid)?), + Poll::Pending => Err(SslVerifyError::Retry), + } + } +} + /// A fatal error to be returned from async select certificate callbacks. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct AsyncSelectCertError; diff --git a/boring/src/ssl/callbacks.rs b/boring/src/ssl/callbacks.rs index c4bcb3f0..7841950c 100644 --- a/boring/src/ssl/callbacks.rs +++ b/boring/src/ssl/callbacks.rs @@ -3,7 +3,7 @@ use super::{ AlpnError, ClientHello, GetSessionPendingError, PrivateKeyMethod, PrivateKeyMethodError, SelectCertError, SniError, Ssl, SslAlert, SslContext, SslContextRef, SslRef, SslSession, - SslSessionRef, SslSignatureAlgorithm, SESSION_CTX_INDEX, + SslSessionRef, SslSignatureAlgorithm, SslVerifyError, SESSION_CTX_INDEX, }; use crate::error::ErrorStack; use crate::ffi; @@ -42,6 +42,66 @@ where verify(preverify_ok != 0, ctx) as c_int } +pub(super) unsafe extern "C" fn raw_custom_verify( + ssl: *mut ffi::SSL, + out_alert: *mut u8, +) -> ffi::ssl_verify_result_t +where + F: Fn(&mut SslRef) -> Result<(), SslVerifyError> + 'static + Sync + Send, +{ + let callback = |ssl: &mut SslRef| { + let custom_verify_idx = SslContext::cached_ex_index::(); + + let ssl_context = ssl.ssl_context().to_owned(); + let callback = ssl_context + .ex_data(custom_verify_idx) + .expect("BUG: custom verify callback missing"); + + callback(ssl) + }; + + unsafe { raw_custom_verify_callback(ssl, out_alert, callback) } +} + +pub(super) unsafe extern "C" fn ssl_raw_custom_verify( + ssl: *mut ffi::SSL, + out_alert: *mut u8, +) -> ffi::ssl_verify_result_t +where + F: Fn(&mut SslRef) -> Result<(), SslVerifyError> + 'static + Sync + Send, +{ + let callback = |ssl: &mut SslRef| { + let callback = ssl + .ex_data(Ssl::cached_ex_index::>()) + .expect("BUG: ssl verify callback missing") + .clone(); + + callback(ssl) + }; + + unsafe { raw_custom_verify_callback(ssl, out_alert, callback) } +} + +unsafe fn raw_custom_verify_callback( + ssl: *mut ffi::SSL, + out_alert: *mut u8, + callback: impl FnOnce(&mut SslRef) -> Result<(), SslVerifyError>, +) -> ffi::ssl_verify_result_t { + // SAFETY: boring provides valid inputs. + let ssl = unsafe { SslRef::from_ptr_mut(ssl) }; + let out_alert = unsafe { &mut *out_alert }; + + match callback(ssl) { + Ok(()) => ffi::ssl_verify_result_t::ssl_verify_ok, + Err(SslVerifyError::Invalid(alert)) => { + *out_alert = alert.0 as u8; + + ffi::ssl_verify_result_t::ssl_verify_invalid + } + Err(SslVerifyError::Retry) => ffi::ssl_verify_result_t::ssl_verify_retry, + } +} + pub(super) unsafe extern "C" fn raw_client_psk( ssl_ptr: *mut ffi::SSL, hint: *const c_char, diff --git a/boring/src/ssl/error.rs b/boring/src/ssl/error.rs index 0f75b818..bc792c11 100644 --- a/boring/src/ssl/error.rs +++ b/boring/src/ssl/error.rs @@ -32,6 +32,9 @@ impl ErrorCode { pub const PENDING_CERTIFICATE: ErrorCode = ErrorCode(ffi::SSL_ERROR_PENDING_CERTIFICATE); + pub const WANT_CERTIFICATE_VERIFY: ErrorCode = + ErrorCode(ffi::SSL_ERROR_WANT_CERTIFICATE_VERIFY); + pub const WANT_PRIVATE_KEY_OPERATION: ErrorCode = ErrorCode(ffi::SSL_ERROR_WANT_PRIVATE_KEY_OPERATION); @@ -101,6 +104,7 @@ impl Error { | ErrorCode::PENDING_SESSION | ErrorCode::PENDING_CERTIFICATE | ErrorCode::WANT_PRIVATE_KEY_OPERATION + | ErrorCode::WANT_CERTIFICATE_VERIFY | ErrorCode::PENDING_TICKET ) } diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index b767457b..d7299f99 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -98,9 +98,9 @@ use crate::x509::{ use crate::{cvt, cvt_0i, cvt_n, cvt_p, init}; pub use self::async_callbacks::{ - AsyncPrivateKeyMethod, AsyncPrivateKeyMethodError, AsyncSelectCertError, BoxGetSessionFinish, - BoxGetSessionFuture, BoxPrivateKeyMethodFinish, BoxPrivateKeyMethodFuture, BoxSelectCertFinish, - BoxSelectCertFuture, ExDataFuture, + AsyncPrivateKeyMethod, AsyncPrivateKeyMethodError, AsyncSelectCertError, BoxCustomVerifyFinish, + BoxCustomVerifyFuture, BoxGetSessionFinish, BoxGetSessionFuture, BoxPrivateKeyMethodFinish, + BoxPrivateKeyMethodFuture, BoxSelectCertFinish, BoxSelectCertFuture, ExDataFuture, }; pub use self::connector::{ ConnectConfiguration, SslAcceptor, SslAcceptorBuilder, SslConnector, SslConnectorBuilder, @@ -323,6 +323,12 @@ bitflags! { } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum SslVerifyError { + Invalid(SslAlert), + Retry, +} + bitflags! { /// Options controlling the behavior of session caching. #[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] @@ -466,10 +472,41 @@ impl SniError { pub struct SslAlert(c_int); impl SslAlert { - /// Alert 112 - `unrecognized_name`. - pub const UNRECOGNIZED_NAME: SslAlert = SslAlert(ffi::SSL_AD_UNRECOGNIZED_NAME); - pub const ILLEGAL_PARAMETER: SslAlert = SslAlert(ffi::SSL_AD_ILLEGAL_PARAMETER); - pub const DECODE_ERROR: SslAlert = SslAlert(ffi::SSL_AD_DECODE_ERROR); + pub const CLOSE_NOTIFY: Self = Self(ffi::SSL_AD_CLOSE_NOTIFY); + pub const UNEXPECTED_MESSAGE: Self = Self(ffi::SSL_AD_UNEXPECTED_MESSAGE); + pub const BAD_RECORD_MAC: Self = Self(ffi::SSL_AD_BAD_RECORD_MAC); + pub const DECRYPTION_FAILED: Self = Self(ffi::SSL_AD_DECRYPTION_FAILED); + pub const RECORD_OVERFLOW: Self = Self(ffi::SSL_AD_RECORD_OVERFLOW); + pub const DECOMPRESSION_FAILURE: Self = Self(ffi::SSL_AD_DECOMPRESSION_FAILURE); + pub const HANDSHAKE_FAILURE: Self = Self(ffi::SSL_AD_HANDSHAKE_FAILURE); + pub const NO_CERTIFICATE: Self = Self(ffi::SSL_AD_NO_CERTIFICATE); + pub const BAD_CERTIFICATE: Self = Self(ffi::SSL_AD_BAD_CERTIFICATE); + pub const UNSUPPORTED_CERTIFICATE: Self = Self(ffi::SSL_AD_UNSUPPORTED_CERTIFICATE); + pub const CERTIFICATE_REVOKED: Self = Self(ffi::SSL_AD_CERTIFICATE_REVOKED); + pub const CERTIFICATE_EXPIRED: Self = Self(ffi::SSL_AD_CERTIFICATE_EXPIRED); + pub const CERTIFICATE_UNKNOWN: Self = Self(ffi::SSL_AD_CERTIFICATE_UNKNOWN); + pub const ILLEGAL_PARAMETER: Self = Self(ffi::SSL_AD_ILLEGAL_PARAMETER); + pub const UNKNOWN_CA: Self = Self(ffi::SSL_AD_UNKNOWN_CA); + pub const ACCESS_DENIED: Self = Self(ffi::SSL_AD_ACCESS_DENIED); + pub const DECODE_ERROR: Self = Self(ffi::SSL_AD_DECODE_ERROR); + pub const DECRYPT_ERROR: Self = Self(ffi::SSL_AD_DECRYPT_ERROR); + pub const EXPORT_RESTRICTION: Self = Self(ffi::SSL_AD_EXPORT_RESTRICTION); + pub const PROTOCOL_VERSION: Self = Self(ffi::SSL_AD_PROTOCOL_VERSION); + pub const INSUFFICIENT_SECURITY: Self = Self(ffi::SSL_AD_INSUFFICIENT_SECURITY); + pub const INTERNAL_ERROR: Self = Self(ffi::SSL_AD_INTERNAL_ERROR); + pub const INAPPROPRIATE_FALLBACK: Self = Self(ffi::SSL_AD_INAPPROPRIATE_FALLBACK); + pub const USER_CANCELLED: Self = Self(ffi::SSL_AD_USER_CANCELLED); + pub const NO_RENEGOTIATION: Self = Self(ffi::SSL_AD_NO_RENEGOTIATION); + pub const MISSING_EXTENSION: Self = Self(ffi::SSL_AD_MISSING_EXTENSION); + pub const UNSUPPORTED_EXTENSION: Self = Self(ffi::SSL_AD_UNSUPPORTED_EXTENSION); + pub const CERTIFICATE_UNOBTAINABLE: Self = Self(ffi::SSL_AD_CERTIFICATE_UNOBTAINABLE); + pub const UNRECOGNIZED_NAME: Self = Self(ffi::SSL_AD_UNRECOGNIZED_NAME); + pub const BAD_CERTIFICATE_STATUS_RESPONSE: Self = + Self(ffi::SSL_AD_BAD_CERTIFICATE_STATUS_RESPONSE); + pub const BAD_CERTIFICATE_HASH_VALUE: Self = Self(ffi::SSL_AD_BAD_CERTIFICATE_HASH_VALUE); + pub const UNKNOWN_PSK_IDENTITY: Self = Self(ffi::SSL_AD_UNKNOWN_PSK_IDENTITY); + pub const CERTIFICATE_REQUIRED: Self = Self(ffi::SSL_AD_CERTIFICATE_REQUIRED); + pub const NO_APPLICATION_PROTOCOL: Self = Self(ffi::SSL_AD_NO_APPLICATION_PROTOCOL); } /// An error returned from an ALPN selection callback. @@ -829,14 +866,24 @@ impl SslContextBuilder { /// Configures the certificate verification method for new connections and /// registers a verification callback. /// - /// The callback is passed a boolean indicating if OpenSSL's internal verification succeeded as - /// well as a reference to the `X509StoreContext` which can be used to examine the certificate - /// chain. It should return a boolean indicating if verification succeeded. + /// *Warning*: This callback does not replace the default certificate verification + /// process and is, instead, called multiple times in the course of that process. + /// It is very difficult to implement this callback correctly, without inadvertently + /// relying on implementation details or making incorrect assumptions about when the + /// callback is called. + /// + /// Instead, use [`SslContextBuilder::set_custom_verify_callback`] to customize certificate verification. + /// Those callbacks can inspect the peer-sent chain, call [`X509StoreContextRef::verify_cert`] + /// and inspect the result, or perform other operations more straightforwardly. /// /// This corresponds to [`SSL_CTX_set_verify`]. /// + /// # Panics + /// + /// This method panics if this `Ssl` is associated with a RPK context. + /// /// [`SSL_CTX_set_verify`]: https://www.openssl.org/docs/man1.1.0/ssl/SSL_CTX_set_verify.html - pub fn set_verify_callback(&mut self, mode: SslVerifyMode, verify: F) + pub fn set_verify_callback(&mut self, mode: SslVerifyMode, callback: F) where F: Fn(bool, &mut X509StoreContextRef) -> bool + 'static + Sync + Send, { @@ -844,11 +891,42 @@ impl SslContextBuilder { assert!(!self.is_rpk, "This API is not supported for RPK"); unsafe { - self.replace_ex_data(SslContext::cached_ex_index::(), verify); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_verify(self.as_ptr(), mode.bits() as c_int, Some(raw_verify::)); } } + /// Configures certificate verification. + /// + /// The callback should return `Ok(())` if the certificate is valid. + /// If the certificate is invalid, the callback should return `SslVerifyError::Invalid(alert)`. + /// Some useful alerts include [`SslAlert::CERTIFICATE_EXPIRED`], [`SslAlert::CERTIFICATE_REVOKED`], + /// [`SslAlert::UNKNOWN_CA`], [`SslAlert::BAD_CERTIFICATE`], [`SslAlert::CERTIFICATE_UNKNOWN`], + /// and [`SslAlert::INTERNAL_ERROR`]. See RFC 5246 section 7.2.2 for their precise meanings. + /// + /// To verify a certificate asynchronously, the callback may return `Err(SslVerifyError::Retry)`. + /// The handshake will then pause with an error with code [`ErrorCode::WANT_CERTIFICATE_VERIFY`]. + /// + /// # Panics + /// + /// This method panics if this `Ssl` is associated with a RPK context. + pub fn set_custom_verify_callback(&mut self, mode: SslVerifyMode, callback: F) + where + F: Fn(&mut SslRef) -> Result<(), SslVerifyError> + 'static + Sync + Send, + { + #[cfg(feature = "rpk")] + assert!(!self.is_rpk, "This API is not supported for RPK"); + + unsafe { + self.replace_ex_data(SslContext::cached_ex_index::(), callback); + ffi::SSL_CTX_set_custom_verify( + self.as_ptr(), + mode.bits() as c_int, + Some(raw_custom_verify::), + ); + } + } + /// Configures the server name indication (SNI) callback for new connections. /// /// SNI is used to allow a single server to handle requests for multiple domains, each of which @@ -2642,11 +2720,25 @@ impl SslRef { /// Like [`SslContextBuilder::set_verify_callback`]. /// + /// *Warning*: This callback does not replace the default certificate verification + /// process and is, instead, called multiple times in the course of that process. + /// It is very difficult to implement this callback correctly, without inadvertently + /// relying on implementation details or making incorrect assumptions about when the + /// callback is called. + /// + /// Instead, use [`SslContextBuilder::set_custom_verify_callback`] to customize + /// certificate verification. Those callbacks can inspect the peer-sent chain, + /// call [`X509StoreContextRef::verify_cert`] and inspect the result, or perform + /// other operations more straightforwardly. + /// /// This corresponds to [`SSL_set_verify`]. /// - /// [`SslContextBuilder::set_verify_callback`]: struct.SslContextBuilder.html#method.set_verify_callback + /// # Panics + /// + /// This method panics if this `Ssl` is associated with a RPK context. + /// /// [`SSL_set_verify`]: https://www.openssl.org/docs/man1.0.2/ssl/SSL_set_verify.html - pub fn set_verify_callback(&mut self, mode: SslVerifyMode, verify: F) + pub fn set_verify_callback(&mut self, mode: SslVerifyMode, callback: F) where F: Fn(bool, &mut X509StoreContextRef) -> bool + 'static + Sync + Send, { @@ -2658,7 +2750,7 @@ impl SslRef { unsafe { // this needs to be in an Arc since the callback can register a new callback! - self.replace_ex_data(Ssl::cached_ex_index(), Arc::new(verify)); + self.replace_ex_data(Ssl::cached_ex_index(), Arc::new(callback)); ffi::SSL_set_verify( self.as_ptr(), mode.bits() as c_int, @@ -2667,6 +2759,34 @@ impl SslRef { } } + /// Like [`SslContextBuilder::set_custom_verify_callback`]. + /// + /// This corresponds to [`SSL_set_custom_verify`]. + /// + /// # Panics + /// + /// This method panics if this `Ssl` is associated with a RPK context. + pub fn set_custom_verify_callback(&mut self, mode: SslVerifyMode, callback: F) + where + F: Fn(&mut SslRef) -> Result<(), SslVerifyError> + 'static + Sync + Send, + { + #[cfg(feature = "rpk")] + assert!( + !self.ssl_context().is_rpk(), + "This API is not supported for RPK" + ); + + unsafe { + // this needs to be in an Arc since the callback can register a new callback! + self.replace_ex_data(Ssl::cached_ex_index(), Arc::new(callback)); + ffi::SSL_set_custom_verify( + self.as_ptr(), + mode.bits() as c_int, + Some(ssl_raw_custom_verify::), + ); + } + } + /// Like [`SslContextBuilder::set_tmp_dh`]. /// /// This corresponds to [`SSL_set_tmp_dh`]. diff --git a/boring/src/ssl/test/custom_verify.rs b/boring/src/ssl/test/custom_verify.rs new file mode 100644 index 00000000..e7feb56c --- /dev/null +++ b/boring/src/ssl/test/custom_verify.rs @@ -0,0 +1,278 @@ +use super::server::Server; +use crate::ssl::{ErrorCode, HandshakeError, SslAlert, SslVerifyMode}; +use crate::x509::X509StoreContext; +use crate::{hash::MessageDigest, ssl::SslVerifyError}; +use hex; +use std::sync::atomic::{AtomicBool, Ordering}; + +#[test] +fn untrusted_callback_override_bad() { + let mut server = Server::builder(); + + server.err_cb(|err| { + let HandshakeError::Failure(handshake) = err else { + panic!("expected failure error"); + }; + + assert_eq!( + handshake.error().to_string(), + "[SSLV3_ALERT_CERTIFICATE_REVOKED]" + ); + }); + + let server = server.build(); + + let mut client = server.client(); + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, |_| { + Err(SslVerifyError::Invalid(SslAlert::CERTIFICATE_REVOKED)) + }); + + client.connect_err(); +} + +#[test] +fn untrusted_callback_override_ok() { + let server = Server::builder().build(); + let mut client = server.client(); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, |ssl| { + assert!(ssl.peer_cert_chain().is_some()); + + Ok(()) + }); + + client.connect(); +} + +#[test] +fn untrusted_with_set_cert() { + let mut server = Server::builder(); + + server.should_error(); + + let server = server.build(); + let mut client = server.client(); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, move |ssl| { + let store = ssl.ssl_context().cert_store(); + let cert = ssl.peer_certificate().unwrap(); + let cert_chain = ssl.peer_cert_chain().unwrap(); + + assert_eq!(store.objects().len(), 0); + + X509StoreContext::new() + .unwrap() + .init(store, &cert, cert_chain, |store_ctx| { + assert!(!store_ctx.verify_cert().unwrap()); + assert!(store_ctx.verify_result().is_err()); + + Ok(()) + }) + .unwrap(); + + Err(SslVerifyError::Invalid(SslAlert::CERTIFICATE_UNKNOWN)) + }); + + client.connect_err(); +} + +#[test] +fn trusted_with_set_cert() { + let server = Server::builder().build(); + let mut client = server.client_with_root_ca(); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, move |ssl| { + let store = ssl.ssl_context().cert_store(); + let cert = ssl.peer_certificate().unwrap(); + let cert_chain = ssl.peer_cert_chain().unwrap(); + + assert_eq!(store.objects().len(), 1); + + X509StoreContext::new() + .unwrap() + .init(store, &cert, cert_chain, |store_ctx| { + assert!(store_ctx.verify_cert().unwrap()); + assert_eq!(store_ctx.verify_result(), Ok(())); + + Ok(()) + }) + .unwrap(); + + Ok(()) + }); + + client.connect(); +} + +#[test] +fn trusted_callback_override_ok() { + let server = Server::builder().build(); + let mut client = server.client_with_root_ca(); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, |ssl| { + assert!(ssl.peer_certificate().is_some()); + + Ok(()) + }); + + client.connect(); +} + +#[test] +fn trusted_callback_override_bad() { + let mut server = Server::builder(); + + server.should_error(); + + let server = server.build(); + let mut client = server.client_with_root_ca(); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, |_| { + Err(SslVerifyError::Invalid(SslAlert::CERTIFICATE_UNKNOWN)) + }); + + client.connect_err(); +} + +#[test] +fn callback() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); + let mut client = server.client(); + let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; + + client + .ctx() + .set_verify_callback(SslVerifyMode::PEER, |_, _| { + panic!("verify callback should not be called"); + }); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, move |ssl| { + CALLED_BACK.store(true, Ordering::SeqCst); + + let cert = ssl.peer_certificate().unwrap(); + let digest = cert.digest(MessageDigest::sha1()).unwrap(); + + assert_eq!(hex::encode(digest), expected); + + Ok(()) + }); + + client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); +} + +#[test] +fn ssl_callback() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); + let mut client = server.client().build().builder(); + let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; + + client + .ssl() + .set_verify_callback(SslVerifyMode::PEER, |_, _| { + panic!("verify callback should not be called"); + }); + + client + .ssl() + .set_custom_verify_callback(SslVerifyMode::PEER, move |ssl| { + CALLED_BACK.store(true, Ordering::SeqCst); + + let cert = ssl.peer_certificate().unwrap(); + let digest = cert.digest(MessageDigest::sha1()).unwrap(); + + assert_eq!(hex::encode(digest), expected); + + Ok(()) + }); + + client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); +} + +#[test] +fn both_callback() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); + let mut client = server.client(); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, |_| { + panic!("verify callback should not be called"); + }); + + let mut client = client.build().builder(); + let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; + + client + .ssl() + .set_custom_verify_callback(SslVerifyMode::PEER, move |ssl| { + CALLED_BACK.store(true, Ordering::SeqCst); + + let cert = ssl.peer_certificate().unwrap(); + let digest = cert.digest(MessageDigest::sha1()).unwrap(); + + assert_eq!(hex::encode(digest), expected); + + Ok(()) + }); + + client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); +} + +#[test] +fn retry() { + let mut server = Server::builder(); + + server.err_cb(|err| { + let HandshakeError::Failure(handshake) = err else { + panic!("expected failure error"); + }; + + assert_eq!( + handshake.error().to_string(), + "[SSLV3_ALERT_CERTIFICATE_REVOKED]" + ); + }); + + let server = server.build(); + let mut client = server.client(); + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + + client + .ctx() + .set_custom_verify_callback(SslVerifyMode::PEER, move |_| { + if !CALLED_BACK.swap(true, Ordering::SeqCst) { + return Err(SslVerifyError::Retry); + } + + Err(SslVerifyError::Invalid(SslAlert::CERTIFICATE_REVOKED)) + }); + + let HandshakeError::WouldBlock(handshake) = client.connect_err() else { + panic!("should be WouldBlock"); + }; + + assert!(CALLED_BACK.load(Ordering::SeqCst)); + assert!(handshake.error().would_block()); + assert_eq!(handshake.error().code(), ErrorCode::WANT_CERTIFICATE_VERIFY); + handshake.handshake().unwrap_err(); +} diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index c6199352..29b85ac3 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -18,204 +18,19 @@ use crate::ssl::{ ExtensionType, ShutdownResult, ShutdownState, Ssl, SslAcceptor, SslAcceptorBuilder, SslConnector, SslContext, SslFiletype, SslMethod, SslOptions, SslStream, SslVerifyMode, }; -use crate::x509::store::X509StoreBuilder; use crate::x509::verify::X509CheckFlags; use crate::x509::{X509Name, X509}; +mod custom_verify; mod private_key_method; mod server; mod session; +mod verify; static ROOT_CERT: &[u8] = include_bytes!("../../../test/root-ca.pem"); static CERT: &[u8] = include_bytes!("../../../test/cert.pem"); static KEY: &[u8] = include_bytes!("../../../test/key.pem"); -#[test] -fn verify_untrusted() { - let mut server = Server::builder(); - server.should_error(); - let server = server.build(); - - let mut client = server.client(); - client.ctx().set_verify(SslVerifyMode::PEER); - - client.connect_err(); -} - -#[test] -fn verify_trusted() { - let server = Server::builder().build(); - let client = server.client_with_root_ca(); - - client.connect(); -} - -#[test] -fn verify_trusted_with_set_cert() { - let server = Server::builder().build(); - - let mut store = X509StoreBuilder::new().unwrap(); - let x509 = X509::from_pem(ROOT_CERT).unwrap(); - store.add_cert(x509).unwrap(); - - let mut client = server.client(); - client.ctx().set_verify(SslVerifyMode::PEER); - client.ctx().set_verify_cert_store(store.build()).unwrap(); - - client.connect(); -} - -#[test] -fn verify_untrusted_callback_override_ok() { - let server = Server::builder().build(); - - let mut client = server.client(); - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, x509| { - assert!(x509.current_cert().is_some()); - true - }); - - client.connect(); -} - -#[test] -fn verify_untrusted_callback_override_bad() { - let mut server = Server::builder(); - server.should_error(); - let server = server.build(); - - let mut client = server.client(); - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, _| false); - - client.connect_err(); -} - -#[test] -fn verify_trusted_callback_override_ok() { - let server = Server::builder().build(); - let mut client = server.client_with_root_ca(); - - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, x509| { - assert!(x509.current_cert().is_some()); - true - }); - - client.connect(); -} - -#[test] -fn verify_trusted_callback_override_bad() { - let mut server = Server::builder(); - - server.should_error(); - - let server = server.build(); - let mut client = server.client_with_root_ca(); - - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, _| false); - - client.connect_err(); -} - -#[test] -fn verify_callback_load_certs() { - let server = Server::builder().build(); - - let mut client = server.client(); - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, x509| { - assert!(x509.current_cert().is_some()); - true - }); - - client.connect(); -} - -#[test] -fn verify_trusted_get_error_ok() { - let server = Server::builder().build(); - let mut client = server.client_with_root_ca(); - - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, x509| { - assert_eq!(x509.verify_result(), Ok(())); - true - }); - - client.connect(); -} - -#[test] -fn verify_trusted_get_error_err() { - let mut server = Server::builder(); - server.should_error(); - let server = server.build(); - - let mut client = server.client(); - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, |_, x509| { - assert!(x509.verify_result().is_err()); - false - }); - - client.connect_err(); -} - -#[test] -fn verify_callback() { - static CALLED_BACK: AtomicBool = AtomicBool::new(false); - - let server = Server::builder().build(); - - let mut client = server.client(); - let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; - client - .ctx() - .set_verify_callback(SslVerifyMode::PEER, move |_, x509| { - CALLED_BACK.store(true, Ordering::SeqCst); - let cert = x509.current_cert().unwrap(); - let digest = cert.digest(MessageDigest::sha1()).unwrap(); - assert_eq!(hex::encode(digest), expected); - true - }); - - client.connect(); - assert!(CALLED_BACK.load(Ordering::SeqCst)); -} - -#[test] -fn ssl_verify_callback() { - static CALLED_BACK: AtomicBool = AtomicBool::new(false); - - let server = Server::builder().build(); - - let mut client = server.client().build().builder(); - let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; - client - .ssl() - .set_verify_callback(SslVerifyMode::PEER, move |_, x509| { - CALLED_BACK.store(true, Ordering::SeqCst); - let cert = x509.current_cert().unwrap(); - let digest = cert.digest(MessageDigest::sha1()).unwrap(); - assert_eq!(hex::encode(digest), expected); - true - }); - - client.connect(); - assert!(CALLED_BACK.load(Ordering::SeqCst)); -} - #[test] fn get_ctx_options() { let ctx = SslContext::builder(SslMethod::tls()).unwrap(); @@ -871,18 +686,6 @@ fn client_ca_list() { ctx.set_client_ca_list(names); } -#[test] -fn cert_store() { - let server = Server::builder().build(); - - let mut client = server.client(); - let cert = X509::from_pem(ROOT_CERT).unwrap(); - client.ctx().cert_store_mut().add_cert(cert).unwrap(); - client.ctx().set_verify(SslVerifyMode::PEER); - - client.connect(); -} - #[test] fn keying_export() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); diff --git a/boring/src/ssl/test/verify.rs b/boring/src/ssl/test/verify.rs new file mode 100644 index 00000000..5fed5703 --- /dev/null +++ b/boring/src/ssl/test/verify.rs @@ -0,0 +1,150 @@ +use super::server::Server; +use crate::hash::MessageDigest; +use crate::ssl::SslVerifyMode; +use crate::x509::store::X509StoreBuilder; +use crate::x509::X509; +use hex; +use std::sync::atomic::{AtomicBool, Ordering}; + +#[test] +fn untrusted() { + let mut server = Server::builder(); + server.should_error(); + let server = server.build(); + + let mut client = server.client(); + client.ctx().set_verify(SslVerifyMode::PEER); + + client.connect_err(); +} + +#[test] +fn trusted() { + let server = Server::builder().build(); + let client = server.client_with_root_ca(); + + client.connect(); +} + +#[test] +fn trusted_with_set_cert() { + let server = Server::builder().build(); + + let mut store = X509StoreBuilder::new().unwrap(); + let x509 = X509::from_pem(super::ROOT_CERT).unwrap(); + store.add_cert(x509).unwrap(); + + let mut client = server.client(); + client.ctx().set_verify(SslVerifyMode::PEER); + client.ctx().set_verify_cert_store(store.build()).unwrap(); + + client.connect(); +} + +#[test] +fn untrusted_callback_override_ok() { + let server = Server::builder().build(); + + let mut client = server.client(); + client + .ctx() + .set_verify_callback(SslVerifyMode::PEER, |_, x509| { + assert!(x509.current_cert().is_some()); + assert!(x509.verify_result().is_err()); + + true + }); + + client.connect(); +} + +#[test] +fn untrusted_callback_override_bad() { + let mut server = Server::builder(); + server.should_error(); + let server = server.build(); + + let mut client = server.client(); + client + .ctx() + .set_verify_callback(SslVerifyMode::PEER, |_, _| false); + + client.connect_err(); +} + +#[test] +fn trusted_callback_override_ok() { + let server = Server::builder().build(); + let mut client = server.client_with_root_ca(); + + client + .ctx() + .set_verify_callback(SslVerifyMode::PEER, |_, x509| { + assert!(x509.current_cert().is_some()); + assert_eq!(x509.verify_result(), Ok(())); + + true + }); + + client.connect(); +} + +#[test] +fn trusted_callback_override_bad() { + let mut server = Server::builder(); + + server.should_error(); + + let server = server.build(); + let mut client = server.client_with_root_ca(); + + client + .ctx() + .set_verify_callback(SslVerifyMode::PEER, |_, _| false); + + client.connect_err(); +} + +#[test] +fn callback() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + + let server = Server::builder().build(); + + let mut client = server.client(); + let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; + client + .ctx() + .set_verify_callback(SslVerifyMode::PEER, move |_, x509| { + CALLED_BACK.store(true, Ordering::SeqCst); + let cert = x509.current_cert().unwrap(); + let digest = cert.digest(MessageDigest::sha1()).unwrap(); + assert_eq!(hex::encode(digest), expected); + true + }); + + client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); +} + +#[test] +fn ssl_callback() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + + let server = Server::builder().build(); + + let mut client = server.client().build().builder(); + let expected = "59172d9313e84459bcff27f967e79e6e9217e584"; + client + .ssl() + .set_verify_callback(SslVerifyMode::PEER, move |_, x509| { + CALLED_BACK.store(true, Ordering::SeqCst); + let cert = x509.current_cert().unwrap(); + let digest = cert.digest(MessageDigest::sha1()).unwrap(); + assert_eq!(hex::encode(digest), expected); + true + }); + + client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); +} diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 30b5e725..10ee1935 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -559,7 +559,7 @@ impl X509Ref { /// /// Returns `true` if verification succeeds. /// - /// This corresponds to [`X509_verify"]. + /// This corresponds to [`X509_verify`]. /// /// [`X509_verify`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_verify.html pub fn verify(&self, key: &PKeyRef) -> Result diff --git a/tokio-boring/tests/async_custom_verify.rs b/tokio-boring/tests/async_custom_verify.rs new file mode 100644 index 00000000..0c5c92b8 --- /dev/null +++ b/tokio-boring/tests/async_custom_verify.rs @@ -0,0 +1,96 @@ +use boring::ssl::{BoxCustomVerifyFinish, BoxCustomVerifyFuture, SslAlert, SslRef, SslVerifyMode}; +use futures::future; +use tokio::task::yield_now; + +mod common; + +use self::common::{connect, create_server, with_trivial_client_server_exchange}; + +#[tokio::test] +async fn test_async_custom_verify_callback_trivial() { + with_trivial_client_server_exchange(|builder| { + builder.set_async_custom_verify_callback(SslVerifyMode::PEER, |_| { + Ok(Box::pin(async { + Ok(Box::new(|_: &mut _| Ok(())) as BoxCustomVerifyFinish) + })) + }); + }) + .await; +} + +#[tokio::test] +async fn test_async_custom_verify_callback_yield() { + with_trivial_client_server_exchange(|builder| { + builder.set_async_custom_verify_callback(SslVerifyMode::PEER, |_| { + Ok(Box::pin(async { + yield_now().await; + + Ok(Box::new(|_: &mut _| Ok(())) as BoxCustomVerifyFinish) + })) + }); + }) + .await; +} + +#[tokio::test] +async fn test_async_custom_verify_callback_return_error() { + with_async_custom_verify_callback_error(|_| Err(SslAlert::CERTIFICATE_REVOKED)).await; +} + +#[tokio::test] +async fn test_async_custom_verify_callback_future_error() { + with_async_custom_verify_callback_error(|_| { + Ok(Box::pin(async move { Err(SslAlert::CERTIFICATE_REVOKED) })) + }) + .await; +} + +#[tokio::test] +async fn test_async_custom_verify_callback_future_yield_error() { + with_async_custom_verify_callback_error(|_| { + Ok(Box::pin(async move { + yield_now().await; + + Err(SslAlert::CERTIFICATE_REVOKED) + })) + }) + .await; +} + +#[tokio::test] +async fn test_async_custom_verify_callback_finish_error() { + with_async_custom_verify_callback_error(|_| { + Ok(Box::pin(async move { + yield_now().await; + + Ok(Box::new(|_: &mut _| Err(SslAlert::CERTIFICATE_REVOKED)) as BoxCustomVerifyFinish) + })) + }) + .await; +} + +async fn with_async_custom_verify_callback_error( + callback: impl Fn(&mut SslRef) -> Result + Send + Sync + 'static, +) { + let (stream, addr) = create_server(|_| {}); + + let server = async { + let err = stream.await.unwrap_err(); + + assert_eq!( + err.to_string(), + "TLS handshake failed [SSLV3_ALERT_CERTIFICATE_REVOKED]" + ); + }; + + let client = async { + let _err = connect(addr, |builder| { + builder.set_async_custom_verify_callback(SslVerifyMode::PEER, callback); + builder.set_ca_file("tests/cert.pem") + }) + .await + .unwrap_err(); + }; + + future::join(server, client).await; +}