Change X509VerifyResult to Result<(), X509VerifyError>

This commit separates X509VerifyResult::OK from the rest
of the codes that actually represent errors, using
a Result type as usual.

boring/examples/mk_certs.rs

@@ -13,7 +13,7 @@ use boring::x509::extension::{
     AuthorityKeyIdentifier, BasicConstraints, KeyUsage, SubjectAlternativeName,
     SubjectKeyIdentifier,
 };
-use boring::x509::{X509NameBuilder, X509Ref, X509Req, X509ReqBuilder, X509VerifyResult, X509};
+use boring::x509::{X509NameBuilder, X509Ref, X509Req, X509ReqBuilder, X509};
 
 /// Make a CA certificate and private key
 fn mk_ca_cert() -> Result<(X509, PKey<Private>), ErrorStack> {
@@ -145,8 +145,8 @@ fn real_main() -> Result<(), ErrorStack> {
 
     // Verify that this cert was issued by this ca
     match ca_cert.issued(&cert) {
-        X509VerifyResult::OK => println!("Certificate verified!"),
+        Ok(()) => println!("Certificate verified!"),
-        ver_err => println!("Failed to verify certificate: {}", ver_err),
+        Err(ver_err) => println!("Failed to verify certificate: {}", ver_err),
     };
 
     Ok(())

boring/src/ssl/error.rs

@@ -7,7 +7,6 @@ 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)]
@@ -200,8 +199,8 @@ fn fmt_mid_handshake_error(
     }
 
     match s.ssl().verify_result() {
-        X509VerifyResult::OK => write!(f, "{}", prefix)?,
+        Ok(()) => write!(f, "{}", prefix)?,
-        verify => write!(f, "{}: cert verification failed - {}", prefix, verify)?,
+        Err(verify) => write!(f, "{}: cert verification failed - {}", prefix, verify)?,
     }
 
     write!(f, " {}", s.error())

boring/src/ssl/mod.rs

@@ -92,7 +92,9 @@ use crate::ssl::error::InnerError;
 use crate::stack::{Stack, StackRef};
 use crate::x509::store::{X509Store, X509StoreBuilderRef, X509StoreRef};
 use crate::x509::verify::X509VerifyParamRef;
-use crate::x509::{X509Name, X509Ref, X509StoreContextRef, X509VerifyResult, X509};
+use crate::x509::{
+    X509Name, X509Ref, X509StoreContextRef, X509VerifyError, X509VerifyResult, X509,
+};
 use crate::{cvt, cvt_0i, cvt_n, cvt_p, init};
 
 pub use crate::ssl::connector::{
@@ -3011,7 +3013,7 @@ impl SslRef {
             "This API is not supported for RPK"
         );
 
-        unsafe { X509VerifyResult::from_raw(ffi::SSL_get_verify_result(self.as_ptr()) as c_int) }
+        unsafe { X509VerifyError::from_raw(ffi::SSL_get_verify_result(self.as_ptr()) as c_int) }
     }
 
     /// Returns a shared reference to the SSL session.

boring/src/ssl/test/mod.rs

@@ -32,7 +32,7 @@ use crate::ssl::{
 };
 use crate::x509::store::X509StoreBuilder;
 use crate::x509::verify::X509CheckFlags;
-use crate::x509::{X509Name, X509StoreContext, X509VerifyResult, X509};
+use crate::x509::{X509Name, X509StoreContext, X509};
 
 mod private_key_method;
 mod server;
@@ -159,7 +159,7 @@ fn verify_trusted_get_error_ok() {
     client
         .ctx()
         .set_verify_callback(SslVerifyMode::PEER, |_, x509| {
-            assert_eq!(x509.error(), X509VerifyResult::OK);
+            assert_eq!(x509.verify_result(), Ok(()));
             true
         });
 
@@ -176,7 +176,7 @@ fn verify_trusted_get_error_err() {
     client
         .ctx()
         .set_verify_callback(SslVerifyMode::PEER, |_, x509| {
-            assert_ne!(x509.error(), X509VerifyResult::OK);
+            assert!(x509.verify_result().is_err());
             false
         });
 

boring/src/x509/mod.rs

@@ -90,13 +90,13 @@ impl X509StoreContextRef {
         }
     }
 
-    /// Returns the error code of the context.
+    /// Returns the verify result of the context.
     ///
     /// This corresponds to [`X509_STORE_CTX_get_error`].
     ///
     /// [`X509_STORE_CTX_get_error`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_STORE_CTX_get_error.html
-    pub fn error(&self) -> X509VerifyResult {
+    pub fn verify_result(&self) -> X509VerifyResult {
-        unsafe { X509VerifyResult::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) }
+        unsafe { X509VerifyError::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) }
     }
 
     /// Initializes this context with the given certificate, certificates chain and certificate
