mirror of https://github.com/apache/cloudstack.git
fixups
This commit is contained in:
parent
56cf3f6b0e
commit
529fd9d661
|
|
@ -28,6 +28,7 @@ import org.apache.cloudstack.api.ServerApiException;
|
|||
import org.apache.cloudstack.api.response.AsyncJobResponse;
|
||||
import org.apache.cloudstack.api.response.HSMProfileResponse;
|
||||
import org.apache.cloudstack.api.response.KMSKeyResponse;
|
||||
import org.apache.cloudstack.api.response.SuccessResponse;
|
||||
import org.apache.cloudstack.framework.kms.KMSException;
|
||||
import org.apache.cloudstack.kms.KMSKey;
|
||||
import org.apache.cloudstack.kms.KMSManager;
|
||||
|
|
@ -81,6 +82,9 @@ public class RotateKMSKeyCmd extends BaseAsyncCmd {
|
|||
public void execute() {
|
||||
try {
|
||||
kmsManager.rotateKMSKey(this);
|
||||
SuccessResponse response = new SuccessResponse();
|
||||
response.setResponseName(getCommandName());
|
||||
setResponseObject(response);
|
||||
} catch (KMSException e) {
|
||||
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,
|
||||
"Failed to rotate KMS key: " + e.getMessage());
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import com.cloud.exception.InsufficientCapacityException;
|
|||
import com.cloud.exception.NetworkRuleConflictException;
|
||||
import com.cloud.exception.ResourceAllocationException;
|
||||
import com.cloud.exception.ResourceUnavailableException;
|
||||
import com.cloud.utils.StringUtils;
|
||||
import org.apache.cloudstack.api.APICommand;
|
||||
import org.apache.cloudstack.api.ApiConstants;
|
||||
import org.apache.cloudstack.api.ApiErrorCode;
|
||||
|
|
@ -37,7 +38,6 @@ import org.apache.cloudstack.framework.kms.KMSException;
|
|||
import org.apache.cloudstack.kms.HSMProfile;
|
||||
import org.apache.cloudstack.kms.KMSManager;
|
||||
import org.apache.commons.collections.MapUtils;
|
||||
import org.apache.commons.lang.StringUtils;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import java.util.Collection;
|
||||
|
|
|
|||
|
|
@ -17,27 +17,27 @@
|
|||
|
||||
package org.apache.cloudstack.kms;
|
||||
|
||||
import java.util.Date;
|
||||
|
||||
import org.apache.cloudstack.api.Identity;
|
||||
import org.apache.cloudstack.api.InternalIdentity;
|
||||
|
||||
import java.util.Date;
|
||||
|
||||
public interface HSMProfile extends Identity, InternalIdentity {
|
||||
String getName();
|
||||
|
||||
|
||||
String getProtocol();
|
||||
|
||||
|
||||
Long getAccountId();
|
||||
|
||||
|
||||
Long getDomainId();
|
||||
|
||||
|
||||
Long getZoneId();
|
||||
|
||||
|
||||
String getVendorName();
|
||||
|
||||
|
||||
boolean isEnabled();
|
||||
|
||||
|
||||
Date getCreated();
|
||||
|
||||
|
||||
Date getRemoved();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -161,15 +161,6 @@ public interface KMSManager extends Manager, Configurable {
|
|||
*/
|
||||
KMSProvider getKMSProvider(String name);
|
||||
|
||||
/**
|
||||
* Get the configured provider for a zone
|
||||
*
|
||||
* @param zoneId the zone ID (null for global default)
|
||||
* @return the configured provider
|
||||
* @throws KMSException if no provider configured
|
||||
*/
|
||||
KMSProvider getKMSProviderForZone(Long zoneId) throws KMSException;
|
||||
|
||||
/**
|
||||
* Check if KMS is enabled for a zone
|
||||
*
|
||||
|
|
|
|||
|
|
@ -120,7 +120,7 @@ public class PKCS11HSMProviderTest {
|
|||
public void testLoadProfileConfig_DecryptsSensitiveValues() {
|
||||
// Setup: Profile details with encrypted pin
|
||||
HSMProfileDetailsVO detail1 = mock(HSMProfileDetailsVO.class);
|
||||
when(detail1.getName()).thenReturn("library_path");
|
||||
when(detail1.getName()).thenReturn("library");
|
||||
when(detail1.getValue()).thenReturn("/path/to/lib.so");
|
||||
|
||||
HSMProfileDetailsVO detail2 = mock(HSMProfileDetailsVO.class);
|
||||
|
|
@ -140,7 +140,7 @@ public class PKCS11HSMProviderTest {
|
|||
// Verify
|
||||
assertNotNull("Config should not be null", config);
|
||||
assertEquals(3, config.size());
|
||||
assertEquals("/path/to/lib.so", config.get("library_path"));
|
||||
assertEquals("/path/to/lib.so", config.get("library"));
|
||||
// Note: In real code, DBEncryptionUtil.decrypt would be called
|
||||
// Here we just verify the structure is correct
|
||||
assertTrue("Config should contain pin", config.containsKey("pin"));
|
||||
|
|
@ -184,7 +184,7 @@ public class PKCS11HSMProviderTest {
|
|||
@Test
|
||||
public void testIsSensitiveKey_IdentifiesNonSensitiveKeys() {
|
||||
// Test
|
||||
assertFalse(provider.isSensitiveKey("library_path"));
|
||||
assertFalse(provider.isSensitiveKey("library"));
|
||||
assertFalse(provider.isSensitiveKey("slot_id"));
|
||||
assertFalse(provider.isSensitiveKey("endpoint"));
|
||||
assertFalse(provider.isSensitiveKey("max_sessions"));
|
||||
|
|
@ -232,7 +232,7 @@ public class PKCS11HSMProviderTest {
|
|||
public void testLoadProfileConfig_CachesConfiguration() {
|
||||
// Setup
|
||||
HSMProfileDetailsVO detail = mock(HSMProfileDetailsVO.class);
|
||||
when(detail.getName()).thenReturn("library_path");
|
||||
when(detail.getName()).thenReturn("library");
|
||||
when(detail.getValue()).thenReturn("/path/to/lib.so");
|
||||
when(hsmProfileDetailsDao.listByProfileId(testProfileId)).thenReturn(Arrays.asList(detail));
|
||||
|
||||
|
|
@ -251,7 +251,7 @@ public class PKCS11HSMProviderTest {
|
|||
public void testGetSessionPool_CreatesPoolForNewProfile() {
|
||||
// Setup
|
||||
HSMProfileDetailsVO detail = mock(HSMProfileDetailsVO.class);
|
||||
when(detail.getName()).thenReturn("library_path");
|
||||
when(detail.getName()).thenReturn("library");
|
||||
when(detail.getValue()).thenReturn("/path/to/lib.so");
|
||||
when(hsmProfileDetailsDao.listByProfileId(testProfileId)).thenReturn(Arrays.asList(detail));
|
||||
|
||||
|
|
@ -270,7 +270,7 @@ public class PKCS11HSMProviderTest {
|
|||
public void testGetSessionPool_ReusesPoolForSameProfile() {
|
||||
// Setup
|
||||
HSMProfileDetailsVO detail = mock(HSMProfileDetailsVO.class);
|
||||
when(detail.getName()).thenReturn("library_path");
|
||||
when(detail.getName()).thenReturn("library");
|
||||
when(detail.getValue()).thenReturn("/path/to/lib.so");
|
||||
when(hsmProfileDetailsDao.listByProfileId(testProfileId)).thenReturn(Arrays.asList(detail));
|
||||
|
||||
|
|
|
|||
|
|
@ -129,13 +129,6 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
return provider;
|
||||
}
|
||||
|
||||
@Override
|
||||
public KMSProvider getKMSProviderForZone(Long zoneId) throws KMSException {
|
||||
// Default to database provider for backward compatibility
|
||||
// HSM-based keys will use provider from HSM profile's protocol field
|
||||
return getKMSProvider("database");
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isKmsEnabled(Long zoneId) {
|
||||
if (zoneId == null) {
|
||||
|
|
@ -147,8 +140,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
/**
|
||||
* Internal method to rotate a KEK (create new version and update KMS key state)
|
||||
*/
|
||||
String rotateKek(KMSKeyVO kmsKey, String oldKekLabel,
|
||||
String newKekLabel, int keyBits, HSMProfileVO newHSMProfile) throws KMSException {
|
||||
String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int keyBits, HSMProfileVO newHSMProfile) throws KMSException {
|
||||
|
||||
validateKmsEnabled(kmsKey.getZoneId());
|
||||
|
||||
|
|
@ -240,7 +232,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
throw KMSException.invalidParameter("HSM Profile not found");
|
||||
}
|
||||
|
||||
// Determine provider from HSM profile or default to database
|
||||
// Determine provider from HSM profile
|
||||
KMSProvider provider = getKMSProvider(profile.getProtocol());
|
||||
|
||||
// Generate unique KEK label
|
||||
|
|
@ -373,16 +365,17 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
throw KMSException.kekNotFound("KMS key not found for wrapped key: " + wrappedKeyId);
|
||||
}
|
||||
|
||||
KMSProvider provider = getKMSProvider(kmsKey.getProviderName());
|
||||
|
||||
// Try the specific version first if available
|
||||
if (wrappedVO.getKekVersionId() != null) {
|
||||
KMSKekVersionVO version = kmsKekVersionDao.findById(wrappedVO.getKekVersionId());
|
||||
if (version != null && version.getStatus() != KMSKekVersionVO.Status.Archived) {
|
||||
try {
|
||||
HSMProfileVO hsmProfile = hsmProfileDao.findById(version.getHsmProfileId());
|
||||
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
|
||||
|
||||
WrappedKey wrapped = new WrappedKey(version.getKekLabel(), kmsKey.getPurpose(),
|
||||
kmsKey.getAlgorithm(), wrappedVO.getWrappedBlob(),
|
||||
kmsKey.getProviderName(), wrappedVO.getCreated(), kmsKey.getZoneId());
|
||||
hsmProfile.getProtocol(), wrappedVO.getCreated(), kmsKey.getZoneId());
|
||||
// Pass HSM profile ID from version
|
||||
byte[] dek = retryOperation(() -> provider.unwrapKey(wrapped, version.getHsmProfileId()));
|
||||
logger.debug("Successfully unwrapped key {} with KEK version {}", wrappedKeyId,
|
||||
|
|
@ -398,6 +391,9 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
List<KMSKekVersionVO> versions = kmsKekVersionDao.getVersionsForDecryption(kmsKey.getId());
|
||||
for (KMSKekVersionVO version : versions) {
|
||||
try {
|
||||
HSMProfileVO hsmProfile = hsmProfileDao.findById(version.getHsmProfileId());
|
||||
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
|
||||
|
||||
WrappedKey wrapped = new WrappedKey(version.getKekLabel(), kmsKey.getPurpose(),
|
||||
kmsKey.getAlgorithm(), wrappedVO.getWrappedBlob(),
|
||||
kmsKey.getProviderName(), wrappedVO.getCreated(), kmsKey.getZoneId());
|
||||
|
|
@ -433,16 +429,15 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
throw KMSException.invalidParameter("KMS key purpose is not VOLUME_ENCRYPTION: " + kmsKey);
|
||||
}
|
||||
|
||||
HSMProfileVO hsmProfile = hsmProfileDao.findById(kmsKey.getHsmProfileId());
|
||||
if (hsmProfile == null) {
|
||||
throw KMSException.invalidParameter("HSM profile not found: " + kmsKey.getHsmProfileId());
|
||||
}
|
||||
|
||||
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
|
||||
|
||||
// Get active KEK version
|
||||
KMSKekVersionVO activeVersion = getActiveKekVersion(kmsKey.getId());
|
||||
|
||||
HSMProfileVO hsmProfile = hsmProfileDao.findById(activeVersion.getHsmProfileId());
|
||||
if (hsmProfile == null) {
|
||||
throw KMSException.invalidParameter("HSM profile not found: " + activeVersion.getHsmProfileId());
|
||||
}
|
||||
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
|
||||
|
||||
// Generate and wrap DEK using active KEK version
|
||||
int dekSize = KMSDekSizeBits.value();
|
||||
WrappedKey wrappedKey;
|
||||
|
|
@ -1081,7 +1076,7 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
}
|
||||
|
||||
// Get active version for this KMS key
|
||||
KMSKekVersionVO activeVersion = kmsKekVersionDao.getActiveVersion(oldVersion.getKmsKeyId());
|
||||
KMSKekVersionVO activeVersion = kmsKekVersionDao.getActiveVersion(oldVersion.getKmsKeyId());
|
||||
if (activeVersion == null) {
|
||||
logger.warn("No active KEK version found for KMS key {}, skipping", kmsKey);
|
||||
return;
|
||||
|
|
@ -1102,7 +1097,8 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
}
|
||||
|
||||
// Get provider
|
||||
KMSProvider provider = getKMSProviderForZone(kmsKey.getZoneId());
|
||||
HSMProfileVO hsmProfile = hsmProfileDao.findById(activeVersion.getHsmProfileId());
|
||||
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
|
||||
|
||||
// Rewrap this batch using the common helper
|
||||
int successCount = 0;
|
||||
|
|
@ -1146,15 +1142,15 @@ public class KMSManagerImpl extends ManagerBase implements KMSManager, Pluggable
|
|||
boolean allDeleted = true;
|
||||
for (KMSKeyVO key : accountKeys) {
|
||||
try {
|
||||
KMSProvider provider = getKMSProviderForZone(key.getZoneId());
|
||||
|
||||
// Step 1: Delete all KEKs from the provider first
|
||||
List<KMSKekVersionVO> kekVersions = kmsKekVersionDao.listByKmsKeyId(key.getId());
|
||||
if (kekVersions != null && !kekVersions.isEmpty()) {
|
||||
logger.debug("Deleting {} KEK version(s) from provider for KMS key {}",
|
||||
kekVersions.size(), key.getUuid());
|
||||
for (KMSKekVersionVO kekVersion : kekVersions) {
|
||||
HSMProfileVO hsmProfile = hsmProfileDao.findById(kekVersion.getHsmProfileId());
|
||||
try {
|
||||
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
|
||||
provider.deleteKek(kekVersion.getKekLabel());
|
||||
logger.debug("Deleted KEK {} (v{}) from provider",
|
||||
kekVersion.getKekLabel(), kekVersion.getVersionNumber());
|
||||
|
|
|
|||
|
|
@ -19,16 +19,11 @@ package org.apache.cloudstack.kms;
|
|||
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.mockito.ArgumentMatchers.anyLong;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
|
||||
import org.apache.cloudstack.api.response.HSMProfileResponse;
|
||||
|
|
@ -41,9 +36,7 @@ import org.mockito.Mock;
|
|||
import org.mockito.Spy;
|
||||
import org.mockito.junit.MockitoJUnitRunner;
|
||||
|
||||
import com.cloud.exception.PermissionDeniedException;
|
||||
import com.cloud.user.AccountManager;
|
||||
import com.cloud.utils.exception.CloudRuntimeException;
|
||||
|
||||
/**
|
||||
* Unit tests for HSM-related business logic in KMSManagerImpl
|
||||
|
|
|
|||
|
|
@ -28,9 +28,6 @@ import static org.mockito.Mockito.mock;
|
|||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.ArrayList;
|
||||
|
||||
import org.apache.cloudstack.framework.kms.KMSException;
|
||||
import org.apache.cloudstack.framework.kms.KMSProvider;
|
||||
import org.apache.cloudstack.framework.kms.KeyPurpose;
|
||||
|
|
@ -90,9 +87,9 @@ public class KMSManagerImplKeyCreationTest {
|
|||
Long hsmProfileId = 10L;
|
||||
|
||||
HSMProfileVO profile = mock(HSMProfileVO.class);
|
||||
when(profile.getId()).thenReturn(hsmProfileId);
|
||||
when(profile.getAccountId()).thenReturn(testAccountId);
|
||||
when(hsmProfileDao.findByName(hsmProfileName)).thenReturn(profile);
|
||||
when(profile.getProtocol()).thenReturn(testProviderName);
|
||||
when(hsmProfileDao.findById(hsmProfileId)).thenReturn(profile);
|
||||
|
||||
// Mock provider KEK creation
|
||||
when(kmsProvider.createKek(any(KeyPurpose.class), anyString(), anyInt(), eq(hsmProfileId)))
|
||||
|
|
@ -106,16 +103,16 @@ public class KMSManagerImplKeyCreationTest {
|
|||
KMSKekVersionVO mockVersion = mock(KMSKekVersionVO.class);
|
||||
when(kmsKekVersionDao.persist(any(KMSKekVersionVO.class))).thenReturn(mockVersion);
|
||||
|
||||
// Mock getKMSProviderForZone to return our mock provider
|
||||
doReturn(kmsProvider).when(kmsManager).getKMSProviderForZone(testZoneId);
|
||||
// Mock getKMSProvider to return our mock provider
|
||||
doReturn(true).when(kmsManager).isKmsEnabled(testZoneId);
|
||||
doReturn(kmsProvider).when(kmsManager).getKMSProvider(testProviderName);
|
||||
|
||||
KMSKey result = kmsManager.createUserKMSKey(testAccountId, testDomainId,
|
||||
testZoneId, "test-key", "Test key", KeyPurpose.VOLUME_ENCRYPTION, 256, hsmProfileId);
|
||||
|
||||
// Verify explicit profile was used
|
||||
assertNotNull(result);
|
||||
verify(hsmProfileDao).findByName(hsmProfileName);
|
||||
verify(hsmProfileDao).findById(hsmProfileId);
|
||||
verify(kmsProvider).createKek(any(KeyPurpose.class), anyString(), eq(256), eq(hsmProfileId));
|
||||
|
||||
// Verify KMSKeyVO was created with correct profile ID
|
||||
|
|
@ -135,7 +132,6 @@ public class KMSManagerImplKeyCreationTest {
|
|||
long hsmProfileId = 1L;
|
||||
when(hsmProfileDao.findById(hsmProfileId)).thenReturn(null);
|
||||
|
||||
doReturn(kmsProvider).when(kmsManager).getKMSProviderForZone(testZoneId);
|
||||
doReturn(true).when(kmsManager).isKmsEnabled(testZoneId);
|
||||
|
||||
kmsManager.createUserKMSKey(testAccountId, testDomainId, testZoneId,
|
||||
|
|
@ -151,10 +147,9 @@ public class KMSManagerImplKeyCreationTest {
|
|||
Long hsmProfileId = 40L;
|
||||
|
||||
HSMProfileVO profile = mock(HSMProfileVO.class);
|
||||
when(profile.getId()).thenReturn(hsmProfileId);
|
||||
when(profile.isEnabled()).thenReturn(true);
|
||||
when(profile.getProtocol()).thenReturn(testProviderName);
|
||||
when(hsmProfileDao.listByAccountId(testAccountId)).thenReturn(Arrays.asList(profile));
|
||||
when(profile.getAccountId()).thenReturn(testAccountId);
|
||||
when(hsmProfileDao.findById(hsmProfileId)).thenReturn(profile);
|
||||
|
||||
when(kmsProvider.createKek(any(KeyPurpose.class), anyString(), anyInt(), eq(hsmProfileId)))
|
||||
.thenReturn("test-kek-label");
|
||||
|
|
@ -166,8 +161,8 @@ public class KMSManagerImplKeyCreationTest {
|
|||
KMSKekVersionVO mockVersion = mock(KMSKekVersionVO.class);
|
||||
when(kmsKekVersionDao.persist(any(KMSKekVersionVO.class))).thenReturn(mockVersion);
|
||||
|
||||
doReturn(kmsProvider).when(kmsManager).getKMSProviderForZone(testZoneId);
|
||||
doReturn(true).when(kmsManager).isKmsEnabled(testZoneId);
|
||||
doReturn(kmsProvider).when(kmsManager).getKMSProvider(testProviderName);
|
||||
|
||||
kmsManager.createUserKMSKey(testAccountId, testDomainId, testZoneId,
|
||||
"test-key", "Test key", KeyPurpose.VOLUME_ENCRYPTION, 256, hsmProfileId);
|
||||
|
|
|
|||
Loading…
Reference in New Issue