Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve X509VerifyResult #171

Merged
merged 2 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
128 changes: 107 additions & 21 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,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),
);
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand All @@ -1414,12 +1426,86 @@ impl X509VerifyResult {
str::from_utf8(CStr::from_ptr(s).to_bytes()).unwrap()
}
}
}

/// Successful peer certifiate verification.
pub const OK: X509VerifyResult = X509VerifyResult(ffi::X509_V_OK);
/// Application verification failure.
pub const APPLICATION_VERIFICATION: X509VerifyResult =
X509VerifyResult(ffi::X509_V_ERR_APPLICATION_VERIFICATION);
#[allow(missing_docs)] // no need to document the constants
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);
pub const UNABLE_TO_DECRYPT_CERT_SIGNATURE: Self =
Self(ffi::X509_V_ERR_UNABLE_TO_DECRYPT_CERT_SIGNATURE);
pub const UNABLE_TO_DECRYPT_CRL_SIGNATURE: Self =
Self(ffi::X509_V_ERR_UNABLE_TO_DECRYPT_CRL_SIGNATURE);
pub const UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY: Self =
Self(ffi::X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY);
pub const CERT_SIGNATURE_FAILURE: Self = Self(ffi::X509_V_ERR_CERT_SIGNATURE_FAILURE);
pub const CRL_SIGNATURE_FAILURE: Self = Self(ffi::X509_V_ERR_CRL_SIGNATURE_FAILURE);
pub const CERT_NOT_YET_VALID: Self = Self(ffi::X509_V_ERR_CERT_NOT_YET_VALID);
pub const CERT_HAS_EXPIRED: Self = Self(ffi::X509_V_ERR_CERT_HAS_EXPIRED);
pub const CRL_NOT_YET_VALID: Self = Self(ffi::X509_V_ERR_CRL_NOT_YET_VALID);
pub const CRL_HAS_EXPIRED: Self = Self(ffi::X509_V_ERR_CRL_HAS_EXPIRED);
pub const ERROR_IN_CERT_NOT_BEFORE_FIELD: Self =
Self(ffi::X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD);
pub const ERROR_IN_CERT_NOT_AFTER_FIELD: Self =
Self(ffi::X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD);
pub const ERROR_IN_CRL_LAST_UPDATE_FIELD: Self =
Self(ffi::X509_V_ERR_ERROR_IN_CRL_LAST_UPDATE_FIELD);
pub const ERROR_IN_CRL_NEXT_UPDATE_FIELD: Self =
Self(ffi::X509_V_ERR_ERROR_IN_CRL_NEXT_UPDATE_FIELD);
pub const OUT_OF_MEM: Self = Self(ffi::X509_V_ERR_OUT_OF_MEM);
pub const DEPTH_ZERO_SELF_SIGNED_CERT: Self = Self(ffi::X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT);
pub const SELF_SIGNED_CERT_IN_CHAIN: Self = Self(ffi::X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN);
pub const UNABLE_TO_GET_ISSUER_CERT_LOCALLY: Self =
Self(ffi::X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY);
pub const UNABLE_TO_VERIFY_LEAF_SIGNATURE: Self =
Self(ffi::X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE);
pub const CERT_CHAIN_TOO_LONG: Self = Self(ffi::X509_V_ERR_CERT_CHAIN_TOO_LONG);
pub const CERT_REVOKED: Self = Self(ffi::X509_V_ERR_CERT_REVOKED);
pub const INVALID_CA: Self = Self(ffi::X509_V_ERR_INVALID_CA);
pub const PATH_LENGTH_EXCEEDED: Self = Self(ffi::X509_V_ERR_PATH_LENGTH_EXCEEDED);
pub const INVALID_PURPOSE: Self = Self(ffi::X509_V_ERR_INVALID_PURPOSE);
pub const CERT_UNTRUSTED: Self = Self(ffi::X509_V_ERR_CERT_UNTRUSTED);
pub const CERT_REJECTED: Self = Self(ffi::X509_V_ERR_CERT_REJECTED);
pub const SUBJECT_ISSUER_MISMATCH: Self = Self(ffi::X509_V_ERR_SUBJECT_ISSUER_MISMATCH);
pub const AKID_SKID_MISMATCH: Self = Self(ffi::X509_V_ERR_AKID_SKID_MISMATCH);
pub const AKID_ISSUER_SERIAL_MISMATCH: Self = Self(ffi::X509_V_ERR_AKID_ISSUER_SERIAL_MISMATCH);
pub const KEYUSAGE_NO_CERTSIGN: Self = Self(ffi::X509_V_ERR_KEYUSAGE_NO_CERTSIGN);
pub const UNABLE_TO_GET_CRL_ISSUER: Self = Self(ffi::X509_V_ERR_UNABLE_TO_GET_CRL_ISSUER);
pub const UNHANDLED_CRITICAL_EXTENSION: Self =
Self(ffi::X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION);
pub const KEYUSAGE_NO_CRL_SIGN: Self = Self(ffi::X509_V_ERR_KEYUSAGE_NO_CRL_SIGN);
pub const UNHANDLED_CRITICAL_CRL_EXTENSION: Self =
Self(ffi::X509_V_ERR_UNHANDLED_CRITICAL_CRL_EXTENSION);
pub const INVALID_NON_CA: Self = Self(ffi::X509_V_ERR_INVALID_NON_CA);
pub const PROXY_PATH_LENGTH_EXCEEDED: Self = Self(ffi::X509_V_ERR_PROXY_PATH_LENGTH_EXCEEDED);
pub const KEYUSAGE_NO_DIGITAL_SIGNATURE: Self =
Self(ffi::X509_V_ERR_KEYUSAGE_NO_DIGITAL_SIGNATURE);
pub const PROXY_CERTIFICATES_NOT_ALLOWED: Self =
Self(ffi::X509_V_ERR_PROXY_CERTIFICATES_NOT_ALLOWED);
pub const INVALID_EXTENSION: Self = Self(ffi::X509_V_ERR_INVALID_EXTENSION);
pub const INVALID_POLICY_EXTENSION: Self = Self(ffi::X509_V_ERR_INVALID_POLICY_EXTENSION);
pub const NO_EXPLICIT_POLICY: Self = Self(ffi::X509_V_ERR_NO_EXPLICIT_POLICY);
pub const DIFFERENT_CRL_SCOPE: Self = Self(ffi::X509_V_ERR_DIFFERENT_CRL_SCOPE);
pub const UNSUPPORTED_EXTENSION_FEATURE: Self =
Self(ffi::X509_V_ERR_UNSUPPORTED_EXTENSION_FEATURE);
pub const UNNESTED_RESOURCE: Self = Self(ffi::X509_V_ERR_UNNESTED_RESOURCE);
pub const PERMITTED_VIOLATION: Self = Self(ffi::X509_V_ERR_PERMITTED_VIOLATION);
pub const EXCLUDED_VIOLATION: Self = Self(ffi::X509_V_ERR_EXCLUDED_VIOLATION);
pub const SUBTREE_MINMAX: Self = Self(ffi::X509_V_ERR_SUBTREE_MINMAX);
pub const APPLICATION_VERIFICATION: Self = Self(ffi::X509_V_ERR_APPLICATION_VERIFICATION);
pub const UNSUPPORTED_CONSTRAINT_TYPE: Self = Self(ffi::X509_V_ERR_UNSUPPORTED_CONSTRAINT_TYPE);
pub const UNSUPPORTED_CONSTRAINT_SYNTAX: Self =
Self(ffi::X509_V_ERR_UNSUPPORTED_CONSTRAINT_SYNTAX);
pub const UNSUPPORTED_NAME_SYNTAX: Self = Self(ffi::X509_V_ERR_UNSUPPORTED_NAME_SYNTAX);
pub const CRL_PATH_VALIDATION_ERROR: Self = Self(ffi::X509_V_ERR_CRL_PATH_VALIDATION_ERROR);
pub const HOSTNAME_MISMATCH: Self = Self(ffi::X509_V_ERR_HOSTNAME_MISMATCH);
pub const EMAIL_MISMATCH: Self = Self(ffi::X509_V_ERR_EMAIL_MISMATCH);
pub const IP_ADDRESS_MISMATCH: Self = Self(ffi::X509_V_ERR_IP_ADDRESS_MISMATCH);
pub const INVALID_CALL: Self = Self(ffi::X509_V_ERR_INVALID_CALL);
pub const STORE_LOOKUP: Self = Self(ffi::X509_V_ERR_STORE_LOOKUP);
pub const NAME_CONSTRAINTS_WITHOUT_SANS: Self =
Self(ffi::X509_V_ERR_NAME_CONSTRAINTS_WITHOUT_SANS);
}

foreign_type_and_impl_send_sync! {
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