Skip to content

Commit cf30e9d

Browse files
harikrishna-patnaladhslove
authored andcommitted
Add certificate validation to check headers (apache#9255)
1 parent dcc4052 commit cf30e9d

3 files changed

Lines changed: 39 additions & 14 deletions

File tree

framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.cloud.agent.api.LogLevel;
2020
import com.cloud.agent.api.LogLevel.Log4jLevel;
21+
import com.cloud.utils.Pair;
2122
import com.cloud.utils.component.Manager;
2223

2324
public interface KeystoreManager extends Manager {
@@ -59,7 +60,7 @@ public String getRootCACert() {
5960
}
6061
}
6162

62-
boolean validateCertificate(String certificate, String key, String domainSuffix);
63+
Pair<Boolean, String> validateCertificate(String certificate, String key, String domainSuffix);
6364

6465
void saveCertificate(String name, String certificate, String key, String domainSuffix);
6566

framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import javax.inject.Inject;
3232

33+
import com.cloud.utils.Pair;
3334
import org.apache.commons.lang3.StringUtils;
3435
import org.springframework.stereotype.Component;
3536

@@ -45,24 +46,28 @@ public class KeystoreManagerImpl extends ManagerBase implements KeystoreManager
4546
private KeystoreDao _ksDao;
4647

4748
@Override
48-
public boolean validateCertificate(String certificate, String key, String domainSuffix) {
49+
public Pair<Boolean, String> validateCertificate(String certificate, String key, String domainSuffix) {
50+
String errMsg = null;
4951
if (StringUtils.isAnyEmpty(certificate, key, domainSuffix)) {
50-
logger.error("Invalid parameter found in (certificate, key, domainSuffix) tuple for domain: " + domainSuffix);
51-
return false;
52+
errMsg = String.format("Invalid parameter found in (certificate, key, domainSuffix) tuple for domain: %s", domainSuffix);
53+
logger.error(errMsg);
54+
return new Pair<>(false, errMsg);
5255
}
5356

5457
try {
5558
String ksPassword = "passwordForValidation";
5659
byte[] ksBits = CertificateHelper.buildAndSaveKeystore(domainSuffix, certificate, getKeyContent(key), ksPassword);
5760
KeyStore ks = CertificateHelper.loadKeystore(ksBits, ksPassword);
58-
if (ks != null)
59-
return true;
60-
61-
logger.error("Unabled to construct keystore for domain: " + domainSuffix);
61+
if (ks != null) {
62+
return new Pair<>(true, errMsg);
63+
}
64+
errMsg = String.format("Unable to construct keystore for domain: %s", domainSuffix);
65+
logger.error(errMsg);
6266
} catch (Exception e) {
63-
logger.error("Certificate validation failed due to exception for domain: " + domainSuffix, e);
67+
errMsg = String.format("Certificate validation failed due to exception for domain: %s", domainSuffix);
68+
logger.error(errMsg, e);
6469
}
65-
return false;
70+
return new Pair<>(false, errMsg);
6671
}
6772

6873
@Override

server/src/main/java/com/cloud/server/ManagementServerImpl.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.cloud.server;
1818

1919
import java.lang.reflect.Field;
20+
import java.security.cert.CertificateException;
2021
import java.util.ArrayList;
2122
import java.util.Arrays;
2223
import java.util.Calendar;
@@ -43,6 +44,7 @@
4344
import javax.inject.Inject;
4445
import javax.naming.ConfigurationException;
4546

47+
import com.cloud.utils.security.CertificateHelper;
4648
import org.apache.cloudstack.acl.ControlledEntity;
4749
import org.apache.cloudstack.acl.SecurityChecker;
4850
import org.apache.cloudstack.affinity.AffinityGroupProcessor;
@@ -4657,13 +4659,12 @@ public String uploadCertificate(final UploadCustomCertificateCmd cmd) {
46574659

46584660
final String certificate = cmd.getCertificate();
46594661
final String key = cmd.getPrivateKey();
4662+
String domainSuffix = cmd.getDomainSuffix();
46604663

4661-
if (cmd.getPrivateKey() != null && !_ksMgr.validateCertificate(certificate, key, cmd.getDomainSuffix())) {
4662-
throw new InvalidParameterValueException("Failed to pass certificate validation check");
4663-
}
4664+
validateCertificate(certificate, key, domainSuffix);
46644665

46654666
if (cmd.getPrivateKey() != null) {
4666-
_ksMgr.saveCertificate(ConsoleProxyManager.CERTIFICATE_NAME, certificate, key, cmd.getDomainSuffix());
4667+
_ksMgr.saveCertificate(ConsoleProxyManager.CERTIFICATE_NAME, certificate, key, domainSuffix);
46674668

46684669
// Reboot ssv m here since private key is present - meaning server cert being passed
46694670
final List<SecondaryStorageVmVO> alreadyRunning = _secStorageVmDao.getSecStorageVmListInStates(null, State.Running, State.Migrating, State.Starting);
@@ -4680,6 +4681,24 @@ public String uploadCertificate(final UploadCustomCertificateCmd cmd) {
46804681
+ "please give a few minutes for console access and storage services service to be up and working again";
46814682
}
46824683

4684+
private void validateCertificate(String certificate, String key, String domainSuffix) {
4685+
if (key != null) {
4686+
Pair<Boolean, String> result = _ksMgr.validateCertificate(certificate, key, domainSuffix);
4687+
if (!result.first()) {
4688+
throw new InvalidParameterValueException(String.format("Failed to pass certificate validation check with error: %s", result.second()));
4689+
}
4690+
} else {
4691+
try {
4692+
s_logger.debug(String.format("Trying to validate the root certificate format"));
4693+
CertificateHelper.buildCertificate(certificate);
4694+
} catch (CertificateException e) {
4695+
String errorMsg = String.format("Failed to pass certificate validation check with error: Certificate validation failed due to exception: %s", e.getMessage());
4696+
s_logger.error(errorMsg);
4697+
throw new InvalidParameterValueException(errorMsg);
4698+
}
4699+
}
4700+
}
4701+
46834702
@Override
46844703
public List<String> getHypervisors(final Long zoneId) {
46854704
final List<String> result = new ArrayList<>();

0 commit comments

Comments
 (0)