fix: wrap_and_cache must not attempt to wrap an HSM-resident key

Author: Manuthor

CHANGELOG/docs_key-autorotation-spec.md

@@ -86,7 +86,8 @@
 - Add 3 negative ReCertify test vectors (missing UID, non-existent, not a certificate) ([#968](https://github.com/Cosmian/kms/pull/968))
 - Add 2 offset state verification vectors (rekey + rekey-keypair: Offset=0 → Active, Offset=86400 → PreActive) ([#968](https://github.com/Cosmian/kms/pull/968))
 - Add 2 negative state restriction vectors: `rekey_preactive_fails`, `rekey_keypair_preactive_fails` ([#968](https://github.com/Cosmian/kms/pull/968))
-- Fix test vectors `rekey_keypair_preactive_fails` and `rsa_sign_verify_roundtrip`: `CryptographicUsageMask` must be in `PrivateKeyAttributes`/`PublicKeyAttributes`, not `CommonAttributes` — FIPS mode reads the mask only from per-key sections ([#968](https://github.com/Cosmian/kms/pull/968))
+- Fix `wrap_and_cache`: skip server-wide KEK wrapping for HSM-resident keys (UID has `hsm::` prefix) — they are hardware-protected and the wrapping step caused a self-wrap error when the key being created IS the configured KEK ([#968](https://github.com/Cosmian/kms/pull/968))
+- Add vector `test_data/vectors/hsm/kek_bootstrap_self_create` + `server_type = "hsm_kek_uncreated"` server type to reproduce and prevent regressions of the HSM self-wrap bug ([#968](https://github.com/Cosmian/kms/pull/968))
 - Add 7 keyset resolution test vectors: `keyset_encrypt_latest`, `keyset_encrypt_bare_name`, `keyset_encrypt_latest_after_rotation`, `keyset_decrypt_try_each`, `keyset_decrypt_double_rotation`, `keyset_decrypt_at_latest`, `keyset_rotate_name_at_rejected` ([#968](https://github.com/Cosmian/kms/pull/968))
 - Add HSM keyset support: `find_due_for_rotation` on HsmStore reads `CKA_START_DATE`/`CKA_END_DATE`; keyset metadata stored in `CKA_LABEL` (`name::gen::key_id[::latest]`); `SetAttribute rotate_name` writes `CKA_LABEL` via `C_SetAttributeValue`; `SetAttribute rotate_interval` writes `CKA_START_DATE`/`CKA_END_DATE`; `Re-Key` on HSM UIDs generates a new HSM key and updates both CKA_LABELs; `walk_keyset_chain` resolves HSM keysets by `CKA_LABEL` without `ReplacedObjectLink` ([#968](https://github.com/Cosmian/kms/pull/968))
 - Add 3 HSM keyset test vectors: `resident_keyset_set_rotate_name`, `resident_keyset_rekey_and_decrypt`, `resident_keyset_double_rotation` + 1 negative: `hsm_rotate_offset_rejected` ([#968](https://github.com/Cosmian/kms/pull/968))

crate/server/src/core/wrapping/wrap.rs

@@ -59,18 +59,28 @@ pub(crate) async fn wrap_and_cache(
     // or in the HSM.
     // Either the user has provided a wrapping key ID or a key wrapping key is
     // provided in the parameters.
-    let Some(wrapping_key_id) = object
+    let uid_str = unique_identifier.to_string();
+    let explicit_wrapping_key_id = object
         .attributes_mut()
         .ok()
-        .and_then(|attrs| attrs.remove_wrapping_key_id(kms.vendor_id()))
-        .or_else(|| kms.params.key_wrapping_key.clone())
-    else {
-        // no wrapping key provided
-        return Ok(());
+        .and_then(|attrs| attrs.remove_wrapping_key_id(kms.vendor_id()));
+    let wrapping_key_id = if let Some(id) = explicit_wrapping_key_id {
+        id
+    } else {
+        let Some(kek) = kms.params.key_wrapping_key.clone() else {
+            // no wrapping key provided
+            return Ok(());
+        };
+        // HSM-resident keys are hardware-protected: skip the server-wide KEK wrapping
+        // to avoid creating a circular dependency where the KEK would wrap itself.
+        if has_prefix(&uid_str).is_some() {
+            return Ok(());
+        }
+        kek
     };
 
     // A key cannot be its own wrapping key.
-    if wrapping_key_id == unique_identifier.to_string() {
+    if wrapping_key_id == uid_str {
         return Err(KmsError::InvalidRequest(format!(
             "Key '{wrapping_key_id}' cannot be used as its own wrapping key: \
              the wrapping key ID must differ from the key ID being created"

crate/test_kms_server/README.md

@@ -228,6 +228,7 @@ replays the steps sequentially.
 | HSM / KEK Create | `hsm/kek_ec_p256_create_sign` | CreateKeyPair (EC P-256, KEK-wrapped), Sign, Destroy ×2 | 3 |
 | HSM / KEK Create | `hsm/kek_ed25519_create_sign` | CreateKeyPair (Ed25519, KEK-wrapped), Sign, Destroy ×2 | 3 |
 | HSM / KEK ReKey | `hsm/kek_rekey_wrapped` | Create (AES-256, KEK-wrapped), Encrypt, ReKey (unwrap from KEK, new material, re-wrap), Encrypt new, GetAttributes | 9 |
+| HSM / KEK Bootstrap | `hsm/kek_bootstrap_self_create` | **Regression**: Create KEK itself (self-wrap guard bypass), Create DEK, Encrypt, Decrypt, Revoke, Destroy DEK | 6 |
 | HSM / Resident Keyset | `hsm/resident_keyset_set_rotate_name` | Create (AES-256, HSM), SetAttribute rotate_name (writes CKA_LABEL), Encrypt by keyset name, Decrypt by UID | 4 |
 | HSM / Resident Keyset | `hsm/resident_keyset_rekey_and_decrypt` | Create, SetAttribute rotate_name, Encrypt (gen-0), ReKey, Decrypt by keyset name (chain: gen-1→gen-0) | 7 |
 | HSM / Resident Keyset | `hsm/resident_keyset_double_rotation` | Create, SetAttribute rotate_name, Encrypt (gen-0), ReKey ×2, Decrypt by keyset name (chain: gen-2→gen-1→gen-0) | 9 |

crate/test_kms_server/src/lib.rs

@@ -13,6 +13,7 @@ pub use test_server::{
     start_default_test_kms_server_with_privileged_users,
     start_default_test_kms_server_with_softhsm2_and_kek,
     start_default_test_kms_server_with_softhsm2_and_kek_for_vectors,
+    start_default_test_kms_server_with_softhsm2_kek_uncreated_for_vectors,
     start_default_test_kms_server_with_three_softhsm2,
     start_default_test_kms_server_with_utimaco_and_kek,
     start_default_test_kms_server_with_utimaco_hsm, start_test_kms_server_with_config,
@@ -24,6 +25,8 @@ mod test_server;
 
 mod test_jwt;
 
+pub mod test_env;
+
 pub mod vector_runner;
 
 use std::sync::Once;

crate/test_kms_server/src/test_env.rs

@@ -0,0 +1,50 @@
+//! Safe in-process environment-variable overrides for test vectors.
+//!
+//! `std::env::set_var` is `unsafe` in Rust 1.87+ and is forbidden by this
+//! crate's `deny(unsafe_code)` policy.  This module provides a thread-safe
+//! alternative that the vector runner consults **before** falling back to the
+//! real process environment.
+//!
+//! Usage (server init):
+//! ```rust,ignore
+//! crate::test_env::set("HSM_BOOTSTRAP_KEK_ID", &kek_id);
+//! ```
+//!
+//! Usage (vector runner placeholder resolution):
+//! ```rust,ignore
+//! let value = crate::test_env::get("HSM_BOOTSTRAP_KEK_ID")
+//!     .or_else(|| std::env::var("HSM_BOOTSTRAP_KEK_ID").ok());
+//! ```
+
+use std::{
+    collections::HashMap,
+    sync::{OnceLock, PoisonError, RwLock},
+};
+
+static OVERRIDES: OnceLock<RwLock<HashMap<String, String>>> = OnceLock::new();
+
+fn map() -> &'static RwLock<HashMap<String, String>> {
+    OVERRIDES.get_or_init(|| RwLock::new(HashMap::new()))
+}
+
+/// Store an in-process env override under `key`.
+///
+/// # Panics
+/// Panics if the internal `RwLock` has been poisoned (should never happen in
+/// a normal test run).
+pub fn set(key: &str, value: &str) {
+    map()
+        .write()
+        .unwrap_or_else(PoisonError::into_inner)
+        .insert(key.to_owned(), value.to_owned());
+}
+
+/// Look up an in-process env override.  Returns `None` if not set.
+#[must_use]
+pub fn get(key: &str) -> Option<String> {
+    map()
+        .read()
+        .unwrap_or_else(PoisonError::into_inner)
+        .get(key)
+        .cloned()
+}

crate/test_kms_server/src/test_server.rs

@@ -589,6 +589,52 @@ pub async fn start_default_test_kms_server_with_softhsm2_and_kek_for_vectors()
     start_server_from_config(config, &config_path).await
 }
 
+/// Start a `SoftHSM2` + KEK test server where the KEK has **not** been pre-created.
+///
+/// This server type is used to reproduce the self-wrap regression (PR #968):
+/// `wrap_and_cache` must not attempt to wrap an HSM-resident key with the
+/// server-wide KEK, even when the key being created IS the KEK itself.
+///
+/// Concretely, `key_encryption_key` is set to `"hsm::{slot}::kek_bootstrap"`
+/// before the server starts.  The first vector step creates that exact HSM key,
+/// which would have triggered the self-wrap error prior to the fix.
+///
+/// # Errors
+/// Returns an error if the server fails to start.
+///
+/// # Panics
+/// Panics if `HSM_SLOT_ID` is not set or is not a valid `usize`.
+pub async fn start_default_test_kms_server_with_softhsm2_kek_uncreated_for_vectors()
+-> Result<TestsContext, KmsClientError> {
+    let slot = get_softhsm2_slot_id();
+    let workspace_dir = std::env::temp_dir().join(format!(
+        "kms_test_softhsm2_kek_bootstrap_{}_{}_{}",
+        std::process::id(),
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap_or_default()
+            .as_nanos(),
+        TEST_DIR_COUNTER.fetch_add(1, Ordering::Relaxed)
+    ));
+    let kek_id = format!("hsm::{slot}::kek_bootstrap_{}", std::process::id());
+    // Export for {{$HSM_BOOTSTRAP_KEK_ID}} substitution in vector steps.
+    // Called once inside OnceCell initialisation before any vector steps run.
+    crate::test_env::set("HSM_BOOTSTRAP_KEK_ID", &kek_id);
+
+    let config_path = hsm_config_path("hsm_softhsm2_kek.toml");
+    let mut config = load_test_config_from_toml(&config_path)?;
+    config.hsm.hsm_slot = vec![slot];
+    config.db.sqlite_path = workspace_dir.join("sqlite-data");
+    config.workspace.root_data_path = workspace_dir.join("workspace");
+    config.workspace.tmp_path = workspace_dir.join("tmp");
+    config.key_encryption_key = Some(kek_id);
+    config.default_unwrap_type = Some(vec!["SecretData".to_owned(), "SymmetricKey".to_owned()]);
+    // Disable Google CSE: starting with an empty workspace means no Google CSE
+    // RSA keypair exists yet, and this test does not need that feature.
+    config.google_cse_config.google_cse_enable = false;
+    start_server_from_config(config, &config_path).await
+}
+
 /// Start a test KMS server with three `SoftHSM2` instances:
 ///
 /// - Slot 1 (`HSM_SLOT_ID_1`): legacy single-HSM config (`hsm:` top-level fields).

crate/test_kms_server/src/vector_runner.rs

@@ -30,6 +30,10 @@ static ONCE_VECTOR_CERT_AUTH: OnceCell<TestsContext> = OnceCell::const_new();
 static ONCE_VECTOR_AUTH_HTTPS: OnceCell<TestsContext> = OnceCell::const_new();
 /// Singleton server for vector tests requiring `SoftHSM2` + KEK.
 static ONCE_VECTOR_HSM_KEK: OnceCell<TestsContext> = OnceCell::const_new();
+/// Singleton server for vector tests where the HSM KEK is configured but **not yet created**.
+/// Used to verify that `wrap_and_cache` does not attempt to self-wrap when the first
+/// operation creates the KEK itself (regression for PR #968 self-wrap bug).
+static ONCE_VECTOR_HSM_KEK_UNCREATED: OnceCell<TestsContext> = OnceCell::const_new();
 
 /// A test vector manifest loaded from a TOML file.
 ///
@@ -438,12 +442,14 @@ fn load_request_json(
             ))
         })?;
         let var_name = &rest[..end];
-        let var_value = std::env::var(var_name).map_err(|_e| {
-            KmsClientError::UnexpectedError(format!(
-                "Environment variable '{var_name}' referenced in {} is not set",
-                path.display()
-            ))
-        })?;
+        let var_value = crate::test_env::get(var_name)
+            .or_else(|| std::env::var(var_name).ok())
+            .ok_or_else(|| {
+                KmsClientError::UnexpectedError(format!(
+                    "Environment variable '{var_name}' referenced in {} is not set",
+                    path.display()
+                ))
+            })?;
         content = format!(
             "{}{var_value}{}",
             &content[..start],
@@ -479,13 +485,15 @@ fn resolve_assertion_value(
         let rest = &result[start + 3..];
         if let Some(end) = rest.find("}}") {
             let var_name = &rest[..end];
-            let var_value = std::env::var(var_name).map_err(|_err| {
-                KmsClientError::UnexpectedError(format!(
-                    "resolve_assertion_value: environment variable '{var_name}' \
-                     referenced in assertion template '{template}' is not set — \
-                     refusing to silently use an empty string"
-                ))
-            })?;
+            let var_value = crate::test_env::get(var_name)
+                .or_else(|| std::env::var(var_name).ok())
+                .ok_or_else(|| {
+                    KmsClientError::UnexpectedError(format!(
+                        "resolve_assertion_value: environment variable '{var_name}' \
+                         referenced in assertion template '{template}' is not set — \
+                         refusing to silently use an empty string"
+                    ))
+                })?;
             result = format!("{}{}{}", &result[..start], var_value, &rest[end + 2..]);
         } else {
             break;
@@ -789,7 +797,7 @@ pub async fn run_test_vector(vector_dir: &str) -> Result<(), KmsClientError> {
 
     // Check required environment variables; skip gracefully if any is missing
     for env_var in &manifest.requires_env {
-        if std::env::var(env_var).is_err() {
+        if crate::test_env::get(env_var).is_none() && std::env::var(env_var).is_err() {
             eprintln!(
                 "SKIP vector '{}': required env var '{env_var}' is not set",
                 manifest.name
@@ -814,6 +822,19 @@ pub async fn run_test_vector(vector_dir: &str) -> Result<(), KmsClientError> {
                 );
                 return execute_steps(context, &manifest, &vector_path).await;
             }
+            "hsm_kek_uncreated" => {
+                let context = ONCE_VECTOR_HSM_KEK_UNCREATED
+                    .get_or_try_init(|| async {
+                        crate::start_default_test_kms_server_with_softhsm2_kek_uncreated_for_vectors()
+                            .await
+                    })
+                    .await?;
+                eprintln!(
+                    "▶ Running vector '{}' on server_type 'hsm_kek_uncreated'",
+                    manifest.name
+                );
+                return execute_steps(context, &manifest, &vector_path).await;
+            }
             other => {
                 return Err(KmsClientError::UnexpectedError(format!(
                     "Unknown server_type '{other}' in manifest for vector '{}'",
@@ -3786,6 +3807,15 @@ ObjectType = "SymmetricKey"
         run_test_vector("test_data/vectors/hsm/kek_rekey_wrapped").await
     }
 
+    /// Regression test for the HSM self-wrap bug (PR #968):
+    /// `wrap_and_cache` must not attempt to wrap an HSM-resident key with the
+    /// server-wide KEK when the key being created IS the configured KEK UID.
+    #[tokio::test]
+    async fn test_vec_hsm_kek_bootstrap_self_create() -> Result<(), KmsClientError> {
+        crate::init_test_logging();
+        run_test_vector("test_data/vectors/hsm/kek_bootstrap_self_create").await
+    }
+
     // ── HSM Resident: Keyset (rotate_name / CKA_LABEL) ───────────────────
 
     #[tokio::test]