Cpiccini/engine addition by CPiccini · Pull Request #250 · Azure/azihsm-sdk

CPiccini · 2026-03-10T02:18:25Z

Please review and approve to pull engine into sdk

Adding engine to plugins folder

Copilot

Pull request overview

This PR adds a new OpenSSL ENGINE implementation (and supporting Rust FFI/safe wrappers) into the SDK to enable AZIHSM-backed crypto operations (RSA/EC/HKDF/AES) plus C-facing control APIs and tests.

Changes:

Introduces the openssl-rust crate with safe wrappers/callback glue for OpenSSL ENGINE, EVP_PKEY, EVP_MD, and EVP_CIPHER integrations.
Adds the azihsmengine engine library implementing RSA/EC/HKDF/AES operations and ENGINE control commands (import, attest, unwrap key, delete key, info, load key).
Adds shared support crates (api-interface, engine-common) and a set of Rust tests for key paths (e.g., RSA encrypt/decrypt, EC, XTS).

Reviewed changes

Copilot reviewed 95 out of 188 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
plugins/ossl_engine/openssl-rust/src/safeapi/evp_pkey/ctx.rs	Adds EVP_PKEY_CTX wrapper with data attach/detach helpers and key extraction.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_pkey/callback/mod.rs	Adds module layout for EVP_PKEY callbacks.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_pkey/callback/hkdf.rs	Implements HKDF derive/ctrl/cleanup C callbacks and callback registration.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_pkey/callback/common.rs	Defines common callback enums/types for EVP_PKEY method wiring.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_md/mod.rs	Adds EVP_MD module surface.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_md/md.rs	Adds digest type wrapper and digest-size helpers.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_md/ctx.rs	Adds EVP_MD_CTX wrapper for digest init/update/final and metadata queries.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_cipher/mod.rs	Adds EVP_CIPHER module surface.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_cipher/method.rs	Adds EVP_CIPHER method wrapper and builder wiring callback trampolines.
plugins/ossl_engine/openssl-rust/src/safeapi/evp_cipher/ctx.rs	Adds EVP_CIPHER_CTX wrapper with cipher-data handle storage and init.
plugins/ossl_engine/openssl-rust/src/safeapi/engine_ctrl.rs	Adds generic engine ctrl command definition/container support.
plugins/ossl_engine/openssl-rust/src/safeapi/ec/mod.rs	Adds EC safeapi module exports.
plugins/ossl_engine/openssl-rust/src/safeapi/ec/method.rs	Adds EC_KEY_METHOD singleton + wiring for EC callbacks.
plugins/ossl_engine/openssl-rust/src/safeapi/ec/ecdsa_sig.rs	Adds ECDSA_SIG wrapper and raw signature conversion helpers.
plugins/ossl_engine/openssl-rust/src/safeapi/callback.rs	Adds ENGINE callback trampolines (ciphers/pkey/ctrl/load-key) and helper macros.
plugins/ossl_engine/openssl-rust/src/lib.rs	Adds crate root + generated bindings inclusion.
plugins/ossl_engine/openssl-rust/build.rs	Adds bindgen generation and OpenSSL version detection/config.
plugins/ossl_engine/openssl-rust/Cargo.toml	Adds `openssl-rust` crate manifest.
plugins/ossl_engine/lib/tests/rsa_encrypt_decrypt.rs	Adds integration-style tests for RSA encrypt/decrypt behavior.
plugins/ossl_engine/lib/src/rsa/mod.rs	Adds RSA module surface.
plugins/ossl_engine/lib/src/rsa/init.rs	Wires RSA method callbacks into the engine.
plugins/ossl_engine/lib/src/rsa/callback.rs	Implements RSA import/attest/sign/verify/encrypt/decrypt callbacks.
plugins/ossl_engine/lib/src/pkey/rsa/mod.rs	Adds EVP_PKEY RSA module surface.
plugins/ossl_engine/lib/src/pkey/open.rs	Implements key-open-by-name routing across EC/RSA.
plugins/ossl_engine/lib/src/pkey/mod.rs	Adds EVP_PKEY module surface.
plugins/ossl_engine/lib/src/pkey/init.rs	Registers EVP_PKEY methods (RSA/EC/HKDF) and exposes engine callback.
plugins/ossl_engine/lib/src/pkey/hkdf/mod.rs	Adds HKDF module surface.
plugins/ossl_engine/lib/src/pkey/hkdf/key.rs	Implements HKDF/KBKDF key-derive state machine and HSM operations.
plugins/ossl_engine/lib/src/pkey/ec/mod.rs	Adds EVP_PKEY EC module surface.
plugins/ossl_engine/lib/src/load_key.rs	Wires ENGINE load-private-key callback to open named keys.
plugins/ossl_engine/lib/src/lib.rs	Adds engine entrypoints, binding logic, logging setup, and tests.
plugins/ossl_engine/lib/src/engine_internal.rs	Adds global engine context (session, unwrap key) and HSM error mapping.
plugins/ossl_engine/lib/src/engine_data.rs	Stores tracing subscriber guard in engine ex-data.
plugins/ossl_engine/lib/src/engine_ctrl/unwrap_key/mod.rs	Adds unwrap-key ctrl module surface.
plugins/ossl_engine/lib/src/engine_ctrl/unwrap_key/get.rs	Implements ctrl commands to retrieve unwrap keys (+ tests).
plugins/ossl_engine/lib/src/engine_ctrl/mod.rs	Adds engine ctrl module surface.
plugins/ossl_engine/lib/src/engine_ctrl/init.rs	Registers ctrl commands into engine.
plugins/ossl_engine/lib/src/engine_ctrl/info.rs	Implements ctrl command to return engine info (+ tests).
plugins/ossl_engine/lib/src/engine_ctrl/import_key/rsa.rs	Implements ctrl command to import RSA into RSA* structure.
plugins/ossl_engine/lib/src/engine_ctrl/import_key/mod.rs	Adds key-import ctrl module surface.
plugins/ossl_engine/lib/src/engine_ctrl/import_key/evp_pkey.rs	Implements ctrl commands to import keys into EVP_PKEY_CTX.
plugins/ossl_engine/lib/src/engine_ctrl/import_key/evp_cipher.rs	Implements ctrl command to import keys into EVP_CIPHER_CTX.
plugins/ossl_engine/lib/src/engine_ctrl/import_key/ec_key.rs	Implements ctrl command to import keys into EC_KEY.
plugins/ossl_engine/lib/src/engine_ctrl/delete_key.rs	Implements ctrl command to delete keys by name.
plugins/ossl_engine/lib/src/engine_ctrl/attest_key/unwrap_key.rs	Implements ctrl command to attest builtin unwrap key.
plugins/ossl_engine/lib/src/engine_ctrl/attest_key/rsa_key.rs	Implements ctrl command to attest RSA key (RSA format).
plugins/ossl_engine/lib/src/engine_ctrl/attest_key/mod.rs	Adds key-attest ctrl module surface.
plugins/ossl_engine/lib/src/engine_ctrl/attest_key/evp_pkey.rs	Implements ctrl commands to attest keys in EVP_PKEY format.
plugins/ossl_engine/lib/src/engine_ctrl/attest_key/ec_key.rs	Implements ctrl command to attest EC key (EC_KEY format).
plugins/ossl_engine/lib/src/ec/mod.rs	Adds EC module surface + tests.
plugins/ossl_engine/lib/src/ec/init.rs	Wires EC callbacks into EC_KEY_METHOD and engine.
plugins/ossl_engine/lib/src/ec/callback.rs	Implements EC import/keygen/sign/verify/ECDH callbacks.
plugins/ossl_engine/lib/src/common/secret_key.rs	Adds secret key handle wrapper and curve→secret mapping.
plugins/ossl_engine/lib/src/common/mod.rs	Adds common module surface.
plugins/ossl_engine/lib/src/common/key_util.rs	Adds attestation payload encoder and a helper macro.
plugins/ossl_engine/lib/src/common/hsm_key.rs	Adds HSM key container with unwrap/open/generate/attest/export behaviors.
plugins/ossl_engine/lib/src/common/hash.rs	Adds hash-NID mapping utilities.
plugins/ossl_engine/lib/src/common/ec_key.rs	Adds EC key data model and HSM-backed sign/verify/ecdh/attest ops.
plugins/ossl_engine/lib/src/common/base_key.rs	Adds handle table-backed key registry for engine-managed keys.
plugins/ossl_engine/lib/src/ciphers/mod.rs	Adds cipher module surface.
plugins/ossl_engine/lib/src/ciphers/key.rs	Adds AES key dispatch (CBC/GCM/XTS) and ctrl/cipher plumbing.
plugins/ossl_engine/lib/src/ciphers/aes_xts.rs	Adds AES-XTS HSM-backed implementation + tests.
plugins/ossl_engine/lib/Cargo.toml	Adds engine crate manifest and dependencies/features.
plugins/ossl_engine/engine-common/src/lib.rs	Adds shared type sizes/constants for key handles.
plugins/ossl_engine/engine-common/src/handle_table.rs	Adds a handle table abstraction for storing engine-owned objects.
plugins/ossl_engine/engine-common/Cargo.toml	Adds engine-common crate manifest.
plugins/ossl_engine/api-interface/src/lib.rs	Adds C-facing interface crate root + generated bindings inclusion.
plugins/ossl_engine/api-interface/src/engine_ctrl/wrapping_key.rs	Adds C-facing API wrapper for unwrap key output buffer.
plugins/ossl_engine/api-interface/src/engine_ctrl/open_key.rs	Adds helpers to parse key name and ECDH hint from C pointers.
plugins/ossl_engine/api-interface/src/engine_ctrl/mod.rs	Adds engine_ctrl module surface for API interface.
plugins/ossl_engine/api-interface/src/engine_ctrl/key_import.rs	Adds C-facing key import struct wrapper helpers.
plugins/ossl_engine/api-interface/src/engine_ctrl/info.rs	Adds C-facing engine info wrapper helper.
plugins/ossl_engine/api-interface/src/engine_ctrl/collateral.rs	Adds C-facing collateral output buffer helper.
plugins/ossl_engine/api-interface/src/engine_ctrl/attest_key.rs	Adds C-facing attestation output buffer helper.
plugins/ossl_engine/api-interface/build.rs	Adds bindgen generation for the AZIHSM engine C header.
plugins/ossl_engine/api-interface/Cargo.toml	Adds api-interface crate manifest.
plugins/ossl_engine/Cargo.toml	Adds a dedicated workspace for the engine and shared deps/lints.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-10T02:20:36Z

