Overhaul verify error type

Also set the error in the hostname verification callback for 1.0.1

4 files changed, 41 insertions, 40 deletions

diff --git a/openssl/src/ssl/connector.rs b/openssl/src/ssl/connector.rs
index 6b2d9864..0c5d0df0 100644
--- a/openssl/src/ssl/connector.rs
+++ b/openssl/src/ssl/connector.rs
@@ -7,11 +7,6 @@ use ssl::{HandshakeError, Ssl, SslContext, SslContextBuilder, SslMethod, SslMode
           SslRef, SslStream, SslVerifyMode};
 use version;
 
-#[cfg(ossl101)]
-lazy_static! {
-    static ref HOSTNAME_IDX: ::ex_data::Index<Ssl, String> = Ssl::new_ex_index().unwrap();
-}
-
 // ffdhe2048 from https://wiki.mozilla.org/Security/Server_Side_TLS#ffdhe2048
 const DHPARAM_PEM: &'static str = "
 -----BEGIN DH PARAMETERS-----
@@ -297,16 +292,7 @@ fn setup_verify(ctx: &mut SslContextBuilder) {
 
 #[cfg(ossl101)]
 fn setup_verify(ctx: &mut SslContextBuilder) {
-    ctx.set_verify_callback(SslVerifyMode::PEER, |p, x509| {
-        let hostname = match x509.ssl() {
-            Ok(Some(ssl)) => ssl.ex_data(*HOSTNAME_IDX),
-            _ => None,
-        };
-        match hostname {
-            Some(hostname) => verify::verify_callback(hostname, p, x509),
-            None => p,
-        }
-    });
+    ctx.set_verify_callback(SslVerifyMode::PEER, verify::verify_callback);
 }
 
 #[cfg(any(ossl102, ossl110))]
@@ -322,7 +308,7 @@ fn setup_verify_hostname(ssl: &mut Ssl, domain: &str) -> Result<(), ErrorStack>
 #[cfg(ossl101)]
 fn setup_verify_hostname(ssl: &mut Ssl, domain: &str) -> Result<(), ErrorStack> {
     let domain = domain.to_string();
-    ssl.set_ex_data(*HOSTNAME_IDX, domain);
+    ssl.set_ex_data(*verify::HOSTNAME_IDX, domain);
     Ok(())
 }
 
@@ -331,23 +317,38 @@ mod verify {
     use std::net::IpAddr;
     use std::str;
 
+    use ex_data::Index;
     use nid::Nid;
-    use x509::{GeneralName, X509NameRef, X509Ref, X509StoreContextRef};
+    use x509::{GeneralName, X509NameRef, X509Ref, X509StoreContextRef, X509VerifyResult};
     use stack::Stack;
+    use ssl::Ssl;
 
-    pub fn verify_callback(
-        domain: &str,
-        preverify_ok: bool,
-        x509_ctx: &X509StoreContextRef,
-    ) -> bool {
+    lazy_static! {
+        pub static ref HOSTNAME_IDX: Index<Ssl, String> = Ssl::new_ex_index().unwrap();
+    }
+
+    pub fn verify_callback(preverify_ok: bool, x509_ctx: &mut X509StoreContextRef) -> bool {
         if !preverify_ok || x509_ctx.error_depth() != 0 {
             return preverify_ok;
         }
 
-        match x509_ctx.current_cert() {
-            Some(x509) => verify_hostname(domain, &x509),
-            None => true,
+        let ok = match (
+            x509_ctx.current_cert(),
+            x509_ctx
+                .ssl()
+                .ok()
+                .and_then(|s| s)
+                .and_then(|s| s.ex_data(*HOSTNAME_IDX)),
+        ) {
+            (Some(x509), Some(domain)) => verify_hostname(domain, &x509),
+            _ => true,
+        };
+
+        if !ok {
+            x509_ctx.set_error(X509VerifyResult::APPLICATION_VERIFICATION);
         }
+
+        ok
     }
 
     fn verify_hostname(domain: &str, cert: &X509Ref) -> bool {
diff --git a/openssl/src/ssl/error.rs b/openssl/src/ssl/error.rs
index 64941f18..b0641dfd 100644
--- a/openssl/src/ssl/error.rs
+++ b/openssl/src/ssl/error.rs
@@ -5,6 +5,7 @@ use std::io;
 
 use error::ErrorStack;
 use ssl::MidHandshakeSslStream;
+use x509::X509VerifyResult;
 
 /// An SSL error.
 // FIXME this is missing variants
@@ -130,8 +131,9 @@ impl<S: fmt::Debug> fmt::Display for HandshakeError<S> {
             HandshakeError::SetupFailure(ref e) => write!(f, ": {}", e)?,
             HandshakeError::Failure(ref s) | HandshakeError::WouldBlock(ref s) => {
                 write!(f, ": {}", s.error())?;
-                if let Some(err) = s.ssl().verify_result() {
-                    write!(f, ": {}", err)?;
+                let verify = s.ssl().verify_result();
+                if verify != X509VerifyResult::OK {
+                    write!(f, ": {}", verify)?;
                 }
             }
         }
diff --git a/openssl/src/ssl/mod.rs b/openssl/src/ssl/mod.rs
index 0748140d..c0ed5572 100644
--- a/openssl/src/ssl/mod.rs
+++ b/openssl/src/ssl/mod.rs
@@ -86,7 +86,7 @@ use dh::{Dh, DhRef};
 use ec::EcKeyRef;
 #[cfg(any(all(feature = "v101", ossl101), all(feature = "v102", ossl102)))]
 use ec::EcKey;
-use x509::{X509, X509Filetype, X509Name, X509Ref, X509StoreContextRef, X509VerifyError};
+use x509::{X509, X509Filetype, X509Name, X509Ref, X509StoreContextRef, X509VerifyResult};
 use x509::store::{X509StoreBuilderRef, X509StoreRef};
 #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]
 use x509::store::X509Store;
@@ -1441,12 +1441,10 @@ impl Ssl {
 
 impl fmt::Debug for SslRef {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        let mut builder = fmt.debug_struct("Ssl");
-        builder.field("state", &self.state_string_long());
-        if let Some(err) = self.verify_result() {
-            builder.field("verify_result", &err);
-        }
-        builder.finish()
+        fmt.debug_struct("Ssl")
+            .field("state", &self.state_string_long())
+            .field("verify_result", &self.verify_result())
+            .finish()
     }
 }
 
@@ -1870,8 +1868,8 @@ impl SslRef {
     /// This corresponds to [`SSL_get_verify_result`].
     ///
     /// [`SSL_get_verify_result`]: https://www.openssl.org/docs/man1.0.2/ssl/SSL_get_verify_result.html
-    pub fn verify_result(&self) -> Option<X509VerifyError> {
-        unsafe { X509VerifyError::from_raw(ffi::SSL_get_verify_result(self.as_ptr())) }
+    pub fn verify_result(&self) -> X509VerifyResult {
+        unsafe { X509VerifyResult::from_raw(ffi::SSL_get_verify_result(self.as_ptr()) as c_int) }
     }
 
     /// Returns a shared reference to the SSL session.
diff --git a/openssl/src/ssl/tests/mod.rs b/openssl/src/ssl/tests/mod.rs
index a5594254..c618a704 100644
--- a/openssl/src/ssl/tests/mod.rs
+++ b/openssl/src/ssl/tests/mod.rs
@@ -20,7 +20,7 @@ use ocsp::{OcspResponse, OcspResponseStatus};
 use ssl;
 use ssl::{Error, HandshakeError, ShutdownResult, Ssl, SslAcceptor, SslConnector, SslContext,
           SslMethod, SslStream, SslVerifyMode, StatusType};
-use x509::{X509, X509Filetype, X509Name, X509StoreContext};
+use x509::{X509, X509Filetype, X509Name, X509StoreContext, X509VerifyResult};
 #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]
 use x509::verify::X509CheckFlags;
 use pkey::PKey;
@@ -133,7 +133,7 @@ macro_rules! run_test(
             use ssl::SslMethod;
             use ssl::{SslContext, Ssl, SslStream, SslVerifyMode, SslOptions};
             use hash::MessageDigest;
-            use x509::X509StoreContext;
+            use x509::{X509StoreContext, X509VerifyResult};
             #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]
             use x509::X509;
             #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]
@@ -255,7 +255,7 @@ run_test!(verify_callback_load_certs, |method, stream| {
 run_test!(verify_trusted_get_error_ok, |method, stream| {
     let mut ctx = SslContext::builder(method).unwrap();
     ctx.set_verify_callback(SslVerifyMode::PEER, |_, x509_ctx| {
-        assert!(x509_ctx.error().is_none());
+        assert!(x509_ctx.error() == X509VerifyResult::OK);
         true
     });
 
@@ -269,7 +269,7 @@ run_test!(verify_trusted_get_error_ok, |method, stream| {
 run_test!(verify_trusted_get_error_err, |method, stream| {
     let mut ctx = SslContext::builder(method).unwrap();
     ctx.set_verify_callback(SslVerifyMode::PEER, |_, x509_ctx| {
-        assert!(x509_ctx.error().is_some());
+        assert_ne!(x509_ctx.error(), X509VerifyResult::OK);
         false
     });