Skip to content

Commit

Permalink
Test adjustments for FIPS 140 (elastic#56526)
Browse files Browse the repository at this point in the history 
This change aims to fix our setup in CI so that we can run 7.x in
FIPS 140 mode. The major issue that we have in 7.x and did not
have in master is that we can't use the diagnostic trust manager
in FIPS mode in Java 8 with SunJSSE in FIPS approved mode as it
explicitly disallows the wrapping of X509TrustManager.

Previous attempts like elastic#56427 and elastic#52211 focused on disabling the
setting in all of our tests when creating a Settings object or
on setting fips_mode.enabled accordingly (which implicitly disables
the diagnostic trust manager). The attempts weren't future proof
though as nothing would forbid someone to add new tests without
setting the necessary setting and forcing this would be very
inconvenient for any other case ( see

This change introduces a runtime check in SSLService that overrides
the configuration value of xpack.security.ssl.diagnose.trust and
disables the diagnostic trust manager when we are running in Java 8
and the SunJSSE provider is set in FIPS mode.
  • Loading branch information
@jkakavas
jkakavas committed Jun 9, 2020
1 parent fa0b82f commit 4091283
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 27 deletions.
8 changes: 6 additions & 2 deletions libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.AlgorithmParameters;
import java.security.Key;
import java.security.KeyStore;
import java.security.PrivateKey;
import java.security.interfaces.ECPrivateKey;
import java.security.spec.ECGenParameterSpec;
import java.security.spec.ECParameterSpec;
import java.util.function.Supplier;

Expand Down Expand Up @@ -72,8 +74,10 @@ public void testReadEcKeyCurves() throws Exception {
PrivateKey privateKey = PemUtils.readPrivateKey(getDataPath("/certs/pem-utils/private_" + curve + ".pem"), ""::toCharArray);
assertThat(privateKey, instanceOf(ECPrivateKey.class));
ECParameterSpec parameterSpec = ((ECPrivateKey) privateKey).getParams();
// This is brittle but we can't access sun.security.util.NamedCurve
assertThat(parameterSpec.toString(), containsString(curve));
ECGenParameterSpec algorithmParameterSpec = new ECGenParameterSpec(curve);
AlgorithmParameters algoParameters = AlgorithmParameters.getInstance("EC");
algoParameters.init(algorithmParameterSpec);
assertThat(parameterSpec, equalTo(algoParameters.getParameterSpec(ECParameterSpec.class)));
}

public void testReadPKCS8EcKey() throws Exception {
Expand Down
44 changes: 36 additions & 8 deletions plugins/discovery-ec2/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,34 @@ task writeTestJavaPolicy {
throw new GradleException("failed to create temporary directory [${tmp}]")
}
final File javaPolicy = file("${tmp}/java.policy")
javaPolicy.write(
[
"grant {",
" permission java.util.PropertyPermission \"com.amazonaws.sdk.ec2MetadataServiceEndpointOverride\", \"write\";",
"};"
].join("\n"))
if (BuildParams.inFipsJvm) {
javaPolicy.write(
[
"grant {",
" permission java.security.SecurityPermission \"putProviderProperty.BCFIPS\";",
" permission java.security.SecurityPermission \"putProviderProperty.BCJSSE\";",
" permission java.lang.RuntimePermission \"getProtectionDomain\";",
" permission java.util.PropertyPermission \"java.runtime.name\", \"read\";",
" permission org.bouncycastle.crypto.CryptoServicesPermission \"tlsAlgorithmsEnabled\";",
" permission java.lang.RuntimePermission \"accessClassInPackage.sun.security.internal.spec\";",
" permission java.lang.RuntimePermission \"accessDeclaredMembers\";",
" permission java.util.PropertyPermission \"intellij.debug.agent\", \"read\";",
" permission java.util.PropertyPermission \"intellij.debug.agent\", \"write\";",
" permission org.bouncycastle.crypto.CryptoServicesPermission \"exportSecretKey\";",
" permission org.bouncycastle.crypto.CryptoServicesPermission \"exportPrivateKey\";",
" permission java.io.FilePermission \"\${javax.net.ssl.trustStore}\", \"read\";",
" permission java.util.PropertyPermission \"com.amazonaws.sdk.ec2MetadataServiceEndpointOverride\", \"write\";",
"};"
].join("\n")
)
} else {
javaPolicy.write(
[
"grant {",
" permission java.util.PropertyPermission \"com.amazonaws.sdk.ec2MetadataServiceEndpointOverride\", \"write\";",
"};"
].join("\n"))
}
}
}

Expand All @@ -78,9 +100,15 @@ test {
// this is needed for insecure plugins, remove if possible!
systemProperty 'tests.artifact', project.name

// this is needed to manipulate com.amazonaws.sdk.ec2MetadataServiceEndpointOverride system property
// Setting a custom policy to manipulate com.amazonaws.sdk.ec2MetadataServiceEndpointOverride system property
// it is better rather disable security manager at all with `systemProperty 'tests.security.manager', 'false'`
systemProperty 'java.security.policy', "file://${buildDir}/tmp/java.policy"
if (BuildParams.inFipsJvm){
// Using the key==value format to override default JVM security settings and policy
// see also: https://docs.oracle.com/javase/8/docs/technotes/guides/security/PolicyFiles.html
systemProperty 'java.security.policy', "=file://${buildDir}/tmp/java.policy"
} else {
systemProperty 'java.security.policy', "file://${buildDir}/tmp/java.policy"
}
}

check {
Expand Down
4 changes: 2 additions & 2 deletions plugins/ingest-attachment/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,6 @@ if (BuildParams.inFipsJvm) {
// rather than provide a long list of exclusions, disable the check on FIPS.
jarHell.enabled = false
test.enabled = false
integTest.enabled = false;
testingConventions.enabled = false;
integTest.enabled = false
testingConventions.enabled = false
}
14 changes: 13 additions & 1 deletion x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.bootstrap.JavaVersion;
import org.elasticsearch.common.CheckedSupplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.DeprecationLogger;
Expand Down Expand Up @@ -47,6 +48,7 @@
import java.security.GeneralSecurityException;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.security.Security;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
Expand Down Expand Up @@ -826,11 +828,21 @@ private static String sslContextAlgorithm(List<String> supportedProtocols) {
}

private boolean shouldEnableDiagnoseTrust() {
if (XPackSettings.FIPS_MODE_ENABLED.get(settings) && DIAGNOSE_TRUST_EXCEPTIONS_SETTING.exists(settings) == false ) {
// We disable the DiagnosticTrustManager in Java 8 when SunJSSE is set in FIPS 140 mode, as it doesn't allow X509TrustManager to be
// wrapped
if (inSunJsseInFipsMode()) {
logger.info("diagnostic messages for SSL/TLS trust cannot be enabled for SunJSSE in FIPS mode.");
return false;
} else if (XPackSettings.FIPS_MODE_ENABLED.get(settings) && DIAGNOSE_TRUST_EXCEPTIONS_SETTING.exists(settings) == false) {
logger.info("diagnostic messages for SSL/TLS trust failures are not enabled in FIPS 140 mode by default.");
return false;
} else {
return DIAGNOSE_TRUST_EXCEPTIONS_SETTING.get(settings);
}
}

static boolean inSunJsseInFipsMode() {
return JavaVersion.current().getVersion().get(0) == 8 && Arrays.stream(Security.getProviders())
.anyMatch(provider -> provider.getName().equals("SunJSSE") && provider.getInfo().contains("FIPS mode"));
}
}
8 changes: 4 additions & 4 deletions ...test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ public void testGetInsecureTransportProfileConfigurations() {
}

private Settings.Builder getBaseSettings() {
final Path keystore = randomBoolean()
? getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks")
: getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12");

final Path keystore = inFipsJvm()
? getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12")
: getDataPath(randomFrom("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks",
"/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12"));
MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode");

Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/PemUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.AlgorithmParameters;
import java.security.Key;
import java.security.KeyStore;
import java.security.PrivateKey;
import java.security.interfaces.ECPrivateKey;
import java.security.spec.ECGenParameterSpec;
import java.security.spec.ECParameterSpec;

import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -70,8 +72,10 @@ public void testReadEcKeyCurves() throws Exception {
("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/private_" + curve + ".pem"), ""::toCharArray);
assertThat(privateKey, instanceOf(ECPrivateKey.class));
ECParameterSpec parameterSpec = ((ECPrivateKey) privateKey).getParams();
// This is brittle but we can't access sun.security.util.NamedCurve
assertThat(parameterSpec.toString(), containsString(curve));
ECGenParameterSpec algorithmParameterSpec = new ECGenParameterSpec(curve);
AlgorithmParameters algoParameters = AlgorithmParameters.getInstance("EC");
algoParameters.init(algorithmParameterSpec);
assertThat(parameterSpec, equalTo(algoParameters.getParameterSpec(ECParameterSpec.class)));
}

public void testReadEncryptedPKCS8Key() throws Exception {
Expand Down
24 changes: 16 additions & 8 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import java.util.concurrent.atomic.AtomicInteger;

import static org.elasticsearch.test.TestMatchers.throwableWithMessage;
import static org.elasticsearch.xpack.core.ssl.SSLService.inSunJsseInFipsMode;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -90,14 +91,19 @@ public class SSLServiceTests extends ESTestCase {

@Before
public void setup() throws Exception {
// Randomise the keystore type (jks/PKCS#12)
if (randomBoolean()) {
testnodeStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks");
// The default is to use JKS. Randomly test with explicit and with the default value.
testnodeStoreType = "jks";
} else {
// Randomise the keystore type (jks/PKCS#12) when possible
if (inFipsJvm()) {
testnodeStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12");
testnodeStoreType = randomBoolean() ? "PKCS12" : null;
} else {
if (randomBoolean()) {
testnodeStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks");
// The default is to use JKS. Randomly test with explicit and with the default value.
testnodeStoreType = "jks";
} else {
testnodeStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12");
testnodeStoreType = randomBoolean() ? "PKCS12" : null;
}
}
logger.info("Using [{}] key/truststore [{}]", testnodeStoreType, testnodeStore);
testnodeCert = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt");
Expand Down Expand Up @@ -820,6 +826,7 @@ public void testThatSSLIOSessionStrategyTrustsJDKTrustedCAs() throws Exception {
}

public void testWrapTrustManagerWhenDiagnosticsEnabled() {
assumeFalse("We explicitly disable diagnostic trust manager in SunJSSE in FIPS mode ", inSunJsseInFipsMode());
final Settings.Builder builder = Settings.builder();
if (randomBoolean()) { // randomly select between default, and explicit enabled
builder.put("xpack.security.ssl.diagnose.trust", true);
Expand All @@ -841,7 +848,7 @@ public void testDontWrapTrustManagerWhenDiagnosticsDisabled() {
assertThat(sslService.wrapWithDiagnostics(baseTrustManager, sslConfiguration), sameInstance(baseTrustManager));
}

public void testDontWrapTrustManagerByDefaultWhenInFips(){
public void testDontWrapTrustManagerByDefaultWhenInFips() {
final Settings.Builder builder = Settings.builder();
builder.put("xpack.security.fips_mode.enabled", true);
final SSLService sslService = new SSLService(builder.build(), env);
Expand All @@ -850,7 +857,8 @@ public void testDontWrapTrustManagerByDefaultWhenInFips(){
assertThat(sslService.wrapWithDiagnostics(baseTrustManager, sslConfiguration), sameInstance(baseTrustManager));
}

public void testWrapTrustManagerWhenInFipsAndExplicitlyConfigured(){
public void testWrapTrustManagerWhenInFipsAndExplicitlyConfigured() {
assumeFalse("We explicitly disable diagnostic trust manager in SunJSSE in FIPS mode ", inSunJsseInFipsMode());
final Settings.Builder builder = Settings.builder();
builder.put("xpack.security.fips_mode.enabled", true);
builder.put("xpack.security.ssl.diagnose.trust", true);
Expand Down

0 comments on commit 4091283

Please sign in to comment.