From 58ca9519ad3b4f12aacfb301c976dd8ae3317151 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Dec 2024 10:53:00 -0500 Subject: [PATCH] Only check validate of certs in the chain of the node certificates Signed-off-by: Craig Perkins --- .../security/ssl/SslConfiguration.java | 30 +++++---- .../ssl/config/KeyStoreConfiguration.java | 11 ++++ .../security/ssl/config/KeyStoreUtils.java | 61 ++++++++++++++++--- .../ssl/config/TrustStoreConfiguration.java | 7 ++- .../ssl/config/SslCertificatesLoaderTest.java | 3 +- 5 files changed, 87 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/opensearch/security/ssl/SslConfiguration.java b/src/main/java/org/opensearch/security/ssl/SslConfiguration.java index cacc887b5e..e74bc28038 100644 --- a/src/main/java/org/opensearch/security/ssl/SslConfiguration.java +++ b/src/main/java/org/opensearch/security/ssl/SslConfiguration.java @@ -17,9 +17,11 @@ import java.security.PrivilegedExceptionAction; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; +import javax.net.ssl.KeyManagerFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -74,10 +76,10 @@ public SslParameters sslParameters() { @SuppressWarnings("removal") SslContext buildServerSslContext(final boolean validateCertificates) { try { - return AccessController.doPrivileged( - (PrivilegedExceptionAction) () -> SslContextBuilder.forServer( - keyStoreConfiguration.createKeyManagerFactory(validateCertificates) - ) + return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + KeyManagerFactory kmFactory = keyStoreConfiguration.createKeyManagerFactory(validateCertificates); + Set issuerDns = keyStoreConfiguration.getIssuerDns(); + return SslContextBuilder.forServer(kmFactory) .sslProvider(sslParameters.provider()) .clientAuth(sslParameters.clientAuth()) .protocols(sslParameters.allowedProtocols().toArray(new String[0])) @@ -102,9 +104,9 @@ SslContext buildServerSslContext(final boolean validateCertificates) { ApplicationProtocolNames.HTTP_1_1 ) ) - .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates)) - .build() - ); + .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates, issuerDns)) + .build(); + }); } catch (PrivilegedActionException e) { throw new OpenSearchException("Failed to build server SSL context", e); } @@ -113,8 +115,10 @@ SslContext buildServerSslContext(final boolean validateCertificates) { @SuppressWarnings("removal") SslContext buildClientSslContext(final boolean validateCertificates) { try { - return AccessController.doPrivileged( - (PrivilegedExceptionAction) () -> SslContextBuilder.forClient() + return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + KeyManagerFactory kmFactory = keyStoreConfiguration.createKeyManagerFactory(validateCertificates); + Set issuerDns = keyStoreConfiguration.getIssuerDns(); + return SslContextBuilder.forClient() .sslProvider(sslParameters.provider()) .protocols(sslParameters.allowedProtocols()) .ciphers(sslParameters.allowedCiphers()) @@ -122,10 +126,10 @@ SslContext buildClientSslContext(final boolean validateCertificates) { .sessionCacheSize(0) .sessionTimeout(0) .sslProvider(sslParameters.provider()) - .keyManager(keyStoreConfiguration.createKeyManagerFactory(validateCertificates)) - .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates)) - .build() - ); + .keyManager(kmFactory) + .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates, issuerDns)) + .build(); + }); } catch (PrivilegedActionException e) { throw new OpenSearchException("Failed to build client SSL context", e); } diff --git a/src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java b/src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java index b1675f093a..4a7896186b 100644 --- a/src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java +++ b/src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java @@ -18,8 +18,10 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.Set; import javax.net.ssl.KeyManagerFactory; import com.google.common.collect.ImmutableList; @@ -41,6 +43,15 @@ default KeyManagerFactory createKeyManagerFactory(boolean validateCertificates) return buildKeyManagerFactory(keyStore.v1(), keyStore.v2()); } + default Set getIssuerDns() { + Set issuerDns = new HashSet<>(); + final List certificates = loadCertificates(); + for (Certificate certificate : certificates) { + issuerDns.add(certificate.x509Certificate().getIssuerX500Principal().getName()); + } + return issuerDns; + } + default KeyManagerFactory buildKeyManagerFactory(final KeyStore keyStore, final char[] password) { try { final var keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); diff --git a/src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java b/src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java index 7c063bd312..c0833d25ca 100644 --- a/src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java +++ b/src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java @@ -24,11 +24,16 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; +import java.util.Arrays; import java.util.List; +import java.util.Set; import javax.crypto.NoSuchPaddingException; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLSessionContext; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import org.opensearch.OpenSearchException; import io.netty.buffer.ByteBufAllocator; @@ -37,6 +42,8 @@ final class KeyStoreUtils { + private final static Logger log = LogManager.getLogger(KeyStoreUtils.class); + private final static class SecuritySslContext extends SslContext { private SecuritySslContext() {} @@ -141,15 +148,10 @@ public static void validateKeyStoreCertificates(final KeyStore keyStore) { final var a = aliases.nextElement(); if (keyStore.isCertificateEntry(a)) { final var c = (X509Certificate) keyStore.getCertificate(a); - if (c == null) { - throw new CertificateException("Alias " + a + " does not contain a certificate entry"); - } c.checkValidity(); - } else if (keyStore.isKeyEntry(a)) { - final var cc = keyStore.getCertificateChain(a); - if (cc == null) { - throw new CertificateException("Alias " + a + " does not contain a certificate chain"); - } + } + final var cc = keyStore.getCertificateChain(a); + if (cc != null) { for (final var c : cc) { ((X509Certificate) c).checkValidity(); } @@ -162,6 +164,49 @@ public static void validateKeyStoreCertificates(final KeyStore keyStore) { } } + // If dnsToCheck is present, this method will only validate the validate for certificates that match the dns in this list or + // up the chain + public static void validateKeyStoreCertificates(final KeyStore keyStore, Set dnsToCheck) { + try { + final var aliases = keyStore.aliases(); + while (aliases.hasMoreElements()) { + final var a = aliases.nextElement(); + if (keyStore.isCertificateEntry(a)) { + final var c = (X509Certificate) keyStore.getCertificate(a); + if (dnsToCheck.contains(c.getSubjectX500Principal().getName())) { + c.checkValidity(); + final var cc = keyStore.getCertificateChain(a); + if (cc != null) { + for (final var c1 : cc) { + ((X509Certificate) c1).checkValidity(); + } + } + } else { + log.info("Skipping validation for " + c.getSubjectX500Principal().getName()); + } + } else { + if (keyStore.isCertificateEntry(a)) { + final var c = (X509Certificate) keyStore.getCertificate(a); + c.checkValidity(); + } + final var cc = keyStore.getCertificateChain(a); + if (cc != null) { + if (Arrays.stream(cc) + .anyMatch(c -> dnsToCheck.contains(((X509Certificate) c).getSubjectX500Principal().getName()))) { + for (final var c : cc) { + ((X509Certificate) c).checkValidity(); + } + } + } + } + } + } catch (KeyStoreException e) { + throw new OpenSearchException("Couldn't load keys store", e); + } catch (CertificateException e) { + throw new OpenSearchException("Invalid certificates", e); + } + } + public static KeyStore loadKeyStore(final Path path, final String type, final char[] password) { try { final var keyStore = KeyStore.getInstance(type); diff --git a/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java b/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java index 4965aa3216..9850334105 100644 --- a/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java +++ b/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.net.ssl.TrustManagerFactory; @@ -46,7 +47,7 @@ public KeyStore createTrustStore() { } @Override - public TrustManagerFactory createTrustManagerFactory(boolean validateCertificates) { + public TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set issuerDns) { return null; } }; @@ -55,10 +56,10 @@ public TrustManagerFactory createTrustManagerFactory(boolean validateCertificate List loadCertificates(); - default TrustManagerFactory createTrustManagerFactory(boolean validateCertificates) { + default TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set issuerDns) { final var trustStore = createTrustStore(); if (validateCertificates) { - KeyStoreUtils.validateKeyStoreCertificates(trustStore); + KeyStoreUtils.validateKeyStoreCertificates(trustStore, issuerDns); } return buildTrustManagerFactory(trustStore); } diff --git a/src/test/java/org/opensearch/security/ssl/config/SslCertificatesLoaderTest.java b/src/test/java/org/opensearch/security/ssl/config/SslCertificatesLoaderTest.java index 0dfc02b386..6d2ff96fec 100644 --- a/src/test/java/org/opensearch/security/ssl/config/SslCertificatesLoaderTest.java +++ b/src/test/java/org/opensearch/security/ssl/config/SslCertificatesLoaderTest.java @@ -13,6 +13,7 @@ import java.nio.file.Path; import java.util.List; +import java.util.Set; import com.carrotsearch.randomizedtesting.RandomizedTest; import org.junit.ClassRule; @@ -49,7 +50,7 @@ void assertTrustStoreConfiguration( assertThat("Truststore configuration created", nonNull(trustStoreConfiguration)); assertThat(trustStoreConfiguration.file(), is(expectedFile)); assertThat(trustStoreConfiguration.loadCertificates(), containsInAnyOrder(expectedCertificates)); - assertThat(trustStoreConfiguration.createTrustManagerFactory(true), is(notNullValue())); + assertThat(trustStoreConfiguration.createTrustManagerFactory(true, Set.of()), is(notNullValue())); } void assertKeyStoreConfiguration(