From 26cf6e2fb14a5a9e7bf2a89f2370ad37b05f8282 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 11 Oct 2023 12:04:20 +0200 Subject: [PATCH] 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 | 6 ++--- boring/src/ssl/error.rs | 5 ++-- boring/src/ssl/mod.rs | 6 +++-- boring/src/ssl/test/mod.rs | 6 ++--- boring/src/x509/mod.rs | 47 +++++++++++++++++++++++-------------- boring/src/x509/tests.rs | 6 ++--- 6 files changed, 44 insertions(+), 32 deletions(-) diff --git a/boring/examples/mk_certs.rs b/boring/examples/mk_certs.rs index 217786e2..ce0c8492 100644 --- a/boring/examples/mk_certs.rs +++ b/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), 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!"), - ver_err => println!("Failed to verify certificate: {}", ver_err), + Ok(()) => println!("Certificate verified!"), + Err(ver_err) => println!("Failed to verify certificate: {}", ver_err), }; Ok(()) diff --git a/boring/src/ssl/error.rs b/boring/src/ssl/error.rs index e77ce291..0f75b818 100644 --- a/boring/src/ssl/error.rs +++ b/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)?, - verify => write!(f, "{}: cert verification failed - {}", prefix, verify)?, + Ok(()) => write!(f, "{}", prefix)?, + Err(verify) => write!(f, "{}: cert verification failed - {}", prefix, verify)?, } write!(f, " {}", s.error()) diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 724d7823..60a649e9 100644 --- a/boring/src/ssl/mod.rs +++ b/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. diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index a68c3dc7..53e5177b 100644 --- a/boring/src/ssl/test/mod.rs +++ b/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 }); diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 1033d364..30b5e725 100644 --- a/boring/src/x509/mod.rs +++ b/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 { - unsafe { X509VerifyResult::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) } + pub fn verify_result(&self) -> X509VerifyResult { + 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. +pub type X509VerifyResult = Result<(), X509VerifyError>; + #[derive(Copy, Clone, PartialEq, Eq)] -pub struct X509VerifyResult(c_int); +pub struct X509VerifyError(c_int); -impl fmt::Debug for X509VerifyResult { +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 { - /// Creates an `X509VerifyResult` from a raw error number. +impl X509VerifyError { + /// 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 { - pub const OK: Self = Self(ffi::X509_V_OK); +impl X509VerifyError { 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); diff --git a/boring/src/x509/tests.rs b/boring/src/x509/tests.rs index e437060b..e656873d 100644 --- a/boring/src/x509/tests.rs +++ b/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 { 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_ne!(cert.issued(&cert), X509VerifyResult::OK); + assert_eq!(ca.issued(&cert), Ok(())); + assert!(cert.issued(&cert).is_err()); } #[test]