Skip to content

Commit

Permalink
Introduce ForeignTypeExt and ForeignTypeRefExt
Browse files Browse the repository at this point in the history
Inspired by sfackler/rust-openssl#1345. These
traits remove a lot of error-prone boilerplate related to casting
`*const X::CType` to `*mut X::CType`, and offers APIs to gracefully
handle null pointers returned from the underlying c++ code.

As an example, not every X509 certificate has a subject key id
available. Instead of panicking when a user attempts to parse a subject
key id on a certificate that doesn't have one, we return an `Option`.

For now, I've implemented some getters in the x509 bindings using the
new traits, as that will avoid a breaking change. Before a 5.0 release,
the plan is to onboard all applicable bindings to this new trait.
  • Loading branch information
rushilmehra committed Aug 13, 2024
1 parent c88f335 commit a6b0ed0
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 25 deletions.
10 changes: 3 additions & 7 deletions boring/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3007,10 +3007,8 @@ impl SslRef {
);

unsafe {
let ptr = cert_store.as_ptr();
cvt(ffi::SSL_set0_verify_cert_store(self.as_ptr(), ptr) as c_int)?;
cvt(ffi::SSL_set0_verify_cert_store(self.as_ptr(), cert_store.as_ptr()) as c_int)?;
mem::forget(cert_store);

Ok(())
}
}
Expand Down Expand Up @@ -3855,10 +3853,8 @@ impl SslRef {
"This API is not supported for RPK"
);

unsafe {
ffi::SSL_set_client_CA_list(self.as_ptr(), list.as_ptr());
mem::forget(list);
}
unsafe { ffi::SSL_set_client_CA_list(self.as_ptr(), list.as_ptr()) }
mem::forget(list);
}

/// Sets the private key.
Expand Down
30 changes: 28 additions & 2 deletions boring/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::error::ErrorStack;
use foreign_types::{ForeignType, ForeignTypeRef};
use libc::{c_char, c_int, c_void};
use std::any::Any;
use std::panic::{self, AssertUnwindSafe};
use std::slice;

use crate::error::ErrorStack;

/// Wraps a user-supplied callback and a slot for panics thrown inside the callback (while FFI
/// frames are on the stack).
///
Expand Down Expand Up @@ -65,3 +65,29 @@ where
}
}
}

pub trait ForeignTypeExt: ForeignType {
unsafe fn from_ptr_opt(ptr: *mut Self::CType) -> Option<Self> {
if ptr.is_null() {
None
} else {
Some(Self::from_ptr(ptr))
}
}
}
impl<FT: ForeignType> ForeignTypeExt for FT {}

pub trait ForeignTypeRefExt: ForeignTypeRef {
unsafe fn from_const_ptr<'a>(ptr: *const Self::CType) -> &'a Self {
Self::from_ptr(ptr as *mut Self::CType)
}

unsafe fn from_const_ptr_opt<'a>(ptr: *const Self::CType) -> Option<&'a Self> {
if ptr.is_null() {
None
} else {
Some(Self::from_const_ptr(ptr as *mut Self::CType))
}
}
}
impl<FT: ForeignTypeRef> ForeignTypeRefExt for FT {}
25 changes: 9 additions & 16 deletions boring/src/x509/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use crate::pkey::{HasPrivate, HasPublic, PKey, PKeyRef, Public};
use crate::ssl::SslRef;
use crate::stack::{Stack, StackRef, Stackable};
use crate::string::OpensslString;
use crate::util::{ForeignTypeExt, ForeignTypeRefExt};
use crate::x509::verify::X509VerifyParamRef;
use crate::{cvt, cvt_n, cvt_p};

Expand Down Expand Up @@ -474,27 +475,19 @@ impl X509Ref {
}
}

/// Returns this certificate's subject key id.
///
/// This corresponds to [`X509_get0_subject_key_id`].
///
/// [`X509_get0_subject_key_id`]: https://docs.openssl.org/1.1.1/man3/X509_get_extension_flags/
pub fn subject_key_id(&self) -> &Asn1StringRef {
/// Returns this certificate's subject key id, if it exists.
pub fn subject_key_id(&self) -> Option<&Asn1StringRef> {
unsafe {
let name = ffi::X509_get0_subject_key_id(self.as_ptr());
Asn1StringRef::from_ptr(name as _)
let data = ffi::X509_get0_subject_key_id(self.as_ptr());
Asn1StringRef::from_const_ptr_opt(data)
}
}

/// Returns this certificate's authority key id.
///
/// This corresponds to [`X509_get0_authority_key_id`].
///
/// [`X509_get0_authority_key_id`]: https://docs.openssl.org/1.1.1/man3/X509_get_extension_flags/
pub fn authority_key_id(&self) -> &Asn1StringRef {
/// Returns this certificate's authority key id, if it exists.
pub fn authority_key_id(&self) -> Option<&Asn1StringRef> {
unsafe {
let name = ffi::X509_get0_authority_key_id(self.as_ptr());
Asn1StringRef::from_ptr(name as _)
let data = ffi::X509_get0_authority_key_id(self.as_ptr());
Asn1StringRef::from_const_ptr_opt(data)
}
}

Expand Down

0 comments on commit a6b0ed0

Please sign in to comment.