Skip to content

Commit 8ea09bb

Browse files
vishesh92vishesh92
committed
fixups
1 parent bfcacbf commit 8ea09bb

File tree

4 files changed

+55
-36
lines changed

4 files changed

+55
-36
lines changed

‎engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKekVersionVO.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,21 +75,26 @@ public enum Status {
7575
private Status status;
7676
@Column(name = "hsm_profile_id")
7777
private Long hsmProfileId;
78-
@Column(name = "hsm_key_label")
79-
private String hsmKeyLabel;
8078
@Column(name = GenericDao.CREATED_COLUMN, nullable = false)
8179
@Temporal(TemporalType.TIMESTAMP)
8280
private Date created;
8381
@Column(name = GenericDao.REMOVED_COLUMN)
8482
@Temporal(TemporalType.TIMESTAMP)
8583
private Date removed;
8684

87-
public KMSKekVersionVO(Long kmsKeyId, Integer versionNumber, String kekLabel, Status status) {
85+
public KMSKekVersionVO(Long kmsKeyId, Integer versionNumber, String kekLabel) {
8886
this();
8987
this.kmsKeyId = kmsKeyId;
9088
this.versionNumber = versionNumber;
9189
this.kekLabel = kekLabel;
92-
this.status = status;
90+
this.status = Status.Active;
91+
}
92+
93+
public KMSKekVersionVO(Long kmsKeyId, String kekLabel) {
94+
this();
95+
this.kmsKeyId = kmsKeyId;
96+
this.kekLabel = kekLabel;
97+
this.status = Status.Active;
9398
}
9499

95100
public KMSKekVersionVO() {
@@ -154,13 +159,6 @@ public void setHsmProfileId(Long hsmProfileId) {
154159
this.hsmProfileId = hsmProfileId;
155160
}
156161

157-
public String getHsmKeyLabel() {
158-
return hsmKeyLabel;
159-
}
160-
161-
public void setHsmKeyLabel(String hsmKeyLabel) {
162-
this.hsmKeyLabel = hsmKeyLabel;
163-
}
164162

165163
public Date getCreated() {
166164
return created;

‎engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ CREATE TABLE IF NOT EXISTS `cloud`.`kms_kek_versions` (
134134
`kek_label` VARCHAR(255) NOT NULL COMMENT 'Provider-specific KEK label/ID for this version',
135135
`status` VARCHAR(32) NOT NULL DEFAULT 'Active' COMMENT 'Active, Previous, Archived',
136136
`hsm_profile_id` BIGINT UNSIGNED COMMENT 'HSM profile where this KEK version is stored',
137-
`hsm_key_label` VARCHAR(255) COMMENT 'Optional HSM-specific key label/alias',
138137
`created` DATETIME NOT NULL COMMENT 'Creation timestamp',
139138
`removed` DATETIME COMMENT 'Removal timestamp for soft delete',
140139
PRIMARY KEY (`id`),

‎plugins/kms/pkcs11/src/main/java/org/apache/cloudstack/kms/provider/pkcs11/PKCS11HSMProvider.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.apache.cloudstack.framework.kms.WrappedKey;
2727
import org.apache.cloudstack.kms.HSMProfileDetailsVO;
2828
import org.apache.cloudstack.kms.KMSKekVersionVO;
29-
import org.apache.cloudstack.kms.dao.HSMProfileDao;
3029
import org.apache.cloudstack.kms.dao.HSMProfileDetailsDao;
3130
import org.apache.cloudstack.kms.dao.KMSKekVersionDao;
3231
import org.apache.commons.lang3.StringUtils;
@@ -75,7 +74,7 @@
7574
public class PKCS11HSMProvider extends AdapterBase implements KMSProvider {
7675
private static final Logger logger = LogManager.getLogger(PKCS11HSMProvider.class);
7776
private static final String PROVIDER_NAME = "pkcs11";
78-
// Security note (#7): AES-CBC provides confidentiality but not authenticity (no
77+
// Security note: AES-CBC provides confidentiality but not authenticity (no
7978
// HMAC).
8079
// While AES-GCM is preferred, SunPKCS11 support for GCM is often buggy or
8180
// missing
@@ -89,8 +88,6 @@ public class PKCS11HSMProvider extends AdapterBase implements KMSProvider {
8988
private static final int[] VALID_KEY_SIZES = {128, 192, 256};
9089
private final Map<Long, HSMSessionPool> sessionPools = new ConcurrentHashMap<>();
9190
@Inject
92-
private HSMProfileDao hsmProfileDao;
93-
@Inject
9491
private HSMProfileDetailsDao hsmProfileDetailsDao;
9592
@Inject
9693
private KMSKekVersionDao kmsKekVersionDao;
@@ -641,6 +638,17 @@ private String buildSunPKCS11Config(Map<String, String> config, String nameSuffi
641638
throw KMSException.invalidParameter("One of 'slot', 'slot_list_index', or 'token_label' is required");
642639
}
643640

641+
// Explicitly configure SunPKCS11 to generate AES keys as Data Encryption Keys.
642+
// Strict HSMs (like Thales Luna in FIPS mode) forbid a key from having both
643+
// CKA_WRAP and CKA_ENCRYPT attributes. Because CloudStack uses Cipher.ENCRYPT_MODE
644+
// (which maps to C_Encrypt) to protect the DEK, the KEK must have CKA_ENCRYPT=true.
645+
configBuilder.append("\nattributes(generate, CKO_SECRET_KEY, CKK_AES) = {\n");
646+
configBuilder.append(" CKA_ENCRYPT = true\n");
647+
configBuilder.append(" CKA_DECRYPT = true\n");
648+
configBuilder.append(" CKA_WRAP = false\n");
649+
configBuilder.append(" CKA_UNWRAP = false\n");
650+
configBuilder.append("}\n");
651+
644652
return configBuilder.toString();
645653
}
646654

‎server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java‎

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import com.cloud.utils.db.SearchCriteria;
4646
import com.cloud.utils.db.Transaction;
4747
import com.cloud.utils.db.TransactionCallbackWithException;
48-
import com.cloud.utils.db.TransactionStatus;
4948
import com.cloud.utils.exception.CloudRuntimeException;
5049
import org.apache.cloudstack.api.ApiCommandResourceType;
5150
import org.apache.cloudstack.api.command.admin.kms.MigrateVolumesToKMSCmd;
@@ -453,8 +452,7 @@ KMSKey createUserKMSKey(Long accountId, Long domainId, Long zoneId,
453452
kmsKey.setHsmProfileId(finalProfileId);
454453
kmsKey = kmsKeyDao.persist(kmsKey);
455454

456-
KMSKekVersionVO initialVersion = new KMSKekVersionVO(kmsKey.getId(), 1, providerKekLabel,
457-
KMSKekVersionVO.Status.Active);
455+
KMSKekVersionVO initialVersion = new KMSKekVersionVO(kmsKey.getId(), 1, providerKekLabel);
458456
initialVersion.setHsmProfileId(finalProfileId);
459457
initialVersion = kmsKekVersionDao.persist(initialVersion);
460458

@@ -578,22 +576,39 @@ public SuccessResponse deleteKMSKey(DeleteKMSKeyCmd cmd) throws KMSException {
578576

579577
KMSKeyVO key = findKMSKeyAndCheckAccess(cmd.getId(), caller);
580578

581-
deleteUserKMSKey(key, caller);
579+
deleteUserKMSKey(key);
582580
return new SuccessResponse();
583581
}
584582

585-
private void deleteUserKMSKey(KMSKeyVO key, Account caller) throws KMSException {
583+
private void deleteUserKMSKey(KMSKeyVO key) throws KMSException {
586584
long wrappedKeyCount = kmsWrappedKeyDao.countByKmsKeyId(key.getId());
587585
if (wrappedKeyCount > 0) {
588586
throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " + wrappedKeyCount +
589587
" wrapped key(s) still reference this key");
590588
}
591589

592-
kmsKeyDao.remove(key.getId());
593590
if (volumeDao.existsWithKmsKey(key.getId())) {
594591
throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " +
595592
"There are Volumes which still reference this key");
596593
}
594+
595+
List<KMSKekVersionVO> kekVersions = kmsKekVersionDao.listByKmsKeyId(key.getId());
596+
for (KMSKekVersionVO kekVersion : kekVersions) {
597+
try {
598+
HSMProfileVO hsmProfile = hsmProfileDao.findById(kekVersion.getHsmProfileId());
599+
if (hsmProfile != null) {
600+
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
601+
provider.deleteKek(kekVersion.getKekLabel());
602+
logger.info("Deleted KEK {} (v{}) from provider {}",
603+
kekVersion.getKekLabel(), kekVersion.getVersionNumber(), provider.getProviderName());
604+
}
605+
} catch (Exception e) {
606+
logger.warn("Failed to delete KEK {} (v{}) from provider during KMS key deletion: {}",
607+
kekVersion.getKekLabel(), kekVersion.getVersionNumber(), e.getMessage());
608+
}
609+
}
610+
611+
kmsKeyDao.remove(key.getId());
597612
logger.info("Deleted KMS key {}", key);
598613
}
599614

@@ -663,13 +678,13 @@ String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int ke
663678
try {
664679
logger.info("Starting KEK rotation from {} to {} for kms key {}", oldKekLabel, newKekLabel, kmsKey);
665680

666-
final KMSKekVersionVO newVersionEntity = new KMSKekVersionVO();
667681
if (StringUtils.isEmpty(newKekLabel)) {
668682
List<KMSKekVersionVO> existingVersions = kmsKekVersionDao.listByKmsKeyId(kmsKey.getId());
669683
int nextVersion = existingVersions.stream().mapToInt(KMSKekVersionVO::getVersionNumber).max().orElse(0) + 1;
670684
newKekLabel = kmsKey.getPurpose().generateKekLabel(kmsKey.getDomainId(), kmsKey.getAccountId(),
671685
kmsKey.getUuid(), nextVersion);
672686
}
687+
final KMSKekVersionVO newVersionEntity = new KMSKekVersionVO(kmsKey.getId(), newKekLabel);
673688

674689
String finalNewKekLabel = newKekLabel;
675690
Long newProfileId = newHSMProfile.getId();
@@ -679,20 +694,19 @@ String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int ke
679694

680695
try {
681696
KMSKekVersionVO newVersion = Transaction
682-
.execute(new TransactionCallbackWithException<KMSKekVersionVO, KMSException>() {
683-
@Override
684-
public KMSKekVersionVO doInTransaction(TransactionStatus status) throws KMSException {
685-
newVersionEntity.setKmsKeyId(kmsKey.getId());
686-
newVersionEntity.setHsmProfileId(newProfileId);
687-
KMSKekVersionVO version = createKekVersion(newVersionEntity);
688-
689-
if (!newProfileId.equals(kmsKey.getHsmProfileId())) {
690-
kmsKey.setHsmProfileId(newProfileId);
691-
kmsKeyDao.update(kmsKey.getId(), kmsKey);
692-
logger.info("Updated KMS key {} to use HSM profile {}", kmsKey, finalHSMProfile);
693-
}
694-
return version;
697+
.execute((TransactionCallbackWithException<KMSKekVersionVO, KMSException>) status -> {
698+
newVersionEntity.setKmsKeyId(kmsKey.getId());
699+
newVersionEntity.setHsmProfileId(newProfileId);
700+
newVersionEntity.setKekLabel(finalNewKekLabel);
701+
KMSKekVersionVO version = createKekVersion(newVersionEntity);
702+
703+
if (!newProfileId.equals(kmsKey.getHsmProfileId())) {
704+
kmsKey.setHsmProfileId(newProfileId);
705+
logger.info("Updated KMS key {} to use HSM profile {}", kmsKey, finalHSMProfile);
695706
}
707+
kmsKey.setKekLabel(finalNewKekLabel);
708+
kmsKeyDao.update(kmsKey.getId(), kmsKey);
709+
return version;
696710
});
697711

698712
logger.info("KEK rotation: KMS key {} now has {} versions (active: v{}, previous: v{})",

0 commit comments

Comments
 (0)