Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
35 changes: 21 additions & 14 deletions src/main/java/org/opensearch/security/ssl/SslConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
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 javax.net.ssl.TrustManagerFactory;
import javax.security.auth.x500.X500Principal;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -74,7 +76,10 @@
}

public TrustManagerFactory trustStoreFactory() {
return trustStoreConfiguration.createTrustManagerFactory(sslParameters.shouldValidateNewCertDNs());
return trustStoreConfiguration.createTrustManagerFactory(
sslParameters.shouldValidateNewCertDNs(),
keyStoreConfiguration.getIssuerDns()

Check warning on line 81 in src/main/java/org/opensearch/security/ssl/SslConfiguration.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/SslConfiguration.java#L79-L81

Added lines #L79 - L81 were not covered by tests
);
}

public SslParameters sslParameters() {
Expand All @@ -84,10 +89,10 @@
@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<X500Principal> issuerDns = keyStoreConfiguration.getIssuerDns();
return SslContextBuilder.forServer(kmFactory)
.sslProvider(sslParameters.provider())
.clientAuth(sslParameters.clientAuth())
.protocols(sslParameters.allowedProtocols().toArray(new String[0]))
Expand All @@ -112,9 +117,9 @@
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 @@ -123,19 +128,21 @@
@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<X500Principal> 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
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
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 javax.security.auth.x500.X500Principal;

import com.google.common.collect.ImmutableList;

Expand All @@ -41,6 +44,15 @@ default KeyManagerFactory createKeyManagerFactory(boolean validateCertificates)
return buildKeyManagerFactory(keyStore.v1(), keyStore.v2());
}

default Set<X500Principal> getIssuerDns() {
Set<X500Principal> issuerDns = new HashSet<>();
final List<Certificate> certificates = loadCertificates();
for (Certificate certificate : certificates) {
issuerDns.add(certificate.x509Certificate().getIssuerX500Principal());
}
return issuerDns;
}

default KeyManagerFactory buildKeyManagerFactory(final KeyStore keyStore, final char[] password) {
try {
final var keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +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 javax.security.auth.x500.X500Principal;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.OpenSearchException;

Expand All @@ -37,6 +43,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 +149,10 @@
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 +165,48 @@
}
}

// 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<X500Principal> 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())) {
c.checkValidity();
final var cc = keyStore.getCertificateChain(a);
if (cc != null) {
for (final var c1 : cc) {
((X509Certificate) c1).checkValidity();

Check warning on line 182 in src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java#L182

Added line #L182 was not covered by tests
}
}
} else {
log.info("Skipping validation for " + c.getSubjectX500Principal().getName());
}
} else {
if (keyStore.isCertificateEntry(a)) {
final var c = (X509Certificate) keyStore.getCertificate(a);
c.checkValidity();

Check warning on line 191 in src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java#L190-L191

Added lines #L190 - L191 were not covered by tests
}
final var cc = keyStore.getCertificateChain(a);

Check warning on line 193 in src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java#L193

Added line #L193 was not covered by tests
if (cc != null) {
if (Arrays.stream(cc).anyMatch(c -> dnsToCheck.contains(((X509Certificate) c).getSubjectX500Principal()))) {
for (final var c : cc) {
((X509Certificate) c).checkValidity();

Check warning on line 197 in src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java#L197

Added line #L197 was not covered by tests
}
}
}
}
}
} catch (KeyStoreException e) {
throw new OpenSearchException("Couldn't load keys store", e);
} catch (CertificateException e) {
throw new OpenSearchException("Invalid certificates", e);

Check warning on line 206 in src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java#L203-L206

Added lines #L203 - L206 were not covered by tests
}
}

