Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only check validity of certs in the chain of the node certificates #38

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Only check validity of certs in the chain of the node certificates #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions src/main/java/org/opensearch/security/ssl/SslConfiguration.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading