Skip to content

Commit 0277e85

Browse files
vishesh92vishesh92
committed
fixups
1 parent 2da2477 commit 0277e85

File tree

8 files changed

+48
-69
lines changed

8 files changed

+48
-69
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/admin/kms/RotateKMSKeyCmd.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.cloudstack.api.response.AsyncJobResponse;
2929
import org.apache.cloudstack.api.response.HSMProfileResponse;
3030
import org.apache.cloudstack.api.response.KMSKeyResponse;
31+
import org.apache.cloudstack.api.response.SuccessResponse;
3132
import org.apache.cloudstack.framework.kms.KMSException;
3233
import org.apache.cloudstack.kms.KMSKey;
3334
import org.apache.cloudstack.kms.KMSManager;
@@ -81,6 +82,9 @@ public String getHsmProfile() {
8182
public void execute() {
8283
try {
8384
kmsManager.rotateKMSKey(this);
85+
SuccessResponse response = new SuccessResponse();
86+
response.setResponseName(getCommandName());
87+
setResponseObject(response);
8488
} catch (KMSException e) {
8589
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
8690
"Failed to rotate KMS key: " + e.getMessage());

‎api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/AddHSMProfileCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.cloud.exception.NetworkRuleConflictException;
2323
import com.cloud.exception.ResourceAllocationException;
2424
import com.cloud.exception.ResourceUnavailableException;
25+
import com.cloud.utils.StringUtils;
2526
import org.apache.cloudstack.api.APICommand;
2627
import org.apache.cloudstack.api.ApiConstants;
2728
import org.apache.cloudstack.api.ApiErrorCode;
@@ -37,7 +38,6 @@
3738
import org.apache.cloudstack.kms.HSMProfile;
3839
import org.apache.cloudstack.kms.KMSManager;
3940
import org.apache.commons.collections.MapUtils;
40-
import org.apache.commons.lang.StringUtils;
4141

4242
import javax.inject.Inject;
4343
import java.util.Collection;

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,27 @@
1717

1818
package org.apache.cloudstack.kms;
1919

20-
import java.util.Date;
21-
2220
import org.apache.cloudstack.api.Identity;
2321
import org.apache.cloudstack.api.InternalIdentity;
2422

23+
import java.util.Date;
24+
2525
public interface HSMProfile extends Identity, InternalIdentity {
2626
String getName();
27-
27+
2828
String getProtocol();
29-
29+
3030
Long getAccountId();
31-
31+
3232
Long getDomainId();
33-
33+
3434
Long getZoneId();
35-
35+
3636
String getVendorName();
37-
37+
3838
boolean isEnabled();
39-
39+
4040
Date getCreated();
41-
41+
4242
Date getRemoved();
4343
}

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,6 @@ public interface KMSManager extends Manager, Configurable {
161161
*/
162162
KMSProvider getKMSProvider(String name);
163163

164-
/**
165-
* Get the configured provider for a zone
166-
*
167-
* @param zoneId the zone ID (null for global default)
168-
* @return the configured provider
169-
* @throws KMSException if no provider configured
170-
*/
171-
KMSProvider getKMSProviderForZone(Long zoneId) throws KMSException;
172-
173164
/**
174165
* Check if KMS is enabled for a zone
175166
*

‎plugins/kms/pkcs11/src/test/java/org/apache/cloudstack/kms/provider/pkcs11/PKCS11HSMProviderTest.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public void testResolveProfileId_ThrowsExceptionWhenProfileIdNull() throws KMSEx
120120
public void testLoadProfileConfig_DecryptsSensitiveValues() {
121121
// Setup: Profile details with encrypted pin
122122
HSMProfileDetailsVO detail1 = mock(HSMProfileDetailsVO.class);
123-
when(detail1.getName()).thenReturn("library_path");
123+
when(detail1.getName()).thenReturn("library");
124124
when(detail1.getValue()).thenReturn("/path/to/lib.so");
125125

126126
HSMProfileDetailsVO detail2 = mock(HSMProfileDetailsVO.class);
@@ -140,7 +140,7 @@ public void testLoadProfileConfig_DecryptsSensitiveValues() {
140140
// Verify
141141
assertNotNull("Config should not be null", config);
142142
assertEquals(3, config.size());
143-
assertEquals("/path/to/lib.so", config.get("library_path"));
143+
assertEquals("/path/to/lib.so", config.get("library"));
144144
// Note: In real code, DBEncryptionUtil.decrypt would be called
145145
// Here we just verify the structure is correct
146146
assertTrue("Config should contain pin", config.containsKey("pin"));
@@ -184,7 +184,7 @@ public void testIsSensitiveKey_IdentifiesSensitiveKeys() {
184184
@Test
185185
public void testIsSensitiveKey_IdentifiesNonSensitiveKeys() {
186186
// Test
187-
assertFalse(provider.isSensitiveKey("library_path"));
187+
assertFalse(provider.isSensitiveKey("library"));
188188
assertFalse(provider.isSensitiveKey("slot_id"));
189189
assertFalse(provider.isSensitiveKey("endpoint"));
190190
assertFalse(provider.isSensitiveKey("max_sessions"));
@@ -232,7 +232,7 @@ public void testCreateKek_RequiresProfileId() throws KMSException {
232232
public void testLoadProfileConfig_CachesConfiguration() {
233233
// Setup
234234
HSMProfileDetailsVO detail = mock(HSMProfileDetailsVO.class);
235-
when(detail.getName()).thenReturn("library_path");
235+
when(detail.getName()).thenReturn("library");
236236
when(detail.getValue()).thenReturn("/path/to/lib.so");
237237
when(hsmProfileDetailsDao.listByProfileId(testProfileId)).thenReturn(Arrays.asList(detail));
238238

@@ -251,7 +251,7 @@ public void testLoadProfileConfig_CachesConfiguration() {
251251
public void testGetSessionPool_CreatesPoolForNewProfile() {
252252
// Setup
253253
HSMProfileDetailsVO detail = mock(HSMProfileDetailsVO.class);
254-
when(detail.getName()).thenReturn("library_path");
254+
when(detail.getName()).thenReturn("library");
255255
when(detail.getValue()).thenReturn("/path/to/lib.so");
256256
when(hsmProfileDetailsDao.listByProfileId(testProfileId)).thenReturn(Arrays.asList(detail));
257257

@@ -270,7 +270,7 @@ public void testGetSessionPool_CreatesPoolForNewProfile() {
270270
public void testGetSessionPool_ReusesPoolForSameProfile() {
271271
// Setup
272272
HSMProfileDetailsVO detail = mock(HSMProfileDetailsVO.class);
273-
when(detail.getName()).thenReturn("library_path");
273+
when(detail.getName()).thenReturn("library");
274274
when(detail.getValue()).thenReturn("/path/to/lib.so");
275275
when(hsmProfileDetailsDao.listByProfileId(testProfileId)).thenReturn(Arrays.asList(detail));
276276

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

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,6 @@ public KMSProvider getKMSProvider(String name) {
129129
return provider;
130130
}
131131

132-
@Override
133-
public KMSProvider getKMSProviderForZone(Long zoneId) throws KMSException {
134-
// Default to database provider for backward compatibility
135-
// HSM-based keys will use provider from HSM profile's protocol field
136-
return getKMSProvider("database");
137-
}
138-
139132
@Override
140133
public boolean isKmsEnabled(Long zoneId) {
141134
if (zoneId == null) {
@@ -147,8 +140,7 @@ public boolean isKmsEnabled(Long zoneId) {
147140
/**
148141
* Internal method to rotate a KEK (create new version and update KMS key state)
149142
*/
150-
String rotateKek(KMSKeyVO kmsKey, String oldKekLabel,
151-
String newKekLabel, int keyBits, HSMProfileVO newHSMProfile) throws KMSException {
143+
String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int keyBits, HSMProfileVO newHSMProfile) throws KMSException {
152144

153145
validateKmsEnabled(kmsKey.getZoneId());
154146

@@ -240,7 +232,7 @@ KMSKey createUserKMSKey(Long accountId, Long domainId, Long zoneId,
240232
throw KMSException.invalidParameter("HSM Profile not found");
241233
}
242234

243-
// Determine provider from HSM profile or default to database
235+
// Determine provider from HSM profile
244236
KMSProvider provider = getKMSProvider(profile.getProtocol());
245237

246238
// Generate unique KEK label
@@ -373,16 +365,17 @@ public byte[] unwrapKey(Long wrappedKeyId) throws KMSException {
373365
throw KMSException.kekNotFound("KMS key not found for wrapped key: " + wrappedKeyId);
374366
}
375367

376-
KMSProvider provider = getKMSProvider(kmsKey.getProviderName());
377-
378368
// Try the specific version first if available
379369
if (wrappedVO.getKekVersionId() != null) {
380370
KMSKekVersionVO version = kmsKekVersionDao.findById(wrappedVO.getKekVersionId());
381371
if (version != null && version.getStatus() != KMSKekVersionVO.Status.Archived) {
382372
try {
373+
HSMProfileVO hsmProfile = hsmProfileDao.findById(version.getHsmProfileId());
374+
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
375+
383376
WrappedKey wrapped = new WrappedKey(version.getKekLabel(), kmsKey.getPurpose(),
384377
kmsKey.getAlgorithm(), wrappedVO.getWrappedBlob(),
385-
kmsKey.getProviderName(), wrappedVO.getCreated(), kmsKey.getZoneId());
378+
hsmProfile.getProtocol(), wrappedVO.getCreated(), kmsKey.getZoneId());
386379
// Pass HSM profile ID from version
387380
byte[] dek = retryOperation(() -> provider.unwrapKey(wrapped, version.getHsmProfileId()));
388381
logger.debug("Successfully unwrapped key {} with KEK version {}", wrappedKeyId,
@@ -398,6 +391,9 @@ public byte[] unwrapKey(Long wrappedKeyId) throws KMSException {
398391
List<KMSKekVersionVO> versions = kmsKekVersionDao.getVersionsForDecryption(kmsKey.getId());
399392
for (KMSKekVersionVO version : versions) {
400393
try {
394+
HSMProfileVO hsmProfile = hsmProfileDao.findById(version.getHsmProfileId());
395+
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
396+
401397
WrappedKey wrapped = new WrappedKey(version.getKekLabel(), kmsKey.getPurpose(),
402398
kmsKey.getAlgorithm(), wrappedVO.getWrappedBlob(),
403399
kmsKey.getProviderName(), wrappedVO.getCreated(), kmsKey.getZoneId());
@@ -433,16 +429,15 @@ public WrappedKey generateVolumeKeyWithKek(KMSKey kmsKey, Long callerAccountId)
433429
throw KMSException.invalidParameter("KMS key purpose is not VOLUME_ENCRYPTION: " + kmsKey);
434430
}
435431

436-
HSMProfileVO hsmProfile = hsmProfileDao.findById(kmsKey.getHsmProfileId());
432+
// Get active KEK version
433+
KMSKekVersionVO activeVersion = getActiveKekVersion(kmsKey.getId());
434+
435+
HSMProfileVO hsmProfile = hsmProfileDao.findById(activeVersion.getHsmProfileId());
437436
if (hsmProfile == null) {
438-
throw KMSException.invalidParameter("HSM profile not found: " + kmsKey.getHsmProfileId());
437+
throw KMSException.invalidParameter("HSM profile not found: " + activeVersion.getHsmProfileId());
439438
}
440-
441439
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
442440

443-
// Get active KEK version
444-
KMSKekVersionVO activeVersion = getActiveKekVersion(kmsKey.getId());
445-
446441
// Generate and wrap DEK using active KEK version
447442
int dekSize = KMSDekSizeBits.value();
448443
WrappedKey wrappedKey;
@@ -1081,7 +1076,7 @@ private void processVersionRewrap(KMSKekVersionVO oldVersion, int batchSize) thr
10811076
}
10821077

10831078
// Get active version for this KMS key
1084-
KMSKekVersionVO activeVersion = kmsKekVersionDao.getActiveVersion(oldVersion.getKmsKeyId());
1079+
KMSKekVersionVO activeVersion = kmsKekVersionDao.getActiveVersion(oldVersion.getKmsKeyId());
10851080
if (activeVersion == null) {
10861081
logger.warn("No active KEK version found for KMS key {}, skipping", kmsKey);
10871082
return;
@@ -1102,7 +1097,8 @@ private void processVersionRewrap(KMSKekVersionVO oldVersion, int batchSize) thr
11021097
}
11031098

11041099
// Get provider
1105-
KMSProvider provider = getKMSProviderForZone(kmsKey.getZoneId());
1100+
HSMProfileVO hsmProfile = hsmProfileDao.findById(activeVersion.getHsmProfileId());
1101+
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
11061102

11071103
// Rewrap this batch using the common helper
11081104
int successCount = 0;
@@ -1146,15 +1142,15 @@ public boolean deleteKMSKeysByAccountId(Long accountId) {
11461142
boolean allDeleted = true;
11471143
for (KMSKeyVO key : accountKeys) {
11481144
try {
1149-
KMSProvider provider = getKMSProviderForZone(key.getZoneId());
1150-
11511145
// Step 1: Delete all KEKs from the provider first
11521146
List<KMSKekVersionVO> kekVersions = kmsKekVersionDao.listByKmsKeyId(key.getId());
11531147
if (kekVersions != null && !kekVersions.isEmpty()) {
11541148
logger.debug("Deleting {} KEK version(s) from provider for KMS key {}",
11551149
kekVersions.size(), key.getUuid());
11561150
for (KMSKekVersionVO kekVersion : kekVersions) {
1151+
HSMProfileVO hsmProfile = hsmProfileDao.findById(kekVersion.getHsmProfileId());
11571152
try {
1153+
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
11581154
provider.deleteKek(kekVersion.getKekLabel());
11591155
logger.debug("Deleted KEK {} (v{}) from provider",
11601156
kekVersion.getKekLabel(), kekVersion.getVersionNumber());

‎server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,11 @@
1919

2020
import static org.junit.Assert.assertFalse;
2121
import static org.junit.Assert.assertNotNull;
22-
import static org.junit.Assert.assertNull;
2322
import static org.junit.Assert.assertTrue;
24-
import static org.junit.Assert.assertEquals;
25-
import static org.mockito.ArgumentMatchers.anyLong;
2623
import static org.mockito.Mockito.mock;
27-
import static org.mockito.Mockito.never;
2824
import static org.mockito.Mockito.verify;
2925
import static org.mockito.Mockito.when;
3026

31-
import java.util.ArrayList;
3227
import java.util.Arrays;
3328

3429
import org.apache.cloudstack.api.response.HSMProfileResponse;
@@ -41,9 +36,7 @@
4136
import org.mockito.Spy;
4237
import org.mockito.junit.MockitoJUnitRunner;
4338

44-
import com.cloud.exception.PermissionDeniedException;
4539
import com.cloud.user.AccountManager;
46-
import com.cloud.utils.exception.CloudRuntimeException;
4740

4841
/**
4942
* Unit tests for HSM-related business logic in KMSManagerImpl

‎server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplKeyCreationTest.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@
2828
import static org.mockito.Mockito.verify;
2929
import static org.mockito.Mockito.when;
3030

31-
import java.util.Arrays;
32-
import java.util.ArrayList;
33-
3431
import org.apache.cloudstack.framework.kms.KMSException;
3532
import org.apache.cloudstack.framework.kms.KMSProvider;
3633
import org.apache.cloudstack.framework.kms.KeyPurpose;
@@ -90,9 +87,9 @@ public void testCreateUserKMSKey_WithExplicitProfile() throws Exception {
9087
Long hsmProfileId = 10L;
9188

9289
HSMProfileVO profile = mock(HSMProfileVO.class);
93-
when(profile.getId()).thenReturn(hsmProfileId);
9490
when(profile.getAccountId()).thenReturn(testAccountId);
95-
when(hsmProfileDao.findByName(hsmProfileName)).thenReturn(profile);
91+
when(profile.getProtocol()).thenReturn(testProviderName);
92+
when(hsmProfileDao.findById(hsmProfileId)).thenReturn(profile);
9693

9794
// Mock provider KEK creation
9895
when(kmsProvider.createKek(any(KeyPurpose.class), anyString(), anyInt(), eq(hsmProfileId)))
@@ -106,16 +103,16 @@ public void testCreateUserKMSKey_WithExplicitProfile() throws Exception {
106103
KMSKekVersionVO mockVersion = mock(KMSKekVersionVO.class);
107104
when(kmsKekVersionDao.persist(any(KMSKekVersionVO.class))).thenReturn(mockVersion);
108105

109-
// Mock getKMSProviderForZone to return our mock provider
110-
doReturn(kmsProvider).when(kmsManager).getKMSProviderForZone(testZoneId);
106+
// Mock getKMSProvider to return our mock provider
111107
doReturn(true).when(kmsManager).isKmsEnabled(testZoneId);
108+
doReturn(kmsProvider).when(kmsManager).getKMSProvider(testProviderName);
112109

113110
KMSKey result = kmsManager.createUserKMSKey(testAccountId, testDomainId,
114111
testZoneId, "test-key", "Test key", KeyPurpose.VOLUME_ENCRYPTION, 256, hsmProfileId);
115112

116113
// Verify explicit profile was used
117114
assertNotNull(result);
118-
verify(hsmProfileDao).findByName(hsmProfileName);
115+
verify(hsmProfileDao).findById(hsmProfileId);
119116
verify(kmsProvider).createKek(any(KeyPurpose.class), anyString(), eq(256), eq(hsmProfileId));
120117

121118
// Verify KMSKeyVO was created with correct profile ID
@@ -135,7 +132,6 @@ public void testCreateUserKMSKey_ThrowsExceptionWhenProfileNotFound() throws KMS
135132
long hsmProfileId = 1L;
136133
when(hsmProfileDao.findById(hsmProfileId)).thenReturn(null);
137134

138-
doReturn(kmsProvider).when(kmsManager).getKMSProviderForZone(testZoneId);
139135
doReturn(true).when(kmsManager).isKmsEnabled(testZoneId);
140136

141137
kmsManager.createUserKMSKey(testAccountId, testDomainId, testZoneId,
@@ -151,10 +147,9 @@ public void testCreateUserKMSKey_CreatesKekVersionWithProfileId() throws Excepti
151147
Long hsmProfileId = 40L;
152148

153149
HSMProfileVO profile = mock(HSMProfileVO.class);
154-
when(profile.getId()).thenReturn(hsmProfileId);
155-
when(profile.isEnabled()).thenReturn(true);
156150
when(profile.getProtocol()).thenReturn(testProviderName);
157-
when(hsmProfileDao.listByAccountId(testAccountId)).thenReturn(Arrays.asList(profile));
151+
when(profile.getAccountId()).thenReturn(testAccountId);
152+
when(hsmProfileDao.findById(hsmProfileId)).thenReturn(profile);
158153

159154
when(kmsProvider.createKek(any(KeyPurpose.class), anyString(), anyInt(), eq(hsmProfileId)))
160155
.thenReturn("test-kek-label");
@@ -166,8 +161,8 @@ public void testCreateUserKMSKey_CreatesKekVersionWithProfileId() throws Excepti
166161
KMSKekVersionVO mockVersion = mock(KMSKekVersionVO.class);
167162
when(kmsKekVersionDao.persist(any(KMSKekVersionVO.class))).thenReturn(mockVersion);
168163

169-
doReturn(kmsProvider).when(kmsManager).getKMSProviderForZone(testZoneId);
170164
doReturn(true).when(kmsManager).isKmsEnabled(testZoneId);
165+
doReturn(kmsProvider).when(kmsManager).getKMSProvider(testProviderName);
171166

172167
kmsManager.createUserKMSKey(testAccountId, testDomainId, testZoneId,
173168
"test-key", "Test key", KeyPurpose.VOLUME_ENCRYPTION, 256, hsmProfileId);

0 commit comments

Comments
 (0)