Skip to content

Commit 1fce0b2

Browse files
vishesh92vishesh92
committed
fixups
1 parent bfcacbf commit 1fce0b2

File tree

16 files changed

+885
-206
lines changed

16 files changed

+885
-206
lines changed

‎api/src/main/java/org/apache/cloudstack/kms/KMSManager.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,17 +118,6 @@ public interface KMSManager extends Manager, Configurable {
118118
*/
119119
KMSProvider getKMSProvider(String name);
120120

121-
/**
122-
* Unwrap a DEK from a wrapped key
123-
* SECURITY: Caller must zeroize returned byte array after use!
124-
*
125-
* @param wrappedKey the wrapped key from database
126-
* @param zoneId the zone ID
127-
* @return plaintext DEK (caller must zeroize!)
128-
* @throws KMSException if unwrap fails
129-
*/
130-
byte[] unwrapVolumeKey(WrappedKey wrappedKey, Long zoneId) throws KMSException;
131-
132121
/**
133122
* Check if caller has permission to use a KMS key
134123
*

‎engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ public VolumeDaoImpl() {
404404
AllFieldsSearch.and("passphraseId", AllFieldsSearch.entity().getPassphraseId(), Op.EQ);
405405
AllFieldsSearch.and("iScsiName", AllFieldsSearch.entity().get_iScsiName(), Op.EQ);
406406
AllFieldsSearch.and("path", AllFieldsSearch.entity().getPath(), Op.EQ);
407+
AllFieldsSearch.and("kmsKeyId", AllFieldsSearch.entity().getKmsKeyId(), Op.EQ);
407408
AllFieldsSearch.done();
408409

409410
RootDiskStateSearch = createSearchBuilder();

‎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/java/org/apache/cloudstack/kms/dao/KMSWrappedKeyDaoImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ public KMSWrappedKeyDaoImpl() {
3838
allFieldSearch.and("kmsKeyId", allFieldSearch.entity().getKmsKeyId(), SearchCriteria.Op.EQ);
3939
allFieldSearch.and("kekVersionId", allFieldSearch.entity().getKekVersionId(), SearchCriteria.Op.EQ);
4040
allFieldSearch.and("zoneId", allFieldSearch.entity().getZoneId(), SearchCriteria.Op.EQ);
41-
allFieldSearch.and("kmsKeyId", allFieldSearch.entity().getKmsKeyId(), SearchCriteria.Op.EQ);
4241
allFieldSearch.done();
4342
}
4443

‎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`),

‎engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.cloud.utils.db.TransactionCallbackNoReturn;
3030
import com.cloud.utils.db.TransactionStatus;
3131
import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
32+
import org.apache.cloudstack.framework.kms.KMSException;
3233
import org.apache.cloudstack.secret.dao.PassphraseDao;
3334
import org.apache.cloudstack.secret.PassphraseVO;
3435
import com.cloud.service.dao.ServiceOfferingDetailsDao;
@@ -971,8 +972,8 @@ public byte[] getPassphrase() {
971972
java.util.Arrays.fill(dekBytes, (byte) 0);
972973
// Return UTF-8 bytes of the base64 string
973974
return base64Dek.getBytes(java.nio.charset.StandardCharsets.UTF_8);
974-
} catch (org.apache.cloudstack.framework.kms.KMSException e) {
975-
logger.error("Failed to unwrap KMS key for volume {}: {}", volumeVO.getId(), e.getMessage());
975+
} catch (KMSException e) {
976+
logger.error("Failed to unwrap KMS key for volume {}: {}", volumeVO, e.getMessage(), e);
976977
return new byte[0];
977978
}
978979
}

‎plugins/kms/database/src/main/java/org/apache/cloudstack/kms/provider/DatabaseKMSProvider.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.cloud.utils.component.AdapterBase;
2121
import com.cloud.utils.crypt.DBEncryptionUtil;
2222
import com.google.crypto.tink.subtle.AesGcmJce;
23+
2324
import org.apache.cloudstack.framework.config.ConfigKey;
2425
import org.apache.cloudstack.framework.kms.KMSException;
2526
import org.apache.cloudstack.framework.kms.KMSProvider;

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

Lines changed: 93 additions & 86 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;
@@ -125,6 +122,85 @@ public void deleteKek(String kekId) throws KMSException {
125122
});
126123
}
127124

125+
Long resolveProfileId(String kekLabel) throws KMSException {
126+
KMSKekVersionVO version = kmsKekVersionDao.findByKekLabel(kekLabel);
127+
if (version != null && version.getHsmProfileId() != null) {
128+
return version.getHsmProfileId();
129+
}
130+
throw new KMSException(KMSException.ErrorType.KEK_NOT_FOUND,
131+
"Could not resolve HSM profile for KEK: " + kekLabel);
132+
}
133+
134+
/**
135+
* Validates HSM profile configuration for PKCS#11 provider.
136+
*
137+
* <p>
138+
* Validates:
139+
* <ul>
140+
* <li>{@code library}: Required, should point to PKCS#11 library</li>
141+
* <li>{@code slot}, {@code slot_list_index}, or {@code token_label}: At least
142+
* one required</li>
143+
* <li>{@code pin}: Required for HSM authentication</li>
144+
* <li>{@code max_sessions}: Optional, must be positive integer if provided</li>
145+
* </ul>
146+
*
147+
* @param config Configuration map from HSM profile details
148+
* @throws KMSException with {@code INVALID_PARAMETER} if validation fails
149+
*/
150+
@Override
151+
public void validateProfileConfig(Map<String, String> config) throws KMSException {
152+
String libraryPath = config.get("library");
153+
if (StringUtils.isBlank(libraryPath)) {
154+
throw KMSException.invalidParameter("library is required for PKCS#11 HSM profile");
155+
}
156+
157+
String slot = config.get("slot");
158+
String slotListIndex = config.get("slot_list_index");
159+
String tokenLabel = config.get("token_label");
160+
if (StringUtils.isAllBlank(slot, slotListIndex, tokenLabel)) {
161+
throw KMSException.invalidParameter(
162+
"One of 'slot', 'slot_list_index', or 'token_label' is required for PKCS#11 HSM profile");
163+
}
164+
165+
if (StringUtils.isNotBlank(slot)) {
166+
try {
167+
Integer.parseInt(slot);
168+
} catch (NumberFormatException e) {
169+
throw KMSException.invalidParameter("slot must be a valid integer: " + slot);
170+
}
171+
}
172+
173+
if (StringUtils.isNotBlank(slotListIndex)) {
174+
try {
175+
int idx = Integer.parseInt(slotListIndex);
176+
if (idx < 0) {
177+
throw KMSException.invalidParameter("slot_list_index must be a non-negative integer");
178+
}
179+
} catch (NumberFormatException e) {
180+
throw KMSException.invalidParameter("slot_list_index must be a valid integer: " + slotListIndex);
181+
}
182+
}
183+
184+
File libraryFile = new File(libraryPath);
185+
if (!libraryFile.exists() && !libraryFile.isAbsolute()) {
186+
// The HSM library might be in the system library path
187+
logger.debug("Library path {} does not exist as absolute path, will rely on system library path",
188+
libraryPath);
189+
}
190+
191+
String max_sessions = config.get("max_sessions");
192+
if (StringUtils.isNotBlank(max_sessions)) {
193+
try {
194+
int idx = Integer.parseInt(max_sessions);
195+
if (idx <= 0) {
196+
throw KMSException.invalidParameter("max_sessions must be greater than 0");
197+
}
198+
} catch (NumberFormatException e) {
199+
throw KMSException.invalidParameter("max_sessions must be a valid integer: " + max_sessions);
200+
}
201+
}
202+
}
203+
128204
@Override
129205
public boolean isKekAvailable(String kekId) throws KMSException {
130206
try {
@@ -245,15 +321,6 @@ public void invalidateProfileCache(Long profileId) {
245321
logger.info("Invalidated HSM session pool for profile {}", profileId);
246322
}
247323

248-
Long resolveProfileId(String kekLabel) throws KMSException {
249-
KMSKekVersionVO version = kmsKekVersionDao.findByKekLabel(kekLabel);
250-
if (version != null && version.getHsmProfileId() != null) {
251-
return version.getHsmProfileId();
252-
}
253-
throw new KMSException(KMSException.ErrorType.KEK_NOT_FOUND,
254-
"Could not resolve HSM profile for KEK: " + kekLabel);
255-
}
256-
257324
/**
258325
* Executes an operation with a session from the pool, handling acquisition and release.
259326
*
@@ -295,82 +362,11 @@ Map<String, String> loadProfileConfig(Long profileId) {
295362
return config;
296363
}
297364

298-
/**
299-
* Validates HSM profile configuration for PKCS#11 provider.
300-
*
301-
* <p>
302-
* Validates:
303-
* <ul>
304-
* <li>{@code library}: Required, should point to PKCS#11 library</li>
305-
* <li>{@code slot}, {@code slot_list_index}, or {@code token_label}: At least
306-
* one required</li>
307-
* <li>{@code pin}: Required for HSM authentication</li>
308-
* <li>{@code max_sessions}: Optional, must be positive integer if provided</li>
309-
* </ul>
310-
*
311-
* @param config Configuration map from HSM profile details
312-
* @throws KMSException with {@code INVALID_PARAMETER} if validation fails
313-
*/
314-
@Override
315-
public void validateProfileConfig(Map<String, String> config) throws KMSException {
316-
String libraryPath = config.get("library");
317-
if (StringUtils.isBlank(libraryPath)) {
318-
throw KMSException.invalidParameter("library is required for PKCS#11 HSM profile");
319-
}
320-
321-
String slot = config.get("slot");
322-
String slotListIndex = config.get("slot_list_index");
323-
String tokenLabel = config.get("token_label");
324-
if (StringUtils.isAllBlank(slot, slotListIndex, tokenLabel)) {
325-
throw KMSException.invalidParameter(
326-
"One of 'slot', 'slot_list_index', or 'token_label' is required for PKCS#11 HSM profile");
327-
}
328-
329-
if (StringUtils.isNotBlank(slot)) {
330-
try {
331-
Integer.parseInt(slot);
332-
} catch (NumberFormatException e) {
333-
throw KMSException.invalidParameter("slot must be a valid integer: " + slot);
334-
}
335-
}
336-
337-
if (StringUtils.isNotBlank(slotListIndex)) {
338-
try {
339-
int idx = Integer.parseInt(slotListIndex);
340-
if (idx < 0) {
341-
throw KMSException.invalidParameter("slot_list_index must be a non-negative integer");
342-
}
343-
} catch (NumberFormatException e) {
344-
throw KMSException.invalidParameter("slot_list_index must be a valid integer: " + slotListIndex);
345-
}
346-
}
347-
348-
File libraryFile = new File(libraryPath);
349-
if (!libraryFile.exists() && !libraryFile.isAbsolute()) {
350-
// The HSM library might be in the system library path
351-
logger.debug("Library path {} does not exist as absolute path, will rely on system library path",
352-
libraryPath);
353-
}
354-
355-
String max_sessions = config.get("max_sessions");
356-
if (StringUtils.isNotBlank(max_sessions)) {
357-
try {
358-
int idx = Integer.parseInt(max_sessions);
359-
if (idx <= 0) {
360-
throw KMSException.invalidParameter("max_sessions must be greater than 0");
361-
}
362-
} catch (NumberFormatException e) {
363-
throw KMSException.invalidParameter("max_sessions must be a valid integer: " + max_sessions);
364-
}
365-
}
366-
}
367-
368365
boolean isSensitiveKey(String key) {
369366
return KMSProvider.isSensitiveKey(key);
370367
}
371368

372369

373-
374370
@Override
375371
public String getConfigComponentName() {
376372
return PKCS11HSMProvider.class.getSimpleName();
@@ -641,6 +637,17 @@ private String buildSunPKCS11Config(Map<String, String> config, String nameSuffi
641637
throw KMSException.invalidParameter("One of 'slot', 'slot_list_index', or 'token_label' is required");
642638
}
643639

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

@@ -823,9 +830,9 @@ String generateKey(String label, int keyBits, KeyPurpose purpose) throws KMSExce
823830

824831
} catch (KeyStoreException e) {
825832
if (e.getMessage() != null
826-
&& e.getMessage().contains("found multiple secret keys sharing same CKA_LABEL")) {
833+
&& e.getMessage().contains("found multiple secret keys sharing same CKA_LABEL")) {
827834
logger.warn("Multiple duplicate keys found with label '{}' in HSM. Reusing the existing key. " +
828-
"Please purge duplicate keys manually if possible.", label);
835+
"Please purge duplicate keys manually if possible.", label);
829836
return label;
830837
}
831838
handlePKCS11Exception(e, "Failed to store key in HSM KeyStore");

‎server/src/main/java/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,6 @@
208208
import org.apache.cloudstack.framework.jobs.AsyncJob;
209209
import org.apache.cloudstack.framework.jobs.AsyncJobManager;
210210
import org.apache.cloudstack.gui.theme.GuiThemeJoin;
211-
import org.apache.cloudstack.kms.dao.HSMProfileDao;
212211
import org.apache.cloudstack.management.ManagementServerHost;
213212
import org.apache.cloudstack.network.BgpPeerVO;
214213
import org.apache.cloudstack.network.RoutedIpv4Manager;
@@ -520,8 +519,6 @@ public class ApiResponseHelper implements ResponseGenerator {
520519
private ASNumberRangeDao asNumberRangeDao;
521520
@Inject
522521
private ASNumberDao asNumberDao;
523-
@Inject
524-
private HSMProfileDao hsmProfileDao;
525522

526523
@Inject
527524
ObjectStoreDao _objectStoreDao;

‎server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Licensed to the Apache Software Foundation (ASF) under one
1+
// Licensed to the Apache Software Foundation (ASF) under one
22
// or more contributor license agreements. See the NOTICE file
33
// distributed with this work for additional information
44
// regarding copyright ownership. The ASF licenses this file
@@ -32,7 +32,6 @@
3232
import org.apache.cloudstack.kms.KMSKekVersionVO;
3333
import org.apache.cloudstack.kms.KMSWrappedKeyVO;
3434
import org.apache.cloudstack.kms.dao.KMSKekVersionDao;
35-
import org.apache.cloudstack.kms.dao.KMSKeyDao;
3635
import org.apache.cloudstack.kms.dao.KMSWrappedKeyDao;
3736
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
3837
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -63,8 +62,6 @@ public class VolumeJoinDaoImpl extends GenericDaoBaseWithTagInformation<VolumeJo
6362
@Inject
6463
private PrimaryDataStoreDao primaryDataStoreDao;
6564
@Inject
66-
private KMSKeyDao kmsKeyDao;
67-
@Inject
6865
private KMSWrappedKeyDao kmsWrappedKeyDao;
6966
@Inject
7067
private KMSKekVersionDao kmsKekVersionDao;

0 commit comments

Comments
 (0)