Skip to content

Commit

Permalink
Use X509_STORE for root certificate storage
Browse files Browse the repository at this point in the history
In {client,server}.c, see that the effects of `SSL_CTX_load_verify_file`
can be observed via `SSL_CTX_get_cert_store`
  • Loading branch information
ctz committed May 13, 2024
1 parent 93b1339 commit 31cef37
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 29 deletions.
35 changes: 14 additions & 21 deletions rustls-libssl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustls::pki_types::{CertificateDer, ServerName};
use rustls::server::{Accepted, Acceptor};
use rustls::{
CipherSuite, ClientConfig, ClientConnection, Connection, HandshakeKind, ProtocolVersion,
RootCertStore, ServerConfig, SignatureScheme, SupportedProtocolVersion,
ServerConfig, SignatureScheme, SupportedProtocolVersion,
};

use not_thread_safe::NotThreadSafe;
Expand Down Expand Up @@ -413,7 +413,6 @@ pub struct SslContext {
raw_options: u64,
verify_mode: VerifyMode,
verify_depth: c_int,
verify_roots: RootCertStore,
verify_x509_store: x509::OwnedX509Store,
alpn: Vec<Vec<u8>>,
default_cert_file: Option<PathBuf>,
Expand All @@ -435,7 +434,6 @@ impl SslContext {
raw_options: 0,
verify_mode: VerifyMode::default(),
verify_depth: -1,
verify_roots: RootCertStore::empty(),
verify_x509_store: OwnedX509Store::default(),
alpn: vec![],
default_cert_file: None,
Expand Down Expand Up @@ -604,12 +602,7 @@ impl SslContext {
&mut self,
certs: Vec<CertificateDer<'static>>,
) -> Result<(), error::Error> {
for c in certs {
self.verify_roots
.add(c)
.map_err(error::Error::from_rustls)?;
}
Ok(())
self.verify_x509_store.add(certs)
}

fn get_x509_store(&self) -> *mut X509_STORE {
Expand Down Expand Up @@ -714,8 +707,8 @@ struct Ssl {
mode: ConnMode,
verify_mode: VerifyMode,
verify_depth: c_int,
verify_roots: RootCertStore,
verify_server_name: Option<ServerName<'static>>,
verify_x509_store: x509::OwnedX509Store,
alpn: Vec<Vec<u8>>,
alpn_callback: callbacks::AlpnCallbackConfig,
cert_callback: callbacks::CertCallbackConfig,
Expand Down Expand Up @@ -754,8 +747,8 @@ impl Ssl {
mode: inner.method.mode(),
verify_mode: inner.verify_mode,
verify_depth: inner.verify_depth,
verify_roots: Self::load_verify_certs(inner)?,
verify_server_name: None,
verify_x509_store: Self::load_verify_certs(inner)?,
alpn: inner.alpn.clone(),
alpn_callback: inner.alpn_callback.clone(),
cert_callback: inner.cert_callback.clone(),
Expand Down Expand Up @@ -989,7 +982,7 @@ impl Ssl {

let provider = Arc::new(provider::default_provider());
let verifier = Arc::new(verifier::ServerVerifier::new(
self.verify_roots.clone().into(),
self.verify_x509_store.clone(),
provider.clone(),
self.verify_mode,
&self.verify_server_name,
Expand Down Expand Up @@ -1074,7 +1067,7 @@ impl Ssl {
let provider = Arc::new(provider::default_provider());
let verifier = Arc::new(
verifier::ClientVerifier::new(
self.verify_roots.clone().into(),
&self.verify_x509_store,
provider.clone(),
self.verify_mode,
)
Expand Down Expand Up @@ -1377,20 +1370,20 @@ impl Ssl {
}
}

fn load_verify_certs(ctx: &SslContext) -> Result<RootCertStore, error::Error> {
let mut verify_roots = ctx.verify_roots.clone();

fn load_verify_certs(ctx: &SslContext) -> Result<x509::OwnedX509Store, error::Error> {
// If verify_roots isn't empty then it was configured with `SSL_CTX_load_verify_file`
// or `SSL_CTX_load_verify_dir` and we should use it as-is.
if !ctx.verify_roots.is_empty() {
return Ok(verify_roots);
if !ctx.verify_x509_store.is_empty() {
return Ok(ctx.verify_x509_store.clone());
}

// Otherwise, try to load the default cert file or cert dir.
let mut verify_roots = x509::OwnedX509Store::default();

if let Some(default_cert_file) = &ctx.default_cert_file {
verify_roots.add_parsable_certificates(x509::load_certs(
verify_roots.add(x509::load_certs(
vec![default_cert_file.to_path_buf()].into_iter(),
)?);
)?)?;
} else if let Some(default_cert_dir) = &ctx.default_cert_dir {
let entries = match fs::read_dir(default_cert_dir) {
Ok(iter) => iter,
Expand All @@ -1399,7 +1392,7 @@ impl Ssl {
.filter_map(|entry| entry.ok())
.map(|dir_entry| dir_entry.path());

verify_roots.add_parsable_certificates(x509::load_certs(entries)?);
verify_roots.add(x509::load_certs(entries)?)?;
}

Ok(verify_roots)
Expand Down
10 changes: 10 additions & 0 deletions rustls-libssl/src/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ pub extern "C" fn X509_STORE_new() -> *mut X509_STORE {
Box::into_raw(Box::new(X509_STORE(())))
}

#[no_mangle]
pub extern "C" fn X509_STORE_get0_objects(s: *mut X509_STORE) -> *mut c_void {
ptr::null_mut()
}

#[no_mangle]
pub extern "C" fn OPENSSL_sk_num(sk: *mut c_void) -> c_int {
0
}

#[no_mangle]
pub extern "C" fn X509_STORE_free(ptr: *mut X509_STORE) {
if ptr.is_null() {
Expand Down
17 changes: 10 additions & 7 deletions rustls-libssl/src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ use rustls::{
pki_types::{CertificateDer, ServerName, UnixTime},
server::danger::{ClientCertVerified, ClientCertVerifier},
server::{ParsedCertificate, WebPkiClientVerifier},
CertificateError, DigitallySignedStruct, DistinguishedName, Error, RootCertStore,
SignatureScheme,
CertificateError, DigitallySignedStruct, DistinguishedName, Error, SignatureScheme,
};

use crate::x509::OwnedX509Store;
use crate::VerifyMode;

/// This is a verifier that implements the selection of bad ideas from OpenSSL:
Expand All @@ -29,7 +29,7 @@ use crate::VerifyMode;
/// - that the behaviour defaults to verifying nothing
#[derive(Debug)]
pub struct ServerVerifier {
root_store: Arc<RootCertStore>,
x509_store: OwnedX509Store,

provider: Arc<CryptoProvider>,

Expand All @@ -47,13 +47,13 @@ pub struct ServerVerifier {

impl ServerVerifier {
pub fn new(
root_store: Arc<RootCertStore>,
x509_store: OwnedX509Store,
provider: Arc<CryptoProvider>,
mode: VerifyMode,
hostname: &Option<ServerName<'static>>,
) -> Self {
Self {
root_store,
x509_store,
provider,
verify_hostname: hostname.clone(),
mode,
Expand All @@ -77,10 +77,11 @@ impl ServerVerifier {
now: UnixTime,
) -> Result<(), Error> {
let end_entity = ParsedCertificate::try_from(end_entity)?;
let root_store = self.x509_store.root_store()?;

verify_server_cert_signed_by_trust_anchor(
&end_entity,
&self.root_store,
&root_store,
intermediates,
now,
self.provider.signature_verification_algorithms.all,
Expand Down Expand Up @@ -169,10 +170,12 @@ pub struct ClientVerifier {

impl ClientVerifier {
pub fn new(
root_store: Arc<RootCertStore>,
x509_store: &OwnedX509Store,
provider: Arc<CryptoProvider>,
mode: VerifyMode,
) -> Result<Self, Error> {
let root_store = x509_store.root_store()?;

let (parent, initial_result) = if !mode.server_must_attempt_client_auth() {
(Ok(WebPkiClientVerifier::no_client_auth()), X509_V_OK)
} else {
Expand Down
67 changes: 66 additions & 1 deletion rustls-libssl/src/x509.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
use core::ffi::{c_int, c_long, c_void};
use core::{ptr, slice};
use std::path::PathBuf;
use std::sync::Arc;
use std::{fs, io};

use openssl_sys::{
d2i_X509, i2d_X509, stack_st_X509, OPENSSL_free, OPENSSL_sk_new_null, OPENSSL_sk_num,
OPENSSL_sk_push, OPENSSL_sk_value, X509_STORE_free, X509_STORE_new, X509_free, OPENSSL_STACK,
OPENSSL_sk_push, OPENSSL_sk_value, X509_STORE_add_cert, X509_STORE_free,
X509_STORE_get0_objects, X509_STORE_get1_all_certs, X509_STORE_new, X509_free, OPENSSL_STACK,
X509, X509_STORE,
};
use rustls::pki_types::CertificateDer;
use rustls::RootCertStore;

use crate::error::Error;

Expand Down Expand Up @@ -232,6 +235,7 @@ impl Drop for OwnedX509 {
}
}

#[derive(Debug)]
pub struct OwnedX509Store {
raw: *mut X509_STORE,
}
Expand All @@ -245,6 +249,54 @@ impl OwnedX509Store {
pub fn pointer(&self) -> *mut X509_STORE {
self.raw
}

pub fn len(&self) -> usize {
match unsafe { OPENSSL_sk_num(X509_STORE_get0_objects(self.raw) as *const OPENSSL_STACK) } {
-1 | 0 => 0,
i => i as usize,
}
}

pub fn is_empty(&self) -> bool {
self.len() == 0
}

pub fn add(
&mut self,
cert_ders: impl IntoIterator<Item = CertificateDer<'static>>,
) -> Result<(), Error> {
for cert_der in cert_ders {
let item = OwnedX509::parse_der(cert_der.as_ref())
.ok_or_else(|| Error::bad_data("cannot parse certificate"))?;
match unsafe { X509_STORE_add_cert(self.raw, item.borrow_ref()) } {
1 => {}
_ => {
return Err(Error::bad_data("cannot X509_STORE_add_cert"));
}
}
}

Ok(())
}

pub fn root_store(&self) -> Result<Arc<RootCertStore>, rustls::Error> {
let ptr = unsafe { X509_STORE_get1_all_certs(self.raw) };

if ptr.is_null() {
return Err(rustls::Error::General(
"cannot retrieve trusted certs".to_string(),
));
}

let certs = OwnedX509Stack::new(ptr);
let mut ret = RootCertStore::empty();

for i in 0..certs.len() {
ret.add(certs.item(i).der_bytes().into())?;
}

Ok(ret.into())
}
}

impl Default for OwnedX509Store {
Expand All @@ -255,6 +307,15 @@ impl Default for OwnedX509Store {
}
}

impl Clone for OwnedX509Store {
fn clone(&self) -> Self {
unsafe {
X509_STORE_up_ref(self.raw);
}
Self { raw: self.raw }
}
}

impl Drop for OwnedX509Store {
fn drop(&mut self) {
unsafe {
Expand All @@ -263,6 +324,9 @@ impl Drop for OwnedX509Store {
}
}

unsafe impl Send for OwnedX509Store {}
unsafe impl Sync for OwnedX509Store {}

pub(crate) fn load_certs<'a>(
file_names: impl Iterator<Item = PathBuf>,
) -> Result<Vec<CertificateDer<'a>>, Error> {
Expand Down Expand Up @@ -294,4 +358,5 @@ extern "C" {
);
fn OPENSSL_sk_dup(st: *const OPENSSL_STACK) -> *mut OPENSSL_STACK;
fn X509_up_ref(x: *mut X509) -> c_int;
fn X509_STORE_up_ref(xs: *mut X509_STORE) -> c_int;
}
4 changes: 4 additions & 0 deletions rustls-libssl/tests/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ int main(int argc, char **argv) {
dump_openssl_error_stack();
assert(SSL_CTX_get_verify_mode(ctx) == SSL_VERIFY_PEER);
assert(SSL_CTX_get_verify_callback(ctx) == NULL);
TRACE(sk_X509_OBJECT_num(
X509_STORE_get0_objects(SSL_CTX_get_cert_store(ctx))));
TRACE(SSL_CTX_load_verify_file(ctx, cacert));
dump_openssl_error_stack();
TRACE(sk_X509_OBJECT_num(
X509_STORE_get0_objects(SSL_CTX_get_cert_store(ctx))));
}
printf("SSL_CTX_get_verify_depth default %d\n",
SSL_CTX_get_verify_depth(ctx));
Expand Down
4 changes: 4 additions & 0 deletions rustls-libssl/tests/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,12 @@ int main(int argc, char **argv) {
if (strcmp(cacert, "unauth") != 0) {
SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
dump_openssl_error_stack();
TRACE(sk_X509_OBJECT_num(
X509_STORE_get0_objects(SSL_CTX_get_cert_store(ctx))));
TRACE(SSL_CTX_load_verify_file(ctx, cacert));
dump_openssl_error_stack();
TRACE(sk_X509_OBJECT_num(
X509_STORE_get0_objects(SSL_CTX_get_cert_store(ctx))));
} else {
printf("client auth disabled\n");
}
Expand Down

0 comments on commit 31cef37

Please sign in to comment.