fixup

vishesh92 · vishesh92 · commit 74106697e90e · 2026-06-10T15:29:57.000+05:30
diff --git a/api/src/main/java/org/apache/cloudstack/api/response/HSMProfileResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/HSMProfileResponse.java
@@ -179,4 +179,76 @@ public void setCreated(Date created) {
     public void setDetails(Map<String, String> details) {
         this.details = details;
     }
+
+    public String getId() {
+        return id;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public String getProtocol() {
+        return protocol;
+    }
+
+    public String getAccountId() {
+        return accountId;
+    }
+
+    public String getAccountName() {
+        return accountName;
+    }
+
+    public String getDomainId() {
+        return domainId;
+    }
+
+    public String getDomainName() {
+        return domainName;
+    }
+
+    public String getDomainPath() {
+        return domainPath;
+    }
+
+    public String getProjectId() {
+        return projectId;
+    }
+
+    public String getProjectName() {
+        return projectName;
+    }
+
+    public String getZoneId() {
+        return zoneId;
+    }
+
+    public String getZoneName() {
+        return zoneName;
+    }
+
+    public String getVendorName() {
+        return vendorName;
+    }
+
+    public String getState() {
+        return state;
+    }
+
+    public Boolean getEnabled() {
+        return enabled;
+    }
+
+    public Boolean getPublic() {
+        return isPublic;
+    }
+
+    public Date getCreated() {
+        return created;
+    }
+
+    public Map<String, String> getDetails() {
+        return details;
+    }
 }
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
@@ -55,14 +55,13 @@
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.Provider;
+import java.security.ProviderException;
 import java.security.SecureRandom;
 import java.security.Security;
 import java.security.UnrecoverableKeyException;
 import java.security.cert.CertificateException;
-import java.util.ArrayDeque;
 import java.util.Arrays;
 import java.util.Date;
-import java.util.Deque;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -427,6 +426,7 @@ PKCS11Session acquireSession(long timeoutMs) throws KMSException {
                     return session;
                 }
                 // Stale idle session: discard it and free its permit so a new one can be created.
+                logger.debug("Discarding stale idle PKCS#11 session for profile {}", profileId);
                 session.close();
                 sessionPermits.release();
             }
