Skip to content

Commit

Permalink
Change X509VerifyResult to Result<(), X509VerifyError>
Browse files Browse the repository at this point in the history
This commit separates X509VerifyResult::OK from the rest
of the codes that actually represent errors, using
a Result type as usual.
  • Loading branch information
nox committed Oct 11, 2023
1 parent 31ca5e7 commit 08c333a
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 32 deletions.
6 changes: 3 additions & 3 deletions boring/examples/mk_certs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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(())
Expand Down
5 changes: 2 additions & 3 deletions boring/src/ssl/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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())
Expand Down
6 changes: 4 additions & 2 deletions boring/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions boring/src/ssl/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
});

Expand All @@ -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
});

Expand Down
44 changes: 26 additions & 18 deletions boring/src/x509/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -161,14 +161,17 @@ 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),
);
}
}

Expand Down Expand Up @@ -542,7 +545,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)
}
}

Expand Down Expand Up @@ -1363,38 +1366,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
Expand All @@ -1417,8 +1426,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);
Expand Down
6 changes: 3 additions & 3 deletions boring/src/x509/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit 08c333a

Please sign in to comment.