Report SSL handshake failures in metric

1 files changed, 126 insertions, 0 deletions

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); }
+    }
+
 }