diff --git a/api/src/main/java/org/apache/cloudstack/kms/KMSManager.java b/api/src/main/java/org/apache/cloudstack/kms/KMSManager.java index 21f5f074c1f..1c473b7f93c 100644 --- a/api/src/main/java/org/apache/cloudstack/kms/KMSManager.java +++ b/api/src/main/java/org/apache/cloudstack/kms/KMSManager.java @@ -118,17 +118,6 @@ public interface KMSManager extends Manager, Configurable { */ KMSProvider getKMSProvider(String name); - /** - * Unwrap a DEK from a wrapped key - * SECURITY: Caller must zeroize returned byte array after use! - * - * @param wrappedKey the wrapped key from database - * @param zoneId the zone ID - * @return plaintext DEK (caller must zeroize!) - * @throws KMSException if unwrap fails - */ - byte[] unwrapVolumeKey(WrappedKey wrappedKey, Long zoneId) throws KMSException; - /** * Check if caller has permission to use a KMS key * diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java index 4d732473dd9..4a9f353fb74 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java @@ -404,6 +404,7 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol AllFieldsSearch.and("passphraseId", AllFieldsSearch.entity().getPassphraseId(), Op.EQ); AllFieldsSearch.and("iScsiName", AllFieldsSearch.entity().get_iScsiName(), Op.EQ); AllFieldsSearch.and("path", AllFieldsSearch.entity().getPath(), Op.EQ); + AllFieldsSearch.and("kmsKeyId", AllFieldsSearch.entity().getKmsKeyId(), Op.EQ); AllFieldsSearch.done(); RootDiskStateSearch = createSearchBuilder(); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKekVersionVO.java b/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKekVersionVO.java index e78fdaba53d..5d7b48e61be 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKekVersionVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKekVersionVO.java @@ -75,8 +75,6 @@ public class KMSKekVersionVO { private Status status; @Column(name = "hsm_profile_id") private Long hsmProfileId; - @Column(name = "hsm_key_label") - private String hsmKeyLabel; @Column(name = GenericDao.CREATED_COLUMN, nullable = false) @Temporal(TemporalType.TIMESTAMP) private Date created; @@ -84,12 +82,19 @@ public class KMSKekVersionVO { @Temporal(TemporalType.TIMESTAMP) private Date removed; - public KMSKekVersionVO(Long kmsKeyId, Integer versionNumber, String kekLabel, Status status) { + public KMSKekVersionVO(Long kmsKeyId, Integer versionNumber, String kekLabel) { this(); this.kmsKeyId = kmsKeyId; this.versionNumber = versionNumber; this.kekLabel = kekLabel; - this.status = status; + this.status = Status.Active; + } + + public KMSKekVersionVO(Long kmsKeyId, String kekLabel) { + this(); + this.kmsKeyId = kmsKeyId; + this.kekLabel = kekLabel; + this.status = Status.Active; } public KMSKekVersionVO() { @@ -154,13 +159,6 @@ public class KMSKekVersionVO { this.hsmProfileId = hsmProfileId; } - public String getHsmKeyLabel() { - return hsmKeyLabel; - } - - public void setHsmKeyLabel(String hsmKeyLabel) { - this.hsmKeyLabel = hsmKeyLabel; - } public Date getCreated() { return created; diff --git a/engine/schema/src/main/java/org/apache/cloudstack/kms/dao/KMSWrappedKeyDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/kms/dao/KMSWrappedKeyDaoImpl.java index ae115576007..fc1a7190ae8 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/kms/dao/KMSWrappedKeyDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/kms/dao/KMSWrappedKeyDaoImpl.java @@ -38,7 +38,6 @@ public class KMSWrappedKeyDaoImpl extends GenericDaoBase allFieldSearch.and("kmsKeyId", allFieldSearch.entity().getKmsKeyId(), SearchCriteria.Op.EQ); allFieldSearch.and("kekVersionId", allFieldSearch.entity().getKekVersionId(), SearchCriteria.Op.EQ); allFieldSearch.and("zoneId", allFieldSearch.entity().getZoneId(), SearchCriteria.Op.EQ); - allFieldSearch.and("kmsKeyId", allFieldSearch.entity().getKmsKeyId(), SearchCriteria.Op.EQ); allFieldSearch.done(); } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql index 875a144bfa7..43079183aa1 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql @@ -199,7 +199,6 @@ CREATE TABLE IF NOT EXISTS `cloud`.`kms_kek_versions` ( `kek_label` VARCHAR(255) NOT NULL COMMENT 'Provider-specific KEK label/ID for this version', `status` VARCHAR(32) NOT NULL DEFAULT 'Active' COMMENT 'Active, Previous, Archived', `hsm_profile_id` BIGINT UNSIGNED COMMENT 'HSM profile where this KEK version is stored', - `hsm_key_label` VARCHAR(255) COMMENT 'Optional HSM-specific key label/alias', `created` DATETIME NOT NULL COMMENT 'Creation timestamp', `removed` DATETIME COMMENT 'Removal timestamp for soft delete', PRIMARY KEY (`id`), diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 13b9553494d..49fce4e7040 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -29,6 +29,7 @@ import com.cloud.utils.db.Transaction; import com.cloud.utils.db.TransactionCallbackNoReturn; import com.cloud.utils.db.TransactionStatus; import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; +import org.apache.cloudstack.framework.kms.KMSException; import org.apache.cloudstack.secret.dao.PassphraseDao; import org.apache.cloudstack.secret.PassphraseVO; import com.cloud.service.dao.ServiceOfferingDetailsDao; @@ -971,8 +972,8 @@ public class VolumeObject implements VolumeInfo { java.util.Arrays.fill(dekBytes, (byte) 0); // Return UTF-8 bytes of the base64 string return base64Dek.getBytes(java.nio.charset.StandardCharsets.UTF_8); - } catch (org.apache.cloudstack.framework.kms.KMSException e) { - logger.error("Failed to unwrap KMS key for volume {}: {}", volumeVO.getId(), e.getMessage()); + } catch (KMSException e) { + logger.error("Failed to unwrap KMS key for volume {}: {}", volumeVO, e.getMessage(), e); return new byte[0]; } } diff --git a/plugins/kms/database/src/main/java/org/apache/cloudstack/kms/provider/DatabaseKMSProvider.java b/plugins/kms/database/src/main/java/org/apache/cloudstack/kms/provider/DatabaseKMSProvider.java index 7a91f8868c5..6daf3025172 100644 --- a/plugins/kms/database/src/main/java/org/apache/cloudstack/kms/provider/DatabaseKMSProvider.java +++ b/plugins/kms/database/src/main/java/org/apache/cloudstack/kms/provider/DatabaseKMSProvider.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.kms.provider; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.crypt.DBEncryptionUtil; import com.google.crypto.tink.subtle.AesGcmJce; + import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.kms.KMSException; import org.apache.cloudstack.framework.kms.KMSProvider; 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 b78dc1947f4..3b3853d53a7 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 @@ -26,7 +26,6 @@ import org.apache.cloudstack.framework.kms.KeyPurpose; import org.apache.cloudstack.framework.kms.WrappedKey; import org.apache.cloudstack.kms.HSMProfileDetailsVO; import org.apache.cloudstack.kms.KMSKekVersionVO; -import org.apache.cloudstack.kms.dao.HSMProfileDao; import org.apache.cloudstack.kms.dao.HSMProfileDetailsDao; import org.apache.cloudstack.kms.dao.KMSKekVersionDao; import org.apache.commons.lang3.StringUtils; @@ -75,7 +74,7 @@ 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 + // Security note: AES-CBC provides confidentiality but not authenticity (no // HMAC). // While AES-GCM is preferred, SunPKCS11 support for GCM is often buggy or // missing @@ -89,8 +88,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { private static final int[] VALID_KEY_SIZES = {128, 192, 256}; private final Map sessionPools = new ConcurrentHashMap<>(); @Inject - private HSMProfileDao hsmProfileDao; - @Inject private HSMProfileDetailsDao hsmProfileDetailsDao; @Inject private KMSKekVersionDao kmsKekVersionDao; @@ -125,6 +122,85 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { }); } + Long resolveProfileId(String kekLabel) throws KMSException { + KMSKekVersionVO version = kmsKekVersionDao.findByKekLabel(kekLabel); + if (version != null && version.getHsmProfileId() != null) { + return version.getHsmProfileId(); + } + throw new KMSException(KMSException.ErrorType.KEK_NOT_FOUND, + "Could not resolve HSM profile for KEK: " + kekLabel); + } + + /** + * Validates HSM profile configuration for PKCS#11 provider. + * + *

+ * Validates: + *

    + *
  • {@code library}: Required, should point to PKCS#11 library
    • + *
  • {@code slot}, {@code slot_list_index}, or {@code token_label}: At least + * one required
    • + *
  • {@code pin}: Required for HSM authentication
    • + *
  • {@code max_sessions}: Optional, must be positive integer if provided
    • + *
+ * + * @param config Configuration map from HSM profile details + * @throws KMSException with {@code INVALID_PARAMETER} if validation fails + */ + @Override + public void validateProfileConfig(Map config) throws KMSException { + String libraryPath = config.get("library"); + if (StringUtils.isBlank(libraryPath)) { + throw KMSException.invalidParameter("library is required for PKCS#11 HSM profile"); + } + + String slot = config.get("slot"); + String slotListIndex = config.get("slot_list_index"); + String tokenLabel = config.get("token_label"); + if (StringUtils.isAllBlank(slot, slotListIndex, tokenLabel)) { + throw KMSException.invalidParameter( + "One of 'slot', 'slot_list_index', or 'token_label' is required for PKCS#11 HSM profile"); + } + + if (StringUtils.isNotBlank(slot)) { + try { + Integer.parseInt(slot); + } catch (NumberFormatException e) { + throw KMSException.invalidParameter("slot must be a valid integer: " + slot); + } + } + + if (StringUtils.isNotBlank(slotListIndex)) { + try { + int idx = Integer.parseInt(slotListIndex); + if (idx < 0) { + throw KMSException.invalidParameter("slot_list_index must be a non-negative integer"); + } + } catch (NumberFormatException e) { + throw KMSException.invalidParameter("slot_list_index must be a valid integer: " + slotListIndex); + } + } + + File libraryFile = new File(libraryPath); + if (!libraryFile.exists() && !libraryFile.isAbsolute()) { + // The HSM library might be in the system library path + logger.debug("Library path {} does not exist as absolute path, will rely on system library path", + libraryPath); + } + + String max_sessions = config.get("max_sessions"); + if (StringUtils.isNotBlank(max_sessions)) { + try { + int idx = Integer.parseInt(max_sessions); + if (idx <= 0) { + throw KMSException.invalidParameter("max_sessions must be greater than 0"); + } + } catch (NumberFormatException e) { + throw KMSException.invalidParameter("max_sessions must be a valid integer: " + max_sessions); + } + } + } + @Override public boolean isKekAvailable(String kekId) throws KMSException { try { @@ -245,15 +321,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { 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) { - return version.getHsmProfileId(); - } - throw new KMSException(KMSException.ErrorType.KEK_NOT_FOUND, - "Could not resolve HSM profile for KEK: " + kekLabel); - } - /** * Executes an operation with a session from the pool, handling acquisition and release. * @@ -295,82 +362,11 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { return config; } - /** - * Validates HSM profile configuration for PKCS#11 provider. - * - *

- * Validates: - *

    - *
  • {@code library}: Required, should point to PKCS#11 library
    • - *
  • {@code slot}, {@code slot_list_index}, or {@code token_label}: At least - * one required
    • - *
  • {@code pin}: Required for HSM authentication
    • - *
  • {@code max_sessions}: Optional, must be positive integer if provided
    • - *
- * - * @param config Configuration map from HSM profile details - * @throws KMSException with {@code INVALID_PARAMETER} if validation fails - */ - @Override - public void validateProfileConfig(Map config) throws KMSException { - String libraryPath = config.get("library"); - if (StringUtils.isBlank(libraryPath)) { - throw KMSException.invalidParameter("library is required for PKCS#11 HSM profile"); - } - - String slot = config.get("slot"); - String slotListIndex = config.get("slot_list_index"); - String tokenLabel = config.get("token_label"); - if (StringUtils.isAllBlank(slot, slotListIndex, tokenLabel)) { - throw KMSException.invalidParameter( - "One of 'slot', 'slot_list_index', or 'token_label' is required for PKCS#11 HSM profile"); - } - - if (StringUtils.isNotBlank(slot)) { - try { - Integer.parseInt(slot); - } catch (NumberFormatException e) { - throw KMSException.invalidParameter("slot must be a valid integer: " + slot); - } - } - - if (StringUtils.isNotBlank(slotListIndex)) { - try { - int idx = Integer.parseInt(slotListIndex); - if (idx < 0) { - throw KMSException.invalidParameter("slot_list_index must be a non-negative integer"); - } - } catch (NumberFormatException e) { - throw KMSException.invalidParameter("slot_list_index must be a valid integer: " + slotListIndex); - } - } - - File libraryFile = new File(libraryPath); - if (!libraryFile.exists() && !libraryFile.isAbsolute()) { - // The HSM library might be in the system library path - logger.debug("Library path {} does not exist as absolute path, will rely on system library path", - libraryPath); - } - - String max_sessions = config.get("max_sessions"); - if (StringUtils.isNotBlank(max_sessions)) { - try { - int idx = Integer.parseInt(max_sessions); - if (idx <= 0) { - throw KMSException.invalidParameter("max_sessions must be greater than 0"); - } - } catch (NumberFormatException e) { - throw KMSException.invalidParameter("max_sessions must be a valid integer: " + max_sessions); - } - } - } - boolean isSensitiveKey(String key) { return KMSProvider.isSensitiveKey(key); } - @Override public String getConfigComponentName() { return PKCS11HSMProvider.class.getSimpleName(); @@ -641,6 +637,17 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { throw KMSException.invalidParameter("One of 'slot', 'slot_list_index', or 'token_label' is required"); } + // Explicitly configure SunPKCS11 to generate AES keys as Data Encryption Keys. + // Strict HSMs (like Thales Luna in FIPS mode) forbid a key from having both + // CKA_WRAP and CKA_ENCRYPT attributes. Because CloudStack uses Cipher.ENCRYPT_MODE + // (which maps to C_Encrypt) to protect the DEK, the KEK must have CKA_ENCRYPT=true. + configBuilder.append("\nattributes(generate, CKO_SECRET_KEY, CKK_AES) = {\n"); + configBuilder.append(" CKA_ENCRYPT = true\n"); + configBuilder.append(" CKA_DECRYPT = true\n"); + configBuilder.append(" CKA_WRAP = false\n"); + configBuilder.append(" CKA_UNWRAP = false\n"); + configBuilder.append("}\n"); + return configBuilder.toString(); } @@ -823,9 +830,9 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider { } catch (KeyStoreException e) { if (e.getMessage() != null - && e.getMessage().contains("found multiple secret keys sharing same CKA_LABEL")) { + && e.getMessage().contains("found multiple secret keys sharing same CKA_LABEL")) { logger.warn("Multiple duplicate keys found with label '{}' in HSM. Reusing the existing key. " + - "Please purge duplicate keys manually if possible.", label); + "Please purge duplicate keys manually if possible.", label); return label; } handlePKCS11Exception(e, "Failed to store key in HSM KeyStore"); diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 7349ec6dc03..541e03b4640 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -218,7 +218,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.gui.theme.GuiThemeJoin; -import org.apache.cloudstack.kms.dao.HSMProfileDao; import org.apache.cloudstack.management.ManagementServerHost; import org.apache.cloudstack.network.BgpPeerVO; import org.apache.cloudstack.network.RoutedIpv4Manager; @@ -530,8 +529,6 @@ public class ApiResponseHelper implements ResponseGenerator { private ASNumberRangeDao asNumberRangeDao; @Inject private ASNumberDao asNumberDao; - @Inject - private HSMProfileDao hsmProfileDao; @Inject ObjectStoreDao _objectStoreDao; diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 6f8647422cd..85e4a84a25d 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -1,4 +1,4 @@ -// Licensed to the Apache Software Foundation (ASF) under one + // Licensed to the Apache Software Foundation (ASF) under one // or more contributor license agreements. See the NOTICE file // distributed with this work for additional information // regarding copyright ownership. The ASF licenses this file @@ -32,7 +32,6 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.kms.KMSKekVersionVO; import org.apache.cloudstack.kms.KMSWrappedKeyVO; 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.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -63,8 +62,6 @@ public class VolumeJoinDaoImpl extends GenericDaoBaseWithTagInformation provider.unwrapKey(wrappedKey)); - } catch (Exception e) { - logger.error("Failed to unwrap key: {}", e.getMessage()); - throw handleKmsException(e); - } - } - @Override public boolean hasPermission(Long callerAccountId, KMSKey key) { if (callerAccountId == null) { @@ -196,8 +179,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable if (kmsKeyId == null) { return; } - checkKmsKeyAccess(caller, kmsKeyId); - KMSKeyVO key = kmsKeyDao.findById(kmsKeyId); + KMSKeyVO key = findKMSKeyAndCheckAccess(kmsKeyId, caller); if (key.getZoneId() != null && zoneId != null && !key.getZoneId().equals(zoneId)) { throw new InvalidParameterValueException( "KMS key belongs to zone " + key.getZoneId() + @@ -213,30 +195,6 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable } } - /** - * Validate that the caller has permission to use a KMS key. - * No-op if kmsKeyId is null. - * - * @param caller the caller's account - * @param kmsKeyId the KMS key database ID - * @throws InvalidParameterValueException if key not found - * @throws PermissionDeniedException if caller lacks access - */ - public void checkKmsKeyAccess(Account caller, Long kmsKeyId) { - if (kmsKeyId == null) { - return; - } - KMSKeyVO key = kmsKeyDao.findById(kmsKeyId); - checkKmsKeyAccess(caller, key); - } - - public void checkKmsKeyAccess(Account caller, KMSKeyVO key) { - if (key == null) { - throw new InvalidParameterValueException("KMS key not found"); - } - accountManager.checkAccess(caller, null, true, key); - } - @Override public byte[] unwrapKey(Long wrappedKeyId) throws KMSException { KMSWrappedKeyVO wrappedVO = kmsWrappedKeyDao.findById(wrappedKeyId); @@ -453,8 +411,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable kmsKey.setHsmProfileId(finalProfileId); kmsKey = kmsKeyDao.persist(kmsKey); - KMSKekVersionVO initialVersion = new KMSKekVersionVO(kmsKey.getId(), 1, providerKekLabel, - KMSKekVersionVO.Status.Active); + KMSKekVersionVO initialVersion = new KMSKekVersionVO(kmsKey.getId(), 1, providerKekLabel); initialVersion.setHsmProfileId(finalProfileId); initialVersion = kmsKekVersionDao.persist(initialVersion); @@ -549,7 +506,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable return createKMSKeyResponse(updatedKey); } - private KMSKey updateUserKMSKey(KMSKeyVO key, String name, String description, Boolean enabled) { + KMSKey updateUserKMSKey(KMSKeyVO key, String name, String description, Boolean enabled) { boolean updated = false; if (name != null && !name.equals(key.getName())) { key.setName(name); @@ -578,22 +535,39 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable KMSKeyVO key = findKMSKeyAndCheckAccess(cmd.getId(), caller); - deleteUserKMSKey(key, caller); + deleteUserKMSKey(key); return new SuccessResponse(); } - private void deleteUserKMSKey(KMSKeyVO key, Account caller) throws KMSException { + void deleteUserKMSKey(KMSKeyVO key) throws KMSException { long wrappedKeyCount = kmsWrappedKeyDao.countByKmsKeyId(key.getId()); if (wrappedKeyCount > 0) { throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " + wrappedKeyCount + " wrapped key(s) still reference this key"); } - kmsKeyDao.remove(key.getId()); if (volumeDao.existsWithKmsKey(key.getId())) { throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " + "There are Volumes which still reference this key"); } + + List kekVersions = kmsKekVersionDao.listByKmsKeyId(key.getId()); + for (KMSKekVersionVO kekVersion : kekVersions) { + try { + HSMProfileVO hsmProfile = hsmProfileDao.findById(kekVersion.getHsmProfileId()); + if (hsmProfile != null) { + KMSProvider provider = getKMSProvider(hsmProfile.getProtocol()); + provider.deleteKek(kekVersion.getKekLabel()); + logger.info("Deleted KEK {} (v{}) from provider {}", + kekVersion.getKekLabel(), kekVersion.getVersionNumber(), provider.getProviderName()); + } + } catch (Exception e) { + logger.warn("Failed to delete KEK {} (v{}) from provider during KMS key deletion: {}", + kekVersion.getKekLabel(), kekVersion.getVersionNumber(), e.getMessage()); + } + } + + kmsKeyDao.remove(key.getId()); logger.info("Deleted KMS key {}", key); } @@ -663,13 +637,14 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable try { logger.info("Starting KEK rotation from {} to {} for kms key {}", oldKekLabel, newKekLabel, kmsKey); - final KMSKekVersionVO newVersionEntity = new KMSKekVersionVO(); if (StringUtils.isEmpty(newKekLabel)) { List existingVersions = kmsKekVersionDao.listByKmsKeyId(kmsKey.getId()); - int nextVersion = existingVersions.stream().mapToInt(KMSKekVersionVO::getVersionNumber).max().orElse(0) + 1; + int nextVersion = existingVersions.stream().mapToInt(KMSKekVersionVO::getVersionNumber).max().orElse(0) + + 1; newKekLabel = kmsKey.getPurpose().generateKekLabel(kmsKey.getDomainId(), kmsKey.getAccountId(), kmsKey.getUuid(), nextVersion); } + final KMSKekVersionVO newVersionEntity = new KMSKekVersionVO(kmsKey.getId(), newKekLabel); String finalNewKekLabel = newKekLabel; Long newProfileId = newHSMProfile.getId(); @@ -679,20 +654,19 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable try { KMSKekVersionVO newVersion = Transaction - .execute(new TransactionCallbackWithException() { - @Override - public KMSKekVersionVO doInTransaction(TransactionStatus status) throws KMSException { - newVersionEntity.setKmsKeyId(kmsKey.getId()); - newVersionEntity.setHsmProfileId(newProfileId); - KMSKekVersionVO version = createKekVersion(newVersionEntity); + .execute((TransactionCallbackWithException) status -> { + newVersionEntity.setKmsKeyId(kmsKey.getId()); + newVersionEntity.setHsmProfileId(newProfileId); + newVersionEntity.setKekLabel(finalNewKekLabel); + KMSKekVersionVO version = createKekVersion(newVersionEntity); - 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; + if (!newProfileId.equals(kmsKey.getHsmProfileId())) { + kmsKey.setHsmProfileId(newProfileId); + logger.info("Updated KMS key {} to use HSM profile {}", kmsKey, finalHSMProfile); } + kmsKey.setKekLabel(finalNewKekLabel); + kmsKeyDao.update(kmsKey.getId(), kmsKey); + return version; }); logger.info("KEK rotation: KMS key {} now has {} versions (active: v{}, previous: v{})", @@ -764,11 +738,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable throw new InvalidParameterValueException("kmsKeyId must be specified"); } - KMSKeyVO kmsKey = kmsKeyDao.findById(kmsKeyId); - if (kmsKey == null) { - throw new InvalidParameterValueException("KMS key not found: " + kmsKeyId); - } - checkKmsKeyAccess(caller, kmsKey); + KMSKeyVO kmsKey = findKMSKeyAndCheckAccess(kmsKeyId, caller); if (!kmsKey.isEnabled()) { throw new InvalidParameterValueException("KMS key is not enabled: " + kmsKey.getUuid()); @@ -1254,18 +1224,6 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable return KMSProvider.isSensitiveKey(key); } - /** - * Find a KMS key by ID and verify the caller has write access to it. - */ - private KMSKeyVO findKMSKeyAndCheckAccess(Long keyId, Account caller) { - KMSKeyVO key = kmsKeyDao.findById(keyId); - if (key == null) { - throw new InvalidParameterValueException("KMS key not found: " + keyId); - } - accountManager.checkAccess(caller, null, true, key); - return key; - } - /** * Find an HSM profile by ID, throwing InvalidParameterValueException if not * found. @@ -1284,7 +1242,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable * root admin. * For owned profiles: delegates to ACL checkAccess. */ - private void checkHSMProfileAccess(Account caller, HSMProfileVO profile, boolean requireModifyAccess) { + void checkHSMProfileAccess(Account caller, HSMProfileVO profile, boolean requireModifyAccess) { if (profile.isSystem()) { if (requireModifyAccess && !accountManager.isRootAdmin(caller.getId())) { throw new PermissionDeniedException("Only root admins can modify system HSM profiles"); @@ -1297,7 +1255,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable /** * Parse and validate a key purpose string. Returns null if the input is null. */ - private KeyPurpose parseKeyPurpose(String purpose) { + KeyPurpose parseKeyPurpose(String purpose) { if (purpose == null) { return null; } @@ -1384,6 +1342,22 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable return KMSException.transientError("KMS operation failed: " + e.getMessage(), e); } + /** + * Find a KMS key by ID and verify the caller has access to it. + * Throws {@link InvalidParameterValueException} if the key does not exist + * and {@link PermissionDeniedException} if the caller lacks access. + * + * @return the resolved {@link KMSKeyVO} + */ + KMSKeyVO findKMSKeyAndCheckAccess(Long keyId, Account caller) { + KMSKeyVO key = kmsKeyDao.findById(keyId); + if (key == null) { + throw new InvalidParameterValueException("KMS key not found: " + keyId); + } + accountManager.checkAccess(caller, null, true, key); + return key; + } + public void setKmsProviders(List kmsProviders) { this.kmsProviders = kmsProviders; initializeKmsProviderMap(); diff --git a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplAccessTest.java b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplAccessTest.java new file mode 100644 index 00000000000..35e84ec20ca --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplAccessTest.java @@ -0,0 +1,287 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.kms; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import org.apache.cloudstack.framework.kms.KeyPurpose; +import org.apache.cloudstack.kms.dao.HSMProfileDao; +import org.apache.cloudstack.kms.dao.KMSKeyDao; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Unit tests covering access and permission helpers in KMSManagerImpl. + */ +@RunWith(MockitoJUnitRunner.class) +public class KMSManagerImplAccessTest { + + @Spy + @InjectMocks + private KMSManagerImpl kmsManager; + + @Mock + private KMSKeyDao kmsKeyDao; + + @Mock + private HSMProfileDao hsmProfileDao; + + @Mock + private AccountManager accountManager; + + @Test + public void testHasPermission_ReturnsFalseWhenCallerAccountIdIsNull() { + assertFalse(kmsManager.hasPermission(null, mock(KMSKey.class))); + } + + @Test + public void testHasPermission_ReturnsFalseWhenKeyIsNull() { + assertFalse(kmsManager.hasPermission(1L, null)); + } + + @Test(expected = InvalidParameterValueException.class) + public void testHasPermission_ThrowsWhenKeyIsDisabled() { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(false); + kmsManager.hasPermission(1L, key); + } + + @Test + public void testHasPermission_ReturnsFalseWhenCallerAccountNotFound() { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(accountManager.getAccount(1L)).thenReturn(null); + + assertFalse(kmsManager.hasPermission(1L, key)); + } + + @Test + public void testHasPermission_ReturnsFalseWhenPermissionDenied() { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(key.getAccountId()).thenReturn(10L); + + Account caller = mock(Account.class); + Account owner = mock(Account.class); + when(accountManager.getAccount(1L)).thenReturn(caller); + when(accountManager.getAccount(10L)).thenReturn(owner); + doThrow(new PermissionDeniedException("denied")) + .when(accountManager).checkAccess(caller, null, true, owner); + + assertFalse(kmsManager.hasPermission(1L, key)); + } + + @Test + public void testHasPermission_ReturnsTrueWhenAccessGranted() { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(key.getAccountId()).thenReturn(10L); + + Account caller = mock(Account.class); + Account owner = mock(Account.class); + when(accountManager.getAccount(1L)).thenReturn(caller); + when(accountManager.getAccount(10L)).thenReturn(owner); + + assertTrue(kmsManager.hasPermission(1L, key)); + } + + @Test(expected = InvalidParameterValueException.class) + public void testFindKMSKeyAndCheckAccess_ThrowsWhenKeyNotFound() { + when(kmsKeyDao.findById(99L)).thenReturn(null); + kmsManager.findKMSKeyAndCheckAccess(99L, mock(Account.class)); + } + + @Test(expected = PermissionDeniedException.class) + public void testFindKMSKeyAndCheckAccess_ThrowsWhenPermissionDenied() { + KMSKeyVO key = mock(KMSKeyVO.class); + Account caller = mock(Account.class); + when(kmsKeyDao.findById(1L)).thenReturn(key); + doThrow(new PermissionDeniedException("denied")) + .when(accountManager).checkAccess(caller, null, true, key); + + kmsManager.findKMSKeyAndCheckAccess(1L, caller); + } + + @Test + public void testFindKMSKeyAndCheckAccess_ReturnsKeyOnSuccess() { + KMSKeyVO key = mock(KMSKeyVO.class); + Account caller = mock(Account.class); + when(kmsKeyDao.findById(1L)).thenReturn(key); + + KMSKeyVO result = kmsManager.findKMSKeyAndCheckAccess(1L, caller); + + assertSame(key, result); + } + + @Test + public void testCheckKmsKeyForVolumeEncryption_NoOpWhenKeyIdIsNull() { + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), null, 1L); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCheckKmsKeyForVolumeEncryption_ThrowsWhenKeyNotFound() { + when(kmsKeyDao.findById(1L)).thenReturn(null); + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), 1L, null); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckKmsKeyForVolumeEncryption_ThrowsWhenPermissionDenied() { + KMSKeyVO key = mock(KMSKeyVO.class); + Account caller = mock(Account.class); + when(kmsKeyDao.findById(1L)).thenReturn(key); + doThrow(new PermissionDeniedException("denied")) + .when(accountManager).checkAccess(caller, null, true, key); + + kmsManager.checkKmsKeyForVolumeEncryption(caller, 1L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCheckKmsKeyForVolumeEncryption_ThrowsOnZoneMismatch() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getZoneId()).thenReturn(2L); + when(kmsKeyDao.findById(1L)).thenReturn(key); + + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), 1L, 3L); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCheckKmsKeyForVolumeEncryption_ThrowsWhenKeyDisabled() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getZoneId()).thenReturn(null); + when(key.isEnabled()).thenReturn(false); + when(kmsKeyDao.findById(1L)).thenReturn(key); + + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), 1L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCheckKmsKeyForVolumeEncryption_ThrowsWhenWrongPurpose() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getZoneId()).thenReturn(null); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.TLS_CERT); + when(kmsKeyDao.findById(1L)).thenReturn(key); + + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), 1L, null); + } + + @Test + public void testCheckKmsKeyForVolumeEncryption_PassesForMatchingZone() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getZoneId()).thenReturn(1L); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(kmsKeyDao.findById(1L)).thenReturn(key); + + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), 1L, 1L); + } + + @Test + public void testCheckKmsKeyForVolumeEncryption_PassesWhenKeyHasNoZoneRestriction() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getZoneId()).thenReturn(null); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(kmsKeyDao.findById(1L)).thenReturn(key); + + kmsManager.checkKmsKeyForVolumeEncryption(mock(Account.class), 1L, 5L); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckHSMProfileAccess_DeniesNonRootModifyOfSystemProfile() { + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isSystem()).thenReturn(true); + + Account caller = mock(Account.class); + when(caller.getId()).thenReturn(1L); + when(accountManager.isRootAdmin(1L)).thenReturn(false); + + kmsManager.checkHSMProfileAccess(caller, profile, true); + } + + @Test + public void testCheckHSMProfileAccess_AllowsRootModifyOfSystemProfile() { + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isSystem()).thenReturn(true); + + Account caller = mock(Account.class); + when(caller.getId()).thenReturn(1L); + when(accountManager.isRootAdmin(1L)).thenReturn(true); + + kmsManager.checkHSMProfileAccess(caller, profile, true); + } + + @Test + public void testCheckHSMProfileAccess_AllowsReadAccessToSystemProfileForAllUsers() { + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isSystem()).thenReturn(true); + + kmsManager.checkHSMProfileAccess(mock(Account.class), profile, false); + } + + @Test + public void testCheckHSMProfileAccess_DelegatesToAclForOwnedProfile() { + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isSystem()).thenReturn(false); + + kmsManager.checkHSMProfileAccess(mock(Account.class), profile, true); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckHSMProfileAccess_ThrowsWhenAclDeniesOwnedProfile() { + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isSystem()).thenReturn(false); + + Account caller = mock(Account.class); + doThrow(new PermissionDeniedException("denied")) + .when(accountManager).checkAccess(caller, null, true, profile); + + kmsManager.checkHSMProfileAccess(caller, profile, true); + } + + @Test + public void testParseKeyPurpose_ReturnsNullForNullInput() { + assertNull(kmsManager.parseKeyPurpose(null)); + } + + @Test + public void testParseKeyPurpose_ReturnsVolumeEncryptionForValidName() { + KeyPurpose result = kmsManager.parseKeyPurpose("volume"); + assertNotNull(result); + } + + @Test(expected = InvalidParameterValueException.class) + public void testParseKeyPurpose_ThrowsForUnknownPurpose() { + kmsManager.parseKeyPurpose("not-a-valid-purpose"); + } +} diff --git a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyLifecycleTest.java b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyLifecycleTest.java new file mode 100644 index 00000000000..67d675bd46b --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyLifecycleTest.java @@ -0,0 +1,426 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.kms; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.storage.dao.VolumeDao; +import org.apache.cloudstack.framework.kms.KMSException; +import org.apache.cloudstack.framework.kms.KMSProvider; +import org.apache.cloudstack.framework.kms.KeyPurpose; +import org.apache.cloudstack.framework.kms.WrappedKey; +import org.apache.cloudstack.kms.dao.HSMProfileDao; +import org.apache.cloudstack.kms.dao.KMSKekVersionDao; +import org.apache.cloudstack.kms.dao.KMSKeyDao; +import org.apache.cloudstack.kms.dao.KMSWrappedKeyDao; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.List; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests covering key lifecycle operations in KMSManagerImpl. + */ +@RunWith(MockitoJUnitRunner.class) +public class KMSManagerImplKeyLifecycleTest { + + @Spy + @InjectMocks + private KMSManagerImpl kmsManager; + + @Mock + private KMSKeyDao kmsKeyDao; + + @Mock + private KMSKekVersionDao kmsKekVersionDao; + + @Mock + private KMSWrappedKeyDao kmsWrappedKeyDao; + + @Mock + private HSMProfileDao hsmProfileDao; + + @Mock + private VolumeDao volumeDao; + + @Mock + private KMSProvider kmsProvider; + + @Before + public void setUp() { + doReturn(5).when(kmsManager).getOperationTimeoutSec(); + doReturn(0).when(kmsManager).getRetryCount(); + doReturn(0).when(kmsManager).getRetryDelayMs(); + } + + @Test(expected = KMSException.class) + public void testUnwrapKey_ThrowsWhenWrappedKeyNotFound() throws KMSException { + when(kmsWrappedKeyDao.findById(1L)).thenReturn(null); + kmsManager.unwrapKey(1L); + } + + @Test(expected = KMSException.class) + public void testUnwrapKey_ThrowsWhenKmsKeyNotFound() throws KMSException { + KMSWrappedKeyVO wrappedVO = mock(KMSWrappedKeyVO.class); + when(wrappedVO.getKmsKeyId()).thenReturn(10L); + when(kmsWrappedKeyDao.findById(1L)).thenReturn(wrappedVO); + when(kmsKeyDao.findById(10L)).thenReturn(null); + + kmsManager.unwrapKey(1L); + } + + @Test + public void testUnwrapKey_SucceedsWithHintVersion() { + Long wrappedKeyId = 1L; + Long kmsKeyId = 10L; + Long versionId = 5L; + + KMSWrappedKeyVO wrappedVO = mock(KMSWrappedKeyVO.class); + when(wrappedVO.getKmsKeyId()).thenReturn(kmsKeyId); + when(wrappedVO.getKekVersionId()).thenReturn(versionId); + when(wrappedVO.getWrappedBlob()).thenReturn(new byte[]{0, 1}); + when(kmsWrappedKeyDao.findById(wrappedKeyId)).thenReturn(wrappedVO); + + KMSKeyVO kmsKey = mock(KMSKeyVO.class); + when(kmsKey.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(kmsKey.getAlgorithm()).thenReturn("AES/GCM/NoPadding"); + when(kmsKeyDao.findById(kmsKeyId)).thenReturn(kmsKey); + + KMSKekVersionVO version = mock(KMSKekVersionVO.class); + when(version.getStatus()).thenReturn(KMSKekVersionVO.Status.Active); + when(version.getHsmProfileId()).thenReturn(20L); + when(version.getKekLabel()).thenReturn("kek-label"); + when(kmsKekVersionDao.findById(versionId)).thenReturn(version); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.getProtocol()).thenReturn("database"); + when(hsmProfileDao.findById(20L)).thenReturn(profile); + + when(kmsProvider.unwrapKey(any(WrappedKey.class), anyLong())).thenReturn(new byte[]{1, 2, 3}); + doReturn(kmsProvider).when(kmsManager).getKMSProvider("database"); + + byte[] result = kmsManager.unwrapKey(wrappedKeyId); + + assertNotNull(result); + verify(kmsKekVersionDao, never()).getVersionsForDecryption(kmsKeyId); + } + + @Test(expected = KMSException.class) + public void testUnwrapKey_FallsBackToAllVersionsWhenNoHint() { + KMSWrappedKeyVO wrappedVO = mock(KMSWrappedKeyVO.class); + when(wrappedVO.getKmsKeyId()).thenReturn(10L); + when(wrappedVO.getKekVersionId()).thenReturn(null); + when(kmsWrappedKeyDao.findById(1L)).thenReturn(wrappedVO); + when(kmsKeyDao.findById(10L)).thenReturn(mock(KMSKeyVO.class)); + + kmsManager.unwrapKey(1L); + } + + @Test(expected = KMSException.class) + public void testUnwrapKey_ThrowsWhenAllVersionsFail() { + KMSWrappedKeyVO wrappedVO = mock(KMSWrappedKeyVO.class); + when(wrappedVO.getKmsKeyId()).thenReturn(10L); + when(wrappedVO.getKekVersionId()).thenReturn(null); + when(kmsWrappedKeyDao.findById(1L)).thenReturn(wrappedVO); + when(kmsKeyDao.findById(10L)).thenReturn(mock(KMSKeyVO.class)); + + kmsManager.unwrapKey(1L); + } + + @Test(expected = KMSException.class) + public void testGenerateVolumeKeyWithKek_ThrowsWhenKeyNull() throws KMSException { + kmsManager.generateVolumeKeyWithKek(null, 1L); + } + + @Test(expected = KMSException.class) + public void testGenerateVolumeKeyWithKek_ThrowsWhenKeyDisabled() throws KMSException { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(false); + kmsManager.generateVolumeKeyWithKek(key, 1L); + } + + @Test(expected = KMSException.class) + public void testGenerateVolumeKeyWithKek_ThrowsWhenWrongPurpose() throws KMSException { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.TLS_CERT); + kmsManager.generateVolumeKeyWithKek(key, 1L); + } + + @Test(expected = KMSException.class) + public void testGenerateVolumeKeyWithKek_ThrowsWhenNoActiveKekVersion() throws KMSException { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(key.getId()).thenReturn(1L); + when(kmsKekVersionDao.getActiveVersion(1L)).thenReturn(null); + + kmsManager.generateVolumeKeyWithKek(key, 1L); + } + + @Test(expected = KMSException.class) + public void testGenerateVolumeKeyWithKek_ThrowsWhenHsmProfileDisabled() throws KMSException { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(key.getId()).thenReturn(1L); + + KMSKekVersionVO activeVersion = mock(KMSKekVersionVO.class); + when(activeVersion.getHsmProfileId()).thenReturn(10L); + when(kmsKekVersionDao.getActiveVersion(1L)).thenReturn(activeVersion); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isEnabled()).thenReturn(false); + when(hsmProfileDao.findById(10L)).thenReturn(profile); + + kmsManager.generateVolumeKeyWithKek(key, 1L); + } + + @Test + public void testGenerateVolumeKeyWithKek_HappyPath() { + KMSKey key = mock(KMSKey.class); + when(key.isEnabled()).thenReturn(true); + when(key.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(key.getId()).thenReturn(1L); + + KMSKekVersionVO activeVersion = mock(KMSKekVersionVO.class); + when(activeVersion.getHsmProfileId()).thenReturn(10L); + when(activeVersion.getKekLabel()).thenReturn("kek-label"); + when(kmsKekVersionDao.getActiveVersion(1L)).thenReturn(activeVersion); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.isEnabled()).thenReturn(true); + when(profile.getProtocol()).thenReturn("database"); + when(hsmProfileDao.findById(10L)).thenReturn(profile); + + WrappedKey wrappedKeyResult = mock(WrappedKey.class); + when(wrappedKeyResult.getWrappedKeyMaterial()).thenReturn(new byte[]{1, 2, 3}); + when(wrappedKeyResult.getKekId()).thenReturn("kek-label"); + when(wrappedKeyResult.getPurpose()).thenReturn(KeyPurpose.VOLUME_ENCRYPTION); + when(wrappedKeyResult.getAlgorithm()).thenReturn("AES/GCM/NoPadding"); + when(wrappedKeyResult.getProviderName()).thenReturn("database"); + when(kmsProvider.generateAndWrapDek(any(KeyPurpose.class), anyString(), anyInt(), anyLong())) + .thenReturn(wrappedKeyResult); + doReturn(kmsProvider).when(kmsManager).getKMSProvider("database"); + + KMSWrappedKeyVO persisted = mock(KMSWrappedKeyVO.class); + when(persisted.getUuid()).thenReturn("wrapped-uuid"); + when(kmsWrappedKeyDao.persist(any(KMSWrappedKeyVO.class))).thenReturn(persisted); + + WrappedKey result = kmsManager.generateVolumeKeyWithKek(key, 1L); + + assertNotNull(result); + verify(kmsProvider).generateAndWrapDek(any(KeyPurpose.class), anyString(), anyInt(), anyLong()); + verify(kmsWrappedKeyDao).persist(any(KMSWrappedKeyVO.class)); + } + + @Test + public void testUpdateUserKMSKey_UpdatesName() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(1L); + when(key.getName()).thenReturn("old-name"); + + kmsManager.updateUserKMSKey(key, "new-name", null, null); + + verify(key).setName("new-name"); + verify(kmsKeyDao).update(1L, key); + } + + @Test + public void testUpdateUserKMSKey_NoUpdateWhenNothingChanges() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getName()).thenReturn("same-name"); + + kmsManager.updateUserKMSKey(key, "same-name", null, null); + + verify(kmsKeyDao, never()).update(anyLong(), any(KMSKeyVO.class)); + } + + @Test + public void testUpdateUserKMSKey_UpdatesDescription() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(1L); + when(key.getDescription()).thenReturn("old"); + + kmsManager.updateUserKMSKey(key, null, "new-desc", null); + + verify(key).setDescription("new-desc"); + verify(kmsKeyDao).update(1L, key); + } + + @Test + public void testUpdateUserKMSKey_TogglesEnabled() { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(1L); + when(key.isEnabled()).thenReturn(true); + + kmsManager.updateUserKMSKey(key, null, null, false); + + verify(key).setEnabled(false); + verify(kmsKeyDao).update(1L, key); + } + + @Test(expected = InvalidParameterValueException.class) + public void testDeleteUserKMSKey_ThrowsWhenWrappedKeysExist() throws KMSException { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(1L); + when(kmsWrappedKeyDao.countByKmsKeyId(1L)).thenReturn(3L); + + kmsManager.deleteUserKMSKey(key); + } + + @Test(expected = InvalidParameterValueException.class) + public void testDeleteUserKMSKey_ThrowsWhenVolumesExist() throws KMSException { + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(1L); + when(kmsWrappedKeyDao.countByKmsKeyId(1L)).thenReturn(0L); + when(volumeDao.existsWithKmsKey(1L)).thenReturn(true); + + kmsManager.deleteUserKMSKey(key); + } + + @Test + public void testDeleteUserKMSKey_DeletesKekFromProviderAndRemovesKey() throws KMSException { + Long keyId = 1L; + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(keyId); + when(kmsWrappedKeyDao.countByKmsKeyId(keyId)).thenReturn(0L); + when(volumeDao.existsWithKmsKey(keyId)).thenReturn(false); + + KMSKekVersionVO version = mock(KMSKekVersionVO.class); + when(version.getHsmProfileId()).thenReturn(10L); + when(version.getKekLabel()).thenReturn("kek-label"); + when(kmsKekVersionDao.listByKmsKeyId(keyId)).thenReturn(List.of(version)); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.getProtocol()).thenReturn("database"); + when(hsmProfileDao.findById(10L)).thenReturn(profile); + doReturn(kmsProvider).when(kmsManager).getKMSProvider("database"); + + kmsManager.deleteUserKMSKey(key); + + verify(kmsProvider).deleteKek("kek-label"); + verify(kmsKeyDao).remove(keyId); + } + + @Test + public void testDeleteUserKMSKey_ContinuesWhenKekDeletionFails() throws KMSException { + Long keyId = 1L; + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(keyId); + when(kmsWrappedKeyDao.countByKmsKeyId(keyId)).thenReturn(0L); + when(volumeDao.existsWithKmsKey(keyId)).thenReturn(false); + + KMSKekVersionVO version = mock(KMSKekVersionVO.class); + when(version.getHsmProfileId()).thenReturn(10L); + when(version.getKekLabel()).thenReturn("kek-label"); + when(kmsKekVersionDao.listByKmsKeyId(keyId)).thenReturn(List.of(version)); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.getProtocol()).thenReturn("database"); + when(hsmProfileDao.findById(10L)).thenReturn(profile); + doReturn(kmsProvider).when(kmsManager).getKMSProvider("database"); + doThrow(KMSException.kekOperationFailed("provider error")).when(kmsProvider).deleteKek(anyString()); + + kmsManager.deleteUserKMSKey(key); + + verify(kmsKeyDao).remove(keyId); + } + + @Test + public void testDeleteKMSKeysByAccountId_ReturnsFalseWhenAccountIdIsNull() { + assertFalse(kmsManager.deleteKMSKeysByAccountId(null)); + } + + @Test + public void testDeleteKMSKeysByAccountId_ReturnsTrueWhenNoKeys() { + when(kmsKeyDao.listByAccount(1L, null, null)).thenReturn(List.of()); + + assertTrue(kmsManager.deleteKMSKeysByAccountId(1L)); + } + + @Test + public void testDeleteKMSKeysByAccountId_DeletesAllKeysAndKeks() { + Long accountId = 1L; + + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(10L); + when(kmsKeyDao.listByAccount(accountId, null, null)).thenReturn(List.of(key)); + when(kmsKeyDao.remove(10L)).thenReturn(true); + + KMSKekVersionVO version = mock(KMSKekVersionVO.class); + when(version.getHsmProfileId()).thenReturn(20L); + when(version.getKekLabel()).thenReturn("kek-label"); + when(kmsKekVersionDao.listByKmsKeyId(10L)).thenReturn(List.of(version)); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.getProtocol()).thenReturn("database"); + when(hsmProfileDao.findById(20L)).thenReturn(profile); + doReturn(kmsProvider).when(kmsManager).getKMSProvider("database"); + + boolean result = kmsManager.deleteKMSKeysByAccountId(accountId); + + assertTrue(result); + verify(kmsProvider).deleteKek("kek-label"); + verify(kmsKeyDao).remove(10L); + } + + @Test + public void testDeleteKMSKeysByAccountId_ToleratesKekProviderFailure() { + Long accountId = 1L; + + KMSKeyVO key = mock(KMSKeyVO.class); + when(key.getId()).thenReturn(10L); + when(kmsKeyDao.listByAccount(accountId, null, null)).thenReturn(List.of(key)); + when(kmsKeyDao.remove(10L)).thenReturn(true); + + KMSKekVersionVO version = mock(KMSKekVersionVO.class); + when(version.getHsmProfileId()).thenReturn(20L); + when(version.getKekLabel()).thenReturn("kek-label"); + when(kmsKekVersionDao.listByKmsKeyId(10L)).thenReturn(List.of(version)); + + HSMProfileVO profile = mock(HSMProfileVO.class); + when(profile.getProtocol()).thenReturn("database"); + when(hsmProfileDao.findById(20L)).thenReturn(profile); + doReturn(kmsProvider).when(kmsManager).getKMSProvider("database"); + doThrow(new RuntimeException("provider unavailable")).when(kmsProvider).deleteKek(anyString()); + + boolean result = kmsManager.deleteKMSKeysByAccountId(accountId); + + assertTrue(result); + verify(kmsKeyDao).remove(10L); + } +} diff --git a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyRotationTest.java b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyRotationTest.java index 9fe809fd020..bf513c5fd3e 100644 --- a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyRotationTest.java +++ b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyRotationTest.java @@ -352,9 +352,9 @@ public class KMSManagerImplKeyRotationTest { // Verify current profile was used (not a different one) verify(kmsProvider).createKek(any(KeyPurpose.class), anyString(), eq(256), eq(currentProfileId)); + verify(kmsKeyDao).update(kmsKeyId, kmsKey); // Verify KMS key was not updated (same profile) verify(kmsKey, never()).setHsmProfileId(currentProfileId); - verify(kmsKeyDao, never()).update(kmsKeyId, kmsKey); } } diff --git a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplRetryTest.java b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplRetryTest.java index 11e694a6441..30279434485 100644 --- a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplRetryTest.java +++ b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplRetryTest.java @@ -35,7 +35,7 @@ import static org.mockito.Mockito.doReturn; /** * Unit tests for KMSManagerImpl's retryOperation() logic, covering * timeout enforcement, retry-on-transient-failure, and non-retryable fast-fail. - * + *

* Config values (retry count, delay, timeout) are spied on so tests remain * fast without needing a full management-server config context. */ @@ -46,7 +46,9 @@ public class KMSManagerImplRetryTest { @InjectMocks private KMSManagerImpl kmsManager; - /** Configure the spy to use a 1-second timeout, the given retry count, and no delay. */ + /** + * Configure the spy to use a 1-second timeout, the given retry count, and no delay. + */ private void useShortConfig(int retries) { doReturn(1).when(kmsManager).getOperationTimeoutSec(); doReturn(retries).when(kmsManager).getRetryCount(); diff --git a/ui/src/views/compute/DeployVM.vue b/ui/src/views/compute/DeployVM.vue index 0b2f778c539..11632853155 100644 --- a/ui/src/views/compute/DeployVM.vue +++ b/ui/src/views/compute/DeployVM.vue @@ -2028,7 +2028,8 @@ export default { zoneid: this.zoneId, account: this.owner.account, domainid: this.owner.domainid, - projectid: this.owner.projectid + projectid: this.owner.projectid, + purpose: 'volume' }).then(response => { const kmskeyMap = response.listkmskeysresponse.kmskey || [] if (kmskeyMap.length > 0) {