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

impl SSL_CTX_set_default_verify_paths and friends #2

Merged
merged 7 commits into from
Apr 5, 2024
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
1 change: 1 addition & 0 deletions rustls-libssl/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
target/
/.idea
7 changes: 7 additions & 0 deletions rustls-libssl/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rustls-libssl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ crate-type = ["cdylib"]
[dependencies]
env_logger = "0.10"
log = "0.4"
openssl-probe = "0.1"
openssl-sys = "0.9.98"
rustls = "0.23"
rustls-pemfile = "2"
8 changes: 4 additions & 4 deletions rustls-libssl/MATRIX.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@
| `SSL_CTX_set_default_passwd_cb` | :white_check_mark: | | :exclamation: [^stub] |
| `SSL_CTX_set_default_passwd_cb_userdata` | :white_check_mark: | | :exclamation: [^stub] |
| `SSL_CTX_set_default_read_buffer_len` | | | |
| `SSL_CTX_set_default_verify_dir` | | | |
| `SSL_CTX_set_default_verify_file` | | | |
| `SSL_CTX_set_default_verify_paths` | | | |
| `SSL_CTX_set_default_verify_store` | | | |
| `SSL_CTX_set_default_verify_dir` | | | :white_check_mark: |
| `SSL_CTX_set_default_verify_file` | | | :white_check_mark: |
| `SSL_CTX_set_default_verify_paths` | | | :white_check_mark: |
| `SSL_CTX_set_default_verify_store` | | | :exclamation: [^stub] |
| `SSL_CTX_set_ex_data` | | :white_check_mark: | :exclamation: [^stub] |
| `SSL_CTX_set_generate_session_id` | | | |
| `SSL_CTX_set_info_callback` | | :white_check_mark: | |
Expand Down
4 changes: 4 additions & 0 deletions rustls-libssl/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ const ENTRYPOINTS: &[&str] = &[
"SSL_CTX_set_ciphersuites",
"SSL_CTX_set_default_passwd_cb",
"SSL_CTX_set_default_passwd_cb_userdata",
"SSL_CTX_set_default_verify_dir",
"SSL_CTX_set_default_verify_file",
"SSL_CTX_set_default_verify_paths",
"SSL_CTX_set_default_verify_store",
"SSL_CTX_set_ex_data",
"SSL_CTX_set_keylog_callback",
"SSL_CTX_set_msg_callback",
Expand Down
86 changes: 64 additions & 22 deletions rustls-libssl/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use core::{mem, ptr};
use std::os::raw::{c_char, c_int, c_long, c_uchar, c_uint, c_void};
use std::sync::Mutex;
use std::{fs, io, path::PathBuf};
use std::{fs, path::PathBuf};

use openssl_sys::{
stack_st_X509, OPENSSL_malloc, EVP_PKEY, X509, X509_STORE, X509_STORE_CTX,
Expand All @@ -19,6 +19,7 @@ use crate::ffi::{
free_arc, str_from_cstring, to_arc_mut_ptr, try_clone_arc, try_from, try_mut_slice_int,
try_ref_from_ptr, try_slice, try_slice_int, try_str, Castable, OwnershipArc, OwnershipRef,
};
use crate::x509::load_certs;
use crate::ShutdownResult;

/// Makes a entry function definition.
Expand Down Expand Up @@ -218,23 +219,10 @@ entry! {
}

fn load_verify_files(ctx: &Mutex<SSL_CTX>, file_names: impl Iterator<Item = PathBuf>) -> c_int {
let mut certs = Vec::new();
for file_name in file_names {
let mut file_reader = match fs::File::open(file_name.clone()) {
Ok(content) => io::BufReader::new(content),
Err(err) => return Error::from_io(err).raise().into(),
};

for cert in rustls_pemfile::certs(&mut file_reader) {
match cert {
Ok(cert) => certs.push(cert),
Err(err) => {
log::trace!("Failed to parse {file_name:?}: {err:?}");
return Error::from_io(err).raise().into();
}
};
}
}
let certs = match load_certs(file_names) {
Err(e) => return e.into(),
Ok(certs) => certs,
};

match ctx
.lock()
Expand All @@ -246,6 +234,48 @@ fn load_verify_files(ctx: &Mutex<SSL_CTX>, file_names: impl Iterator<Item = Path
}
}

entry! {
pub fn _SSL_CTX_set_default_verify_paths(ctx: *mut SSL_CTX) -> c_int {
let ctx = try_clone_arc!(ctx);
match ctx
.lock()
.map_err(|_| Error::cannot_lock())
.map(|mut ctx| ctx.set_default_verify_paths())
{
Err(e) => e.raise().into(),
Ok(()) => C_INT_SUCCESS,
}
}
}
Comment on lines +237 to +249
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern of casting an arc from the pointer, juggling the lock, invoking a fn with the unlocked ctx, and then juggling a c_int error return might be worth refactoring into something shared 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed. Should be possible to take a closure that gets a &mut SslContext. Seems like a good refactoring for later.


entry! {
pub fn _SSL_CTX_set_default_verify_dir(ctx: *mut SSL_CTX) -> c_int {
let ctx = try_clone_arc!(ctx);
match ctx
.lock()
.map_err(|_| Error::cannot_lock())
.map(|mut ctx| ctx.set_default_verify_dir())
{
Err(e) => e.raise().into(),
Ok(()) => C_INT_SUCCESS,
}
}
}

entry! {
pub fn _SSL_CTX_set_default_verify_file(ctx: *mut SSL_CTX) -> c_int {
let ctx = try_clone_arc!(ctx);
match ctx
.lock()
.map_err(|_| Error::cannot_lock())
.map(|mut ctx| ctx.set_default_verify_file())
{
Err(e) => e.raise().into(),
Ok(()) => C_INT_SUCCESS,
}
}
}

entry! {
pub fn _SSL_CTX_load_verify_file(ctx: *mut SSL_CTX, ca_file: *const c_char) -> c_int {
let ctx = try_clone_arc!(ctx);
Expand Down Expand Up @@ -311,10 +341,17 @@ entry! {
pub fn _SSL_new(ctx: *mut SSL_CTX) -> *mut SSL {
let ctx = try_clone_arc!(ctx);

ctx.lock()
.ok()
.map(|c| to_arc_mut_ptr(Mutex::new(crate::Ssl::new(ctx.clone(), &c))))
.unwrap_or_else(ptr::null_mut)
let ssl_ctx = match ctx.lock().ok() {
Some(ssl_ctx) => ssl_ctx,
None => return ptr::null_mut(),
};

let ssl = match crate::Ssl::new(ctx.clone(), &ssl_ctx).ok() {
Some(ssl) => ssl,
None => return ptr::null_mut(),
};

to_arc_mut_ptr(Mutex::new(ssl))
}
}

Expand Down Expand Up @@ -1012,6 +1049,11 @@ entry_stub! {
) -> c_int;
}

// The SSL_CTX X509_STORE isn't being meaningfully used yet.
entry_stub! {
pub fn _SSL_CTX_set_default_verify_store(_ctx: *mut SSL_CTX) -> c_int;
}

pub struct SSL_SESSION;

entry_stub! {
Expand Down
62 changes: 58 additions & 4 deletions rustls-libssl/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use core::ffi::{c_int, CStr};
use std::fs;
use std::io::{ErrorKind, Read, Write};
use std::path::PathBuf;
use std::sync::{Arc, Mutex};

use openssl_probe::ProbeResult;
use openssl_sys::{
SSL_ERROR_NONE, SSL_ERROR_SSL, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE, X509_STORE,
X509_V_ERR_UNSPECIFIED,
Expand Down Expand Up @@ -205,6 +208,8 @@ pub struct SslContext {
verify_roots: RootCertStore,
verify_x509_store: x509::OwnedX509Store,
alpn: Vec<Vec<u8>>,
default_cert_file: Option<PathBuf>,
default_cert_dir: Option<PathBuf>,
}

impl SslContext {
Expand All @@ -216,6 +221,8 @@ impl SslContext {
verify_roots: RootCertStore::empty(),
verify_x509_store: x509::OwnedX509Store::new(),
alpn: vec![],
default_cert_file: None,
default_cert_dir: None,
}
}

Expand All @@ -237,6 +244,25 @@ impl SslContext {
self.verify_mode = mode;
}

fn set_default_verify_paths(&mut self) {
let ProbeResult {
cert_file,
cert_dir,
} = openssl_probe::probe();
self.default_cert_file = cert_file;
self.default_cert_dir = cert_dir;
}

fn set_default_verify_dir(&mut self) {
let ProbeResult { cert_dir, .. } = openssl_probe::probe();
self.default_cert_dir = cert_dir;
}

fn set_default_verify_file(&mut self) {
let ProbeResult { cert_file, .. } = openssl_probe::probe();
self.default_cert_file = cert_file;
}

fn add_trusted_certs(
&mut self,
certs: Vec<CertificateDer<'static>>,
Expand Down Expand Up @@ -297,13 +323,13 @@ struct Ssl {
}

impl Ssl {
fn new(ctx: Arc<Mutex<SslContext>>, inner: &SslContext) -> Self {
Self {
fn new(ctx: Arc<Mutex<SslContext>>, inner: &SslContext) -> Result<Self, error::Error> {
Ok(Self {
ctx,
raw_options: inner.raw_options,
mode: inner.method.mode(),
verify_mode: inner.verify_mode,
verify_roots: inner.verify_roots.clone(),
verify_roots: Self::load_verify_certs(inner)?,
verify_server_name: None,
alpn: inner.alpn.clone(),
sni_server_name: None,
Expand All @@ -313,7 +339,7 @@ impl Ssl {
peer_cert: None,
peer_cert_chain: None,
shutdown_flags: ShutdownFlags::default(),
}
})
}

fn get_options(&self) -> u64 {
Expand Down Expand Up @@ -633,6 +659,34 @@ impl Ssl {
None => SSL_ERROR_SSL,
}
}

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

// 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);
}

// Otherwise, try to load the default cert file or cert dir.
if let Some(default_cert_file) = &ctx.default_cert_file {
verify_roots.add_parsable_certificates(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,
Err(err) => return Err(error::Error::from_io(err).raise()),
}
.filter_map(|entry| entry.ok())
.map(|dir_entry| dir_entry.path());

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

Ok(verify_roots)
}
}

#[derive(Default)]
Expand Down
28 changes: 28 additions & 0 deletions rustls-libssl/src/x509.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
use core::ffi::{c_int, c_long, c_void};
use core::ptr;
use std::path::PathBuf;
use std::{fs, io};

use openssl_sys::{
d2i_X509, stack_st_X509, OPENSSL_sk_new_null, OPENSSL_sk_push, X509_STORE_free, X509_STORE_new,
X509_free, OPENSSL_STACK, X509, X509_STORE,
};
use rustls::pki_types::CertificateDer;

use crate::error::Error;

/// Safe, owning wrapper around an OpenSSL `STACK_OF(X509)` object.
///
Expand Down Expand Up @@ -122,6 +127,29 @@ impl Drop for OwnedX509Store {
}
}

pub(crate) fn load_certs<'a>(
file_names: impl Iterator<Item = PathBuf>,
) -> Result<Vec<CertificateDer<'a>>, Error> {
let mut certs = Vec::new();
for file_name in file_names {
let mut file_reader = match fs::File::open(file_name.clone()) {
Ok(content) => io::BufReader::new(content),
Err(err) => return Err(Error::from_io(err).raise()),
};

for cert in rustls_pemfile::certs(&mut file_reader) {
match cert {
Ok(cert) => certs.push(cert),
Err(err) => {
log::trace!("Failed to parse {file_name:?}: {err:?}");
return Err(Error::from_io(err).raise());
}
};
}
}
Ok(certs)
}

extern "C" {
/// XXX: these missing from openssl-sys(?) investigate why that is.
fn OPENSSL_sk_pop_free(
Expand Down
16 changes: 13 additions & 3 deletions rustls-libssl/tests/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,17 @@ int main(int argc, char **argv) {
dump_openssl_error_stack();
SSL_CTX *ctx = SSL_CTX_new(TLS_method());
dump_openssl_error_stack();
if (strcmp(cacert, "insecure") != 0) {
if (strcmp(cacert, "insecure") == 0) {
printf("certificate verification disabled\n");
} else if (strcmp(cacert, "default") == 0) {
printf("using system default CA certs\n");
SSL_CTX_set_default_verify_paths(ctx);
dump_openssl_error_stack();
} else {
SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
dump_openssl_error_stack();
TRACE(SSL_CTX_load_verify_file(ctx, cacert));
dump_openssl_error_stack();
} else {
printf("certificate verification disabled\n");
}
TRACE(SSL_CTX_set_alpn_protos(ctx, (const uint8_t *)"\x02hi\x05world", 9));
dump_openssl_error_stack();
Expand Down Expand Up @@ -113,6 +117,11 @@ int main(int argc, char **argv) {
printf("server cert chain absent\n");
}

if (getenv("NO_ECHO")) {
printf("NO_ECHO set, skipping echo test\n");
goto cleanup;
}

// write some data and close
int wr = TRACE(SSL_write(ssl, "hello", 5));
dump_openssl_error_stack();
Expand All @@ -132,6 +141,7 @@ int main(int argc, char **argv) {
hexdump("result", buf, rd + rd2);
assert(memcmp(buf, "olleh\n", 6) == 0);

cleanup:
close(sock);
SSL_free(ssl);
SSL_CTX_free(ctx);
Expand Down
Loading