Skip to content

Commit

Permalink
Kerberos, forbiddenApis, SecureRandom, SunJCE, AzureTests
Browse files Browse the repository at this point in the history
Summery:
- replace unsecure kerberos crypto algorithms
- 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 13, 2024
1 parent 00ca592 commit 2fdb220
Show file tree
Hide file tree
Showing 42 changed files with 535 additions and 251 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());

Check warning on line 253 in buildSrc/src/main/java/org/opensearch/gradle/http/WaitForHttpResource.java

View check run for this annotation

Codecov / codecov/patch

buildSrc/src/main/java/org/opensearch/gradle/http/WaitForHttpResource.java#L253

Added line #L253 was not covered by tests
return sslContext;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ public synchronized void start() {

logToProcessStdout("Creating opensearch keystore with password set to [" + keystorePassword + "]");
if (keystorePassword.length() > 0) {
runOpenSearchBinScriptWithInput(keystorePassword + "\n" + keystorePassword, "opensearch-keystore", "create", "-p");
runOpenSearchBinScriptWithInput(keystorePassword + "\n" + keystorePassword + "\n", "opensearch-keystore", "create", "-p");

Check warning on line 551 in buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java

View check run for this annotation

Codecov / codecov/patch

buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java#L551

Added line #L551 was not covered by tests
} else {
runOpenSearchBinScript("opensearch-keystore", "-v", "create");
}
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, this::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, this::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;

Check warning on line 57 in libs/common/src/main/java/org/opensearch/common/crypto/KeyStoreFactory.java

View check run for this annotation

Codecov / codecov/patch

libs/common/src/main/java/org/opensearch/common/crypto/KeyStoreFactory.java#L57

Added line #L57 was not covered by tests
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.tests.analysis.BaseTokenStreamTestCase;
import org.opensearch.Version;
import org.opensearch.bootstrap.SecureRandomInitializer;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.index.IndexSettings;
Expand All @@ -47,6 +48,10 @@

public class IcuAnalyzerTests extends BaseTokenStreamTestCase {

static {
SecureRandomInitializer.init();
}

public void testMixedAlphabetTokenization() throws IOException {

Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build();
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 @@ -214,13 +214,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 @@ -311,6 +304,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());

Check warning on line 344 in plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Service.java

View check run for this annotation

Codecov / codecov/patch

plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Service.java#L344

Added line #L344 was not covered by tests

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
Loading

0 comments on commit 2fdb220

Please sign in to comment.