plugins/ossl_engine/openssl-rust/src/safeapi/ec/method.rs

+impl Drop for EcKeyMethod {
+    fn drop(&mut self) {
+        if self.0.is_null() {
+            unsafe {
+                EC_KEY_METHOD_free(self.0);
+            }
+        }


The Drop implementation frees the EC_KEY_METHOD only when self.0 is null, which is inverted logic. This will leak the allocated EC_KEY_METHOD in normal cases; change the condition to free only when the pointer is non-null, and then null it out.

Copilot · 2026-03-10T02:20:37Z

plugins/ossl_engine/openssl-rust/src/safeapi/engine_ctrl.rs

+            let _ = unsafe { CString::from_raw(item.cmd_name as *mut c_char) };
+            let _ = unsafe { CString::from_raw(item.cmd_desc as *mut c_char) };


This drop converts every cmd_name/cmd_desc back using CString::from_raw, but build_cmd_defns() appends a terminal sentinel where cmd_name/cmd_desc are null. Calling CString::from_raw(null) is undefined behavior. Skip entries with null pointers (or track which entries were created via into_raw) before calling from_raw.

Suggested change

let _ = unsafe { CString::from_raw(item.cmd_name as *mut c_char) };

let _ = unsafe { CString::from_raw(item.cmd_desc as *mut c_char) };

// Skip entries (such as the terminal sentinel) that have null pointers

if !item.cmd_name.is_null() {

let _ = unsafe { CString::from_raw(item.cmd_name as *mut c_char) };

}

if !item.cmd_desc.is_null() {

let _ = unsafe { CString::from_raw(item.cmd_desc as *mut c_char) };

}

Copilot · 2026-03-10T02:20:37Z

plugins/ossl_engine/api-interface/src/engine_ctrl/key_import.rs

+    pub fn data_as_mut_ref(&self) -> &T {
+        unsafe { &*self.mut_data_ptr() }


data_as_mut_ref returns &T but its name implies a mutable reference, and it dereferences a *mut T as immutable. If callers expect to mutate the underlying object, this API is incorrect. Change the signature to return &mut T and dereference as &mut *self.mut_data_ptr().

Suggested change

pub fn data_as_mut_ref(&self) -> &T {

unsafe { &*self.mut_data_ptr() }

pub fn data_as_mut_ref(&self) -> &mut T {

unsafe { &mut *self.mut_data_ptr() }

Copilot · 2026-03-10T02:20:37Z

plugins/ossl_engine/openssl-rust/build.rs

+fn openssl_lib_flag() -> Option<Vec<String>> {
+    let result = Command::new("pkg-config")
+        .arg("libcrypto")
+        .arg("--libs-only-l")


The build script asks pkg-config for --libs-only-l (library names like -lcrypto) but then treats the output as a link-search directory and emits cargo:rustc-link-search=.... This is incorrect and can break linking (e.g., it would emit crypto as a search path). Use --libs-only-L for search paths, and (optionally) parse --libs-only-l to emit cargo:rustc-link-lib= entries, or switch to a standard Cargo approach for OpenSSL discovery.

Suggested change

.arg("--libs-only-l")

.arg("--libs-only-L")

Copilot · 2026-03-10T02:20:37Z

plugins/ossl_engine/openssl-rust/build.rs

+    if let Some(lib_flags) = lib_flags {
+        for out in &lib_flags {
+            let out = out.chars().skip(2).collect::<String>();
+            if !out.is_empty() {
+                println!("cargo:rustc-link-search={}", out);
+            }
+        }
+    }


The build script asks pkg-config for --libs-only-l (library names like -lcrypto) but then treats the output as a link-search directory and emits cargo:rustc-link-search=.... This is incorrect and can break linking (e.g., it would emit crypto as a search path). Use --libs-only-L for search paths, and (optionally) parse --libs-only-l to emit cargo:rustc-link-lib= entries, or switch to a standard Cargo approach for OpenSSL discovery.

plugins/ossl_engine/api-interface/src/engine_ctrl/info.rs

Copilot · 2026-03-10T02:20:38Z

plugins/ossl_engine/Cargo.toml

+# Code that needs unsafe should opt-in via a module-scoped allow.
+unsafe_code = "deny"


This workspace sets unsafe_code = \"deny\", but the newly added engine/FFI wrappers use many unsafe blocks throughout the workspace members (required for OpenSSL FFI). As-is, this will fail builds with lint errors unless each crate/module explicitly opts in. Either add targeted #![allow(unsafe_code)] at the crate/module boundaries that must use FFI (and keep unsafe localized), or relax this workspace lint for the affected crates.

Suggested change

# Code that needs unsafe should opt-in via a module-scoped allow.

unsafe_code = "deny"

# Unsafe is generally discouraged and will emit warnings at the workspace level.

unsafe_code = "warn"

Copilot · 2026-03-10T02:20:38Z

plugins/ossl_engine/lib/src/engine_ctrl/unwrap_key/get.rs

+    fn test_get_unwrap_key() {
+        let engine = create_engine();
+        let cmd_get_builtin_unwrap_key = CmdGetBuiltinUnwrapKey;
+        let cmd_get_unwrap_key = CmdGetBuiltinUnwrapKey;


cmd_get_unwrap_key is instantiated as CmdGetBuiltinUnwrapKey instead of CmdGetUnwrapKey, so this test does not actually exercise the 'current unwrap key' command path. Update the test to use CmdGetUnwrapKey for cmd_get_unwrap_key.

Suggested change

let cmd_get_unwrap_key = CmdGetBuiltinUnwrapKey;

let cmd_get_unwrap_key = CmdGetUnwrapKey;

Copilot · 2026-03-10T02:20:39Z

plugins/ossl_engine/openssl-rust/src/safeapi/ec/ecdsa_sig.rs

+use crate::BIGNUM;
+use crate::ECDSA_SIG;
+
+pub struct Ecdsa_Sig {


Rust type names should be UpperCamelCase. Ecdsa_Sig is non-idiomatic and makes public API usage inconsistent with the rest of the codebase. Rename it to EcdsaSig (and update its impl block and call sites) to follow Rust naming conventions.

Copilot · 2026-03-10T02:20:39Z

plugins/ossl_engine/lib/src/common/key_util.rs

+        } else {
+            let keydata = <$keydata_type>::new();
+            $ctx.set_data(keydata);
+            $ctx.get_data().ok_or(OpenSSLError::InvalidKeyData)


The macro references OpenSSLError::InvalidKeyData without a path, which is resolved at the macro call site and can cause compilation failures unless callers import OpenSSLError into scope. Prefer using a fully-qualified path (or $crate::... where applicable) inside the macro so it is self-contained.

Suggested change

$ctx.get_data().ok_or(OpenSSLError::InvalidKeyData)

$ctx.get_data().ok_or($crate::OpenSSLError::InvalidKeyData)

plugins/ossl_engine/lib/src/engine_ctrl/import_key/evp_pkey.rs

+
+        let import_data = KeyImport::<EVP_PKEY_CTX>::new(import_data as *mut AziHsmKeyImport)?;
+        match nid as c_uint {
+            NID_RSA_ENCRYPTION | NID_RSA_SIGNATURE => {


plugins/ossl_engine/lib/src/engine_ctrl/import_key/evp_pkey.rs

+        }
+
+        match curve_name as c_uint {
+            NID_EC_P256 | NID_EC_P384 | NID_EC_P521 => {


In general, the way to fix this problem is to ensure that any constants used in a match pattern are actually defined/imported, and then only remove them if they are truly not needed. Here, the intent is obviously to handle three specific EC curves (P-256, P-384, P-521) in the same branch, so the correct fix is to import the missing NID constants rather than remove them.

Concretely, in plugins/ossl_engine/lib/src/engine_ctrl/import_key/evp_pkey.rs, we already import NID_X9_62_prime256v1 as NID_EC_P256. We should add analogous imports for the P-384 and P-521 curves (e.g., NID_secp384r1 → NID_EC_P384, NID_secp521r1 → NID_EC_P521) from openssl_rust, assuming that crate re-exports the OpenSSL NID constants with those names (these are standard OpenSSL identifiers). This is the smallest change that aligns with the existing naming and clearly matches the intent of the match curve_name as c_uint block, without altering runtime behavior beyond correctly enabling those additional curves.

The steps are:

Edit the import section at the top of evp_pkey.rs.

Add two new use openssl_rust::... lines for the missing NIDs, aliased as NID_EC_P384 and NID_EC_P521.

Leave the rest of the function and match arm unchanged.

No new methods or complex logic are needed; only additional imports of well-known constants from the openssl_rust crate.

plugins/ossl_engine/lib/src/pkey/rsa/callback.rs

+/// * `OpenSSLResult<()>` - The result of the init operation
+#[allow(unused)]
+pub fn rsa_sign_verify_ctx_init_cb(
+    ctx_ptr: *mut EVP_PKEY_CTX,


In general, unused variables or parameters should be removed, or, when required for an external API or trait signature, they should be explicitly marked as intentionally unused (e.g., by prefixing with _ in Rust). This preserves functionality while making the intent clear and silencing lints and static analysis warnings.

For this specific case, rsa_sign_verify_ctx_init_cb is a callback whose signature must match the external/OpenSSL expectations, so we should not remove the ctx_ptr parameter. Instead, we should rename the parameter to _ctx_ptr to indicate it is intentionally unused. This keeps the function’s signature shape (two pointer arguments) exactly the same from the ABI perspective while clearly expressing that the first argument is not used. No other code inside the function needs to change, and no additional imports or definitions are required.

Concretely:

In plugins/ossl_engine/lib/src/pkey/rsa/callback.rs, on the function definition starting at line 200, change the parameter name ctx_ptr to _ctx_ptr.

Leave md_ctx_ptr and the function body unchanged.

CPiccini added 7 commits March 9, 2026 19:03

Add placeholder readme for ossl_engine plugin

9d0d9c9

Adding engine to plugins folder

Adding Engine files to branch

85aad03

Add files via upload

d15853a

Add full tests directory

6ebd720

Add full lib directory

0afecd5

Add full openssl-rust directory

fab0ab5

Add full engine-common dir

abb59b9

Copilot AI review requested due to automatic review settings March 10, 2026 02:18

CPiccini assigned jaygmsft Mar 10, 2026

Merge branch 'main' into cpiccini/Engine

6b43dd6

Copilot AI reviewed Mar 10, 2026

View reviewed changes

github-advanced-security bot found potential problems Mar 10, 2026

View reviewed changes

@@ -13,6 +13,8 @@
             use openssl_rust::safeapi::error::OpenSSLError;
             use openssl_rust::safeapi::error::OpenSSLResult;
             use openssl_rust::NID_X9_62_prime256v1 as NID_EC_P256;
+            use openssl_rust::NID_secp384r1 as NID_EC_P384;
+            use openssl_rust::NID_secp521r1 as NID_EC_P521;
             use openssl_rust::NID_rsaEncryption as NID_RSA_ENCRYPTION;
             use openssl_rust::NID_rsaSignature as NID_RSA_SIGNATURE;
             use openssl_rust::NID_secp384r1 as NID_EC_P384;

@@ -198,7 +198,7 @@
             /// * `OpenSSLResult<()>` - The result of the init operation
             #[allow(unused)]
             pub fn rsa_sign_verify_ctx_init_cb(
-                ctx_ptr: *mut EVP_PKEY_CTX,
+                _ctx_ptr: *mut EVP_PKEY_CTX,
                 md_ctx_ptr: *mut EVP_MD_CTX,
             ) -> OpenSSLResult<()> {
                 let md_ctx = EvpMdCtx::new_from_ptr(md_ctx_ptr);

		let _ = unsafe { CString::from_raw(item.cmd_name as *mut c_char) };
		let _ = unsafe { CString::from_raw(item.cmd_desc as *mut c_char) };

		pub fn data_as_mut_ref(&self) -> &T {
		unsafe { &*self.mut_data_ptr() }

		# Code that needs unsafe should opt-in via a module-scoped allow.
		unsafe_code = "deny"

	let cmd_get_unwrap_key = CmdGetBuiltinUnwrapKey;
	let cmd_get_unwrap_key = CmdGetUnwrapKey;

	$ctx.get_data().ok_or(OpenSSLError::InvalidKeyData)
	$ctx.get_data().ok_or($crate::OpenSSLError::InvalidKeyData)

Conversation

CPiccini commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants