diff --git a/build.gradle b/build.gradle index 9c18254218..d6486a910c 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,6 @@ import com.diffplug.gradle.spotless.JavaExtension -import org.opensearch.gradle.info.FipsBuildParams import org.opensearch.gradle.test.RestIntegTestTask import groovy.json.JsonBuilder @@ -77,6 +76,7 @@ apply plugin: 'opensearch.pluginzip' apply plugin: 'opensearch.rest-test' apply plugin: 'opensearch.testclusters' apply from: 'gradle/formatting.gradle' +apply from: 'gradle/fips.gradle' licenseFile = rootProject.file('LICENSE.txt') noticeFile = rootProject.file('NOTICE.txt') @@ -236,6 +236,7 @@ tasks.register("listTasksAsJSON") { } } + tasks.register('copyExtraTestResources', Copy) { dependsOn testClasses @@ -548,6 +549,9 @@ subprojects { configurations.integrationTestImplementation.extendsFrom configurations.implementation configurations.integrationTestRuntimeOnly.extendsFrom configurations.runtimeOnly } + tasks.withType(Test).configureEach { t -> + rootProject.ext.configureFipsJvmArgs(t) + } } } } @@ -593,6 +597,7 @@ allprojects { integrationTestImplementation "org.apache.logging.log4j:log4j-jul:${versions.log4j}" integrationTestImplementation 'org.hamcrest:hamcrest:2.2' integrationTestImplementation "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" + integrationTestImplementation "org.bouncycastle:bctls-fips:${versions.bouncycastle_tls}" integrationTestImplementation "org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}" integrationTestImplementation "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}" integrationTestImplementation('org.awaitility:awaitility:4.3.0') { @@ -697,16 +702,10 @@ dependencies { implementation libs.google.guava implementation 'org.greenrobot:eventbus-java:3.3.1' implementation 'commons-cli:commons-cli:1.11.0' - // When building with crypto.standard=FIPS-140-3 (set in gradle.properties), bcFips jars are provided by OpenSearch - if (FipsBuildParams.isInFipsMode()) { - compileOnly "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" - compileOnly "org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}" - compileOnly "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}" - } else { - implementation "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" - implementation "org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}" - implementation "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}" - } + compileOnly "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" + compileOnly "org.bouncycastle:bctls-fips:${versions.bouncycastle_tls}" + compileOnly "org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}" + compileOnly "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}" implementation 'org.ldaptive:ldaptive:1.2.3' implementation 'com.nimbusds:nimbus-jose-jwt:10.9.1' implementation 'com.rfksystems:blake2b:2.0.0' @@ -812,6 +811,7 @@ dependencies { exclude(group: 'org.hamcrest', module: 'hamcrest') } testImplementation "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" + testImplementation "org.bouncycastle:bctls-fips:${versions.bouncycastle_tls}" testImplementation "org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}" testImplementation "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}" // JUnit build requirement @@ -849,6 +849,18 @@ tasks.register('testsJar', Jar) { from(sourceSets.test.output) } +def configureSecurityAdminBcFips(AbstractArchiveTask task) { + def bcFipsJars = configurations.detachedConfiguration( + dependencies.create("org.bouncycastle:bc-fips:${versions.bouncycastle_jce}"), + dependencies.create("org.bouncycastle:bctls-fips:${versions.bouncycastle_tls}"), + dependencies.create("org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}"), + dependencies.create("org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}") + ) + task.from(bcFipsJars) { + into 'deps/' + } +} + tasks.register("bundleSecurityAdminStandalone", Zip) { dependsOn(jar) archiveClassifier = 'securityadmin-standalone' @@ -865,6 +877,7 @@ tasks.register("bundleSecurityAdminStandalone", Zip) { into 'deps/securityconfig' } } +configureSecurityAdminBcFips(bundleSecurityAdminStandalone) tasks.register("bundleSecurityAdminStandaloneTarGz", Tar) { dependsOn(jar) @@ -884,6 +897,7 @@ tasks.register("bundleSecurityAdminStandaloneTarGz", Tar) { into 'deps/securityconfig' } } +configureSecurityAdminBcFips(bundleSecurityAdminStandaloneTarGz) buildRpm { arch = 'NOARCH' diff --git a/gradle/fips.gradle b/gradle/fips.gradle new file mode 100644 index 0000000000..d5e50681aa --- /dev/null +++ b/gradle/fips.gradle @@ -0,0 +1,152 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * Single place where "FIPS mode" is decided and applied to the build's test surface, following the + * schema used by the OpenSearch core repository (gradle/fips.gradle). It owns two things: + * + * 1. which test classes exist and run in each mode (the *FipsTests / *FipsIT split); + * 2. the JVM arguments the test workers need to actually be in FIPS mode. + * + * Keeping them together matters because they have to agree: selecting the FIPS classes without the + * matching JVM arguments produces a suite that looks FIPS-shaped but runs on the ordinary + * providers. Deciding the mode twice is what makes that drift possible, so it is decided once here. + * The BC FIPS artifacts themselves stay in build.gradle with every other dependency -- they are on + * the classpath in both modes, so they are not part of what "FIPS mode" switches. + * + * A FIPS variant is a class named 'FipsTests' (unit tests) or 'FipsIT' (integration + * tests) that extends its baseline, overrides whatever the FIPS providers change, and adds the + * assertions that only hold under FIPS. Baseline classes therefore stay free of + * 'FipsMode.isEnabled()' branches. + * + * The two modes select disjoint sets of classes: + * + * non-FIPS the variants are excluded from compilation and from every Test task, + * so only the baseline classes run. + * FIPS the variants are compiled and run, and each baseline class that has a + * variant is excluded, so the variant replaces it rather than adding to it. + * + * FIPS mode is driven by the OPENSEARCH_FIPS_MODE environment variable. + */ + +ext.fipsTestsEnabled = 'true'.equalsIgnoreCase(System.getenv('OPENSEARCH_FIPS_MODE')) + +// Source file patterns identifying a FIPS variant. +ext.fipsSourcePatterns = ['**/*FipsTests.java', '**/*FipsIT.java'] + +// Compiled counterparts of the patterns above. +def fipsClassSuffixes = ['FipsTests.class', 'FipsIT.class'] + +// Suffixes a baseline class may carry in this repository. Test classes here are named +// inconsistently ('...Test', '...Tests', '...IT', '...IntegrationTest', and names such as +// 'LdapBackendTestNewStyleConfig' that end in none of those), so a variant is matched against +// every combination rather than a single suffix rewrite. The trailing '.class' entry is the +// "no suffix" case: the variant is then simply the full baseline name plus the FIPS suffix. +def baselineClassSuffixes = ['IntegrationTest.class', 'Tests.class', 'Test.class', 'IT.class', '.class'] + +/** + * Keeps FIPS variants out of the given source set unless FIPS mode is on. They reference + * BC FIPS types and assert FIPS-only behaviour, so compiling them outside FIPS mode is + * pointless and would drag BC FIPS onto the non-FIPS compile classpath. + */ +ext.excludeFipsSources = { sourceSet -> + if (!project.ext.fipsTestsEnabled) { + project.ext.fipsSourcePatterns.each { pattern -> sourceSet.java.exclude pattern } + } +} + +/** + * Puts a test worker into (or deliberately out of) FIPS mode. Shared with the subprojects block in + * build.gradle so every test JVM in the build agrees on the security providers it is running with. + */ +ext.configureFipsJvmArgs = { Test task -> + if (project.rootProject.ext.fipsTestsEnabled) { + def fipsSecurityFile = project.rootProject.file('src/test/resources/fips_java_test.security') + // BCFKS truststore is sourced from the core project: buildSrc/src/main/resources/opensearch-fips-truststore.bcfks + // It can also be regenerated by running distribution/src/bin/opensearch-fips-demo-installer + def fipsTrustStore = project.rootProject.file('src/test/resources/fips-jvm-truststore.bcfks') + // Silence BC FIPS JSSE per-handshake INFO chatter on tasks using the default JUL LogManager. + // Tasks that bridge JUL to log4j (integrationTest) ignore this and are muted via their + // log4j2-test.properties instead. + def fipsJulLoggingFile = project.rootProject.file('src/test/resources/fips-jul-test-logging.properties') + task.jvmArgs += "-Djava.security.properties==${fipsSecurityFile}" + task.jvmArgs += "-Djava.util.logging.config.file=${fipsJulLoggingFile}" + task.jvmArgs += "-Dorg.bouncycastle.fips.approved_only=true" + task.jvmArgs += "-Djavax.net.ssl.trustStore=${fipsTrustStore}" + task.jvmArgs += "-Djavax.net.ssl.trustStoreProvider=BCFIPS" + task.jvmArgs += "-Djavax.net.ssl.trustStoreType=BCFKS" + task.jvmArgs += "-Djavax.net.ssl.trustStorePassword=changeit" + } else { + def nonFipsSecurityFile = project.rootProject.file('src/test/resources/java_test.security') + task.jvmArgs += "-Djava.security.properties==${nonFipsSecurityFile}" + } +} + +/** + * Restricts a Test task to the class set matching the current mode. Applied to every Test task so + * that 'test', 'citest', the split CI tasks and 'integrationTest' all agree. + */ +ext.configureFipsTestScope = { Test task -> + if (!project.ext.fipsTestsEnabled) { + fipsClassSuffixes.each { suffix -> task.exclude "**/*${suffix}" } + return + } + + // Report each replacement once per task rather than once per candidate class file. + def reported = [] as Set + + task.exclude { fileTreeElement -> + def name = fileTreeElement.name + // Nested and synthetic classes carry a '$'; they are pulled in by their outer class. + if (!name.endsWith('.class') || name.contains('$')) { + return false + } + // A variant never excludes itself. + if (fipsClassSuffixes.any { name.endsWith(it) }) { + return false + } + def parent = fileTreeElement.file.parentFile + if (parent == null) { + return false + } + for (baselineSuffix in baselineClassSuffixes) { + if (!name.endsWith(baselineSuffix)) { + continue + } + def baseName = name.substring(0, name.length() - baselineSuffix.length()) + for (fipsSuffix in fipsClassSuffixes) { + if (!new File(parent, baseName + fipsSuffix).exists()) { + continue + } + def replaced = name - '.class' + if (reported.add(replaced)) { + task.logger.lifecycle( + "FIPS mode: excluding ${replaced} in favour of ${baseName}${fipsSuffix - '.class'}" + ) + } + return true + } + } + return false + } +} + +// Lazy on purpose: this fires for 'test' straight away and for 'integrationTest' when build.gradle +// creates it further down, so the script can be applied next to the other 'apply from' lines +// instead of having to sit below the source-set declarations. +sourceSets.matching { it.name in ['test', 'integrationTest'] }.configureEach { sourceSet -> + project.ext.excludeFipsSources(sourceSet) +} + +// 'test', 'citest', the split CI tasks and 'integrationTest' are all Test tasks, so a single hook +// keeps scope and providers consistent no matter which one CI invokes -- and a Test task added +// later cannot silently miss its FIPS setup. +tasks.withType(Test).configureEach { task -> + project.ext.configureFipsTestScope(task) + project.ext.configureFipsJvmArgs(task) +} diff --git a/src/integrationTest/java/org/opensearch/security/http/UntrustedLdapServerCertificateTest.java b/src/integrationTest/java/org/opensearch/security/http/UntrustedLdapServerCertificateTest.java index 9a29151830..8ac7984a69 100644 --- a/src/integrationTest/java/org/opensearch/security/http/UntrustedLdapServerCertificateTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/UntrustedLdapServerCertificateTest.java @@ -87,6 +87,11 @@ public class UntrustedLdapServerCertificateTest { @Rule public LogsRule logsRule = new LogsRule("org.opensearch.security.auth.ldap.backend.LDAPAuthenticationBackend"); + /** Exception the JSSE provider logs for an untrusted server certificate; BCJSSE raises its own. */ + protected String getUntrustedCertificateExceptionName() { + return "javax.net.ssl.SSLHandshakeException"; + } + @Test public void shouldNotAuthenticateUserWithLdap() { try (TestRestClient client = cluster.getRestClient(USER_SPOCK, PASSWORD_SPOCK)) { @@ -94,7 +99,7 @@ public void shouldNotAuthenticateUserWithLdap() { response.assertStatusCode(401); } - logsRule.assertThatStackTraceContain("javax.net.ssl.SSLHandshakeException"); + logsRule.assertThatStackTraceContain(getUntrustedCertificateExceptionName()); } } diff --git a/src/integrationTest/resources/log4j2-test.properties b/src/integrationTest/resources/log4j2-test.properties index 538b1d9cca..db78dbdc60 100644 --- a/src/integrationTest/resources/log4j2-test.properties +++ b/src/integrationTest/resources/log4j2-test.properties @@ -53,6 +53,12 @@ logger.securenetty4transport.name = org.opensearch.transport.netty4.ssl.SecureNe logger.securenetty4transport.level = error logger.securenetty4transport.appenderRef.capturing.ref = logCapturingAppender +# BC FIPS JSSE logs every TLS handshake lifecycle event at INFO via JUL, which this task +# bridges to log4j. Raise it to warn so genuine handshake failures still surface while the +# per-handshake chatter is silenced. +logger.bcjsse.name = org.bouncycastle.jsse +logger.bcjsse.level = warn + logger.privileges.name=org.opensearch.security.privileges logger.privileges.level=INFO logger.privileges.appenderRef.capturing.ref = logCapturingAppender diff --git a/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthenticationBackend.java b/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthenticationBackend.java index 17000c7eb2..8f78528cfa 100755 --- a/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthenticationBackend.java +++ b/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthenticationBackend.java @@ -309,7 +309,7 @@ private static LdapEntry existsSearchingAllBases( ); if (isDebugEnabled) { - log.debug("Results for LDAP search for " + user + " in base " + entry.getKey() + ":\n" + result); + log.debug("Results for LDAP search for {} in base {}:\n{}", user, entry.getKey(), foundEntries); } if (foundEntries != null) { diff --git a/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthorizationBackend.java b/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthorizationBackend.java index 0dbf183fa4..64e207290d 100755 --- a/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthorizationBackend.java +++ b/src/main/java/org/opensearch/security/auth/ldap/backend/LDAPAuthorizationBackend.java @@ -13,6 +13,8 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.security.KeyStore; @@ -36,6 +38,8 @@ import javax.naming.ldap.LdapName; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableList; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -48,6 +52,8 @@ import org.opensearch.security.auth.ldap.util.ConfigConstants; import org.opensearch.security.auth.ldap.util.LdapHelper; import org.opensearch.security.auth.ldap.util.Utils; +import org.opensearch.security.auth.ldap2.SNISettingTLSSocketFactory; +import org.opensearch.security.auth.ldap2.SocketFactoryClassLoader; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.PemKeyReader; import org.opensearch.security.support.WildcardMatcher; @@ -68,6 +74,7 @@ import org.ldaptive.SearchFilter; import org.ldaptive.SearchScope; import org.ldaptive.control.RequestControl; +import org.ldaptive.provider.ProviderConfig; import org.ldaptive.provider.ProviderConnection; import org.ldaptive.sasl.Mechanism; import org.ldaptive.sasl.SaslConfig; @@ -76,7 +83,6 @@ import org.ldaptive.ssl.CredentialConfig; import org.ldaptive.ssl.CredentialConfigFactory; import org.ldaptive.ssl.SslConfig; -import org.ldaptive.ssl.ThreadLocalTLSSocketFactory; import static org.opensearch.security.ssl.SecureSSLSettings.SSLSetting.SECURITY_SSL_TRANSPORT_KEYSTORE_PASSWORD; import static org.opensearch.security.ssl.SecureSSLSettings.SSLSetting.SECURITY_SSL_TRANSPORT_TRUSTSTORE_PASSWORD; @@ -86,7 +92,7 @@ public class LDAPAuthorizationBackend implements AuthorizationBackend { private static final AtomicInteger CONNECTION_COUNTER = new AtomicInteger(); private static final String COM_SUN_JNDI_LDAP_OBJECT_DISABLE_ENDPOINT_IDENTIFICATION = "com.sun.jndi.ldap.object.disableEndpointIdentification"; - private static final List DEFAULT_TLS_PROTOCOLS = Arrays.asList("TLSv1.2", "TLSv1.1"); + private static final List DEFAULT_TLS_PROTOCOLS = ImmutableList.copyOf(SSLConfigConstants.ALLOWED_SSL_PROTOCOLS); static final int ONE_PLACEHOLDER = 1; static final int TWO_PLACEHOLDER = 2; static final String DEFAULT_ROLEBASE = ""; @@ -129,7 +135,7 @@ public static void checkConnection(final ConnectionConfig connectionConfig, Stri ClassLoader originalClassloader = null; if (isJava9OrHigher) { originalClassloader = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(new Java9CL()); + Thread.currentThread().setContextClassLoader(new SocketFactoryClassLoader()); } checkConnection0(connectionConfig, bindDn, password, originalClassloader, isJava9OrHigher); @@ -144,7 +150,7 @@ public static Connection getConnection(final Settings settings, final Path confi ClassLoader originalClassloader = null; if (isJava9OrHigher) { originalClassloader = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(new Java9CL()); + Thread.currentThread().setContextClassLoader(new SocketFactoryClassLoader()); } return getConnection0(settings, configPath, originalClassloader, isJava9OrHigher); @@ -183,7 +189,7 @@ private static void checkConnection0( byte[] password, final ClassLoader cl, final boolean needRestore - ) throws KeyStoreException, NoSuchAlgorithmException, CertificateException, FileNotFoundException, IOException, LdapException { + ) throws LdapException { Connection connection = null; @@ -201,9 +207,29 @@ private static void checkConnection0( config.setConnectionInitializer(new BindConnectionInitializer(bindDn, new Credential(password))); DefaultConnectionFactory connFactory = new DefaultConnectionFactory(config); - connection = connFactory.getConnection(); - connection.open(); + String sniHostname = null; + if (config.getLdapUrl() != null && config.getLdapUrl().startsWith("ldaps:")) { + configureSNISocketFactory(connFactory); + String ldapUrl = config.getLdapUrl(); + try { + sniHostname = new URI(ldapUrl).getHost(); + if (StringUtils.isBlank(sniHostname)) { + log.warn("Could not extract hostname from LDAP URL '{}'; proceeding without SNI configuration", ldapUrl); + } + } catch (URISyntaxException e) { + log.warn("Malformed LDAP URL '{}'; proceeding without SNI configuration: {}", ldapUrl, e.getMessage()); + } + } + + try ( + SNISettingTLSSocketFactory.SniContext ignored = StringUtils.isBlank(sniHostname) + ? null + : SNISettingTLSSocketFactory.configure(sniHostname) + ) { + connection = connFactory.getConnection(); + connection.open(); + } } finally { Utils.unbindAndCloseSilently(connection); connection = null; @@ -302,9 +328,20 @@ private static Connection getConnection0( } DefaultConnectionFactory connFactory = new DefaultConnectionFactory(config); - connection = connFactory.getConnection(); - connection.open(); + // Register custom socket factory for SNI hostname verification with BouncyCastle FIPS + // This addresses a known issue where JNDI LDAP doesn't pass hostname to SSLSocketFactory + // See: https://github.com/bcgit/bc-java/issues/460 + if (enableSSL) { + configureSNISocketFactory(connFactory); + try (var ignored = SNISettingTLSSocketFactory.configure(split[0])) { + connection = connFactory.getConnection(); + connection.open(); + } + } else { + connection = connFactory.getConnection(); + connection.open(); + } if (connection != null && connection.isOpen()) { break; @@ -462,6 +499,19 @@ private static void restoreClassLoader0(final ClassLoader cl) { } } + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static boolean shouldVerifyHostname(SslConfig sslConf) { + return sslConf != null && !(sslConf.getHostnameVerifier() instanceof AllowAnyHostnameVerifier); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private static void configureSNISocketFactory(DefaultConnectionFactory connFactory) { + Map props = new HashMap<>(); + props.put("java.naming.ldap.factory.socket", "org.opensearch.security.auth.ldap2.SNISettingTLSSocketFactory"); + final ProviderConfig providerConfig = connFactory.getProvider().getProviderConfig(); + providerConfig.setProperties(props); + } + private static void configureSSL(final ConnectionConfig config, final Settings settings, final Path configPath) throws Exception { final boolean isDebugEnabled = log.isDebugEnabled(); @@ -528,7 +578,21 @@ private static void configureSSL(final ConnectionConfig config, final Settings s ); } - cc = CredentialConfigFactory.createX509CredentialConfig(trustCertificates, authenticationCertificate, authenticationKey); + final KeyStore pemTruststore = PemKeyReader.toTruststore("al", trustCertificates); + final char[] pemInMemoryPassword = SSLConfigConstants.DEFAULT_STORE_PASSWORD.toCharArray(); + final KeyStore pemKeystore = PemKeyReader.toKeystore( + "al", + pemInMemoryPassword, + authenticationCertificate != null ? new X509Certificate[] { authenticationCertificate } : null, + authenticationKey + ); + cc = CredentialConfigFactory.createKeyStoreCredentialConfig( + pemTruststore, + null, + pemKeystore, + pemKeystore != null ? new String(pemInMemoryPassword) : null, + null + ); if (isDebugEnabled) { log.debug("Use PEM to secure communication with LDAP server (client auth is {})", authenticationKey != null); @@ -604,9 +668,6 @@ private static void configureSSL(final ConnectionConfig config, final Settings s // https://www.oracle.com/technetwork/java/javase/10-0-2-relnotes-4477557.html // https://www.oracle.com/technetwork/java/javase/11-0-1-relnotes-5032023.html } - - System.setProperty(COM_SUN_JNDI_LDAP_OBJECT_DISABLE_ENDPOINT_IDENTIFICATION, "true"); - } final List enabledCipherSuites = settings.getAsList(ConfigConstants.LDAPS_ENABLED_SSL_CIPHERS, Collections.emptyList()); @@ -1154,31 +1215,4 @@ private String getRoleFromEntry(final Connection ldapConnection, final LdapName return null; } - - @SuppressWarnings("rawtypes") - private final static Class clazz = ThreadLocalTLSSocketFactory.class; - - private final static class Java9CL extends ClassLoader { - - public Java9CL() { - super(); - } - - @SuppressWarnings("unused") - public Java9CL(ClassLoader parent) { - super(parent); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Override - public Class loadClass(String name) throws ClassNotFoundException { - - if (!name.equalsIgnoreCase("org.ldaptive.ssl.ThreadLocalTLSSocketFactory")) { - return super.loadClass(name); - } - - return clazz; - } - - } } diff --git a/src/main/java/org/opensearch/security/auth/ldap2/HostnameAwareConnectionFactory.java b/src/main/java/org/opensearch/security/auth/ldap2/HostnameAwareConnectionFactory.java new file mode 100644 index 0000000000..793ed3484d --- /dev/null +++ b/src/main/java/org/opensearch/security/auth/ldap2/HostnameAwareConnectionFactory.java @@ -0,0 +1,47 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.ldaptive.Connection; +import org.ldaptive.ConnectionConfig; +import org.ldaptive.DefaultConnectionFactory; +import org.ldaptive.LdapURL; + +/** + * {@link DefaultConnectionFactory} that extracts the hostname from the LDAP URL and returns a + * {@link SniAwareConnection}, so the SNI context is established when the connection is opened — + * the TLS socket is created at {@code open()}, not at {@code getConnection()}. + * + *

Extending {@link DefaultConnectionFactory} (rateher than merely implementing + * {@code ConnectionFactory}) lets the same class back a connection pool, which requires a + * concrete {@link DefaultConnectionFactory}, as well as serve non-pooled connections directly. + * The pool creates each physical connection via {@link #getConnection()} and then calls + * {@code open()} on it, so the SNI wrapping applies uniformly to pooled and non-pooled paths. + * + *

This is necessary because JNDI LDAP resolves hostnames to IP addresses before creating + * SSL sockets, making the hostname unavailable for SNI configuration. + */ +public class HostnameAwareConnectionFactory extends DefaultConnectionFactory { + + private final String ldapUrl; + + public HostnameAwareConnectionFactory(ConnectionConfig config, String ldapUrl) { + super(config); + this.ldapUrl = ldapUrl; + } + + @Override + public Connection getConnection() { + String hostname = new LdapURL(ldapUrl).getEntry().getHostname(); + return new SniAwareConnection(super.getConnection(), hostname); + } +} diff --git a/src/main/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactory.java b/src/main/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactory.java index 124fc92da9..5b7f711167 100644 --- a/src/main/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactory.java +++ b/src/main/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactory.java @@ -12,11 +12,14 @@ package org.opensearch.security.auth.ldap2; import java.nio.file.Path; +import java.security.GeneralSecurityException; import java.time.Duration; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.net.ssl.TrustManagerFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -79,24 +82,37 @@ public LDAPConnectionFactoryFactory(Settings settings, Path configPath) throws S public ConnectionFactory createConnectionFactory(ConnectionPool connectionPool) { if (connectionPool != null) { return new PooledConnectionFactory(connectionPool); - } else { - return createBasicConnectionFactory(); } + return createHostnameAwareConnectionFactory(); } - @SuppressWarnings("unchecked") - public DefaultConnectionFactory createBasicConnectionFactory() { - DefaultConnectionFactory result = new DefaultConnectionFactory(getConnectionConfig()); + /** + * Creates a {@link HostnameAwareConnectionFactory} that returns SNI-aware connections, so the + * SNI context is set when the connection is opened (at {@code open()}, not + * {@code getConnection()}). Being a {@link DefaultConnectionFactory}, it serves both non-pooled + * connections directly and backs a connection pool. + */ + private DefaultConnectionFactory createHostnameAwareConnectionFactory() { + return configureFactory(new HostnameAwareConnectionFactory(getConnectionConfig(), getLdapUrlString())); + } - result.setProvider(new PrivilegedProvider((Provider) result.getProvider())); + public DefaultConnectionFactory createBasicConnectionFactory() { + return configureFactory(new DefaultConnectionFactory(getConnectionConfig())); + } - JndiProviderConfig jndiProviderConfig = (JndiProviderConfig) result.getProvider().getProviderConfig(); + /** + * Applies the shared provider (privileged, for the JNDI socket-factory classloader) and SSL + * wiring that every factory this class builds needs. + */ + @SuppressWarnings("unchecked") + private T configureFactory(T factory) { + factory.setProvider(new PrivilegedProvider((Provider) factory.getProvider())); if (this.sslConfig != null) { - configureSSLinConnectionFactory(result); + configureSSLinConnectionFactory(factory); } - return result; + return factory; } public ConnectionPool createConnectionPool() { @@ -120,10 +136,14 @@ public ConnectionPool createConnectionPool() { AbstractConnectionPool result; + // Use hostname-aware DefaultConnectionFactory for pool to ensure SNI is set for all connections + // including those created during pool initialization + DefaultConnectionFactory hostnameAwareFactory = createHostnameAwareConnectionFactory(); + if ("blocking".equals(this.settings.get(ConfigConstants.LDAP_POOL_TYPE))) { - result = new BlockingConnectionPool(poolConfig, createBasicConnectionFactory()); + result = new BlockingConnectionPool(poolConfig, hostnameAwareFactory); } else { - result = new SoftLimitConnectionPool(poolConfig, createBasicConnectionFactory()); + result = new SoftLimitConnectionPool(poolConfig, hostnameAwareFactory); } result.setValidator(getConnectionValidator()); @@ -301,7 +321,7 @@ private void configureSSL(ConnectionConfig config) { this.sslConfig.getEffectiveTruststoreAliasesArray(), this.sslConfig.getEffectiveKeystore(), this.sslConfig.getEffectiveKeyPasswordString(), - this.sslConfig.getEffectiveKeyAliasesArray() + null ); ldaptiveSslConfig.setCredentialConfig(cc); @@ -322,14 +342,26 @@ private void configureSSL(ConnectionConfig config) { } } - if (this.sslConfig.getSupportedCipherSuites() != null && this.sslConfig.getSupportedCipherSuites().length > 0) { - ldaptiveSslConfig.setEnabledCipherSuites(this.sslConfig.getSupportedCipherSuites()); + final String[] enabledCipherSuites = this.sslConfig.getSupportedCipherSuites(); + if (enabledCipherSuites != null && enabledCipherSuites.length > 0) { + ldaptiveSslConfig.setEnabledCipherSuites(enabledCipherSuites); + log.debug("enabled ssl cipher suites for ldaps {}", Arrays.toString(enabledCipherSuites)); } - ldaptiveSslConfig.setEnabledProtocols(this.sslConfig.getSupportedProtocols()); + final String[] enabledProtocols = this.sslConfig.getSupportedProtocols(); + log.debug("enabled ssl/tls protocols for ldaps {}", Arrays.toString(enabledProtocols)); + ldaptiveSslConfig.setEnabledProtocols(enabledProtocols); if (this.sslConfig.isTrustAllEnabled()) { ldaptiveSslConfig.setTrustManagers(new AllowAnyTrustManager()); + } else { + try { + TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX"); + tmf.init(this.sslConfig.getEffectiveTruststore()); + ldaptiveSslConfig.setTrustManagers(tmf.getTrustManagers()[0]); + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Failed to initialize PKIX TrustManager for LDAPS", e); + } } config.setSslConfig(ldaptiveSslConfig); @@ -352,7 +384,15 @@ private void configureSSLinConnectionFactory(DefaultConnectionFactory connection props.put("jndi.starttls.allowAnyHostname", "true"); } - connectionFactory.getProvider().getProviderConfig().setProperties(props); + // Register the custom socket factory with JNDI for SSL/TLS connections + // SNISettingTLSSocketFactory uses BouncyCastle's CustomSSLSocketFactory to properly + // set SNI hostname and endpoint identification for hostname verification. + // This addresses a known issue where JNDI LDAP doesn't pass hostname to SSLSocketFactory. + // See: https://github.com/bcgit/bc-java/issues/460 + props.put("java.naming.ldap.factory.socket", "org.opensearch.security.auth.ldap2.SNISettingTLSSocketFactory"); + JndiProviderConfig providerConfig = (JndiProviderConfig) connectionFactory.getProvider().getProviderConfig(); + providerConfig.setProperties(props); + providerConfig.setClassLoader(new SocketFactoryClassLoader(SNISettingTLSSocketFactory.class.getClassLoader())); } } diff --git a/src/main/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactory.java b/src/main/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactory.java new file mode 100644 index 0000000000..a8a0155194 --- /dev/null +++ b/src/main/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactory.java @@ -0,0 +1,179 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.Socket; +import java.util.Collections; +import javax.net.ssl.SNIHostName; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.bouncycastle.util.IPAddress; + +import org.ldaptive.ssl.ThreadLocalTLSSocketFactory; + +/** + * Custom socket factory for LDAP connections that ensures SNI hostname is properly set + * for BouncyCastle JSSE provider hostname verification. + * + *

This addresses a known issue where JNDI LDAP's socket creation doesn't pass hostname + * information to the SSLSocketFactory, causing BouncyCastle's hostname verification to fail. + * @see https://github.com/bcgit/bc-java/issues/460 + * + *

The solution wraps the delegate SSLSocketFactory and intercepts all socket creation + * methods to set SNI parameters after socket creation but before the socket is returned to + * the caller. The hostname is provided via ThreadLocal by the connection factory before + * establishing the connection. + */ +public class SNISettingTLSSocketFactory extends SSLSocketFactory { + + private static final Logger log = LogManager.getLogger(SNISettingTLSSocketFactory.class); + + private static final ThreadLocal hostnameThreadLocal = new ThreadLocal<>(); + + private final SSLSocketFactory delegate; + + /** + * Required by JNDI to get the socket factory instance. + * This method is called by JNDI when java.naming.ldap.factory.socket is set. + */ + public static SSLSocketFactory getDefault() { + log.debug("SNISettingTLSSocketFactory.getDefault() called by JNDI"); + // Get the configured SSL socket factory from ldaptive's ThreadLocal + SSLSocketFactory delegate = (SSLSocketFactory) ThreadLocalTLSSocketFactory.getDefault(); + log.debug("Wrapping delegate factory: {}", delegate.getClass().getName()); + return new SNISettingTLSSocketFactory(delegate); + } + + /** + * A no-throw {@link AutoCloseable} returned by {@link #configure} for use in try-with-resources. + */ + @FunctionalInterface + public interface SniContext extends AutoCloseable { + @Override + void close(); + } + + /** + * Sets the SNI hostname for the current thread, returning an {@link SniContext} that clears it + * on close. Intended for use in try-with-resources: + * + *

{@code
+     * try (var ignored = SNISettingTLSSocketFactory.configure(hostname)) {
+     *     connection.open();
+     * }
+     * }
+ */ + public static SniContext configure(String hostname) { + hostnameThreadLocal.set(hostname); + log.debug("Configured SNI context: hostname={}", hostname); + return SNISettingTLSSocketFactory::clearContext; + } + + static String getHostname() { + return hostnameThreadLocal.get(); + } + + static void clearContext() { + hostnameThreadLocal.remove(); + } + + SNISettingTLSSocketFactory(SSLSocketFactory delegate) { + this.delegate = delegate; + } + + @Override + public String[] getDefaultCipherSuites() { + return delegate.getDefaultCipherSuites(); + } + + @Override + public String[] getSupportedCipherSuites() { + return delegate.getSupportedCipherSuites(); + } + + @Override + public Socket createSocket(Socket socket, String host, int port, boolean autoClose) throws IOException { + log.debug("createSocket(Socket, host={}, port={}) called", host, port); + Socket result = delegate.createSocket(socket, host, port, autoClose); + return configureSocket(result); + } + + @Override + public Socket createSocket(String host, int port) throws IOException { + log.debug("createSocket(host={}, port={}) called", host, port); + Socket result = delegate.createSocket(host, port); + return configureSocket(result); + } + + @Override + public Socket createSocket(String host, int port, InetAddress localHost, int localPort) throws IOException { + log.debug("createSocket(host={}, port={}, localHost={}, localPort={}) called", host, port, localHost, localPort); + Socket result = delegate.createSocket(host, port, localHost, localPort); + return configureSocket(result); + } + + @Override + public Socket createSocket(InetAddress host, int port) throws IOException { + log.debug("createSocket(host={}, port={}) called", host, port); + Socket result = delegate.createSocket(host, port); + return configureSocket(result); + } + + @Override + public Socket createSocket(InetAddress address, int port, InetAddress localAddress, int localPort) throws IOException { + log.debug("createSocket(host={}, port={}, localAddress={}, localPort={}) called", address, port, localAddress, localPort); + Socket result = delegate.createSocket(address, port, localAddress, localPort); + return configureSocket(result); + } + + /** + * Sets the SNI {@code server_name} on a newly created socket from the hostname carried on the + * ThreadLocal. Hostname verification is handled by JNDI's endpoint identification (and + * ldaptive's verifier) — not here. + * + * @param socket the created socket + * @return the configured socket + */ + protected Socket configureSocket(Socket socket) { + if (!(socket instanceof SSLSocket sslSocket)) { + log.debug("Socket is not an SSLSocket, skipping SNI configuration"); + return socket; + } + + String hostname = getHostname(); + + log.debug("Configuring SNI for socket, hostname: {}", hostname); + + if (hostname == null) { + log.warn("No hostname available for SNI configuration on socket: {}", socket.getClass().getName()); + return socket; + } + + SSLParameters params = sslSocket.getSSLParameters(); + if (!IPAddress.isValid(hostname)) { + log.debug("Configuring SNI for hostname: {} on socket: {}", hostname, socket.getClass().getName()); + params.setServerNames(Collections.singletonList(new SNIHostName(hostname))); + } + + sslSocket.setSSLParameters(params); + log.debug("Successfully configured socket for: {}", hostname); + + return socket; + } + +} diff --git a/src/main/java/org/opensearch/security/auth/ldap2/SniAwareConnection.java b/src/main/java/org/opensearch/security/auth/ldap2/SniAwareConnection.java new file mode 100644 index 0000000000..880d2fac9b --- /dev/null +++ b/src/main/java/org/opensearch/security/auth/ldap2/SniAwareConnection.java @@ -0,0 +1,108 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.ldaptive.BindRequest; +import org.ldaptive.Connection; +import org.ldaptive.ConnectionConfig; +import org.ldaptive.LdapException; +import org.ldaptive.Response; +import org.ldaptive.control.RequestControl; +import org.ldaptive.provider.ProviderConnection; + +/** + * {@link Connection} decorator that establishes the {@link SNISettingTLSSocketFactory} SNI + * context for the duration of every socket-creating call ({@code open} / {@code reopen}). + * + *

ldaptive's {@code getConnection()} returns an unopened connection; the TLS socket + * is created later, at {@code open()}. Setting the SNI ThreadLocal only around + * {@code getConnection()} therefore clears it before the socket exists, so the socket factory + * sees no hostname (no SNI) and no verify flag (hostname verification skipped). Wrapping + * {@code open()}/{@code reopen()} keeps the hostname and verify flag available exactly when the + * socket is created — for non-pooled and pooled connections alike, since a pool opens + * connections on the thread that initialises/borrows them. + * + *

This whole mechanism (this decorator + the SNI ThreadLocal in + * {@link SNISettingTLSSocketFactory} + {@link HostnameAwareConnectionFactory}) is a workaround for + * the JNDI LDAP provider resolving hostnames to IPs before socket creation (bc-java#460). ldaptive + * 2.x's native (Netty) transport opens sockets with the real hostname, giving SNI and hostname + * verification natively — migrating off the JNDI provider would remove this class, + * {@link SNISettingTLSSocketFactory}, and {@link HostnameAwareConnectionFactory}. + */ +class SniAwareConnection implements Connection { + + private final Connection delegate; + private final String hostname; + + SniAwareConnection(Connection delegate, String hostname) { + this.delegate = delegate; + this.hostname = hostname; + } + + /** The SNI hostname this connection establishes at {@code open()}. Package-private for tests. */ + String hostname() { + return hostname; + } + + @Override + public Response open() throws LdapException { + try (var ignored = SNISettingTLSSocketFactory.configure(hostname)) { + return delegate.open(); + } + } + + @Override + public Response open(BindRequest request) throws LdapException { + try (var ignored = SNISettingTLSSocketFactory.configure(hostname)) { + return delegate.open(request); + } + } + + @Override + public Response reopen() throws LdapException { + try (var ignored = SNISettingTLSSocketFactory.configure(hostname)) { + return delegate.reopen(); + } + } + + @Override + public Response reopen(BindRequest request) throws LdapException { + try (var ignored = SNISettingTLSSocketFactory.configure(hostname)) { + return delegate.reopen(request); + } + } + + @Override + public ConnectionConfig getConnectionConfig() { + return delegate.getConnectionConfig(); + } + + @Override + public boolean isOpen() { + return delegate.isOpen(); + } + + @Override + public ProviderConnection getProviderConnection() { + return delegate.getProviderConnection(); + } + + @Override + public void close() { + delegate.close(); + } + + @Override + public void close(RequestControl[] controls) { + delegate.close(controls); + } +} diff --git a/src/main/java/org/opensearch/security/auth/ldap2/SocketFactoryClassLoader.java b/src/main/java/org/opensearch/security/auth/ldap2/SocketFactoryClassLoader.java new file mode 100644 index 0000000000..e2faf0d1f5 --- /dev/null +++ b/src/main/java/org/opensearch/security/auth/ldap2/SocketFactoryClassLoader.java @@ -0,0 +1,45 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.ldaptive.ssl.ThreadLocalTLSSocketFactory; + +/** + * Classloader that resolves the JNDI LDAP socket-factory classes by name. + * + *

On Java 9+ the module system means JNDI's {@code com.sun.jndi.ldap.Connection.getSocketFactory} + * loads the class named by {@code java.naming.ldap.factory.socket} through the {@code java.naming} + * module loader, which cannot see application/plugin classes on the classpath. Installing this as the + * thread-context classloader lets JNDI resolve {@link SNISettingTLSSocketFactory} (and ldaptive's + * {@link ThreadLocalTLSSocketFactory}) by name; everything else delegates to the parent loader. + */ +public final class SocketFactoryClassLoader extends ClassLoader { + + public SocketFactoryClassLoader() { + super(); + } + + public SocketFactoryClassLoader(ClassLoader parent) { + super(parent); + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + if (SNISettingTLSSocketFactory.class.getName().equals(name)) { + return SNISettingTLSSocketFactory.class; + } + if (ThreadLocalTLSSocketFactory.class.getName().equalsIgnoreCase(name)) { + return ThreadLocalTLSSocketFactory.class; + } + return super.loadClass(name); + } +} diff --git a/src/main/java/org/opensearch/security/ssl/SslConfiguration.java b/src/main/java/org/opensearch/security/ssl/SslConfiguration.java index ad5f835f53..bb07d212b7 100644 --- a/src/main/java/org/opensearch/security/ssl/SslConfiguration.java +++ b/src/main/java/org/opensearch/security/ssl/SslConfiguration.java @@ -61,8 +61,7 @@ public SslConfiguration( } public List dependentFiles() { - return Stream.concat(keyStoreConfiguration.files().stream(), Stream.of(trustStoreConfiguration.file())) - .collect(Collectors.toList()); + return Stream.concat(keyStoreConfiguration.files().stream(), trustStoreConfiguration.files().stream()).collect(Collectors.toList()); } public List certificates() { @@ -90,7 +89,7 @@ SslContext buildServerSslContext(final boolean validateCertificates) { return AccessController.doPrivilegedChecked(() -> { KeyManagerFactory kmFactory = keyStoreConfiguration.createKeyManagerFactory(validateCertificates); Set issuerDns = keyStoreConfiguration.getIssuerDns(); - return SslContextBuilder.forServer(kmFactory) + final SslContextBuilder builder = SslContextBuilder.forServer(kmFactory) .sslProvider(sslParameters.provider()) .clientAuth(sslParameters.clientAuth()) .protocols(sslParameters.allowedProtocols().toArray(new String[0])) @@ -115,8 +114,9 @@ SslContext buildServerSslContext(final boolean validateCertificates) { ApplicationProtocolNames.HTTP_1_1 ) ) - .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates, issuerDns)) - .build(); + .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates, issuerDns)); + keyStoreConfiguration.configure(builder); + return builder.build(); }); } catch (SSLException e) { throw new OpenSearchException("Failed to build server SSL context", e); @@ -128,7 +128,7 @@ SslContext buildClientSslContext(final boolean validateCertificates) { return AccessController.doPrivilegedChecked(() -> { KeyManagerFactory kmFactory = keyStoreConfiguration.createKeyManagerFactory(validateCertificates); Set issuerDns = keyStoreConfiguration.getIssuerDns(); - return SslContextBuilder.forClient() + final SslContextBuilder builder = SslContextBuilder.forClient() .sslProvider(sslParameters.provider()) .protocols(sslParameters.allowedProtocols()) .ciphers(sslParameters.allowedCiphers()) @@ -138,8 +138,9 @@ SslContext buildClientSslContext(final boolean validateCertificates) { .sslProvider(sslParameters.provider()) .keyManager(kmFactory) .trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates, issuerDns)) - .endpointIdentificationAlgorithm(null) - .build(); + .endpointIdentificationAlgorithm(null); + keyStoreConfiguration.configure(builder); + return builder.build(); }); } catch (Exception e) { throw new OpenSearchException("Failed to build client SSL context", e); diff --git a/src/main/java/org/opensearch/security/ssl/SslSettingsManager.java b/src/main/java/org/opensearch/security/ssl/SslSettingsManager.java index 0af172e95f..3f05fbc84a 100644 --- a/src/main/java/org/opensearch/security/ssl/SslSettingsManager.java +++ b/src/main/java/org/opensearch/security/ssl/SslSettingsManager.java @@ -36,6 +36,7 @@ import org.opensearch.security.ssl.config.SslCertificatesLoader; import org.opensearch.security.ssl.config.SslParameters; import org.opensearch.security.ssl.config.TrustStoreConfiguration; +import org.opensearch.security.support.PemKeyReader; import org.opensearch.watcher.FileChangesListener; import org.opensearch.watcher.FileWatcher; import org.opensearch.watcher.ResourceWatcherService; @@ -48,6 +49,7 @@ import static org.opensearch.security.ssl.util.SSLConfigConstants.EXTENDED_KEY_USAGE_ENABLED; import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_ALIAS; import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_FILEPATH; +import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_TYPE; import static org.opensearch.security.ssl.util.SSLConfigConstants.PEM_CERT_FILEPATH; import static org.opensearch.security.ssl.util.SSLConfigConstants.PEM_KEY_FILEPATH; import static org.opensearch.security.ssl.util.SSLConfigConstants.PEM_TRUSTED_CAS_FILEPATH; @@ -74,6 +76,7 @@ import static org.opensearch.security.ssl.util.SSLConfigConstants.SSL_TRANSPORT_SERVER_EXTENDED_PREFIX; import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_ALIAS; import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_FILEPATH; +import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_TYPE; import static org.opensearch.transport.AuxTransport.AUX_TRANSPORT_TYPES_SETTING; public class SslSettingsManager { @@ -346,7 +349,10 @@ private void validateKeyStoreSettings(CertType transportType, final Settings set final var clientAuth = ClientAuth.valueOf( transportSettings.get(CLIENT_AUTH_MODE, ClientAuth.OPTIONAL.name()).toUpperCase(Locale.ROOT) ); - if (!transportSettings.hasValue(KEYSTORE_FILEPATH)) { + // A PKCS#11 keystore/truststore loads its material from the token, so no filepath is required. + final boolean isPkcs11Keystore = PemKeyReader.PKCS11.equalsIgnoreCase(transportSettings.get(KEYSTORE_TYPE)); + final boolean isPkcs11Truststore = PemKeyReader.PKCS11.equalsIgnoreCase(transportSettings.get(TRUSTSTORE_TYPE)); + if (!isPkcs11Keystore && !transportSettings.hasValue(KEYSTORE_FILEPATH)) { throw new OpenSearchException( "Wrong " + transportType.id().toLowerCase(Locale.ROOT) @@ -355,7 +361,7 @@ private void validateKeyStoreSettings(CertType transportType, final Settings set + " must be set" ); } - if (clientAuth == ClientAuth.REQUIRE && !transportSettings.hasValue(TRUSTSTORE_FILEPATH)) { + if (clientAuth == ClientAuth.REQUIRE && !isPkcs11Truststore && !transportSettings.hasValue(TRUSTSTORE_FILEPATH)) { throw new OpenSearchException( "Wrong " + transportType.id().toLowerCase(Locale.ROOT) @@ -448,7 +454,12 @@ private void validateTransportSettings(final Settings transportSettings) { } private void verifyKeyAndTrustStoreSettings(final Settings settings) { - if (!settings.hasValue(KEYSTORE_FILEPATH) || !settings.hasValue(TRUSTSTORE_FILEPATH)) { + // PKCS#11 keystores/truststores have no filepath - their material comes from the token. + final boolean keyStorePresent = settings.hasValue(KEYSTORE_FILEPATH) + || PemKeyReader.PKCS11.equalsIgnoreCase(settings.get(KEYSTORE_TYPE)); + final boolean trustStorePresent = settings.hasValue(TRUSTSTORE_FILEPATH) + || PemKeyReader.PKCS11.equalsIgnoreCase(settings.get(TRUSTSTORE_TYPE)); + if (!keyStorePresent || !trustStorePresent) { throw new OpenSearchException( "Wrong Transport/Tran SSL configuration. One of Keystore and Truststore files or X.509 PEM certificates and " + "PKCS#8 keys groups should be set to configure Transport layer properly" @@ -461,7 +472,10 @@ private boolean hasExtendedKeyUsageEnabled(final Settings settings) { } private boolean hasKeyOrTrustStoreSettings(final Settings settings) { - return settings.hasValue(KEYSTORE_FILEPATH) || settings.hasValue(TRUSTSTORE_FILEPATH); + return settings.hasValue(KEYSTORE_FILEPATH) + || settings.hasValue(TRUSTSTORE_FILEPATH) + || PemKeyReader.PKCS11.equalsIgnoreCase(settings.get(KEYSTORE_TYPE)) + || PemKeyReader.PKCS11.equalsIgnoreCase(settings.get(TRUSTSTORE_TYPE)); } private boolean hasPemStoreSettings(final Settings settings) { 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 aed7c9b4c6..224ee30ed0 100644 --- a/src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java +++ b/src/main/java/org/opensearch/security/ssl/config/KeyStoreConfiguration.java @@ -14,13 +14,12 @@ import java.nio.file.Path; import java.security.GeneralSecurityException; import java.security.KeyStore; -import java.security.KeyStoreException; +import java.security.Security; import java.security.cert.X509Certificate; -import java.util.Arrays; -import java.util.Collections; import java.util.List; -import java.util.Objects; +import java.util.Locale; import java.util.Set; +import java.util.function.Supplier; import java.util.stream.Collectors; import javax.net.ssl.KeyManagerFactory; import javax.security.auth.x500.X500Principal; @@ -29,8 +28,34 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; - -public interface KeyStoreConfiguration { +import org.opensearch.security.support.PemKeyReader; + +import io.netty.handler.ssl.SslContextBuilder; + +public sealed interface KeyStoreConfiguration { + + /** + * Picks the implementation the configured store {@code type} asks for: a PKCS#11 token when it names one, + * a file-based store otherwise. + * + * @param type store type as configured, {@code null} to detect it from the content of the file + * @param file resolves the key store file, evaluated only when the type turns out to be file-based - a + * token has no file setting to resolve, and asking for one would fail + */ + static KeyStoreConfiguration buildKeyStoreConfiguration( + final String type, + final Supplier file, + final String alias, + final StorePassword keyStorePassword, + final StorePassword keyPassword + ) { + if (Pkcs11KeyStoreConfiguration.TYPE.equalsIgnoreCase(type)) { + return new Pkcs11KeyStoreConfiguration(alias, keyStorePassword, keyPassword); + } + final var path = file.get(); + final var resolvedType = PemKeyReader.extractStoreType(path.toString(), type).toUpperCase(Locale.ROOT); + return new JdkKeyStoreConfiguration(path, resolvedType, alias, keyStorePassword, keyPassword); + } List files(); @@ -51,124 +76,118 @@ default Set getIssuerDns() { .collect(Collectors.toSet()); } - default KeyManagerFactory buildKeyManagerFactory(final KeyStore keyStore, final char[] password) { + default KeyManagerFactory buildKeyManagerFactory(final KeyStore keyStore, final StorePassword password) { try { final var keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); - keyManagerFactory.init(keyStore, password); + keyManagerFactory.init(keyStore, password.chars()); return keyManagerFactory; } catch (GeneralSecurityException e) { throw new OpenSearchException("Failed to create KeyManagerFactory", e); } } - Tuple createKeyStore(); + Tuple createKeyStore(); + + /** + * Adjusts the TLS context to how the key of this store has to be used. + */ + default void configure(final SslContextBuilder builder) {} + + /** + * A file-based key store in any type a registered provider offers, e.g. JKS, JCEKS, PKCS12 or BCFKS. + * + * @param path location of the key store file + * @param type store type, as resolved from the settings or detected from the file + * @param alias optional alias of the key entry to use + * @param keyStorePassword password of the store itself + * @param keyPassword password of the key entry + */ + record JdkKeyStoreConfiguration(Path path, String type, String alias, StorePassword keyStorePassword, StorePassword keyPassword) + implements + KeyStoreConfiguration { - final class JdkKeyStoreConfiguration implements KeyStoreConfiguration { - private final Path path; - - private final String type; + @Override + public List loadCertificates() { + final var keyStore = KeyStoreUtils.loadKeyStore(path, type, keyStorePassword.chars()); + return KeyStoreUtils.loadKeyEntryCertificates(keyStore, type, alias, path.toString()); + } - private final String alias; + @Override + public List files() { + return List.of(path); + } - private final char[] keyStorePassword; + @Override + public Tuple createKeyStore() { + return Tuple.tuple(KeyStoreUtils.newKeyStore(path, type, alias, keyStorePassword.chars(), keyPassword.chars()), keyPassword); + } + } - private final char[] keyPassword; + /** + * Key material held in a PKCS#11 token: there is no file on disk, and the private key is non-exportable, + * so it can be neither copied into another store nor signed with by the BouncyCastle FIPS JSSE provider. + * {@link #configure(SslContextBuilder)} routes the handshake around that. + * + * @param alias optional alias of the key entry to report certificates for + * @param pin the token PIN, taken from the {@code keystore_password} setting + * @param keyPassword password of the key entry, usually the PIN as well + */ + record Pkcs11KeyStoreConfiguration(String alias, StorePassword pin, StorePassword keyPassword) implements KeyStoreConfiguration { - public JdkKeyStoreConfiguration( - final Path path, - final String type, - final String alias, - final char[] keyStorePassword, - final char[] keyPassword - ) { - this.path = path; - this.type = type; - this.alias = alias; - this.keyStorePassword = keyStorePassword; - this.keyPassword = keyPassword; - } + static final String TYPE = PemKeyReader.PKCS11; - private void loadCertificateChain(final String alias, final KeyStore keyStore, final ImmutableList.Builder listBuilder) - throws KeyStoreException { - final var cc = keyStore.getCertificateChain(alias); - var first = true; - for (final var c : cc) { - if (c instanceof X509Certificate) { - listBuilder.add(new Certificate((X509Certificate) c, type, alias, first)); - first = false; - } - } - } + private static final String SOURCE = "PKCS#11 token"; @Override public List loadCertificates() { - final var keyStore = KeyStoreUtils.loadKeyStore(path, type, keyStorePassword); - final var listBuilder = ImmutableList.builder(); - - try { - if (alias != null) { - if (keyStore.isKeyEntry(alias)) { - loadCertificateChain(alias, keyStore, listBuilder); - } - } else { - for (final var a : Collections.list(keyStore.aliases())) { - if (keyStore.isKeyEntry(a)) { - loadCertificateChain(a, keyStore, listBuilder); - } - } - } - final var list = listBuilder.build(); - if (list.isEmpty()) { - throw new OpenSearchException("The file " + path + " does not contain any certificates"); - } - return listBuilder.build(); - } catch (GeneralSecurityException e) { - throw new OpenSearchException("Couldn't load certificates from file " + path, e); - } + return KeyStoreUtils.loadKeyEntryCertificates(loadToken(), TYPE, alias, SOURCE); } + /** + * @return no files - a token is not backed by anything on disk, hence nothing to watch for reloads + */ @Override public List files() { - return List.of(path); + return List.of(); } + /** + * Returns the token store as it is. Unlike the file-based configurations this cannot narrow the store + * down to {@link #alias()}, because that requires extracting the key, which the token does not permit. + */ @Override - public Tuple createKeyStore() { - final var keyStore = KeyStoreUtils.newKeyStore(path, type, alias, keyStorePassword, keyPassword); - return Tuple.tuple(keyStore, keyPassword); + public Tuple createKeyStore() { + return Tuple.tuple(loadToken(), keyPassword); } + /** + * A token-resident private key is non-exportable, so the BouncyCastle FIPS JSSE provider cannot sign with + * it (it fails with "no encoding for key" during the TLS CertificateVerify). SunJSSE instead delegates the + * signature operation to the key's own provider (SunPKCS11), letting the token perform it. This only + * affects the TLS engine's handshake signing; the JDK {@link io.netty.handler.ssl.SslProvider} is unchanged. + */ @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - JdkKeyStoreConfiguration that = (JdkKeyStoreConfiguration) o; - return Objects.equals(path, that.path) - && Objects.equals(type, that.type) - && Objects.equals(alias, that.alias) - && Objects.deepEquals(keyStorePassword, that.keyStorePassword) - && Objects.deepEquals(keyPassword, that.keyPassword); + public void configure(final SslContextBuilder builder) { + final var sunJSSE = Security.getProvider("SunJSSE"); + if (sunJSSE == null) { + throw new OpenSearchException("SunJSSE provider not available; required for PKCS#11 key store support"); + } + builder.sslContextProvider(sunJSSE); } - @Override - public int hashCode() { - return Objects.hash(path, type, alias, Arrays.hashCode(keyStorePassword), Arrays.hashCode(keyPassword)); + private KeyStore loadToken() { + return KeyStoreUtils.loadPkcs11Store(pin.chars()); } } - final class PemKeyStoreConfiguration implements KeyStoreConfiguration { - - private final Path certificateChainPath; - - private final Path keyPath; - - private final char[] keyPassword; - - public PemKeyStoreConfiguration(final Path certificateChainPath, final Path keyPath, final char[] keyPassword) { - this.certificateChainPath = certificateChainPath; - this.keyPath = keyPath; - this.keyPassword = keyPassword; - } + /** + * A certificate chain and a private key, both in PEM format. + * + * @param certificateChainPath location of the certificate chain + * @param keyPath location of the private key + * @param keyPassword password of the private key, {@link StorePassword#NONE} when it is not encrypted + */ + record PemKeyStoreConfiguration(Path certificateChainPath, Path keyPath, StorePassword keyPassword) implements KeyStoreConfiguration { @Override public List loadCertificates() { @@ -187,24 +206,8 @@ public List files() { } @Override - public Tuple createKeyStore() { - final var keyStore = KeyStoreUtils.newKeyStoreFromPem(certificateChainPath, keyPath, keyPassword); - return Tuple.tuple(keyStore, keyPassword); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - PemKeyStoreConfiguration that = (PemKeyStoreConfiguration) o; - return Objects.equals(certificateChainPath, that.certificateChainPath) - && Objects.equals(keyPath, that.keyPath) - && Objects.deepEquals(keyPassword, that.keyPassword); - } - - @Override - public int hashCode() { - return Objects.hash(certificateChainPath, keyPath, Arrays.hashCode(keyPassword)); + public Tuple createKeyStore() { + return Tuple.tuple(KeyStoreUtils.newKeyStoreFromPem(certificateChainPath, keyPath, keyPassword.chars()), keyPassword); } } 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 5f7e6c0fe8..e0e817b0b8 100644 --- a/src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java +++ b/src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.security.GeneralSecurityException; import java.security.InvalidAlgorithmParameterException; import java.security.KeyException; import java.security.KeyStore; @@ -25,6 +26,7 @@ import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Set; import javax.crypto.NoSuchPaddingException; @@ -32,10 +34,12 @@ import javax.net.ssl.SSLSessionContext; import javax.security.auth.x500.X500Principal; +import com.google.common.collect.ImmutableList; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchException; +import org.opensearch.security.support.PemKeyReader; import io.netty.buffer.ByteBufAllocator; import io.netty.handler.ssl.ApplicationProtocolNegotiator; @@ -104,23 +108,119 @@ public static X509Certificate[] x509Certificates(final Path file) { return certificates; } - public static KeyStore loadTrustStore(final Path path, final String type, final String alias, final char[] password) { + /** + * Collects the certificate chains of all key entries of the given store, optionally narrowed down to a single alias. + * + * @param source human-readable origin of the store, used for error messages only + */ + public static List loadKeyEntryCertificates( + final KeyStore keyStore, + final String type, + final String alias, + final String source + ) { + final var listBuilder = ImmutableList.builder(); try { - var keyStore = loadKeyStore(path, type, password); if (alias != null) { - if (!keyStore.isCertificateEntry(alias)) { - throw new OpenSearchException("Alias " + alias + " does not contain a certificate entry"); + if (keyStore.isKeyEntry(alias)) { + addCertificateChain(keyStore, type, alias, listBuilder); } - final var aliasCertificate = (X509Certificate) keyStore.getCertificate(alias); - if (aliasCertificate == null) { - throw new OpenSearchException("Couldn't find SSL certificate for alias " + alias); + } else { + for (final var a : Collections.list(keyStore.aliases())) { + if (keyStore.isKeyEntry(a)) { + addCertificateChain(keyStore, type, a, listBuilder); + } } - keyStore = newKeyStore(type); - keyStore.setCertificateEntry(alias, aliasCertificate); } - return keyStore; + } catch (GeneralSecurityException e) { + throw new OpenSearchException("Couldn't load certificates from " + source, e); + } + final var list = listBuilder.build(); + if (list.isEmpty()) { + throw new OpenSearchException("The keystore " + source + " does not contain any certificates"); + } + return list; + } + + private static void addCertificateChain( + final KeyStore keyStore, + final String type, + final String alias, + final ImmutableList.Builder listBuilder + ) throws KeyStoreException { + final var cc = keyStore.getCertificateChain(alias); + if (cc == null) { + return; + } + var first = true; + for (final var c : cc) { + if (c instanceof X509Certificate) { + listBuilder.add(new Certificate((X509Certificate) c, type, alias, first)); + first = false; + } + } + } + + /** + * Collects the trusted certificates of the given store, optionally narrowed down to a single alias. + * + * @param source human-readable origin of the store, used for error messages only + */ + public static List loadTrustedCertificates( + final KeyStore trustStore, + final String type, + final String alias, + final String source + ) { + final var listBuilder = ImmutableList.builder(); + try { + if (alias != null) { + final var c = trustStore.getCertificate(alias); + if (c instanceof X509Certificate) { + listBuilder.add(new Certificate((X509Certificate) c, type, alias, false)); + } + } else { + for (final var a : Collections.list(trustStore.aliases())) { + if (!trustStore.isCertificateEntry(a)) continue; + final var c = trustStore.getCertificate(a); + if (c instanceof X509Certificate) { + listBuilder.add(new Certificate((X509Certificate) c, type, a, false)); + } + } + } + } catch (GeneralSecurityException e) { + throw new OpenSearchException("Couldn't load certificates from " + source, e); + } + final var list = listBuilder.build(); + if (list.isEmpty()) { + throw new OpenSearchException("The truststore " + source + " does not contain any certificates"); + } + return list; + } + + public static KeyStore loadTrustStore(final Path path, final String type, final String alias, final char[] password) { + final var trustStore = loadKeyStore(path, type, password); + return alias != null ? narrowToAlias(trustStore, type, alias, path.toString()) : trustStore; + } + + /** + * Copies the certificate of a single alias into a new in-memory store of {@code targetType}, so that only + * that certificate is trusted. The source store is never modified. + */ + public static KeyStore narrowToAlias(final KeyStore trustStore, final String targetType, final String alias, final String source) { + try { + if (!trustStore.isCertificateEntry(alias)) { + throw new OpenSearchException("Alias " + alias + " does not contain a certificate entry"); + } + final var aliasCertificate = (X509Certificate) trustStore.getCertificate(alias); + if (aliasCertificate == null) { + throw new OpenSearchException("Couldn't find SSL certificate for alias " + alias); + } + final var narrowed = newKeyStore(targetType); + narrowed.setCertificateEntry(alias, aliasCertificate); + return narrowed; } catch (Exception e) { - throw new OpenSearchException("Failed to load trust store from " + path, e); + throw new OpenSearchException("Failed to load trust store from " + source, e); } } @@ -214,15 +314,29 @@ public static KeyStore loadKeyStore(final Path path, final String type, final ch final var keyStore = KeyStore.getInstance(type); try (final var in = Files.newInputStream(path)) { keyStore.load(in, password); - return keyStore; } catch (IOException e) { throw new RuntimeException(e); } + return keyStore; } catch (Exception e) { throw new OpenSearchException("Failed to load keystore from " + path, e); } } + /** + * Opens the PKCS#11 token registered with the JVM. The token holds the key material itself, so there is + * nothing to read from disk - the PIN only unlocks the session. + */ + public static KeyStore loadPkcs11Store(final char[] pin) { + try { + final var keyStore = KeyStore.getInstance(PemKeyReader.PKCS11); + keyStore.load(null, pin); + return keyStore; + } catch (Exception e) { + throw new OpenSearchException("Failed to load keystore from the PKCS#11 token", e); + } + } + public static KeyStore newKeyStore( final Path path, final String type, diff --git a/src/main/java/org/opensearch/security/ssl/config/SslCertificatesLoader.java b/src/main/java/org/opensearch/security/ssl/config/SslCertificatesLoader.java index 1f9315bfd3..0849b9e38e 100644 --- a/src/main/java/org/opensearch/security/ssl/config/SslCertificatesLoader.java +++ b/src/main/java/org/opensearch/security/ssl/config/SslCertificatesLoader.java @@ -61,109 +61,145 @@ public SslCertificatesLoader(final String sslConfigSuffix, final String extended public Tuple loadConfiguration(final Environment environment) { final var settings = environment.settings(); final var sslConfigSettings = settings.getByPrefix(fullSslConfigSuffix); - if (settings.hasValue(sslConfigSuffix + KEYSTORE_FILEPATH)) { - final var keyStorePassword = resolvePassword(sslConfigSuffix + KEYSTORE_PASSWORD, settings, DEFAULT_STORE_PASSWORD); - return Tuple.tuple( - environment.settings().hasValue(sslConfigSuffix + TRUSTSTORE_FILEPATH) - ? buildJdkTrustStoreConfiguration( - sslConfigSettings, - environment, - resolvePassword(sslConfigSuffix + TRUSTSTORE_PASSWORD, settings, DEFAULT_STORE_PASSWORD) - ) - : TrustStoreConfiguration.EMPTY_CONFIGURATION, - buildJdkKeyStoreConfiguration( - sslConfigSettings, - environment, - keyStorePassword, - resolvePassword( - fullSslConfigSuffix + KEYSTORE_KEY_PASSWORD, - settings, - keyStorePassword != null ? String.valueOf(keyStorePassword) : null - ) + final var keyStoreType = settings.get(sslConfigSuffix + KEYSTORE_TYPE); + final var trustStoreType = settings.get(sslConfigSuffix + TRUSTSTORE_TYPE); + final boolean isPkcs11Keystore = PemKeyReader.PKCS11.equalsIgnoreCase(keyStoreType); + final boolean isPkcs11Truststore = PemKeyReader.PKCS11.equalsIgnoreCase(trustStoreType); + final boolean usesKeyStore = settings.hasValue(sslConfigSuffix + KEYSTORE_FILEPATH) || isPkcs11Keystore; + final boolean usesTrustStore = settings.hasValue(sslConfigSuffix + TRUSTSTORE_FILEPATH) || isPkcs11Truststore; + final boolean usesPemTrustedCas = sslConfigSettings.hasValue(PEM_TRUSTED_CAS_FILEPATH); + if (usesKeyStore) { + warnIfPemTrustedCasAreIgnored(usesPemTrustedCas); + final var keyStorePassword = resolvePassword(sslConfigSuffix + KEYSTORE_PASSWORD, settings, defaultStorePassword()); + final var trustStoreConfiguration = usesTrustStore + ? TrustStoreConfiguration.buildTrustStoreConfiguration( + trustStoreType, + () -> resolvePath(settings.get(sslConfigSuffix + TRUSTSTORE_FILEPATH), environment), + sslConfigSettings.get(TRUSTSTORE_ALIAS, null), + resolvePassword(sslConfigSuffix + TRUSTSTORE_PASSWORD, settings, defaultStorePassword()) ) + : TrustStoreConfiguration.EMPTY_CONFIGURATION; + final var keyStoreConfiguration = KeyStoreConfiguration.buildKeyStoreConfiguration( + keyStoreType, + () -> resolvePath(settings.get(sslConfigSuffix + KEYSTORE_FILEPATH), environment), + sslConfigSettings.get(KEYSTORE_ALIAS, null), + keyStorePassword, + // the key password defaults to the store password, as keytool does when only one is given + resolvePassword(fullSslConfigSuffix + KEYSTORE_KEY_PASSWORD, settings, keyStorePassword) ); + warnIfTokenAliasCannotSelectTheKey(keyStoreConfiguration); + return Tuple.tuple(trustStoreConfiguration, keyStoreConfiguration); } else { - return Tuple.tuple( - sslConfigSettings.hasValue(PEM_TRUSTED_CAS_FILEPATH) - ? new TrustStoreConfiguration.PemTrustStoreConfiguration( - resolvePath(sslConfigSettings.get(PEM_TRUSTED_CAS_FILEPATH), environment) - ) - : TrustStoreConfiguration.EMPTY_CONFIGURATION, - buildPemKeyStoreConfiguration( - sslConfigSettings, - environment, - resolvePassword(fullSslConfigSuffix + PEM_KEY_PASSWORD, settings, null) + warnIfTrustStoreSettingsAreIgnored(usesTrustStore); + final var trustStoreConfiguration = usesPemTrustedCas + ? new TrustStoreConfiguration.PemTrustStoreConfiguration( + resolvePath(sslConfigSettings.get(PEM_TRUSTED_CAS_FILEPATH), environment) ) + : TrustStoreConfiguration.EMPTY_CONFIGURATION; + final var keyStoreConfiguration = new KeyStoreConfiguration.PemKeyStoreConfiguration( + resolvePath(sslConfigSettings.get(PEM_CERT_FILEPATH), environment), + resolvePath(sslConfigSettings.get(PEM_KEY_FILEPATH), environment), + resolvePassword(fullSslConfigSuffix + PEM_KEY_PASSWORD, settings, StorePassword.NONE) ); + return Tuple.tuple(trustStoreConfiguration, keyStoreConfiguration); } } - private char[] resolvePassword(final String legacyPasswordSettings, final Settings settings, final String defaultPassword) { + /** + * @return a fresh instance each time, so that the key store and the trust store never end up sharing one array + */ + private static StorePassword defaultStorePassword() { + return StorePassword.of(DEFAULT_STORE_PASSWORD.toCharArray()); + } + + /** + * Resolves a password from the secure settings, falling back to the legacy plain-text setting and finally to + * {@code defaultPassword}. The default is applied only once neither source provided a value, so that "unset" + * stays distinguishable from "explicitly set to the default value". + *

+ * The password never becomes a {@link String}: that would place a critical security parameter into memory + * that cannot be overwritten. + * + * @param defaultPassword returned as is when the password is configured nowhere, {@link StorePassword#NONE} + * where there is no password to fall back to + */ + private StorePassword resolvePassword( + final String legacyPasswordSettings, + final Settings settings, + final StorePassword defaultPassword + ) { final var securePasswordSetting = String.format("%s%s", legacyPasswordSettings, SECURE_SUFFIX); final var securePassword = SecureSetting.secureString(securePasswordSetting, null).get(settings); - final var legacyPassword = settings.get(legacyPasswordSettings, defaultPassword); - if (!securePassword.isEmpty() && legacyPassword != null && !legacyPassword.equals(defaultPassword)) { + final var legacyPassword = settings.get(legacyPasswordSettings); + if (!securePassword.isEmpty() && legacyPassword != null) { throw new OpenSearchException("One of " + legacyPasswordSettings + " or " + securePasswordSetting + " must be set not both"); } if (!securePassword.isEmpty()) { - return securePassword.getChars(); - } else { - if (legacyPassword != null) { - LOGGER.warn( - "Setting [{}] has a secure counterpart [{}] which should be used instead - allowing for legacy SSL setups", - legacyPasswordSettings, - securePasswordSetting - ); - return legacyPassword.toCharArray(); - } + return StorePassword.of(securePassword.getChars()); } - return null; + if (legacyPassword != null) { + LOGGER.warn( + "Setting [{}] has a secure counterpart [{}] which should be used instead - allowing for legacy SSL setups", + legacyPasswordSettings, + securePasswordSetting + ); + return StorePassword.of(legacyPassword.toCharArray()); + } + return defaultPassword; } - private KeyStoreConfiguration.JdkKeyStoreConfiguration buildJdkKeyStoreConfiguration( - final Settings settings, - final Environment environment, - final char[] keyStorePassword, - final char[] keyPassword - ) { - final Path path = resolvePath(environment.settings().get(sslConfigSuffix + KEYSTORE_FILEPATH), environment); - final String explicitType = environment.settings().get(sslConfigSuffix + KEYSTORE_TYPE); - final String resolvedType = PemKeyReader.extractStoreType(path.toString(), explicitType); - return new KeyStoreConfiguration.JdkKeyStoreConfiguration( - path, - resolvedType, - settings.get(KEYSTORE_ALIAS, null), - keyStorePassword, - keyPassword + /** + * The trusted certificates of a key store configuration are read from a store as well, so PEM ones are dropped - + * and the peer verification falls back to whatever the TLS engine defaults to, rather than using the certificates + * that were configured. + */ + private void warnIfPemTrustedCasAreIgnored(final boolean usesPemTrustedCas) { + if (!usesPemTrustedCas) { + return; + } + LOGGER.warn( + "Setting [{}{}] is ignored because the key material comes from a key store - configure the trusted " + + "certificates in [{}{}], or in a PKCS#11 token via [{}{}], to have this node actually use them", + fullSslConfigSuffix, + PEM_TRUSTED_CAS_FILEPATH, + sslConfigSuffix, + TRUSTSTORE_FILEPATH, + sslConfigSuffix, + TRUSTSTORE_TYPE ); } - private TrustStoreConfiguration.JdkTrustStoreConfiguration buildJdkTrustStoreConfiguration( - final Settings settings, - final Environment environment, - final char[] trustStorePassword - ) { - final Path path = resolvePath(environment.settings().get(sslConfigSuffix + TRUSTSTORE_FILEPATH), environment); - final String explicitType = environment.settings().get(sslConfigSuffix + TRUSTSTORE_TYPE); - final String resolvedType = PemKeyReader.extractStoreType(path.toString(), explicitType); - return new TrustStoreConfiguration.JdkTrustStoreConfiguration( - path, - resolvedType, - settings.get(TRUSTSTORE_ALIAS, null), - trustStorePassword + /** + * The counterpart of {@link #warnIfPemTrustedCasAreIgnored(boolean)}: PEM key material reads its trusted + * certificates from a PEM file, so a trust store or token configured alongside it is dropped. + */ + private void warnIfTrustStoreSettingsAreIgnored(final boolean usesTrustStore) { + if (!usesTrustStore) { + return; + } + LOGGER.warn( + "Settings [{}{}] and [{}{}] are ignored because the key material comes from PEM files - configure the " + + "trusted certificates in [{}{}] to have this node actually use them", + sslConfigSuffix, + TRUSTSTORE_FILEPATH, + sslConfigSuffix, + TRUSTSTORE_TYPE, + fullSslConfigSuffix, + PEM_TRUSTED_CAS_FILEPATH ); } - private KeyStoreConfiguration.PemKeyStoreConfiguration buildPemKeyStoreConfiguration( - final Settings settings, - final Environment environment, - final char[] pemKeyPassword - ) { - return new KeyStoreConfiguration.PemKeyStoreConfiguration( - resolvePath(settings.get(PEM_CERT_FILEPATH), environment), - resolvePath(settings.get(PEM_KEY_FILEPATH), environment), - pemKeyPassword - ); + private void warnIfTokenAliasCannotSelectTheKey(final KeyStoreConfiguration keyStoreConfiguration) { + if (keyStoreConfiguration instanceof KeyStoreConfiguration.Pkcs11KeyStoreConfiguration token && token.alias() != null) { + LOGGER.warn( + "Setting [{}{}] selects the certificates reported for the PKCS#11 token, but not the key used to " + + "handshake with - a token key cannot be extracted, so the key manager picks among all keys of the token. " + + "Remove the setting to silence this warning, and let the token slot this node logs into hold only the " + + "key it should use", + fullSslConfigSuffix, + KEYSTORE_ALIAS + ); + } } private Path resolvePath(final String filePath, final Environment environment) { diff --git a/src/main/java/org/opensearch/security/ssl/config/StorePassword.java b/src/main/java/org/opensearch/security/ssl/config/StorePassword.java new file mode 100644 index 0000000000..4704d4804e --- /dev/null +++ b/src/main/java/org/opensearch/security/ssl/config/StorePassword.java @@ -0,0 +1,67 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.security.ssl.config; + +import java.util.Arrays; + +/** + * A key store password, key password or PKCS#11 token PIN - a critical security parameter. Wrapping the + * characters keeps them off the public surface of the store configurations: the array is unwrapped only + * within this package, at the JCA calls that insist on a {@code char[]}. Equality is by content, and + * {@link #toString()} never reveals the characters. + */ +public final class StorePassword { + + public static final StorePassword NONE = new StorePassword(null); + + private final char[] password; + + private StorePassword(final char[] password) { + this.password = password; + } + + public static StorePassword of(final char[] password) { + return password != null ? new StorePassword(password) : NONE; + } + + /** + * @return the characters, or {@code null} for {@link #NONE}. + */ + char[] chars() { + return password; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final var other = ((StorePassword) o).password; + if (password == null || other == null) { + return password == other; + } + if (password.length != other.length) { + return false; + } + var difference = 0; + for (int i = 0; i < password.length; i++) { + difference |= password[i] ^ other[i]; + } + return difference == 0; + } + + @Override + public int hashCode() { + return Arrays.hashCode(password); + } + + @Override + public String toString() { + return password != null ? "***" : ""; + } +} 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 62b4a707a7..2429f427d5 100644 --- a/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java +++ b/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java @@ -14,46 +14,47 @@ import java.nio.file.Path; import java.security.GeneralSecurityException; import java.security.KeyStore; -import java.security.cert.X509Certificate; -import java.util.Arrays; -import java.util.Collections; import java.util.List; -import java.util.Objects; +import java.util.Locale; import java.util.Set; +import java.util.function.Supplier; 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; - import org.opensearch.OpenSearchException; +import org.opensearch.security.support.PemKeyReader; + +import static org.opensearch.security.ssl.util.SSLConfigConstants.DEFAULT_STORE_TYPE; + +public sealed interface TrustStoreConfiguration { + + TrustStoreConfiguration EMPTY_CONFIGURATION = new EmptyTrustStoreConfiguration(); + + /** + * Picks the implementation the configured store {@code type} asks for: a PKCS#11 token when it names one, + * a file-based store otherwise. + * + * @param type store type as configured, {@code null} to detect it from the content of the file + * @param file resolves the trust store file, evaluated only when the type turns out to be file-based - a + * token has no file setting to resolve, and asking for one would fail + */ + static TrustStoreConfiguration buildTrustStoreConfiguration( + final String type, + final Supplier file, + final String alias, + final StorePassword password + ) { + if (Pkcs11TrustStoreConfiguration.TYPE.equalsIgnoreCase(type)) { + return new Pkcs11TrustStoreConfiguration(alias, password); + } + final var path = file.get(); + final var resolvedType = PemKeyReader.extractStoreType(path.toString(), type).toUpperCase(Locale.ROOT); + return new JdkTrustStoreConfiguration(path, resolvedType, alias, password); + } -public interface TrustStoreConfiguration { - - TrustStoreConfiguration EMPTY_CONFIGURATION = new TrustStoreConfiguration() { - @Override - public Path file() { - return null; - } - - @Override - public List loadCertificates() { - return List.of(); - } - - @Override - public KeyStore createTrustStore() { - return null; - } - - @Override - public TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set issuerDns) { - return null; - } - }; - - Path file(); + List files(); List loadCertificates(); @@ -77,110 +78,114 @@ default TrustManagerFactory buildTrustManagerFactory(final KeyStore keyStore) { KeyStore createTrustStore(); - final class JdkTrustStoreConfiguration implements TrustStoreConfiguration { - - private final Path path; - - private final String type; - - private final String alias; + /** + * No trust store configured at all - see {@link #EMPTY_CONFIGURATION}, the only instance worth holding. + * Returning a {@code null} trust manager factory leaves the peer verification to whatever the TLS engine + * defaults to; the empty file list keeps it out of the reload watch. + */ + record EmptyTrustStoreConfiguration() implements TrustStoreConfiguration { - private final char[] password; - - public JdkTrustStoreConfiguration(final Path path, final String type, final String alias, final char[] password) { - this.path = path; - this.type = type; - this.alias = alias; - this.password = password; + @Override + public List files() { + return List.of(); } @Override public List loadCertificates() { - final var keyStore = KeyStoreUtils.loadKeyStore(path, type, password); - final var listBuilder = ImmutableList.builder(); - try { - if (alias != null) { - listBuilder.add(new Certificate((X509Certificate) keyStore.getCertificate(alias), type, alias, false)); - } else { - for (final var a : Collections.list(keyStore.aliases())) { - if (!keyStore.isCertificateEntry(a)) continue; - final var c = keyStore.getCertificate(a); - if (c instanceof X509Certificate) { - listBuilder.add(new Certificate((X509Certificate) c, type, a, false)); - } - } - } - final var list = listBuilder.build(); - if (list.isEmpty()) { - throw new OpenSearchException("The file " + path + " does not contain any certificates"); - } - return listBuilder.build(); - } catch (GeneralSecurityException e) { - throw new OpenSearchException("Couldn't load certificates from file " + path, e); - } + return List.of(); } @Override - public Path file() { - return path; + public KeyStore createTrustStore() { + return null; } @Override - public KeyStore createTrustStore() { - return KeyStoreUtils.loadTrustStore(path, type, alias, password); + public TrustManagerFactory createTrustManagerFactory(boolean validateCertificates, Set issuerDns) { + return null; + } + } + + /** + * A file-based trust store in any type a registered provider offers, e.g. JKS, JCEKS, PKCS12 or BCFKS. + * + * @param path location of the trust store file + * @param type store type, as resolved from the settings or detected from the file + * @param alias optional alias to narrow the trusted certificates down to + * @param password password of the store + */ + record JdkTrustStoreConfiguration(Path path, String type, String alias, StorePassword password) implements TrustStoreConfiguration { + + @Override + public List loadCertificates() { + final var trustStore = KeyStoreUtils.loadKeyStore(path, type, password.chars()); + return KeyStoreUtils.loadTrustedCertificates(trustStore, type, alias, path.toString()); } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - JdkTrustStoreConfiguration that = (JdkTrustStoreConfiguration) o; - return Objects.equals(path, that.path) - && Objects.equals(type, that.type) - && Objects.equals(alias, that.alias) - && Objects.deepEquals(password, that.password); + public List files() { + return List.of(path); } @Override - public int hashCode() { - return Objects.hash(path, type, alias, Arrays.hashCode(password)); + public KeyStore createTrustStore() { + return KeyStoreUtils.loadTrustStore(path, type, alias, password.chars()); } + } - final class PemTrustStoreConfiguration implements TrustStoreConfiguration { + /** + * Trusted certificates held in a PKCS#11 token. There is no file on disk, so nothing to watch for reloads. + * Unlike private keys, certificates can be read out of a token, so narrowing down to {@link #alias()} is + * possible - it is done into an in-memory store rather than the token, which is never written to. + * + * @param alias optional alias to narrow the trusted certificates down to + * @param pin the token PIN, taken from the {@code truststore_password} setting + */ + record Pkcs11TrustStoreConfiguration(String alias, StorePassword pin) implements TrustStoreConfiguration { - private final Path path; + static final String TYPE = PemKeyReader.PKCS11; - public PemTrustStoreConfiguration(final Path path) { - this.path = path; - } + private static final String SOURCE = "PKCS#11 token"; @Override public List loadCertificates() { - return Stream.of(KeyStoreUtils.x509Certificates(path)).map(c -> new Certificate(c, false)).collect(Collectors.toList()); + return KeyStoreUtils.loadTrustedCertificates(KeyStoreUtils.loadPkcs11Store(pin.chars()), TYPE, alias, SOURCE); } @Override - public Path file() { - return path; + public List files() { + return List.of(); } @Override public KeyStore createTrustStore() { - return KeyStoreUtils.newTrustStoreFromPem(path); + final var tokenStore = KeyStoreUtils.loadPkcs11Store(pin.chars()); + return alias != null ? KeyStoreUtils.narrowToAlias(tokenStore, DEFAULT_STORE_TYPE, alias, SOURCE) : tokenStore; + } + + } + + /** + * Trusted certificates in PEM format. + * + * @param path location of the PEM file holding the trusted certificates + */ + record PemTrustStoreConfiguration(Path path) implements TrustStoreConfiguration { + + @Override + public List loadCertificates() { + return Stream.of(KeyStoreUtils.x509Certificates(path)).map(c -> new Certificate(c, false)).collect(Collectors.toList()); } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - PemTrustStoreConfiguration that = (PemTrustStoreConfiguration) o; - return Objects.equals(path, that.path); + public List files() { + return List.of(path); } @Override - public int hashCode() { - return Objects.hashCode(path); + public KeyStore createTrustStore() { + return KeyStoreUtils.newTrustStoreFromPem(path); } } diff --git a/src/main/java/org/opensearch/security/support/FipsMode.java b/src/main/java/org/opensearch/security/support/FipsMode.java new file mode 100644 index 0000000000..169a5d4208 --- /dev/null +++ b/src/main/java/org/opensearch/security/support/FipsMode.java @@ -0,0 +1,26 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.security.support; + +/** + * Single source of truth for FIPS mode detection. + * Set {@code OPENSEARCH_FIPS_MODE=true} in the environment to enable. + */ +public final class FipsMode { + + public static java.util.function.Supplier envSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); + + public static boolean isEnabled() { + return "true".equalsIgnoreCase(envSupplier.get()); + } + + private FipsMode() { + throw new UnsupportedOperationException(); + } +} diff --git a/src/main/java/org/opensearch/security/support/PemKeyReader.java b/src/main/java/org/opensearch/security/support/PemKeyReader.java index e99ee5161c..e2c38d7411 100644 --- a/src/main/java/org/opensearch/security/support/PemKeyReader.java +++ b/src/main/java/org/opensearch/security/support/PemKeyReader.java @@ -54,6 +54,7 @@ import java.security.spec.PKCS8EncodedKeySpec; import java.util.Collection; import java.util.Locale; +import java.util.stream.Stream; import javax.crypto.Cipher; import javax.crypto.EncryptedPrivateKeyInfo; import javax.crypto.NoSuchPaddingException; @@ -84,6 +85,7 @@ public final class PemKeyReader { public static final String JKS = "JKS"; public static final String PKCS12 = "PKCS12"; public static final String BCFKS = "BCFKS"; + public static final String PKCS11 = "PKCS11"; private static byte[] readPrivateKey(File file) throws KeyException { try (final InputStream in = new FileInputStream(file)) { @@ -186,13 +188,30 @@ public static X509Certificate loadCertificateFromStream(InputStream in) throws E } public static KeyStore loadKeyStore(final String storePath, final String keyStorePassword, final String type) throws Exception { - if (storePath == null) { + // A PKCS#11 store lives on the token, not on disk, so it is the one case with no path. + if (storePath == null && !PKCS11.equalsIgnoreCase(type)) { return null; } String storeType = extractStoreType(storePath, type); - - final KeyStore store = KeyStore.getInstance(storeType); - store.load(new FileInputStream(storePath), keyStorePassword == null ? null : keyStorePassword.toCharArray()); + final char[] password = keyStorePassword == null ? null : keyStorePassword.toCharArray(); + final KeyStore store; + if (PKCS11.equalsIgnoreCase(storeType)) { + try { + store = KeyStore.getInstance(storeType); + store.load(null, password); + } catch (Exception e) { + throw new OpenSearchException( + "Failed to initialize PKCS#11 keystore. Ensure a PKCS#11 provider is registered and configured " + + "(e.g. SunPKCS11, IBMPKCS11Impl, or your HSM vendor's provider).", + e + ); + } + } else { + store = KeyStore.getInstance(storeType); + try (final var in = new FileInputStream(storePath)) { + store.load(in, password); + } + } return store; } @@ -358,9 +377,11 @@ public static String extractStoreType(String storePath, String storeType) { if (null == storeType) { storeType = detectStoreType(storePath); } - if (CryptoServicesRegistrar.isInApprovedOnlyMode() && !PemKeyReader.BCFKS.equalsIgnoreCase(storeType)) { + final String finalStoreType = storeType; + if (CryptoServicesRegistrar.isInApprovedOnlyMode() + && Stream.of(PKCS11, BCFKS).noneMatch(it -> it.equalsIgnoreCase(finalStoreType))) { throw new IllegalArgumentException( - storeType.toUpperCase(Locale.ROOT) + " keystores / truststores are not supported in FIPS mode - use BCFKS." + storeType.toUpperCase(Locale.ROOT) + " keystores / truststores are not supported in FIPS mode - use BCFKS or PKCS#11" ); } return storeType; diff --git a/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTest.java b/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTest.java index 0b7d5eec42..38cb51e932 100755 --- a/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTest.java +++ b/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTest.java @@ -174,7 +174,6 @@ public void testLdapAuthenticationSSL() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -194,7 +193,6 @@ public void testLdapAuthenticationSSLPEMFile() throws Exception { ConfigConstants.LDAPS_PEMTRUSTEDCAS_FILEPATH, FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").toFile().getName() ) - .put("verify_hostnames", false) .put("path.home", ".") .put("path.conf", FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").getParent()) .build(); @@ -216,7 +214,6 @@ public void testLdapAuthenticationSSLPEMText() throws Exception { @Test public void testLdapAuthenticationSSLSSLv3() throws Exception { - final Settings settings = Settings.builder() .putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapsPort) .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") @@ -244,7 +241,6 @@ public void testLdapAuthenticationSSLUnknowCipher() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_ciphers", "AAA") .put("path.home", ".") .build(); @@ -266,7 +262,6 @@ public void testLdapAuthenticationSpecialCipherProtocol() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_protocols", "TLSv1.2") .putList("enabled_ssl_ciphers", "TLS_DHE_RSA_WITH_AES_128_CBC_SHA") .put("path.home", ".") @@ -286,7 +281,6 @@ public void testLdapAuthenticationSSLNoKeystore() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -721,7 +715,6 @@ public void testLdapAuthenticationStartTLS() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_START_TLS, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); diff --git a/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTestNewStyleConfig.java b/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTestNewStyleConfig.java index 7fc2c59f98..40f87d55d3 100644 --- a/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTestNewStyleConfig.java +++ b/src/test/java/org/opensearch/security/auth/ldap/LdapBackendTestNewStyleConfig.java @@ -173,7 +173,6 @@ public void testLdapAuthenticationSSL() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -193,7 +192,6 @@ public void testLdapAuthenticationSSLPEMFile() throws Exception { ConfigConstants.LDAPS_PEMTRUSTEDCAS_FILEPATH, FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").toFile().getName() ) - .put("verify_hostnames", false) .put("path.home", ".") .put("path.conf", FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").getParent()) .build(); @@ -216,13 +214,11 @@ public void testLdapAuthenticationSSLPEMText() throws Exception { @Test public void testLdapAuthenticationSSLSSLv3() throws Exception { - final Settings settings = Settings.builder() .putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapsPort) .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_protocols", "SSLv3") .put("path.home", ".") .build(); @@ -244,7 +240,6 @@ public void testLdapAuthenticationSSLUnknownCipher() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_ciphers", "AAA") .put("path.home", ".") .build(); @@ -266,7 +261,6 @@ public void testLdapAuthenticationSpecialCipherProtocol() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_protocols", "TLSv1.2") .putList("enabled_ssl_ciphers", "TLS_DHE_RSA_WITH_AES_128_CBC_SHA") .put("path.home", ".") @@ -286,7 +280,6 @@ public void testLdapAuthenticationSSLNoKeystore() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -570,7 +563,6 @@ public void testLdapAuthenticationStartTLS() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_START_TLS, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); diff --git a/src/test/java/org/opensearch/security/auth/ldap/srv/LdapServer.java b/src/test/java/org/opensearch/security/auth/ldap/srv/LdapServer.java index adeec6b06f..10d9aaccaa 100644 --- a/src/test/java/org/opensearch/security/auth/ldap/srv/LdapServer.java +++ b/src/test/java/org/opensearch/security/auth/ldap/srv/LdapServer.java @@ -18,6 +18,7 @@ import java.io.StringReader; import java.net.BindException; import java.nio.charset.StandardCharsets; +import java.security.KeyStore; import java.util.ArrayList; import java.util.Collection; import java.util.concurrent.TimeUnit; @@ -25,6 +26,7 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Handler; import java.util.logging.LogRecord; +import javax.net.ssl.TrustManagerFactory; import com.google.common.io.CharStreams; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -45,7 +47,6 @@ import com.unboundid.ldif.LDIFReader; import com.unboundid.util.ssl.KeyStoreKeyManager; import com.unboundid.util.ssl.SSLUtil; -import com.unboundid.util.ssl.TrustStoreTrustManager; final class LdapServer { private final static Logger LOG = LogManager.getLogger(LdapServer.class); @@ -115,11 +116,15 @@ private Collection getInMemoryListenerConfigs() throws E Collection listenerConfigs = new ArrayList(); String serverKeyStorePath = FileHelper.resolveStore("ldap/node-0-keystore").path().toFile().getAbsolutePath(); + + KeyStore trustStore = FileHelper.getKeystoreFromClassPath("ldap/truststore", "changeit"); + TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(trustStore); + final SSLUtil serverSSLUtil = new SSLUtil( new KeyStoreKeyManager(serverKeyStorePath, "changeit".toCharArray()), - new TrustStoreTrustManager(serverKeyStorePath) + tmf.getTrustManagers()[0] ); - // final SSLUtil clientSSLUtil = new SSLUtil(new TrustStoreTrustManager(serverKeyStorePath)); ldapPort = SocketUtils.findAvailableTcpPort(); ldapsPort = SocketUtils.findAvailableTcpPort(); diff --git a/src/test/java/org/opensearch/security/auth/ldap2/HostnameAwareConnectionFactoryTest.java b/src/test/java/org/opensearch/security/auth/ldap2/HostnameAwareConnectionFactoryTest.java new file mode 100644 index 0000000000..de6f0667a1 --- /dev/null +++ b/src/test/java/org/opensearch/security/auth/ldap2/HostnameAwareConnectionFactoryTest.java @@ -0,0 +1,52 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.junit.After; +import org.junit.Test; + +import org.ldaptive.Connection; +import org.ldaptive.ConnectionConfig; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class HostnameAwareConnectionFactoryTest { + + @After + public void clearThreadLocal() { + SNISettingTLSSocketFactory.clearContext(); + } + + @Test + public void getConnection_wrapsWithHostnameFromUrl_withoutSettingContextAtBuild() { + HostnameAwareConnectionFactory factory = new HostnameAwareConnectionFactory( + new ConnectionConfig("ldaps://example.com:636"), + "ldaps://example.com:636" + ); + + Connection connection = factory.getConnection(); + + // The connection is wrapped with the hostname parsed from the LDAP URL; the wrapper then + // establishes it as the SNI context at open() — see SniAwareConnectionTest. + assertTrue(connection instanceof SniAwareConnection); + assertEquals("example.com", ((SniAwareConnection) connection).hostname()); + // Building the connection must NOT set the context — the socket isn't created yet. + assertNull(SNISettingTLSSocketFactory.getHostname()); + } + + @Test(expected = IllegalArgumentException.class) + public void getConnection_throwsOnUnparseableUrl() { + new HostnameAwareConnectionFactory(new ConnectionConfig("ldaps://example.com:636"), "not-a-valid-url").getConnection(); + } +} diff --git a/src/test/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactoryClassLoaderTest.java b/src/test/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactoryClassLoaderTest.java new file mode 100644 index 0000000000..944c97e3b4 --- /dev/null +++ b/src/test/java/org/opensearch/security/auth/ldap2/LDAPConnectionFactoryFactoryClassLoaderTest.java @@ -0,0 +1,77 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.junit.Test; + +import org.opensearch.common.settings.Settings; +import org.opensearch.security.auth.ldap.util.ConfigConstants; +import org.opensearch.security.ssl.util.SSLConfigConstants; +import org.opensearch.security.test.helper.file.FileHelper; + +import org.ldaptive.DefaultConnectionFactory; +import org.ldaptive.provider.Provider; +import org.ldaptive.provider.jndi.JndiProviderConfig; +import org.ldaptive.ssl.ThreadLocalTLSSocketFactory; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.Assert.assertThrows; + +public class LDAPConnectionFactoryFactoryClassLoaderTest { + + private static final String SNI_FACTORY = "org.opensearch.security.auth.ldap2.SNISettingTLSSocketFactory"; + + /** Builds a real factory and returns the classloader ldap2 sets on the JNDI provider config. */ + private static ClassLoader factoryProvidedClassLoader() throws Exception { + Settings settings = Settings.builder() + .putList(ConfigConstants.LDAP_HOSTS, "localhost:636") + .put(ConfigConstants.LDAPS_ENABLE_SSL, true) + .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) + .put("path.home", ".") + .build(); + + DefaultConnectionFactory connectionFactory = new LDAPConnectionFactoryFactory(settings, null).createBasicConnectionFactory(); + + @SuppressWarnings("unchecked") + Provider provider = (Provider) connectionFactory.getProvider(); + return provider.getProviderConfig().getClassLoader(); + } + + @Test + public void providerConfig_carriesClassLoaderThatResolvesSniSocketFactory() throws Exception { + ClassLoader factoryClassLoader = factoryProvidedClassLoader(); + assertThat(factoryClassLoader.loadClass(SNI_FACTORY), is(sameInstance(SNISettingTLSSocketFactory.class))); + } + + @Test + public void providerConfig_classLoaderResolvesThreadLocalTlsSocketFactory() throws Exception { + ClassLoader factoryClassLoader = factoryProvidedClassLoader(); + assertThat( + factoryClassLoader.loadClass(ThreadLocalTLSSocketFactory.class.getName()), + is(sameInstance(ThreadLocalTLSSocketFactory.class)) + ); + } + + @Test + public void providerConfig_classLoaderDelegatesUnknownClassesToParent() throws Exception { + ClassLoader factoryClassLoader = factoryProvidedClassLoader(); + assertThrows(ClassNotFoundException.class, () -> factoryClassLoader.loadClass("com.example.DoesNotExist")); + } + + @Test + public void noArgConstructor_resolvesSniSocketFactory() throws Exception { + ClassLoader loader = new SocketFactoryClassLoader(); + assertThat(loader.loadClass(SNI_FACTORY), is(sameInstance(SNISettingTLSSocketFactory.class))); + } +} diff --git a/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestNewStyleConfig2.java b/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestNewStyleConfig2.java index 1127fd80e8..49f3cdc65e 100644 --- a/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestNewStyleConfig2.java +++ b/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestNewStyleConfig2.java @@ -38,7 +38,6 @@ import org.opensearch.security.auth.ldap.util.ConfigConstants; import org.opensearch.security.auth.ldap.util.LdapHelper; import org.opensearch.security.ssl.util.SSLConfigConstants; -import org.opensearch.security.support.WildcardMatcher; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.user.User; @@ -195,7 +194,6 @@ public void testLdapAuthenticationSSL() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -214,7 +212,6 @@ public void testLdapAuthenticationSSLPEMFile() throws Exception { ConfigConstants.LDAPS_PEMTRUSTEDCAS_FILEPATH, FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").toFile().getName() ) - .put("verify_hostnames", false) .put("path.home", ".") .put("path.conf", FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").getParent()) .build(); @@ -237,7 +234,6 @@ public void testLdapAuthenticationSSLPEMText() throws Exception { @Test public void testLdapAuthenticationSSLSSLv3() throws Exception { - final Settings settings = createBaseSettings().putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapsPort) .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) @@ -264,7 +260,6 @@ public void testLdapAuthenticationSSLUnknownCipher() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_ciphers", "AAA") .put("path.home", ".") .build(); @@ -274,14 +269,15 @@ public void testLdapAuthenticationSSLUnknownCipher() throws Exception { Assert.fail("Expected Exception"); } catch (Exception e) { assertThat(e.getCause().getClass(), is(org.ldaptive.provider.ConnectionException.class)); - Assert.assertTrue( - ExceptionUtils.getStackTrace(e), - WildcardMatcher.from("*unsupported*ciphersuite*aaa*").test(ExceptionUtils.getStackTrace(e).toLowerCase()) - ); + Assert.assertTrue(ExceptionUtils.getStackTrace(e).contains(getUnsupportedCipherMessage())); } } + protected String getUnsupportedCipherMessage() { + return "Unsupported CipherSuite: AAA"; + } + @Test public void testLdapAuthenticationSpecialCipherProtocol() throws Exception { @@ -289,7 +285,6 @@ public void testLdapAuthenticationSpecialCipherProtocol() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_protocols", "TLSv1.2") .putList("enabled_ssl_ciphers", "TLS_DHE_RSA_WITH_AES_128_CBC_SHA") .put("path.home", ".") @@ -308,7 +303,6 @@ public void testLdapAuthenticationSSLNoKeystore() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -612,7 +606,6 @@ public void testLdapAuthenticationStartTLS() throws Exception { .put("users.u1.search", "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_START_TLS, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); diff --git a/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestOldStyleConfig2.java b/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestOldStyleConfig2.java index f25c20b19a..11cbefec6b 100755 --- a/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestOldStyleConfig2.java +++ b/src/test/java/org/opensearch/security/auth/ldap2/LdapBackendTestOldStyleConfig2.java @@ -219,7 +219,6 @@ public void testLdapAuthenticationSSL() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -236,7 +235,6 @@ public void testLdapAuthenticationSSLPooled() throws Exception { .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(ConfigConstants.LDAP_POOL_ENABLED, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -255,7 +253,6 @@ public void testLdapAuthenticationSSLPEMFile() throws Exception { ConfigConstants.LDAPS_PEMTRUSTEDCAS_FILEPATH, FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").toFile().getName() ) - .put("verify_hostnames", false) .put("path.home", ".") .put("path.conf", FileHelper.getAbsoluteFilePathFromClassPath("ldap/root-ca.pem").getParent()) .build(); @@ -278,7 +275,6 @@ public void testLdapAuthenticationSSLPEMText() throws Exception { @Test public void testLdapAuthenticationSSLSSLv3() throws Exception { - final Settings settings = createBaseSettings().putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapsPort) .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) @@ -305,7 +301,6 @@ public void testLdapAuthenticationSSLUnknowCipher() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_ciphers", "AAA") .put("path.home", ".") .build(); @@ -314,12 +309,16 @@ public void testLdapAuthenticationSSLUnknowCipher() throws Exception { new LDAPAuthenticationBackend2(settings, null).authenticate(ctx("jacksonm", "secret")); Assert.fail("Expected Exception"); } catch (Exception e) { - assertThat(e.getCause().getClass().toString(), org.ldaptive.provider.ConnectionException.class, is(e.getCause().getClass())); - Assert.assertTrue(ExceptionUtils.getStackTrace(e), EXCEPTION_MATCHER.test(ExceptionUtils.getStackTrace(e).toLowerCase())); + assertThat(e.getCause().getClass(), is(org.ldaptive.provider.ConnectionException.class)); + Assert.assertTrue(ExceptionUtils.getStackTrace(e).contains(getUnsupportedCipherMessage())); } } + protected String getUnsupportedCipherMessage() { + return "Unsupported CipherSuite: AAA"; + } + @Test public void testLdapAuthenticationSpecialCipherProtocol() throws Exception { @@ -327,7 +326,6 @@ public void testLdapAuthenticationSpecialCipherProtocol() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .putList("enabled_ssl_protocols", "TLSv1.2") .putList("enabled_ssl_ciphers", "TLS_DHE_RSA_WITH_AES_128_CBC_SHA") .put("path.home", ".") @@ -346,7 +344,6 @@ public void testLdapAuthenticationSSLNoKeystore() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_SSL, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); @@ -646,7 +643,6 @@ public void testLdapAuthenticationStartTLS() throws Exception { .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})") .put(ConfigConstants.LDAPS_ENABLE_START_TLS, true) .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) - .put("verify_hostnames", false) .put("path.home", ".") .build(); diff --git a/src/test/java/org/opensearch/security/auth/ldap2/LdapMtlsSniAuthenticationTest.java b/src/test/java/org/opensearch/security/auth/ldap2/LdapMtlsSniAuthenticationTest.java new file mode 100644 index 0000000000..5c861cd5e5 --- /dev/null +++ b/src/test/java/org/opensearch/security/auth/ldap2/LdapMtlsSniAuthenticationTest.java @@ -0,0 +1,77 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.opensearch.common.settings.Settings; +import org.opensearch.security.auth.AuthenticationContext; +import org.opensearch.security.auth.ldap.srv.EmbeddedLDAPServer; +import org.opensearch.security.auth.ldap.util.ConfigConstants; +import org.opensearch.security.ssl.util.SSLConfigConstants; +import org.opensearch.security.test.helper.file.FileHelper; +import org.opensearch.security.user.AuthCredentials; +import org.opensearch.security.user.User; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; + +/** + * Proves that SNI hostname is correctly propagated via ThreadLocal when connecting over LDAPS. + * + * The LDAP client connects to "localhost" with explicit hostname verification enabled. + * BouncyCastle requires the hostname from ThreadLocal (set by HostnameAwareConnectionFactory + * before JNDI resolves it to an IP) to verify the server certificate's SAN against "localhost". + * Successful authentication proves SNI was correctly set — without it, BouncyCastle cannot + * determine the expected hostname and hostname verification would fail. + */ +public class LdapMtlsSniAuthenticationTest { + + private static EmbeddedLDAPServer ldapServer; + private static int ldapsPort; + + @BeforeClass + public static void startLdapServer() throws Exception { + ldapServer = new EmbeddedLDAPServer(); + ldapServer.applyLdif("base.ldif"); + ldapsPort = ldapServer.getLdapsPort(); + } + + @AfterClass + public static void stopLdapServer() throws Exception { + if (ldapServer != null) { + ldapServer.stop(); + } + } + + @Test + public void authenticate_succeeds_provingSnIAndHostnameVerification() throws Exception { + Settings settings = Settings.builder() + .putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapsPort) + .put("users.u1.search", "(uid={0})") + .put(ConfigConstants.LDAPS_ENABLE_SSL, true) + .put(ConfigConstants.LDAPS_VERIFY_HOSTNAMES, true) + .put(SSLConfigConstants.SECURITY_SSL_TRANSPORT_TRUSTSTORE_FILEPATH, FileHelper.resolveStore("ldap/truststore").path()) + .put("path.home", ".") + .build(); + + User user = new LDAPAuthenticationBackend2(settings, null).authenticate( + new AuthenticationContext(new AuthCredentials("jacksonm", "secret".getBytes())) + ); + + assertThat(user, is(notNullValue())); + assertThat(user.getName(), is("cn=Michael Jackson,ou=people,o=TEST")); + } +} diff --git a/src/test/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactoryTest.java b/src/test/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactoryTest.java new file mode 100644 index 0000000000..47b2723936 --- /dev/null +++ b/src/test/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactoryTest.java @@ -0,0 +1,204 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import java.net.InetAddress; +import java.net.Socket; +import java.util.List; +import javax.net.ssl.SNIHostName; +import javax.net.ssl.SNIServerName; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + +import org.junit.After; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +public class SNISettingTLSSocketFactoryTest { + + private final SNISettingTLSSocketFactory factory = new SNISettingTLSSocketFactory(null); + + @After + public void clearThreadLocal() { + SNISettingTLSSocketFactory.clearContext(); + } + + // --- configureSocket --- + + @Test + public void configureSocket_setsSni() throws Exception { + SSLSocket socket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory.configure("example.com"); + + factory.configureSocket(socket); + + List serverNames = socket.getSSLParameters().getServerNames(); + assertEquals(1, serverNames.size()); + assertEquals("example.com", ((SNIHostName) serverNames.get(0)).getAsciiName()); + // Endpoint identification (hostname verification) is JNDI's job, not the factory's. + assertNull(socket.getSSLParameters().getEndpointIdentificationAlgorithm()); + } + + @Test + public void configureSocket_skipsSniForIpAddress() throws Exception { + SSLSocket socket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory.configure("192.168.1.1"); + + factory.configureSocket(socket); + + assertNull(socket.getSSLParameters().getServerNames()); + } + + @Test + public void configureSocket_skipsWhenNoHostname() throws Exception { + SSLSocket socket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + + factory.configureSocket(socket); + + assertNull(socket.getSSLParameters().getServerNames()); + assertNull(socket.getSSLParameters().getEndpointIdentificationAlgorithm()); + } + + @Test(expected = IllegalArgumentException.class) + public void configureSocket_throwsOnInvalidHostname() throws Exception { + SSLSocket socket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory.configure("invalid..hostname"); + + factory.configureSocket(socket); + } + + @Test + public void configureSocket_passesThroughNonSslSocket() { + Socket socket = new Socket(); + SNISettingTLSSocketFactory.configure("example.com"); + + Socket result = factory.configureSocket(socket); + + assertSame(socket, result); + } + + // --- cipher suite delegation --- + + @Test + public void getDefaultCipherSuites_delegatesToDelegate() throws Exception { + SSLSocketFactory real = SSLContext.getDefault().getSocketFactory(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(real); + + assertEquals(real.getDefaultCipherSuites(), f.getDefaultCipherSuites()); + } + + @Test + public void getSupportedCipherSuites_delegatesToDelegate() throws Exception { + SSLSocketFactory real = SSLContext.getDefault().getSocketFactory(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(real); + + assertEquals(real.getSupportedCipherSuites(), f.getSupportedCipherSuites()); + } + + // --- createSocket --- + + @Test + public void createSocket_wrappingSocket_configuresSni() throws Exception { + SSLSocket sslSocket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(stubDelegate(sslSocket)); + SNISettingTLSSocketFactory.configure("example.com"); + + Socket result = f.createSocket(new Socket(), "example.com", 636, true); + + assertSame(sslSocket, result); + assertEquals("example.com", ((SNIHostName) ((SSLSocket) result).getSSLParameters().getServerNames().get(0)).getAsciiName()); + } + + @Test + public void createSocket_stringHost_configuresSni() throws Exception { + SSLSocket sslSocket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(stubDelegate(sslSocket)); + SNISettingTLSSocketFactory.configure("example.com"); + + Socket result = f.createSocket("example.com", 636); + + assertSame(sslSocket, result); + assertEquals("example.com", ((SNIHostName) ((SSLSocket) result).getSSLParameters().getServerNames().get(0)).getAsciiName()); + } + + @Test + public void createSocket_stringHostWithLocalAddress_configuresSni() throws Exception { + SSLSocket sslSocket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(stubDelegate(sslSocket)); + SNISettingTLSSocketFactory.configure("example.com"); + + Socket result = f.createSocket("example.com", 636, InetAddress.getLoopbackAddress(), 0); + + assertSame(sslSocket, result); + assertEquals("example.com", ((SNIHostName) ((SSLSocket) result).getSSLParameters().getServerNames().get(0)).getAsciiName()); + } + + @Test + public void createSocket_inetAddress_configuresSni() throws Exception { + SSLSocket sslSocket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(stubDelegate(sslSocket)); + SNISettingTLSSocketFactory.configure("example.com"); + + Socket result = f.createSocket(InetAddress.getLoopbackAddress(), 636); + + assertSame(sslSocket, result); + assertEquals("example.com", ((SNIHostName) ((SSLSocket) result).getSSLParameters().getServerNames().get(0)).getAsciiName()); + } + + @Test + public void createSocket_inetAddressWithLocalAddress_configuresSni() throws Exception { + SSLSocket sslSocket = (SSLSocket) SSLContext.getDefault().getSocketFactory().createSocket(); + SNISettingTLSSocketFactory f = new SNISettingTLSSocketFactory(stubDelegate(sslSocket)); + SNISettingTLSSocketFactory.configure("example.com"); + + Socket result = f.createSocket(InetAddress.getLoopbackAddress(), 636, InetAddress.getLoopbackAddress(), 0); + + assertSame(sslSocket, result); + assertEquals("example.com", ((SNIHostName) ((SSLSocket) result).getSSLParameters().getServerNames().get(0)).getAsciiName()); + } + + private static SSLSocketFactory stubDelegate(Socket socket) { + return new SSLSocketFactory() { + public String[] getDefaultCipherSuites() { + return new String[0]; + } + + public String[] getSupportedCipherSuites() { + return new String[0]; + } + + public Socket createSocket(Socket s, String host, int port, boolean autoClose) { + return socket; + } + + public Socket createSocket(String host, int port) { + return socket; + } + + public Socket createSocket(String host, int port, InetAddress localHost, int localPort) { + return socket; + } + + public Socket createSocket(InetAddress host, int port) { + return socket; + } + + public Socket createSocket(InetAddress address, int port, InetAddress localAddress, int localPort) { + return socket; + } + }; + } +} diff --git a/src/test/java/org/opensearch/security/auth/ldap2/SniAwareConnectionTest.java b/src/test/java/org/opensearch/security/auth/ldap2/SniAwareConnectionTest.java new file mode 100644 index 0000000000..d5bd3fb3ac --- /dev/null +++ b/src/test/java/org/opensearch/security/auth/ldap2/SniAwareConnectionTest.java @@ -0,0 +1,260 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth.ldap2; + +import org.junit.After; +import org.junit.Test; + +import org.ldaptive.BindRequest; +import org.ldaptive.Connection; +import org.ldaptive.ConnectionConfig; +import org.ldaptive.LdapException; +import org.ldaptive.Response; +import org.ldaptive.ResultCode; +import org.ldaptive.control.RequestControl; +import org.ldaptive.provider.ProviderConnection; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class SniAwareConnectionTest { + + private static final String HOST = "example.com"; + + @After + public void clearThreadLocal() { + SNISettingTLSSocketFactory.clearContext(); + } + + // --- socket-creating calls: SNI context is live during the delegate call and cleared afterwards --- + + @Test + public void open_setsSniDuringCall_returnsDelegateResponse_clearsAfter() throws LdapException { + RecordingConnection delegate = new RecordingConnection(); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + + // Before open(), no context is set — the socket isn't created yet. + assertNull(SNISettingTLSSocketFactory.getHostname()); + + Response response = connection.open(); + + assertEquals(HOST, delegate.hostnameAtOpen); // hostname was live during the delegate's open() + assertSame(delegate.response, response); // delegate's response is passed straight through + assertNull(SNISettingTLSSocketFactory.getHostname()); // and cleared afterwards (no ThreadLocal leak) + } + + @Test + public void openWithBindRequest_setsSniDuringCall_forwardsRequest_clearsAfter() throws LdapException { + RecordingConnection delegate = new RecordingConnection(); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + BindRequest request = new BindRequest(); + + Response response = connection.open(request); + + assertEquals(HOST, delegate.hostnameAtOpen); + assertSame(request, delegate.bindRequest); + assertSame(delegate.response, response); + assertNull(SNISettingTLSSocketFactory.getHostname()); + } + + @Test + public void reopen_setsSniDuringCall_returnsDelegateResponse_clearsAfter() throws LdapException { + RecordingConnection delegate = new RecordingConnection(); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + + Response response = connection.reopen(); + + assertEquals(HOST, delegate.hostnameAtOpen); + assertSame(delegate.response, response); + assertNull(SNISettingTLSSocketFactory.getHostname()); + } + + @Test + public void reopenWithBindRequest_setsSniDuringCall_forwardsRequest_clearsAfter() throws LdapException { + RecordingConnection delegate = new RecordingConnection(); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + BindRequest request = new BindRequest(); + + Response response = connection.reopen(request); + + assertEquals(HOST, delegate.hostnameAtOpen); + assertSame(request, delegate.bindRequest); + assertSame(delegate.response, response); + assertNull(SNISettingTLSSocketFactory.getHostname()); + } + + // --- socket-creating calls clear the SNI context even when the delegate fails --- + + @Test + public void open_clearsSniContext_whenDelegateThrows() { + assertSniContextClearedOnFailure(SniAwareConnection::open); + } + + @Test + public void openWithBindRequest_clearsSniContext_whenDelegateThrows() { + assertSniContextClearedOnFailure(connection -> connection.open(new BindRequest())); + } + + @Test + public void reopen_clearsSniContext_whenDelegateThrows() { + assertSniContextClearedOnFailure(SniAwareConnection::reopen); + } + + @Test + public void reopenWithBindRequest_clearsSniContext_whenDelegateThrows() { + assertSniContextClearedOnFailure(connection -> connection.reopen(new BindRequest())); + } + + /** + * Invokes a socket-creating call whose delegate throws, and asserts the failure propagates while + * the SNI context was live during the call and is still cleared afterwards (try-with-resources). + */ + private void assertSniContextClearedOnFailure(SocketCall call) { + RecordingConnection delegate = new RecordingConnection(); + delegate.failure = new LdapException("simulated open failure"); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + + LdapException thrown = assertThrows(LdapException.class, () -> call.invoke(connection)); + + assertSame(delegate.failure, thrown); // the delegate's exception propagates unchanged + assertEquals(HOST, delegate.hostnameAtOpen); // context was live when the delegate ran + assertNull(SNISettingTLSSocketFactory.getHostname()); // and still cleared despite the failure + } + + @FunctionalInterface + private interface SocketCall { + void invoke(SniAwareConnection connection) throws LdapException; + } + + // --- pass-through methods: no SNI context, plain delegation --- + + @Test + public void hostname_returnsConfiguredHostname() { + assertEquals(HOST, new SniAwareConnection(new RecordingConnection(), HOST).hostname()); + } + + @Test + public void getConnectionConfig_delegates() { + RecordingConnection delegate = new RecordingConnection(); + assertSame(delegate.connectionConfig, new SniAwareConnection(delegate, HOST).getConnectionConfig()); + } + + @Test + public void isOpen_delegates() { + RecordingConnection delegate = new RecordingConnection(); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + + delegate.open = true; + assertTrue(connection.isOpen()); + + delegate.open = false; + assertFalse(connection.isOpen()); + } + + @Test + public void getProviderConnection_delegates() { + RecordingConnection delegate = new RecordingConnection(); + SniAwareConnection connection = new SniAwareConnection(delegate, HOST); + + assertNull(connection.getProviderConnection()); + assertTrue(delegate.getProviderConnectionCalled); + } + + @Test + public void close_delegates() { + RecordingConnection delegate = new RecordingConnection(); + new SniAwareConnection(delegate, HOST).close(); + assertEquals(1, delegate.closeCalls); + } + + @Test + public void closeWithControls_delegates() { + RecordingConnection delegate = new RecordingConnection(); + RequestControl[] controls = new RequestControl[0]; + + new SniAwareConnection(delegate, HOST).close(controls); + + assertSame(controls, delegate.closeControls); + } + + /** + * Recording ldaptive {@link Connection} double: captures the SNI hostname observed during + * open()/reopen() and records the arguments/invocations of the pass-through methods. + */ + private static final class RecordingConnection implements Connection { + final Response response = new Response<>(null, ResultCode.SUCCESS); + final ConnectionConfig connectionConfig = new ConnectionConfig("ldaps://example.com:636"); + String hostnameAtOpen; + BindRequest bindRequest; + boolean open; + boolean getProviderConnectionCalled; + int closeCalls; + RequestControl[] closeControls; + LdapException failure; + + @Override + public Response open() throws LdapException { + hostnameAtOpen = SNISettingTLSSocketFactory.getHostname(); + if (failure != null) { + throw failure; + } + return response; + } + + @Override + public Response open(BindRequest request) throws LdapException { + bindRequest = request; + return open(); + } + + @Override + public Response reopen() throws LdapException { + return open(); + } + + @Override + public Response reopen(BindRequest request) throws LdapException { + bindRequest = request; + return open(); + } + + @Override + public ConnectionConfig getConnectionConfig() { + return connectionConfig; + } + + @Override + public boolean isOpen() { + return open; + } + + @Override + public ProviderConnection getProviderConnection() { + getProviderConnectionCalled = true; + return null; + } + + @Override + public void close() { + closeCalls++; + } + + @Override + public void close(RequestControl[] controls) { + closeControls = controls; + } + } +} diff --git a/src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java b/src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java index 7fb8fd43a6..da290a1f28 100644 --- a/src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java +++ b/src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java @@ -33,11 +33,13 @@ import org.opensearch.security.ssl.config.CertType; import org.opensearch.security.ssl.config.KeyStoreConfiguration; import org.opensearch.security.ssl.config.SslParameters; +import org.opensearch.security.ssl.config.StorePassword; import org.opensearch.security.ssl.config.TrustStoreConfiguration; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.opensearch.security.ssl.CertificatesUtils.privateKeyToPemObject; import static org.opensearch.security.ssl.CertificatesUtils.writePemContent; import static org.junit.Assert.assertThrows; @@ -313,6 +315,23 @@ public void reloadSslContextForShuffledSameSans() throws Exception { assertThat("Context reloaded", is(not(sslContextBefore.equals(sslContextHandler.sslContext())))); } + @Test + public void dependentFilesOfConfigurationWithoutTrustStoreContainNoNulls() { + final var sslParameters = SslParameters.loader(CertType.TRANSPORT, Settings.EMPTY).load(); + final var keyStoreConfiguration = new KeyStoreConfiguration.PemKeyStoreConfiguration( + accessCertificatePath, + accessCertificatePrivateKeyPath, + StorePassword.of(certificatesRule.privateKeyPassword().toCharArray()) + ); + final var sslConfiguration = new SslConfiguration( + sslParameters, + TrustStoreConfiguration.EMPTY_CONFIGURATION, + keyStoreConfiguration + ); + + assertThat(sslConfiguration.dependentFiles(), contains(accessCertificatePath, accessCertificatePrivateKeyPath)); + } + List shuffledSans(Extension currentSans) { final var san1Sequence = ASN1Sequence.getInstance(currentSans.getParsedValue().toASN1Primitive()); @@ -333,7 +352,7 @@ SslContextHandler sslContextHandler() { final var keyStoreConfiguration = new KeyStoreConfiguration.PemKeyStoreConfiguration( accessCertificatePath, accessCertificatePrivateKeyPath, - certificatesRule.privateKeyPassword().toCharArray() + StorePassword.of(certificatesRule.privateKeyPassword().toCharArray()) ); SslConfiguration sslConfiguration = new SslConfiguration(sslParameters, trustStoreConfiguration, keyStoreConfiguration); diff --git a/src/test/java/org/opensearch/security/ssl/SslSettingsManagerReloadListenerTest.java b/src/test/java/org/opensearch/security/ssl/SslSettingsManagerReloadListenerTest.java index 8f02f16613..65fb5a966d 100644 --- a/src/test/java/org/opensearch/security/ssl/SslSettingsManagerReloadListenerTest.java +++ b/src/test/java/org/opensearch/security/ssl/SslSettingsManagerReloadListenerTest.java @@ -23,6 +23,7 @@ import java.util.concurrent.TimeUnit; import com.carrotsearch.randomizedtesting.RandomizedTest; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import org.awaitility.Awaitility; import org.junit.After; import org.junit.Before; @@ -36,6 +37,8 @@ import org.opensearch.env.Environment; import org.opensearch.env.TestEnvironment; import org.opensearch.security.ssl.config.CertType; +import org.opensearch.security.util.BCFipsEntropyDaemonFilter; +import org.opensearch.test.BouncyCastleThreadFilter; import org.opensearch.threadpool.TestThreadPool; import org.opensearch.threadpool.ThreadPool; import org.opensearch.watcher.ResourceWatcherService; @@ -54,6 +57,7 @@ import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_TYPE; import static org.opensearch.transport.AuxTransport.AUX_TRANSPORT_TYPES_SETTING; +@ThreadLeakFilters(filters = { BouncyCastleThreadFilter.class, BCFipsEntropyDaemonFilter.class }) public class SslSettingsManagerReloadListenerTest extends RandomizedTest { @ClassRule diff --git a/src/test/java/org/opensearch/security/ssl/SslSettingsManagerTest.java b/src/test/java/org/opensearch/security/ssl/SslSettingsManagerTest.java index 8e4131e173..f5bf5ff259 100644 --- a/src/test/java/org/opensearch/security/ssl/SslSettingsManagerTest.java +++ b/src/test/java/org/opensearch/security/ssl/SslSettingsManagerTest.java @@ -16,6 +16,7 @@ import java.util.Locale; import com.carrotsearch.randomizedtesting.RandomizedTest; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -26,6 +27,8 @@ import org.opensearch.env.Environment; import org.opensearch.env.TestEnvironment; import org.opensearch.security.ssl.config.CertType; +import org.opensearch.security.util.BCFipsEntropyDaemonFilter; +import org.opensearch.test.BouncyCastleThreadFilter; import io.netty.handler.ssl.ClientAuth; import io.netty.handler.ssl.SslContext; @@ -66,6 +69,7 @@ import static org.opensearch.transport.AuxTransport.AUX_TRANSPORT_TYPES_SETTING; import static org.junit.Assert.assertThrows; +@ThreadLeakFilters(filters = { BouncyCastleThreadFilter.class, BCFipsEntropyDaemonFilter.class }) public class SslSettingsManagerTest extends RandomizedTest { @ClassRule diff --git a/src/test/java/org/opensearch/security/ssl/config/JdkSslCertificatesLoaderTest.java b/src/test/java/org/opensearch/security/ssl/config/JdkSslCertificatesLoaderTest.java index 9a00d1ca59..8613f85cd7 100644 --- a/src/test/java/org/opensearch/security/ssl/config/JdkSslCertificatesLoaderTest.java +++ b/src/test/java/org/opensearch/security/ssl/config/JdkSslCertificatesLoaderTest.java @@ -17,24 +17,32 @@ import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.function.Function; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; import org.junit.Test; +import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.MockSecureSettings; import org.opensearch.env.TestEnvironment; +import org.opensearch.test.MockLogAppender; import static java.util.Objects.isNull; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.opensearch.security.ssl.util.SSLConfigConstants.DEFAULT_STORE_PASSWORD; import static org.opensearch.security.ssl.util.SSLConfigConstants.DEFAULT_STORE_TYPE; import static org.opensearch.security.ssl.util.SSLConfigConstants.ENABLED; import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_ALIAS; import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_FILEPATH; +import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_PASSWORD; import static org.opensearch.security.ssl.util.SSLConfigConstants.KEYSTORE_TYPE; +import static org.opensearch.security.ssl.util.SSLConfigConstants.PEM_TRUSTED_CAS_FILEPATH; import static org.opensearch.security.ssl.util.SSLConfigConstants.SECURITY_SSL_TRANSPORT_CLIENT_KEYSTORE_ALIAS; import static org.opensearch.security.ssl.util.SSLConfigConstants.SECURITY_SSL_TRANSPORT_CLIENT_TRUSTSTORE_ALIAS; import static org.opensearch.security.ssl.util.SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED; @@ -52,10 +60,11 @@ import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_ALIAS; import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_FILEPATH; import static org.opensearch.security.ssl.util.SSLConfigConstants.TRUSTSTORE_TYPE; +import static org.junit.Assert.assertThrows; public class JdkSslCertificatesLoaderTest extends SslCertificatesLoaderTest { - static final Function resolveKeyStoreType = s -> isNull(s) ? DEFAULT_STORE_TYPE : s; + static final Function resolveKeyStoreType = s -> isNull(s) ? DEFAULT_STORE_TYPE : s.toUpperCase(Locale.ROOT); static final String SERVER_TRUSTSTORE_ALIAS = "server-truststore-alias"; @@ -180,6 +189,72 @@ public void loadTransportJdkBasedSslExtendedConfiguration() throws Exception { ); } + @Test + public void failsWhenBothLegacyAndSecureKeyStorePasswordsAreSet() throws Exception { + final var keyStoreType = randomKeyStoreType(); + final var keyStorePath = createKeyStore( + keyStoreType, + DEFAULT_STORE_PASSWORD, + Map.of( + "default-keystore-alias", + Tuple.tuple(certificatesRule.accessCertificatePrivateKey(), certificatesRule.x509AccessCertificate()) + ) + ); + + final var secureSettings = new MockSecureSettings(); + secureSettings.setString(SSL_HTTP_PREFIX + "keystore_password_secure", DEFAULT_STORE_PASSWORD); + + final var settings = defaultSettingsBuilder().put(SSL_HTTP_PREFIX + ENABLED, true) + .put(SSL_HTTP_PREFIX + KEYSTORE_FILEPATH, keyStorePath) + .put(SSL_HTTP_PREFIX + KEYSTORE_TYPE, keyStoreType) + .put(SSL_HTTP_PREFIX + KEYSTORE_PASSWORD, DEFAULT_STORE_PASSWORD) // the legacy setting + .setSecureSettings(secureSettings) // the secure setting + .build(); + + final var e = assertThrows( + OpenSearchException.class, + () -> new SslCertificatesLoader(SSL_HTTP_PREFIX).loadConfiguration(TestEnvironment.newEnvironment(settings)) + ); + assertThat(e.getMessage(), containsString("must be set not both")); + } + + @Test + public void warnsThatPemTrustedCasAreIgnoredAlongsideAKeyStore() throws Exception { + final var keyStoreType = randomKeyStoreType(); + final var keyStorePath = createKeyStore( + keyStoreType, + DEFAULT_STORE_PASSWORD, + Map.of( + "default-keystore-alias", + Tuple.tuple(certificatesRule.accessCertificatePrivateKey(), certificatesRule.x509AccessCertificate()) + ) + ); + + final var settings = defaultSettingsBuilder().put(SSL_HTTP_PREFIX + ENABLED, true) + .put(SSL_HTTP_PREFIX + KEYSTORE_FILEPATH, keyStorePath) + .put(SSL_HTTP_PREFIX + KEYSTORE_TYPE, keyStoreType) + .put(SSL_HTTP_PREFIX + PEM_TRUSTED_CAS_FILEPATH, "root-ca.pem") + .build(); + + try (final var appender = MockLogAppender.createForLoggers(LogManager.getLogger(SslCertificatesLoader.class))) { + appender.addExpectation( + new MockLogAppender.SeenEventExpectation( + "names the ignored setting and the one to configure instead", + LOGGER_NAME, + Level.WARN, + "*" + SSL_HTTP_PREFIX + PEM_TRUSTED_CAS_FILEPATH + "*" + SSL_HTTP_PREFIX + TRUSTSTORE_FILEPATH + "*" + ) + ); + + final var configuration = new SslCertificatesLoader(SSL_HTTP_PREFIX).loadConfiguration( + TestEnvironment.newEnvironment(settings) + ); + + assertThat(configuration.v1(), is(TrustStoreConfiguration.EMPTY_CONFIGURATION)); + appender.assertAllExpectationsMatched(); + } + } + private void testJdkBasedSslConfiguration(final String sslConfigPrefix, final boolean useAuthorityCertificate) throws Exception { final var useSecurePassword = randomBoolean(); @@ -270,7 +345,7 @@ private void testJdkBasedSslConfiguration(final String sslConfigPrefix, final bo } String randomKeyStoreType() { - return randomFrom(new String[] { "jks", "pkcs12", null }); + return randomFrom(new String[] { "bcfks", "jks", "pkcs12", null }); } String randomKeyStorePassword(final boolean useSecurePassword) { @@ -282,7 +357,7 @@ Path createTrustStore(final String type, final String password, Map")); + } + + @Test + public void configurationsHoldingEqualPasswordsAreEqual() { + final var one = new KeyStoreConfiguration.JdkKeyStoreConfiguration( + Path.of("keystore.bcfks"), + "BCFKS", + "alias", + StorePassword.of("changeit".toCharArray()), + StorePassword.of("changeit".toCharArray()) + ); + final var another = new KeyStoreConfiguration.JdkKeyStoreConfiguration( + Path.of("keystore.bcfks"), + "BCFKS", + "alias", + StorePassword.of("changeit".toCharArray()), + StorePassword.of("changeit".toCharArray()) + ); + + assertThat(one, is(another)); + assertThat(one.toString(), is(not(containsString("changeit")))); + } +} diff --git a/src/test/java/org/opensearch/security/support/FipsModeTest.java b/src/test/java/org/opensearch/security/support/FipsModeTest.java new file mode 100644 index 0000000000..06f5ebe6ad --- /dev/null +++ b/src/test/java/org/opensearch/security/support/FipsModeTest.java @@ -0,0 +1,55 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.security.support; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertThrows; + +public class FipsModeTest { + + private java.util.function.Supplier originalSupplier; + + @Before + public void saveSupplier() { + originalSupplier = FipsMode.envSupplier; + } + + @After + public void restoreSupplier() { + FipsMode.envSupplier = originalSupplier; + } + + @Test + public void isEnabled_isTrueOnlyForCaseInsensitiveTrueValue() { + for (String enabled : new String[] { "true", "TRUE", "True" }) { + FipsMode.envSupplier = () -> enabled; + assertThat("expected enabled for: " + enabled, FipsMode.isEnabled(), equalTo(true)); + } + for (String disabled : new String[] { "false", null, "", "yes" }) { + FipsMode.envSupplier = () -> disabled; + assertThat("expected disabled for: " + disabled, FipsMode.isEnabled(), equalTo(false)); + } + } + + @Test + public void constructor_isNotInstantiable() throws Exception { + Constructor constructor = FipsMode.class.getDeclaredConstructor(); + constructor.setAccessible(true); + Exception ex = assertThrows(InvocationTargetException.class, constructor::newInstance); + assertThat(ex.getCause(), instanceOf(UnsupportedOperationException.class)); + } +} diff --git a/src/test/java/org/opensearch/security/util/BCFipsEntropyDaemonFilter.java b/src/test/java/org/opensearch/security/util/BCFipsEntropyDaemonFilter.java new file mode 100644 index 0000000000..5fb39b0160 --- /dev/null +++ b/src/test/java/org/opensearch/security/util/BCFipsEntropyDaemonFilter.java @@ -0,0 +1,22 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.security.util; + +import com.carrotsearch.randomizedtesting.ThreadFilter; + +/** + * Thread-leak filter for the "BC FIPS Entropy Daemon", which the framework's {@code BouncyCastleThreadFilter} + * does not yet cover. Shared by tests that touch BC FIPS keystores/crypto under {@code RandomizedRunner}. + */ +public class BCFipsEntropyDaemonFilter implements ThreadFilter { + @Override + public boolean reject(Thread t) { + return "BC FIPS Entropy Daemon".equals(t.getName()); + } +} diff --git a/src/test/resources/fips-jul-test-logging.properties b/src/test/resources/fips-jul-test-logging.properties new file mode 100644 index 0000000000..3fb4ae3d2c --- /dev/null +++ b/src/test/resources/fips-jul-test-logging.properties @@ -0,0 +1,10 @@ +# java.util.logging (JUL) config for FIPS test workers that use the default LogManager +# (the :test / citest / sharded test tasks). Points java.util.logging.config.file here. +# +# BC FIPS JSSE (org.bouncycastle.jsse.provider.ProvTls{Server,Client}) logs every TLS +# handshake at INFO via JUL, flooding worker stderr under FIPS. Raise its level to WARNING +# so genuine handshake failures still surface while the per-handshake chatter is silenced. +handlers=java.util.logging.ConsoleHandler +.level=INFO +java.util.logging.ConsoleHandler.level=INFO +org.bouncycastle.jsse.level=WARNING diff --git a/src/test/resources/fips-jvm-truststore.bcfks b/src/test/resources/fips-jvm-truststore.bcfks new file mode 100644 index 0000000000..22f4b6c455 Binary files /dev/null and b/src/test/resources/fips-jvm-truststore.bcfks differ diff --git a/src/test/resources/fips_java_test.security b/src/test/resources/fips_java_test.security new file mode 100644 index 0000000000..77bbea2fa9 --- /dev/null +++ b/src/test/resources/fips_java_test.security @@ -0,0 +1,58 @@ +# Security properties for FIPS test runs. +# Used with == (complete override): only the providers listed here are available. +# SunJCE is intentionally absent — this removes DES, MD5-based PBE, and other +# non-FIPS algorithms without requiring programmatic Security.removeProvider() calls. + +security.provider.1=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider C:HYBRID;ENABLE{All}; +security.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider fips:BCFIPS +security.provider.3=SunPKCS11 +security.provider.4=SUN +security.provider.5=SunJGSS +security.provider.6=JdkLDAP + +login.configuration.provider=sun.security.provider.ConfigFile + +policy.expandProperties=true +policy.allowSystemProperty=true + +keystore.type=BCFKS +keystore.type.compat=true + +ssl.KeyManagerFactory.algorithm=PKIX +ssl.TrustManagerFactory.algorithm=PKIX + +jdk.certpath.disabledAlgorithms=MD2, MD5, SHA1 jdkCA & usage TLSServer, \ + RSA keySize < 1024, DSA keySize < 1024, EC keySize < 224, \ + SHA1 usage SignedJAR & denyAfter 2019-01-01 + +jdk.security.legacyAlgorithms=SHA1, \ + RSA keySize < 2048, DSA keySize < 2048, \ + DES, DESede, MD5, RC2, ARCFOUR + +jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, \ + DSA keySize < 1024, SHA1 denyAfter 2019-01-01 + +jdk.tls.disabledAlgorithms=SSLv3, TLSv1, TLSv1.1, DTLSv1.0, RC4, DES, \ + MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, \ + ECDH, TLS_RSA_* + +jdk.tls.legacyAlgorithms=NULL, anon, RC4, DES, 3DES_EDE_CBC + +jdk.tls.keyLimits=AES/GCM/NoPadding KeyUpdate 2^37, \ + ChaCha20-Poly1305 KeyUpdate 2^37 + +crypto.policy=unlimited + +jdk.security.caDistrustPolicies=SYMANTEC_TLS,ENTRUST_TLS,CAMERFIRMA_TLS + +jdk.tls.alpnCharset=ISO_8859_1 + +# Revocation via BCTLS TrustManager (covers all TLS including LDAPS) +com.sun.net.ssl.checkRevocation=true + +# BC FIPS CertPath revocation mechanisms +ocsp.enable=true +org.bouncycastle.x509.enableCRLDP=true + +# OCSP stapling — request stapled response from server +jdk.tls.client.enableStatusRequestExtension=true diff --git a/src/test/resources/java_test.security b/src/test/resources/java_test.security new file mode 100644 index 0000000000..82cf1ed27c --- /dev/null +++ b/src/test/resources/java_test.security @@ -0,0 +1,55 @@ +# Security properties for non-FIPS test runs. +# Used with == (complete override) so all desired providers must be listed explicitly. +# Standard JDK providers are preserved in their default order; BCFIPS is appended last +# so it is available for tests that rely on BC-specific features without displacing JDK providers. + +security.provider.1=SUN +security.provider.2=SunRsaSign +security.provider.3=SunEC +security.provider.4=SunJSSE +security.provider.5=SunJCE +security.provider.6=SunJGSS +security.provider.7=SunSASL +security.provider.8=XMLDSig +security.provider.9=SunPCSC +security.provider.10=JdkLDAP +security.provider.11=JdkSASL +security.provider.12=SunPKCS11 +security.provider.13=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider C:HYBRID;ENABLE{All}; + +login.configuration.provider=sun.security.provider.ConfigFile + +policy.expandProperties=true +policy.allowSystemProperty=true + +keystore.type=pkcs12 +keystore.type.compat=true + +ssl.KeyManagerFactory.algorithm=SunX509 +ssl.TrustManagerFactory.algorithm=PKIX + +jdk.certpath.disabledAlgorithms=MD2, MD5, SHA1 jdkCA & usage TLSServer, \ + RSA keySize < 1024, DSA keySize < 1024, EC keySize < 224, \ + SHA1 usage SignedJAR & denyAfter 2019-01-01 + +jdk.security.legacyAlgorithms=SHA1, \ + RSA keySize < 2048, DSA keySize < 2048, \ + DES, DESede, MD5, RC2, ARCFOUR + +jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, \ + DSA keySize < 1024, SHA1 denyAfter 2019-01-01 + +jdk.tls.disabledAlgorithms=SSLv3, TLSv1, TLSv1.1, DTLSv1.0, RC4, DES, \ + MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, \ + ECDH, TLS_RSA_* + +jdk.tls.legacyAlgorithms=NULL, anon, RC4, DES, 3DES_EDE_CBC + +jdk.tls.keyLimits=AES/GCM/NoPadding KeyUpdate 2^37, \ + ChaCha20-Poly1305 KeyUpdate 2^37 + +crypto.policy=unlimited + +jdk.security.caDistrustPolicies=SYMANTEC_TLS,ENTRUST_TLS,CAMERFIRMA_TLS + +jdk.tls.alpnCharset=ISO_8859_1 diff --git a/src/test/resources/ldap/config.yml b/src/test/resources/ldap/config.yml index b7a099a36c..e327d85536 100644 --- a/src/test/resources/ldap/config.yml +++ b/src/test/resources/ldap/config.yml @@ -40,7 +40,6 @@ config: hosts: "localhost:${ldapsPort}" usersearch: "(uid={0})" enable_ssl: true - verify_hostnames: false description: "Migrated from v6" authz: {} do_not_fail_on_forbidden: false diff --git a/src/test/resources/ldap/config_ldap2.yml b/src/test/resources/ldap/config_ldap2.yml index cea994dd02..5ef6da0a4a 100644 --- a/src/test/resources/ldap/config_ldap2.yml +++ b/src/test/resources/ldap/config_ldap2.yml @@ -40,7 +40,6 @@ config: hosts: "localhost:${ldapsPort}" usersearch: "(uid={0})" enable_ssl: true - verify_hostnames: false description: "Migrated from v6" authz: {} do_not_fail_on_forbidden: false diff --git a/src/test/resources/ldap/test1.yml b/src/test/resources/ldap/test1.yml index e0ad96ceea..c6d926b68f 100644 --- a/src/test/resources/ldap/test1.yml +++ b/src/test/resources/ldap/test1.yml @@ -110,4 +110,3 @@ pemtrustedcas_content: | pTIIfrQcZ1vrDg0lYzVgQ1iT -----END CERTIFICATE----- usersearch: "(uid={0})" -verify_hostnames: false