From 424db312c32b07e365f05ae27eb6069c0062df9f Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Fri, 15 May 2020 18:10:45 +0300 Subject: [PATCH] Test adjustments for FIPS 140 (#56526) 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 #56427 and #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 #56427 (comment) for the full argumentation). 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. --- .../common/ssl/PemUtilsTests.java | 8 +++- plugins/discovery-ec2/build.gradle | 44 +++++++++++++++---- plugins/ingest-attachment/build.gradle | 4 +- .../xpack/core/ssl/SSLService.java | 14 +++++- .../transport/ProfileConfigurationsTests.java | 8 ++-- .../xpack/core/ssl/PemUtilsTests.java | 8 +++- .../xpack/core/ssl/SSLServiceTests.java | 24 ++++++---- .../ssl/SSLReloadDuringStartupIntegTests.java | 6 +++ 8 files changed, 89 insertions(+), 27 deletions(-) diff --git a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemUtilsTests.java b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemUtilsTests.java index d5657b5517b94..711bbfed22d36 100644 --- a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemUtilsTests.java +++ b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemUtilsTests.java @@ -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; @@ -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 { diff --git a/plugins/discovery-ec2/build.gradle b/plugins/discovery-ec2/build.gradle index 7b0f412a05b49..3c96ad0134243 100644 --- a/plugins/discovery-ec2/build.gradle +++ b/plugins/discovery-ec2/build.gradle @@ -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")) + } } } @@ -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 { diff --git a/plugins/ingest-attachment/build.gradle b/plugins/ingest-attachment/build.gradle index 32ea5b8134c19..c6cbd3fceaeb3 100644 --- a/plugins/ingest-attachment/build.gradle +++ b/plugins/ingest-attachment/build.gradle @@ -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 } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 90c37aefa778e..d1854920d69be 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -14,6 +14,7 @@ import org.apache.logging.log4j.Logger; 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; @@ -46,6 +47,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; @@ -829,11 +831,21 @@ private static String sslContextAlgorithm(List 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")); + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java index fd7315d7457c2..14f12914dca5d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java @@ -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"); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/PemUtilsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/PemUtilsTests.java index 6cc0f4763b92a..5b9f6d6117a16 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/PemUtilsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/PemUtilsTests.java @@ -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; @@ -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 { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java index 08df2d1b65907..f131de78150d1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java @@ -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; @@ -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"); @@ -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); @@ -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); @@ -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); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java index 5458202d80e82..90b3d89dc5c27 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.InternalTestCluster.RestartCallback; import org.elasticsearch.test.SecurityIntegTestCase; +import org.junit.BeforeClass; import java.io.IOException; import java.io.UncheckedIOException; @@ -28,6 +29,11 @@ @ClusterScope(transportClientRatio = 0) public class SSLReloadDuringStartupIntegTests extends SecurityIntegTestCase { + @BeforeClass + public static void skipInFips() { + assumeFalse("Can't use JKS keystores in FIPS JVM", inFipsJvm()); + } + @Override public Settings nodeSettings(int nodeOrdinal) { Settings settings = super.nodeSettings(nodeOrdinal);