Remove old Manhattan keystore hack

3 files changed, 11 insertions, 143 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 96180f48229..097d0f6970e 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
@@ -28,10 +28,7 @@ import java.io.Reader;
 import java.lang.reflect.Field;
 import java.net.Socket;
 import java.net.SocketException;
-import java.nio.channels.Channels;
-import java.nio.channels.FileChannel;
 import java.nio.channels.ServerSocketChannel;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -43,10 +40,8 @@ import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static com.google.common.io.Closeables.closeQuietly;
 import static com.yahoo.jdisc.http.ConnectorConfig.Ssl.KeyStoreType.Enum.JKS;
 import static com.yahoo.jdisc.http.ConnectorConfig.Ssl.KeyStoreType.Enum.PEM;
-import static com.yahoo.jdisc.http.server.jetty.Exceptions.throwUnchecked;
 
 /**
  * @author Einar M R Rosenvinge
@@ -84,11 +79,11 @@ public class ConnectorFactory {
         return connectorConfig;
     }
 
-    public ServerConnector createConnector(final Metric metric, final Server server, final ServerSocketChannel ch, Map<Path, FileChannel> keyStoreChannels) {
+    public ServerConnector createConnector(final Metric metric, final Server server, final ServerSocketChannel ch) {
         ServerConnector connector;
         if (connectorConfig.ssl().enabled()) {
             connector = new JDiscServerConnector(connectorConfig, metric, server, ch,
-                                                 newSslConnectionFactory(keyStoreChannels),
+                                                 newSslConnectionFactory(),
                                                  newHttpConnectionFactory());
         } else {
             connector = new JDiscServerConnector(connectorConfig, metric, server, ch,
@@ -125,7 +120,7 @@ public class ConnectorFactory {
     }
 
     //TODO: does not support loading non-yahoo readable JKS key stores.
-    private SslConnectionFactory newSslConnectionFactory(Map<Path, FileChannel> keyStoreChannels) {
+    private SslConnectionFactory newSslConnectionFactory() {
         Ssl sslConfig = connectorConfig.ssl();
 
         SslContextFactory factory = new SslContextFactory();
@@ -175,7 +170,7 @@ public class ConnectorFactory {
         Optional<String> keyDbPassword = secret(sslConfig.keyDbKey());
         switch (sslConfig.keyStoreType()) {
             case PEM:
-                factory.setKeyStore(getKeyStore(sslConfig.pemKeyStore(), keyStoreChannels));
+                factory.setKeyStore(getKeyStore(sslConfig.pemKeyStore()));
                 if (keyDbPassword.isPresent())
                     log.warning("Encrypted PEM key stores are not supported.");
                 break;
@@ -208,29 +203,16 @@ public class ConnectorFactory {
         return () -> new RuntimeException(String.format("Password is required for JKS %s store", type));
     }
 
-    private KeyStore getKeyStore(PemKeyStore pemKeyStore, Map<Path, FileChannel> keyStoreChannels) {
+    private static KeyStore getKeyStore(PemKeyStore pemKeyStore) {
         Preconditions.checkArgument(!pemKeyStore.certificatePath().isEmpty(), "Missing certificate path.");
         Preconditions.checkArgument(!pemKeyStore.keyPath().isEmpty(), "Missing key path.");
 
         class KeyStoreReaderForPath implements AutoCloseable {
-            private final Optional<FileChannel> channel;
             public final ReaderForPath readerForPath;
 
-
             KeyStoreReaderForPath(String pathString) {
                 Path path = Paths.get(pathString);
-                channel = Optional.ofNullable(keyStoreChannels.get(path));
-                readerForPath = new ReaderForPath(channel.map(this::getReader).orElseGet(() -> getReader(path)), path);
-            }
-
-            private Reader getReader(FileChannel channel) {
-                try {
-                    channel.position(0);
-                    return Channels.newReader(channel, StandardCharsets.UTF_8.newDecoder(), -1);
-                } catch (IOException e) {
-                    throw throwUnchecked(e);
-                }
-
+                readerForPath = new ReaderForPath(getReader(path), path);
             }
 
             private Reader getReader(Path path) {
@@ -242,12 +224,7 @@ public class ConnectorFactory {
             }
 
             @Override
-            public void close()  {
-                //channels are reused
-                if (!channel.isPresent()) {
-                    closeQuietly(readerForPath.reader);
-                }
-            }
+            public void close() {}
         }
 
         try (KeyStoreReaderForPath certificateReader = new KeyStoreReaderForPath(pemKeyStore.certificatePath());
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 7feca14ef29..aaa213095c6 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
@@ -44,15 +44,11 @@ import javax.servlet.DispatcherType;
 import java.lang.management.ManagementFactory;
 import java.net.BindException;
 import java.net.MalformedURLException;
-import java.nio.channels.FileChannel;
 import java.nio.channels.ServerSocketChannel;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
@@ -63,7 +59,6 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 import static com.yahoo.jdisc.http.server.jetty.ConnectorFactory.JDiscServerConnector;
-import static com.yahoo.jdisc.http.server.jetty.Exceptions.throwUnchecked;
 
 /**
  * @author Simon Thoresen Hult
@@ -147,11 +142,9 @@ public class JettyHttpServer extends AbstractServerProvider {
         setupJmx(server, serverConfig);
         ((QueuedThreadPool)server.getThreadPool()).setMaxThreads(serverConfig.maxWorkerThreads());
 
-        Map<Path, FileChannel> keyStoreChannels = getKeyStoreFileChannels(osgiFramework.bundleContext());
-
         for (ConnectorFactory connectorFactory : connectorFactories.allComponents()) {
             ServerSocketChannel preBoundChannel = getChannelFromServiceLayer(connectorFactory.getConnectorConfig().listenPort(), osgiFramework.bundleContext());
-            server.addConnector(connectorFactory.createConnector(metric, server, preBoundChannel, keyStoreChannels));
+            server.addConnector(connectorFactory.createConnector(metric, server, preBoundChannel));
             listenedPorts.add(connectorFactory.getConnectorConfig().listenPort());
         }
 
@@ -257,43 +250,6 @@ public class JettyHttpServer extends AbstractServerProvider {
         return "/" + servletPathsConfig.servlets(id.stringValue()).path();
     }
 
-    // Ugly trick to get generic type literal.
-    @SuppressWarnings("unchecked")
-    private static final Class<Map<?, ?>> mapClass = (Class<Map<?, ?>>) (Object) Map.class;
-
-    private Map<Path, FileChannel> getKeyStoreFileChannels(BundleContext bundleContext) {
-        try {
-            Collection<ServiceReference<Map<?, ?>>> serviceReferences = bundleContext.getServiceReferences(mapClass,
-                    "(role=com.yahoo.container.standalone.StandaloneContainerActivator.KeyStoreFileChannels)");
-
-            if (serviceReferences == null || serviceReferences.isEmpty())
-                return Collections.emptyMap();
-
-            if (serviceReferences.size() != 1)
-                throw new IllegalStateException("Multiple KeyStoreFileChannels registered");
-
-            return getKeyStoreFileChannels(bundleContext, serviceReferences.iterator().next());
-        } catch (InvalidSyntaxException e) {
-            throw throwUnchecked(e);
-        }
-    }
-
-    @SuppressWarnings("unchecked")
-    private Map<Path, FileChannel> getKeyStoreFileChannels(BundleContext bundleContext, ServiceReference<Map<?, ?>> keyStoreFileChannelReference) {
-        Map<?, ?> fileChannelMap = bundleContext.getService(keyStoreFileChannelReference);
-        try {
-            if (fileChannelMap == null)
-                return Collections.emptyMap();
-
-            Map<Path, FileChannel> result = (Map<Path, FileChannel>) fileChannelMap;
-            log.fine("Using file channel for " + result.keySet());
-            return result;
-        } finally {
-            //if we change this to be anything other than a simple map, we should hold the reference as long as the object is in use.
-            bundleContext.ungetService(keyStoreFileChannelReference);
-        }
-    }
-
     private ServletContextHandler createServletContextHandler() {
         ServletContextHandler servletContextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
         servletContextHandler.setContextPath("/");
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java
index 25457c0c6c6..dfa30a3b7db 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.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.google.common.collect.ImmutableMap;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.http.ConnectorConfig;
 import com.yahoo.jdisc.http.SecretStore;
-import com.yahoo.jdisc.http.ssl.ReaderForPath;
 import com.yahoo.jdisc.http.SslContextFactory;
+import com.yahoo.jdisc.http.ssl.ReaderForPath;
 import com.yahoo.jdisc.http.ssl.SslKeyStore;
 import com.yahoo.jdisc.http.ssl.pem.PemKeyStore;
 import com.yahoo.jdisc.http.ssl.pem.PemSslKeyStore;
@@ -15,19 +14,12 @@ import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.testng.annotations.Test;
 
-import javax.net.ssl.SSLContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.net.InetSocketAddress;
-import java.nio.channels.FileChannel;
 import java.nio.channels.ServerSocketChannel;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.nio.file.StandardOpenOption;
-import java.util.Collections;
 import java.util.Map;
 
 import static com.yahoo.jdisc.http.ConnectorConfig.Ssl;
@@ -73,7 +65,7 @@ public class ConnectorFactoryTest {
             ConnectorFactory factory = new ConnectorFactory(new ConnectorConfig(new ConnectorConfig.Builder()),
                                                             new ThrowingSecretStore());
             ConnectorFactory.JDiscServerConnector connector =
-                    (ConnectorFactory.JDiscServerConnector)factory.createConnector(new DummyMetric(), server, null, Collections.emptyMap());
+                    (ConnectorFactory.JDiscServerConnector)factory.createConnector(new DummyMetric(), server, null);
             server.addConnector(connector);
             server.setHandler(new HelloWorldHandler());
             server.start();
@@ -99,7 +91,7 @@ public class ConnectorFactoryTest {
             serverChannel.socket().bind(new InetSocketAddress(0));
 
             ConnectorFactory factory = new ConnectorFactory(new ConnectorConfig(new ConnectorConfig.Builder()), new ThrowingSecretStore());
-            ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel, Collections.emptyMap());
+            ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel);
             server.addConnector(connector);
             server.setHandler(new HelloWorldHandler());
             server.start();
@@ -117,63 +109,6 @@ public class ConnectorFactoryTest {
         }
     }
 
-    @Test
-    public void pre_bound_keystore_file_channels_are_used() throws Exception {
-        Path pemKeyStoreDirectory = Paths.get("src/test/resources/pem/");
-
-        Path certificateFile = pemKeyStoreDirectory.resolve("test.crt");
-        Path privateKeyFile = pemKeyStoreDirectory.resolve("test.key");
-
-        Server server = new Server();
-        try {
-            ServerSocketChannel serverChannel = ServerSocketChannel.open();
-            serverChannel.socket().bind(new InetSocketAddress(0));
-
-            String fakeCertificatePath = "ensure-certificate-path-is-not-used-to-open-the-file";
-            String fakeKeyPath = "ensure-key-path-is-not-used-to-open-the-file";
-
-            ConnectorConfig.Builder builder = new ConnectorConfig.Builder();
-            builder.ssl(
-                    new Ssl.Builder().
-                            enabled(true).
-                            keyStoreType(PEM).
-                            pemKeyStore(new Ssl.PemKeyStore.Builder().
-                                    certificatePath(fakeCertificatePath).
-                                    keyPath(fakeKeyPath)));
-
-            FileChannel certificateChannel = FileChannel.open(certificateFile, StandardOpenOption.READ);
-            FileChannel privateKeyChannel = FileChannel.open(privateKeyFile, StandardOpenOption.READ);
-
-            Map<Path, FileChannel> keyStoreChannels = ImmutableMap.<Path, FileChannel>builder().
-                    put(Paths.get(fakeCertificatePath), certificateChannel).
-                    put(Paths.get(fakeKeyPath), privateKeyChannel).
-                    build();
-
-
-            ConnectorFactory factory = new ConnectorFactory(new ConnectorConfig(builder), new ThrowingSecretStore());
-            ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel, keyStoreChannels);
-            server.addConnector(connector);
-            server.setHandler(new HelloWorldHandler());
-            server.start();
-
-            SslKeyStore trustStore = new PemSslKeyStore(
-                    new PemKeyStore.TrustStoreLoadParameter(
-                            new ReaderForPath(Files.newBufferedReader(certificateFile), certificateFile)));
-
-            SSLContext clientSslContext = SslContextFactory.newInstanceFromTrustStore(trustStore).getServerSSLContext();
-            SimpleHttpClient client = new SimpleHttpClient(clientSslContext, connector.getLocalPort(), false);
-            SimpleHttpClient.RequestExecutor ex = client.newGet("/ignored");
-            SimpleHttpClient.ResponseValidator val = ex.execute();
-            val.expectContent(equalTo("Hello world"));
-        } finally {
-            try {
-                server.stop();
-            } catch (Exception e) {
-                //ignore
-            }
-        }
-    }
-
     private static class HelloWorldHandler extends AbstractHandler {
         @Override
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {