Add Jetty handler that enforces TLS client authentication at http layer

6 files changed, 177 insertions, 13 deletions

diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json
index f915dc1e8c1..6f41c4ced06 100644
--- a/jdisc_http_service/abi-spec.json
+++ b/jdisc_http_service/abi-spec.json
@@ -39,6 +39,7 @@
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder tcpNoDelay(boolean)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder throttling(com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder ssl(com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$Builder tlsClientAuthEnforcer(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder)",
       "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)",
       "public final java.lang.String getDefMd5()",
       "public final java.lang.String getDefName()",
@@ -47,7 +48,8 @@
     ],
     "fields": [
       "public com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder throttling",
-      "public com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder ssl"
+      "public com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder ssl",
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder tlsClientAuthEnforcer"
     ]
   },
   "com.yahoo.jdisc.http.ConnectorConfig$Producer": {
@@ -178,6 +180,41 @@
     ],
     "fields": []
   },
+  "com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder": {
+    "superClass": "java.lang.Object",
+    "interfaces": [
+      "com.yahoo.config.ConfigBuilder"
+    ],
+    "attributes": [
+      "public"
+    ],
+    "methods": [
+      "public void <init>()",
+      "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder enable(boolean)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder pathWhitelist(java.lang.String)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder pathWhitelist(java.util.Collection)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer build()"
+    ],
+    "fields": [
+      "public java.util.List pathWhitelist"
+    ]
+  },
+  "com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer": {
+    "superClass": "com.yahoo.config.InnerNode",
+    "interfaces": [],
+    "attributes": [
+      "public",
+      "final"
+    ],
+    "methods": [
+      "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder)",
+      "public boolean enable()",
+      "public java.util.List pathWhitelist()",
+      "public java.lang.String pathWhitelist(int)"
+    ],
+    "fields": []
+  },
   "com.yahoo.jdisc.http.ConnectorConfig": {
     "superClass": "com.yahoo.config.ConfigInstance",
     "interfaces": [],
@@ -204,7 +241,8 @@
       "public boolean tcpKeepAliveEnabled()",
       "public boolean tcpNoDelay()",
       "public com.yahoo.jdisc.http.ConnectorConfig$Throttling throttling()",
-      "public com.yahoo.jdisc.http.ConnectorConfig$Ssl ssl()"
+      "public com.yahoo.jdisc.http.ConnectorConfig$Ssl ssl()",
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer tlsClientAuthEnforcer()"
     ],
     "fields": [
       "public static final java.lang.String CONFIG_DEF_MD5",
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 30a1b1d885c..2b9cb426dda 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
@@ -9,6 +9,7 @@ import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.application.OsgiFramework;
+import com.yahoo.jdisc.http.ConnectorConfig;
 import com.yahoo.jdisc.http.ServerConfig;
 import com.yahoo.jdisc.http.ServletPathsConfig;
 import com.yahoo.jdisc.http.server.FilterBindings;
@@ -145,10 +146,13 @@ public class JettyHttpServer extends AbstractServerProvider {
         setupJmx(server, serverConfig);
         ((QueuedThreadPool)server.getThreadPool()).setMaxThreads(serverConfig.maxWorkerThreads());
 
+        List<ConnectorConfig> connectorConfigs = new ArrayList<>();
         for (ConnectorFactory connectorFactory : connectorFactories.allComponents()) {
-            ServerSocketChannel preBoundChannel = getChannelFromServiceLayer(connectorFactory.getConnectorConfig().listenPort(), osgiFramework.bundleContext());
+            ConnectorConfig connectorConfig = connectorFactory.getConnectorConfig();
+            connectorConfigs.add(connectorConfig);
+            ServerSocketChannel preBoundChannel = getChannelFromServiceLayer(connectorConfig.listenPort(), osgiFramework.bundleContext());
             server.addConnector(connectorFactory.createConnector(metric, server, preBoundChannel));
-            listenedPorts.add(connectorFactory.getConnectorConfig().listenPort());
+            listenedPorts.add(connectorConfig.listenPort());
         }
 
         janitor = newJanitor(threadFactory);
@@ -168,6 +172,7 @@ public class JettyHttpServer extends AbstractServerProvider {
                 getHandlerCollection(
                         serverConfig,
                         servletPathsConfig,
+                        connectorConfigs,
                         jdiscServlet,
                         servletHolders,
                         jDiscFilterInvokerFilter));
@@ -217,6 +222,7 @@ public class JettyHttpServer extends AbstractServerProvider {
     private HandlerCollection getHandlerCollection(
             ServerConfig serverConfig,
             ServletPathsConfig servletPathsConfig,
+            List<ConnectorConfig> connectorConfigs,
             ServletHolder jdiscServlet,
             ComponentRegistry<ServletHolder> servletHolders,
             FilterHolder jDiscFilterInvokerFilter) {
@@ -231,8 +237,11 @@ public class JettyHttpServer extends AbstractServerProvider {
 
         servletContextHandler.addServlet(jdiscServlet, "/*");
 
+        var authEnforcer = new TlsClientAuthenticationEnforcer(connectorConfigs);
+        authEnforcer.setHandler(servletContextHandler);
+
         GzipHandler gzipHandler = newGzipHandler(serverConfig);
-        gzipHandler.setHandler(servletContextHandler);
+        gzipHandler.setHandler(authEnforcer);
 
         HttpResponseStatisticsCollector statisticsCollector = new HttpResponseStatisticsCollector();
         statisticsCollector.setHandler(gzipHandler);
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java
new file mode 100644
index 00000000000..546741b3322
--- /dev/null
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java
@@ -0,0 +1,78 @@
+// Copyright 2019 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.Response;
+import com.yahoo.jdisc.http.ConnectorConfig;
+import com.yahoo.jdisc.http.servlet.ServletRequest;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.handler.HandlerWrapper;
+
+import javax.servlet.DispatcherType;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A Jetty handler that enforces TLS client authentication with configurable white list.
+ *
+ * @author bjorncs
+ */
+class TlsClientAuthenticationEnforcer extends HandlerWrapper {
+
+    private final Map<Integer, List<String>> portToWhitelistedPathsMapping;
+
+    TlsClientAuthenticationEnforcer(List<ConnectorConfig> connectorConfigs) {
+        portToWhitelistedPathsMapping = createWhitelistMapping(connectorConfigs);
+    }
+
+    @Override
+    public void handle(String target, Request request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws IOException, ServletException {
+        if (isHttpsRequest(request)
+                && !isRequestToWhitelistedBinding(servletRequest)
+                && !isClientAuthenticated(servletRequest)) {
+            servletResponse.sendError(Response.Status.UNAUTHORIZED, "Client did not present a x509 certificate.");
+        } else {
+            _handler.handle(target, request, servletRequest, servletResponse);
+        }
+    }
+
+    private static Map<Integer, List<String>> createWhitelistMapping(List<ConnectorConfig> connectorConfigs) {
+        var mapping = new HashMap<Integer, List<String>>();
+        for (ConnectorConfig connectorConfig : connectorConfigs) {
+            var enforcerConfig = connectorConfig.tlsClientAuthEnforcer();
+            if (enforcerConfig.enable()) {
+                mapping.put(connectorConfig.listenPort(), enforcerConfig.pathWhitelist());
+            }
+        }
+        return mapping;
+    }
+
+    private boolean isHttpsRequest(Request request) {
+        return request.getDispatcherType() == DispatcherType.REQUEST && request.getScheme().equalsIgnoreCase("https");
+    }
+
+    private boolean isRequestToWhitelistedBinding(HttpServletRequest servletRequest) {
+        int localPort = servletRequest.getLocalPort();
+        List<String> whiteListedPaths = getWhitelistedPathsForPort(localPort);
+        if (whiteListedPaths == null) {
+            return true; // enforcer not enabled
+        }
+        // Note: Same path definition as HttpRequestFactory.getUri()
+        return whiteListedPaths.contains(servletRequest.getRequestURI());
+    }
+
+    private List<String> getWhitelistedPathsForPort(int localPort) {
+        if (portToWhitelistedPathsMapping.containsKey(0) && portToWhitelistedPathsMapping.size() == 1) {
+            return portToWhitelistedPathsMapping.get(0); // for unit tests which uses 0 for listen port
+        }
+        return portToWhitelistedPathsMapping.get(localPort);
+    }
+
+    private boolean isClientAuthenticated(HttpServletRequest servletRequest) {
+        return servletRequest.getAttribute(ServletRequest.SERVLET_REQUEST_X509CERT) != null;
+    }
+}
diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def
index c6c6fad345b..9ffcc9c41b5 100644
--- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def
+++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def
@@ -73,3 +73,11 @@ ssl.caCertificateFile               string default=""
 
 # Client authentication mode. See SSLEngine.getNeedClientAuth()/getWantClientAuth() for details.
 ssl.clientAuth                      enum { DISABLED, WANT_AUTH, NEED_AUTH }  default=DISABLED
+
+# Enforce TLS client authentication for https requests at the http layer.
+# Intended to be used with connectors with optional client authentication enabled.
+# 401 status code is returned for requests from non-authenticated clients.
+tlsClientAuthEnforcer.enable          bool   default=false
+
+# Paths where client authentication should not be enforced. To be used in combination with NEED_AUTH. Typically used for health checks.
+tlsClientAuthEnforcer.pathWhitelist[]  string
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 ec9c90ffa50..3c04a4f0d5f 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
@@ -23,6 +23,7 @@ import com.yahoo.jdisc.http.HttpResponse;
 import com.yahoo.jdisc.http.ServerConfig;
 import com.yahoo.jdisc.service.BindingSetNotFoundException;
 import com.yahoo.security.KeyUtils;
+import com.yahoo.security.SslContextBuilder;
 import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.security.X509CertificateUtils;
 import org.apache.http.entity.ContentType;
@@ -32,7 +33,9 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import javax.net.ssl.SSLContext;
 import javax.security.auth.x500.X500Principal;
+import java.io.IOException;
 import java.math.BigInteger;
 import java.net.BindException;
 import java.net.URI;
@@ -58,6 +61,7 @@ import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
 import static com.yahoo.jdisc.Response.Status.NOT_FOUND;
 import static com.yahoo.jdisc.Response.Status.OK;
 import static com.yahoo.jdisc.Response.Status.REQUEST_URI_TOO_LONG;
+import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED;
 import static com.yahoo.jdisc.Response.Status.UNSUPPORTED_MEDIA_TYPE;
 import static com.yahoo.jdisc.http.HttpHeaders.Names.CONNECTION;
 import static com.yahoo.jdisc.http.HttpHeaders.Names.CONTENT_TYPE;
@@ -470,16 +474,9 @@ public class HttpServerTest {
 
     @Test
     public void requireThatServerCanRespondToSslRequest() throws Exception {
-        KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048);
         Path privateKeyFile = tmpFolder.newFile().toPath();
-        Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate()));
-
-        X509Certificate certificate = X509CertificateBuilder
-                .fromKeypair(
-                        keyPair, new X500Principal("CN=localhost"), Instant.EPOCH, Instant.EPOCH.plus(100_000, ChronoUnit.DAYS), SHA256_WITH_RSA, BigInteger.ONE)
-                .build();
         Path certificateFile = tmpFolder.newFile().toPath();
-        Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate));
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
 
         final TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile);
         driver.client().get("/status.html")
@@ -488,6 +485,24 @@ public class HttpServerTest {
     }
 
     @Test
+    public void requireThatTlsClientAuthenticationEnforcerRejectsRequestsForNonWhitelistedPaths() throws IOException {
+        Path privateKeyFile = tmpFolder.newFile().toPath();
+        Path certificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
+        TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile);
+
+        SSLContext trustStoreOnlyCtx = new SslContextBuilder()
+                .withTrustStore(certificateFile)
+                .build();
+
+        new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false)
+                .get("/dummy.html")
+                .expectStatusCode(is(UNAUTHORIZED));
+
+        assertThat(driver.close(), is(true));
+    }
+
+    @Test
     public void requireThatConnectedAtReturnsNonZero() throws Exception {
         final TestDriver driver = TestDrivers.newInstance(new ConnectedAtRequestHandler());
         driver.client().get("/status.html")
@@ -526,6 +541,17 @@ public class HttpServerTest {
         assertThat(driver.close(), is(true));
     }
 
+    private static void generatePrivateKeyAndCertificate(Path privateKeyFile, Path certificateFile) throws IOException {
+        KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048);
+        Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate()));
+
+        X509Certificate certificate = X509CertificateBuilder
+                .fromKeypair(
+                        keyPair, new X500Principal("CN=localhost"), Instant.EPOCH, Instant.EPOCH.plus(100_000, ChronoUnit.DAYS), SHA256_WITH_RSA, BigInteger.ONE)
+                .build();
+        Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate));
+    }
+
     private static RequestHandler mockRequestHandler() {
         final RequestHandler mockRequestHandler = mock(RequestHandler.class);
         when(mockRequestHandler.refer()).thenReturn(References.NOOP_REFERENCE);
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 10fe0f1328f..e0933ac485e 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
@@ -55,8 +55,13 @@ public class TestDrivers {
                 newConfigModule(
                         new ServerConfig.Builder(),
                         new ConnectorConfig.Builder()
+                                .tlsClientAuthEnforcer(
+                                        new ConnectorConfig.TlsClientAuthEnforcer.Builder()
+                                                .enable(true)
+                                                .pathWhitelist("/status.html"))
                                 .ssl(new ConnectorConfig.Ssl.Builder()
                                              .enabled(true)
+                                             .clientAuth(ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH)
                                              .privateKeyFile(privateKeyFile.toString())
                                              .certificateFile(certificateFile.toString())
                                              .caCertificateFile(certificateFile.toString())),