Report SSL handshake failures in metric

4 files changed, 224 insertions, 7 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..1c4421859f1 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 0e7bdd409e1..3b926225b19 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,7 @@ import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLHandshakeException;
 import javax.security.auth.x500.X500Principal;
 import java.io.IOException;
 import java.math.BigInteger;
@@ -81,7 +86,10 @@ 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;
 
 /**
@@ -560,6 +568,110 @@ 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));
+        }
+    }
+
     private static void generatePrivateKeyAndCertificate(Path privateKeyFile, Path certificateFile) throws IOException {
         KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048);
         Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate()));
@@ -750,4 +862,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); }
+    }
+
 }