Skip to content

Commit

Permalink
Configure FIPS mode through global ENV VAR instead of thread-based.
Browse files Browse the repository at this point in the history
Signed-off-by: Iwan Igonin <[email protected]>
  • Loading branch information
iigonin committed Oct 18, 2024
1 parent 0054804 commit 8228037
Show file tree
Hide file tree
Showing 22 changed files with 60 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public class GlobalBuildInfoPlugin implements Plugin<Project> {
private static final Logger LOGGER = Logging.getLogger(GlobalBuildInfoPlugin.class);
private static final String DEFAULT_LEGACY_VERSION_JAVA_FILE_PATH = "libs/core/src/main/java/org/opensearch/LegacyESVersion.java";
private static final String DEFAULT_VERSION_JAVA_FILE_PATH = "libs/core/src/main/java/org/opensearch/Version.java";
protected static final String OPENSEARCH_CRYPTO_STANDARD = "OPENSEARCH_CRYPTO_STANDARD";
private static Integer _defaultParallel = null;

private final JvmMetadataDetector jvmMetadataDetector;
Expand Down Expand Up @@ -112,6 +113,8 @@ public void apply(Project project) {
BuildParams.init(params -> {
// Initialize global build parameters
boolean isInternal = GlobalBuildInfoPlugin.class.getResource("/buildSrc.marker") != null;
var cryptoStandard = System.getenv(OPENSEARCH_CRYPTO_STANDARD);
var inFipsJvm = cryptoStandard != null && cryptoStandard.equals("FIPS-140-2");

params.reset();
params.setRuntimeJavaHome(runtimeJavaHome);
Expand All @@ -129,7 +132,7 @@ public void apply(Project project) {
params.setIsCi(System.getenv("JENKINS_URL") != null);
params.setIsInternal(isInternal);
params.setDefaultParallel(findDefaultParallel(project));
params.setInFipsJvm(Util.getBooleanProperty("tests.fips.enabled", false));
params.setInFipsJvm(inFipsJvm);
params.setIsSnapshotBuild(Util.getBooleanProperty("build.snapshot", true));
if (isInternal) {
params.setBwcVersions(resolveBwcVersions(rootDir));
Expand Down Expand Up @@ -179,7 +182,7 @@ private void logGlobalBuildInfo() {
LOGGER.quiet(" JAVA_HOME : " + gradleJvm.getJavaHome());
}
LOGGER.quiet(" Random Testing Seed : " + BuildParams.getTestSeed());
LOGGER.quiet(" In FIPS 140 mode : " + BuildParams.isInFipsJvm());
LOGGER.quiet(" Crypto Standard : " + Optional.ofNullable(System.getenv(OPENSEARCH_CRYPTO_STANDARD)).orElse("any-supported"));
LOGGER.quiet("=======================================");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ public synchronized void start() {
if (keystoreSettings.isEmpty() == false || keystoreFiles.isEmpty() == false) {
logToProcessStdout("Adding " + keystoreSettings.size() + " keystore settings and " + keystoreFiles.size() + " keystore files");

keystoreSettings.forEach((key, value) -> runKeystoreCommandWithPassword(keystorePassword, value.toString(), "add", "-x", key));
keystoreSettings.forEach((key, value) -> runKeystoreCommandWithPassword(keystorePassword, value.toString(), "add", key));

for (Map.Entry<String, File> entry : keystoreFiles.entrySet()) {
File file = entry.getValue();
Expand Down Expand Up @@ -738,7 +738,12 @@ private void runOpenSearchBinScriptWithInput(String input, String tool, CharSequ
}

private void runKeystoreCommandWithPassword(String keystorePassword, String input, CharSequence... args) {
final String actualInput = keystorePassword.length() > 0 ? keystorePassword + "\n" + input : input;
final String actualInput;
if (keystorePassword.length() > 0) {
actualInput = keystorePassword + "\n" + input + "\n" + input;
} else {
actualInput = input + "\n" + input;
}
runOpenSearchBinScriptWithInput(actualInput, "opensearch-keystore", args);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
import org.apache.hc.core5.reactor.ssl.TlsDetails;
import org.apache.hc.core5.util.Timeout;
import org.bouncycastle.crypto.CryptoServicesRegistrar;

import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
Expand Down Expand Up @@ -332,20 +331,9 @@ public TlsDetails create(final SSLEngine sslEngine) {
.setTlsStrategy(tlsStrategy)
.build();

var inFipsJvm = CryptoServicesRegistrar.isInApprovedOnlyMode();

HttpAsyncClientBuilder httpClientBuilder = HttpAsyncClientBuilder.create()
.setDefaultRequestConfig(requestConfigBuilder.build())
.setConnectionManager(connectionManager)
.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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

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;
Expand All @@ -60,7 +59,6 @@
import java.security.SecureRandom;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

import static org.hamcrest.MatcherAssert.assertThat;
Expand All @@ -80,17 +78,6 @@ 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 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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@
import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;

import org.apache.hc.core5.http.Header;
import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.junit.BeforeClass;

import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -127,14 +125,4 @@ private static void addValueToListEntry(final Map<String, List<String>> map, fin
values.add(value);
}

@BeforeClass
public static void setFipsJvm() {
boolean isFipsEnabled = Boolean.parseBoolean(System.getProperty("tests.fips.enabled", "false"));
CryptoServicesRegistrar.setApprovedOnlyMode(isFipsEnabled);
}

public static boolean inFipsJvm() {
return CryptoServicesRegistrar.isInApprovedOnlyMode();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,23 @@ static List<String> systemJvmOptions(final Path config) {
// log4j 2
"-Dlog4j.shutdownHookEnabled=false",
"-Dlog4j2.disable.jmx=true",
// security manager
// security settings
enableFips(),
allowSecurityManagerOption(),
loadJavaSecurityProperties(config),
javaLocaleProviders()
)
).stream().filter(e -> e.isEmpty() == false).collect(Collectors.toList());
}

private static String enableFips() {
var cryptoStandard = System.getenv("OPENSEARCH_CRYPTO_STANDARD");
if (cryptoStandard != null && cryptoStandard.equals("FIPS-140-2")) {
return "-Dorg.bouncycastle.fips.approved_only=true";
}
return "";
}

private static String loadJavaSecurityProperties(final Path config) {
var securityFile = config.resolve("fips_java.security");
return "-Djava.security.properties=" + securityFile.toAbsolutePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.apache.hc.core5.util.Timeout;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.opensearch.action.DocWriteRequest;
import org.opensearch.action.bulk.BackoffPolicy;
import org.opensearch.action.bulk.BulkItemResponse;
Expand Down Expand Up @@ -183,14 +182,7 @@ private ActionListener<BulkByScrollResponse> getRemoteReindexWrapperListener(
}

static RestClient buildRestClient(RemoteInfo remoteInfo, ReindexSslConfig sslConfig, long taskId, List<Thread> threadCollector) {
return buildRestClient(
remoteInfo,
sslConfig,
taskId,
threadCollector,
Optional.empty(),
CryptoServicesRegistrar.isInApprovedOnlyMode()
);
return buildRestClient(remoteInfo, sslConfig, taskId, threadCollector, Optional.empty());
}

/**
Expand All @@ -207,8 +199,7 @@ static RestClient buildRestClient(
ReindexSslConfig sslConfig,
long taskId,
List<Thread> threadCollector,
Optional<HttpRequestInterceptor> restInterceptor,
boolean isFipsEnabled
Optional<HttpRequestInterceptor> restInterceptor
) {
Header[] clientHeaders = new Header[remoteInfo.getHeaders().size()];
int i = 0;
Expand All @@ -235,16 +226,11 @@ static RestClient buildRestClient(
}
// Stick the task id in the thread name so we can track down tasks from stack traces
AtomicInteger threads = new AtomicInteger();
c.setThreadFactory((Runnable r) -> {
Runnable runnable = () -> {
if (isFipsEnabled) {
CryptoServicesRegistrar.setApprovedOnlyMode(true);
}
r.run();
};
var thread = new Thread(runnable, "os-client-" + taskId + "-" + threads.getAndIncrement());
threadCollector.add(thread);
return thread;
c.setThreadFactory(r -> {
String name = "es-client-" + taskId + "-" + threads.getAndIncrement();
Thread t = new Thread(r, name);
threadCollector.add(t);
return t;
});
// Limit ourselves to one reactor thread because for now the search process is single threaded.
c.setIOReactorConfig(IOReactorConfig.custom().setIoThreadCount(1).build());
Expand Down Expand Up @@ -327,14 +313,7 @@ protected ScrollableHitSource buildScrollableResultSource(BackoffPolicy backoffP
RemoteInfo remoteInfo = mainRequest.getRemoteInfo();
createdThreads = synchronizedList(new ArrayList<>());
assert sslConfig != null : "Reindex ssl config must be set";
RestClient restClient = buildRestClient(
remoteInfo,
sslConfig,
task.getId(),
createdThreads,
this.interceptor,
CryptoServicesRegistrar.isInApprovedOnlyMode()
);
RestClient restClient = buildRestClient(remoteInfo, sslConfig, task.getId(), createdThreads, this.interceptor);
return new RemoteScrollableHitSource(
logger,
backoffPolicy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void testBuildRestClient() throws Exception {
assertBusy(() -> assertThat(threads, hasSize(2)));
int i = 0;
for (Thread thread : threads) {
assertEquals("os-client-" + taskId + "-" + i, thread.getName());
assertEquals("es-client-" + taskId + "-" + i, thread.getName());
i++;
}
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import com.sun.net.httpserver.HttpsParameters;
import com.sun.net.httpserver.HttpsServer;

import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.opensearch.client.Request;
import org.opensearch.client.Response;
import org.opensearch.client.RestClient;
Expand Down Expand Up @@ -73,7 +72,6 @@
import java.util.List;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
Expand All @@ -99,17 +97,6 @@ public static void setupHttpServer() throws Exception {
SSLContext sslContext = buildServerSslContext();
server = HttpsServer.create(address, 0);
server.setHttpsConfigurator(new ClientAuthHttpsConfigurator(sslContext));
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 -> {
assert http instanceof HttpsExchange;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Setti
@Override
public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException {
try {
final KeyStore keyStore = KeyStoreFactory.getInstance(KeyStoreType.PKCS_12);
final KeyStore keyStore = KeyStoreFactory.getInstance(KeyStoreType.JKS);
keyStore.load(
SecureNetty4HttpServerTransportTests.class.getResourceAsStream("/netty4-secure.p12"),
SecureNetty4HttpServerTransportTests.class.getResourceAsStream("/netty4-secure.jks"),
"password".toCharArray()
);

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

package org.opensearch.transport.netty4.ssl;

import org.apache.lucene.tests.util.LuceneTestCase;
import org.opensearch.Version;
import org.opensearch.cluster.node.DiscoveryNode;
import org.opensearch.common.crypto.KeyStoreFactory;
Expand Down Expand Up @@ -66,6 +67,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.lessThanOrEqualTo;

@LuceneTestCase.AwaitsFix(bugUrl = "")
public class SimpleSecureNetty4TransportTests extends AbstractSimpleTransportTestCase {
@Override
protected Transport build(Settings settings, final Version version, ClusterSettings clusterSettings, boolean doHandshake) {
Expand All @@ -79,9 +81,9 @@ public Optional<TransportExceptionHandler> buildServerTransportExceptionHandler(
@Override
public Optional<SSLEngine> buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException {
try {
final KeyStore keyStore = KeyStoreFactory.getInstance(KeyStoreType.PKCS_12);
final KeyStore keyStore = KeyStoreFactory.getInstance(KeyStoreType.JKS);
keyStore.load(
SimpleSecureNetty4TransportTests.class.getResourceAsStream("/netty4-secure.p12"),
SimpleSecureNetty4TransportTests.class.getResourceAsStream("/netty4-secure.jks"),
"password".toCharArray()
);

Expand Down
8 changes: 4 additions & 4 deletions modules/transport-netty4/src/test/resources/README.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

# 1. Create certificate key

openssl req -x509 -sha256 -newkey rsa:2048 -keyout certificate.key -out certificate.crt -days 1024 -nodes
openssl req -x509 -sha256 -newkey rsa:2048 -keyout certificate.key -out certificate.crt -days 1024 -nodes

# 2. Export the certificate in pkcs12 format

openssl pkcs12 -export -in certificate.crt -inkey certificate.key -out server.p12 -name netty4-secure -password pass:password
openssl pkcs12 -export -in certificate.crt -inkey certificate.key -out netty4-secure.p12 -name netty4-secure -password pass:password

# 3. Import the certificate into JDK keystore (PKCS12 type)
# 3. Migrate from P12 to JKS keystore

keytool -importkeystore -srcstorepass password -destkeystore netty4-secure.jks -srckeystore server.p12 -srcstoretype PKCS12 -alias netty4-secure -deststorepass password
keytool -importkeystore -srcstorepass password -destkeystore netty4-secure.jks -srckeystore server.p12 -srcstoretype PKCS12 -alias netty4-secure -deststorepass password
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@
import org.opensearch.transport.TransportSettings;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.rules.ExternalResource;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
Expand Down Expand Up @@ -107,17 +105,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {

private static Path keyStoreFile;

@ClassRule
public static final ExternalResource MUTE_IN_FIPS_JVM = new ExternalResource() {
@Override
protected void before() {
assumeFalse(
"Can't run in a FIPS JVM because none of the supported Keystore types can be used",
Boolean.parseBoolean(System.getProperty(FIPS_SYSPROP))
);
}
};

@BeforeClass
public static void setupKeyStore() throws IOException {
Path tempDir = createTempDir();
Expand Down
Loading

0 comments on commit 8228037

Please sign in to comment.