From 217fdd5168b85616a38fe7cfa94a855056ad6381 Mon Sep 17 00:00:00 2001 From: vishesh92 Date: Tue, 12 May 2026 18:31:45 +0530 Subject: [PATCH] Fix issues due to malformed or bad input --- .../ca/provider/RootCAProvider.java | 87 ++++++++++++++----- .../ca/provider/RootCAProviderTest.java | 64 ++++++++++++-- 2 files changed, 123 insertions(+), 28 deletions(-) diff --git a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java index 32c601386ac..44cbc00497c 100644 --- a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java +++ b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java @@ -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 rootCAPrivateKey = new ConfigKey<>("Hidden", String.class, - "ca.plugin.root.private.key", - null, + private static ConfigKey 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 rootCAPublicKey = new ConfigKey<>("Hidden", String.class, - "ca.plugin.root.public.key", - null, + private static ConfigKey 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 rootCACertificate = new ConfigKey<>("Hidden", String.class, - "ca.plugin.root.ca.certificate", - null, + private static ConfigKey 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 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 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 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()) { diff --git a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java index 7ec2ce9a2f4..21f00c66a1d 100644 --- a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java +++ b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java @@ -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> 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> 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 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 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 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")); + } + } }