From aaf20aefa4b29971dbbb16c9fe39e6272c7c9dd5 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Thu, 27 Jun 2019 15:51:20 -0700 Subject: [PATCH] SOLR-12988: SSLTestConfig has been changed to throw AssumptionViolatedException when tests/seeds request SSL but the JVM appears to be an OpenJDK version known to have SSL bugs SOLR-13574: Add CHANGES entry that was overlooked --- solr/CHANGES.txt | 8 +- .../cloud/TestMiniSolrCloudClusterSSL.java | 2 - .../solr/cloud/TestSSLRandomization.java | 2 - .../HttpSolrClientSSLAuthConPoolTest.java | 3 - .../org/apache/solr/util/SSLTestConfig.java | 60 +++++++++++-- .../apache/solr/util/TestSSLTestConfig.java | 87 +++++++++++++++++++ 6 files changed, 145 insertions(+), 17 deletions(-) create mode 100644 solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 5dcf499049d..5485927ad98 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -189,7 +189,10 @@ Bug Fixes * SOLR-12979: Improve error message and change error code to 400 when collapse field is non-docValued and non-uninvertible (hossman, Munendra S N) - + +* SOLR-13574: Fix many test and test-framework classes to not fail on After/AfterClass cleanup if + assumptions fail in Before/BeforeClass setup (hossman) + Other Changes ---------------------- @@ -208,6 +211,9 @@ Other Changes * SOLR-13511: Add SearchHandler.newResponseBuilder method to facilitate custom plugins' maintenance of per-request state in a custom ResponseBuilder. (Ramsey Haddad, Christine Poerschke) + +* SOLR-12988: SSLTestConfig has been changed to throw AssumptionViolatedException when tests/seeds + request SSL but the JVM appears to be an OpenJDK version known to have SSL bugs (hossman, Cao Manh Dat) ================== 8.1.2 ================== diff --git a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java index b9e8a0417c6..b659a1f397b 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java @@ -85,8 +85,6 @@ public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 { @Before public void before() { - assumeFalse("@AwaitsFix: SOLR-12988 - ssl issues on Java 11/12", Constants.JRE_IS_MINIMUM_JAVA11); - // undo the randomization of our super class log.info("NOTE: This Test ignores the randomized SSL & clientAuth settings selected by base class"); HttpClientUtil.resetHttpClientBuilder(); // also resets SchemaRegistryProvider diff --git a/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java b/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java index 12411895553..e846f73bc33 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java @@ -19,7 +19,6 @@ package org.apache.solr.cloud; import java.lang.invoke.MethodHandles; import java.util.Arrays; -import org.apache.lucene.util.Constants; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.util.SSLTestConfig; import org.apache.solr.util.RandomizeSSL; @@ -44,7 +43,6 @@ public class TestSSLRandomization extends SolrCloudTestCase { @BeforeClass public static void createMiniSolrCloudCluster() throws Exception { - assumeFalse("@AwaitsFix: SOLR-12988 - ssl issues on Java 11/12", Constants.JRE_IS_MINIMUM_JAVA11); configureCluster(TestMiniSolrCloudClusterSSL.NUM_SERVERS).configure(); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java index 3b590493c88..cab94ac19f0 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java @@ -20,7 +20,6 @@ package org.apache.solr.client.solrj.impl; import java.net.URL; import java.util.Arrays; -import org.apache.lucene.util.Constants; import org.apache.solr.util.RandomizeSSL; import org.junit.BeforeClass; @@ -30,8 +29,6 @@ public class HttpSolrClientSSLAuthConPoolTest extends HttpSolrClientConPoolTest @BeforeClass public static void checkUrls() throws Exception { - assumeFalse("@AwaitsFix: SOLR-12988 - ssl issues on Java 11/12", Constants.JRE_IS_MINIMUM_JAVA11); - URL[] urls = new URL[] { jetty.getBaseUrl(), yetty.getBaseUrl() }; diff --git a/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java b/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java index 502df407915..88b6a1c1a9b 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java +++ b/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java @@ -25,7 +25,8 @@ import java.security.SecureRandom; import java.security.SecureRandomSpi; import java.security.UnrecoverableKeyException; import java.util.Random; - +import java.util.regex.Pattern; + import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; import org.apache.http.conn.socket.ConnectionSocketFactory; @@ -43,6 +44,8 @@ import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.security.CertificateUtils; import org.eclipse.jetty.util.ssl.SslContextFactory; +import com.carrotsearch.randomizedtesting.RandomizedTest; + /** * An SSLConfig that provides {@link SSLConfig} and {@link SchemaRegistryProvider} for both clients and servers * that supports reading key/trust store information directly from resource files provided with the @@ -56,8 +59,8 @@ public class SSLTestConfig { private final boolean checkPeerName; private final Resource keyStore; private final Resource trustStore; - private boolean useSsl; - private boolean clientAuth; + private final boolean useSsl; + private final boolean clientAuth; /** Creates an SSLTestConfig that does not use SSL or client authentication */ public SSLTestConfig() { @@ -99,15 +102,14 @@ public class SSLTestConfig { * @see HttpClientUtil#SYS_PROP_CHECK_PEER_NAME */ public SSLTestConfig(boolean useSSL, boolean clientAuth, boolean checkPeerName) { - // @AwaitsFix: SOLR-12988 - ssl issues on Java 11/12 - if (Constants.JRE_IS_MINIMUM_JAVA11) { - this.useSsl = false; - } else { - this.useSsl = useSSL; - } + this.useSsl = useSSL; this.clientAuth = clientAuth; this.checkPeerName = checkPeerName; + if (useSsl) { + assumeSslIsSafeToTest(); + } + final String resourceName = checkPeerName ? TEST_KEYSTORE_LOCALHOST_RESOURCE : TEST_KEYSTORE_BOGUSHOST_RESOURCE; trustStore = keyStore = Resource.newClassPathResource(resourceName); @@ -339,4 +341,44 @@ public class SSLTestConfig { synchronized public void setSeed(long seed) { /* NOOP */ } } + + /** + * Helper method for sanity checking if it's safe to use SSL on this JVM + * + * @see SOLR-12988 + * @throws org.junit.internal.AssumptionViolatedException if this JVM is known to have SSL problems + */ + public static void assumeSslIsSafeToTest() { + if (Constants.JVM_NAME.startsWith("OpenJDK") || + Constants.JVM_NAME.startsWith("Java HotSpot(TM)")) { + RandomizedTest.assumeFalse("Test (or randomization for this seed) wants to use SSL, " + + "but SSL is known to fail on your JVM: " + + Constants.JVM_NAME + " / " + Constants.JVM_VERSION, + isOpenJdkJvmVersionKnownToHaveProblems(Constants.JVM_VERSION)); + } + } + + /** + * package visibility for tests + * @see Constants#JVM_VERSION + * @lucene.internal + */ + static boolean isOpenJdkJvmVersionKnownToHaveProblems(final String jvmVersion) { + // TODO: would be nice to replace with Runtime.Version once we don't have to + // worry about java8 support when backporting to branch_8x + return KNOWN_BAD_OPENJDK_JVMS.matcher(jvmVersion).matches(); + + } + private static final Pattern KNOWN_BAD_OPENJDK_JVMS + = Pattern.compile(// 11 to 11.0.2 were all definitely problematic + // - https://bugs.openjdk.java.net/browse/JDK-8212885 + // - https://bugs.openjdk.java.net/browse/JDK-8213202 + "(^11(\\.0(\\.0|\\.1|\\.2)?)?($|(\\_|\\+|\\-).*$))|" + + // early (pre-ea) "testing" builds of 11, 12, and 13 were also buggy + // - https://bugs.openjdk.java.net/browse/JDK-8224829 + "(^(11|12|13).*-testing.*$)|" + + // So far, all 13-ea builds (up to 13-ea-26) have been buggy + // - https://bugs.openjdk.java.net/browse/JDK-8226338 + "(^13-ea.*$)" + ); } diff --git a/solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java b/solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java new file mode 100644 index 00000000000..4e3995469c0 --- /dev/null +++ b/solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util; + +import java.util.Arrays; +import java.util.List; + +import org.apache.solr.SolrTestCase; + +public class TestSSLTestConfig extends SolrTestCase { + + /** Sanity check that our JVM version parsing logic seems correct */ + public void testIsOpenJdkJvmVersionKnownToHaveProblems() { + final List rel_suffixes = Arrays.asList("", "+42"); + final List ea_suffixes = Arrays.asList("-ea", "-ea+42"); + final List suffixes = Arrays.asList("", "+42", "-ea", "-ea+42"); + + // as far as we know, any Java 8, 9 or 10 impl should be fine... + for (String base : Arrays.asList("1.8", "1.8.0", "1.8.1", + "9", "9.0", "9.1", "9.0.0", "9.1.0", "9.1.1", + "10", "10.0", "10.1", "10.0.0", "10.1.0", "10.1.1")) { + for (String suffix : suffixes) { + final String v = base + suffix; + assertFalse(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v)); + } + } + + // Known Problems start with Java 11... + + // java 11 releases below 11.0.3 were all bad... + for (String bad : Arrays.asList("11", "11.0", "11.0.1", "11.0.2")) { + for (String suffix : suffixes) { + final String v = bad + suffix; + assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v)); + } + } + + // ...but 11.0.3 or higher should be ok. + for (String ok : Arrays.asList("11.0.3", "11.0.42", "11.1", "11.1.42")) { + for (String suffix : suffixes) { + final String v = ok + suffix; + assertFalse(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v)); + } + } + + // As far as we know/hope, all "official" java 12 and higher impls should be fine... + for (String major : Arrays.asList("12", "13", "99")) { + for (String minor : Arrays.asList("", ".0", ".42", ".0.42")) { + for (String suffix : rel_suffixes) { + final String v = major + minor + suffix; + assertFalse(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v)); + } + } + } + + // ...but pre EA "testing" builds of 11, 12, and 13 are all definitely problematic... + for (String major : Arrays.asList("11", "12", "13")) { + for (String suffix : suffixes) { + final String v = major + "-testing" + suffix; + assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v)); + } + } + + // ...and all 13-ea builds (so far) have definitely been problematic. + for (String suffix : ea_suffixes) { + final String v = "13" + suffix; + assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v)); + } + + } + +}