From ce6d6fd6865f17cca3e3689cfdcfdcce6ec3c444 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Tue, 14 Jul 2026 17:34:00 +0200 Subject: [PATCH 1/8] feat(engine): add a load_private_key hook to the engine-core toolkit Add Engine::set_load_privkey::() and the LoadPrivKeyHandler trait, mirroring the existing set_destroy plumbing. The C trampoline returns *mut EVP_PKEY, so it wraps its body in catch_panic with a NULL fallback and uses a new result_to_ptr helper (the pointer-returning analogue of result_to_int) that records the error on the OpenSSL ERR queue before returning NULL. Signed-off-by: Christian Walter --- .../azihsm_ossl_engine_core/src/engine.rs | 94 +++++++++++++++++++ .../azihsm_ossl_engine_core/src/error.rs | 20 ++++ 2 files changed, 114 insertions(+) diff --git a/plugins/ossl_engine/azihsm_ossl_engine_core/src/engine.rs b/plugins/ossl_engine/azihsm_ossl_engine_core/src/engine.rs index e3133d84e..609219aa1 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine_core/src/engine.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine_core/src/engine.rs @@ -6,6 +6,7 @@ use std::ffi::CStr; use std::ffi::c_char; use std::ffi::c_int; +use std::ffi::c_void; use std::ptr::NonNull; use std::ptr::null_mut; @@ -17,6 +18,7 @@ use crate::error::RetCode; use crate::error::catch_panic; use crate::error::ossl_check; use crate::error::result_to_int; +use crate::error::result_to_ptr; pub struct Engine { ptr: *mut ffi::ENGINE, @@ -120,6 +122,20 @@ impl Engine { EngineError::SetDestroyFailed, ) } + + /// Register the private-key loader OpenSSL invokes for + /// `ENGINE_load_private_key` (e.g. `openssl … -keyform engine -inform engine + /// -in `). Like [`set_destroy`](Self::set_destroy), each `H` + /// monomorphizes a distinct C trampoline. + #[allow(unsafe_code)] + pub fn set_load_privkey(&self) -> EngineResult<()> { + // SAFETY: self.ptr is valid (from NonNull); c_load_privkey:: has the + // correct ENGINE_LOAD_KEY_PTR signature and is a 'static fn item. + ossl_check( + unsafe { ffi::ENGINE_set_load_privkey_function(self.ptr, Some(c_load_privkey::)) }, + EngineError::SetLoadPrivKeyFailed, + ) + } } /// Caller-supplied destroy logic, invoked by OpenSSL when an `ENGINE` is @@ -161,6 +177,59 @@ unsafe fn destroy_inner(e: *mut ffi::ENGINE) -> EngineResult< H::destroy(&mut engine) } +/// Caller-supplied private-key loader, invoked by OpenSSL for +/// `ENGINE_load_private_key`. Implement on a marker type and pass it as the +/// type parameter to [`Engine::set_load_privkey`]. +pub trait LoadPrivKeyHandler { + /// Load the key named `key_id` and return an owning `*mut EVP_PKEY` + /// (ownership transfers to OpenSSL). Return an error to surface a NULL + /// result plus an ERR-queue entry to the caller. + fn load(engine: &Engine, key_id: &CStr) -> EngineResult<*mut ffi::EVP_PKEY>; +} + +/// C trampoline for `ENGINE_set_load_privkey_function` (`ENGINE_LOAD_KEY_PTR`). +/// Catches panics and dispatches to `H::load`, returning NULL on panic/error. +/// The `UI_METHOD`/`cb_data` params are unused: keys are named by id, no prompt. +/// +/// # Safety +/// Called only by OpenSSL. `e` is the loading ENGINE and `key_id` the key +/// identifier string, per the callback contract. +#[allow(unsafe_code)] +unsafe extern "C" fn c_load_privkey( + e: *mut ffi::ENGINE, + key_id: *const c_char, + _ui: *mut ffi::UI_METHOD, + _cb_data: *mut c_void, +) -> *mut ffi::EVP_PKEY { + catch_panic( + // SAFETY: `e` and `key_id` are the pointers OpenSSL passes to the + // load_privkey callback per ENGINE_set_load_privkey_function. + || result_to_ptr(unsafe { load_privkey_inner::(e, key_id) }), + null_mut(), + ) +} + +/// Inner body of [`c_load_privkey`]: validate the raw pointers, rebuild a safe +/// [`Engine`], and dispatch to `H::load`. +/// +/// # Safety +/// `e` must be the loading `ENGINE` and `key_id` a valid C string (or NULL). +#[allow(unsafe_code)] +unsafe fn load_privkey_inner( + e: *mut ffi::ENGINE, + key_id: *const c_char, +) -> EngineResult<*mut ffi::EVP_PKEY> { + let nn = NonNull::new(e).ok_or(EngineError::NullParam("engine"))?; + if key_id.is_null() { + return Err(EngineError::NullParam("key_id")); + } + // SAFETY: `e` is non-null (checked) and the ENGINE OpenSSL is loading from. + let engine = unsafe { Engine::from_ptr(nn) }; + // SAFETY: `key_id` is non-null (checked) and a valid C string per contract. + let key_id = unsafe { CStr::from_ptr(key_id) }; + H::load(&engine, key_id) +} + #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] @@ -202,4 +271,29 @@ mod tests { "destroy callback should run exactly once on ENGINE_free" ); } + + struct NullLoader; + impl LoadPrivKeyHandler for NullLoader { + fn load(_: &Engine, _: &CStr) -> EngineResult<*mut ffi::EVP_PKEY> { + Ok(null_mut()) + } + } + + #[test] + #[allow(unsafe_code)] + fn set_load_privkey_registers_the_hook() { + // SAFETY: ENGINE_new / ENGINE_free are standard OpenSSL entry points. + let raw = unsafe { ffi::ENGINE_new() }; + let nn = NonNull::new(raw).expect("ENGINE_new returned NULL"); + // SAFETY: nn is non-null and owned until ENGINE_free below. + let e = unsafe { Engine::from_ptr(nn) }; + + e.set_load_privkey::().unwrap(); + // SAFETY: e.as_ptr() is a valid ENGINE. + let f = unsafe { ffi::ENGINE_get_load_privkey_function(e.as_ptr()) }; + assert!(f.is_some(), "load_privkey hook should be registered"); + + // SAFETY: same as above. + unsafe { ffi::ENGINE_free(e.as_ptr()) }; + } } diff --git a/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs b/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs index b74ffb50e..f685e27ad 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs @@ -60,6 +60,9 @@ pub enum EngineError { #[error("ENGINE_set_destroy_function failed")] SetDestroyFailed, + #[error("ENGINE_set_load_privkey_function failed")] + SetLoadPrivKeyFailed, + /// A standalone message with no underlying error. #[error("{0}")] Other(String), @@ -166,6 +169,23 @@ pub fn result_to_int(result: EngineResult) -> c_int { } } +/// Convert an [`EngineResult`] holding a raw pointer into that pointer, for a +/// C callback that returns `*mut T` (e.g. the `load_privkey` hook returning +/// `*mut EVP_PKEY`). On `Err`, push the message onto the OpenSSL ERR queue, +/// log via `tracing`, and return NULL — the pointer-returning analogue of +/// [`result_to_int`]. +pub fn result_to_ptr(result: EngineResult<*mut T>) -> *mut T { + match result { + Ok(ptr) => ptr, + Err(e) => { + let reason = c_int::from(&e); + openssl_err(reason, &e.to_string()); + tracing::error!("{e}"); + std::ptr::null_mut() + } + } +} + /// Run `f` under [`catch_unwind`]. A panic is logged and turned into the /// caller-supplied `on_panic` value. /// From 41288504d8d1420eef66502ca73a718fb9495dc3 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Tue, 14 Jul 2026 17:34:17 +0200 Subject: [PATCH 2/8] feat(engine): load EC private keys from azihsm:// URIs Implement ENGINE_load_private_key for EC keys. A key id is an azihsm://;type= URI (parsed in uri.rs, mirroring the provider's STORE). Loading opens the HSM on first use, unmasks the key pair, rebuilds the public EVP_PKEY from the SDK's DER via the openssl crate, and attaches the live HSM private key to the EC_KEY ex_data so it survives for later operations and is deleted from the HSM when the EVP_PKEY is freed. EngineData gains with_session() to run an operation against the open session. RSA/RSA-PSS URIs parse but return a clear not-supported error for now; RSA loading needs an HSM import path and lands with its own tests. A mock round-trip test generates an EC key, exports its masked blob, loads it back through the engine, and verifies the public key matches. Signed-off-by: Christian Walter --- .../ossl_engine/azihsm_ossl_engine/Cargo.toml | 13 +- .../azihsm_ossl_engine/src/context.rs | 95 +++++++++ .../azihsm_ossl_engine/src/keyload.rs | 182 ++++++++++++++++++ .../ossl_engine/azihsm_ossl_engine/src/lib.rs | 22 +++ .../ossl_engine/azihsm_ossl_engine/src/uri.rs | 151 +++++++++++++++ 5 files changed, 459 insertions(+), 4 deletions(-) create mode 100644 plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs create mode 100644 plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs diff --git a/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml b/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml index 37f8ef1d6..1b59c09f8 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml +++ b/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml @@ -29,11 +29,16 @@ zeroize.workspace = true [target.'cfg(unix)'.dependencies] libc.workspace = true -# Only the Linux-only `context` tests use these; gating keeps the workspace -# `--all-targets` build on Windows from pulling OpenSSL (openssl-sys won't build -# there). Mirrors azihsm_ossl_engine_resiliency. -[target.'cfg(target_os = "linux")'.dev-dependencies] +# openssl (→ openssl-sys) can't build on Windows and the engine is Linux-only, +# so gate it — mirrors azihsm_ossl_engine_resiliency. Used to reconstruct a +# public EVP_PKEY from the SDK's DER in the key loader. +[target.'cfg(target_os = "linux")'.dependencies] +foreign-types.workspace = true openssl.workspace = true + +# serial_test is only used by the Linux-only mock tests; keep it gated so the +# Windows `--all-targets` build stays clean. +[target.'cfg(target_os = "linux")'.dev-dependencies] serial_test.workspace = true [lints] diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs index 0dd2c58d2..14d00344c 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs @@ -90,12 +90,33 @@ impl EngineData { /// Open the HSM by reading settings + credentials from the process /// environment. + /// + /// Idempotent: if the HSM is already open (e.g. opened earlier via + /// [`open_hsm_with`]) this returns `Ok` without touching the environment, + /// so a caller like the key loader can invoke it unconditionally. pub fn open_hsm_from_env(&self) -> EngineResult<()> { + if self.is_hsm_open() { + return Ok(()); + } let settings = ResiliencySettings::from_env() .map_err(|e| EngineError::wrap("resiliency settings", e))?; let creds = credentials_from_env()?; self.open_hsm_with(settings, creds) } + + /// Run `f` with the open HSM session. The lock is held across `f`, so key + /// operations serialize with each other and with the open. Errors if the + /// HSM has not been opened. + pub fn with_session(&self, f: F) -> EngineResult + where + F: FnOnce(&HsmSession) -> EngineResult, + { + let guard = self.hsm.lock(); + let ctx = guard + .as_ref() + .ok_or_else(|| EngineError::Other("HSM session not open".into()))?; + f(&ctx.session) + } } /// Live HSM partition + session. @@ -412,10 +433,19 @@ mod tests { use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; + use azihsm_api::HsmEccCurve; + use azihsm_api::HsmEccKeyGenAlgo; + use azihsm_api::HsmKeyClass; + use azihsm_api::HsmKeyCommonProps; + use azihsm_api::HsmKeyKind; + use azihsm_api::HsmKeyManager; + use azihsm_api::HsmKeyPropsBuilder; + use foreign_types::ForeignType; use openssl::ec::EcGroup; use openssl::ec::EcKey; use openssl::nid::Nid; use openssl::pkey::PKey; + use openssl::pkey::Public; use serial_test::serial; use super::*; @@ -494,6 +524,71 @@ mod tests { assert!(data.is_hsm_open()); } + /// Full engine key-load round trip: generate an EC key + its masked blob in + /// the HSM, write the blob to disk, load it back through the engine's + /// `load_private_key` path, and verify the returned EVP_PKEY's public key + /// matches. Dropping the EVP_PKEY exercises the ex_data free callback (HSM + /// key deletion) — it must not crash. + #[test] + #[serial] + #[allow(unsafe_code)] + fn load_ec_key_round_trips_through_engine() { + let scratch = Scratch::new("load"); + // Leave the `res` storage dir to the open path (setup_storage_dir), + // which creates it mode 0700; pre-creating it here would inherit the + // umask and be rejected as insecure (see open_hsm_with_resiliency_succeeds). + let data = EngineData::new(); + data.open_hsm_with( + caller_settings(&scratch), + HsmCredentials::new(&DEFAULT_CRED_ID, &DEFAULT_CRED_PIN), + ) + .unwrap(); + + // Generate a persistent EC P-384 key in the HSM and export its masked + // blob (the form the engine loads). + let (masked, expected_pub_der) = data + .with_session(|session| { + let priv_props = HsmKeyPropsBuilder::default() + .class(HsmKeyClass::Private) + .key_kind(HsmKeyKind::Ecc) + .ecc_curve(HsmEccCurve::P384) + .is_session(false) + .can_sign(true) + .build() + .unwrap(); + let pub_props = HsmKeyPropsBuilder::default() + .class(HsmKeyClass::Public) + .key_kind(HsmKeyKind::Ecc) + .ecc_curve(HsmEccCurve::P384) + .is_session(false) + .can_verify(true) + .build() + .unwrap(); + let mut algo = HsmEccKeyGenAlgo::default(); + let (priv_key, _pub) = + HsmKeyManager::generate_key_pair(session, &mut algo, priv_props, pub_props) + .unwrap(); + Ok(( + priv_key.masked_key_vec().unwrap(), + priv_key.pub_key_der_vec().unwrap(), + )) + }) + .unwrap(); + + let blob_path = scratch.0.join("ec_key.bin"); + fs::write(&blob_path, &masked).unwrap(); + + let uri = format!("azihsm://{};type=ec", blob_path.display()); + let raw = crate::keyload::load_key(&data, &uri).unwrap(); + assert!(!raw.is_null()); + + // Take ownership of the returned EVP_PKEY, confirm its public key, then + // drop it — freeing the EVP_PKEY and running the ex_data free callback. + // SAFETY: raw is an owning *mut EVP_PKEY returned by load_key. + let loaded: PKey = unsafe { PKey::from_ptr(raw.cast()) }; + assert_eq!(loaded.public_key_to_der().unwrap(), expected_pub_der); + } + #[test] #[serial] fn open_hsm_is_idempotent() { diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs new file mode 100644 index 000000000..2eec6ec57 --- /dev/null +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs @@ -0,0 +1,182 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! `ENGINE_load_private_key` implementation: resolve an `azihsm://` URI to an +//! HSM key and return an `EVP_PKEY`. +//! +//! The returned key carries the real public key (rebuilt from the SDK's DER via +//! the safe `openssl` crate) plus the live HSM private-key wrapper, stashed in +//! the underlying `EC_KEY` ex_data so it stays alive for later sign/derive +//! callbacks and is deleted from the HSM when the `EVP_PKEY` is freed. The +//! sign/derive methods themselves are not wired yet (a later change), so a +//! loaded key is currently usable for public-key operations (e.g. `-pubout`). +//! +//! Only EC keys are supported here; RSA loading (an HSM import + unmask path) +//! lands in a follow-up together with its test coverage. + +use std::ffi::c_int; +use std::ffi::c_long; +use std::ffi::c_void; +use std::ptr::null_mut; +use std::sync::OnceLock; + +use azihsm_api::HsmEccKeyUnmaskAlgo; +use azihsm_api::HsmEccPrivateKey; +use azihsm_api::HsmKeyCommonProps; +use azihsm_api::HsmKeyManager; +use azihsm_ossl_engine_core::error::EngineError; +use azihsm_ossl_engine_core::error::EngineResult; +use azihsm_ossl_engine_core::error::catch_panic; +use azihsm_ossl_engine_core::ffi; +use foreign_types::ForeignTypeRef; +use openssl::pkey::PKey; +use parking_lot::Mutex; + +use crate::context::EngineData; +use crate::uri; +use crate::uri::KeyType; + +/// Load the private key named by `key_id` (an `azihsm://…` URI) and return an +/// owning `*mut EVP_PKEY`. +pub fn load_key(data: &EngineData, key_id: &str) -> EngineResult<*mut ffi::EVP_PKEY> { + let parsed = uri::parse(key_id)?; + + // First real caller of the lazy HSM open (idempotent). + data.open_hsm_from_env()?; + + let masked = std::fs::read(&parsed.masked_key_path).map_err(|e| { + EngineError::wrap( + format!("read masked key {}", parsed.masked_key_path.display()), + e, + ) + })?; + + match parsed.key_type { + KeyType::Ec => load_ec(data, &masked), + // RSA loading lands in a follow-up (it needs an HSM import path plus + // test coverage); recognize the type but reject it clearly for now. + KeyType::Rsa | KeyType::RsaPss => Err(EngineError::Other( + "RSA key loading is not yet supported".into(), + )), + } +} + +fn load_ec(data: &EngineData, masked: &[u8]) -> EngineResult<*mut ffi::EVP_PKEY> { + let priv_key = data.with_session(|session| { + let mut algo = HsmEccKeyUnmaskAlgo {}; + HsmKeyManager::unmask_key_pair(session, &mut algo, masked) + .map(|(private, _public)| private) + .map_err(|e| EngineError::wrap("EC key unmask", e)) + })?; + + let der = priv_key + .pub_key_der_vec() + .map_err(|e| EngineError::wrap("read EC public key DER", e))?; + + build_ec_pkey(&der, priv_key) +} + +/// Rebuild the public `EVP_PKEY` from `der` (safe `openssl` crate) and attach +/// the live HSM key to it before transferring ownership to OpenSSL. +fn build_ec_pkey(der: &[u8], key: HsmEccPrivateKey) -> EngineResult<*mut ffi::EVP_PKEY> { + let pkey = + PKey::public_key_from_der(der).map_err(|e| EngineError::wrap("parse public key DER", e))?; + + // The `openssl` crate's `EVP_PKEY` and the engine-ABI `EVP_PKEY` are the + // same C struct (both link the one system libcrypto), so casting the raw + // pointer at this single boundary is sound. + let raw = pkey.as_ptr().cast::(); + + // Attach while `pkey` still owns `raw`: if `attach_ec` fails, `pkey` drops + // at end of scope and frees the EVP_PKEY. + attach_ec(raw, key)?; + + // Success: hand ownership to OpenSSL. + std::mem::forget(pkey); + Ok(raw) +} + +#[allow(unsafe_code)] +fn attach_ec(pkey: *mut ffi::EVP_PKEY, key: HsmEccPrivateKey) -> EngineResult<()> { + let idx = ec_key_ex_index()?; + + // SAFETY: pkey is a valid EVP_PKEY holding an EC key (built above). + let ec = unsafe { ffi::EVP_PKEY_get0_EC_KEY(pkey) }; + if ec.is_null() { + return Err(EngineError::Other("EVP_PKEY has no EC_KEY".into())); + } + + let boxed = Box::into_raw(Box::new(key)).cast::(); + // SAFETY: ec is valid, idx is a registered EC_KEY ex_data slot, and boxed is + // a fresh Box::into_raw whose ownership passes to the ex_data slot on success. + let rc = unsafe { ffi::EC_KEY_set_ex_data(ec, idx, boxed) }; + if rc != 1 { + // Reclaim the box we failed to store so it is not leaked. + // SAFETY: boxed is the Box we just created and did not transfer. + drop(unsafe { Box::from_raw(boxed.cast::()) }); + return Err(EngineError::Other("EC_KEY_set_ex_data failed".into())); + } + Ok(()) +} + +/// Process-global EC_KEY ex_data slot index, registered once with +/// [`free_ec_key`] so freeing the EC_KEY deletes the HSM key. +#[allow(unsafe_code)] +fn ec_key_ex_index() -> EngineResult { + static IDX: OnceLock = OnceLock::new(); + static INIT: Mutex<()> = Mutex::new(()); + + if let Some(i) = IDX.get() { + return Ok(*i); + } + let _guard = INIT.lock(); + if let Some(i) = IDX.get() { + return Ok(*i); + } + // EC_KEY_get_ex_new_index is a macro in 1.1.x, so call the underlying + // CRYPTO_get_ex_new_index with the EC_KEY class directly. + // SAFETY: standard ex_data index registration; free_ec_key matches the + // CRYPTO_EX_free signature. + let idx = unsafe { + ffi::CRYPTO_get_ex_new_index( + ffi::CRYPTO_EX_INDEX_EC_KEY as c_int, + 0, + null_mut(), + None, + None, + Some(free_ec_key), + ) + }; + if idx < 0 { + return Err(EngineError::ExDataRegisterFailed); + } + let _ = IDX.set(idx); + Ok(idx) +} + +/// ex_data free callback: drops the `Box` (whose `Drop` +/// deletes the HSM key). Runs when the owning EC_KEY is freed via +/// `EVP_PKEY_free`. +/// +/// # Safety +/// Called by OpenSSL with the `ptr` previously stored via `EC_KEY_set_ex_data`. +#[allow(unsafe_code)] +unsafe extern "C" fn free_ec_key( + _parent: *mut c_void, + ptr: *mut c_void, + _ad: *mut ffi::CRYPTO_EX_DATA, + _idx: c_int, + _argl: c_long, + _argp: *mut c_void, +) { + if ptr.is_null() { + return; + } + catch_panic( + || { + // SAFETY: ptr is the Box stored via EC_KEY_set_ex_data. + drop(unsafe { Box::from_raw(ptr.cast::()) }); + }, + (), + ); +} diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/lib.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/lib.rs index f1373ad77..735e052d8 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/lib.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/lib.rs @@ -26,6 +26,12 @@ pub mod context; #[cfg(all(target_os = "linux", feature = "engine"))] mod logging; +#[cfg(all(target_os = "linux", feature = "engine"))] +mod uri; + +#[cfg(all(target_os = "linux", feature = "engine"))] +mod keyload; + #[cfg(all(target_os = "linux", feature = "engine"))] mod engine_impl { use std::ffi::CStr; @@ -36,6 +42,7 @@ mod engine_impl { use azihsm_ossl_engine_core::engine::DestroyHandler; use azihsm_ossl_engine_core::engine::Engine; + use azihsm_ossl_engine_core::engine::LoadPrivKeyHandler; use azihsm_ossl_engine_core::error::EngineError; use azihsm_ossl_engine_core::error::EngineResult; use azihsm_ossl_engine_core::error::RetCode; @@ -71,6 +78,20 @@ mod engine_impl { Ok(slot) } + struct AzihsmLoadPrivKey; + impl LoadPrivKeyHandler for AzihsmLoadPrivKey { + fn load(engine: &Engine, key_id: &CStr) -> EngineResult<*mut ffi::EVP_PKEY> { + let slot = engine_data_slot()?; + let data = slot + .get(engine) + .ok_or(EngineError::NullParam("engine_data"))?; + let key_id = key_id + .to_str() + .map_err(|_| EngineError::Other("key id is not valid UTF-8".into()))?; + crate::keyload::load_key(data, key_id) + } + } + struct AzihsmDestroy; impl DestroyHandler for AzihsmDestroy { fn destroy(engine: &mut Engine) -> EngineResult<()> { @@ -158,6 +179,7 @@ mod engine_impl { engine.set_id(ENGINE_ID)?; engine.set_name(ENGINE_NAME)?; engine.set_destroy::()?; + engine.set_load_privkey::()?; // Park an empty EngineData. Its HSM session is opened on demand via // EngineData::open_hsm_from_env; AzihsmDestroy::destroy takes() and diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs new file mode 100644 index 000000000..03e7a28e7 --- /dev/null +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Parser for `azihsm://` key URIs used by `ENGINE_load_private_key`. +//! +//! Grammar (mirrors the OpenSSL 3.x provider's STORE): +//! +//! ```text +//! azihsm://;type= +//! ``` +//! +//! The identifier is a filesystem path to a masked-key blob; `type=` is +//! mandatory. `aes` is a valid provider key type but is not loadable as a +//! private key here, so it is rejected. + +use std::path::PathBuf; + +use azihsm_ossl_engine_core::error::EngineError; +use azihsm_ossl_engine_core::error::EngineResult; + +const SCHEME: &str = "azihsm://"; + +/// Key type selected by the URI `type=` attribute. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum KeyType { + Ec, + Rsa, + RsaPss, +} + +/// A parsed `azihsm://;type=` key URI. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct KeyUri { + /// Path to the masked-key blob on disk. + pub masked_key_path: PathBuf, + /// Declared key type. + pub key_type: KeyType, +} + +/// Parse an `azihsm://` key URI. +pub fn parse(uri: &str) -> EngineResult { + let rest = uri + .strip_prefix(SCHEME) + .ok_or_else(|| EngineError::Other(format!("key id is not an {SCHEME} URI: {uri}")))?; + + let (path, attrs) = rest.split_once(';').unwrap_or((rest, "")); + if path.is_empty() { + return Err(EngineError::Other("azihsm URI has no key path".into())); + } + + let mut key_type = None; + for attr in attrs.split(';').filter(|a| !a.is_empty()) { + let (key, value) = attr + .split_once('=') + .ok_or_else(|| EngineError::Other(format!("malformed azihsm URI attribute: {attr}")))?; + match key { + "type" => key_type = Some(parse_key_type(value)?), + other => { + return Err(EngineError::Other(format!( + "unknown azihsm URI attribute: {other}" + ))); + } + } + } + + let key_type = key_type + .ok_or_else(|| EngineError::Other("azihsm URI missing required type= attribute".into()))?; + + Ok(KeyUri { + masked_key_path: PathBuf::from(path), + key_type, + }) +} + +fn parse_key_type(value: &str) -> EngineResult { + match value { + "ec" => Ok(KeyType::Ec), + "rsa" => Ok(KeyType::Rsa), + "rsa-pss" => Ok(KeyType::RsaPss), + "aes" => Err(EngineError::Other( + "aes keys cannot be loaded via load_private_key".into(), + )), + other => Err(EngineError::Other(format!( + "unknown azihsm key type: {other}" + ))), + } +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + + use super::*; + + #[test] + fn parses_relative_ec() { + let u = parse("azihsm://./key.bin;type=ec").unwrap(); + assert_eq!(u.masked_key_path, PathBuf::from("./key.bin")); + assert_eq!(u.key_type, KeyType::Ec); + } + + #[test] + fn parses_absolute_rsa() { + let u = parse("azihsm:///var/lib/azihsm/k.bin;type=rsa").unwrap(); + assert_eq!(u.masked_key_path, PathBuf::from("/var/lib/azihsm/k.bin")); + assert_eq!(u.key_type, KeyType::Rsa); + } + + #[test] + fn parses_rsa_pss() { + assert_eq!( + parse("azihsm://k;type=rsa-pss").unwrap().key_type, + KeyType::RsaPss + ); + } + + #[test] + fn rejects_missing_scheme() { + assert!(parse("./key.bin;type=ec").is_err()); + } + + #[test] + fn rejects_empty_path() { + assert!(parse("azihsm://;type=ec").is_err()); + } + + #[test] + fn rejects_missing_type() { + assert!(parse("azihsm://./key.bin").is_err()); + } + + #[test] + fn rejects_unknown_type() { + assert!(parse("azihsm://k;type=dsa").is_err()); + } + + #[test] + fn rejects_aes() { + assert!(parse("azihsm://k;type=aes").is_err()); + } + + #[test] + fn rejects_unknown_attribute() { + assert!(parse("azihsm://k;type=ec;foo=bar").is_err()); + } + + #[test] + fn rejects_malformed_attribute() { + assert!(parse("azihsm://k;type").is_err()); + } +} From 63cca2c86c824765b68abf1a3a7ce28f98accaa0 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Sat, 18 Jul 2026 21:01:39 +0200 Subject: [PATCH 3/8] test(engine): add an ignored hardware key-loading smoke test Add load_ec_key_from_env_smoke to the hw_tests module: open the HSM from the ambient AZIHSM_* environment, generate a persistent EC P-384 key on the device, export its masked blob, load it back through the engine's load_private_key path, and verify the returned EVP_PKEY's public key matches. Dropping the key exercises the ex_data free callback (HSM delete). This is the same round trip tests::load_ec_key_round_trips_through_engine covers against the mock, but against a real device. Like open_from_env_smoke it is compiled only in non-mock builds and #[ignore]d, so it never runs in the mock CI cell; invoke it explicitly on a provisioned host. Signed-off-by: Christian Walter --- .../azihsm_ossl_engine/src/context.rs | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs index 14d00344c..7fe702617 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs @@ -666,6 +666,17 @@ mod tests { /// ``` #[cfg(all(test, not(feature = "mock")))] mod hw_tests { + use azihsm_api::HsmEccCurve; + use azihsm_api::HsmEccKeyGenAlgo; + use azihsm_api::HsmKeyClass; + use azihsm_api::HsmKeyCommonProps; + use azihsm_api::HsmKeyKind; + use azihsm_api::HsmKeyManager; + use azihsm_api::HsmKeyPropsBuilder; + use foreign_types::ForeignType; + use openssl::pkey::PKey; + use openssl::pkey::Public; + use super::*; #[test] @@ -679,4 +690,79 @@ mod hw_tests { ); Ok(()) } + + /// Hardware key-loading round trip. Generates a persistent EC P-384 key on + /// the device, exports its masked blob, loads it back through the engine's + /// `load_private_key` path, and verifies the returned EVP_PKEY's public key + /// matches. Dropping the EVP_PKEY exercises the ex_data free callback (HSM + /// key deletion). `tests::load_ec_key_round_trips_through_engine` covers this + /// against the mock; this runs the same cycle on a real device. + /// + /// Same env setup as `open_from_env_smoke` (configure `AZIHSM_*` first): + /// + /// ```text + /// cargo test -p azihsm_ossl_engine --features engine load_ec_key_from_env_smoke -- --ignored --nocapture + /// ``` + #[test] + #[ignore = "requires a provisioned HSM host; configure AZIHSM_* env first"] + #[allow(unsafe_code)] + fn load_ec_key_from_env_smoke() -> EngineResult<()> { + let data = EngineData::new(); + data.open_hsm_from_env()?; + + // Generate a persistent EC P-384 key on the device and export the masked + // blob (the form the engine loads). + let (masked, expected_pub_der) = data.with_session(|session| { + let priv_props = HsmKeyPropsBuilder::default() + .class(HsmKeyClass::Private) + .key_kind(HsmKeyKind::Ecc) + .ecc_curve(HsmEccCurve::P384) + .is_session(false) + .can_sign(true) + .build() + .map_err(|e| EngineError::wrap("build private key props", e))?; + let pub_props = HsmKeyPropsBuilder::default() + .class(HsmKeyClass::Public) + .key_kind(HsmKeyKind::Ecc) + .ecc_curve(HsmEccCurve::P384) + .is_session(false) + .can_verify(true) + .build() + .map_err(|e| EngineError::wrap("build public key props", e))?; + let mut algo = HsmEccKeyGenAlgo::default(); + let (priv_key, _pub) = + HsmKeyManager::generate_key_pair(session, &mut algo, priv_props, pub_props) + .map_err(|e| EngineError::wrap("generate EC key pair", e))?; + Ok(( + priv_key + .masked_key_vec() + .map_err(|e| EngineError::wrap("export masked key", e))?, + priv_key + .pub_key_der_vec() + .map_err(|e| EngineError::wrap("read public key DER", e))?, + )) + })?; + + let blob_path = + std::env::temp_dir().join(format!("engine-hw-ec-{}.bin", std::process::id())); + std::fs::write(&blob_path, &masked).map_err(|e| { + EngineError::wrap(format!("write masked blob {}", blob_path.display()), e) + })?; + + let uri = format!("azihsm://{};type=ec", blob_path.display()); + let raw = crate::keyload::load_key(&data, &uri)?; + assert!(!raw.is_null(), "load_key returned a NULL EVP_PKEY"); + + // Take ownership of the returned key, confirm its public half, then drop + // it — freeing the EVP_PKEY runs the ex_data free callback (HSM delete). + // SAFETY: raw is an owning *mut EVP_PKEY returned by load_key. + let loaded: PKey = unsafe { PKey::from_ptr(raw.cast()) }; + let loaded_der = loaded + .public_key_to_der() + .map_err(|e| EngineError::wrap("encode loaded public key", e))?; + assert_eq!(loaded_der, expected_pub_der, "loaded public key mismatch"); + + let _ = std::fs::remove_file(&blob_path); + Ok(()) + } } From 05be7613a92f341f8e0c7856b0c1b5fcc780a9d1 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Sat, 18 Jul 2026 22:33:36 +0200 Subject: [PATCH 4/8] fix(engine): report a null EVP_PKEY as failure in result_to_ptr result_to_ptr returned Ok(ptr) verbatim, so an Ok(NULL) would reach a pointer-returning OpenSSL callback as a bare NULL with nothing on the ERR queue, contradicting the helper's documented contract. Treat Ok(NULL) like Err: push a reason onto the ERR queue, log, and return NULL. Signed-off-by: Christian Walter --- .../azihsm_ossl_engine_core/src/error.rs | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs b/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs index f685e27ad..f5f48e495 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine_core/src/error.rs @@ -171,19 +171,22 @@ pub fn result_to_int(result: EngineResult) -> c_int { /// Convert an [`EngineResult`] holding a raw pointer into that pointer, for a /// C callback that returns `*mut T` (e.g. the `load_privkey` hook returning -/// `*mut EVP_PKEY`). On `Err`, push the message onto the OpenSSL ERR queue, -/// log via `tracing`, and return NULL — the pointer-returning analogue of -/// [`result_to_int`]. +/// `*mut EVP_PKEY`). Only a non-null `Ok` is success. On `Err` — or on an `Ok` +/// holding NULL, which a pointer-returning OpenSSL callback still reports as +/// failure — push the message onto the OpenSSL ERR queue, log via `tracing`, +/// and return NULL. The pointer-returning analogue of [`result_to_int`]. pub fn result_to_ptr(result: EngineResult<*mut T>) -> *mut T { - match result { - Ok(ptr) => ptr, - Err(e) => { - let reason = c_int::from(&e); - openssl_err(reason, &e.to_string()); - tracing::error!("{e}"); - std::ptr::null_mut() - } - } + let err = match result { + Ok(ptr) if !ptr.is_null() => return ptr, + // A NULL success is a contract violation: fall through so the caller + // still gets a reason on the ERR queue instead of a bare NULL. + Ok(_) => EngineError::Other("callback produced a null pointer".into()), + Err(e) => e, + }; + let reason = c_int::from(&err); + openssl_err(reason, &err.to_string()); + tracing::error!("{err}"); + std::ptr::null_mut() } /// Run `f` under [`catch_unwind`]. A panic is logged and turned into the From bf629cdd3acda513c17eaa3ad8272c6446fb3bf7 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Sat, 18 Jul 2026 22:33:46 +0200 Subject: [PATCH 5/8] fix(engine): harden EC key loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Own loaded HSM keys in EngineData and delete them from the engine's destroy handler instead of registering an EC_KEY ex_data free callback. A libcrypto-held free callback holds a function pointer into this .so and can fire after the .so is unloaded (the hazard exdata.rs already avoids for ENGINE ex_data). EngineData now owns each loaded key and drops it — deleting it from the HSM — while the .so is still loaded; the ex_data slot keeps only a non-owning pointer, with no free callback. - Read the masked-key blob with O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC and a regular-file check, matching the hardening used elsewhere for key-material files, instead of a bare std::fs::read that follows symlinks. - Reject unsupported key types before any HSM open or file I/O, so they fail with a clear error instead of a misleading environment/filesystem one. - Gate the openssl/foreign-types dependencies behind the engine feature so the crate's manifest declares them only where they are used. Signed-off-by: Christian Walter --- .../ossl_engine/azihsm_ossl_engine/Cargo.toml | 13 +- .../azihsm_ossl_engine/src/context.rs | 44 +++++- .../azihsm_ossl_engine/src/keyload.rs | 143 ++++++++++-------- 3 files changed, 125 insertions(+), 75 deletions(-) diff --git a/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml b/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml index 1b59c09f8..9aa72c6b7 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml +++ b/plugins/ossl_engine/azihsm_ossl_engine/Cargo.toml @@ -12,7 +12,7 @@ crate-type = ["cdylib", "lib"] [features] default = [] -engine = ["azihsm_ossl_engine_core/engine"] +engine = ["azihsm_ossl_engine_core/engine", "dep:foreign-types", "dep:openssl"] mock = ["azihsm_api/mock"] [dependencies] @@ -29,12 +29,13 @@ zeroize.workspace = true [target.'cfg(unix)'.dependencies] libc.workspace = true -# openssl (→ openssl-sys) can't build on Windows and the engine is Linux-only, -# so gate it — mirrors azihsm_ossl_engine_resiliency. Used to reconstruct a -# public EVP_PKEY from the SDK's DER in the key loader. +# openssl (→ openssl-sys) can't build on Windows and is only used by the +# key loader (to reconstruct a public EVP_PKEY from the SDK's DER), which is +# entirely behind the `engine` feature. Keep it optional and enabled by that +# feature so a non-engine build stays a stub and pulls no OpenSSL toolchain. [target.'cfg(target_os = "linux")'.dependencies] -foreign-types.workspace = true -openssl.workspace = true +foreign-types = { optional = true, workspace = true } +openssl = { optional = true, workspace = true } # serial_test is only used by the Linux-only mock tests; keep it gated so the # Windows `--all-targets` build stays clean. diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs index 7fe702617..d8b2594cd 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs @@ -10,6 +10,7 @@ use std::path::Path; use azihsm_api::HsmCredentials; +use azihsm_api::HsmEccPrivateKey; use azihsm_api::HsmError; use azihsm_api::HsmOwnerBackupKey; use azihsm_api::HsmOwnerBackupKeyConfig; @@ -48,6 +49,19 @@ const ENV_CREDENTIALS_PIN: &str = "AZIHSM_CREDENTIALS_PIN"; /// Per-engine state stored in `ENGINE` ex_data. pub struct EngineData { + /// HSM private keys loaded via the engine's `load_private_key` path. Owned + /// here — never via an `EC_KEY` ex_data free callback, which would leave a + /// libcrypto-held function pointer into this `.so` (see the module docs of + /// [`azihsm_ossl_engine_core::exdata`]). Each key's `Drop` deletes it from + /// the HSM; that runs when this `EngineData` is dropped by the destroy + /// handler, while the `.so` is loaded. Declared before `hsm` so it drops + /// first, while the session is still open. + // + // `Box` gives each key a stable heap address: keyload stashes a raw pointer + // to it in EC_KEY ex_data, which must stay valid across `Vec` growth — so + // the `vec_box` suggestion to drop the `Box` does not apply. + #[allow(clippy::vec_box)] + loaded_keys: Mutex>>, hsm: Mutex>, } @@ -60,6 +74,7 @@ impl Default for EngineData { impl EngineData { pub fn new() -> Self { Self { + loaded_keys: Mutex::new(Vec::new()), hsm: Mutex::new(None), } } @@ -117,6 +132,18 @@ impl EngineData { .ok_or_else(|| EngineError::Other("HSM session not open".into()))?; f(&ctx.session) } + + /// Take ownership of a loaded HSM private key so it lives until the engine + /// is destroyed (its `Drop` deletes it from the HSM), and return a stable + /// non-owning pointer to it for stashing in `EC_KEY` ex_data. Boxing keeps + /// the address stable across `Vec` growth; the returned pointer stays valid + /// until this `EngineData` is dropped by the destroy handler. + pub fn retain_loaded_key(&self, key: HsmEccPrivateKey) -> *const HsmEccPrivateKey { + let boxed = Box::new(key); + let ptr: *const HsmEccPrivateKey = boxed.as_ref(); + self.loaded_keys.lock().push(boxed); + ptr + } } /// Live HSM partition + session. @@ -527,8 +554,8 @@ mod tests { /// Full engine key-load round trip: generate an EC key + its masked blob in /// the HSM, write the blob to disk, load it back through the engine's /// `load_private_key` path, and verify the returned EVP_PKEY's public key - /// matches. Dropping the EVP_PKEY exercises the ex_data free callback (HSM - /// key deletion) — it must not crash. + /// matches. Dropping the EVP_PKEY (no ex_data free callback) and then the + /// EngineData (which deletes the loaded key from the HSM) must not crash. #[test] #[serial] #[allow(unsafe_code)] @@ -583,7 +610,8 @@ mod tests { assert!(!raw.is_null()); // Take ownership of the returned EVP_PKEY, confirm its public key, then - // drop it — freeing the EVP_PKEY and running the ex_data free callback. + // drop it. The EC_KEY ex_data slot has no free callback, so this frees + // only the OpenSSL objects; the HSM key is deleted when `data` drops. // SAFETY: raw is an owning *mut EVP_PKEY returned by load_key. let loaded: PKey = unsafe { PKey::from_ptr(raw.cast()) }; assert_eq!(loaded.public_key_to_der().unwrap(), expected_pub_der); @@ -694,9 +722,10 @@ mod hw_tests { /// Hardware key-loading round trip. Generates a persistent EC P-384 key on /// the device, exports its masked blob, loads it back through the engine's /// `load_private_key` path, and verifies the returned EVP_PKEY's public key - /// matches. Dropping the EVP_PKEY exercises the ex_data free callback (HSM - /// key deletion). `tests::load_ec_key_round_trips_through_engine` covers this - /// against the mock; this runs the same cycle on a real device. + /// matches. Dropping the EVP_PKEY, then the EngineData (which deletes the + /// loaded key from the HSM), must not crash. + /// `tests::load_ec_key_round_trips_through_engine` covers this against the + /// mock; this runs the same cycle on a real device. /// /// Same env setup as `open_from_env_smoke` (configure `AZIHSM_*` first): /// @@ -754,7 +783,8 @@ mod hw_tests { assert!(!raw.is_null(), "load_key returned a NULL EVP_PKEY"); // Take ownership of the returned key, confirm its public half, then drop - // it — freeing the EVP_PKEY runs the ex_data free callback (HSM delete). + // it. The ex_data slot has no free callback; the HSM key is deleted when + // `data` drops at the end of the test. // SAFETY: raw is an owning *mut EVP_PKEY returned by load_key. let loaded: PKey = unsafe { PKey::from_ptr(raw.cast()) }; let loaded_der = loaded diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs index 2eec6ec57..3c969c904 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs @@ -5,9 +5,13 @@ //! HSM key and return an `EVP_PKEY`. //! //! The returned key carries the real public key (rebuilt from the SDK's DER via -//! the safe `openssl` crate) plus the live HSM private-key wrapper, stashed in -//! the underlying `EC_KEY` ex_data so it stays alive for later sign/derive -//! callbacks and is deleted from the HSM when the `EVP_PKEY` is freed. The +//! the safe `openssl` crate). The live HSM private-key wrapper is owned by +//! [`EngineData`], which deletes it from the HSM when the engine is destroyed; +//! a non-owning pointer to it is stashed in the underlying `EC_KEY` ex_data for +//! later sign/derive callbacks. Deletion runs from the engine's destroy handler +//! (while this `.so` is loaded), never from a libcrypto ex_data free callback — +//! such a callback holds a function pointer into this `.so` and can be invoked +//! after the `.so` is unloaded (see [`azihsm_ossl_engine_core::exdata`]). The //! sign/derive methods themselves are not wired yet (a later change), so a //! loaded key is currently usable for public-key operations (e.g. `-pubout`). //! @@ -15,8 +19,11 @@ //! lands in a follow-up together with its test coverage. use std::ffi::c_int; -use std::ffi::c_long; use std::ffi::c_void; +use std::fs::OpenOptions; +use std::io::Read; +use std::os::unix::fs::OpenOptionsExt; +use std::path::Path; use std::ptr::null_mut; use std::sync::OnceLock; @@ -26,7 +33,6 @@ use azihsm_api::HsmKeyCommonProps; use azihsm_api::HsmKeyManager; use azihsm_ossl_engine_core::error::EngineError; use azihsm_ossl_engine_core::error::EngineResult; -use azihsm_ossl_engine_core::error::catch_panic; use azihsm_ossl_engine_core::ffi; use foreign_types::ForeignTypeRef; use openssl::pkey::PKey; @@ -41,24 +47,52 @@ use crate::uri::KeyType; pub fn load_key(data: &EngineData, key_id: &str) -> EngineResult<*mut ffi::EVP_PKEY> { let parsed = uri::parse(key_id)?; + // Reject unsupported key types before any HSM open or file I/O, so they + // fail with a clear error instead of a misleading environment/filesystem + // one. RSA loading lands in a follow-up (it needs an HSM import path plus + // test coverage). + match parsed.key_type { + KeyType::Ec => {} + KeyType::Rsa | KeyType::RsaPss => { + return Err(EngineError::Other( + "RSA key loading is not yet supported".into(), + )); + } + } + // First real caller of the lazy HSM open (idempotent). data.open_hsm_from_env()?; + let masked = read_masked_key(&parsed.masked_key_path)?; + load_ec(data, &masked) +} - let masked = std::fs::read(&parsed.masked_key_path).map_err(|e| { - EngineError::wrap( - format!("read masked key {}", parsed.masked_key_path.display()), - e, - ) - })?; - - match parsed.key_type { - KeyType::Ec => load_ec(data, &masked), - // RSA loading lands in a follow-up (it needs an HSM import path plus - // test coverage); recognize the type but reject it clearly for now. - KeyType::Rsa | KeyType::RsaPss => Err(EngineError::Other( - "RSA key loading is not yet supported".into(), - )), +/// Read a masked-key blob with the same hardening the rest of the engine +/// applies to key-material files: `O_NOFOLLOW` refuses a symlink at `path`, +/// `O_NONBLOCK` avoids blocking on a FIFO/device before the regular-file check, +/// `O_CLOEXEC` keeps the fd from leaking across exec, and a non-regular file is +/// rejected outright. Mirrors the hardened open in `logging` and +/// `read_regular_hardened` in azihsm_ossl_engine_resiliency. +fn read_masked_key(path: &Path) -> EngineResult> { + let mut file = OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK | libc::O_CLOEXEC) + .open(path) + .map_err(|e| EngineError::wrap(format!("open masked key {}", path.display()), e))?; + + let meta = file + .metadata() + .map_err(|e| EngineError::wrap(format!("stat masked key {}", path.display()), e))?; + if !meta.is_file() { + return Err(EngineError::Other(format!( + "masked key {} is not a regular file", + path.display() + ))); } + + let mut buf = Vec::new(); + file.read_to_end(&mut buf) + .map_err(|e| EngineError::wrap(format!("read masked key {}", path.display()), e))?; + Ok(buf) } fn load_ec(data: &EngineData, masked: &[u8]) -> EngineResult<*mut ffi::EVP_PKEY> { @@ -73,12 +107,16 @@ fn load_ec(data: &EngineData, masked: &[u8]) -> EngineResult<*mut ffi::EVP_PKEY> .pub_key_der_vec() .map_err(|e| EngineError::wrap("read EC public key DER", e))?; - build_ec_pkey(&der, priv_key) + build_ec_pkey(data, &der, priv_key) } /// Rebuild the public `EVP_PKEY` from `der` (safe `openssl` crate) and attach /// the live HSM key to it before transferring ownership to OpenSSL. -fn build_ec_pkey(der: &[u8], key: HsmEccPrivateKey) -> EngineResult<*mut ffi::EVP_PKEY> { +fn build_ec_pkey( + data: &EngineData, + der: &[u8], + key: HsmEccPrivateKey, +) -> EngineResult<*mut ffi::EVP_PKEY> { let pkey = PKey::public_key_from_der(der).map_err(|e| EngineError::wrap("parse public key DER", e))?; @@ -89,7 +127,7 @@ fn build_ec_pkey(der: &[u8], key: HsmEccPrivateKey) -> EngineResult<*mut ffi::EV // Attach while `pkey` still owns `raw`: if `attach_ec` fails, `pkey` drops // at end of scope and frees the EVP_PKEY. - attach_ec(raw, key)?; + attach_ec(data, raw, key)?; // Success: hand ownership to OpenSSL. std::mem::forget(pkey); @@ -97,7 +135,11 @@ fn build_ec_pkey(der: &[u8], key: HsmEccPrivateKey) -> EngineResult<*mut ffi::EV } #[allow(unsafe_code)] -fn attach_ec(pkey: *mut ffi::EVP_PKEY, key: HsmEccPrivateKey) -> EngineResult<()> { +fn attach_ec( + data: &EngineData, + pkey: *mut ffi::EVP_PKEY, + key: HsmEccPrivateKey, +) -> EngineResult<()> { let idx = ec_key_ex_index()?; // SAFETY: pkey is a valid EVP_PKEY holding an EC key (built above). @@ -106,21 +148,26 @@ fn attach_ec(pkey: *mut ffi::EVP_PKEY, key: HsmEccPrivateKey) -> EngineResult<() return Err(EngineError::Other("EVP_PKEY has no EC_KEY".into())); } - let boxed = Box::into_raw(Box::new(key)).cast::(); - // SAFETY: ec is valid, idx is a registered EC_KEY ex_data slot, and boxed is - // a fresh Box::into_raw whose ownership passes to the ex_data slot on success. - let rc = unsafe { ffi::EC_KEY_set_ex_data(ec, idx, boxed) }; + // Hand the key to EngineData, which owns it and deletes it from the HSM when + // the engine is destroyed (while this `.so` is loaded). Stash the non-owning + // pointer it returns for later sign/derive retrieval. The ex_data slot has + // no free callback (see ec_key_ex_index), so libcrypto never calls back into + // this `.so` for cleanup — even if the EVP_PKEY outlives the engine. + let ptr = data.retain_loaded_key(key).cast_mut().cast::(); + // SAFETY: ec is valid, idx is a registered EC_KEY ex_data slot, and ptr is a + // non-owning pointer into a key EngineData keeps alive for the engine's + // lifetime; the slot has no free callback, so no ownership is transferred. + let rc = unsafe { ffi::EC_KEY_set_ex_data(ec, idx, ptr) }; if rc != 1 { - // Reclaim the box we failed to store so it is not leaked. - // SAFETY: boxed is the Box we just created and did not transfer. - drop(unsafe { Box::from_raw(boxed.cast::()) }); return Err(EngineError::Other("EC_KEY_set_ex_data failed".into())); } Ok(()) } -/// Process-global EC_KEY ex_data slot index, registered once with -/// [`free_ec_key`] so freeing the EC_KEY deletes the HSM key. +/// Process-global EC_KEY ex_data slot index, registered once **without** a free +/// callback: the stored pointer is non-owning (the key is owned by +/// [`EngineData`]), and a libcrypto-held free callback into this `.so` would be +/// unsafe across `.so` unload (see [`azihsm_ossl_engine_core::exdata`]). #[allow(unsafe_code)] fn ec_key_ex_index() -> EngineResult { static IDX: OnceLock = OnceLock::new(); @@ -135,8 +182,7 @@ fn ec_key_ex_index() -> EngineResult { } // EC_KEY_get_ex_new_index is a macro in 1.1.x, so call the underlying // CRYPTO_get_ex_new_index with the EC_KEY class directly. - // SAFETY: standard ex_data index registration; free_ec_key matches the - // CRYPTO_EX_free signature. + // SAFETY: standard ex_data index registration with no new/dup/free callbacks. let idx = unsafe { ffi::CRYPTO_get_ex_new_index( ffi::CRYPTO_EX_INDEX_EC_KEY as c_int, @@ -144,7 +190,7 @@ fn ec_key_ex_index() -> EngineResult { null_mut(), None, None, - Some(free_ec_key), + None, ) }; if idx < 0 { @@ -153,30 +199,3 @@ fn ec_key_ex_index() -> EngineResult { let _ = IDX.set(idx); Ok(idx) } - -/// ex_data free callback: drops the `Box` (whose `Drop` -/// deletes the HSM key). Runs when the owning EC_KEY is freed via -/// `EVP_PKEY_free`. -/// -/// # Safety -/// Called by OpenSSL with the `ptr` previously stored via `EC_KEY_set_ex_data`. -#[allow(unsafe_code)] -unsafe extern "C" fn free_ec_key( - _parent: *mut c_void, - ptr: *mut c_void, - _ad: *mut ffi::CRYPTO_EX_DATA, - _idx: c_int, - _argl: c_long, - _argp: *mut c_void, -) { - if ptr.is_null() { - return; - } - catch_panic( - || { - // SAFETY: ptr is the Box stored via EC_KEY_set_ex_data. - drop(unsafe { Box::from_raw(ptr.cast::()) }); - }, - (), - ); -} From f6f0b8c9c51739428ed57daaf463c1d1e982f31b Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Sun, 19 Jul 2026 12:02:13 +0200 Subject: [PATCH 6/8] fix(engine): case-insensitive URI attributes and a capped masked-key read - Parse the azihsm:// URI attribute name and value case-insensitively, matching the provider's STORE (which compares with strcasecmp), so e.g. TYPE=EC parses the same as type=ec. - Cap read_masked_key at 64 KiB (a metadata pre-check plus a bounded read), mirroring read_regular_hardened in azihsm_ossl_engine_resiliency, so a caller-supplied path to a very large or unbounded file cannot drive an unbounded allocation. Signed-off-by: Christian Walter --- .../azihsm_ossl_engine/src/keyload.rs | 32 ++++++++++++++++--- .../ossl_engine/azihsm_ossl_engine/src/uri.rs | 31 ++++++++++++++---- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs index 3c969c904..c921588f7 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs @@ -66,14 +66,20 @@ pub fn load_key(data: &EngineData, key_id: &str) -> EngineResult<*mut ffi::EVP_P load_ec(data, &masked) } +/// Upper bound on a masked-key blob. A masked EC/RSA key is far smaller; this +/// bounds allocation against a caller-supplied path pointing at a very large or +/// unbounded file. Mirrors `MAX_STORAGE_FILE_SIZE` in +/// azihsm_ossl_engine_resiliency. +const MAX_MASKED_KEY_SIZE: u64 = 64 * 1024; + /// Read a masked-key blob with the same hardening the rest of the engine /// applies to key-material files: `O_NOFOLLOW` refuses a symlink at `path`, /// `O_NONBLOCK` avoids blocking on a FIFO/device before the regular-file check, -/// `O_CLOEXEC` keeps the fd from leaking across exec, and a non-regular file is -/// rejected outright. Mirrors the hardened open in `logging` and -/// `read_regular_hardened` in azihsm_ossl_engine_resiliency. +/// `O_CLOEXEC` keeps the fd from leaking across exec, a non-regular file is +/// rejected outright, and the read is capped at [`MAX_MASKED_KEY_SIZE`]. +/// Mirrors `read_regular_hardened` in azihsm_ossl_engine_resiliency. fn read_masked_key(path: &Path) -> EngineResult> { - let mut file = OpenOptions::new() + let file = OpenOptions::new() .read(true) .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK | libc::O_CLOEXEC) .open(path) @@ -88,10 +94,26 @@ fn read_masked_key(path: &Path) -> EngineResult> { path.display() ))); } + if meta.len() > MAX_MASKED_KEY_SIZE { + return Err(EngineError::Other(format!( + "masked key {} is too large ({} bytes, max {MAX_MASKED_KEY_SIZE})", + path.display(), + meta.len() + ))); + } + // Cap the read too: metadata can under-report (a growing or virtual file), + // so bound the bytes actually read and reject anything over the limit. let mut buf = Vec::new(); - file.read_to_end(&mut buf) + file.take(MAX_MASKED_KEY_SIZE + 1) + .read_to_end(&mut buf) .map_err(|e| EngineError::wrap(format!("read masked key {}", path.display()), e))?; + if buf.len() as u64 > MAX_MASKED_KEY_SIZE { + return Err(EngineError::Other(format!( + "masked key {} exceeds {MAX_MASKED_KEY_SIZE} bytes", + path.display() + ))); + } Ok(buf) } diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs index 03e7a28e7..fc16e6755 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/uri.rs @@ -53,11 +53,13 @@ pub fn parse(uri: &str) -> EngineResult { let (key, value) = attr .split_once('=') .ok_or_else(|| EngineError::Other(format!("malformed azihsm URI attribute: {attr}")))?; - match key { + // Attribute names are case-insensitive, mirroring the provider's STORE + // (which compares with `strcasecmp`). + match key.to_ascii_lowercase().as_str() { "type" => key_type = Some(parse_key_type(value)?), - other => { + _ => { return Err(EngineError::Other(format!( - "unknown azihsm URI attribute: {other}" + "unknown azihsm URI attribute: {key}" ))); } } @@ -73,15 +75,16 @@ pub fn parse(uri: &str) -> EngineResult { } fn parse_key_type(value: &str) -> EngineResult { - match value { + // Values are case-insensitive, mirroring the provider's STORE (`strcasecmp`). + match value.to_ascii_lowercase().as_str() { "ec" => Ok(KeyType::Ec), "rsa" => Ok(KeyType::Rsa), "rsa-pss" => Ok(KeyType::RsaPss), "aes" => Err(EngineError::Other( "aes keys cannot be loaded via load_private_key".into(), )), - other => Err(EngineError::Other(format!( - "unknown azihsm key type: {other}" + _ => Err(EngineError::Other(format!( + "unknown azihsm key type: {value}" ))), } } @@ -114,6 +117,22 @@ mod tests { ); } + // The provider's STORE compares the attribute name and value with + // strcasecmp, so mixed case must parse the same here. + #[test] + fn parses_uppercase_attr_name_and_value() { + let u = parse("azihsm://./key.bin;TYPE=EC").unwrap(); + assert_eq!(u.key_type, KeyType::Ec); + } + + #[test] + fn parses_mixed_case_rsa_pss() { + assert_eq!( + parse("azihsm://k;Type=Rsa-PSS").unwrap().key_type, + KeyType::RsaPss + ); + } + #[test] fn rejects_missing_scheme() { assert!(parse("./key.bin;type=ec").is_err()); From 9611099e1d22ce4c38ea9f9884b3f3ea7c380857 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Sun, 19 Jul 2026 12:02:13 +0200 Subject: [PATCH 7/8] test(engine): stage masked-key blobs with hardened owner-only writes The round-trip tests wrote the masked-key blob with std::fs::write, which honors the caller's umask and can clobber or follow a symlink at the path. Add a write_key_material helper (create-new, mode 0600, O_NOFOLLOW|O_CLOEXEC) and use it in both the mock and hardware round-trip tests. Signed-off-by: Christian Walter --- .../azihsm_ossl_engine/src/context.rs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs index d8b2594cd..219f3aeb0 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs @@ -451,6 +451,24 @@ fn hex_decode_16(s: &str, var: &'static str) -> EngineResult<[u8; 16]> { Ok(out) } +/// Test helper: write key-material bytes to `path` with owner-only permissions +/// and the same open hardening the engine uses for secret files — create a new +/// file (`O_EXCL`, no clobber) mode 0600 and refuse a symlink at the path +/// (`O_NOFOLLOW`). Used by the round-trip tests to stage a masked-key blob. +#[cfg(test)] +fn write_key_material(path: &Path, data: &[u8]) -> std::io::Result<()> { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(SECRET_FILE_MODE) + .custom_flags(libc::O_NOFOLLOW | libc::O_CLOEXEC) + .open(path)?; + file.write_all(data) +} + #[cfg(all(test, feature = "mock"))] mod tests { #![allow(clippy::unwrap_used)] @@ -603,7 +621,7 @@ mod tests { .unwrap(); let blob_path = scratch.0.join("ec_key.bin"); - fs::write(&blob_path, &masked).unwrap(); + write_key_material(&blob_path, &masked).unwrap(); let uri = format!("azihsm://{};type=ec", blob_path.display()); let raw = crate::keyload::load_key(&data, &uri).unwrap(); @@ -774,7 +792,10 @@ mod hw_tests { let blob_path = std::env::temp_dir().join(format!("engine-hw-ec-{}.bin", std::process::id())); - std::fs::write(&blob_path, &masked).map_err(|e| { + // Clear any leftover from a prior crashed run so the O_EXCL create in + // write_key_material succeeds. + let _ = std::fs::remove_file(&blob_path); + write_key_material(&blob_path, &masked).map_err(|e| { EngineError::wrap(format!("write masked blob {}", blob_path.display()), e) })?; From 76037d1c75fc579fff9569c7fc6d862b57c1c771 Mon Sep 17 00:00:00 2001 From: Christian Walter Date: Sun, 19 Jul 2026 14:36:09 +0200 Subject: [PATCH 8/8] fix(engine): tighten key-loader visibility and roll back on attach failure - Make EngineData::with_session and retain_loaded_key pub(crate): they are internal to the key loader (with_session hands out a raw HsmSession, and retain_loaded_key is an implementation detail), not public engine API. - Add EngineData::release_loaded_key to drop a retained key by pointer, and call it from attach_ec when EC_KEY_set_ex_data fails, so a failed load no longer leaves the key in the HSM until engine teardown. Signed-off-by: Christian Walter --- .../azihsm_ossl_engine/src/context.rs | 24 ++++++++++++++++--- .../azihsm_ossl_engine/src/keyload.rs | 11 +++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs index 219f3aeb0..8732410c2 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/context.rs @@ -122,7 +122,10 @@ impl EngineData { /// Run `f` with the open HSM session. The lock is held across `f`, so key /// operations serialize with each other and with the open. Errors if the /// HSM has not been opened. - pub fn with_session(&self, f: F) -> EngineResult + /// + /// Crate-internal: it hands out a raw `HsmSession`, so it is used only by + /// the key loader (and tests), not exposed as public engine API. + pub(crate) fn with_session(&self, f: F) -> EngineResult where F: FnOnce(&HsmSession) -> EngineResult, { @@ -137,13 +140,28 @@ impl EngineData { /// is destroyed (its `Drop` deletes it from the HSM), and return a stable /// non-owning pointer to it for stashing in `EC_KEY` ex_data. Boxing keeps /// the address stable across `Vec` growth; the returned pointer stays valid - /// until this `EngineData` is dropped by the destroy handler. - pub fn retain_loaded_key(&self, key: HsmEccPrivateKey) -> *const HsmEccPrivateKey { + /// until this `EngineData` is dropped by the destroy handler, or until + /// [`release_loaded_key`](Self::release_loaded_key) drops it early. + /// + /// Crate-internal: an implementation detail of the key loader. + pub(crate) fn retain_loaded_key(&self, key: HsmEccPrivateKey) -> *const HsmEccPrivateKey { let boxed = Box::new(key); let ptr: *const HsmEccPrivateKey = boxed.as_ref(); self.loaded_keys.lock().push(boxed); ptr } + + /// Drop a key previously handed to [`retain_loaded_key`](Self::retain_loaded_key), + /// identified by the pointer it returned. Used to roll back on a partial + /// load failure (e.g. if stashing the ex_data pointer fails) so the key + /// doesn't linger in the HSM until engine teardown. A no-op if the pointer + /// isn't currently retained. Dropping the box deletes the key from the HSM. + pub(crate) fn release_loaded_key(&self, ptr: *const HsmEccPrivateKey) { + let mut keys = self.loaded_keys.lock(); + if let Some(pos) = keys.iter().position(|k| std::ptr::eq(k.as_ref(), ptr)) { + keys.remove(pos); + } + } } /// Live HSM partition + session. diff --git a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs index c921588f7..7b88dd90b 100644 --- a/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs +++ b/plugins/ossl_engine/azihsm_ossl_engine/src/keyload.rs @@ -175,12 +175,15 @@ fn attach_ec( // pointer it returns for later sign/derive retrieval. The ex_data slot has // no free callback (see ec_key_ex_index), so libcrypto never calls back into // this `.so` for cleanup — even if the EVP_PKEY outlives the engine. - let ptr = data.retain_loaded_key(key).cast_mut().cast::(); - // SAFETY: ec is valid, idx is a registered EC_KEY ex_data slot, and ptr is a - // non-owning pointer into a key EngineData keeps alive for the engine's + let key_ptr = data.retain_loaded_key(key); + // SAFETY: ec is valid, idx is a registered EC_KEY ex_data slot, and key_ptr + // is a non-owning pointer into a key EngineData keeps alive for the engine's // lifetime; the slot has no free callback, so no ownership is transferred. - let rc = unsafe { ffi::EC_KEY_set_ex_data(ec, idx, ptr) }; + let rc = unsafe { ffi::EC_KEY_set_ex_data(ec, idx, key_ptr.cast_mut().cast::()) }; if rc != 1 { + // Roll back the retain so the failed load doesn't leave the key in the + // HSM until engine teardown. + data.release_loaded_key(key_ptr); return Err(EngineError::Other("EC_KEY_set_ex_data failed".into())); } Ok(())