Skip to content

Commit

Permalink
Add set_cert_verify_callback (SSL_CTX_set_cert_verify)
Browse files Browse the repository at this point in the history
Add a wrapper for `SSL_CTX_set_cert_verify`, which allows consumers to
override the default certificate verification behavior.

The binding resembles `SSL_CTX_set_verify`'s.

See
https://docs.openssl.org/master/man3/SSL_CTX_set_cert_verify_callback/
for more details.
  • Loading branch information
semaj-cf authored and rushilmehra committed Oct 22, 2024
1 parent ec3b412 commit bb373e5
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 0 deletions.
28 changes: 28 additions & 0 deletions boring/src/ssl/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,34 @@ where
unsafe { raw_custom_verify_callback(ssl, out_alert, callback) }
}

pub(super) unsafe extern "C" fn raw_cert_verify<F>(
x509_ctx: *mut ffi::X509_STORE_CTX,
_arg: *mut c_void,
) -> c_int
where
F: Fn(&mut X509StoreContextRef) -> bool + 'static + Sync + Send,
{
// SAFETY: boring provides valid inputs.
let ctx = unsafe { X509StoreContextRef::from_ptr_mut(x509_ctx) };

let ssl_idx = X509StoreContext::ssl_idx().expect("BUG: store context ssl index missing");
let verify_idx = SslContext::cached_ex_index::<F>();

let verify = ctx
.ex_data(ssl_idx)
.expect("BUG: store context missing ssl")
.ssl_context()
.ex_data(verify_idx)
.expect("BUG: verify callback missing");

// SAFETY: The callback won't outlive the context it's associated with
// because there is no way to get a mutable reference to the `SslContext`,
// so the callback can't replace itself.
let verify = unsafe { &*(verify as *const F) };

verify(ctx) as c_int
}

pub(super) unsafe extern "C" fn ssl_raw_custom_verify<F>(
ssl: *mut ffi::SSL,
out_alert: *mut u8,
Expand Down
43 changes: 43 additions & 0 deletions boring/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,49 @@ impl SslContextBuilder {
self.ctx.as_ptr()
}

/// Registers a certificate verification callback that replaces the default verification
/// process.
///
/// The callback returns true if the certificate chain is valid, and false if not.
/// A viable verification result value (either `Ok(())` or an `Err(X509VerifyError)`) must be
/// reflected in the error member of `X509StoreContextRef`, which can be done by calling
/// `X509StoreContextRef::set_error`. However, the callback's return value determines
/// whether the chain is accepted or not.
///
/// *Warning*: Providing a complete verification procedure is a complex task. See
/// https://docs.openssl.org/master/man3/SSL_CTX_set_cert_verify_callback/#notes for more
/// information.
///
/// TODO: Add the ability to unset the callback by either adding a new function or wrapping the
/// callback in an `Option`.
///
/// # Panics
///
/// This method panics if this `SslContext` is associated with a RPK context.
#[corresponds(SSL_CTX_set_cert_verify_callback)]
pub fn set_cert_verify_callback<F>(&mut self, callback: F)
where
F: Fn(&mut X509StoreContextRef) -> bool + 'static + Sync + Send,
{
#[cfg(feature = "rpk")]
assert!(!self.is_rpk, "This API is not supported for RPK");

// NOTE(jlarisch): Q: Why don't we wrap the callback in an Arc, since
// `set_verify_callback` does?
// A: I don't think that Arc is necessary, and I don't think one is necessary here.
// There's no way to get a mutable reference to the `Ssl` or `SslContext`, which
// is what you need to register a new callback.
// See the NOTE in `ssl_raw_verify` for confirmation.
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
unsafe {
ffi::SSL_CTX_set_cert_verify_callback(
self.as_ptr(),
Some(raw_cert_verify::<F>),
ptr::null_mut(),
);
}
}

/// Configures the certificate verification method for new connections.
#[corresponds(SSL_CTX_set_verify)]
pub fn set_verify(&mut self, mode: SslVerifyMode) {
Expand Down
99 changes: 99 additions & 0 deletions boring/src/ssl/test/cert_verify.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
use crate::hash::MessageDigest;
use crate::ssl::test::Server;
use crate::ssl::SslVerifyMode;

#[test]
fn error_when_trusted_but_callback_returns_false() {
let mut server = Server::builder();
server.should_error();
let server = server.build();
let mut client = server.client_with_root_ca();
client.ctx().set_verify(SslVerifyMode::PEER);
client.ctx().set_cert_verify_callback(|x509| {
// The cert is OK
assert!(x509.verify_cert().unwrap());
assert!(x509.current_cert().is_some());
assert!(x509.verify_result().is_ok());
// But we return false
false
});

client.connect_err();
}

#[test]
fn no_error_when_untrusted_but_callback_returns_true() {
let server = Server::builder().build();
let mut client = server.client();
client.ctx().set_verify(SslVerifyMode::PEER);
client.ctx().set_cert_verify_callback(|x509| {
// The cert is not OK
assert!(!x509.verify_cert().unwrap());
assert!(x509.current_cert().is_some());
assert!(x509.verify_result().is_err());
// But we return true
true
});

client.connect();
}

#[test]
fn no_error_when_trusted_and_callback_returns_true() {
let server = Server::builder().build();
let mut client = server.client_with_root_ca();
client.ctx().set_verify(SslVerifyMode::PEER);
client.ctx().set_cert_verify_callback(|x509| {
// The cert is OK
assert!(x509.verify_cert().unwrap());
assert!(x509.current_cert().is_some());
assert!(x509.verify_result().is_ok());
// And we return true
true
});
client.connect();
}

#[test]
fn callback_receives_correct_certificate() {
let server = Server::builder().build();
let mut client = server.client();
let expected = "59172d9313e84459bcff27f967e79e6e9217e584";
client.ctx().set_verify(SslVerifyMode::PEER);
client.ctx().set_cert_verify_callback(move |x509| {
assert!(!x509.verify_cert().unwrap());
assert!(x509.current_cert().is_some());
assert!(x509.verify_result().is_err());
let cert = x509.current_cert().unwrap();
let digest = cert.digest(MessageDigest::sha1()).unwrap();
assert_eq!(hex::encode(digest), expected);
true
});

client.connect();
}

#[test]
fn callback_receives_correct_chain() {
let server = Server::builder().build();
let mut client = server.client_with_root_ca();
let leaf_sha1 = "59172d9313e84459bcff27f967e79e6e9217e584";
let root_sha1 = "c0cbdf7cdd03c9773e5468e1f6d2da7d5cbb1875";
client.ctx().set_verify(SslVerifyMode::PEER);
client.ctx().set_cert_verify_callback(move |x509| {
assert!(x509.verify_cert().unwrap());
assert!(x509.current_cert().is_some());
assert!(x509.verify_result().is_ok());
let chain = x509.chain().unwrap();
assert!(chain.len() == 2);
let leaf_cert = chain.get(0).unwrap();
let leaf_digest = leaf_cert.digest(MessageDigest::sha1()).unwrap();
assert_eq!(hex::encode(leaf_digest), leaf_sha1);
let root_cert = chain.get(1).unwrap();
let root_digest = root_cert.digest(MessageDigest::sha1()).unwrap();
assert_eq!(hex::encode(root_digest), root_sha1);
true
});

client.connect();
}
1 change: 1 addition & 0 deletions boring/src/ssl/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::x509::{X509Name, X509};
#[cfg(not(feature = "fips"))]
use super::CompliancePolicy;

mod cert_verify;
mod custom_verify;
mod private_key_method;
mod server;
Expand Down

0 comments on commit bb373e5

Please sign in to comment.