Revert "Revert "Bjorncs/ssl handshake metric""

6 files changed, 268 insertions, 61 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
index 01d6fa02d6e..2796ee2d271 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
@@ -42,7 +42,7 @@ public class ConnectorFactory {
 
     public ServerConnector createConnector(final Metric metric, final Server server, final ServerSocketChannel ch) {
         ServerConnector connector = new JDiscServerConnector(
-                connectorConfig, metric, server, ch, createConnectionFactories().toArray(ConnectionFactory[]::new));
+                connectorConfig, metric, server, ch, createConnectionFactories(metric).toArray(ConnectionFactory[]::new));
         connector.setPort(connectorConfig.listenPort());
         connector.setName(connectorConfig.name());
         connector.setAcceptQueueSize(connectorConfig.acceptQueueSize());
@@ -51,14 +51,14 @@ public class ConnectorFactory {
         return connector;
     }
 
-    private List<ConnectionFactory> createConnectionFactories() {
+    private List<ConnectionFactory> createConnectionFactories(Metric metric) {
         HttpConnectionFactory httpConnectionFactory = newHttpConnectionFactory();
         if (connectorConfig.healthCheckProxy().enable()) {
             return List.of(httpConnectionFactory);
         } else if (connectorConfig.ssl().enabled()) {
-            return List.of(newSslConnectionFactory(), httpConnectionFactory);
+            return List.of(newSslConnectionFactory(metric), httpConnectionFactory);
         } else if (TransportSecurityUtils.isTransportSecurityEnabled()) {
-            SslConnectionFactory sslConnectionsFactory = newSslConnectionFactory();
+            SslConnectionFactory sslConnectionsFactory = newSslConnectionFactory(metric);
             switch (TransportSecurityUtils.getInsecureMixedMode()) {
                 case TLS_CLIENT_MIXED_SERVER:
                 case PLAINTEXT_CLIENT_MIXED_SERVER:
@@ -88,9 +88,11 @@ public class ConnectorFactory {
         return new HttpConnectionFactory(httpConfig);
     }
 
-    private SslConnectionFactory newSslConnectionFactory() {
-        SslContextFactory factory = sslContextFactoryProvider.getInstance(connectorConfig.name(), connectorConfig.listenPort());
-        return new SslConnectionFactory(factory, HttpVersion.HTTP_1_1.asString());
+    private SslConnectionFactory newSslConnectionFactory(Metric metric) {
+        SslContextFactory ctxFactory = sslContextFactoryProvider.getInstance(connectorConfig.name(), connectorConfig.listenPort());
+        SslConnectionFactory connectionFactory = new SslConnectionFactory(ctxFactory, HttpVersion.HTTP_1_1.asString());
+        connectionFactory.addBean(new SslHandshakeFailedListener(metric, connectorConfig.name(), connectorConfig.listenPort()));
+        return connectionFactory;
     }
 
     private OptionalSslConnectionFactory newOptionalSslConnectionFactory(SslConnectionFactory sslConnectionsFactory) {
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 140feb75026..5c658d7b3df 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -114,6 +114,12 @@ public class JettyHttpServer extends AbstractServerProvider {
 
         String URI_LENGTH = "jdisc.http.request.uri_length";
         String CONTENT_SIZE = "jdisc.http.request.content_size";
+
+        String SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT = "jdisc.http.ssl.handshake_failure.missing_client_cert";
+        String SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT = "jdisc.http.ssl.handshake_failure.invalid_client_cert";
+        String SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_PROTOCOLS = "jdisc.http.ssl.handshake_failure.incompatible_protocols";
+        String SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_CIPHERS = "jdisc.http.ssl.handshake_failure.incompatible_ciphers";
+        String SSL_HANDSHAKE_FAILURE_UNKNOWN = "jdisc.http.ssl.handshake_failure.unknown";
     }
 
     private final static Logger log = Logger.getLogger(JettyHttpServer.class.getName());
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java
new file mode 100644
index 00000000000..84f4fd118cc
--- /dev/null
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/SslHandshakeFailedListener.java
@@ -0,0 +1,83 @@
+// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.jdisc.http.server.jetty;
+
+import com.yahoo.jdisc.Metric;
+import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics;
+import org.eclipse.jetty.io.ssl.SslHandshakeListener;
+
+import javax.net.ssl.SSLHandshakeException;
+import java.util.Map;
+import java.util.Optional;
+import java.util.function.Predicate;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.regex.Pattern;
+
+/**
+ * A {@link SslHandshakeListener} that reports metrics for SSL handshake failures.
+ *
+ * @author bjorncs
+ */
+class SslHandshakeFailedListener implements SslHandshakeListener {
+
+    private final static Logger log = Logger.getLogger(SslHandshakeFailedListener.class.getName());
+
+    private final Metric metric;
+    private final String connectorName;
+    private final int listenPort;
+
+    SslHandshakeFailedListener(Metric metric, String connectorName, int listenPort) {
+        this.metric = metric;
+        this.connectorName = connectorName;
+        this.listenPort = listenPort;
+    }
+
+    @Override
+    public void handshakeFailed(Event event, Throwable throwable) {
+        log.log(Level.FINE, throwable, () -> "Ssl handshake failed: " + throwable.getMessage());
+        String metricName = SslHandshakeFailure.fromSslHandshakeException((SSLHandshakeException) throwable)
+                .map(SslHandshakeFailure::metricName)
+                .orElse(Metrics.SSL_HANDSHAKE_FAILURE_UNKNOWN);
+        metric.add(metricName, 1L, metric.createContext(createDimensions()));
+    }
+
+    private Map<String, Object> createDimensions() {
+        return Map.of(Metrics.NAME_DIMENSION, connectorName, Metrics.PORT_DIMENSION, listenPort);
+    }
+
+    private enum SslHandshakeFailure {
+        INCOMPATIBLE_PROTOCOLS(
+                Metrics.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_PROTOCOLS,
+                "(Client requested protocol \\S+? is not enabled or supported in server context" +
+                        "|The client supported protocol versions \\[\\S+?\\] are not accepted by server preferences \\[\\S+?\\])"),
+        INCOMPATIBLE_CIPHERS(
+                Metrics.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_CIPHERS,
+                "no cipher suites in common"),
+        MISSING_CLIENT_CERT(
+                Metrics.SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT,
+                "Empty server certificate chain"),
+        INVALID_CLIENT_CERT(
+                Metrics.SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT,
+                "PKIX path (building|validation) failed: .+");
+
+        private final String metricName;
+        private final Predicate<String> messageMatcher;
+
+        SslHandshakeFailure(String metricName, String messagePattern) {
+            this.metricName = metricName;
+            this.messageMatcher = Pattern.compile(messagePattern).asMatchPredicate();
+        }
+
+        String metricName() { return metricName; }
+
+        static Optional<SslHandshakeFailure> fromSslHandshakeException(SSLHandshakeException exception) {
+            String message = exception.getMessage();
+            for (SslHandshakeFailure failure : values()) {
+                if (failure.messageMatcher.test(message)) {
+                    return Optional.of(failure);
+                }
+            }
+            return Optional.empty();
+        }
+    }
+}
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
index 31ecf3ca2fc..7ecf1b00fc1 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
@@ -5,10 +5,12 @@ import com.google.inject.AbstractModule;
 import com.google.inject.Module;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.logging.AccessLogEntry;
+import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.References;
 import com.yahoo.jdisc.Request;
 import com.yahoo.jdisc.Response;
 import com.yahoo.jdisc.application.BindingSetSelector;
+import com.yahoo.jdisc.application.MetricConsumer;
 import com.yahoo.jdisc.handler.AbstractRequestHandler;
 import com.yahoo.jdisc.handler.CompletionHandler;
 import com.yahoo.jdisc.handler.ContentChannel;
@@ -21,6 +23,8 @@ import com.yahoo.jdisc.http.Cookie;
 import com.yahoo.jdisc.http.HttpRequest;
 import com.yahoo.jdisc.http.HttpResponse;
 import com.yahoo.jdisc.http.ServerConfig;
+import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics;
+import com.yahoo.jdisc.http.server.jetty.TestDrivers.TlsClientAuth;
 import com.yahoo.jdisc.service.BindingSetNotFoundException;
 import com.yahoo.security.KeyUtils;
 import com.yahoo.security.SslContextBuilder;
@@ -34,6 +38,8 @@ import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLHandshakeException;
 import javax.security.auth.x500.X500Principal;
 import java.io.IOException;
 import java.math.BigInteger;
@@ -54,6 +60,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 import static com.yahoo.jdisc.Response.Status.GATEWAY_TIMEOUT;
@@ -81,15 +89,21 @@ import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyMap;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 /**
  * @author Oyvind Bakksjo
  * @author Simon Thoresen Hult
+ * @author bjorncs
  */
 public class HttpServerTest {
 
+    private static final Logger log = Logger.getLogger(HttpServerTest.class.getName());
+
     @Rule
     public TemporaryFolder tmpFolder = new TemporaryFolder();
 
@@ -478,7 +492,7 @@ public class HttpServerTest {
         Path certificateFile = tmpFolder.newFile().toPath();
         generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
 
-        final TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile);
+        final TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT);
         driver.client().get("/status.html")
               .expectStatusCode(is(OK));
         assertThat(driver.close(), is(true));
@@ -489,7 +503,7 @@ public class HttpServerTest {
         Path privateKeyFile = tmpFolder.newFile().toPath();
         Path certificateFile = tmpFolder.newFile().toPath();
         generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
-        TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile);
+        TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT);
 
         SSLContext trustStoreOnlyCtx = new SslContextBuilder()
                 .withTrustStore(certificateFile)
@@ -507,7 +521,7 @@ public class HttpServerTest {
         Path privateKeyFile = tmpFolder.newFile().toPath();
         Path certificateFile = tmpFolder.newFile().toPath();
         generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
-        TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile);
+        TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT);
 
         SSLContext trustStoreOnlyCtx = new SslContextBuilder()
                 .withTrustStore(certificateFile)
@@ -559,6 +573,114 @@ public class HttpServerTest {
         assertThat(driver.close(), is(true));
     }
 
+    @Test
+    public void requireThatMetricIsIncrementedWhenClientIsMissingCertificateOnHandshake() throws IOException {
+        Path privateKeyFile = tmpFolder.newFile().toPath();
+        Path certificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
+        var metricConsumer = new MetricConsumerMock();
+        TestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer);
+
+        SSLContext clientCtx = new SslContextBuilder()
+                .withTrustStore(certificateFile)
+                .build();
+        assertHttpsRequestTriggersSslHandshakeException(
+                driver, clientCtx, null, null, "Received fatal alert: bad_certificate");
+        verify(metricConsumer.mockitoMock())
+                .add(Metrics.SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT);
+        assertThat(driver.close(), is(true));
+    }
+
+    @Test
+    public void requireThatMetricIsIncrementedWhenClientUsesIncompatibleTlsVersion() throws IOException {
+        Path privateKeyFile = tmpFolder.newFile().toPath();
+        Path certificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
+        var metricConsumer = new MetricConsumerMock();
+        TestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer);
+
+        SSLContext clientCtx = new SslContextBuilder()
+                .withTrustStore(certificateFile)
+                .withKeyStore(privateKeyFile, certificateFile)
+                .build();
+
+        assertHttpsRequestTriggersSslHandshakeException(
+                driver, clientCtx, "TLSv1.1", null, "Received fatal alert: protocol_version");
+        verify(metricConsumer.mockitoMock())
+                .add(Metrics.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_PROTOCOLS, 1L, MetricConsumerMock.STATIC_CONTEXT);
+        assertThat(driver.close(), is(true));
+    }
+
+    @Test
+    public void requireThatMetricIsIncrementedWhenClientUsesIncompatibleCiphers() throws IOException {
+        Path privateKeyFile = tmpFolder.newFile().toPath();
+        Path certificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
+        var metricConsumer = new MetricConsumerMock();
+        TestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer);
+
+        SSLContext clientCtx = new SslContextBuilder()
+                .withTrustStore(certificateFile)
+                .withKeyStore(privateKeyFile, certificateFile)
+                .build();
+
+        assertHttpsRequestTriggersSslHandshakeException(
+                driver, clientCtx, null, "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "Received fatal alert: handshake_failure");
+        verify(metricConsumer.mockitoMock())
+                .add(Metrics.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_CIPHERS, 1L, MetricConsumerMock.STATIC_CONTEXT);
+        assertThat(driver.close(), is(true));
+    }
+
+    @Test
+    public void requireThatMetricIsIncrementedWhenClientUsesInvalidCertificateInHandshake() throws IOException {
+        Path serverPrivateKeyFile = tmpFolder.newFile().toPath();
+        Path serverCertificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(serverPrivateKeyFile, serverCertificateFile);
+        var metricConsumer = new MetricConsumerMock();
+        TestDriver driver = createSslTestDriver(serverCertificateFile, serverPrivateKeyFile, metricConsumer);
+
+        Path clientPrivateKeyFile = tmpFolder.newFile().toPath();
+        Path clientCertificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(clientPrivateKeyFile, clientCertificateFile);
+
+        SSLContext clientCtx = new SslContextBuilder()
+                .withKeyStore(clientPrivateKeyFile, clientCertificateFile)
+                .withTrustStore(serverCertificateFile)
+                .build();
+
+        assertHttpsRequestTriggersSslHandshakeException(
+                driver, clientCtx, null, null, "Received fatal alert: certificate_unknown");
+        verify(metricConsumer.mockitoMock())
+                .add(Metrics.SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT);
+        assertThat(driver.close(), is(true));
+    }
+
+    private static TestDriver createSslTestDriver(
+            Path serverCertificateFile, Path serverPrivateKeyFile, MetricConsumerMock metricConsumer) throws IOException {
+        return TestDrivers.newInstanceWithSsl(
+                new EchoRequestHandler(), serverCertificateFile, serverPrivateKeyFile, TlsClientAuth.NEED, metricConsumer.asGuiceModule());
+    }
+
+    private static void assertHttpsRequestTriggersSslHandshakeException(
+            TestDriver testDriver,
+            SSLContext sslContext,
+            String protocolOverride,
+            String cipherOverride,
+            String expectedExceptionSubstring) throws IOException {
+        List<String> protocols = protocolOverride != null ? List.of(protocolOverride) : null;
+        List<String> ciphers = cipherOverride != null ? List.of(cipherOverride) : null;
+        try (var client = new SimpleHttpClient(sslContext, protocols, ciphers, testDriver.server().getListenPort(), false)) {
+            client.get("/status.html");
+            fail("SSLHandshakeException expected");
+        } catch (SSLHandshakeException e) {
+            assertThat(e.getMessage(), containsString(expectedExceptionSubstring));
+        } catch (SSLException e) {
+            // Jetty may sometime close the connection before the apache client has fully consumed the TLS handshake frame
+            log.log(Level.WARNING, "Client failed to get a proper TLS handshake response: " + e.getMessage(), e);
+            assertThat(e.getMessage(), containsString("readHandshakeRecord")); // Only ignore this specific ssl exception
+        }
+    }
+
     private static void generatePrivateKeyAndCertificate(Path privateKeyFile, Path certificateFile) throws IOException {
         KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048);
         Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate()));
@@ -749,4 +871,18 @@ public class HttpServerTest {
             return lhs.getName().compareTo(rhs.getName());
         }
     }
