From 48e745cad28d4ff146b57c62501bbcb908308082 Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Wed, 28 Aug 2024 15:06:44 +0530 Subject: [PATCH] Add certificate validation to check headers (#9255) --- .../security/keystore/KeystoreManager.java | 3 ++- .../keystore/KeystoreManagerImpl.java | 23 +++++++++------- .../cloud/server/ManagementServerImpl.java | 27 ++++++++++++++++--- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java index c44347c85ef..18b840e7a8c 100644 --- a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java +++ b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java @@ -18,6 +18,7 @@ package org.apache.cloudstack.framework.security.keystore; import com.cloud.agent.api.LogLevel; import com.cloud.agent.api.LogLevel.Log4jLevel; +import com.cloud.utils.Pair; import com.cloud.utils.component.Manager; public interface KeystoreManager extends Manager { @@ -59,7 +60,7 @@ public interface KeystoreManager extends Manager { } } - boolean validateCertificate(String certificate, String key, String domainSuffix); + Pair validateCertificate(String certificate, String key, String domainSuffix); void saveCertificate(String name, String certificate, String key, String domainSuffix); diff --git a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java index 03a91fecc8e..f89b6eebf9b 100644 --- a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java +++ b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java @@ -30,6 +30,7 @@ import java.util.regex.Pattern; import javax.inject.Inject; +import com.cloud.utils.Pair; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -47,24 +48,28 @@ public class KeystoreManagerImpl extends ManagerBase implements KeystoreManager private KeystoreDao _ksDao; @Override - public boolean validateCertificate(String certificate, String key, String domainSuffix) { + public Pair validateCertificate(String certificate, String key, String domainSuffix) { + String errMsg = null; if (StringUtils.isAnyEmpty(certificate, key, domainSuffix)) { - s_logger.error("Invalid parameter found in (certificate, key, domainSuffix) tuple for domain: " + domainSuffix); - return false; + errMsg = String.format("Invalid parameter found in (certificate, key, domainSuffix) tuple for domain: %s", domainSuffix); + s_logger.error(errMsg); + return new Pair<>(false, errMsg); } try { String ksPassword = "passwordForValidation"; byte[] ksBits = CertificateHelper.buildAndSaveKeystore(domainSuffix, certificate, getKeyContent(key), ksPassword); KeyStore ks = CertificateHelper.loadKeystore(ksBits, ksPassword); - if (ks != null) - return true; - - s_logger.error("Unabled to construct keystore for domain: " + domainSuffix); + if (ks != null) { + return new Pair<>(true, errMsg); + } + errMsg = String.format("Unable to construct keystore for domain: %s", domainSuffix); + s_logger.error(errMsg); } catch (Exception e) { - s_logger.error("Certificate validation failed due to exception for domain: " + domainSuffix, e); + errMsg = String.format("Certificate validation failed due to exception for domain: %s", domainSuffix); + s_logger.error(errMsg, e); } - return false; + return new Pair<>(false, errMsg); } @Override diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 9d69a52cfb8..5ac34fba1d5 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -17,6 +17,7 @@ package com.cloud.server; import java.lang.reflect.Field; +import java.security.cert.CertificateException; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -43,6 +44,7 @@ import javax.crypto.spec.SecretKeySpec; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.utils.security.CertificateHelper; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroupProcessor; @@ -4484,13 +4486,12 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe final String certificate = cmd.getCertificate(); final String key = cmd.getPrivateKey(); + String domainSuffix = cmd.getDomainSuffix(); - if (cmd.getPrivateKey() != null && !_ksMgr.validateCertificate(certificate, key, cmd.getDomainSuffix())) { - throw new InvalidParameterValueException("Failed to pass certificate validation check"); - } + validateCertificate(certificate, key, domainSuffix); if (cmd.getPrivateKey() != null) { - _ksMgr.saveCertificate(ConsoleProxyManager.CERTIFICATE_NAME, certificate, key, cmd.getDomainSuffix()); + _ksMgr.saveCertificate(ConsoleProxyManager.CERTIFICATE_NAME, certificate, key, domainSuffix); // Reboot ssvm here since private key is present - meaning server cert being passed final List alreadyRunning = _secStorageVmDao.getSecStorageVmListInStates(null, State.Running, State.Migrating, State.Starting); @@ -4507,6 +4508,24 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe + "please give a few minutes for console access and storage services service to be up and working again"; } + private void validateCertificate(String certificate, String key, String domainSuffix) { + if (key != null) { + Pair result = _ksMgr.validateCertificate(certificate, key, domainSuffix); + if (!result.first()) { + throw new InvalidParameterValueException(String.format("Failed to pass certificate validation check with error: %s", result.second())); + } + } else { + try { + s_logger.debug(String.format("Trying to validate the root certificate format")); + CertificateHelper.buildCertificate(certificate); + } catch (CertificateException e) { + String errorMsg = String.format("Failed to pass certificate validation check with error: Certificate validation failed due to exception: %s", e.getMessage()); + s_logger.error(errorMsg); + throw new InvalidParameterValueException(errorMsg); + } + } + } + @Override public List getHypervisors(final Long zoneId) { final List result = new ArrayList();