mirror of https://github.com/apache/cloudstack.git
Fix issues due to malformed or bad input
This commit is contained in:
parent
5d8e79710f
commit
217fdd5168
|
|
@ -59,6 +59,7 @@ import org.apache.cloudstack.framework.ca.CAProvider;
|
|||
import org.apache.cloudstack.framework.ca.Certificate;
|
||||
import org.apache.cloudstack.framework.config.ConfigKey;
|
||||
import org.apache.cloudstack.framework.config.Configurable;
|
||||
import org.apache.cloudstack.framework.config.ValidatedConfigKey;
|
||||
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
|
||||
import org.apache.cloudstack.utils.security.CertUtils;
|
||||
import org.apache.cloudstack.utils.security.KeyStoreUtils;
|
||||
|
|
@ -103,25 +104,25 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con
|
|||
/////////////// Root CA Settings ///////////////////
|
||||
////////////////////////////////////////////////////
|
||||
|
||||
private static ConfigKey<String> rootCAPrivateKey = new ConfigKey<>("Hidden", String.class,
|
||||
"ca.plugin.root.private.key",
|
||||
null,
|
||||
private static ConfigKey<String> rootCAPrivateKey = new ValidatedConfigKey<>("Hidden", String.class,
|
||||
"ca.plugin.root.private.key", null,
|
||||
"The ROOT CA private key in PEM format. " +
|
||||
"When set along with the public key and certificate, CloudStack uses this custom CA instead of auto-generating one. " +
|
||||
"All three ca.plugin.root.* keys must be set together. Restart management server(s) when changed.", false);
|
||||
"All three ca.plugin.root.* keys must be set together. Restart management server(s) when changed.",
|
||||
false, ConfigKey.Scope.Global, null, RootCAProvider::validatePrivateKeyPem);
|
||||
|
||||
private static ConfigKey<String> rootCAPublicKey = new ConfigKey<>("Hidden", String.class,
|
||||
"ca.plugin.root.public.key",
|
||||
null,
|
||||
private static ConfigKey<String> rootCAPublicKey = new ValidatedConfigKey<>("Hidden", String.class,
|
||||
"ca.plugin.root.public.key", null,
|
||||
"The ROOT CA public key in PEM format (X.509/SPKI: must start with '-----BEGIN PUBLIC KEY-----'). " +
|
||||
"Required when providing a custom CA. Restart management server(s) when changed.", false);
|
||||
"Required when providing a custom CA. Restart management server(s) when changed.",
|
||||
false, ConfigKey.Scope.Global, null, RootCAProvider::validatePublicKeyPem);
|
||||
|
||||
private static ConfigKey<String> rootCACertificate = new ConfigKey<>("Hidden", String.class,
|
||||
"ca.plugin.root.ca.certificate",
|
||||
null,
|
||||
private static ConfigKey<String> rootCACertificate = new ValidatedConfigKey<>("Hidden", String.class,
|
||||
"ca.plugin.root.ca.certificate", null,
|
||||
"The CA certificate(s) in PEM format (must start with '-----BEGIN CERTIFICATE-----'). " +
|
||||
"For intermediate CAs, concatenate the signing cert first, followed by intermediate(s) and root. " +
|
||||
"Required when providing a custom CA. Restart management server(s) when changed.", false);
|
||||
"Required when providing a custom CA. Restart management server(s) when changed.",
|
||||
false, ConfigKey.Scope.Global, null, RootCAProvider::validateCACertificatePem);
|
||||
|
||||
private static ConfigKey<String> rootCAIssuerDN = new ConfigKey<>("Advanced", String.class,
|
||||
"ca.plugin.root.issuer.dn",
|
||||
|
|
@ -327,24 +328,26 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con
|
|||
return loadRootCAKeyPair();
|
||||
}
|
||||
|
||||
private boolean saveNewRootCACertificate() {
|
||||
boolean saveNewRootCACertificate() {
|
||||
if (caKeyPair == null) {
|
||||
throw new CloudRuntimeException("Cannot issue self-signed root CA certificate as CA keypair is not initialized");
|
||||
}
|
||||
try {
|
||||
logger.debug("Generating root CA certificate");
|
||||
final X509Certificate rootCaCertificate = CertUtils.generateV3Certificate(
|
||||
final X509Certificate generatedCACert = CertUtils.generateV3Certificate(
|
||||
null, caKeyPair, caKeyPair.getPublic(),
|
||||
rootCAIssuerDN.value(), CAManager.CertSignatureAlgorithm.value(),
|
||||
getCaValidityDays(), null, null);
|
||||
if (!configDao.update(rootCACertificate.key(), rootCACertificate.category(), CertUtils.x509CertificateToPem(rootCaCertificate))) {
|
||||
if (!configDao.update(rootCACertificate.key(), rootCACertificate.category(), CertUtils.x509CertificateToPem(generatedCACert))) {
|
||||
logger.error("Failed to update RootCA public/x509 certificate");
|
||||
}
|
||||
caCertificates = new ArrayList<>(java.util.Collections.singletonList(generatedCACert));
|
||||
caCertificate = generatedCACert;
|
||||
} catch (final CertificateException | NoSuchAlgorithmException | NoSuchProviderException | SignatureException | InvalidKeyException | OperatorCreationException | IOException e) {
|
||||
logger.error("Failed to generate RootCA certificate from private/public keys due to exception:", e);
|
||||
return false;
|
||||
}
|
||||
return loadRootCACertificate();
|
||||
return caCertificate != null;
|
||||
}
|
||||
|
||||
private boolean loadRootCAKeyPair() {
|
||||
|
|
@ -360,27 +363,31 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con
|
|||
return caKeyPair.getPrivate() != null && caKeyPair.getPublic() != null;
|
||||
}
|
||||
|
||||
private boolean loadRootCACertificate() {
|
||||
boolean loadRootCACertificate() {
|
||||
caCertificate = null;
|
||||
caCertificates = null;
|
||||
if (StringUtils.isEmpty(rootCACertificate.value())) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
caCertificates = CertUtils.pemToX509Certificates(rootCACertificate.value());
|
||||
if (CollectionUtils.isEmpty(caCertificates)) {
|
||||
final List<X509Certificate> loadedCerts = CertUtils.pemToX509Certificates(rootCACertificate.value());
|
||||
if (CollectionUtils.isEmpty(loadedCerts)) {
|
||||
logger.error("No certificates found in ca.plugin.root.ca.certificate");
|
||||
return false;
|
||||
}
|
||||
caCertificate = caCertificates.get(0);
|
||||
final X509Certificate loadedCACert = loadedCerts.get(0);
|
||||
|
||||
// Verify key ownership without enforcing self-signature
|
||||
if (!caCertificate.getPublicKey().equals(caKeyPair.getPublic())) {
|
||||
if (!loadedCACert.getPublicKey().equals(caKeyPair.getPublic())) {
|
||||
logger.error("The public key in the CA certificate does not match the configured CA public key");
|
||||
return false;
|
||||
}
|
||||
|
||||
if (caCertificates.size() > 1) {
|
||||
logger.info("Loaded CA certificate chain with {} certificate(s)", caCertificates.size());
|
||||
if (loadedCerts.size() > 1) {
|
||||
logger.info("Loaded CA certificate chain with {} certificate(s)", loadedCerts.size());
|
||||
}
|
||||
caCertificates = loadedCerts;
|
||||
caCertificate = loadedCACert;
|
||||
} catch (final IOException | CertificateException e) {
|
||||
logger.error("Failed to load saved RootCA certificate due to exception:", e);
|
||||
return false;
|
||||
|
|
@ -446,6 +453,40 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con
|
|||
}
|
||||
|
||||
|
||||
private static void validatePrivateKeyPem(String value) {
|
||||
if (StringUtils.isEmpty(value)) return;
|
||||
try {
|
||||
CertUtils.pemToPrivateKey(value);
|
||||
} catch (InvalidKeySpecException | IOException e) {
|
||||
throw new IllegalArgumentException(
|
||||
"ca.plugin.root.private.key is not a valid PEM private key: " + e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
private static void validatePublicKeyPem(String value) {
|
||||
if (StringUtils.isEmpty(value)) return;
|
||||
try {
|
||||
CertUtils.pemToPublicKey(value);
|
||||
} catch (InvalidKeySpecException | IOException e) {
|
||||
throw new IllegalArgumentException(
|
||||
"ca.plugin.root.public.key is not a valid PEM public key: " + e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
static void validateCACertificatePem(String value) {
|
||||
if (StringUtils.isEmpty(value)) return;
|
||||
try {
|
||||
final List<X509Certificate> certs = CertUtils.pemToX509Certificates(value);
|
||||
if (CollectionUtils.isEmpty(certs)) {
|
||||
throw new IllegalArgumentException(
|
||||
"ca.plugin.root.ca.certificate contains no certificates");
|
||||
}
|
||||
} catch (IOException | CertificateException e) {
|
||||
throw new IllegalArgumentException(
|
||||
"ca.plugin.root.ca.certificate is not a valid PEM certificate: " + e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
private boolean setupCA() {
|
||||
if (!loadRootCAKeyPair()) {
|
||||
if (hasUserProvidedCAKeys()) {
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ import javax.net.ssl.SSLEngine;
|
|||
|
||||
import org.apache.cloudstack.framework.ca.Certificate;
|
||||
import org.apache.cloudstack.framework.config.ConfigKey;
|
||||
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
|
||||
import org.apache.cloudstack.utils.security.CertUtils;
|
||||
import org.apache.cloudstack.utils.security.SSLUtils;
|
||||
import org.bouncycastle.asn1.x509.GeneralName;
|
||||
|
|
@ -50,7 +51,6 @@ import org.junit.Test;
|
|||
import org.junit.runner.RunWith;
|
||||
import org.mockito.Mockito;
|
||||
import org.mockito.junit.MockitoJUnitRunner;
|
||||
import org.springframework.test.util.ReflectionTestUtils;
|
||||
|
||||
|
||||
@RunWith(MockitoJUnitRunner.class)
|
||||
|
|
@ -218,8 +218,8 @@ public class RootCAProviderTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void testIsManagementCertificateNoMatch() {
|
||||
ReflectionTestUtils.setField(provider, "managementCertificateCustomSAN", "cloudstack");
|
||||
public void testIsManagementCertificateNoMatch() throws Exception {
|
||||
addField(provider, "managementCertificateCustomSAN", "cloudstack");
|
||||
try {
|
||||
X509Certificate certificate = Mockito.mock(X509Certificate.class);
|
||||
List<List<?>> altNames = new ArrayList<>();
|
||||
|
|
@ -234,9 +234,9 @@ public class RootCAProviderTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void testIsManagementCertificateMatch() {
|
||||
public void testIsManagementCertificateMatch() throws Exception {
|
||||
String customSAN = "cloudstack";
|
||||
ReflectionTestUtils.setField(provider, "managementCertificateCustomSAN", customSAN);
|
||||
addField(provider, "managementCertificateCustomSAN", customSAN);
|
||||
try {
|
||||
X509Certificate certificate = Mockito.mock(X509Certificate.class);
|
||||
List<List<?>> altNames = new ArrayList<>();
|
||||
|
|
@ -249,4 +249,58 @@ public class RootCAProviderTest {
|
|||
Assert.fail(String.format("Exception occurred: %s", e.getMessage()));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadRootCACertificateWithMismatchedCert() throws Exception {
|
||||
KeyPair otherKeyPair = CertUtils.generateRandomKeyPair(1024);
|
||||
X509Certificate mismatchedCert = CertUtils.generateV3Certificate(null, otherKeyPair, otherKeyPair.getPublic(), "CN=other", "SHA256withRSA", 365, null, null);
|
||||
String mismatchedPem = CertUtils.x509CertificateToPem(mismatchedCert);
|
||||
|
||||
ConfigKey<String> mockCertKey = Mockito.mock(ConfigKey.class);
|
||||
Mockito.when(mockCertKey.value()).thenReturn(mismatchedPem);
|
||||
addField(provider, "rootCACertificate", mockCertKey);
|
||||
|
||||
addField(provider, "caCertificate", null);
|
||||
addField(provider, "caCertificates", null);
|
||||
|
||||
Boolean result = provider.loadRootCACertificate();
|
||||
Assert.assertFalse(result);
|
||||
Assert.assertNull(provider.getCaCertificate());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSaveNewRootCACertificateWithStaleCache() throws Exception {
|
||||
ConfigurationDao configDao = Mockito.mock(ConfigurationDao.class);
|
||||
addField(provider, "configDao", configDao);
|
||||
|
||||
ConfigKey<String> mockCertKey = Mockito.mock(ConfigKey.class);
|
||||
Mockito.when(mockCertKey.key()).thenReturn("ca.plugin.root.ca.certificate");
|
||||
Mockito.when(mockCertKey.category()).thenReturn("Hidden");
|
||||
addField(provider, "rootCACertificate", mockCertKey);
|
||||
|
||||
ConfigKey<String> mockIssuerKey = Mockito.mock(ConfigKey.class);
|
||||
Mockito.when(mockIssuerKey.value()).thenReturn("CN=ca.cloudstack.apache.org");
|
||||
addField(provider, "rootCAIssuerDN", mockIssuerKey);
|
||||
|
||||
addField(provider, "caCertificate", null);
|
||||
addField(provider, "caCertificates", null);
|
||||
|
||||
Mockito.when(configDao.update(Mockito.anyString(), Mockito.anyString(), Mockito.anyString())).thenReturn(true);
|
||||
|
||||
Boolean result = provider.saveNewRootCACertificate();
|
||||
Assert.assertTrue(result);
|
||||
Assert.assertNotNull(provider.getCaCertificate());
|
||||
Assert.assertEquals(1, provider.getCaCertificate().size());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testValidateCACertificatePem() throws Exception {
|
||||
String truncatedPem = "-----BEGIN CERTIFICATE-----\nMIICxTCCAa0CAQAw\n";
|
||||
try {
|
||||
RootCAProvider.validateCACertificatePem(truncatedPem);
|
||||
Assert.fail("Expected IllegalArgumentException");
|
||||
} catch (IllegalArgumentException e) {
|
||||
Assert.assertTrue(e.getMessage().contains("is not a valid PEM certificate"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue