Migrate from BC to BCFIPS libraries

Signed-off-by: Igonin <[email protected]>
Co-authored-by: Benny Goerzig <[email protected]>
Co-authored-by: Karsten Schnitter <[email protected]>
Co-authored-by: Kai Sternad <[email protected]>

Author: 4 people

CHANGELOG-3.0.md

@@ -52,6 +52,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Wildcard field index only 3gram of the input data [#17349](https://github.com/opensearch-project/OpenSearch/pull/17349)
 - Use BC libraries to parse PEM files, increase key length, allow general use of known cryptographic binary extensions, remove unused BC dependencies ([#3420](https://github.com/opensearch-project/OpenSearch/pull/14912))
 - Add optional enum set read / write functionality to stream input / output ([#17556](https://github.com/opensearch-project/OpenSearch/pull/17556))
+- Migrate BC libs to their FIPS counterparts ([#3420](https://github.com/opensearch-project/OpenSearch/pull/14912))
 
 ### Deprecated
 

buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java

@@ -164,6 +164,12 @@ public void execute(Task t) {
                 test.systemProperty("tests.seed", BuildParams.getTestSeed());
             }
 
+            var securityFile = "java.security";
+            test.systemProperty(
+                "java.security.properties",
+                project.getRootProject().getLayout().getProjectDirectory() + "/distribution/src/config/" + securityFile
+            );
+
             // don't track these as inputs since they contain absolute paths and break cache relocatability
             File gradleHome = project.getGradle().getGradleUserHomeDir();
             String gradleVersion = project.getGradle().getGradleVersion();

client/rest/build.gradle

@@ -29,6 +29,7 @@
  */
 
 import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis
+import org.opensearch.gradle.precommit.TestingConventionsTasks
 
 apply plugin: 'opensearch.build'
 apply plugin: 'opensearch.publish'
@@ -51,6 +52,9 @@ dependencies {
   api "commons-codec:commons-codec:${versions.commonscodec}"
   api "commons-logging:commons-logging:${versions.commonslogging}"
   api "org.slf4j:slf4j-api:${versions.slf4j}"
+  runtimeOnly "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}"
+  runtimeOnly "org.bouncycastle:bctls-fips:${versions.bouncycastle_tls}"
+  runtimeOnly "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}"
 
   // reactor
   api "io.projectreactor:reactor-core:${versions.reactor}"
@@ -70,6 +74,10 @@ dependencies {
   testImplementation "org.apache.logging.log4j:log4j-slf4j-impl:${versions.log4j}"
 }
 
+tasks.named("dependencyLicenses").configure {
+  mapping from: /bc.*/, to: 'bouncycastle'
+}
+
 tasks.withType(CheckForbiddenApis).configureEach {
   //client does not depend on server, so only jdk and http signatures should be checked
   replaceSignatureFiles('jdk-signatures', 'http-signatures')

distribution/tools/plugin-cli/licenses/bc-fips-2.0.0.jar.sha1 client/rest/licenses/bc-fips-2.0.0.jar.sha1

@@ -0,0 +1 @@
+9cc33650ede63bc1a8281ed5c8e1da314d50bc76

client/rest/licenses/bctls-fips-2.0.19.jar.sha1

@@ -0,0 +1 @@
+a1857cd639295b10cc90e6d31ecbc523cdafcc19

client/rest/licenses/bcutil-fips-2.0.3.jar.sha1

@@ -0,0 +1,14 @@
+Copyright (c) 2000 - 2023 The Legion of the Bouncy Castle Inc. (https://www.bouncycastle.org)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+documentation files (the "Software"), to deal in the Software without restriction, including without limitation
+the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
+and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

client/rest/licenses/bouncycastle-LICENSE.txt

@@ -0,0 +1 @@
+

client/rest/licenses/bouncycastle-NOTICE.txt

@@ -38,27 +38,23 @@
 import com.sun.net.httpserver.HttpsServer;
 
 import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.ssl.SSLContextBuilder;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
-import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLException;
 import javax.net.ssl.TrustManagerFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
-import java.nio.file.Files;
-import java.nio.file.Paths;
 import java.security.AccessController;
-import java.security.KeyFactory;
 import java.security.KeyStore;
 import java.security.PrivilegedAction;
-import java.security.cert.Certificate;
-import java.security.cert.CertificateFactory;
-import java.security.spec.PKCS8EncodedKeySpec;
+import java.security.SecureRandom;
 
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertEquals;
@@ -75,7 +71,7 @@ public class RestClientBuilderIntegTests extends RestClientTestCase {
     @BeforeClass
     public static void startHttpServer() throws Exception {
         httpsServer = HttpsServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
-        httpsServer.setHttpsConfigurator(new HttpsConfigurator(getSslContext()));
+        httpsServer.setHttpsConfigurator(new HttpsConfigurator(getSslContext(true)));
         httpsServer.createContext("/", new ResponseHandler());
         httpsServer.start();
     }
@@ -103,11 +99,11 @@ public void testBuilderUsesDefaultSSLContext() throws Exception {
                     client.performRequest(new Request("GET", "/"));
                     fail("connection should have been rejected due to SSL handshake");
                 } catch (Exception e) {
-                    assertThat(e, instanceOf(SSLHandshakeException.class));
+                    assertThat(e.getCause(), instanceOf(SSLException.class));
                 }
             }
 
-            SSLContext.setDefault(getSslContext());
+            SSLContext.setDefault(getSslContext(false));
             try (RestClient client = buildRestClient()) {
                 Response response = client.performRequest(new Request("GET", "/"));
                 assertEquals(200, response.getStatusLine().getStatusCode());
@@ -122,34 +118,37 @@ private RestClient buildRestClient() {
         return RestClient.builder(new HttpHost("https", address.getHostString(), address.getPort())).build();
     }
 
-    private static SSLContext getSslContext() throws Exception {
-        SSLContext sslContext = SSLContext.getInstance(getProtocol());
+    private static SSLContext getSslContext(boolean server) throws Exception {
+        SSLContext sslContext;
+        char[] password = "password".toCharArray();
+        SecureRandom secureRandom = SecureRandom.getInstance("DEFAULT", "BCFIPS");
+        String fileExtension = ".jks";
+
         try (
-            InputStream certFile = RestClientBuilderIntegTests.class.getResourceAsStream("/test.crt");
-            InputStream keyStoreFile = RestClientBuilderIntegTests.class.getResourceAsStream("/test_truststore.jks")
+            InputStream trustStoreFile = RestClientBuilderIntegTests.class.getResourceAsStream("/test_truststore" + fileExtension);
+            InputStream keyStoreFile = RestClientBuilderIntegTests.class.getResourceAsStream("/testks" + fileExtension)
         ) {
-            // Build a keystore of default type programmatically since we can't use JKS keystores to
-            // init a KeyManagerFactory in FIPS 140 JVMs.
-            KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
-            keyStore.load(null, "password".toCharArray());
-            CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
-            PKCS8EncodedKeySpec privateKeySpec = new PKCS8EncodedKeySpec(
-                Files.readAllBytes(Paths.get(RestClientBuilderIntegTests.class.getResource("/test.der").toURI()))
-            );
-            KeyFactory keyFactory = KeyFactory.getInstance("RSA");
-            keyStore.setKeyEntry(
-                "mykey",
-                keyFactory.generatePrivate(privateKeySpec),
-                "password".toCharArray(),
-                new Certificate[] { certFactory.generateCertificate(certFile) }
-            );
-            KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
-            kmf.init(keyStore, "password".toCharArray());
+            KeyStore keyStore = KeyStore.getInstance("JKS");
+            keyStore.load(keyStoreFile, password);
+            KeyManagerFactory kmf = KeyManagerFactory.getInstance("PKIX", "BCJSSE");
+            kmf.init(keyStore, password);
+
             KeyStore trustStore = KeyStore.getInstance("JKS");
-            trustStore.load(keyStoreFile, "password".toCharArray());
-            TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+            trustStore.load(trustStoreFile, password);
+            TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX", "BCJSSE");
             tmf.init(trustStore);
-            sslContext.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);
+
+            SSLContextBuilder sslContextBuilder = SSLContextBuilder.create()
+                .setProvider("BCJSSE")
+                .setProtocol(getProtocol())
+                .setSecureRandom(secureRandom);
+
+            if (server) {
+                sslContextBuilder.loadKeyMaterial(keyStore, password);
+            }
+            sslContextBuilder.loadTrustMaterial(trustStore, null);
+            sslContext = sslContextBuilder.build();
+
         }
         return sslContext;
     }

client/rest/src/test/java/org/opensearch/client/RestClientBuilderIntegTests.java

@@ -0,0 +1,10 @@
+# Security properties for non-approved mode 'org.bouncycastle.fips.approved_only=false'.
+# Intended to be used complementary with a single equal sign e.g. 'java.security.properties=java.security'
+
+security.provider.1=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider
+security.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider
+security.provider.3=SUN
+security.provider.4=SunJGSS
+
+ssl.KeyManagerFactory.algorithm=PKIX
+ssl.TrustManagerFactory.algorithm=PKIX

distribution/src/config/java.security

@@ -211,17 +211,13 @@ public void testIncorrectPassword() throws Exception {
         terminal.addSecretInput("thewrongkeystorepassword");
         UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString()));
         assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode);
-        if (inFipsJvm()) {
-            assertThat(
-                e.getMessage(),
-                anyOf(
-                    containsString("Provided keystore password was incorrect"),
-                    containsString("Keystore has been corrupted or tampered with")
-                )
-            );
-        } else {
-            assertThat(e.getMessage(), containsString("Provided keystore password was incorrect"));
-        }
+        assertThat(
+            e.getMessage(),
+            anyOf(
+                containsString("Provided keystore password was incorrect"),
+                containsString("Keystore has been corrupted or tampered with")
+            )
+        );
     }
 
     public void testAddToUnprotectedKeystore() throws Exception {

distribution/tools/keystore-cli/src/test/java/org/opensearch/tools/cli/keystore/AddFileKeyStoreCommandTests.java

@@ -72,18 +72,13 @@ public void testInvalidPassphrease() throws Exception {
         terminal.addSecretInput("thewrongpassword");
         UserException e = expectThrows(UserException.class, () -> execute("foo2"));
         assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode);
-        if (inFipsJvm()) {
-            assertThat(
-                e.getMessage(),
-                anyOf(
-                    containsString("Provided keystore password was incorrect"),
-                    containsString("Keystore has been corrupted or tampered with")
-                )
-            );
-        } else {
-            assertThat(e.getMessage(), containsString("Provided keystore password was incorrect"));
-        }
-
+        assertThat(
+            e.getMessage(),
+            anyOf(
+                containsString("Provided keystore password was incorrect"),
+                containsString("Keystore has been corrupted or tampered with")
+            )
+        );
     }
 
     public void testMissingPromptCreateWithoutPasswordWhenPrompted() throws Exception {

distribution/tools/keystore-cli/src/test/java/org/opensearch/tools/cli/keystore/AddStringKeyStoreCommandTests.java

@@ -104,16 +104,12 @@ public void testChangeKeyStorePasswordWrongExistingPassword() throws Exception {
         // We'll only be prompted once (for the old password)
         UserException e = expectThrows(UserException.class, this::execute);
         assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode);
-        if (inFipsJvm()) {
-            assertThat(
-                e.getMessage(),
-                anyOf(
-                    containsString("Provided keystore password was incorrect"),
-                    containsString("Keystore has been corrupted or tampered with")
-                )
-            );
-        } else {
-            assertThat(e.getMessage(), containsString("Provided keystore password was incorrect"));
-        }
+        assertThat(
+            e.getMessage(),
+            anyOf(
+                containsString("Provided keystore password was incorrect"),
+                containsString("Keystore has been corrupted or tampered with")
+            )
+        );
     }
 }