From bca1b25d26e6b184f5f68a3d68b8fe18fbc3b041 Mon Sep 17 00:00:00 2001 From: vishesh92 Date: Tue, 24 Feb 2026 18:02:22 +0530 Subject: [PATCH] directly create the key in the store --- .../user/kms/hsm/AddHSMProfileCmd.java | 2 +- .../user/kms/hsm/DeleteHSMProfileCmd.java | 2 +- .../user/kms/hsm/UpdateHSMProfileCmd.java | 2 +- .../cloudstack/kms/KMSWrappedKeyVO.java | 6 +- .../cloudstack/framework/kms/KMSProvider.java | 122 +++++++----- .../provider/pkcs11/PKCS11HSMProvider.java | 185 +++++++----------- .../apache/cloudstack/kms/KMSManagerImpl.java | 156 ++++++++++----- .../cloudstack/kms/KMSManagerImplHSMTest.java | 18 +- ui/src/config/section/kms.js | 9 + 9 files changed, 277 insertions(+), 225 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/AddHSMProfileCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/AddHSMProfileCmd.java index 9617a48b260..b7410e4cfea 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/AddHSMProfileCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/AddHSMProfileCmd.java @@ -47,7 +47,7 @@ import java.util.Map; @APICommand(name = "addHSMProfile", description = "Adds a new HSM profile", responseObject = HSMProfileResponse.class, requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, since = "4.23.0", - authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) + authorized = { RoleType.Admin }) public class AddHSMProfileCmd extends BaseCmd { @Inject diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/DeleteHSMProfileCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/DeleteHSMProfileCmd.java index 8fc0a47c3ad..5f70eb7a899 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/DeleteHSMProfileCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/DeleteHSMProfileCmd.java @@ -40,7 +40,7 @@ import javax.inject.Inject; @APICommand(name = "deleteHSMProfile", description = "Deletes an HSM profile", responseObject = SuccessResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.23.0", - authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) + authorized = { RoleType.Admin }) public class DeleteHSMProfileCmd extends BaseCmd { @Inject diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/UpdateHSMProfileCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/UpdateHSMProfileCmd.java index 36de6d447e1..a408c967362 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/UpdateHSMProfileCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/UpdateHSMProfileCmd.java @@ -40,7 +40,7 @@ import javax.inject.Inject; @APICommand(name = "updateHSMProfile", description = "Updates an HSM profile", responseObject = HSMProfileResponse.class, requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, since = "4.23.0", - authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) + authorized = { RoleType.Admin }) public class UpdateHSMProfileCmd extends BaseCmd { @Inject diff --git a/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSWrappedKeyVO.java b/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSWrappedKeyVO.java index d444588a32a..8a763623fba 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSWrappedKeyVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSWrappedKeyVO.java @@ -170,9 +170,7 @@ public class KMSWrappedKeyVO { @Override public String toString() { - return String.format("KMSWrappedKey %s", - ReflectionToStringBuilderUtils.reflectOnlySelectedFields( - this, "id", "uuid", "kmsKeyId", "kekVersionId", "accountId", "zoneId", "state", "created", - "removed")); + return String.format("KMSWrappedKey %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields( + this, "id", "uuid", "kmsKeyId", "kekVersionId", "zoneId", "created", "removed")); } } diff --git a/framework/kms/src/main/java/org/apache/cloudstack/framework/kms/KMSProvider.java b/framework/kms/src/main/java/org/apache/cloudstack/framework/kms/KMSProvider.java index cea8ccd82a9..2b793fe560a 100644 --- a/framework/kms/src/main/java/org/apache/cloudstack/framework/kms/KMSProvider.java +++ b/framework/kms/src/main/java/org/apache/cloudstack/framework/kms/KMSProvider.java @@ -36,6 +36,31 @@ import org.apache.cloudstack.framework.config.Configurable; */ public interface KMSProvider extends Configurable, Adapter { + /** + * Returns {@code true} if the given HSM profile configuration key name refers + * to a + * sensitive value (PIN, password, secret, or private key) that must be + * encrypted at + * rest and masked in API responses. + * + *

+ * This is a shared naming-convention helper used by both KMS providers (when + * loading/storing profile details) and the KMS manager (when building API + * responses). + * + * @param key configuration key name (case-insensitive); null returns false + * @return true if the key is considered sensitive + */ + static boolean isSensitiveKey(String key) { + if (key == null) { + return false; + } + return key.equalsIgnoreCase("pin") || + key.equalsIgnoreCase("password") || + key.toLowerCase().contains("secret") || + key.equalsIgnoreCase("private_key"); + } + /** * Get the unique name of this provider * @@ -43,18 +68,6 @@ public interface KMSProvider extends Configurable, Adapter { */ String getProviderName(); - /** - * Create a new Key Encryption Key (KEK) in the secure backend with explicit HSM profile. - * - * @param purpose the purpose/scope for this KEK - * @param label human-readable label for the KEK (must be unique within purpose) - * @param keyBits key size in bits (typically 128, 192, or 256) - * @param hsmProfileId optional HSM profile ID to create the KEK in (null for auto-resolution/default) - * @return the KEK identifier (label or handle) for later reference - * @throws KMSException if KEK creation fails - */ - String createKek(KeyPurpose purpose, String label, int keyBits, Long hsmProfileId) throws KMSException; - /** * Create a new Key Encryption Key (KEK) in the secure backend. * Delegates to {@link #createKek(KeyPurpose, String, int, Long)} with null profile ID. @@ -69,6 +82,18 @@ public interface KMSProvider extends Configurable, Adapter { return createKek(purpose, label, keyBits, null); } + /** + * Create a new Key Encryption Key (KEK) in the secure backend with explicit HSM profile. + * + * @param purpose the purpose/scope for this KEK + * @param label human-readable label for the KEK (must be unique within purpose) + * @param keyBits key size in bits (typically 128, 192, or 256) + * @param hsmProfileId optional HSM profile ID to create the KEK in (null for auto-resolution/default) + * @return the KEK identifier (label or handle) for later reference + * @throws KMSException if KEK creation fails + */ + String createKek(KeyPurpose purpose, String label, int keyBits, Long hsmProfileId) throws KMSException; + /** * Delete a KEK from the secure backend. * WARNING: This will make all DEKs wrapped by this KEK unrecoverable. @@ -78,7 +103,6 @@ public interface KMSProvider extends Configurable, Adapter { */ void deleteKek(String kekId) throws KMSException; - /** * Check if a KEK exists and is accessible * @@ -88,18 +112,6 @@ public interface KMSProvider extends Configurable, Adapter { */ boolean isKekAvailable(String kekId) throws KMSException; - /** - * Wrap (encrypt) a plaintext Data Encryption Key with a KEK using explicit HSM profile. - * - * @param plainDek the plaintext DEK to wrap (caller must zeroize after call) - * @param purpose the intended purpose of this DEK - * @param kekLabel the label of the KEK to use for wrapping - * @param hsmProfileId optional HSM profile ID to use (null for auto-resolution/default) - * @return WrappedKey containing the encrypted DEK and metadata - * @throws KMSException if wrapping fails or KEK not found - */ - WrappedKey wrapKey(byte[] plainDek, KeyPurpose purpose, String kekLabel, Long hsmProfileId) throws KMSException; - /** * Wrap (encrypt) a plaintext Data Encryption Key with a KEK. * Delegates to {@link #wrapKey(byte[], KeyPurpose, String, Long)} with null profile ID. @@ -115,16 +127,16 @@ public interface KMSProvider extends Configurable, Adapter { } /** - * Unwrap (decrypt) a wrapped DEK to obtain the plaintext key using explicit HSM profile. - *

- * SECURITY: Caller MUST zeroize the returned byte array after use + * Wrap (encrypt) a plaintext Data Encryption Key with a KEK using explicit HSM profile. * - * @param wrappedKey the wrapped key to decrypt + * @param plainDek the plaintext DEK to wrap (caller must zeroize after call) + * @param purpose the intended purpose of this DEK + * @param kekLabel the label of the KEK to use for wrapping * @param hsmProfileId optional HSM profile ID to use (null for auto-resolution/default) - * @return plaintext DEK (caller must zeroize!) - * @throws KMSException if unwrapping fails or KEK not found + * @return WrappedKey containing the encrypted DEK and metadata + * @throws KMSException if wrapping fails or KEK not found */ - byte[] unwrapKey(WrappedKey wrappedKey, Long hsmProfileId) throws KMSException; + WrappedKey wrapKey(byte[] plainDek, KeyPurpose purpose, String kekLabel, Long hsmProfileId) throws KMSException; /** * Unwrap (decrypt) a wrapped DEK to obtain the plaintext key. @@ -141,18 +153,16 @@ public interface KMSProvider extends Configurable, Adapter { } /** - * Generate a new random DEK and immediately wrap it with a KEK using explicit HSM profile. - * (convenience method combining generation + wrapping) + * Unwrap (decrypt) a wrapped DEK to obtain the plaintext key using explicit HSM profile. + *

+ * SECURITY: Caller MUST zeroize the returned byte array after use * - * @param purpose the intended purpose of the new DEK - * @param kekLabel the label of the KEK to use for wrapping - * @param keyBits DEK size in bits (typically 128, 192, or 256) + * @param wrappedKey the wrapped key to decrypt * @param hsmProfileId optional HSM profile ID to use (null for auto-resolution/default) - * @return WrappedKey containing the newly generated and wrapped DEK - * @throws KMSException if generation or wrapping fails + * @return plaintext DEK (caller must zeroize!) + * @throws KMSException if unwrapping fails or KEK not found */ - WrappedKey generateAndWrapDek(KeyPurpose purpose, String kekLabel, int keyBits, - Long hsmProfileId) throws KMSException; + byte[] unwrapKey(WrappedKey wrappedKey, Long hsmProfileId) throws KMSException; /** * Generate a new random DEK and immediately wrap it with a KEK. @@ -170,16 +180,18 @@ public interface KMSProvider extends Configurable, Adapter { } /** - * Rewrap a DEK with a different KEK (used during key rotation) using explicit target HSM profile. - * This unwraps with the old KEK and wraps with the new KEK without exposing the plaintext DEK. + * Generate a new random DEK and immediately wrap it with a KEK using explicit HSM profile. + * (convenience method combining generation + wrapping) * - * @param oldWrappedKey the currently wrapped key - * @param newKekLabel the label of the new KEK to wrap with - * @param targetHsmProfileId optional target HSM profile ID to wrap with (null for auto-resolution/default) - * @return new WrappedKey encrypted with the new KEK - * @throws KMSException if rewrapping fails + * @param purpose the intended purpose of the new DEK + * @param kekLabel the label of the KEK to use for wrapping + * @param keyBits DEK size in bits (typically 128, 192, or 256) + * @param hsmProfileId optional HSM profile ID to use (null for auto-resolution/default) + * @return WrappedKey containing the newly generated and wrapped DEK + * @throws KMSException if generation or wrapping fails */ - WrappedKey rewrapKey(WrappedKey oldWrappedKey, String newKekLabel, Long targetHsmProfileId) throws KMSException; + WrappedKey generateAndWrapDek(KeyPurpose purpose, String kekLabel, int keyBits, + Long hsmProfileId) throws KMSException; /** * Rewrap a DEK with a different KEK (used during key rotation). @@ -195,6 +207,18 @@ public interface KMSProvider extends Configurable, Adapter { return rewrapKey(oldWrappedKey, newKekLabel, null); } + /** + * Rewrap a DEK with a different KEK (used during key rotation) using explicit target HSM profile. + * This unwraps with the old KEK and wraps with the new KEK without exposing the plaintext DEK. + * + * @param oldWrappedKey the currently wrapped key + * @param newKekLabel the label of the new KEK to wrap with + * @param targetHsmProfileId optional target HSM profile ID to wrap with (null for auto-resolution/default) + * @return new WrappedKey encrypted with the new KEK + * @throws KMSException if rewrapping fails + */ + WrappedKey rewrapKey(WrappedKey oldWrappedKey, String newKekLabel, Long targetHsmProfileId) throws KMSException; + /** * Perform health check on the provider backend * diff --git a/plugins/kms/pkcs11/src/main/java/org/apache/cloudstack/kms/provider/pkcs11/PKCS11HSMProvider.java b/plugins/kms/pkcs11/src/main/java/org/apache/cloudstack/kms/provider/pkcs11/PKCS11HSMProvider.java index ae86d8ced7f..54376f55c08 100644 --- a/plugins/kms/pkcs11/src/main/java/org/apache/cloudstack/kms/provider/pkcs11/PKCS11HSMProvider.java +++ b/plugins/kms/pkcs11/src/main/java/org/apache/cloudstack/kms/provider/pkcs11/PKCS11HSMProvider.java @@ -37,10 +37,10 @@ import javax.annotation.PostConstruct; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; +import javax.crypto.KeyGenerator; import javax.crypto.NoSuchPaddingException; import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; -import javax.crypto.spec.SecretKeySpec; import javax.inject.Inject; import java.io.Closeable; import java.io.File; @@ -75,6 +75,12 @@ import java.util.concurrent.TimeUnit; public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { private static final Logger logger = LogManager.getLogger(PKCS11HSMProvider.class); private static final String PROVIDER_NAME = "pkcs11"; + // Security note (#7): AES-CBC provides confidentiality but not authenticity (no + // HMAC). + // While AES-GCM is preferred, SunPKCS11 support for GCM is often buggy or + // missing + // depending on the underlying driver. We rely on the HSM/storage for tamper + // resistance. // AES-CBC with PKCS5Padding: FIPS-compliant (NIST SP 800-38A) with universal PKCS#11 support private static final String CIPHER_ALGORITHM = "AES/CBC/PKCS5Padding"; @@ -228,6 +234,15 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } } + @Override + public void invalidateProfileCache(Long profileId) { + HSMSessionPool pool = sessionPools.remove(profileId); + if (pool != null) { + pool.invalidate(); + } + logger.info("Invalidated HSM session pool for profile {}", profileId); + } + Long resolveProfileId(String kekLabel) throws KMSException { KMSKekVersionVO version = kmsKekVersionDao.findByKekLabel(kekLabel); if (version != null && version.getHsmProfileId() != null) { @@ -312,11 +327,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } } - String pin = config.get("pin"); - if (StringUtils.isEmpty(pin)) { - throw KMSException.invalidParameter("pin is required for PKCS#11 HSM profile"); - } - File libraryFile = new File(libraryPath); if (!libraryFile.exists() && !libraryFile.isAbsolute()) { // The HSM library might be in the system library path @@ -353,10 +363,7 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } boolean isSensitiveKey(String key) { - return key.equalsIgnoreCase("pin") || - key.equalsIgnoreCase("password") || - key.toLowerCase().contains("secret") || - key.equalsIgnoreCase("private_key"); + return KMSProvider.isSensitiveKey(key); } String generateKekLabel(KeyPurpose purpose) { @@ -373,15 +380,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { return new ConfigKey[0]; } - @Override - public void invalidateProfileCache(Long profileId) { - HSMSessionPool pool = sessionPools.remove(profileId); - if (pool != null) { - pool.invalidate(); - } - logger.info("Invalidated HSM session pool for profile {}", profileId); - } - @FunctionalInterface private interface SessionOperation { T execute(PKCS11Session session) throws KMSException; @@ -404,11 +402,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { this.availableSessions = new ArrayBlockingQueue<>(maxSessions); } - private PKCS11Session createNewSession() throws KMSException { - // Config (including decrypted PIN) is loaded fresh each time and not stored. - return new PKCS11Session(provider.loadProfileConfig(profileId)); - } - PKCS11Session acquireSession(long timeoutMs) throws KMSException { // Try to get an existing idle session first (no semaphore change: it already owns a permit). PKCS11Session session = availableSessions.poll(); @@ -451,6 +444,11 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } } + private PKCS11Session createNewSession() throws KMSException { + // Config (including decrypted PIN) is loaded fresh each time and not stored. + return new PKCS11Session(provider.loadProfileConfig(profileId)); + } + void releaseSession(PKCS11Session session) { if (session == null) return; if (!invalidated && session.isValid() && availableSessions.offer(session)) { @@ -725,15 +723,8 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { /** * Closes the PKCS#11 session and cleans up resources. * - *

This method: - *

    - *
  1. Closes the KeyStore (if it implements Closeable)
    2. - *
  2. Logs out from the HSM token
    3. - *
  3. Removes the provider from Security
    4. - *
  4. Clears all references
    5. - *
- * - *

Note: Errors during cleanup are logged but do not throw exceptions + *

+ * Note: Errors during cleanup are logged but do not throw exceptions * to ensure cleanup continues even if some steps fail. */ void close() { @@ -770,25 +761,26 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { /** * Generates an AES key directly in the HSM with the specified label. * - *

Implementation note: Due to limitations in the Java PKCS#11 API, this method: - *

    - *
  1. Generates a secure random key in software using SecureRandom
    2. - *
  2. Imports it into the HSM via KeyStore.setEntry() with the label
    3. - *
  3. Clears the key material from memory immediately
    4. - *
+ *

+ * This method generates the key natively inside the HSM using a + * {@link KeyGenerator} configured with the PKCS#11 provider, so the key + * material never leaves the HSM boundary. The returned PKCS#11-native key + * reference ({@code P11Key}) is then stored in the KeyStore under the + * requested label. * - *

While the key is briefly in software memory, this is necessary because: - *

+ *

+ * Using {@code KeyGenerator} with the HSM provider is required for + * HSMs such as NetHSM that do not support importing raw secret-key bytes + * via {@code KeyStore.setKeyEntry()}. By generating the key on the HSM first, + * the value passed to {@code setKeyEntry()} is already a PKCS#11 token object, + * so no raw-bytes import is attempted. * - *

Once imported, the key: + *

+ * Once stored, the key: *

* * @param label Unique label for the key in the HSM @@ -800,36 +792,35 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { String generateKey(String label, int keyBits, KeyPurpose purpose) throws KMSException { validateKeySize(keyBits); - byte[] keyBytes = null; try { // Check if key with this label already exists if (keyStore.containsAlias(label)) { throw KMSException.keyAlreadyExists("Key with label '" + label + "' already exists in HSM"); } - // Generate cryptographically secure random key material - // Using SecureRandom instead of HSM generation due to Java PKCS#11 API limitations - keyBytes = new byte[keyBits / 8]; - SecureRandom.getInstanceStrong().nextBytes(keyBytes); + // Generate the AES key natively inside the HSM using the PKCS#11 provider. + // This avoids importing raw key bytes into the HSM, which is not supported + // by all HSMs (e.g. NetHSM rejects SecretKeySpec via storeSkey()). + // The resulting key is a PKCS#11-native P11Key that lives inside the token. + KeyGenerator keyGen = KeyGenerator.getInstance("AES", provider); + keyGen.init(keyBits); + SecretKey hsmKey = keyGen.generateKey(); - // Wrap key bytes in a SecretKeySpec for import into HSM - SecretKey secretKey = new SecretKeySpec(keyBytes, "AES"); + // Associate the HSM-generated key with the requested label by storing + // it in the PKCS#11 KeyStore. Because hsmKey is already a P11Key + // (not a software SecretKeySpec), P11KeyStore.storeSkey() stores it + // as a persistent token object (CKA_TOKEN=true) with CKA_LABEL=label + // without attempting any raw-bytes conversion. + keyStore.setKeyEntry(label, hsmKey, null, null); - // Import into PKCS#11 KeyStore with label - // Uses setKeyEntry(String, Key, char[], Certificate[]) which is the only - // variant supported by P11KeyStore (the byte[] variant throws UnsupportedOperationException) - // The P11KeyStore will internally convert the SecretKeySpec to a P11 token object with: - // - CKA_TOKEN=true, CKA_LABEL=label, CKA_EXTRACTABLE=false - keyStore.setKeyEntry(label, secretKey, null, null); - - logger.info("Generated and imported AES-{} key '{}' into HSM (purpose: {})", + logger.info("Generated AES-{} key '{}' in HSM (purpose: {})", keyBits, label, purpose); return label; } catch (KeyStoreException e) { - handlePKCS11Exception(e, "Failed to import key into HSM KeyStore"); + handlePKCS11Exception(e, "Failed to store key in HSM KeyStore"); } catch (NoSuchAlgorithmException e) { - handlePKCS11Exception(e, "SecureRandom algorithm not available or key not retrievable"); + handlePKCS11Exception(e, "AES KeyGenerator not available via PKCS#11 provider"); } catch (Exception e) { String errorMsg = e.getMessage(); if (errorMsg != null && (errorMsg.contains("CKR_OBJECT_HANDLE_INVALID") @@ -838,13 +829,8 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } else { handlePKCS11Exception(e, "Failed to generate key in HSM"); } - } finally { - // Immediately clear sensitive key material from memory - if (keyBytes != null) { - Arrays.fill(keyBytes, (byte) 0); - } } - return null; // Unreachable + return null; } /** @@ -886,22 +872,15 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { throw KMSException.invalidParameter("Plain DEK cannot be null or empty"); } - SecretKey kek = null; + SecretKey kek = getKekFromKeyStore(kekLabel); try { - kek = getKekFromKeyStore(kekLabel); - - // Generate random IV for CBC byte[] iv = new byte[IV_LENGTH]; new SecureRandom().nextBytes(iv); - // Create cipher with AES-CBC Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM, provider); cipher.init(Cipher.ENCRYPT_MODE, kek, new IvParameterSpec(iv)); - - // Encrypt the plaintext DEK byte[] ciphertext = cipher.doFinal(plainDek); - // Prepend IV to ciphertext: [IV][ciphertext] byte[] result = new byte[IV_LENGTH + ciphertext.length]; System.arraycopy(iv, 0, result, 0, IV_LENGTH); System.arraycopy(ciphertext, 0, result, IV_LENGTH, ciphertext.length); @@ -917,10 +896,9 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } catch (Exception e) { handlePKCS11Exception(e, "Failed to wrap key with HSM"); } finally { - // Zeroize KEK reference (actual key material is in HSM, but clear reference) kek = null; } - return null; // Unreachable + return null; } /** @@ -947,33 +925,29 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } catch (KeyStoreException e) { handlePKCS11Exception(e, "Failed to retrieve KEK from HSM"); } - return null; // Unreachable + return null; } - /** * Unwraps (decrypts) a wrapped DEK using a KEK stored in the HSM. * - *

Uses AES-CBC with PKCS5Padding (FIPS 197 + NIST SP 800-38A): - *

    - *
  1. Extracts IV from the wrapped blob
    2. - *
  2. Retrieves KEK from HSM using the label
    3. - *
  3. Decrypts using AES-CBC
    4. - *
  4. Returns plaintext DEK
    5. - *
+ *

+ * Uses AES-CBC with PKCS5Padding. Expected format: [IV (16 bytes)][ciphertext]. * - *

Security: The returned plaintext DEK must be zeroized by the caller after use. - * - *

Expected format: [IV (16 bytes)][ciphertext] + *

+ * Security: The returned plaintext DEK must be zeroized by the caller after + * use. * * @param wrappedBlob Wrapped DEK blob (IV + ciphertext) * @param kekLabel Label of the KEK stored in the HSM * @return Plaintext DEK * @throws KMSException with appropriate ErrorType: *

*/ byte[] unwrapKey(byte[] wrappedBlob, String kekLabel) throws KMSException { @@ -981,27 +955,21 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { throw KMSException.invalidParameter("Wrapped blob cannot be null or empty"); } - // Minimum size: IV (16) + at least one block of ciphertext (16) + // Minimum size: IV (16 bytes) + at least one AES block (16 bytes) if (wrappedBlob.length < IV_LENGTH + 16) { throw KMSException.invalidParameter("Wrapped blob too short: expected at least " + (IV_LENGTH + 16) + " bytes"); } - SecretKey kek = null; + SecretKey kek = getKekFromKeyStore(kekLabel); try { - kek = getKekFromKeyStore(kekLabel); - - // Extract IV and ciphertext from wrapped blob byte[] iv = new byte[IV_LENGTH]; System.arraycopy(wrappedBlob, 0, iv, 0, IV_LENGTH); byte[] ciphertext = new byte[wrappedBlob.length - IV_LENGTH]; System.arraycopy(wrappedBlob, IV_LENGTH, ciphertext, 0, ciphertext.length); - // Create cipher with AES-CBC Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM, provider); cipher.init(Cipher.DECRYPT_MODE, kek, new IvParameterSpec(iv)); - - // Decrypt the ciphertext to get plaintext DEK byte[] plainDek = cipher.doFinal(ciphertext); logger.debug("Unwrapped key with KEK '{}' using AES-CBC", kekLabel); @@ -1018,13 +986,11 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } catch (Exception e) { handlePKCS11Exception(e, "Failed to unwrap key with HSM"); } finally { - // Zeroize KEK reference kek = null; } - return null; // Unreachable + return null; } - /** * Deletes a key from the HSM. * @@ -1040,12 +1006,10 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { */ void deleteKey(String label) throws KMSException { try { - // Check if key exists first if (!keyStore.containsAlias(label)) { throw KMSException.kekNotFound("Key with label '" + label + "' not found in HSM"); } - // Delete key from KeyStore keyStore.deleteEntry(label); logger.debug("Deleted key '{}' from HSM", label); @@ -1074,7 +1038,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { */ boolean checkKeyExists(String label) throws KMSException { try { - // Try to retrieve key from HSM KeyStore Key key = keyStore.getKey(label, null); return key != null; } catch (KeyStoreException e) { diff --git a/server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java b/server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java index fe552358a3f..9f791d75131 100644 --- a/server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java @@ -40,8 +40,12 @@ import com.cloud.utils.component.ManagerBase; import com.cloud.utils.component.PluggableService; import com.cloud.utils.crypt.DBEncryptionUtil; import com.cloud.utils.db.Filter; +import com.cloud.utils.db.GlobalLock; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; +import com.cloud.utils.db.Transaction; +import com.cloud.utils.db.TransactionCallbackWithException; +import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.command.admin.kms.MigrateVolumesToKMSCmd; @@ -69,7 +73,7 @@ import org.apache.cloudstack.kms.dao.HSMProfileDetailsDao; import org.apache.cloudstack.kms.dao.KMSKekVersionDao; import org.apache.cloudstack.kms.dao.KMSKeyDao; import org.apache.cloudstack.kms.dao.KMSWrappedKeyDao; -import org.apache.cloudstack.managed.context.ManagedContextTimerTask; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.secret.PassphraseVO; import org.apache.cloudstack.secret.dao.PassphraseDao; import org.apache.commons.collections4.CollectionUtils; @@ -83,19 +87,22 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Timer; import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; public class KMSManagerImpl extends ManagerBase implements KMSManager, PluggableService { private static final Logger logger = LogManager.getLogger(KMSManagerImpl.class); private static final Map kmsProviderMap = new HashMap<>(); - private final ExecutorService kmsOperationExecutor = Executors.newCachedThreadPool(r -> { + private final ExecutorService kmsOperationExecutor = new ThreadPoolExecutor( + 2, 100, 60L, TimeUnit.SECONDS, new SynchronousQueue<>(), r -> { Thread t = new Thread(r, "kms-operation"); t.setDaemon(true); return t; @@ -119,7 +126,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable @Inject private PassphraseDao passphraseDao; private List kmsProviders; - private Timer rewrapTimer; + private ScheduledExecutorService rewrapExecutor; @Override public List listKMSProviders() { @@ -585,12 +592,11 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " + "There are Volumes which still reference this key"); } - checkKmsKeyAccess(caller, key); logger.info("Deleted KMS key {}", key); } @Override - @ActionEvent(eventType = EventTypes.EVENT_KMS_KEY_CREATE, eventDescription = "rotating KMS key", async = true) + @ActionEvent(eventType = EventTypes.EVENT_KMS_KEY_ROTATE, eventDescription = "rotating KMS key", async = true) public String rotateKMSKey(RotateKMSKeyCmd cmd) throws KMSException { Account caller = CallContext.current().getCallingAccount(); Integer keyBits = cmd.getKeyBits(); @@ -632,7 +638,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable kmsWrappedKeyDao.countByKmsKeyId(kmsKey.getId())); ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), kmsKey.getAccountId(), - EventVO.LEVEL_INFO, EventTypes.EVENT_KMS_KEY_CREATE, + EventVO.LEVEL_INFO, EventTypes.EVENT_KMS_KEY_ROTATE, String.format("KMS key rotation completed for KMS key from version %d to version %d", currentActive.getVersionNumber(), newVersion.getVersionNumber()), kmsKey.getId(), ApiCommandResourceType.KmsKey.toString(), CallContext.current().getStartEventId()); @@ -661,23 +667,45 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable String finalNewKekLabel = newKekLabel; Long newProfileId = newHSMProfile.getId(); + final HSMProfileVO finalHSMProfile = newHSMProfile; String newKekId = retryOperation( () -> provider.createKek(kmsKey.getPurpose(), finalNewKekLabel, keyBits, newProfileId)); - KMSKekVersionVO newVersion = createKekVersion(kmsKey.getId(), newKekId, newProfileId); + try { + KMSKekVersionVO newVersion = Transaction + .execute(new TransactionCallbackWithException() { + @Override + public KMSKekVersionVO doInTransaction(TransactionStatus status) throws KMSException { + KMSKekVersionVO version = createKekVersion(kmsKey.getId(), newKekId, newProfileId); - if (!newProfileId.equals(kmsKey.getHsmProfileId())) { - kmsKey.setHsmProfileId(newProfileId); - kmsKeyDao.update(kmsKey.getId(), kmsKey); - logger.info("Updated KMS key {} to use HSM profile {}", kmsKey, newHSMProfile); + if (!newProfileId.equals(kmsKey.getHsmProfileId())) { + kmsKey.setHsmProfileId(newProfileId); + kmsKeyDao.update(kmsKey.getId(), kmsKey); + logger.info("Updated KMS key {} to use HSM profile {}", kmsKey, finalHSMProfile); + } + return version; + } + }); + + logger.info("KEK rotation: KMS key {} now has {} versions (active: v{}, previous: v{})", + kmsKey, newVersion.getVersionNumber(), newVersion.getVersionNumber(), + newVersion.getVersionNumber() - 1); + + return newKekId; + } catch (KMSException e) { + logger.error( + "Database update failed during KEK rotation for kmsKey {}. Attempting to delete orphaned KEK " + + "{} from provider {}", + kmsKey, newKekId, provider.getProviderName()); + try { + provider.deleteKek(newKekId); + } catch (KMSException ex) { + logger.error("Failed to delete orphaned KEK {} from provider {} after DB failure: {}", + newKekId, provider.getProviderName(), ex.getMessage()); + } + throw e; } - logger.info("KEK rotation: KMS key {} now has {} versions (active: v{}, previous: v{})", - kmsKey, newVersion.getVersionNumber(), newVersion.getVersionNumber(), - newVersion.getVersionNumber() - 1); - - return newKekId; - } catch (Exception e) { logger.error("KEK rotation failed for kmsKey {}: {}", kmsKey, e.getMessage()); throw handleKmsException(e); @@ -830,7 +858,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable if (passphrase == null) { logger.warn( "Skipping migration of volume from to the KMS key {} because passphrase id: {} not found for " - + "volume {}}", + + "volume {}", kmsKey, volume.getPassphraseId(), volume); return false; } @@ -1214,10 +1242,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable } boolean isSensitiveKey(String key) { - return key.equalsIgnoreCase("pin") || - key.equalsIgnoreCase("password") || - key.toLowerCase().contains("secret") || - key.equalsIgnoreCase("private_key"); + return KMSProvider.isSensitiveKey(key); } /** @@ -1394,7 +1419,18 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable } private void scheduleRewrapWorker() { - final ManagedContextTimerTask rewrapTask = new ManagedContextTimerTask() { + long intervalMs = KMSRewrapIntervalMs.value(); + if (intervalMs <= 0) { + return; + } + + rewrapExecutor = Executors.newScheduledThreadPool(1, r -> { + Thread t = new Thread(r, "KMSRewrapWorker"); + t.setDaemon(true); + return t; + }); + + rewrapExecutor.scheduleAtFixedRate(new ManagedContextRunnable() { @Override protected void runInContext() { try { @@ -1403,11 +1439,8 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable logger.error("Error while running KMS rewrap worker", e); } } - }; + }, 10000L, intervalMs, TimeUnit.MILLISECONDS); - long intervalMs = KMSRewrapIntervalMs.value(); - rewrapTimer = new Timer("KMSRewrapWorker", true); // daemon so it doesn't block JVM shutdown - rewrapTimer.schedule(rewrapTask, 10000L, intervalMs); logger.info("KMS rewrap worker scheduled with interval: {} ms", intervalMs); } @@ -1416,28 +1449,41 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable * using the active version. */ private void processRewrapBatch() { + GlobalLock lock = GlobalLock.getInternLock("kms.rewrap.worker"); try { - List previousVersions = kmsKekVersionDao.findByStatus(KMSKekVersionVO.Status.Previous); - - if (previousVersions.isEmpty()) { - logger.trace("No KEK versions pending rewrap"); - return; - } - - logger.debug("Found {} KEK version(s) with status Previous - processing rewrap batches", - previousVersions.size()); - - int batchSize = KMSRewrapBatchSize.value(); - - for (KMSKekVersionVO oldVersion : previousVersions) { + if (lock.lock(5)) { try { - processVersionRewrap(oldVersion, batchSize); - } catch (Exception e) { - logger.error("Error processing rewrap for KEK version {}: {}", oldVersion, e.getMessage(), e); + List previousVersions = kmsKekVersionDao + .findByStatus(KMSKekVersionVO.Status.Previous); + + if (previousVersions.isEmpty()) { + logger.trace("No KEK versions pending rewrap"); + return; + } + + logger.debug("Found {} KEK version(s) with status Previous - processing rewrap batches", + previousVersions.size()); + + int batchSize = KMSRewrapBatchSize.value(); + + for (KMSKekVersionVO oldVersion : previousVersions) { + try { + processVersionRewrap(oldVersion, batchSize); + } catch (Exception e) { + logger.error("Error processing rewrap for KEK version {}: {}", oldVersion, e.getMessage(), + e); + } + } + } finally { + lock.unlock(); } + } else { + logger.trace("KMS rewrap worker: could not acquire cluster lock, skipping batch"); } } catch (Exception e) { logger.error("Error in rewrap worker: {}", e.getMessage(), e); + } finally { + lock.releaseRef(); } } @@ -1457,12 +1503,26 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable List keysToRewrap = kmsWrappedKeyDao.listByKekVersionId(oldVersion.getId(), batchSize); if (keysToRewrap.isEmpty()) { - logger.info("All wrapped keys rewrapped for KEK version {} (v{}) - archiving", + logger.info("All wrapped keys rewrapped for KEK version {} (v{}) - archiving and deleting from provider", oldVersion.getUuid(), oldVersion.getVersionNumber()); oldVersion.setStatus(KMSKekVersionVO.Status.Archived); kmsKekVersionDao.update(oldVersion.getId(), oldVersion); + // Delete the old KEK from the HSM since no wrapped keys reference it anymore + try { + HSMProfileVO oldProfile = hsmProfileDao.findById(oldVersion.getHsmProfileId()); + if (oldProfile != null) { + KMSProvider provider = getKMSProvider(oldProfile.getProtocol()); + provider.deleteKek(oldVersion.getKekLabel()); + logger.info("Deleted archived KEK {} (v{}) from provider {}", + oldVersion.getKekLabel(), oldVersion.getVersionNumber(), provider.getProviderName()); + } + } catch (Exception e) { + logger.warn("Failed to delete archived KEK {} (v{}) from provider: {}", + oldVersion.getKekLabel(), oldVersion.getVersionNumber(), e.getMessage()); + } + return; } @@ -1513,9 +1573,9 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable @Override public boolean stop() { - if (rewrapTimer != null) { - rewrapTimer.cancel(); - rewrapTimer = null; + if (rewrapExecutor != null) { + rewrapExecutor.shutdownNow(); + rewrapExecutor = null; } kmsOperationExecutor.shutdownNow(); return super.stop(); diff --git a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java index 8aad751d4b9..94218d0e1e0 100644 --- a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java +++ b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.kms; +import com.cloud.api.ApiResponseHelper; import com.cloud.dc.dao.DataCenterDao; import com.cloud.domain.dao.DomainDao; import com.cloud.user.AccountManager; @@ -27,6 +28,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; @@ -130,7 +133,6 @@ public class KMSManagerImplHSMTest { when(profile.getUuid()).thenReturn("profile-uuid"); when(profile.getName()).thenReturn("test-profile"); when(profile.getProtocol()).thenReturn("PKCS11"); - when(profile.getAccountId()).thenReturn(testAccountId); when(profile.getVendorName()).thenReturn("TestVendor"); when(profile.isEnabled()).thenReturn(true); when(profile.getCreated()).thenReturn(new java.util.Date()); @@ -145,15 +147,11 @@ public class KMSManagerImplHSMTest { when(hsmProfileDetailsDao.listByProfileId(profileId)).thenReturn(Arrays.asList(detail1, detail2)); - com.cloud.user.Account mockAccount = mock(com.cloud.user.Account.class); - when(mockAccount.getUuid()).thenReturn("account-uuid"); - when(mockAccount.getAccountName()).thenReturn("testaccount"); - when(accountManager.getAccount(testAccountId)).thenReturn(mockAccount); + try (MockedStatic mockedApiResponseHelper = Mockito.mockStatic(ApiResponseHelper.class)) { + HSMProfileResponse response = kmsManager.createHSMProfileResponse(profile); - HSMProfileResponse response = kmsManager.createHSMProfileResponse(profile); - - assertNotNull("Response should not be null", response); - verify(accountManager).getAccount(testAccountId); - verify(hsmProfileDetailsDao).listByProfileId(profileId); + assertNotNull("Response should not be null", response); + verify(hsmProfileDetailsDao).listByProfileId(profileId); + } } } diff --git a/ui/src/config/section/kms.js b/ui/src/config/section/kms.js index 672d2e2bbff..9802b3dd9bf 100644 --- a/ui/src/config/section/kms.js +++ b/ui/src/config/section/kms.js @@ -206,6 +206,9 @@ export default { listView: true, popup: true, dataView: false, + show: (record, store) => { + return ['Admin'].includes(store.userInfo.roletype) + }, args: (record, store, group) => { return ['Admin'].includes(store.userInfo.roletype) ? ['name', 'zoneid', 'vendorname', 'domainid', 'account', 'projectid', 'details', 'system'] @@ -224,6 +227,9 @@ export default { label: 'label.update.hsm.profile', dataView: true, popup: true, + show: (record, store) => { + return ['Admin'].includes(store.userInfo.roletype) + }, args: ['id', 'name', 'enabled'], mapping: { id: { @@ -239,6 +245,9 @@ export default { message: 'message.action.delete.hsm.profile', dataView: true, popup: true, + show: (record, store) => { + return ['Admin'].includes(store.userInfo.roletype) + }, args: ['id'], mapping: { id: {