+
+    private static class MetricConsumerMock {
+        static final Metric.Context STATIC_CONTEXT = new Metric.Context() {};
+
+        private final MetricConsumer mockitoMock = mock(MetricConsumer.class);
+
+        MetricConsumerMock() {
+            when(mockitoMock.createContext(anyMap())).thenReturn(STATIC_CONTEXT);
+        }
+
+        MetricConsumer mockitoMock() { return mockitoMock; }
+        Module asGuiceModule() { return binder -> binder.bind(MetricConsumer.class).toInstance(mockitoMock); }
+    }
+
 }
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java
index b0f570317d6..8035734a76c 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java
@@ -1,12 +1,11 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http.server.jetty;
 
-import com.yahoo.jdisc.http.HttpHeaders;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.entity.GzipCompressingEntity;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpUriRequest;
@@ -19,6 +18,7 @@ import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.entity.mime.FormBodyPart;
 import org.apache.http.entity.mime.MultipartEntityBuilder;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.conn.BasicHttpClientConnectionManager;
 import org.apache.http.util.EntityUtils;
@@ -26,16 +26,11 @@ import org.hamcrest.Matcher;
 import org.hamcrest.MatcherAssert;
 
 import javax.net.ssl.SSLContext;
-import java.io.ByteArrayOutputStream;
-import java.io.EOFException;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStreamWriter;
-import java.net.Socket;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
-import java.util.regex.Pattern;
+import java.util.List;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
@@ -46,14 +41,20 @@ import static org.hamcrest.MatcherAssert.assertThat;
  * A simple http client for testing
  *
  * @author Simon Thoresen Hult
