Skip to content

Commit

Permalink
replace unsecure kerberos crypto algorithms
Browse files Browse the repository at this point in the history
add 'java.security.KeyStore' to forbidden-apis
instantiate and use SecureRandom from BCFIPS library
exclude SunJCE from security providers list at runtime, when running in FIPS JVM
exclude Azure tests when running in FIPS JVM

Signed-off-by: Iwan Igonin <iigonin@sternad.de>
  • Loading branch information
iigonin committed Dec 6, 2024
1 parent d331153 commit af1f3ee
Show file tree
Hide file tree
Showing 38 changed files with 420 additions and 250 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@

package org.opensearch.gradle.http;

import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.internal.impldep.com.jcraft.jsch.annotations.SuppressForbiddenApi;

import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.KeyManager;
Expand All @@ -51,7 +53,6 @@
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.SecureRandom;
import java.security.cert.Certificate;
import java.security.cert.CertificateFactory;
import java.util.Arrays;
Expand Down Expand Up @@ -216,15 +217,15 @@ KeyStore buildTrustStore() throws GeneralSecurityException, IOException {
}

private KeyStore buildTrustStoreFromFile() throws GeneralSecurityException, IOException {
KeyStore keyStore = KeyStore.getInstance(trustStoreFile.getName().endsWith(".jks") ? "JKS" : "PKCS12");
var keyStore = getKeyStoreInstance(trustStoreFile.getName().endsWith(".jks") ? "JKS" : "PKCS12");
try (InputStream input = new FileInputStream(trustStoreFile)) {
keyStore.load(input, trustStorePassword == null ? null : trustStorePassword.toCharArray());
}
return keyStore;
}

private KeyStore buildTrustStoreFromCA() throws GeneralSecurityException, IOException {
final KeyStore store = KeyStore.getInstance(KeyStore.getDefaultType());
var store = getKeyStoreInstance(KeyStore.getDefaultType());
store.load(null, null);
final CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
int counter = 0;
Expand All @@ -239,12 +240,17 @@ private KeyStore buildTrustStoreFromCA() throws GeneralSecurityException, IOExce
return store;
}

@SuppressForbiddenApi("runs exclusively in test-context without KeyStoreFactory on classpath.")
private KeyStore getKeyStoreInstance(String type) throws KeyStoreException {
return KeyStore.getInstance(type);
}

private SSLContext createSslContext(KeyStore trustStore) throws GeneralSecurityException {
checkForTrustEntry(trustStore);
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
tmf.init(trustStore);
SSLContext sslContext = SSLContext.getInstance("TLSv1.2");
sslContext.init(new KeyManager[0], tmf.getTrustManagers(), new SecureRandom());
sslContext.init(new KeyManager[0], tmf.getTrustManagers(), CryptoServicesRegistrar.getSecureRandom());
return sslContext;
}

Expand Down
6 changes: 6 additions & 0 deletions buildSrc/src/main/resources/forbidden/jdk-signatures.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ java.nio.file.Path#toFile()
java.nio.file.Files#createTempDirectory(java.lang.String,java.nio.file.attribute.FileAttribute[])
java.nio.file.Files#createTempFile(java.lang.String,java.lang.String,java.nio.file.attribute.FileAttribute[])

@defaultMessage Use org.opensearch.common.crypto.KeyStoreFactory instead of java.security.KeyStore
java.security.KeyStore#getInstance(java.lang.String)
java.security.KeyStore#getInstance(java.lang.String,java.lang.String)
java.security.KeyStore#getInstance(java.lang.String,java.security.Provider)
java.security.KeyStore#getInstance(java.io.File,char[])

@defaultMessage Don't use java serialization - this can break BWC without noticing it
java.io.ObjectOutputStream
java.io.ObjectOutput
Expand Down
4 changes: 0 additions & 4 deletions distribution/src/config/fips_java.security
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ security.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider fips
security.provider.3=SUN
security.provider.4=SunJGSS

securerandom.source=file:/dev/urandom
securerandom.strongAlgorithms=NativePRNGBlocking:SUN,DRBG:SUN
securerandom.drbg.config=

