Skip to content

Commit

Permalink
Only check validate of certs in the chain of the node certificates
Browse files Browse the repository at this point in the history 
Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
@cwperks
cwperks committed Dec 20, 2024
1 parent 3c7404f commit 58ca951
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 25 deletions.
30 changes: 17 additions & 13 deletions src/main/java/org/opensearch/security/ssl/SslConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,10 +76,10 @@ public SslParameters sslParameters() {
@SuppressWarnings("removal")
SslContext buildServerSslContext(final boolean validateCertificates) {
try {
return AccessController.doPrivileged(
(PrivilegedExceptionAction<SslContext>) () -> SslContextBuilder.forServer(
keyStoreConfiguration.createKeyManagerFactory(validateCertificates)
)
return AccessController.doPrivileged((PrivilegedExceptionAction<SslContext>) () -> {
KeyManagerFactory kmFactory = keyStoreConfiguration.createKeyManagerFactory(validateCertificates);
Set<String> issuerDns = keyStoreConfiguration.getIssuerDns();
return SslContextBuilder.forServer(kmFactory)
.sslProvider(sslParameters.provider())
.clientAuth(sslParameters.clientAuth())
.protocols(sslParameters.allowedProtocols().toArray(new String[0]))
Expand All @@ -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);
}
Expand All @@ -113,19 +115,21 @@ SslContext buildServerSslContext(final boolean validateCertificates) {
@SuppressWarnings("removal")
SslContext buildClientSslContext(final boolean validateCertificates) {
try {
return AccessController.doPrivileged(
(PrivilegedExceptionAction<SslContext>) () -> SslContextBuilder.forClient()
return AccessController.doPrivileged((PrivilegedExceptionAction<SslContext>) () -> {
KeyManagerFactory kmFactory = keyStoreConfiguration.createKeyManagerFactory(validateCertificates);
Set<String> issuerDns = keyStoreConfiguration.getIssuerDns();
return SslContextBuilder.forClient()
.sslProvider(sslParameters.provider())
.protocols(sslParameters.allowedProtocols())
.ciphers(sslParameters.allowedCiphers())
.applicationProtocolConfig(ApplicationProtocolConfig.DISABLED)
.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);
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,6 +43,15 @@ default KeyManagerFactory createKeyManagerFactory(boolean validateCertificates)
return buildKeyManagerFactory(keyStore.v1(), keyStore.v2());
}

default Set<String> getIssuerDns() {
Set<String> issuerDns = new HashSet<>();
final List<Certificate> 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());
Expand Down
61 changes: 53 additions & 8 deletions src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {}
Expand Down Expand Up @@ -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();
}
Expand All @@ -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<String> 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);
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,7 +47,7 @@ public KeyStore createTrustStore() {
}

@Override
public TrustManagerFactory createTrustManagerFactory(boolean validateCertificates) {
public TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set<String> issuerDns) {
return null;
}
};
Expand All @@ -55,10 +56,10 @@ public TrustManagerFactory createTrustManagerFactory(boolean validateCertificate

List<Certificate> loadCertificates();

default TrustManagerFactory createTrustManagerFactory(boolean validateCertificates) {
default TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set<String> issuerDns) {
final var trustStore = createTrustStore();
if (validateCertificates) {
KeyStoreUtils.validateKeyStoreCertificates(trustStore);
KeyStoreUtils.validateKeyStoreCertificates(trustStore, issuerDns);
}
return buildTrustManagerFactory(trustStore);
}
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/org/opensearch/security/ssl/config/SslCertificatesLoaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 58ca951

Please sign in to comment.