public static KeyStore loadKeyStore(final Path path, final String type, final char[] password) {
try {
final var keyStore = KeyStore.getInstance(type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
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;
import javax.security.auth.x500.X500Principal;

import com.google.common.collect.ImmutableList;

Expand All @@ -46,7 +48,7 @@ public KeyStore createTrustStore() {
}

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

List<Certificate> loadCertificates();

default TrustManagerFactory createTrustManagerFactory(boolean validateCertificates) {
default TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set<X500Principal> issuerDns) {
final var trustStore = createTrustStore();
if (validateCertificates) {
KeyStoreUtils.validateKeyStoreCertificates(trustStore);
KeyStoreUtils.validateKeyStoreCertificates(trustStore, issuerDns);
}
return buildTrustManagerFactory(trustStore);
}
Expand Down
80 changes: 80 additions & 0 deletions src/test/java/org/opensearch/security/ssl/SSLTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,86 @@ public void testHttps() throws Exception {

}

@Test
public void testHttpsWithTrustStoreContainingValidCertsNotInChain() throws Exception {

final Settings settings = Settings.builder()
.put(ConfigConstants.SECURITY_SSL_ONLY, true)
.put(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true)
.put(
SSLConfigConstants.SECURITY_SSL_HTTP_KEYSTORE_FILEPATH,
FileHelper.getAbsoluteFilePathFromClassPath("ssl/node-0-keystore.jks")
)
.put(
SSLConfigConstants.SECURITY_SSL_HTTP_TRUSTSTORE_FILEPATH,
FileHelper.getAbsoluteFilePathFromClassPath("ssl/truststore_valid.jks")
)
.build();

setupSslOnlyMode(settings);

RestHelper rh = restHelper();
rh.enableHTTPClientSSL = true;
rh.trustHTTPServerCertificate = true;
rh.sendAdminCertificate = true;
rh.keystore = "node-untspec5-keystore.p12";

String res = rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty&show_dn=true");
Assert.assertTrue(res.contains("[email protected]"));
Assert.assertTrue(res.contains("local_certificates_list"));
Assert.assertFalse(
rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty&show_dn=false").contains("local_certificates_list")
);
Assert.assertFalse(rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty").contains("local_certificates_list"));

res = rh.executeSimpleRequest("_nodes/settings?pretty");
Assert.assertTrue(res.contains(clusterInfo.clustername));
Assert.assertFalse(res.contains("\"opendistro_security\""));
Assert.assertFalse(res.contains("keystore_filepath"));
// Assert.assertTrue(rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty").contains("CN=node-0.example.com,OU=SSL,O=Test,L=Test,C=DE"));

}

@Test
public void testHttpsWithTrustStoreContainingInvalidCertsNotInChain() throws Exception {

final Settings settings = Settings.builder()
.put(ConfigConstants.SECURITY_SSL_ONLY, true)
.put(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true)
.put(
SSLConfigConstants.SECURITY_SSL_HTTP_KEYSTORE_FILEPATH,
FileHelper.getAbsoluteFilePathFromClassPath("ssl/node-0-keystore.jks")
)
.put(
SSLConfigConstants.SECURITY_SSL_HTTP_TRUSTSTORE_FILEPATH,
FileHelper.getAbsoluteFilePathFromClassPath("ssl/truststore_invalid.jks")
)
.build();

setupSslOnlyMode(settings);

RestHelper rh = restHelper();
rh.enableHTTPClientSSL = true;
rh.trustHTTPServerCertificate = true;
rh.sendAdminCertificate = true;
rh.keystore = "node-untspec5-keystore.p12";

String res = rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty&show_dn=true");
Assert.assertTrue(res.contains("[email protected]"));
Assert.assertTrue(res.contains("local_certificates_list"));
Assert.assertFalse(
rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty&show_dn=false").contains("local_certificates_list")
);
Assert.assertFalse(rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty").contains("local_certificates_list"));

res = rh.executeSimpleRequest("_nodes/settings?pretty");
Assert.assertTrue(res.contains(clusterInfo.clustername));
Assert.assertFalse(res.contains("\"opendistro_security\""));
Assert.assertFalse(res.contains("keystore_filepath"));
// Assert.assertTrue(rh.executeSimpleRequest("_opendistro/_security/sslinfo?pretty").contains("CN=node-0.example.com,OU=SSL,O=Test,L=Test,C=DE"));

}

@Test
public void testCipherAndProtocols() throws Exception {

Expand Down
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
28 changes: 28 additions & 0 deletions src/test/resources/ssl/root-ca-invalid.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN CERTIFICATE-----
MIIE2DCCA8CgAwIBAgIUI6tTo8QuZUoMod2uJWb587o/WSQwDQYJKoZIhvcNAQEL
BQAwgZUxEzARBgoJkiaJk/IsZAEZFgNjb20xFzAVBgoJkiaJk/IsZAEZFgdleGFt
cGxlMRkwFwYDVQQKDBBFeGFtcGxlIENvbSBJbmMuMSQwIgYDVQQLDBtFeGFtcGxl
IENvbSBJbmMuIEludmFsaWQgQ0ExJDAiBgNVBAMMG0V4YW1wbGUgQ29tIEluYy4g
SW52YWxpZCBDQTAeFw0yNTA0MDgxOTAwMzNaFw0yNTA0MDkxOTAwMzNaMIGVMRMw
EQYKCZImiZPyLGQBGRYDY29tMRcwFQYKCZImiZPyLGQBGRYHZXhhbXBsZTEZMBcG
A1UECgwQRXhhbXBsZSBDb20gSW5jLjEkMCIGA1UECwwbRXhhbXBsZSBDb20gSW5j
LiBJbnZhbGlkIENBMSQwIgYDVQQDDBtFeGFtcGxlIENvbSBJbmMuIEludmFsaWQg
Q0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCmFZGps52e1aA0bmAo
Gy8cIT5ZbqZlBK9jEAXj8m4snFPpS87MtEN5vDA3OLsB44tt3mRLKc/GkvG4v4yl
MDILNSarVWME53fGzoyc0i0gd/VZxPwxR0e+LphnmCbec44GzfN3dOxCdqZ3AnZA
9JCsogf36omgb6hYXsi6FsPkB0YxnUHviiYVtxPcV/Q11cGLq55kwm5INSnGuUxX
NEz0RRj9Hm+4PEGnGBRyWOuAAxQYeexXGHpXDy2II8X7uyCb1gbd3zI+hlM9hgeA
bMUaR4P42INB6SJkk/nzHg+g3ssZ+N3ZFN9QxDCG+v+LA6p6oRNL1J6iICkN6vfi
ZuVdAgMBAAGjggEcMIIBGDAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIB
hjAdBgNVHQ4EFgQUA+uQ1+AW5h2XztdP2wbwhix8XWgwgdUGA1UdIwSBzTCByoAU
A+uQ1+AW5h2XztdP2wbwhix8XWihgZukgZgwgZUxEzARBgoJkiaJk/IsZAEZFgNj
b20xFzAVBgoJkiaJk/IsZAEZFgdleGFtcGxlMRkwFwYDVQQKDBBFeGFtcGxlIENv
bSBJbmMuMSQwIgYDVQQLDBtFeGFtcGxlIENvbSBJbmMuIEludmFsaWQgQ0ExJDAi
BgNVBAMMG0V4YW1wbGUgQ29tIEluYy4gSW52YWxpZCBDQYIUI6tTo8QuZUoMod2u
JWb587o/WSQwDQYJKoZIhvcNAQELBQADggEBAIxYxDaCwhovQmvSkK/E8BqwSuG5
yKbfo5BHYlhNbhmBzOvXkoF9jka2TqKRZceWCq4MTztKLQrjpm3jnKAGxNnhTXqL
/edEE3RJ13EuzJyk13Chem8asgoZlXwjJwSFqWJS30P75HTuVwFwMFNLLtbCGvKb
oobDQMM6lHtlnvagUrAQG9sT0wPO43sfRzEabWs2CvMnbMZpJ7mn7zDmiIU1z+ub
rXCG2eSDILaFB0FACkRRdpm0SStF2ftPua3HSKKGh99TjGECxmpcQoO0kKp+fsdA
SdyR6KGYHKFRftw0eMZ6RsOwF1BhGq8nT3UlEJTpaBJSAkzzgbMeUFkLRGk=
-----END CERTIFICATE-----
Loading
Loading