login.configuration.provider=sun.security.provider.ConfigFile
policy.provider=sun.security.provider.PolicyFile
policy.expandProperties=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.NIOFSDirectory;
import org.opensearch.common.Randomness;
import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.opensearch.common.crypto.KeyStoreFactory;
import org.opensearch.common.crypto.KeyStoreType;
import org.opensearch.common.util.io.IOUtils;
Expand Down Expand Up @@ -205,7 +205,7 @@ public void testFailWhenCannotConsumeSecretStream() throws Exception {
try (IndexOutput indexOutput = directory.createOutput("opensearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "opensearch.keystore", 3);
indexOutput.writeByte((byte) 0); // No password
SecureRandom random = Randomness.createSecure();
SecureRandom random = CryptoServicesRegistrar.getSecureRandom();
byte[] salt = new byte[64];
random.nextBytes(salt);
byte[] iv = new byte[12];
Expand Down Expand Up @@ -233,7 +233,7 @@ public void testFailWhenCannotConsumeEncryptedBytesStream() throws Exception {
try (IndexOutput indexOutput = directory.createOutput("opensearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "opensearch.keystore", 3);
indexOutput.writeByte((byte) 0); // No password
SecureRandom random = Randomness.createSecure();
SecureRandom random = CryptoServicesRegistrar.getSecureRandom();
byte[] salt = new byte[64];
random.nextBytes(salt);
byte[] iv = new byte[12];
Expand Down Expand Up @@ -262,7 +262,7 @@ public void testFailWhenSecretStreamNotConsumed() throws Exception {
try (IndexOutput indexOutput = directory.createOutput("opensearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "opensearch.keystore", 3);
indexOutput.writeByte((byte) 0); // No password
SecureRandom random = Randomness.createSecure();
SecureRandom random = CryptoServicesRegistrar.getSecureRandom();
byte[] salt = new byte[64];
random.nextBytes(salt);
byte[] iv = new byte[12];
Expand All @@ -289,7 +289,7 @@ public void testFailWhenEncryptedBytesStreamIsNotConsumed() throws Exception {
try (IndexOutput indexOutput = directory.createOutput("opensearch.keystore", IOContext.DEFAULT)) {
CodecUtil.writeHeader(indexOutput, "opensearch.keystore", 3);
indexOutput.writeByte((byte) 0); // No password
SecureRandom random = Randomness.createSecure();
SecureRandom random = CryptoServicesRegistrar.getSecureRandom();
byte[] salt = new byte[64];
random.nextBytes(salt);
byte[] iv = new byte[12];
Expand Down Expand Up @@ -372,15 +372,15 @@ public void testIllegalSettingName() throws Exception {
public void testFailLoadV1KeystoresInFipsJvm() throws Exception {
assumeTrue("Test in FIPS JVM", inFipsJvm());

Exception e = assertThrows(SecurityException.class, () -> generateV1());
assertThat(e.getMessage(), containsString("Only PKCS_11, BCFKS keystores are allowed in FIPS JVM"));
Exception e = assertThrows(NoSuchProviderException.class, () -> generateV1());
assertThat(e.getMessage(), containsString("no such provider: SunJCE"));
}

public void testFailLoadV2KeystoresInFipsJvm() throws Exception {
assumeTrue("Test in FIPS JVM", inFipsJvm());

Exception e = assertThrows(SecurityException.class, () -> generateV2());
assertThat(e.getMessage(), containsString("Only PKCS_11, BCFKS keystores are allowed in FIPS JVM"));
Exception e = assertThrows(NoSuchProviderException.class, () -> generateV2());
assertThat(e.getMessage(), containsString("no such provider: SunJCE"));
}

public void testBackcompatV1() throws Exception {
Expand Down
6 changes: 6 additions & 0 deletions libs/common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ base {
dependencies {
// This dependency is used only by :libs:core for null-checking interop with other tools
compileOnly "com.google.code.findbugs:jsr305:3.0.2"

compileOnly "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}"
compileOnly "org.bouncycastle:bcutil-fips:${versions.bouncycastle_util}"
api "org.bouncycastle:bcpkix-fips:${versions.bouncycastle_pkix}"

/*******
* !!!! NO THIRD PARTY DEPENDENCIES !!!!
Expand All @@ -46,6 +48,10 @@ tasks.named('forbiddenApisMain').configure {
replaceSignatureFiles 'jdk-signatures'
}

tasks.named("dependencyLicenses").configure {
mapping from: /bc.*/, to: 'bouncycastle'
}

// Add support for incubator modules on supported Java versions.
if (BuildParams.runtimeJavaVersion >= JavaVersion.VERSION_20) {
sourceSets {
Expand Down
1 change: 1 addition & 0 deletions libs/common/licenses/bcpkix-fips-2.0.7.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
01eea0f325315ca6295b0a6926ff862d8001cdf9
14 changes: 14 additions & 0 deletions libs/common/licenses/bouncycastle-LICENSE.txt
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions libs/common/licenses/bouncycastle-NOTICE.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

package org.opensearch.common;

import org.bouncycastle.crypto.CryptoServicesRegistrar;

import java.security.SecureRandom;

/**
Expand All @@ -41,5 +43,5 @@
*/
class SecureRandomHolder {
// class loading is atomic - this is a lazy & safe singleton to be used by this package
public static final SecureRandom INSTANCE = new SecureRandom();
public static final SecureRandom INSTANCE = CryptoServicesRegistrar.getSecureRandom();
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.opensearch.common.crypto;

import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.opensearch.common.SuppressForbidden;

import java.security.KeyStore;
import java.security.KeyStoreException;
Expand Down Expand Up @@ -55,7 +56,11 @@ public static KeyStore getInstance(KeyStoreType type, String provider) {
}
provider = FIPS_PROVIDER;
}
return get(type, provider);
}

@SuppressForbidden(reason = "centralized instantiation of a KeyStore")
private static KeyStore get(KeyStoreType type, String provider) {
try {
if (provider == null) {
return KeyStore.getInstance(type.getJcaName());
Expand Down
11 changes: 0 additions & 11 deletions modules/transport-netty4/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,6 @@ thirdPartyAudit {
'io.netty.internal.tcnative.SSLContext',
'io.netty.internal.tcnative.SSLPrivateKeyMethod',

// from io.netty.handler.ssl.util.BouncyCastleSelfSignedCertGenerator (netty)
'org.bouncycastle.cert.X509v3CertificateBuilder',
'org.bouncycastle.cert.jcajce.JcaX509CertificateConverter',
'org.bouncycastle.operator.jcajce.JcaContentSignerBuilder',
'org.bouncycastle.openssl.PEMEncryptedKeyPair',
'org.bouncycastle.openssl.PEMParser',
'org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter',
'org.bouncycastle.openssl.jcajce.JceOpenSSLPKCS8DecryptorProviderBuilder',
'org.bouncycastle.openssl.jcajce.JcePEMDecryptorProviderBuilder',
'org.bouncycastle.pkcs.PKCS8EncryptedPrivateKeyInfo',

// from io.netty.handler.ssl.JettyNpnSslEngine (netty)
'org.eclipse.jetty.npn.NextProtoNego$ClientProvider',
'org.eclipse.jetty.npn.NextProtoNego$ServerProvider',
Expand Down
11 changes: 4 additions & 7 deletions plugins/repository-azure/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,6 @@ thirdPartyAudit {
// Worth nothing that, the latest dependency "net.shibboleth.utilities:java-support:8.0.0" has many vulnerabilities.
// Hence ignored.
'net.shibboleth.utilities.java.support.xml.SerializeSupport',
'org.bouncycastle.cert.X509CertificateHolder',
'org.bouncycastle.cert.jcajce.JcaX509CertificateHolder',
'org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder',
'org.bouncycastle.openssl.PEMKeyPair',
'org.bouncycastle.openssl.PEMParser',
'org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter',
'org.bouncycastle.operator.jcajce.JcaContentSignerBuilder',
'org.cryptomator.siv.SivMode',
'org.opensaml.core.config.InitializationException',
'org.opensaml.core.config.InitializationService',
Expand Down Expand Up @@ -310,6 +303,10 @@ Map<String, Object> expansions = [
'base_path': azureBasePath + "_integration_tests"
]

tasks.withType(Test).configureEach {
onlyIf { BuildParams.inFipsJvm == false }
}

processYamlRestTestResources {
inputs.properties(expansions)
MavenFilteringHack.filter(it, expansions)
Expand Down
11 changes: 0 additions & 11 deletions plugins/repository-s3/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -486,17 +486,6 @@ thirdPartyAudit {
'net.jpountz.xxhash.XXHash32',
'net.jpountz.xxhash.XXHashFactory',
// from io.netty.handler.ssl.util.BouncyCastleSelfSignedCertGenerator (netty)
'org.bouncycastle.cert.X509v3CertificateBuilder',
'org.bouncycastle.cert.jcajce.JcaX509CertificateConverter',
'org.bouncycastle.operator.jcajce.JcaContentSignerBuilder',
'org.bouncycastle.openssl.PEMEncryptedKeyPair',
'org.bouncycastle.openssl.PEMParser',
'org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter',
'org.bouncycastle.openssl.jcajce.JceOpenSSLPKCS8DecryptorProviderBuilder',
'org.bouncycastle.openssl.jcajce.JcePEMDecryptorProviderBuilder',
'org.bouncycastle.pkcs.PKCS8EncryptedPrivateKeyInfo',
'org.conscrypt.AllocatedBuffer',
'org.conscrypt.BufferAllocator',
'org.conscrypt.Conscrypt',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ protected HttpHandler createErroneousHttpHandler(final HttpHandler delegate) {
protected Settings nodeSettings(int nodeOrdinal) {
final MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString(S3ClientSettings.ACCESS_KEY_SETTING.getConcreteSettingForNamespace("test").getKey(), "access");
secureSettings.setString(S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace("test").getKey(), "secret");
secureSettings.setString(S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace("test").getKey(), "secret_password");

final Settings.Builder builder = Settings.builder()
.put(ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING.getKey(), 0) // We have tests that verify an exact wait time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import org.apache.http.protocol.HttpContext;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.opensearch.cluster.metadata.RepositoryMetadata;
import org.opensearch.common.Nullable;
import org.opensearch.common.SuppressForbidden;
Expand All @@ -88,7 +89,6 @@
import java.nio.file.Path;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.time.Duration;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -341,7 +341,8 @@ private static SSLConnectionSocketFactory createSocksSslConnectionSocketFactory(
// This part was taken from AWS settings
try {
final SSLContext sslCtx = SSLContext.getInstance("TLS");
sslCtx.init(SystemPropertyTlsKeyManagersProvider.create().keyManagers(), null, new SecureRandom());
sslCtx.init(SystemPropertyTlsKeyManagersProvider.create().keyManagers(), null, CryptoServicesRegistrar.getSecureRandom());

return new SdkTlsSocketFactory(sslCtx, new DefaultHostnameVerifier()) {
@Override
public Socket createSocket(final HttpContext ctx) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,10 @@ protected AsyncMultiStreamBlobContainer createBlobContainer(

final MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString(S3ClientSettings.ACCESS_KEY_SETTING.getConcreteSettingForNamespace(clientName).getKey(), "access");
secureSettings.setString(S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace(clientName).getKey(), "secret");
secureSettings.setString(
S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace(clientName).getKey(),
"secret_password"
);
clientSettings.setSecureSettings(secureSettings);
service.refreshAndClearCache(S3ClientSettings.load(clientSettings.build(), configPath()));
asyncService.refreshAndClearCache(S3ClientSettings.load(clientSettings.build(), configPath()));
Expand Down
11 changes: 0 additions & 11 deletions plugins/transport-nio/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,6 @@ thirdPartyAudit {
'org.apache.log4j.Level',
'org.apache.log4j.Logger',

// from io.netty.handler.ssl.util.BouncyCastleSelfSignedCertGenerator (netty)
'org.bouncycastle.cert.X509v3CertificateBuilder',
'org.bouncycastle.cert.jcajce.JcaX509CertificateConverter',
'org.bouncycastle.operator.jcajce.JcaContentSignerBuilder',
'org.bouncycastle.openssl.PEMEncryptedKeyPair',
'org.bouncycastle.openssl.PEMParser',
'org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter',
'org.bouncycastle.openssl.jcajce.JceOpenSSLPKCS8DecryptorProviderBuilder',
'org.bouncycastle.openssl.jcajce.JcePEMDecryptorProviderBuilder',
'org.bouncycastle.pkcs.PKCS8EncryptedPrivateKeyInfo',

// from io.netty.handler.ssl.JettyNpnSslEngine (netty)
'org.eclipse.jetty.npn.NextProtoNego$ClientProvider',
'org.eclipse.jetty.npn.NextProtoNego$ServerProvider',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestRequest;
import org.opensearch.telemetry.tracing.noop.NoopTracer;
import org.opensearch.test.KeyStoreUtils;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.threadpool.TestThreadPool;
Expand All @@ -45,6 +46,7 @@
import org.junit.After;
import org.junit.Before;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLException;

Expand Down Expand Up @@ -77,6 +79,7 @@
import static org.opensearch.core.rest.RestStatus.OK;
import static org.opensearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_ORIGIN;
import static org.opensearch.http.HttpTransportSettings.SETTING_CORS_ENABLED;
import static org.opensearch.test.KeyStoreUtils.KEYSTORE_PASSWORD;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -111,12 +114,14 @@ public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Setti
@Override
public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException {
try {
SSLEngine engine = SslContextBuilder.forServer(
SecureNioHttpServerTransportTests.class.getResourceAsStream("/certificate.crt"),
SecureNioHttpServerTransportTests.class.getResourceAsStream("/certificate.key")
).trustManager(InsecureTrustManagerFactory.INSTANCE).build().newEngine(UnpooledByteBufAllocator.DEFAULT);
var keyManagerFactory = KeyManagerFactory.getInstance("PKIX");
keyManagerFactory.init(KeyStoreUtils.createServerKeyStore(), KEYSTORE_PASSWORD);
SSLEngine engine = SslContextBuilder.forServer(keyManagerFactory)
.trustManager(InsecureTrustManagerFactory.INSTANCE)
.build()
.newEngine(UnpooledByteBufAllocator.DEFAULT);
return Optional.of(engine);
} catch (final IOException ex) {
} catch (Exception ex) {
throw new SSLException(ex);
}
}
Expand Down
Loading

0 comments on commit af1f3ee

Please sign in to comment.