@@ -161,14 +161,20 @@ impl X509StoreContextRef {
         unsafe { cvt_n(ffi::X509_verify_cert(self.as_ptr())).map(|n| n != 0) }
     }
 
-    /// Set the error code of the context.
+    /// Set the verify result of the context.
     ///
     /// This corresponds to [`X509_STORE_CTX_set_error`].
     ///
     /// [`X509_STORE_CTX_set_error`]:  https://www.openssl.org/docs/man1.1.0/crypto/X509_STORE_CTX_set_error.html
     pub fn set_error(&mut self, result: X509VerifyResult) {
         unsafe {
-            ffi::X509_STORE_CTX_set_error(self.as_ptr(), result.as_raw());
+            ffi::X509_STORE_CTX_set_error(
+                self.as_ptr(),
+                result
+                    .err()
+                    .as_ref()
+                    .map_or(ffi::X509_V_OK, X509VerifyError::as_raw),
+            );
         }
     }
 
@@ -542,7 +548,7 @@ impl X509Ref {
     pub fn issued(&self, subject: &X509Ref) -> X509VerifyResult {
         unsafe {
             let r = ffi::X509_check_issued(self.as_ptr(), subject.as_ptr());
-            X509VerifyResult::from_raw(r)
+            X509VerifyError::from_raw(r)
         }
     }
 
@@ -1363,38 +1369,44 @@ impl X509ReqRef {
 }
 
 /// The result of peer certificate verification.
-#[derive(Copy, Clone, PartialEq, Eq)]
+pub type X509VerifyResult = Result<(), X509VerifyError>;
-pub struct X509VerifyResult(c_int);
 
-impl fmt::Debug for X509VerifyResult {
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub struct X509VerifyError(c_int);
+
+impl fmt::Debug for X509VerifyError {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.debug_struct("X509VerifyResult")
+        fmt.debug_struct("X509VerifyError")
             .field("code", &self.0)
             .field("error", &self.error_string())
             .finish()
     }
 }
 
-impl fmt::Display for X509VerifyResult {
+impl fmt::Display for X509VerifyError {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.write_str(self.error_string())
     }
 }
 
-impl Error for X509VerifyResult {}
+impl Error for X509VerifyError {}
 
-impl X509VerifyResult {
+impl X509VerifyError {
-    /// Creates an `X509VerifyResult` from a raw error number.
+    /// Creates an [`X509VerifyResult`] from a raw error number.
     ///
     /// # Safety
     ///
-    /// Some methods on `X509VerifyResult` are not thread safe if the error
+    /// Some methods on [`X509VerifyError`] are not thread safe if the error
     /// number is invalid.
     pub unsafe fn from_raw(err: c_int) -> X509VerifyResult {
-        X509VerifyResult(err)
+        if err == ffi::X509_V_OK {
+            Ok(())
+        } else {
+            Err(X509VerifyError(err))
+        }
     }
 
-    /// Return the integer representation of an `X509VerifyResult`.
+    /// Return the integer representation of an [`X509VerifyError`].
     #[allow(clippy::trivially_copy_pass_by_ref)]
     pub fn as_raw(&self) -> c_int {
         self.0
@@ -1417,8 +1429,7 @@ impl X509VerifyResult {
 }
 
 #[allow(missing_docs)] // no need to document the constants
-impl X509VerifyResult {
+impl X509VerifyError {
-    pub const OK: Self = Self(ffi::X509_V_OK);
     pub const UNSPECIFIED: Self = Self(ffi::X509_V_ERR_UNSPECIFIED);
     pub const UNABLE_TO_GET_ISSUER_CERT: Self = Self(ffi::X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT);
     pub const UNABLE_TO_GET_CRL: Self = Self(ffi::X509_V_ERR_UNABLE_TO_GET_CRL);

boring/src/x509/tests.rs

@@ -12,7 +12,7 @@ use crate::x509::extension::{
     SubjectKeyIdentifier,
 };
 use crate::x509::store::X509StoreBuilder;
-use crate::x509::{X509Extension, X509Name, X509Req, X509StoreContext, X509VerifyResult, X509};
+use crate::x509::{X509Extension, X509Name, X509Req, X509StoreContext, X509};
 
 fn pkey() -> PKey<Private> {
     let rsa = Rsa::generate(2048).unwrap();
@@ -363,8 +363,8 @@ fn issued() {
     let ca = include_bytes!("../../test/root-ca.pem");
     let ca = X509::from_pem(ca).unwrap();
 
-    assert_eq!(ca.issued(&cert), X509VerifyResult::OK);
+    assert_eq!(ca.issued(&cert), Ok(()));
-    assert_ne!(cert.issued(&cert), X509VerifyResult::OK);
+    assert!(cert.issued(&cert).is_err());
 }
 
 #[test]