diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8b47dfc7..3d9219db 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -51,12 +51,21 @@ jobs: with: toolchain: ${{ matrix.rustc }} + - name: Install OpenSSL (Windows) + if: runner.os == 'Windows' + shell: powershell + run: | + echo "VCPKG_ROOT=$env:VCPKG_INSTALLATION_ROOT" | Out-File -FilePath $env:GITHUB_ENV -Append + vcpkg install openssl:x64-windows-static-md + - name: Build (debug) run: cargo build --locked - name: Run tests (debug) run: cargo test --locked - name: Check FFI header - run: git diff --exit-code -- upki-ffi/upki.h + run: | + git diff --exit-code -- upki-ffi/upki.h + git diff --exit-code -- upki-openssl/upki-openssl.h - name: Build (release) run: cargo build --locked --release diff --git a/Cargo.lock b/Cargo.lock index 8ac099ab..8bd05d02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1549,6 +1549,18 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" +[[package]] +name = "openssl-sys" +version = "0.9.111" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82cab2d520aa75e3c58898289429321eb788c3106963d0dc886ec7a5f4adc321" +dependencies = [ + "cc", + "libc", + "pkg-config", + "vcpkg", +] + [[package]] name = "option-ext" version = "0.2.0" @@ -1577,6 +1589,12 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "pkg-config" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" + [[package]] name = "plotters" version = "0.3.7" @@ -1882,6 +1900,7 @@ dependencies = [ "eyre", "http", "insta-cmd", + "openssl-sys", "rustls", "rustls-pki-types", "rustls-upki", @@ -1890,6 +1909,8 @@ dependencies = [ "tokio", "tokio-rustls", "upki", + "upki-ffi", + "upki-openssl", "webpki-ccadb", "webpki-roots", "x509-parser", @@ -2706,6 +2727,17 @@ dependencies = [ "upki", ] +[[package]] +name = "upki-openssl" +version = "0.1.0" +dependencies = [ + "cbindgen", + "openssl-sys", + "rustls-pki-types", + "upki", + "upki-ffi", +] + [[package]] name = "url" version = "2.5.8" @@ -2730,6 +2762,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "vcpkg" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" + [[package]] name = "version_check" version = "0.9.5" diff --git a/Cargo.toml b/Cargo.toml index ebeb2963..e0621ae1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["upki", "upki-cli", "upki-mirror", "revoke-test", "rustls-upki", "upki-ffi"] +members = ["upki", "upki-cli", "upki-mirror", "revoke-test", "rustls-upki", "upki-ffi", "upki-openssl"] resolver = "3" [workspace.package] @@ -22,6 +22,7 @@ hex = { version = "0.4", features = ["serde"] } http = "1" insta = { version = "1.44.3", features = ["filters"] } insta-cmd = "0.6.0" +openssl-sys = "0" reqwest = { version = "0.13", default-features = false, features = ["charset", "default-tls", "h2", "http2", "json"] } rand = "0.10" regex = "1.12" diff --git a/revoke-test/Cargo.toml b/revoke-test/Cargo.toml index 6d624f54..138b226a 100644 --- a/revoke-test/Cargo.toml +++ b/revoke-test/Cargo.toml @@ -23,9 +23,12 @@ x509-parser.workspace = true [dev-dependencies] criterion.workspace = true insta-cmd.workspace = true +openssl-sys.workspace = true rustls-pki-types.workspace = true rustls-upki.workspace = true upki = { path = "../upki" } +upki-ffi = { path = "../upki-ffi" } +upki-openssl = { path = "../upki-openssl" } [features] __bench_codspeed = ["dep:codspeed-criterion-compat"] diff --git a/revoke-test/src/lib.rs b/revoke-test/src/lib.rs index 3478c8d9..982fcc17 100644 --- a/revoke-test/src/lib.rs +++ b/revoke-test/src/lib.rs @@ -94,6 +94,26 @@ impl CertificateDetail { scts, }) } + + pub fn end_entity_cert_der(&self) -> Result> { + Ok(CertificateDer::from( + BASE64_STANDARD + .decode(&self.end_entity_cert) + .map_err(|e| eyre::eyre!("cannot base64-decode certificate {e:?}"))?, + )) + } + + pub fn intermediates_der(&self) -> Result>> { + let mut certs = Vec::new(); + + for i in &self.intermediates { + certs.push(CertificateDer::from(BASE64_STANDARD.decode(i).map_err( + |e| eyre::eyre!("cannot base64-decode certificate {e:?}"), + )?)); + } + + Ok(certs) + } } fn parse_octet_string(data: &[u8]) -> Result<&[u8]> { diff --git a/revoke-test/tests/api/ffi.rs b/revoke-test/tests/api/ffi.rs new file mode 100644 index 00000000..a2ea56a5 --- /dev/null +++ b/revoke-test/tests/api/ffi.rs @@ -0,0 +1,58 @@ +use core::ptr; +use std::ffi::CString; + +use revoke_test::CertificateDetail; +use upki_ffi::{ + upki_certificate_der, upki_check_revocation, upki_config, upki_config_free, upki_config_new, + upki_result, +}; + +use super::{TEST_CONFIG_PATH, TestResult}; + +pub(super) fn ffi(detail: &CertificateDetail) -> TestResult { + let certs = [detail.end_entity_cert_der().unwrap()] + .into_iter() + .chain(detail.intermediates_der().unwrap()) + .collect::>(); + + let mut cert_pointers = Vec::new(); + for c in &certs { + cert_pointers.push(upki_certificate_der { + data: c.as_ptr(), + len: c.len(), + }); + } + + let mut config = OwnedConfig(ptr::null_mut()); + assert!(matches!( + unsafe { + upki_config_new( + CString::new(TEST_CONFIG_PATH) + .unwrap() + .as_ptr(), + &mut config.0, + ) + }, + upki_result::UPKI_OK + )); + let rc = + unsafe { upki_check_revocation(config.0, cert_pointers.as_ptr(), cert_pointers.len()) }; + + drop(certs); // extend lifetime for benefit of cert_pointers + + match rc { + upki_result::UPKI_REVOCATION_REVOKED => TestResult::CorrectlyRevoked, + upki_result::UPKI_REVOCATION_NOT_COVERED | upki_result::UPKI_REVOCATION_NOT_REVOKED => { + TestResult::IncorrectlyNotRevoked + } + e => panic!("upki_check_revocation() failed with {:?}", e as usize), + } +} + +struct OwnedConfig(*mut upki_config); + +impl Drop for OwnedConfig { + fn drop(&mut self) { + unsafe { upki_config_free(self.0) }; + } +} diff --git a/revoke-test/tests/api/openssl.rs b/revoke-test/tests/api/openssl.rs new file mode 100644 index 00000000..436b224b --- /dev/null +++ b/revoke-test/tests/api/openssl.rs @@ -0,0 +1,153 @@ +use core::ffi::{c_int, c_long, c_void}; +use core::{mem, ptr}; +use std::ffi::CString; + +use openssl_sys::{ + OPENSSL_STACK, OPENSSL_sk_free, OPENSSL_sk_new_null, OPENSSL_sk_push, SSL, SSL_CTX, + SSL_CTX_free, SSL_CTX_new, SSL_free, SSL_get_ex_data_X509_STORE_CTX_idx, SSL_new, + TLS_client_method, X509_STORE_CTX, X509_STORE_CTX_free, X509_STORE_CTX_get_error, + X509_STORE_CTX_new, X509_V_ERR_CERT_REVOKED, d2i_X509, stack_st_X509, +}; +use revoke_test::CertificateDetail; +use upki_ffi::{upki_config_new, upki_result}; +use upkiopenssl::{upki_openssl_set_config, upki_openssl_verify_callback}; + +use super::{TEST_CONFIG_PATH, TestResult}; + +pub(super) fn openssl(detail: &CertificateDetail) -> TestResult { + let mut chain = Chain::new(); + + for cert in [detail.end_entity_cert_der().unwrap()] + .into_iter() + .chain(detail.intermediates_der().unwrap()) + { + chain.push(&cert); + } + + let mut config = ptr::null_mut(); + assert!(matches!( + unsafe { + upki_config_new( + CString::new(TEST_CONFIG_PATH) + .unwrap() + .as_ptr(), + &mut config, + ) + }, + upki_result::UPKI_OK + )); + + let ssl_ctx = SslCtx::new(); + unsafe { upki_openssl_set_config(ssl_ctx.0, config) }; + + let ssl = ssl_ctx.new_ssl(); + + let mut store_ctx = StoreCtx::new(); + store_ctx.attach_ssl(ssl.0); + store_ctx.set_error_depth(0); + store_ctx.set_verified_chain(chain); + + let rc = unsafe { upki_openssl_verify_callback(1, store_ctx.0) }; + + drop(ssl); // extend lifetime over store_ctx + + match rc { + 1 => TestResult::IncorrectlyNotRevoked, + 0 if store_ctx.error() == X509_V_ERR_CERT_REVOKED => TestResult::CorrectlyRevoked, + _ => panic!( + "upki_openssl_verify_callback failed with rc={rc:?} store_ctx.error={:?}", + store_ctx.error() + ), + } +} + +struct SslCtx(*mut SSL_CTX); + +impl SslCtx { + fn new() -> Self { + Self(unsafe { SSL_CTX_new(TLS_client_method()) }) + } + + fn new_ssl(&self) -> Ssl { + Ssl(unsafe { SSL_new(self.0) }) + } +} + +impl Drop for SslCtx { + fn drop(&mut self) { + unsafe { SSL_CTX_free(self.0) }; + } +} + +struct Ssl(*mut SSL); + +impl Drop for Ssl { + fn drop(&mut self) { + unsafe { SSL_free(self.0) }; + } +} + +struct StoreCtx(*mut X509_STORE_CTX); + +impl StoreCtx { + fn new() -> Self { + Self(unsafe { X509_STORE_CTX_new() }) + } + + fn error(&self) -> c_int { + unsafe { X509_STORE_CTX_get_error(self.0) } + } + + fn set_error_depth(&mut self, depth: i32) { + unsafe { X509_STORE_CTX_set_error_depth(self.0, depth) }; + } + + fn set_verified_chain(&mut self, mut chain: Chain) { + unsafe { X509_STORE_CTX_set0_verified_chain(self.0, chain.steal()) }; + } + + fn attach_ssl(&mut self, ssl: *mut SSL) { + unsafe { + X509_STORE_CTX_set_ex_data(self.0, SSL_get_ex_data_X509_STORE_CTX_idx(), ssl.cast()) + }; + } +} + +impl Drop for StoreCtx { + fn drop(&mut self) { + unsafe { X509_STORE_CTX_free(self.0) } + } +} + +struct Chain(*mut stack_st_X509); + +impl Chain { + fn new() -> Self { + Self(unsafe { OPENSSL_sk_new_null() as *mut stack_st_X509 }) + } + + fn push(&mut self, der_bytes: &[u8]) { + unsafe { + let mut ptr = der_bytes.as_ptr(); + let x509 = d2i_X509(ptr::null_mut(), &mut ptr, der_bytes.len() as c_long); + assert!(!x509.is_null()); + OPENSSL_sk_push(self.0 as *mut OPENSSL_STACK, x509 as *mut c_void); + } + } + + fn steal(&mut self) -> *mut stack_st_X509 { + mem::replace(&mut self.0, ptr::null_mut()) + } +} + +impl Drop for Chain { + fn drop(&mut self) { + unsafe { OPENSSL_sk_free(self.0 as *mut OPENSSL_STACK) } + } +} + +unsafe extern "C" { + fn X509_STORE_CTX_set_error_depth(ctx: *mut X509_STORE_CTX, depth: c_int); + fn X509_STORE_CTX_set0_verified_chain(ctx: *mut X509_STORE_CTX, chain: *mut stack_st_X509); + fn X509_STORE_CTX_set_ex_data(ctx: *mut X509_STORE_CTX, idx: c_int, ptr: *mut c_void); +} diff --git a/revoke-test/tests/system_tests.rs b/revoke-test/tests/system_tests.rs index 413c5ebc..7e5cfca2 100644 --- a/revoke-test/tests/system_tests.rs +++ b/revoke-test/tests/system_tests.rs @@ -9,14 +9,18 @@ use std::process::{Command, Stdio}; use std::sync::Arc; use std::time::SystemTime; -use base64::prelude::*; use insta_cmd::get_cargo_bin; use revoke_test::{CertificateDetail, RevocationTestSite, RevocationTestSites}; use rustls::client::danger::ServerCertVerifier; -use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; +use rustls::pki_types::{ServerName, UnixTime}; use rustls::{CertificateError, Error, RootCertStore}; use rustls_upki::{Policy, ServerVerifier}; +#[path = "api/ffi.rs"] +mod ffi; +#[path = "api/openssl.rs"] +mod openssl; + #[ignore] #[test] fn real_world_system_tests() { @@ -46,6 +50,8 @@ fn real_world_system_tests() { ); let high_level_cli = test_each_site(tests.sites.iter(), high_level_cli, "cli"); + let ffi = test_each_site(tests.sites.iter(), ffi::ffi, "ffi"); + let openssl = test_each_site(tests.sites.iter(), openssl::openssl, "openssl"); let verifier = ServerVerifier::new( Policy::default(), @@ -59,38 +65,38 @@ fn real_world_system_tests() { let rustls_results = test_each_site(tests.sites.iter(), verifier, "rustls"); - for ((site, high), rustls) in tests + for ((((site, high), rustls), ffi), openssl) in tests .sites .iter() .zip(high_level_cli.iter()) .zip(rustls_results.iter()) + .zip(ffi.iter()) + .zip(openssl.iter()) { assert!( high == rustls || *high == rustls.expired_as_revoked(), "site {site:?} revocation result disagrees between high-level API ({high:?}) and rustls verifier ({rustls:?})" ); + assert!( + high == ffi, + "site {site:?} revocation result disagrees between high-level API ({high:?}) and FFI API ({ffi:?})" + ); + assert!( + high == openssl, + "site {site:?} revocation result disagrees between high-level API ({high:?}) and OpenSSL API ({openssl:?})" + ); } } impl TestCase for ServerVerifier { fn run(&self, detail: &CertificateDetail, test: &RevocationTestSite) -> TestResult { // Decode certificates from base64 - let end_entity = CertificateDer::from( - BASE64_STANDARD - .decode(&detail.end_entity_cert) - .expect("cannot decode end_entity_cert"), - ); + let end_entity = detail + .end_entity_cert_der() + .expect("cannot decode end_entity_cert"); let intermediates = detail - .intermediates - .iter() - .map(|c| { - CertificateDer::from( - BASE64_STANDARD - .decode(c) - .expect("cannot decode issuer_cert"), - ) - }) - .collect::>(); + .intermediates_der() + .expect("cannot decode issuer_cert"); let url = &test.test_website_revoked; let host = url diff --git a/upki-ffi/Cargo.toml b/upki-ffi/Cargo.toml index 2ed0fec2..6e42da04 100644 --- a/upki-ffi/Cargo.toml +++ b/upki-ffi/Cargo.toml @@ -8,9 +8,7 @@ license.workspace = true description = "C FFI bindings for upki" [lib] -name = "upki" -crate-type = ["cdylib"] -doc = false # Can't document this and `upki` because they have the same name +crate-type = ["cdylib", "lib"] [dependencies] rustls-pki-types.workspace = true diff --git a/upki-openssl/Cargo.toml b/upki-openssl/Cargo.toml new file mode 100644 index 00000000..94f31516 --- /dev/null +++ b/upki-openssl/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "upki-openssl" +version = "0.1.0" +license.workspace = true +rust-version.workspace = true +edition.workspace = true +repository.workspace = true + +[lib] +name = "upkiopenssl" +crate-type = ["cdylib", "lib"] + +[dependencies] +openssl-sys = { workspace = true } +rustls-pki-types.workspace = true +upki = { path = "../upki", version = "0.2.0" } +upki-ffi = { path = "../upki-ffi" } + +[build-dependencies] +cbindgen.workspace = true + +[lints] +workspace = true diff --git a/upki-openssl/build.rs b/upki-openssl/build.rs new file mode 100644 index 00000000..8b689d10 --- /dev/null +++ b/upki-openssl/build.rs @@ -0,0 +1,16 @@ +use std::env; +use std::path::PathBuf; + +use cbindgen::Language; + +fn main() { + let crate_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()); + cbindgen::Builder::new() + .with_crate(&crate_dir) + .with_language(Language::C) + .with_sys_include("openssl/x509_vfy.h") + .with_include_guard("UPKI_OPENSSL_H") + .generate() + .expect("unable to generate bindings") + .write_to_file(crate_dir.join("upki-openssl.h")); +} diff --git a/upki-openssl/src/lib.rs b/upki-openssl/src/lib.rs new file mode 100644 index 00000000..40b9059e --- /dev/null +++ b/upki-openssl/src/lib.rs @@ -0,0 +1,326 @@ +#![warn(clippy::undocumented_unsafe_blocks)] + +use core::ffi::{c_long, c_void}; +use core::ptr; +use std::os::raw::c_int; +use std::slice; +use std::sync::LazyLock; + +use openssl_sys::{ + CRYPTO_EX_DATA, CRYPTO_EX_INDEX_SSL_CTX, CRYPTO_get_ex_new_index, OPENSSL_free, OPENSSL_sk_num, + OPENSSL_sk_value, SSL, SSL_CTX, SSL_CTX_get_ex_data, SSL_CTX_set_ex_data, SSL_get_SSL_CTX, + SSL_get_ex_data_X509_STORE_CTX_idx, X509, X509_STORE_CTX, X509_STORE_CTX_get_error_depth, + X509_STORE_CTX_get_ex_data, X509_STORE_CTX_get0_chain, X509_STORE_CTX_set_error, + X509_V_ERR_APPLICATION_VERIFICATION, X509_V_ERR_CERT_REVOKED, i2d_X509, stack_st_X509, +}; +use rustls_pki_types::CertificateDer; +use upki_ffi::{ + upki_certificate_der, upki_check_revocation, upki_config, upki_config_free, upki_config_new, + upki_result, +}; + +/// Sets the upki config to use for connections based upon `ctx`. +/// +/// `config` becomes owned by `SSL_CTX`. If `config` is NULL the previous configuration is +/// freed. +/// +/// # Thread safety +/// +/// This inherits the property of the OpenSSL API, whereby a single `SSL_CTX` cannot be shared +/// between threads. +/// +/// # Safety +/// +/// This does nothing if `ctx` is NULL. `config` is required to be a valid `upki_config` pointer, +/// or NULL. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn upki_openssl_set_config(ctx: *mut SSL_CTX, config: *const upki_config) { + if ctx.is_null() { + return; + } + + let Some(index) = *UPKI_SSL_CTX_CONFIG_INDEX else { + return; + }; + + // SAFETY: `upki_config_free` is defined for a previous valid pointer, or NULL. We rely on + // OpenSSL being bug-free -- only returning NULL or a previous pointer provided to + // `SSL_CTX_set_ex_data`. + unsafe { + // free any previous value. + upki_config_free(SSL_CTX_get_ex_data(ctx, index).cast()); + } + + // SAFETY: `ctx` is required to be non-NULL (as established above). + unsafe { + SSL_CTX_set_ex_data(ctx, index, config.cast_mut().cast()); + } +} + +/// This is a function matching OpenSSL's `SSL_verify_cb` type which does +/// revocation checking using upki. +/// +/// # Configuration +/// If ``upki_openssl_set_config()` was previously called against the `SSL_CTX` available +/// from `X509_STORE_CTX`, this configuration is used. +/// +/// Otherwise, if that function wasn't called, or no `SSL_CTX` can be obtained from `X509_STORE_CTX`, +/// the configuration file and data location is found automatically based on defaults. +/// +/// # Errors +/// This function returns 0 if called with 0 for the `preverify_ok` parameter. +/// As a result, it never allows a verification to pass if the previous verification +/// step has failed. +/// +/// If the certificate chain obtained from `x509_ctx` is revoked, this function returns 0 +/// and sets the `X509_V_ERR_CERT_REVOKED` error on `x509_ctx` (using +/// `X509_STORE_CTX_set_error(3SSL)`). +/// +/// If the certificate chain obtained from `x509_ctx` is not included in the revocation data, +/// this function returns `preverify_ok`. +/// +/// If the revocation status cannot be determined, this function returns 0 and sets +/// the `X509_V_ERR_APPLICATION_VERIFICATION` error on `x509_ctx` (using +/// `X509_STORE_CTX_set_error(3SSL)`). +/// +/// On unexpected/unrecoverable errors, this function returns 0. +/// +/// # Safety +/// This function is called by OpenSSL typically, and its correct operation +/// hinges almost entirely on being called properly. For example, that +/// `x509_ctx` is a valid pointer, or NULL. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn upki_openssl_verify_callback( + mut preverify_ok: c_int, + x509_ctx: *mut X509_STORE_CTX, +) -> c_int { + // Revocation checking never improves the situation if the verification has failed. + if preverify_ok == 0 { + return preverify_ok; + } + + // SAFETY: via essential and established principles of the C type system, we rely on + // OpenSSL to call this function with a valid or NULL `x509_ctx` pointer. + let Some(mut x509_ctx) = (unsafe { BorrowedX509StoreCtx::from_ptr(x509_ctx) }) else { + return 0; + }; + + // This callback is called once per certificate, with the final call being for the + // leaf certificate denoted by error_depth = 0. We only process the chain as a whole; + // do this at the leaf certificate level. + if x509_ctx.error_depth() != 0 { + return preverify_ok; + } + + let Some(chain) = x509_ctx.chain() else { + return 0; + }; + + let Some(certs) = chain.copy_certs() else { + return 0; + }; + + let cert_descriptors = certs + .iter() + .map(|cert| upki_certificate_der { + data: cert.as_ptr(), + len: cert.len(), + }) + .collect::>(); + + let mut config: *const upki_config = x509_ctx.upki_config_from_ssl_ctx(); + + let _our_config = match config.is_null() { + true => { + let Ok(owned) = OwnedUpkiConfig::new() else { + return 0; + }; + config = owned.ptr(); + Some(owned) + } + false => None, + }; + + // SAFETY: `upki_check_revocation` requires: + // - a valid config pointer, established above (either transitively via the safety + // preconditions on `upki_openssl_set_config`, or by creating one stored in `_our_config`) + // - valid pointers to a sequence of certificates, which is provided by the `cert_descriptors` vec. + match unsafe { + upki_check_revocation(config, cert_descriptors.as_ptr(), cert_descriptors.len()) + } { + upki_result::UPKI_REVOCATION_REVOKED => { + x509_ctx.set_error(X509_V_ERR_CERT_REVOKED); + preverify_ok = 0; + } + upki_result::UPKI_REVOCATION_NOT_COVERED | upki_result::UPKI_REVOCATION_NOT_REVOKED => {} + _e => { + x509_ctx.set_error(X509_V_ERR_APPLICATION_VERIFICATION); + preverify_ok = 0; + } + } + + drop(certs); + + preverify_ok +} + +struct BorrowedX509StoreCtx<'a>(&'a mut X509_STORE_CTX); + +impl<'a> BorrowedX509StoreCtx<'a> { + unsafe fn from_ptr(ptr: *mut X509_STORE_CTX) -> Option { + // SAFETY: we pass up the requirements of `ptr::as_mut()` to our caller + unsafe { ptr.as_mut() }.map(Self) + } + + fn error_depth(&self) -> c_int { + // SAFETY: the input pointer is valid, because it comes from our reference. + unsafe { X509_STORE_CTX_get_error_depth(ptr::from_ref(self.0)) } + } + + fn chain(&self) -> Option> { + // SAFETY: This type guarantees that the pointer is of the correct type, alignment, etc, + // and is non-NULL (via coming from a reference.) + let chain = unsafe { X509_STORE_CTX_get0_chain(ptr::from_ref(self.0)) }; + + // SAFETY: we require that openssl correctly returns a valid pointer, or NULL. + unsafe { chain.as_ref() }.map(BorrowedX509Stack) + } + + fn set_error(&mut self, err: i32) { + // SAFETY: the input pointer is valid, because it comes from our reference. + // OpenSSL does not document any other preconditions. + unsafe { X509_STORE_CTX_set_error(ptr::from_mut(self.0), err) }; + } + + fn upki_config_from_ssl_ctx(&self) -> *const upki_config { + let ssl_ctx = self.ssl_ctx(); + + match (ssl_ctx.is_null(), *UPKI_SSL_CTX_CONFIG_INDEX) { + // SAFETY: `ssl_ctx` is non-NULL, the index only has a upki_config pointer inserted into it. + (false, Some(index)) => unsafe { SSL_CTX_get_ex_data(ssl_ctx, index).cast() }, + (_, _) => ptr::null(), + } + } + + fn ssl_ctx(&self) -> *const SSL_CTX { + // SAFETY: the input pointer is valid, because it comes from our reference. + let ssl: *const SSL = unsafe { + X509_STORE_CTX_get_ex_data(ptr::from_ref(self.0), SSL_get_ex_data_X509_STORE_CTX_idx()) + .cast() + }; + + match ssl.is_null() { + true => ptr::null(), + // SAFETY: `SSL_get_SSL_CTX` requires non-NULL parameter, established here. + false => unsafe { SSL_get_SSL_CTX(ssl) }, + } + } +} + +struct BorrowedX509Stack<'a>(&'a stack_st_X509); + +impl<'a> BorrowedX509Stack<'a> { + fn copy_certs(&self) -> Option>> { + // SAFETY: the stack pointer is valid, thanks to it being from a reference. + let count = unsafe { OPENSSL_sk_num(ptr::from_ref(self.0).cast()) }; + if count < 0 { + return None; + } + + let mut certs = vec![]; + for i in 0..count { + // SAFETY: the stack pointer is valid, thanks to it being from a reference. `OPENSSL_sk_value` returns + // a valid pointer to the item or NULL. + let x509: *const X509 = + unsafe { OPENSSL_sk_value(ptr::from_ref(self.0).cast(), i).cast() }; + + // SAFETY: we require OpenSSL only fills the stack with valid pointers to X509 objects (or NULL) + let x509 = unsafe { x509.as_ref() }?; + certs.push(x509_to_certificate_der(x509)?); + } + + Some(certs) + } +} + +struct OwnedUpkiConfig(*mut upki_config); + +impl OwnedUpkiConfig { + fn new() -> Result { + let mut ptr = ptr::null_mut(); + // SAFETY: `upki_config_new` requires a pointer output, as established here. + let rc = unsafe { upki_config_new(ptr::null(), &mut ptr) }; + + match ptr.is_null() { + true => Err(rc), + false => Ok(Self(ptr)), + } + } + + fn ptr(&self) -> *const upki_config { + self.0.cast_const() + } +} + +impl Drop for OwnedUpkiConfig { + fn drop(&mut self) { + // SAFETY: `upki_config_free` defined for a valid pointer or NULL. + unsafe { upki_config_free(self.0) } + } +} + +fn x509_to_certificate_der(x509: &'_ X509) -> Option> { + // SAFETY: the x509 pointer is valid, thanks to it coming from a reference. + let (ptr, len) = unsafe { + let mut ptr = ptr::null_mut(); + let len = i2d_X509(ptr::from_ref(x509), &mut ptr); + (ptr, len) + }; + + if len <= 0 || ptr.is_null() { + return None; + } + let len = len as usize; + + let mut v = Vec::with_capacity(len); + // SAFETY: we rely on i2d_X509 allocating `ptr` correctly and signalling an error via negative `len` if not. + // `ptr` must be an allocated pointer. + unsafe { + v.extend_from_slice(slice::from_raw_parts(ptr, len)); + OPENSSL_free(ptr as *mut _); + } + Some(v.into()) +} + +static UPKI_SSL_CTX_CONFIG_INDEX: LazyLock> = LazyLock::new(|| unsafe { + // SAFETY: no documented safety conditions for this function. + let index = CRYPTO_get_ex_new_index( + CRYPTO_EX_INDEX_SSL_CTX, + 0, + ptr::null_mut(), + None, + None, + Some(ssl_ctx_upki_config_free), + ); + match index { + -1 => None, + _ => Some(index), + } +}); + +/// Funnel `CRYPTO_EX_free` into calling `upki_config_free`. +unsafe extern "C" fn ssl_ctx_upki_config_free( + _parent: *mut c_void, + config: *mut c_void, + _ad: *mut CRYPTO_EX_DATA, + _idx: c_int, + _argl: c_long, + _argp: *mut c_void, +) { + // SAFETY: The previous value is either NULL or a valid config pointer. + // This matches the precondition of `upki_config_free`. + unsafe { upki_config_free(config.cast()) }; +} + +#[cfg(test)] +mod test; diff --git a/upki-openssl/src/test.rs b/upki-openssl/src/test.rs new file mode 100644 index 00000000..326fbdd4 --- /dev/null +++ b/upki-openssl/src/test.rs @@ -0,0 +1,193 @@ +#![allow(clippy::undocumented_unsafe_blocks)] + +use core::{mem, ptr}; +use std::os::raw::c_int; + +use openssl_sys::{ + OPENSSL_STACK, OPENSSL_sk_free, OPENSSL_sk_new_null, OPENSSL_sk_push, SSL_CTX_free, + SSL_CTX_new, TLS_client_method, X509_STORE_CTX, X509_STORE_CTX_free, X509_STORE_CTX_get_error, + X509_STORE_CTX_new, X509_STORE_CTX_set_error, stack_st_X509, +}; +use upki_ffi::upki_config_new; + +use super::upki_openssl_verify_callback; +use crate::upki_openssl_set_config; + +#[test] +fn preverify_zero_returns_zero() { + assert_eq!( + unsafe { upki_openssl_verify_callback(0, ptr::null_mut()) }, + 0 + ); +} + +#[test] +fn null_ctx_returns_zero() { + assert_eq!( + unsafe { upki_openssl_verify_callback(1, ptr::null_mut()) }, + 0 + ); +} + +#[test] +fn preverify_ok_zero_valid_ctx_returns_zero() { + let ctx = StoreCtx::new(); + assert_eq!(unsafe { upki_openssl_verify_callback(0, ctx.0) }, 0); + assert!(ctx.untouched_error()); +} + +#[test] +fn preverify_ok_zero_nonzero_depth_returns_zero() { + let mut ctx = StoreCtx::new(); + ctx.set_error_depth(3); + + assert_eq!(unsafe { upki_openssl_verify_callback(0, ctx.0) }, 0); + assert!(ctx.untouched_error()); +} + +#[test] +fn error_depth_various_nonzero_all_pass_through() { + for depth in [-1, 1, 3, 5, 10, 100] { + let mut ctx = StoreCtx::new(); + ctx.set_error_depth(depth); + + assert_eq!( + unsafe { upki_openssl_verify_callback(1, ctx.0) }, + 1, + "expected preverify_ok pass-through at error_depth={depth}" + ); + assert!(ctx.untouched_error()); + } +} + +#[test] +fn error_depth_nonzero_preserves_arbitrary_preverify_ok_value() { + for (depth, pv) in [(1, 2), (2, 42), (5, -1), (10, 100)] { + let mut ctx = StoreCtx::new(); + ctx.set_error_depth(depth); + + assert_eq!( + unsafe { upki_openssl_verify_callback(pv, ctx.0) }, + pv, + "preverify_ok={pv} should be returned unchanged at error_depth={depth}" + ); + assert!(ctx.untouched_error()); + } +} + +#[test] +fn missing_chain_returns_zero() { + let ctx = StoreCtx::new(); + assert_eq!(unsafe { upki_openssl_verify_callback(1, ctx.0) }, 0); + assert!(ctx.untouched_error()); +} + +#[test] +fn malformed_chain_returns_zero() { + let mut chain = Chain::new(); + chain.push_null(); + let mut ctx = StoreCtx::new(); + ctx.set_verified_chain(chain); + assert_eq!(unsafe { upki_openssl_verify_callback(1, ctx.0) }, 0); + assert!(ctx.untouched_error()); +} + +#[test] +fn set_config_twice() { + unsafe { + let ctx = SSL_CTX_new(TLS_client_method()); + let mut config = ptr::null_mut(); + + upki_config_new(ptr::null(), &mut config); + upki_openssl_set_config(ctx, config); + + upki_config_new(ptr::null(), &mut config); + upki_openssl_set_config(ctx, config); + + SSL_CTX_free(ctx); + } +} + +#[test] +fn set_config_is_freed() { + unsafe { + let ctx = SSL_CTX_new(TLS_client_method()); + let mut config = ptr::null_mut(); + + upki_config_new(ptr::null(), &mut config); + upki_openssl_set_config(ctx, config); // takes ownership. + + // frees config for us. + SSL_CTX_free(ctx); + } +} + +struct StoreCtx(*mut X509_STORE_CTX); + +impl StoreCtx { + fn new() -> Self { + let mut v = Self(unsafe { X509_STORE_CTX_new() }); + // we set a placeholder error here, which is intended to be unused + // anywhere else, and allows detection of whether the error has been + // set or preserved. + assert!(!v.untouched_error()); + v.set_error(UNTOUCHED_ERROR); + assert!(v.untouched_error()); + v + } + + fn set_error_depth(&mut self, depth: i32) { + unsafe { X509_STORE_CTX_set_error_depth(self.0, depth) }; + } + + fn error(&self) -> c_int { + unsafe { X509_STORE_CTX_get_error(self.0) } + } + + fn untouched_error(&self) -> bool { + self.error() == UNTOUCHED_ERROR + } + + fn set_error(&mut self, e: c_int) { + unsafe { X509_STORE_CTX_set_error(self.0, e) } + } + + fn set_verified_chain(&mut self, mut chain: Chain) { + unsafe { X509_STORE_CTX_set0_verified_chain(self.0, chain.steal()) }; + } +} + +impl Drop for StoreCtx { + fn drop(&mut self) { + unsafe { X509_STORE_CTX_free(self.0) } + } +} + +struct Chain(*mut stack_st_X509); + +impl Chain { + fn new() -> Self { + Self(unsafe { OPENSSL_sk_new_null() as *mut stack_st_X509 }) + } + + fn push_null(&mut self) { + unsafe { OPENSSL_sk_push(self.0 as *mut OPENSSL_STACK, ptr::null()) }; + } + + fn steal(&mut self) -> *mut stack_st_X509 { + mem::replace(&mut self.0, ptr::null_mut()) + } +} + +impl Drop for Chain { + fn drop(&mut self) { + unsafe { OPENSSL_sk_free(self.0 as *mut OPENSSL_STACK) } + } +} + +const UNTOUCHED_ERROR: c_int = 12345678; + +unsafe extern "C" { + fn X509_STORE_CTX_set_error_depth(ctx: *mut X509_STORE_CTX, depth: c_int); + fn X509_STORE_CTX_set0_verified_chain(ctx: *mut X509_STORE_CTX, chain: *mut stack_st_X509); +} diff --git a/upki-openssl/upki-openssl.h b/upki-openssl/upki-openssl.h new file mode 100644 index 00000000..36eae9db --- /dev/null +++ b/upki-openssl/upki-openssl.h @@ -0,0 +1,64 @@ +#ifndef UPKI_OPENSSL_H +#define UPKI_OPENSSL_H + +#include +#include +#include +#include +#include + +/** + * Sets the upki config to use for connections based upon `ctx`. + * + * `config` becomes owned by `SSL_CTX`. If `config` is NULL the previous configuration is + * freed. + * + * # Thread safety + * + * This inherits the property of the OpenSSL API, whereby a single `SSL_CTX` cannot be shared + * between threads. + * + * # Safety + * + * This does nothing if `ctx` is NULL. `config` is required to be a valid `upki_config` pointer, + * or NULL. + */ +void upki_openssl_set_config(SSL_CTX *ctx, const upki_config *config); + +/** + * This is a function matching OpenSSL's `SSL_verify_cb` type which does + * revocation checking using upki. + * + * # Configuration + * If ``upki_openssl_set_config()` was previously called against the `SSL_CTX` available + * from `X509_STORE_CTX`, this configuration is used. + * + * Otherwise, if that function wasn't called, or no `SSL_CTX` can be obtained from `X509_STORE_CTX`, + * the configuration file and data location is found automatically based on defaults. + * + * # Errors + * This function returns 0 if called with 0 for the `preverify_ok` parameter. + * As a result, it never allows a verification to pass if the previous verification + * step has failed. + * + * If the certificate chain obtained from `x509_ctx` is revoked, this function returns 0 + * and sets the `X509_V_ERR_CERT_REVOKED` error on `x509_ctx` (using + * `X509_STORE_CTX_set_error(3SSL)`). + * + * If the certificate chain obtained from `x509_ctx` is not included in the revocation data, + * this function returns `preverify_ok`. + * + * If the revocation status cannot be determined, this function returns 0 and sets + * the `X509_V_ERR_APPLICATION_VERIFICATION` error on `x509_ctx` (using + * `X509_STORE_CTX_set_error(3SSL)`). + * + * On unexpected/unrecoverable errors, this function returns 0. + * + * # Safety + * This function is called by OpenSSL typically, and its correct operation + * hinges almost entirely on being called properly. For example, that + * `x509_ctx` is a valid pointer, or NULL. + */ +int upki_openssl_verify_callback(int preverify_ok, X509_STORE_CTX *x509_ctx); + +#endif /* UPKI_OPENSSL_H */ diff --git a/upki/src/lib.rs b/upki/src/lib.rs index 315381fc..a70afa2f 100644 --- a/upki/src/lib.rs +++ b/upki/src/lib.rs @@ -156,6 +156,12 @@ impl fmt::Display for Error { } } +impl From for Error { + fn from(value: revocation::Error) -> Self { + Self::Revocation(value) + } +} + fn user_config_file() -> Result { Ok(project_dirs()? .config_dir()