From 22b62ce5f6fc52c3c4644347b460d55182e23b9a Mon Sep 17 00:00:00 2001 From: Iwan Igonin Date: Mon, 23 Sep 2024 11:12:07 +0200 Subject: [PATCH] fixing additional tests. Signed-off-by: Iwan Igonin --- .../client/FipsEnabledThreadFactory.java | 36 ------------------- .../opensearch/client/RestClientBuilder.java | 12 ++++++- .../client/RestClientBuilderIntegTests.java | 13 +++++-- .../licenses/bcpg-fips-2.0.8.jar.sha1 | 1 + .../licenses/bcpg-fips-2.0.9.jar.sha1 | 1 - .../common/crypto/KeyStoreType.java | 3 ++ .../common/ssl/SslConfigurationLoader.java | 6 ++-- .../reindex/ReindexRestClientSslTests.java | 14 ++++++-- .../netty4/Netty4ClientYamlTestSuiteIT.java | 5 --- .../reactor/netty4/ReactorHttpClient.java | 13 ++++++- .../nodes.reload_secure_settings/10_basic.yml | 3 ++ .../action/admin/ReloadSecureSettingsIT.java | 14 +++++--- .../org/opensearch/bootstrap/security.policy | 1 + .../junit/listeners/ReproduceInfoPrinter.java | 2 ++ 14 files changed, 67 insertions(+), 57 deletions(-) delete mode 100644 client/rest/src/main/java/org/opensearch/client/FipsEnabledThreadFactory.java create mode 100644 distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.8.jar.sha1 delete mode 100644 distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.9.jar.sha1 diff --git a/client/rest/src/main/java/org/opensearch/client/FipsEnabledThreadFactory.java b/client/rest/src/main/java/org/opensearch/client/FipsEnabledThreadFactory.java deleted file mode 100644 index 2efdc48c70f59..0000000000000 --- a/client/rest/src/main/java/org/opensearch/client/FipsEnabledThreadFactory.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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.client; - -import org.apache.hc.core5.concurrent.DefaultThreadFactory; -import org.bouncycastle.crypto.CryptoServicesRegistrar; - -import java.util.concurrent.ThreadFactory; - -public class FipsEnabledThreadFactory implements ThreadFactory { - - private final ThreadFactory defaultFactory; - private final boolean isFipsEnabled; - - public FipsEnabledThreadFactory(String namePrefix, boolean isFipsEnabled) { - this.defaultFactory = new DefaultThreadFactory(namePrefix); - this.isFipsEnabled = isFipsEnabled; - } - - @Override - public Thread newThread(final Runnable target) { - return defaultFactory.newThread(() -> { - if (isFipsEnabled) { - CryptoServicesRegistrar.setApprovedOnlyMode(true); - } - target.run(); - }); - } - -} diff --git a/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java b/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java index 0a46da6e1e8b0..325c7b0c0fbb8 100644 --- a/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java @@ -332,10 +332,20 @@ public TlsDetails create(final SSLEngine sslEngine) { .setTlsStrategy(tlsStrategy) .build(); + var inFipsJvm = CryptoServicesRegistrar.isInApprovedOnlyMode(); + HttpAsyncClientBuilder httpClientBuilder = HttpAsyncClientBuilder.create() .setDefaultRequestConfig(requestConfigBuilder.build()) .setConnectionManager(connectionManager) - .setThreadFactory(new FipsEnabledThreadFactory("os-client-dispatcher", CryptoServicesRegistrar.isInApprovedOnlyMode())) + .setThreadFactory((Runnable r) -> { + Runnable runnable = () -> { + if (inFipsJvm) { + CryptoServicesRegistrar.setApprovedOnlyMode(true); + } + r.run(); + }; + return new Thread(runnable, "os-client-dispatcher"); + }) .setTargetAuthenticationStrategy(DefaultAuthenticationStrategy.INSTANCE) .disableAutomaticRetries(); if (httpClientConfigCallback != null) { diff --git a/client/rest/src/test/java/org/opensearch/client/RestClientBuilderIntegTests.java b/client/rest/src/test/java/org/opensearch/client/RestClientBuilderIntegTests.java index db578ebb196ac..aab9e008ca708 100644 --- a/client/rest/src/test/java/org/opensearch/client/RestClientBuilderIntegTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RestClientBuilderIntegTests.java @@ -39,6 +39,7 @@ import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.ssl.SSLContextBuilder; +import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.opensearch.common.crypto.KeyStoreFactory; import org.opensearch.common.crypto.KeyStoreType; import org.junit.AfterClass; @@ -79,8 +80,16 @@ public static void startHttpServer() throws Exception { httpsServer = HttpsServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); httpsServer.setHttpsConfigurator(new HttpsConfigurator(getSslContext(true))); httpsServer.createContext("/", new ResponseHandler()); - var threadFactory = new FipsEnabledThreadFactory("test-httpserver-dispatch", inFipsJvm()); - Executor executor = Executors.newFixedThreadPool(1, threadFactory); + var inFipsJvm = inFipsJvm(); + Executor executor = Executors.newFixedThreadPool(1, (Runnable r) -> { + Runnable runnable = () -> { + if (inFipsJvm) { + CryptoServicesRegistrar.setApprovedOnlyMode(true); + } + r.run(); + }; + return new Thread(runnable, "test-httpserver-dispatcher"); + }); httpsServer.setExecutor(executor); httpsServer.start(); } diff --git a/distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.8.jar.sha1 b/distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.8.jar.sha1 new file mode 100644 index 0000000000000..758ee2fdf9de6 --- /dev/null +++ b/distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.8.jar.sha1 @@ -0,0 +1 @@ +51c2f633e0c32d10de1ebab4c86f93310ff820f8 \ No newline at end of file diff --git a/distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.9.jar.sha1 b/distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.9.jar.sha1 deleted file mode 100644 index 20cdbf6dc8aa8..0000000000000 --- a/distribution/tools/plugin-cli/licenses/bcpg-fips-2.0.9.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -f69719ef8dbf34d5f906ce480496446b2fd2ae27 \ No newline at end of file diff --git a/libs/common/src/main/java/org/opensearch/common/crypto/KeyStoreType.java b/libs/common/src/main/java/org/opensearch/common/crypto/KeyStoreType.java index d92b8067d3368..2b458d431215f 100644 --- a/libs/common/src/main/java/org/opensearch/common/crypto/KeyStoreType.java +++ b/libs/common/src/main/java/org/opensearch/common/crypto/KeyStoreType.java @@ -13,6 +13,9 @@ import java.util.Map; import java.util.stream.Stream; +/** + * Enum representing the types of KeyStores supported by {@link KeyStoreFactory}. + */ public enum KeyStoreType { JKS("JKS"), diff --git a/libs/ssl-config/src/main/java/org/opensearch/common/ssl/SslConfigurationLoader.java b/libs/ssl-config/src/main/java/org/opensearch/common/ssl/SslConfigurationLoader.java index 4deeed2bbc45e..98bc97c1c8787 100644 --- a/libs/ssl-config/src/main/java/org/opensearch/common/ssl/SslConfigurationLoader.java +++ b/libs/ssl-config/src/main/java/org/opensearch/common/ssl/SslConfigurationLoader.java @@ -247,8 +247,7 @@ private SslTrustConfig buildTrustConfig(Path basePath, SslVerificationMode verif if (trustStorePath != null) { final char[] password = resolvePasswordSetting(TRUSTSTORE_SECURE_PASSWORD, TRUSTSTORE_LEGACY_PASSWORD); final Optional maybeStoreType = Optional.ofNullable(resolveSetting(TRUSTSTORE_TYPE, Function.identity(), null)); - final KeyStoreType storeType = maybeStoreType - .map(KeyStoreType::getByJcaName) + final KeyStoreType storeType = maybeStoreType.map(KeyStoreType::getByJcaName) .orElse(inferStoreType(trustStorePath.toString().toLowerCase(Locale.ROOT))); final String algorithm = resolveSetting(TRUSTSTORE_ALGORITHM, Function.identity(), TrustManagerFactory.getDefaultAlgorithm()); @@ -291,8 +290,7 @@ private SslKeyConfig buildKeyConfig(Path basePath) { } final Optional maybeStoreType = Optional.ofNullable(resolveSetting(KEYSTORE_TYPE, Function.identity(), null)); - final KeyStoreType storeType = maybeStoreType - .map(KeyStoreType::getByJcaName) + final KeyStoreType storeType = maybeStoreType.map(KeyStoreType::getByJcaName) .orElse(inferStoreType(keyStorePath.toString().toLowerCase(Locale.ROOT))); final String algorithm = resolveSetting(KEYSTORE_ALGORITHM, Function.identity(), KeyManagerFactory.getDefaultAlgorithm()); diff --git a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRestClientSslTests.java b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRestClientSslTests.java index 36b6edaa9c703..2fc7ab1654d41 100644 --- a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRestClientSslTests.java +++ b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRestClientSslTests.java @@ -37,7 +37,7 @@ import com.sun.net.httpserver.HttpsParameters; import com.sun.net.httpserver.HttpsServer; -import org.opensearch.client.FipsEnabledThreadFactory; +import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.opensearch.client.Request; import org.opensearch.client.Response; import org.opensearch.client.RestClient; @@ -99,8 +99,16 @@ public static void setupHttpServer() throws Exception { SSLContext sslContext = buildServerSslContext(); server = HttpsServer.create(address, 0); server.setHttpsConfigurator(new ClientAuthHttpsConfigurator(sslContext)); - var threadFactory = new FipsEnabledThreadFactory("test-httpserver-dispatcher", inFipsJvm()); - Executor executor = Executors.newFixedThreadPool(1, threadFactory); + var inFipsJvm = inFipsJvm(); + Executor executor = Executors.newFixedThreadPool(1, (Runnable r) -> { + Runnable runnable = () -> { + if (inFipsJvm) { + CryptoServicesRegistrar.setApprovedOnlyMode(true); + } + r.run(); + }; + return new Thread(runnable, "test-httpserver-dispatcher"); + }); server.setExecutor(executor); server.start(); server.createContext("/", http -> { diff --git a/modules/transport-netty4/src/yamlRestTest/java/org/opensearch/http/netty4/Netty4ClientYamlTestSuiteIT.java b/modules/transport-netty4/src/yamlRestTest/java/org/opensearch/http/netty4/Netty4ClientYamlTestSuiteIT.java index 45693078174a8..9c10dc98202eb 100644 --- a/modules/transport-netty4/src/yamlRestTest/java/org/opensearch/http/netty4/Netty4ClientYamlTestSuiteIT.java +++ b/modules/transport-netty4/src/yamlRestTest/java/org/opensearch/http/netty4/Netty4ClientYamlTestSuiteIT.java @@ -39,15 +39,10 @@ import org.apache.lucene.tests.util.TimeUnits; import org.opensearch.test.rest.yaml.ClientYamlTestCandidate; import org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase; -import org.junit.BeforeClass; //TODO: This is a *temporary* workaround to ensure a timeout does not mask other problems @TimeoutSuite(millis = 30 * TimeUnits.MINUTE) public class Netty4ClientYamlTestSuiteIT extends OpenSearchClientYamlSuiteTestCase { - @BeforeClass - public static void muteInFips() { - assumeFalse("We run with DEFAULT distribution in FIPS mode and default to security4 instead of netty4", inFipsJvm()); - } public Netty4ClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) { super(testCandidate); diff --git a/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java b/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java index 0953e51484bd3..1622a9093d765 100644 --- a/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java +++ b/plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java @@ -13,6 +13,7 @@ package org.opensearch.http.reactor.netty4; +import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.opensearch.common.collect.Tuple; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.xcontent.ToXContent; @@ -65,6 +66,7 @@ public class ReactorHttpClient implements Closeable { private final boolean compression; private final boolean secure; + private final boolean fipsEnabled; static Collection returnHttpResponseBodies(Collection responses) { List list = new ArrayList<>(responses.size()); @@ -85,6 +87,7 @@ static Collection returnOpaqueIds(Collection responses public ReactorHttpClient(boolean compression, boolean secure) { this.compression = compression; this.secure = secure; + this.fipsEnabled = CryptoServicesRegistrar.isInApprovedOnlyMode(); } public static ReactorHttpClient create() { @@ -183,7 +186,15 @@ private List sendRequests( final Collection requests, boolean orderer ) { - final NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup(1); + final NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup(1, (Runnable r) -> { + Runnable runnable = () -> { + if (fipsEnabled) { + CryptoServicesRegistrar.setApprovedOnlyMode(true); + } + r.run(); + }; + return new Thread(runnable); + }); try { final HttpClient client = createClient(remoteAddress, eventLoopGroup); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.reload_secure_settings/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.reload_secure_settings/10_basic.yml index 0866c71b87e12..6b5c75fdd76ab 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.reload_secure_settings/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.reload_secure_settings/10_basic.yml @@ -24,6 +24,9 @@ setup: --- "node_reload_secure_settings test correct(empty) password": + - skip: + version: "3.0.0 - " + reason: "Running this test in active FIPS mode is not supported" - do: nodes.reload_secure_settings: {} diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/ReloadSecureSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/ReloadSecureSettingsIT.java index c81d491719e4b..2bb14e09beaf6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/ReloadSecureSettingsIT.java @@ -68,6 +68,9 @@ @OpenSearchIntegTestCase.ClusterScope(minNumDataNodes = 2) public class ReloadSecureSettingsIT extends OpenSearchIntegTestCase { + // Minimal required characters to fulfill the requirement of 112 bit strong passwords + protected static final int MIN_112_BIT_STRONG = 14; + public void testMissingKeystoreFile() throws Exception { final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) @@ -182,7 +185,7 @@ public void testReloadAllNodesWithPasswordWithoutTLSFails() throws Exception { final Environment environment = internalCluster().getInstance(Environment.class); final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); - final char[] password = randomAlphaOfLength(12).toCharArray(); + final char[] password = randomAlphaOfLength(MIN_112_BIT_STRONG).toCharArray(); writeEmptyKeystore(environment, password); final CountDownLatch latch = new CountDownLatch(1); client().admin() @@ -229,7 +232,7 @@ public void onFailure(Exception e) { public void testReloadLocalNodeWithPasswordWithoutTLSSucceeds() throws Exception { final Environment environment = internalCluster().getInstance(Environment.class); final AtomicReference reloadSettingsError = new AtomicReference<>(); - final char[] password = randomAlphaOfLength(12).toCharArray(); + final char[] password = randomAlphaOfLength(MIN_112_BIT_STRONG).toCharArray(); writeEmptyKeystore(environment, password); final CountDownLatch latch = new CountDownLatch(1); client().admin() @@ -275,14 +278,15 @@ public void testWrongKeystorePassword() throws Exception { final Environment environment = internalCluster().getInstance(Environment.class); final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + final char[] password = inFipsJvm() ? randomAlphaOfLength(MIN_112_BIT_STRONG).toCharArray() : new char[0]; // "some" keystore should be present in this case - writeEmptyKeystore(environment, new char[0]); + writeEmptyKeystore(environment, password); final CountDownLatch latch = new CountDownLatch(1); client().admin() .cluster() .prepareReloadSecureSettings() .setNodesIds("_local") - .setSecureStorePassword(new SecureString(new char[] { 'W', 'r', 'o', 'n', 'g' })) + .setSecureStorePassword(new SecureString("thewrongkeystorepassword".toCharArray())) .execute(new ActionListener() { @Override public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { @@ -316,6 +320,7 @@ public void onFailure(Exception e) { } public void testMisbehavingPlugin() throws Exception { + assumeFalse("Can't use empty password in a FIPS JVM", inFipsJvm()); final Environment environment = internalCluster().getInstance(Environment.class); final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) @@ -382,6 +387,7 @@ public void onFailure(Exception e) { } public void testReloadWhileKeystoreChanged() throws Exception { + assumeFalse("Can't use empty password in a FIPS JVM", inFipsJvm()); final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) .stream() diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index 0bdabba823394..b680a91458321 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -108,6 +108,7 @@ grant { permission java.security.SecurityPermission "putProviderProperty.BCFIPS"; permission java.security.SecurityPermission "putProviderProperty.BCJSSE"; permission java.util.PropertyPermission "java.runtime.name", "read"; + permission org.bouncycastle.crypto.CryptoServicesPermission "changeToApprovedModeEnabled"; permission org.bouncycastle.crypto.CryptoServicesPermission "exportSecretKey"; permission org.bouncycastle.crypto.CryptoServicesPermission "exportPrivateKey"; }; diff --git a/test/framework/src/main/java/org/opensearch/test/junit/listeners/ReproduceInfoPrinter.java b/test/framework/src/main/java/org/opensearch/test/junit/listeners/ReproduceInfoPrinter.java index e2d59773a76cb..2b5f3fdc3e6b4 100644 --- a/test/framework/src/main/java/org/opensearch/test/junit/listeners/ReproduceInfoPrinter.java +++ b/test/framework/src/main/java/org/opensearch/test/junit/listeners/ReproduceInfoPrinter.java @@ -194,6 +194,8 @@ private ReproduceErrorMessageBuilder appendESProperties() { appendOpt("tests.timezone", TimeZone.getDefault().getID()); appendOpt("runtime.java", Integer.toString(Runtime.version().version().get(0))); appendOpt(OpenSearchTestCase.FIPS_SYSPROP, System.getProperty(OpenSearchTestCase.FIPS_SYSPROP)); + appendOpt("org.bouncycastle.jca.enable_jks", "true"); + appendOpt("org.bouncycastle.rsa.allow_multi_use", "true"); return this; }