@@ -517,15 +517,13 @@ void invalidate() {
     private static class PKCS11Session {
         private static final int IV_LENGTH = 16; // 128 bits for CBC mode
 
-        // Counter to bypass JDK SunPKCS11 caching when the HSM restarts
+        // Counter to bypass JDK SunPKCS11 caching when the HSM restarts. The JDK's
+        // PKCS11.getInstance caches the native module in a static map keyed by the library
+        // path string and never removes entries, so a module attached to a dead HSM can
+        // never be re-initialized under its original path. Each bump adds an extra slash
+        // to the path (a distinct map key for the same file), forcing a fresh C_Initialize.
         private static final AtomicInteger hsmRestartCount = new AtomicInteger(0);
 
-        // Holds strong references to recently-closed SunPKCS11 providers until a new
-        // session has successfully completed C_Initialize + C_OpenSession. This prevents
-        // GC from running the old provider's finalizer (which calls C_Finalize on the
-        // shared native PKCS11 module) while a new session is still initialising.
-        private static final Deque<Provider> pendingFinalization = new ArrayDeque<>();
-
         private KeyStore keyStore;
         private Provider provider;
         private String providerName;
@@ -568,6 +566,7 @@ private static class PKCS11Session {
          *                      </ul>
          */
         private void connect(Map<String, String> config) throws KMSException {
+            boolean connected = false;
             try {
                 // Unique suffix ensures each session gets its own provider name in java.security.Security,
                 // allowing Security.removeProvider() in close() to target exactly this session's provider.
@@ -606,20 +605,20 @@ private void connect(Map<String, String> config) throws KMSException {
                     throw KMSException.invalidParameter("pin is required");
                 }
                 char[] pinChars = pin.toCharArray();
-                keyStore.load(null, pinChars);
-                Arrays.fill(pinChars, '\0');
+                try {
+                    keyStore.load(null, pinChars);
+                } finally {
+                    // Wipe the PIN copy on every path, including load() failures
+                    // (wrong PIN, CRYPTOKI_NOT_INITIALIZED after an HSM restart, etc.).
+                    Arrays.fill(pinChars, '\0');
+                }
 
                 // The temp file is only needed during configure()/load(); delete it immediately
                 // rather than holding it until the session is eventually closed.
                 Files.deleteIfExists(tempConfigFile);
                 tempConfigFile = null;
 
-                // New session is fully initialised (C_Initialize + C_OpenSession succeeded).
-                // Release the deferred old providers so they can now be GC'd and finalised
-                // without racing against this session's initialisation.
-                synchronized (pendingFinalization) {
-                    pendingFinalization.clear();
-                }
+                connected = true;
 
                 logger.debug("Successfully connected to PKCS#11 HSM at {}", config.get("library"));
             } catch (KeyStoreException | CertificateException | NoSuchAlgorithmException e) {
@@ -636,6 +635,12 @@ private void connect(Map<String, String> config) throws KMSException {
                 }
             } catch (Exception e) {
                 handlePKCS11Exception(e, "Unexpected error during PKCS#11 connection");
+            } finally {
+                if (!connected) {
+                    // connect that failed after Security.addProvider() would otherwise leave
+                    // the half-initialized provider registered in java.security.Security.
+                    close();
+                }
             }
         }
 
@@ -663,7 +668,6 @@ private String buildSunPKCS11Config(Map<String, String> config, String nameSuffi
                     libraryPath = "./" + extraSlashes + libraryPath;
                 }
             }
-
             StringBuilder configBuilder = new StringBuilder();
             // Include the unique suffix so that each session is registered under a distinct
             // provider name (SunPKCS11-CloudStackHSM-{suffix}), preventing name collisions
@@ -720,30 +724,36 @@ private String buildSunPKCS11Config(Map<String, String> config, String nameSuffi
          */
         private void handlePKCS11Exception(Exception e, String context) throws KMSException {
             String errorMsg = e.getMessage();
-            String causeErrorMessage = e.getCause() != null ? e.getCause().getMessage() : "";
             if (errorMsg == null) {
                 errorMsg = e.getClass().getSimpleName();
             }
             logger.warn("PKCS#11 error: {} - {}", errorMsg, context, e);
 
-            if (causeErrorMessage.contains("CRYPTOKI_NOT_INITIALIZED") || errorMsg.contains("CRYPTOKI_NOT_INITIALIZED")) {
+            // The PKCS#11 error code (CKR_*) may be on the exception itself or its cause, so
+            // match against both. KeyStore.getKey/load wrap the real code in a ProviderException.
+            String causeMsg = e.getCause() != null ? e.getCause().getMessage() : null;
+            String combinedMsg = errorMsg + " " + (causeMsg != null ? causeMsg : "");
+
+            if (combinedMsg.contains("CRYPTOKI_NOT_INITIALIZED")) {
                 hsmRestartCount.incrementAndGet();
                 throw new KMSException(KMSException.ErrorType.CONNECTION_FAILED,
                         context + ": HSM requires re-initialization (CRYPTOKI_NOT_INITIALIZED)", e);
-            } else if (causeErrorMessage.contains("PIN_INCORRECT") || errorMsg.contains("PIN_INCORRECT")) {
+            } else if (combinedMsg.contains("PIN_INCORRECT")) {
                 throw new KMSException(KMSException.ErrorType.AUTHENTICATION_FAILED,
                         context + ": Incorrect PIN", e);
-            } else if (causeErrorMessage.contains("SLOT_ID_INVALID") || errorMsg.contains("SLOT_ID_INVALID")) {
+            } else if (combinedMsg.contains("SLOT_ID_INVALID")) {
                 throw KMSException.invalidParameter(context + ": Invalid slot ID");
-            } else if (causeErrorMessage.contains("KEY_NOT_FOUND") || errorMsg.contains("KEY_NOT_FOUND")) {
+            } else if (combinedMsg.contains("KEY_NOT_FOUND")) {
                 throw KMSException.kekNotFound(context + ": Key not found");
-            } else if (causeErrorMessage.contains("DEVICE_ERROR") || errorMsg.contains("DEVICE_ERROR")) {
+            } else if (combinedMsg.contains("DEVICE_ERROR")) {
+                hsmRestartCount.incrementAndGet();
                 throw new KMSException(KMSException.ErrorType.CONNECTION_FAILED,
                         context + ": HSM device error", e);
-            } else if (causeErrorMessage.contains("SESSION_HANDLE_INVALID") || errorMsg.contains("SESSION_HANDLE_INVALID")) {
+            } else if (combinedMsg.contains("SESSION_HANDLE_INVALID")) {
+                hsmRestartCount.incrementAndGet();
                 throw new KMSException(KMSException.ErrorType.CONNECTION_FAILED,
                         context + ": Invalid session handle", e);
-            } else if (causeErrorMessage.contains("KEY_ALREADY_EXISTS") || errorMsg.contains("KEY_ALREADY_EXISTS")) {
+            } else if (combinedMsg.contains("KEY_ALREADY_EXISTS")) {
                 throw KMSException.keyAlreadyExists(context);
             } else if (e instanceof KeyStoreException) {
                 throw new KMSException(KMSException.ErrorType.WRAP_UNWRAP_FAILED,
@@ -800,12 +810,6 @@ void close() {
                 if (provider != null && providerName != null) {
                     try {
                         Security.removeProvider(providerName);
-                        // Keep a strong reference so GC cannot finalize this provider
-                        // (and call C_Finalize on the shared native module) until the
-                        // next session has fully initialised. Drained in connect().
-                        synchronized (pendingFinalization) {
-                            pendingFinalization.addLast(provider);
-                        }
                     } catch (Exception e) {
                         logger.debug("Failed to remove provider {}: {}", providerName, e.getMessage());
                     }
@@ -971,8 +975,6 @@ byte[] wrapKey(byte[] plainDek, String kekLabel) throws KMSException {
                 handlePKCS11Exception(e, "Invalid IV for CBC mode");
             } catch (Exception e) {
                 handlePKCS11Exception(e, "Failed to wrap key with HSM");
-            } finally {
-                kek = null;
             }
             return null;
         }
@@ -1000,6 +1002,11 @@ private SecretKey getKekFromKeyStore(String kekLabel) throws KMSException {
                 handlePKCS11Exception(e, "Algorithm not supported");
             } catch (KeyStoreException e) {
                 handlePKCS11Exception(e, "Failed to retrieve KEK from HSM");
+            } catch (ProviderException e) {
+                // KeyStore.getKey() wraps PKCS#11 errors (e.g. CKR_CRYPTOKI_NOT_INITIALIZED after
+                // an HSM restart) in ProviderException, an unchecked RuntimeException. Route it
+                // through handlePKCS11Exception so hsmRestartCount is bumped and the caller retries.
+                handlePKCS11Exception(e, "Failed to retrieve KEK from HSM");
             }
             return null;
         }
@@ -1061,8 +1068,6 @@ byte[] unwrapKey(byte[] wrappedBlob, String kekLabel) throws KMSException {
                 handlePKCS11Exception(e, "Invalid IV for CBC mode");
             } catch (Exception e) {
                 handlePKCS11Exception(e, "Failed to unwrap key with HSM");
-            } finally {
-                kek = null;
             }
             return null;
         }
diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
@@ -480,9 +480,9 @@ private DeployDestination deployInVmLastHost(VirtualMachineProfile vmProfile, De
 
         logger.debug("This VM has last host_id: {}", vm.getLastHostId());
         HostVO lastHost = _hostDao.findById(vm.getLastHostId());
+        _hostDao.loadDetails(lastHost);
         if (canUseLastHost(lastHost, avoids, plan, vm, offering, volumesRequireEncryption)) {
             _hostDao.loadHostTags(lastHost);
-            _hostDao.loadDetails(lastHost);
             if (lastHost.getStatus() != Status.Up) {
                 logger.debug("Cannot deploy VM [{}] to the last host [{}] because this host is not in UP state or is not enabled. Host current status [{}] and resource status [{}].",
                         vm, lastHost, lastHost.getState().name(), lastHost.getResourceState());
diff --git a/server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java b/server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java
@@ -216,6 +216,8 @@ public byte[] unwrapKey(Long wrappedKeyId) throws KMSException {
             throw KMSException.kekNotFound("KMS key not found for wrapped key: " + wrappedKeyId);
         }
 
+        Exception lastException = null;
+
         if (wrappedVO.getKekVersionId() != null) {
             KMSKekVersionVO version = kmsKekVersionDao.findById(wrappedVO.getKekVersionId());
             if (version != null && version.getStatus() != KMSKekVersionVO.Status.Archived) {
@@ -235,7 +237,8 @@ public byte[] unwrapKey(Long wrappedKeyId) throws KMSException {
                     logger.debug("Successfully unwrapped key {} with KEK version {}", wrappedKeyId, version.getVersionNumber());
                     return dek;
                 } catch (Exception e) {
-                    logger.warn("Failed to unwrap with version {}: {}", version.getVersionNumber(), e.getMessage());
+                    lastException = e;
+                    logger.warn("Failed to unwrap with version {}: {}", version.getVersionNumber(), e.getMessage(), e);
                 }
             }
         }
@@ -260,11 +263,12 @@ public byte[] unwrapKey(Long wrappedKeyId) throws KMSException {
                         wrappedKeyId, version.getVersionNumber());
                 return dek;
             } catch (Exception e) {
-                logger.debug("Failed to unwrap with version {}: {}", version.getVersionNumber(), e.getMessage());
+                lastException = e;
+                logger.warn("Failed to unwrap with version {}: {}", version.getVersionNumber(), e.getMessage(), e);
             }
         }
 
-        throw KMSException.wrapUnwrapFailed("Failed to unwrap key with any available KEK version");
+        throw KMSException.wrapUnwrapFailed("Failed to unwrap key: no available KEK version for wrapped key " + wrappedKeyId, lastException);
     }
 
     private byte[] getUnwrappedKey(KMSWrappedKeyVO wrappedVO, KMSKeyVO kmsKey,
@@ -366,6 +370,7 @@ public KMSKeyResponse createKMSKey(CreateKMSKeyCmd cmd) throws KMSException {
 
         HSMProfileVO profile = getHSMProfile(cmd.getHsmProfileId());
         checkHSMProfileAccess(caller, profile, false);
+        validateProfileScopeForOwner(profile, targetAccount.getId(), targetAccount.getDomainId());
         if (!profile.isEnabled()) {
             throw new InvalidParameterValueException("HSM profile is not enabled: " + profile.getName());
         }
@@ -665,6 +670,7 @@ public String rotateKMSKey(RotateKMSKeyCmd cmd) throws KMSException {
                 throw new InvalidParameterValueException("Target HSM Profile not found: " + hsmProfileId);
             }
             checkHSMProfileAccess(caller, profile, false);
+            validateProfileScopeForOwner(profile, kmsKey.getAccountId(), kmsKey.getDomainId());
             if (!profile.isEnabled()) {
                 throw new InvalidParameterValueException("HSM profile is not enabled: " + profile.getName());
             }
@@ -1362,7 +1368,7 @@ private HSMProfileResponse createHSMProfileResponse(HSMProfile profile, boolean
             if (domain != null) {
                 response.setDomainId(domain.getUuid());
                 response.setDomainName(domain.getName());
-                response.setDomainPath(domain.getPath());
+                response.setDomainPath(ApiResponseHelper.getPrettyDomainPath(domain.getPath()));
             }
         } else {
             ApiResponseHelper.populateOwner(response, profile);
@@ -1424,6 +1430,33 @@ void checkHSMProfileAccess(Account caller, HSMProfileVO profile, boolean require
         }
     }
 
+    /**
+     * Validate that an HSM profile's scope is compatible with a prospective KMS key owner.
+     * This is a consistency rule (not a caller-permission check): it applies to all callers,
+     * including root admins, so a key cannot be bound to a profile outside its scope.
+     * - public profile: usable by any owner.
+     * - domain-scoped (account-less) profile: the owner must be directly in the profile's domain.
+     * - account-owned profile: the owner must be that account.
+     */
+    void validateProfileScopeForOwner(HSMProfileVO profile, long ownerAccountId, long ownerDomainId) {
+        if (profile.getIsPublic()) {
+            return;
+        }
+        if (profile.getAccountId() == -1 && profile.getDomainId() != -1) {
+            if (ownerDomainId != profile.getDomainId()) {
+                throw new InvalidParameterValueException(String.format(
+                        "HSM profile '%s' is domain-scoped and can only be used by accounts in its domain",
+                        profile.getName()));
+            }
+        } else if (profile.getAccountId() != -1) {
+            if (ownerAccountId != profile.getAccountId()) {
+                throw new InvalidParameterValueException(String.format(
+                        "HSM profile '%s' is owned by another account and cannot be used for this key's owner",
+                        profile.getName()));
+            }
+        }
+    }
+
     /**
      * Parse and validate a key purpose string. Returns null if the input is null.
      */
diff --git a/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java b/server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java
diff --git a/ui/src/components/view/VolumesTab.vue b/ui/src/components/view/VolumesTab.vue
