SOLR-9028: Fixed some test related bugs preventing SSL + ClientAuth from ever being tested

lucene/tools/junit4/solr-tests.policy

@@ -73,6 +73,7 @@ grant {
 
   // SSL related properties for Solr tests
   permission java.security.SecurityPermission "getProperty.ssl.*";
+  permission javax.net.ssl.SSLPermission "setDefaultSSLContext";
 
   // SASL/Kerberos related properties for Solr tests
   permission javax.security.auth.PrivateCredentialPermission "javax.security.auth.kerberos.KerberosTicket * \"*\"", "read";

solr/CHANGES.txt

@@ -162,6 +162,8 @@ Bug Fixes
 * SOLR-9034: Atomic updates failed to work when there were copyField targets that had docValues
   enabled. (Karthik Ramachandran, Ishan Chattopadhyaya, yonik)
 
+* SOLR-9028: Fixed some test related bugs preventing SSL + ClientAuth from ever being tested (hossman)
+
 Optimizations
 ----------------------
 * SOLR-8722: Don't force a full ZkStateReader refresh on every Overseer operation.

solr/core/src/java/org/apache/solr/client/solrj/embedded/SSLConfig.java

@@ -18,6 +18,11 @@ package org.apache.solr.client.solrj.embedded;
 
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 
+/** 
+ * Encapsulates settings related to SSL Configuration for an embedded Jetty Server.
+ * NOTE: all other settings are ignogred if {@link #isSSLMode} is false.
+ * @see #setUseSSL
+ */
 public class SSLConfig {
   
   private boolean useSsl;
@@ -26,7 +31,8 @@ public class SSLConfig {
   private String keyStorePassword;
   private String trustStore;
   private String trustStorePassword;
-  
+
+  /** NOTE: all other settings are ignored if useSSL is false; trustStore settings are ignored if clientAuth is false */
   public SSLConfig(boolean useSSL, boolean clientAuth, String keyStore, String keyStorePassword, String trustStore, String trustStorePassword) {
     this.useSsl = useSSL;
     this.clientAuth = clientAuth;
@@ -44,6 +50,7 @@ public class SSLConfig {
     this.clientAuth = clientAuth;
   }
   
+  /** All other settings on this object are ignored unless this is true */
   public boolean isSSLMode() {
     return useSsl;
   }
@@ -68,28 +75,41 @@ public class SSLConfig {
     return trustStorePassword;
   }
 
+  /**
+   * Returns an SslContextFactory that should be used by a jetty server based on the specified 
+   * configuration, or null if no SSL should be used.
+   *
+   * The specified sslConfig will be completely ignored if the "tests.jettySsl" system property is 
+   * true - in which case standard "javax.net.ssl.*" system properties will be used instead, along 
+   * with "tests.jettySsl.clientAuth"
+   * 
+   * @see #isSSLMode
+   */
   public static SslContextFactory createContextFactory(SSLConfig sslConfig) {
 
     if (sslConfig == null) {
-      if (Boolean.getBoolean(System.getProperty("tests.jettySsl"))) {
+      if (Boolean.getBoolean("tests.jettySsl")) {
         return configureSslFromSysProps();
       }
       return null;
     }
 
-    if (!sslConfig.useSsl)
+    if (!sslConfig.isSSLMode()) 
-      return null;
+       return null;
 
     SslContextFactory factory = new SslContextFactory(false);
     if (sslConfig.getKeyStore() != null)
       factory.setKeyStorePath(sslConfig.getKeyStore());
     if (sslConfig.getKeyStorePassword() != null)
       factory.setKeyStorePassword(sslConfig.getKeyStorePassword());
-    if (sslConfig.getTrustStore() != null)
+    factory.setNeedClientAuth(sslConfig.isClientAuthMode());
-      factory.setTrustStorePath(sslConfig.getTrustStore());
+    
-    if (sslConfig.getTrustStorePassword() != null)
+    if (sslConfig.isClientAuthMode()) {
-      factory.setTrustStorePassword(sslConfig.getTrustStorePassword());
+      if (sslConfig.getTrustStore() != null)
-
+        factory.setTrustStorePath(sslConfig.getTrustStore());
+      if (sslConfig.getTrustStorePassword() != null)
+        factory.setTrustStorePassword(sslConfig.getTrustStorePassword());
+    }
     return factory;
 
   }

solr/core/src/test/org/apache/solr/cloud/SSLMigrationTest.java

@@ -70,7 +70,7 @@ public class SSLMigrationTest extends AbstractFullDistribZkTestBase {
       runner.stop();
     }
     
-    HttpClientUtil.setHttpClientBuilder(sslConfig.getHttpClientBuilder());
+    HttpClientUtil.setSchemaRegistryProvider(sslConfig.buildClientSchemaRegistryProvider());
     for(int i = 0; i < this.jettys.size(); i++) {
       JettySolrRunner runner = jettys.get(i);
       JettyConfig config = JettyConfig.builder()

solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java

@@ -16,69 +16,336 @@
  */
 package org.apache.solr.cloud;
 
+import java.lang.invoke.MethodHandles;
+
+import java.util.Collections;
 import java.util.List;
+import java.io.File;
+import java.io.IOException;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLHandshakeException;
 
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.client.solrj.embedded.JettyConfig;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction;
-import org.junit.AfterClass;
+import org.apache.solr.util.SSLTestConfig;
-import org.junit.BeforeClass;
+
-import org.junit.Test;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.config.Registry;
+import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.socket.PlainConnectionSocketFactory;
+import org.apache.http.conn.ssl.SSLSocketFactory;
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+
+import org.apache.lucene.util.Constants;
+
+import org.junit.After;
+import org.junit.Before;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Tests SSL (if test framework selects it) with MiniSolrCloudCluster.
+ * Tests various permutations of SSL options with {@link MiniSolrCloudCluster}.
- * {@link TestMiniSolrCloudCluster} does not inherit from {@link SolrTestCaseJ4}
+ * <b>NOTE: This Test ignores the randomized SSL &amp; clientAuth settings selected by base class</b>,
- * so does not support SSL.
+ * instead each method initializes a {@link SSLTestConfig} will specific combinations of settings to test.
+ *
+ * @see TestSSLRandomization
  */
 public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 {
 
-  private static MiniSolrCloudCluster miniCluster;
+  private static final SSLContext DEFAULT_SSL_CONTEXT;
-  private static final int NUM_SERVERS = 5;
+  static {
-
+    try {
-  @BeforeClass
+      DEFAULT_SSL_CONTEXT = SSLContext.getDefault();
-  public static void startup() throws Exception {
+      assert null != DEFAULT_SSL_CONTEXT;
-    JettyConfig config = JettyConfig.builder().withSSLConfig(sslConfig).build();
+    } catch (Exception e) {
-    miniCluster = new MiniSolrCloudCluster(NUM_SERVERS, createTempDir(), config);
+      throw new RuntimeException("Unable to initialize 'Default' SSLContext Algorithm, JVM is borked", e);
-  }
-
-  @AfterClass
-  public static void shutdown() throws Exception {
-    if (miniCluster != null) {
-      miniCluster.shutdown();
     }
-    miniCluster = null;
+  }
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final int NUM_SERVERS = 3;
+  public static final String CONF_NAME = MethodHandles.lookup().lookupClass().getName();
+  
+  @Before
+  public void before() {
+    // 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
+    System.clearProperty(ZkStateReader.URL_SCHEME);
+  }
+  @After
+  public void after() {
+    HttpClientUtil.resetHttpClientBuilder(); // also resets SchemaRegistryProvider
+    System.clearProperty(ZkStateReader.URL_SCHEME);
+    SSLContext.setDefault(DEFAULT_SSL_CONTEXT);
+  }
+  
+  public void testNoSsl() throws Exception {
+    final SSLTestConfig sslConfig = new SSLTestConfig(false, false);
+    HttpClientUtil.setSchemaRegistryProvider(sslConfig.buildClientSchemaRegistryProvider());
+    System.setProperty(ZkStateReader.URL_SCHEME, "http");
+    checkClusterWithNodeReplacement(sslConfig);
+  }
+  
+  public void testNoSslButSillyClientAuth() throws Exception {
+    // this combination doesn't really make sense, since ssl==false the clientauth option will be ignored
+    // but we test it anyway for completeness of sanity checking the behavior of code that looks at those
+    // options.
+    final SSLTestConfig sslConfig = new SSLTestConfig(false, true);
+    HttpClientUtil.setSchemaRegistryProvider(sslConfig.buildClientSchemaRegistryProvider());
+    System.setProperty(ZkStateReader.URL_SCHEME, "http");
+    checkClusterWithNodeReplacement(sslConfig);
+  }
+  
+  public void testSslAndNoClientAuth() throws Exception {
+    final SSLTestConfig sslConfig = new SSLTestConfig(true, false);
+    HttpClientUtil.setSchemaRegistryProvider(sslConfig.buildClientSchemaRegistryProvider());
+    System.setProperty(ZkStateReader.URL_SCHEME, "https");
+    checkClusterWithNodeReplacement(sslConfig);
   }
 
-  @Test
+  public void testSslAndClientAuth() throws Exception {
-  public void testMiniSolrCloudClusterSSL() throws Exception {
+    assumeFalse("SOLR-9039: SSL w/clientAuth does not work on MAC_OS_X", Constants.MAC_OS_X);
-    // test send request to each server
+    
-    sendRequestToEachServer();
+    final SSLTestConfig sslConfig = new SSLTestConfig(true, true);
 
+    HttpClientUtil.setSchemaRegistryProvider(sslConfig.buildClientSchemaRegistryProvider());
+    System.setProperty(ZkStateReader.URL_SCHEME, "https");
+    checkClusterWithNodeReplacement(sslConfig);
+  }
+  
+  /**
+   * Constructs a cluster with the specified sslConfigs, runs {@link #checkClusterWithCollectionCreations}, 
+   * then verifies that if we modify the default SSLContext (mimicing <code>javax.net.ssl.*</code> 
+   * sysprops set on JVM startup) and reset to the default HttpClientBuilder, new HttpSolrClient instances 
+   * will still be able to talk to our servers.
+   *
+   * @see SSLContext#setDefault
+   * @see HttpClientUtil#resetHttpClientBuilder
+   * @see #checkClusterWithCollectionCreations
+   */
+  private void checkClusterWithNodeReplacement(SSLTestConfig sslConfig) throws Exception {
+    
+    final JettyConfig config = JettyConfig.builder().withSSLConfig(sslConfig).build();
+    final MiniSolrCloudCluster cluster = new MiniSolrCloudCluster(NUM_SERVERS, createTempDir(), config);
+    try {
+      checkClusterWithCollectionCreations(cluster, sslConfig);
+
+      
+      // Change the defaul SSLContext to match our test config, or to match our original system default if
+      // our test config doesn't use SSL, and reset HttpClientUtil to it's defaults so it picks up our
+      // SSLContext that way.
+      SSLContext.setDefault( sslConfig.isSSLMode() ? sslConfig.buildClientSSLContext() : DEFAULT_SSL_CONTEXT);
+      HttpClientUtil.resetHttpClientBuilder();
+      
+      // recheck that we can communicate with all the jetty instances in our cluster
+      checkClusterJettys(cluster, sslConfig);
+    } finally {
+      cluster.shutdown();
+    }
+  }
+
+  /**
+   * General purpose cluster sanity check...
+   * <ol>
+   * <li>Upload a config set</li>
+   * <li>verifies a collection can be created</li>
+   * <li>verifies many things that should succeed/fail when communicating with the cluster according to the specified sslConfig</li>
+   * <li>shutdown a server &amp; startup a new one in it's place</li>
+   * <li>repeat the verifications of ssl / no-ssl communication</li>
+   * <li>create a second collection</li>
+   * </ol>
+   * @see #CONF_NAME
+   * @see #NUM_SERVERS
+   */
+  public static void checkClusterWithCollectionCreations(final MiniSolrCloudCluster cluster,
+                                                         final SSLTestConfig sslConfig) throws Exception {
+
+    cluster.uploadConfigDir(new File(SolrTestCaseJ4.TEST_HOME() + File.separator +
+                                     "collection1" + File.separator + "conf"),
+                            CONF_NAME);
+    
+    checkCreateCollection(cluster, "first_collection");
+    
+    checkClusterJettys(cluster, sslConfig);
+    
     // shut down a server
-    JettySolrRunner stoppedServer = miniCluster.stopJettySolrRunner(0);
+    JettySolrRunner stoppedServer = cluster.stopJettySolrRunner(0);
     assertTrue(stoppedServer.isStopped());
-    assertEquals(NUM_SERVERS - 1, miniCluster.getJettySolrRunners().size());
+    assertEquals(NUM_SERVERS - 1, cluster.getJettySolrRunners().size());
-
+    
     // create a new server
-    JettySolrRunner startedServer = miniCluster.startJettySolrRunner();
+    JettySolrRunner startedServer = cluster.startJettySolrRunner();
     assertTrue(startedServer.isRunning());
-    assertEquals(NUM_SERVERS, miniCluster.getJettySolrRunners().size());
+    assertEquals(NUM_SERVERS, cluster.getJettySolrRunners().size());
+    
+    checkClusterJettys(cluster, sslConfig);
+    
+    checkCreateCollection(cluster, "second_collection");
+  }
+  
+  /**
+   * Verify that we can create a collection that involves one replica per node using the
+   * CloudSolrClient available for the cluster
+   */
+  private static void checkCreateCollection(final MiniSolrCloudCluster cluster,
+                                            final String collection) throws Exception {
+    assertNotNull(cluster.createCollection(collection,
+                                           /* 1 shard/replica per server */ NUM_SERVERS, 1,
+                                           CONF_NAME, null, null,
+                                           Collections.singletonMap("config","solrconfig-tlog.xml")));
+    final CloudSolrClient cloudClient = cluster.getSolrClient();
+    ZkStateReader zkStateReader = cloudClient.getZkStateReader();
+    AbstractDistribZkTestBase.waitForRecoveriesToFinish(collection, zkStateReader, true, true, 330);
+    assertEquals("sanity query", 0, cloudClient.query(collection, params("q","*:*")).getStatus());
+  }
+  
+  /** 
+   * verify that we can query all of the Jetty instances the specified cluster using the expected
+   * options (based on the sslConfig), and that we can <b>NOT</b> query the Jetty instances in 
+   * specified cluster in the ways that should fail (based on the sslConfig)
+   *
+   * @see #getRandomizedHttpSolrClient
+   */
+  private static void checkClusterJettys(final MiniSolrCloudCluster cluster,
+                                         final SSLTestConfig sslConfig) throws Exception {
 
-    // test send request to each server
+    final boolean ssl = sslConfig.isSSLMode();
-    sendRequestToEachServer();
+    List<JettySolrRunner> jettys = cluster.getJettySolrRunners();
+
+    for (JettySolrRunner jetty : jettys) {
+      final String baseURL = jetty.getBaseUrl().toString();
+
+      // basic base URL sanity checks
+      assertTrue("WTF baseURL: " + baseURL, null != baseURL && 10 < baseURL.length());
+      assertEquals("http vs https: " + baseURL,
+                   ssl ? "https" : "http:", baseURL.substring(0,5));
+      
+      // verify solr client success with expected protocol
+      try (HttpSolrClient client = getRandomizedHttpSolrClient(baseURL)) {
+        assertEquals(0, CoreAdminRequest.getStatus(/* all */ null, client).getStatus());
+      }
+      
+      // sanity check the HttpClient used under the hood by our the cluster's CloudSolrClient
+      // ensure it has the neccessary protocols/credentials for each jetty server
+      //
+      // NOTE: we're not responsible for closing the cloud client
+      final HttpClient cloudClient = cluster.getSolrClient().getLbClient().getHttpClient();
+      try (HttpSolrClient client = getRandomizedHttpSolrClient(baseURL)) {
+        assertEquals(0, CoreAdminRequest.getStatus(/* all */ null, client).getStatus());
+      }
+
+      final String wrongBaseURL = baseURL.replaceFirst((ssl ? "https://" : "http://"),
+                                                       (ssl ? "http://" : "https://"));
+          
+      // verify solr client using wrong protocol can't talk to server
+      expectThrows(SolrServerException.class, () -> {
+          try (HttpSolrClient client = getRandomizedHttpSolrClient(wrongBaseURL)) {
+            CoreAdminRequest req = new CoreAdminRequest();
+            req.setAction( CoreAdminAction.STATUS );
+            client.request(req);
+          }
+        });
+      
+      if (! sslConfig.isClientAuthMode()) {
+        // verify simple HTTP(S) client can't do HEAD request for URL with wrong protocol
+        try (CloseableHttpClient client = getSslAwareClientWithNoClientCerts()) {
+          final String wrongUrl = wrongBaseURL + "/admin/cores";
+          // vastly diff exception details betwen plain http vs https, not worried about details here
+          expectThrows(IOException.class, () -> {
+              doHeadRequest(client, wrongUrl);
+            });
+        }
+      }
+      
+      if (ssl) {
+        // verify expected results for a HEAD request to valid URL from HTTP(S) client w/o client certs
+        try (CloseableHttpClient client = getSslAwareClientWithNoClientCerts()) {
+          final String url = baseURL + "/admin/cores";
+          if (sslConfig.isClientAuthMode()) {
+            // w/o a valid client cert, SSL connection should fail
+
+            expectThrows(SSLHandshakeException.class, () -> {
+                doHeadRequest(client, url);
+              });
+          } else {
+            assertEquals("Wrong status for head request ("+url+") when clientAuth="
+                         + sslConfig.isClientAuthMode(),
+                         200, doHeadRequest(client, url));
+          }
+        }
+      }
+
+    }
   }
 
-  private void sendRequestToEachServer() throws Exception {
+  /** 
-    List<JettySolrRunner> jettys = miniCluster.getJettySolrRunners();
+   * Trivial helper method for doing a HEAD request of the specified URL using the specified client 
-    for (JettySolrRunner jetty : jettys) {
+   * and getting the HTTP statusCode from the response
-      try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString())) {
+   */
-        CoreAdminRequest req = new CoreAdminRequest();
+  private static int doHeadRequest(final CloseableHttpClient client, final String url) throws Exception {
-        req.setAction( CoreAdminAction.STATUS );
+    return client.execute(new HttpHead(url)).getStatusLine().getStatusCode();
-        client.request(req);
+  }
-      }
+  
-    }
+  /**
+   * Returns a new HttpClient that supports both HTTP and HTTPS (with the default test truststore), but 
+   * has no keystore -- so servers requiring client authentication should fail.
+   */
+  private static CloseableHttpClient getSslAwareClientWithNoClientCerts() throws Exception {
+    
+    // NOTE: This method explicitly does *NOT* use HttpClientUtil code because that
+    // will muck with the global static HttpClientBuilder / SchemeRegistryProvider
+    // and we can't do that and still test the entire purpose of what we are trying to test here.
+
+    final SSLTestConfig clientConfig = new SSLTestConfig(true, false);
+    
+    final SSLConnectionSocketFactory sslFactory = clientConfig.buildClientSSLConnectionSocketFactory();
+    assert null != sslFactory;
+
+    final Registry<ConnectionSocketFactory> socketFactoryReg = 
+      RegistryBuilder.<ConnectionSocketFactory> create()
+      .register("https", sslFactory)
+      .register("http", PlainConnectionSocketFactory.INSTANCE )
+      .build();
+    
+    final HttpClientBuilder builder = HttpClientBuilder.create();
+    builder.setConnectionManager(new PoolingHttpClientConnectionManager(socketFactoryReg));
+
+    return builder.build();
+  }
+
+  /** 
+   * Generates an HttpSolrClient, either by using the test framework helper method or by direct 
+   * instantiation (determined randomly)
+   * @see #getHttpSolrClient
+   */
+  public static HttpSolrClient getRandomizedHttpSolrClient(String url) {
+    // NOTE: at the moment, SolrTestCaseJ4 already returns "new HttpSolrClient" most of the time,
+    // so this method may seem redundent -- but the point here is to sanity check 2 things:
+    // 1) a direct test that "new HttpSolrClient" works given the current JVM/sysprop defaults
+    // 2) a sanity check that whatever getHttpSolrClient(String) returns will work regardless of
+    //    current test configuration.
+    // ... so we are hopefully future proofing against possible changes to SolrTestCaseJ4.getHttpSolrClient
+    // that "optimize" the test client construction in a way that would prevent us from finding bugs with
+    // regular HttpSolrClient instantiation.
+    if (random().nextBoolean()) {
+      return new HttpSolrClient(url);
+    } // else...
+    return getHttpSolrClient(url);
   }
 }

solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java

@@ -0,0 +1,54 @@
+/*
+ * 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.cloud;
+
+import java.lang.invoke.MethodHandles;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.util.SSLTestConfig;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A "test the test" method that verifies the SSL options randomized by {@link SolrTestCaseJ4} are 
+ * correctly used in the various helper methods available from the test framework and
+ * {@link MiniSolrCloudCluster}.
+ *
+ * @see TestMiniSolrCloudClusterSSL
+ */
+public class TestSSLRandomization extends SolrCloudTestCase {
+
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @BeforeClass
+  public static void createMiniSolrCloudCluster() throws Exception {
+    configureCluster(TestMiniSolrCloudClusterSSL.NUM_SERVERS).configure();
+  }
+  
+  public void testRandomizedSslAndClientAuth() throws Exception {
+    TestMiniSolrCloudClusterSSL.checkClusterWithCollectionCreations(cluster,sslConfig);
+  }
+  
+  public void testBaseUrl() throws Exception {
+    String url = buildUrl(6666, "/foo");
+    assertEquals(sslConfig.isSSLMode() ? "https://127.0.0.1:6666/foo" : "http://127.0.0.1:6666/foo", url);
+  }
+}

solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java

@@ -161,7 +161,7 @@ public class HttpClientUtil {
     httpClientBuilder = newHttpClientBuilder;
   }
 
-  public static void setSchemeRegistryProvider(SchemaRegistryProvider newRegistryProvider) {
+  public static void setSchemaRegistryProvider(SchemaRegistryProvider newRegistryProvider) {
     schemaRegistryProvider = newRegistryProvider;
   }
   

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -232,8 +232,8 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     newRandomConfig();
     
     sslConfig = buildSSLConfig();
-    //will use ssl specific or default depending on sslConfig
+    // based on randomized SSL config, set SchemaRegistryProvider appropriately
-    HttpClientUtil.setHttpClientBuilder(sslConfig.getHttpClientBuilder());
+    HttpClientUtil.setSchemaRegistryProvider(sslConfig.buildClientSchemaRegistryProvider());
     if(isSSLMode()) {
       // SolrCloud tests should usually clear this
       System.setProperty("urlScheme", "https");
@@ -324,13 +324,18 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     
     // we don't choose ssl that often because of SOLR-5776
     final boolean trySsl = random().nextInt(10) < 2;
+    // NOTE: clientAuth is useless unless trySsl==true, but we randomize it independently
+    // just in case it might find bugs in our test/ssl client code (ie: attempting to use
+    // SSL w/client cert to non-ssl servers)
     boolean trySslClientAuth = random().nextInt(10) < 2;
     if (Constants.MAC_OS_X) {
-      trySslClientAuth = false;
+      // see SOLR-9039
+      // If a solution is found to remove this, please make sure to also update
+      // TestMiniSolrCloudClusterSSL.testSslAndClientAuth as well.
+      trySslClientAuth = false; 
     }
     
-    log.info("Randomized ssl ({}) and clientAuth ({})", trySsl,
+    log.info("Randomized ssl ({}) and clientAuth ({})", trySsl, trySslClientAuth);
-        trySslClientAuth);
     
     return new SSLTestConfig(trySsl, trySslClientAuth);
   }

solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java

@@ -30,12 +30,12 @@ import org.apache.http.config.RegistryBuilder;
 import org.apache.http.conn.scheme.Scheme;
 import org.apache.http.conn.scheme.SchemeRegistry;
 import org.apache.http.conn.socket.ConnectionSocketFactory;
-import org.apache.http.conn.socket.LayeredConnectionSocketFactory;
+import org.apache.http.conn.socket.PlainConnectionSocketFactory;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.conn.ssl.SSLContexts;
+import org.apache.http.conn.ssl.SSLContextBuilder;
 import org.apache.http.conn.ssl.SSLSocketFactory;
 import org.apache.http.conn.ssl.TrustSelfSignedStrategy;
-import org.apache.http.impl.client.DefaultHttpClient;
 import org.apache.solr.client.solrj.embedded.SSLConfig;
 import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.client.solrj.impl.HttpClientUtil.SchemaRegistryProvider;
@@ -56,7 +56,7 @@ public class SSLTestConfig extends SSLConfig {
   }
   
   public SSLTestConfig(boolean useSSL, boolean clientAuth) {
-    super(useSSL, clientAuth, TEST_KEYSTORE_PATH, TEST_KEYSTORE_PASSWORD, TEST_KEYSTORE_PATH, TEST_KEYSTORE_PASSWORD);
+    this(useSSL, clientAuth, TEST_KEYSTORE_PATH, TEST_KEYSTORE_PASSWORD, TEST_KEYSTORE_PATH, TEST_KEYSTORE_PASSWORD);
   }
  
   public SSLTestConfig(boolean useSSL, boolean clientAuth, String keyStore, String keyStorePassword, String trustStore, String trustStorePassword) {
@@ -64,28 +64,49 @@ public class SSLTestConfig extends SSLConfig {
   }
   
   /**
-   * Will provide an SolrHttpClientBuilder for SSL support (adds https and
+   * Creates a {@link SchemaRegistryProvider} for HTTP <b>clients</b> to use when communicating with servers 
-   * removes http schemes) is SSL is enabled, otherwise return the default
+   * which have been configured based on the settings of this object.  When {@link #isSSLMode} is true, this 
-   * SolrHttpClientBuilder
+   * <code>SchemaRegistryProvider</code> will <i>only</i> support HTTPS (no HTTP scheme) using the 
+   * appropriate certs.  When {@link #isSSLMode} is false, <i>only</i> HTTP (no HTTPS scheme) will be 
+   * supported.
    */
-  public SolrHttpClientBuilder getHttpClientBuilder() {
+  public SchemaRegistryProvider buildClientSchemaRegistryProvider() {
-    SolrHttpClientBuilder builder = HttpClientUtil.getHttpClientBuilder();
+    if (isSSLMode()) {
-    return isSSLMode() ? new SSLHttpClientBuilderProvider().getBuilder(builder) : builder;
+      SSLConnectionSocketFactory sslConnectionFactory = buildClientSSLConnectionSocketFactory();
+      assert null != sslConnectionFactory;
+      return new SSLSchemaRegistryProvider(sslConnectionFactory);
+    } else {
+      return HTTP_ONLY_SCHEMA_PROVIDER;
+    }
   }
-
+  
   /**
-   * Builds a new SSLContext with the given configuration and allows the uses of
+   * Builds a new SSLContext for HTTP <b>clients</b> to use when communicating with servers which have 
-   * self-signed certificates during testing.
+   * been configured based on the settings of this object.  Also explicitly allows the use of self-signed 
+   * certificates (since that's what is almost always used during testing).
    */
-  protected SSLContext buildSSLContext() throws KeyManagementException, 
+  public SSLContext buildClientSSLContext() throws KeyManagementException, 
     UnrecoverableKeyException, NoSuchAlgorithmException, KeyStoreException {
+
+    assert isSSLMode();
     
-    return SSLContexts.custom()
+    SSLContextBuilder builder = SSLContexts.custom();
-        .loadKeyMaterial(buildKeyStore(getKeyStore(), getKeyStorePassword()), getKeyStorePassword().toCharArray())
+
-        .loadTrustMaterial(buildKeyStore(getTrustStore(), getTrustStorePassword()), new TrustSelfSignedStrategy()).build();
+    // NOTE: KeyStore & TrustStore are swapped because they are from configured from server perspective...
+    // we are a client - our keystore contains the keys the server trusts, and vice versa
+    builder.loadTrustMaterial(buildKeyStore(getKeyStore(), getKeyStorePassword()), new TrustSelfSignedStrategy()).build();
+
+    if (isClientAuthMode()) {
+      builder.loadKeyMaterial(buildKeyStore(getTrustStore(), getTrustStorePassword()), getTrustStorePassword().toCharArray());
+      
+    }
+
+    return builder.build();
   }
   
-  
+  /**
+   * Constructs a KeyStore using the specified filename and password
+   */
   protected static KeyStore buildKeyStore(String keyStoreLocation, String password) {
     try {
       return CertificateUtils.getKeyStore(Resource.newResource(keyStoreLocation), "JKS", null, password);
@@ -93,38 +114,53 @@ public class SSLTestConfig extends SSLConfig {
       throw new IllegalStateException("Unable to build KeyStore from file: " + keyStoreLocation, ex);
     }
   }
-  
-  private class SSLHttpClientBuilderProvider  {
-    
 
-    public SolrHttpClientBuilder getBuilder(SolrHttpClientBuilder builder) {
+  /** 
-
+   * Constructs a new SSLConnectionSocketFactory for HTTP <b>clients</b> to use when communicating 
-      HttpClientUtil.setSchemeRegistryProvider(new SchemaRegistryProvider() {
+   * with servers which have been configured based on the settings of this object. Will return null
-        
+   * unless {@link #isSSLMode} is true.
-        @Override
+   */
-        public Registry<ConnectionSocketFactory> getSchemaRegistry() {
+  public SSLConnectionSocketFactory buildClientSSLConnectionSocketFactory() {
-          SSLConnectionSocketFactory sslConnectionFactory;
+    if (!isSSLMode()) {
-          try {
+      return null;
-            boolean sslCheckPeerName = toBooleanDefaultIfNull(
-                toBooleanObject(System.getProperty(HttpClientUtil.SYS_PROP_CHECK_PEER_NAME)), true);
-            if (sslCheckPeerName == false) {
-              sslConnectionFactory = new SSLConnectionSocketFactory(buildSSLContext(),
-                  SSLSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER);
-            } else {
-              sslConnectionFactory = new SSLConnectionSocketFactory(buildSSLContext());
-            }
-          } catch (KeyManagementException | UnrecoverableKeyException | NoSuchAlgorithmException | KeyStoreException e) {
-            throw new IllegalStateException("Unable to setup https scheme for HTTPClient to test SSL.", e);
-          }
-          return  RegistryBuilder.<ConnectionSocketFactory>create()
-              .register("https", sslConnectionFactory).build();
-        }
-      });
-      HttpClientUtil.setHttpClientBuilder(builder);
-      return builder;
     }
-
+    SSLConnectionSocketFactory sslConnectionFactory;
+    try {
+      boolean sslCheckPeerName = toBooleanDefaultIfNull(toBooleanObject(System.getProperty(HttpClientUtil.SYS_PROP_CHECK_PEER_NAME)), true);
+      SSLContext sslContext = buildClientSSLContext();
+      if (sslCheckPeerName == false) {
+        sslConnectionFactory = new SSLConnectionSocketFactory
+          (sslContext, SSLSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER);
+      } else {
+        sslConnectionFactory = new SSLConnectionSocketFactory(sslContext);
+      }
+    } catch (KeyManagementException | UnrecoverableKeyException | NoSuchAlgorithmException | KeyStoreException e) {
+      throw new IllegalStateException("Unable to setup https scheme for HTTPClient to test SSL.", e);
+    }
+    return sslConnectionFactory;
   }
+
+  /** A SchemaRegistryProvider that only knows about SSL using a specified SSLConnectionSocketFactory */
+  private static class SSLSchemaRegistryProvider extends SchemaRegistryProvider {
+    private final SSLConnectionSocketFactory sslConnectionFactory;
+    public SSLSchemaRegistryProvider(SSLConnectionSocketFactory sslConnectionFactory) {
+      this.sslConnectionFactory = sslConnectionFactory;
+    }
+    @Override
+    public Registry<ConnectionSocketFactory> getSchemaRegistry() {
+      return RegistryBuilder.<ConnectionSocketFactory>create()
+        .register("https", sslConnectionFactory).build();
+    }
+  }
+
+  /** A SchemaRegistryProvider that only knows about HTTP */
+  private static final SchemaRegistryProvider HTTP_ONLY_SCHEMA_PROVIDER = new SchemaRegistryProvider() {
+    @Override
+    public Registry<ConnectionSocketFactory> getSchemaRegistry() {
+      return RegistryBuilder.<ConnectionSocketFactory>create()
+        .register("http", PlainConnectionSocketFactory.getSocketFactory()).build();
+    }
+  };
   
   public static boolean toBooleanDefaultIfNull(Boolean bool, boolean valueIfNull) {
     if (bool == null) {
@@ -142,7 +178,12 @@ public class SSLTestConfig extends SSLConfig {
     // no match
     return null;
   }
-  
+
+  /**
+   * @deprecated this method has very little practical use, in most cases you'll want to use 
+   * {@link SSLContext#setDefault} with {@link #buildClientSSLContext} instead.
+   */
+  @Deprecated
   public static void setSSLSystemProperties() {
     System.setProperty("javax.net.ssl.keyStore", TEST_KEYSTORE_PATH);
     System.setProperty("javax.net.ssl.keyStorePassword", TEST_KEYSTORE_PASSWORD);
@@ -150,6 +191,11 @@ public class SSLTestConfig extends SSLConfig {
     System.setProperty("javax.net.ssl.trustStorePassword", TEST_KEYSTORE_PASSWORD);
   }
   
+  /**
+   * @deprecated this method has very little practical use, in most cases you'll want to use 
+   * {@link SSLContext#setDefault} with {@link #buildClientSSLContext} instead.
+   */
+  @Deprecated
   public static void clearSSLSystemProperties() {
     System.clearProperty("javax.net.ssl.keyStore");
     System.clearProperty("javax.net.ssl.keyStorePassword");
@@ -157,4 +203,4 @@ public class SSLTestConfig extends SSLConfig {
     System.clearProperty("javax.net.ssl.trustStorePassword");
   }
   
-}
+}