diff options
4 files changed, 12 insertions, 203 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 { diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java index d03a08a27db..79947b5a347 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneContainerActivator.java @@ -19,24 +19,15 @@ import org.osgi.framework.Bundle; import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; -import java.io.FileInputStream; import java.io.IOException; import java.net.InetSocketAddress; -import java.nio.channels.FileChannel; import java.nio.channels.ServerSocketChannel; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Hashtable; import java.util.List; -import java.util.Map; -import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.Stream; - -import static java.util.stream.Collectors.toMap; /** * @author Einar M R Rosenvinge @@ -45,62 +36,12 @@ public class StandaloneContainerActivator implements BundleActivator { @Override public void start(BundleContext bundleContext) throws Exception { - Container container = getContainer(); - List<ConnectorConfig> connectorConfigs = getConnectorConfigs(container); - - Stream<Path> keyStorePaths = getKeyStorePaths(connectorConfigs); - Map<Path, FileChannel> fileChannels = openFiles(keyStorePaths); - registerKeyStoreFileChannels(bundleContext, fileChannels); - - for (ConnectorConfig config: connectorConfigs) { + for (ConnectorConfig config: getConnectorConfigs(getContainer())) { ServerSocketChannel socketChannel = bindChannel(config); registerChannels(bundleContext, config.listenPort(), socketChannel); } } - private void registerKeyStoreFileChannels(BundleContext bundleContext, Map<Path, FileChannel> fileChannels) { - Hashtable<String, Object> properties = new Hashtable<>(); - properties.put("role", "com.yahoo.container.standalone.StandaloneContainerActivator.KeyStoreFileChannels"); - //Since Standalone container and jdisc http service don't have a suitable common module for placing a wrapper class for fileChannels, - //we register it with the type map. In the future, we should wrap this. - bundleContext.registerService(Map.class, - Collections.unmodifiableMap(fileChannels), - properties); - } - - private Map<Path, FileChannel> openFiles(Stream<Path> keyStorePaths) { - return keyStorePaths.collect(toMap( - Function.<Path>identity(), - StandaloneContainerActivator::getFileChannel)); - } - - private static FileChannel getFileChannel(Path path) { - try { - FileInputStream inputStream = new FileInputStream(path.toFile()); - //don't close the inputStream, as that will close the underlying channel. - return inputStream.getChannel(); - } catch (IOException e) { - throw new RuntimeException("Failed opening path " + path, e); - } - } - - private Stream<Path> getKeyStorePaths(List<ConnectorConfig> connectorConfigs) { - return connectorConfigs.stream(). - map(ConnectorConfig::ssl). - flatMap(StandaloneContainerActivator::getKeyStorePaths); - } - - private static Stream<Path> getKeyStorePaths(ConnectorConfig.Ssl ssl) { - Stream<String> paths = Stream.of( - ssl.keyStorePath(), - ssl.pemKeyStore().certificatePath(), - ssl.pemKeyStore().keyPath()); - - return paths. - filter(path -> !path.isEmpty()). - map(Paths::get); - } - void registerChannels(BundleContext bundleContext, int listenPort, ServerSocketChannel boundChannel) { Hashtable<String, Integer> properties = new Hashtable<>(); properties.put("port", listenPort); |