Overhaul verify error type

Also set the error in the hostname verification callback for 1.0.1

6 files changed, 71 insertions, 68 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
     });
 
diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs
index 71c0c83a..70d82c61 100644
--- a/openssl/src/x509/mod.rs
+++ b/openssl/src/x509/mod.rs
@@ -60,8 +60,14 @@ foreign_type_and_impl_send_sync! {
 }
 
 impl X509StoreContextRef {
-    pub fn error(&self) -> Option<X509VerifyError> {
-        unsafe { X509VerifyError::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr()) as c_long) }
+    pub fn error(&self) -> X509VerifyResult {
+        unsafe { X509VerifyResult::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) }
+    }
+
+    pub fn set_error(&mut self, result: X509VerifyResult) {
+        unsafe {
+            ffi::X509_STORE_CTX_set_error(self.as_ptr(), result.as_raw());
+        }
     }
 
     pub fn current_cert(&self) -> Option<&X509Ref> {
@@ -343,13 +349,10 @@ impl X509Ref {
     }
 
     /// Checks that this certificate issued `subject`.
-    pub fn issued(&self, subject: &X509Ref) -> Result<(), X509VerifyError> {
+    pub fn issued(&self, subject: &X509Ref) -> X509VerifyResult {
         unsafe {
             let r = ffi::X509_check_issued(self.as_ptr(), subject.as_ptr());
-            match X509VerifyError::from_raw(r as c_long) {
-                Some(e) => Err(e),
-                None => Ok(()),
-            }
+            X509VerifyResult::from_raw(r)
         }
     }
 
@@ -746,47 +749,42 @@ impl X509ReqRef {
     }
 }
 
-pub struct X509VerifyError(c_long);
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub struct X509VerifyResult(c_int);
 
-impl fmt::Debug for X509VerifyError {
+impl fmt::Debug for X509VerifyResult {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.debug_struct("X509VerifyError")
+        fmt.debug_struct("X509VerifyResult")
             .field("code", &self.0)
             .field("error", &self.error_string())
             .finish()
     }
 }
 
-impl fmt::Display for X509VerifyError {
+impl fmt::Display for X509VerifyResult {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.write_str(self.error_string())
     }
 }
 
-impl Error for X509VerifyError {
+impl Error for X509VerifyResult {
     fn description(&self) -> &str {
         "an X509 validation error"
     }
 }
 
-impl X509VerifyError {
-    /// Creates an `X509VerifyError` from a raw error number.
-    ///
-    /// `None` will be returned if `err` is `X509_V_OK`.
+impl X509VerifyResult {
+    /// Creates an `X509VerifyResult` from a raw error number.
     ///
     /// # Safety
     ///
-    /// Some methods on `X509VerifyError` are not thread safe if the error
+    /// Some methods on `X509VerifyResult` are not thread safe if the error
     /// number is invalid.
-    pub unsafe fn from_raw(err: c_long) -> Option<X509VerifyError> {
-        if err == ffi::X509_V_OK as c_long {
-            None
-        } else {
-            Some(X509VerifyError(err))
-        }
+    pub unsafe fn from_raw(err: c_int) -> X509VerifyResult {
+        X509VerifyResult(err)
     }
 
-    pub fn as_raw(&self) -> c_long {
+    pub fn as_raw(&self) -> c_int {
         self.0
     }
 
@@ -794,10 +792,14 @@ impl X509VerifyError {
         ffi::init();
 
         unsafe {
-            let s = ffi::X509_verify_cert_error_string(self.0);
+            let s = ffi::X509_verify_cert_error_string(self.0 as c_long);
             str::from_utf8(CStr::from_ptr(s).to_bytes()).unwrap()
         }
     }
+
+    pub const OK: X509VerifyResult = X509VerifyResult(ffi::X509_V_OK);
+    pub const APPLICATION_VERIFICATION: X509VerifyResult =
+        X509VerifyResult(ffi::X509_V_ERR_APPLICATION_VERIFICATION);
 }
 
 foreign_type_and_impl_send_sync! {
diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs
index a86aa30a..366b91e7 100644
--- a/openssl/src/x509/tests.rs
+++ b/openssl/src/x509/tests.rs
@@ -7,7 +7,7 @@ use nid::Nid;
 use pkey::PKey;
 use rsa::Rsa;
 use stack::Stack;
-use x509::{X509, X509Name, X509Req};
+use x509::{X509, X509Name, X509Req, X509VerifyResult};
 use x509::extension::{AuthorityKeyIdentifier, BasicConstraints, ExtendedKeyUsage, KeyUsage,
                       SubjectAlternativeName, SubjectKeyIdentifier};
 
@@ -253,8 +253,8 @@ fn issued() {
     let ca = include_bytes!("../../test/root-ca.pem");
     let ca = X509::from_pem(ca).unwrap();
 
-    ca.issued(&cert).unwrap();
-    cert.issued(&cert).err().unwrap();
+    assert_eq!(ca.issued(&cert), X509VerifyResult::OK);
+    assert_ne!(cert.issued(&cert), X509VerifyResult::OK);
 }
 
 #[test]