+ * @author bjorncs
  */
-public class SimpleHttpClient {
+public class SimpleHttpClient implements AutoCloseable {
 
-    private final HttpClient delegate;
+    private final CloseableHttpClient delegate;
     private final String scheme;
     private final int listenPort;
 
-    public SimpleHttpClient(final SSLContext sslContext, final int listenPort, final boolean useCompression) {
+    public SimpleHttpClient(SSLContext sslContext, int listenPort, boolean useCompression) {
+        this(sslContext, null, null, listenPort, useCompression);
+    }
+
+    public SimpleHttpClient(SSLContext sslContext, List<String> enabledProtocols, List<String> enabledCiphers,
+                            int listenPort, boolean useCompression) {
         HttpClientBuilder builder = HttpClientBuilder.create();
         if (!useCompression) {
             builder.disableContentCompression();
@@ -61,6 +62,8 @@ public class SimpleHttpClient {
         if (sslContext != null) {
             SSLConnectionSocketFactory sslConnectionFactory = new SSLConnectionSocketFactory(
                     sslContext,
+                    toArray(enabledProtocols),
+                    toArray(enabledCiphers),
                     new DefaultHostnameVerifier());
             builder.setSSLSocketFactory(sslConnectionFactory);
 
@@ -76,6 +79,10 @@ public class SimpleHttpClient {
         this.listenPort = listenPort;
     }
 
+    private static String[] toArray(List<String> list) {
+        return list != null ? list.toArray(new String[0]) : null;
+    }
+
     public URI newUri(final String path) {
         return URI.create(scheme + "://localhost:" + listenPort + path);
     }
@@ -100,40 +107,9 @@ public class SimpleHttpClient {
         return newGet(path).execute();
     }
 
-    public String raw(final String request) throws IOException {
-        final Socket socket = new Socket("localhost", listenPort);
-        final OutputStreamWriter out = new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8);
-        out.write(request);
-        out.flush();
-
-        final ByteArrayOutputStream buf = new ByteArrayOutputStream();
-        final InputStream in = socket.getInputStream();
-        final int[] TERMINATOR = { '\r', '\n', '\r', '\n' };
-        for (int pos = 0; pos < TERMINATOR.length; ++pos) {
-            final int b = in.read();
-            if (b < 0) {
-                throw new EOFException();
-            }
-            if (b != TERMINATOR[pos]) {
-                pos = -1;
-            }
-            buf.write(b);
-        }
-        final String response = buf.toString(StandardCharsets.UTF_8.name());
-        final java.util.regex.Matcher matcher = Pattern.compile(HttpHeaders.Names.CONTENT_LENGTH + ": (.+)\r\n").matcher(response);
-        if (matcher.find()) {
-            final int len = Integer.valueOf(matcher.group(1));
-            for (int i = 0; i < len; ++i) {
-                final int b = in.read();
-                if (b < 0) {
-                    throw new EOFException();
-                }
-                buf.write(b);
-            }
-        }
-
-        socket.close();
-        return buf.toString(StandardCharsets.UTF_8.name());
+    @Override
+    public void close() throws IOException {
+        delegate.close();
     }
 
     public class RequestExecutor {
@@ -177,7 +153,9 @@ public class SimpleHttpClient {
             if (entity != null) {
                 ((HttpPost)request).setEntity(entity);
             }
-            return new ResponseValidator(delegate.execute(request));
+            try (CloseableHttpResponse response = delegate.execute(request)){
+                return new ResponseValidator(response);
+            }
         }
     }
 
@@ -218,9 +196,5 @@ public class SimpleHttpClient {
             return this;
         }
 
-        public ResponseValidator expectTrailer(final String trailerName, final Matcher<String> matcher) {
-            // TODO: check trailer, not header
-            return expectHeader(trailerName, matcher);
-        }
     }
 }
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java
index e0933ac485e..4908da2ba75 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java
@@ -20,6 +20,7 @@ import java.nio.file.Path;
 
 /**
  * @author Simon Thoresen Hult
+ * @author bjorncs
  */
 public class TestDrivers {
 
@@ -45,9 +46,12 @@ public class TestDrivers {
                 ));
     }
 
+    public enum TlsClientAuth { NEED, WANT }
+
     public static TestDriver newInstanceWithSsl(final RequestHandler requestHandler,
                                                 Path certificateFile,
                                                 Path privateKeyFile,
+                                                TlsClientAuth tlsClientAuth,
                                                 final Module... guiceModules) throws IOException {
         return TestDriver.newInstance(
                 JettyHttpServer.class,
@@ -61,7 +65,9 @@ public class TestDrivers {
                                                 .pathWhitelist("/status.html"))
                                 .ssl(new ConnectorConfig.Ssl.Builder()
                                              .enabled(true)
-                                             .clientAuth(ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH)
+                                             .clientAuth(tlsClientAuth == TlsClientAuth.NEED
+                                                                 ? ConnectorConfig.Ssl.ClientAuth.Enum.NEED_AUTH
+                                                                 : ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH)
                                              .privateKeyFile(privateKeyFile.toString())
                                              .certificateFile(certificateFile.toString())
                                              .caCertificateFile(certificateFile.toString())),