diff options
author | Martin Polden <mpolden@mpolden.no> | 2017-11-10 18:50:24 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-11-10 18:50:24 +0100 |
commit | c56091ffa4da34a2913b73f860cff2b6fa746c43 (patch) | |
tree | 1d7b53c77e927688fb041157de68797187620163 | |
parent | ef22b222d1862f6b5a56521f43830abae30eec70 (diff) |
Revert "Bjorncs/jdisc http service cleanup"
15 files changed, 366 insertions, 157 deletions
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java b/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java index e7039e85e5e..d2b58090665 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java @@ -24,6 +24,7 @@ import java.io.StringWriter; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.net.URI; import java.nio.ByteBuffer; import java.util.HashSet; import java.util.Set; @@ -34,18 +35,14 @@ import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.logging.Logger; import java.util.stream.Stream; /** - * @author Simon Thoresen Hult + * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a> */ @SuppressWarnings("UnusedDeclaration") @Beta public abstract class ServerProviderConformanceTest { - - private static final Logger log = Logger.getLogger(ServerProviderConformanceTest.class.getName()); - private static final int NUM_RUNS_EACH_TEST = 10; /** @@ -2793,7 +2790,7 @@ public abstract class ServerProviderConformanceTest { serverProvider.release(); for (int i = 0; i < NUM_RUNS_EACH_TEST; ++i) { - log.fine("Test run #" + i); + System.out.println("Test run #" + i); requestHandler.reset(adapter.newResponseContent()); final U client = adapter.newClient(serverProvider); final boolean withRequestContent = requestType == RequestType.WITH_CONTENT; 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 54338c64c1e..96180f48229 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 @@ -8,6 +8,8 @@ import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ConnectorConfig.Ssl; import com.yahoo.jdisc.http.ConnectorConfig.Ssl.PemKeyStore; import com.yahoo.jdisc.http.SecretStore; +import com.yahoo.jdisc.http.ssl.ReaderForPath; +import com.yahoo.jdisc.http.ssl.SslKeyStore; import com.yahoo.jdisc.http.ssl.pem.PemSslKeyStore; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.server.ConnectionFactory; @@ -22,11 +24,15 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import javax.servlet.ServletRequest; import java.io.IOException; -import java.io.UncheckedIOException; +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; import java.security.KeyStore; @@ -37,8 +43,10 @@ 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 @@ -76,11 +84,11 @@ public class ConnectorFactory { return connectorConfig; } - public ServerConnector createConnector(final Metric metric, final Server server, final ServerSocketChannel ch) { + public ServerConnector createConnector(final Metric metric, final Server server, final ServerSocketChannel ch, Map<Path, FileChannel> keyStoreChannels) { ServerConnector connector; if (connectorConfig.ssl().enabled()) { connector = new JDiscServerConnector(connectorConfig, metric, server, ch, - newSslConnectionFactory(), + newSslConnectionFactory(keyStoreChannels), newHttpConnectionFactory()); } else { connector = new JDiscServerConnector(connectorConfig, metric, server, ch, @@ -117,7 +125,7 @@ public class ConnectorFactory { } //TODO: does not support loading non-yahoo readable JKS key stores. - private SslConnectionFactory newSslConnectionFactory() { + private SslConnectionFactory newSslConnectionFactory(Map<Path, FileChannel> keyStoreChannels) { Ssl sslConfig = connectorConfig.ssl(); SslContextFactory factory = new SslContextFactory(); @@ -167,7 +175,7 @@ public class ConnectorFactory { Optional<String> keyDbPassword = secret(sslConfig.keyDbKey()); switch (sslConfig.keyStoreType()) { case PEM: - factory.setKeyStore(getKeyStore(sslConfig.pemKeyStore())); + factory.setKeyStore(getKeyStore(sslConfig.pemKeyStore(), keyStoreChannels)); if (keyDbPassword.isPresent()) log.warning("Encrypted PEM key stores are not supported."); break; @@ -200,16 +208,54 @@ public class ConnectorFactory { return () -> new RuntimeException(String.format("Password is required for JKS %s store", type)); } - private static KeyStore getKeyStore(PemKeyStore pemKeyStore) { + private KeyStore getKeyStore(PemKeyStore pemKeyStore, Map<Path, FileChannel> keyStoreChannels) { Preconditions.checkArgument(!pemKeyStore.certificatePath().isEmpty(), "Missing certificate path."); Preconditions.checkArgument(!pemKeyStore.keyPath().isEmpty(), "Missing key path."); - try { - Path certificatePath = Paths.get(pemKeyStore.certificatePath()); - Path keyPath = Paths.get(pemKeyStore.keyPath()); - return new PemSslKeyStore(certificatePath, keyPath) - .loadJavaKeyStore(); - } catch (IOException e) { - throw new UncheckedIOException(e); + + 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); + } + + } + + private Reader getReader(Path path) { + try { + return Files.newBufferedReader(path); + } catch (IOException e) { + throw new RuntimeException("Failed opening " + path, e); + } + } + + @Override + public void close() { + //channels are reused + if (!channel.isPresent()) { + closeQuietly(readerForPath.reader); + } + } + } + + try (KeyStoreReaderForPath certificateReader = new KeyStoreReaderForPath(pemKeyStore.certificatePath()); + KeyStoreReaderForPath keyReader = new KeyStoreReaderForPath(pemKeyStore.keyPath())) { + SslKeyStore keyStore = new PemSslKeyStore( + new com.yahoo.jdisc.http.ssl.pem.PemKeyStore.KeyStoreLoadParameter( + certificateReader.readerForPath, keyReader.readerForPath)); + return keyStore.loadJavaKeyStore(); } catch (Exception e) { throw new RuntimeException("Failed setting up key store for " + pemKeyStore.keyPath() + ", " + pemKeyStore.certificatePath(), e); } 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 aaa213095c6..7feca14ef29 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,11 +44,15 @@ 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; @@ -59,6 +63,7 @@ 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 @@ -142,9 +147,11 @@ 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)); + server.addConnector(connectorFactory.createConnector(metric, server, preBoundChannel, keyStoreChannels)); listenedPorts.add(connectorFactory.getConnectorConfig().listenPort()); } @@ -250,6 +257,43 @@ 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/main/java/com/yahoo/jdisc/http/ssl/ReaderForPath.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/ReaderForPath.java new file mode 100644 index 00000000000..b04d91d7403 --- /dev/null +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/ReaderForPath.java @@ -0,0 +1,22 @@ +// 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.ssl; + +import java.io.Reader; +import java.nio.file.Path; + +/** + * A reader along with the path used to construct it. + * + * @author tonytv + */ +public final class ReaderForPath { + + public final Reader reader; + public final Path path; + + public ReaderForPath(Reader reader, Path path) { + this.reader = reader; + this.path = path; + } + +} diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/SslKeyStore.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/SslKeyStore.java index c282c94c1bd..1201bb08afc 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/SslKeyStore.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/SslKeyStore.java @@ -1,12 +1,29 @@ // 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.ssl; +import java.io.IOException; import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateException; +import java.util.Optional; /** * - * @author bjorncs + * @author <a href="mailto:charlesk@yahoo-inc.com">Charles Kim</a> */ -public interface SslKeyStore { - KeyStore loadJavaKeyStore() throws Exception; +public abstract class SslKeyStore { + + private Optional<String> keyStorePassword = Optional.empty(); + + public Optional<String> getKeyStorePassword() { + return keyStorePassword; + } + + public void setKeyStorePassword(String keyStorePassword) { + this.keyStorePassword = Optional.of(keyStorePassword); + } + + public abstract KeyStore loadJavaKeyStore() throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException; + } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/jks/JksKeyStore.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/jks/JKSKeyStore.java index 9cb040fb97d..2ca53b731c3 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/jks/JksKeyStore.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/jks/JKSKeyStore.java @@ -13,33 +13,22 @@ import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; /** - * @author Tony Vaagenes - * @author bjorncs + * @author tonytv */ -public class JksKeyStore implements SslKeyStore { +public class JKSKeyStore extends SslKeyStore { - private static final String KEY_STORE_TYPE = "JKS"; + private static final String keyStoreType = "JKS"; private final Path keyStoreFile; - private final String keyStorePassword; - public JksKeyStore(Path keyStoreFile) { - this(keyStoreFile, null); - } - - public JksKeyStore(Path keyStoreFile, String keyStorePassword) { + public JKSKeyStore(Path keyStoreFile) { this.keyStoreFile = keyStoreFile; - this.keyStorePassword = keyStorePassword; - } - - public String getKeyStorePassword() { - return keyStorePassword; } @Override public KeyStore loadJavaKeyStore() throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { try(InputStream stream = Files.newInputStream(keyStoreFile)) { - KeyStore keystore = KeyStore.getInstance(KEY_STORE_TYPE); - keystore.load(stream, keyStorePassword != null ? keyStorePassword.toCharArray() : null); + KeyStore keystore = KeyStore.getInstance(keyStoreType); + keystore.load(stream, getKeyStorePassword().map(String::toCharArray).orElse(null)); return keystore; } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemKeyStore.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemKeyStore.java index 787c976f6a0..21272f202ea 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemKeyStore.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemKeyStore.java @@ -2,6 +2,7 @@ package com.yahoo.jdisc.http.ssl.pem; import com.google.common.base.Preconditions; +import com.yahoo.jdisc.http.ssl.ReaderForPath; import org.bouncycastle.asn1.pkcs.PrivateKeyInfo; import org.bouncycastle.cert.X509CertificateHolder; import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter; @@ -15,13 +16,9 @@ import javax.annotation.concurrent.GuardedBy; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.Reader; -import java.io.UncheckedIOException; -import java.nio.file.Files; -import java.nio.file.Path; import java.security.Key; -import java.security.KeyStore; import java.security.KeyStore.LoadStoreParameter; +import java.security.KeyStore.ProtectionParameter; import java.security.KeyStoreException; import java.security.KeyStoreSpi; import java.security.NoSuchAlgorithmException; @@ -46,7 +43,7 @@ import static com.yahoo.jdisc.http.server.jetty.Exceptions.throwUnchecked; * @author Tony Vaagenes * @author bjorncs */ -class PemKeyStore extends KeyStoreSpi { +public class PemKeyStore extends KeyStoreSpi { private static String KEY_ALIAS = "KEY"; @@ -61,7 +58,9 @@ class PemKeyStore extends KeyStoreSpi { @GuardedBy("this") private final Map<String, Certificate> aliasToCertificate = new LinkedHashMap<>(); - PemKeyStore() {} + + public PemKeyStore() {} + /** * The user is responsible for closing any readers given in the parameter. @@ -288,51 +287,30 @@ class PemKeyStore extends KeyStoreSpi { } } - // A reader along with the path used to construct it. - private static class ReaderForPath { - final Reader reader; - final Path path; - - private ReaderForPath(Reader reader, Path path) { - this.reader = reader; - this.path = path; - } - - static ReaderForPath of(Path path) { - try { - return new ReaderForPath(Files.newBufferedReader(path), path); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - } - - static class TrustStoreLoadParameter implements KeyStore.LoadStoreParameter { - final ReaderForPath certificateReader; - - TrustStoreLoadParameter(Path certificateReader) { - this.certificateReader = ReaderForPath.of(certificateReader); - } + public static class PemLoadStoreParameter implements LoadStoreParameter { + private PemLoadStoreParameter() {} @Override - public KeyStore.ProtectionParameter getProtectionParameter() { + public ProtectionParameter getProtectionParameter() { return null; } } - static class KeyStoreLoadParameter implements KeyStore.LoadStoreParameter { - final ReaderForPath certificateReader; - final ReaderForPath keyReader; + public static final class KeyStoreLoadParameter extends PemLoadStoreParameter { + public final ReaderForPath certificateReader; + public final ReaderForPath keyReader; - KeyStoreLoadParameter(Path certificateReader, Path keyReader) { - this.certificateReader = ReaderForPath.of(certificateReader); - this.keyReader = ReaderForPath.of(keyReader); + public KeyStoreLoadParameter(ReaderForPath certificateReader, ReaderForPath keyReader) { + this.certificateReader = certificateReader; + this.keyReader = keyReader; } + } - @Override - public KeyStore.ProtectionParameter getProtectionParameter() { - return null; + public static final class TrustStoreLoadParameter extends PemLoadStoreParameter { + public final ReaderForPath certificateReader; + + public TrustStoreLoadParameter(ReaderForPath certificateReader) { + this.certificateReader = certificateReader; } } - } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemSslKeyStore.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemSslKeyStore.java index 9f0a635f7c1..bbb8232f78e 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemSslKeyStore.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemSslKeyStore.java @@ -3,12 +3,11 @@ package com.yahoo.jdisc.http.ssl.pem; import com.yahoo.jdisc.http.ssl.SslKeyStore; import com.yahoo.jdisc.http.ssl.pem.PemKeyStore.KeyStoreLoadParameter; +import com.yahoo.jdisc.http.ssl.pem.PemKeyStore.PemLoadStoreParameter; import com.yahoo.jdisc.http.ssl.pem.PemKeyStore.TrustStoreLoadParameter; import java.io.IOException; -import java.nio.file.Path; import java.security.KeyStore; -import java.security.KeyStore.LoadStoreParameter; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.Provider; @@ -19,32 +18,34 @@ import java.security.cert.CertificateException; * Responsible for creating pem key stores. * * @author Tony Vaagenes - * @author bjorncs */ -public class PemSslKeyStore implements SslKeyStore { +public class PemSslKeyStore extends SslKeyStore { static { Security.addProvider(new PemKeyStoreProvider()); } - private static final String KEY_STORE_TYPE = "PEM"; - - private final LoadStoreParameter loadParameter; + private static final String keyStoreType = "PEM"; + private final PemLoadStoreParameter loadParameter; private KeyStore keyStore; - public PemSslKeyStore(Path certificatePath, Path keyPath) { - this.loadParameter = new KeyStoreLoadParameter(certificatePath, keyPath); + public PemSslKeyStore(KeyStoreLoadParameter loadParameter) { + this.loadParameter = loadParameter; } - public PemSslKeyStore(Path certificatePath) { - this.loadParameter = new TrustStoreLoadParameter(certificatePath); + public PemSslKeyStore(TrustStoreLoadParameter loadParameter) { + this.loadParameter = loadParameter; } @Override public KeyStore loadJavaKeyStore() throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { + if (getKeyStorePassword().isPresent()) { + throw new UnsupportedOperationException("PEM key store with password is currently not supported. Please file a feature request."); + } + //cached since Reader(in loadParameter) can only be used one time. if (keyStore == null) { - keyStore = KeyStore.getInstance(KEY_STORE_TYPE); + keyStore = KeyStore.getInstance(keyStoreType); keyStore.load(loadParameter); } return keyStore; @@ -60,6 +61,6 @@ public class PemSslKeyStore implements SslKeyStore { super(NAME, VERSION, DESCRIPTION); putService(new Service(this, "KeyStore", "PEM", PemKeyStore. class.getName(), PemKeyStore.aliases, PemKeyStore.attributes)); } - } + } } 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 7a13ec2485f..8d709cb8ab1 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 @@ -77,7 +77,6 @@ ssl.trustStorePath string default="" # Whether we should use keyDbKey as password to the trust store (true, default), # or use no password with the trust store (false) ssl.useTrustStorePassword bool default=true -# TODO Fix broken semantics with truststore and keystore password in Vespa 7 / Vespa 8 # The algorithm name used by the KeyManagerFactory. ssl.sslKeyManagerFactoryAlgorithm string default="SunX509" diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java index 5dd5dca1667..e71bd190a37 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java @@ -1,11 +1,16 @@ // 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; -import com.yahoo.jdisc.http.ssl.jks.JksKeyStore; +import com.yahoo.jdisc.http.ssl.SslKeyStore; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManagerFactory; +import java.io.IOException; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.UnrecoverableKeyException; +import java.security.cert.CertificateException; import java.util.logging.Level; import java.util.logging.Logger; @@ -27,16 +32,16 @@ public class SslContextFactory { return this.sslContext; } - public static SslContextFactory newInstanceFromTrustStore(JksKeyStore trustStore) { + public static SslContextFactory newInstanceFromTrustStore(SslKeyStore trustStore) { return newInstance(DEFAULT_ALGORITHM, DEFAULT_PROTOCOL, null, trustStore); } - public static SslContextFactory newInstance(JksKeyStore trustStore, JksKeyStore keyStore) { + public static SslContextFactory newInstance(SslKeyStore trustStore, SslKeyStore keyStore) { return newInstance(DEFAULT_ALGORITHM, DEFAULT_PROTOCOL, keyStore, trustStore); } public static SslContextFactory newInstance(String sslAlgorithm, String sslProtocol, - JksKeyStore keyStore, JksKeyStore trustStore) { + SslKeyStore keyStore, SslKeyStore trustStore) { log.fine("Configuring SSLContext..."); log.fine("Using " + sslAlgorithm + " algorithm."); try { @@ -55,14 +60,15 @@ public class SslContextFactory { /** * Used for the key store, which contains the SSL cert and private key. */ - public static javax.net.ssl.KeyManager[] getKeyManagers(JksKeyStore keyStore, - String sslAlgorithm) throws Exception { + public static javax.net.ssl.KeyManager[] getKeyManagers(SslKeyStore keyStore, + String sslAlgorithm) + throws NoSuchAlgorithmException, CertificateException, IOException, UnrecoverableKeyException, + KeyStoreException { KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(sslAlgorithm); - String keyStorePassword = keyStore.getKeyStorePassword(); keyManagerFactory.init( keyStore.loadJavaKeyStore(), - keyStorePassword != null ? keyStorePassword.toCharArray() : null); + keyStore.getKeyStorePassword().map(String::toCharArray).orElse(null)); log.fine("KeyManagerFactory initialized with keystore"); return keyManagerFactory.getKeyManagers(); } @@ -71,9 +77,9 @@ public class SslContextFactory { * Used for the trust store, which contains certificates from other parties that you expect to communicate with, * or from Certificate Authorities that you trust to identify other parties. */ - public static javax.net.ssl.TrustManager[] getTrustManagers(JksKeyStore trustStore, + public static javax.net.ssl.TrustManager[] getTrustManagers(SslKeyStore trustStore, String sslAlgorithm) - throws Exception { + throws NoSuchAlgorithmException, CertificateException, IOException, KeyStoreException { TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(sslAlgorithm); trustManagerFactory.init(trustStore.loadJavaKeyStore()); 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 1380abc03f3..25457c0c6c6 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,20 +1,33 @@ // 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.SslKeyStore; +import com.yahoo.jdisc.http.ssl.pem.PemKeyStore; +import com.yahoo.jdisc.http.ssl.pem.PemSslKeyStore; import org.eclipse.jetty.server.Request; 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; @@ -60,7 +73,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); + (ConnectorFactory.JDiscServerConnector)factory.createConnector(new DummyMetric(), server, null, Collections.emptyMap()); server.addConnector(connector); server.setHandler(new HelloWorldHandler()); server.start(); @@ -86,7 +99,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); + ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel, Collections.emptyMap()); server.addConnector(connector); server.setHandler(new HelloWorldHandler()); server.start(); @@ -104,6 +117,63 @@ 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/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java index cc7095dadda..d588ace8268 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java @@ -24,7 +24,6 @@ import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.TypeSafeMatcher; import org.testng.annotations.AfterClass; -import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; import java.net.URI; @@ -34,8 +33,6 @@ import java.util.Collections; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.logging.Level; -import java.util.logging.Logger; import java.util.regex.Pattern; import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; @@ -52,34 +49,13 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; /** - * @author Simon Thoresen Hult + * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a> */ public class HttpServerConformanceTest extends ServerProviderConformanceTest { - private static final Logger log = Logger.getLogger(HttpServerConformanceTest.class.getName()); - private static final String REQUEST_CONTENT = "myRequestContent"; private static final String RESPONSE_CONTENT = "myResponseContent"; - @SuppressWarnings("LoggerInitializedWithForeignClass") - private static Logger httpRequestDispatchLogger = Logger.getLogger(HttpRequestDispatch.class.getName()); - private static Level httpRequestDispatchLoggerOriginalLevel; - - /* - * Reduce logging of every stack trace for {@link ServerProviderConformanceTest.ConformanceException} thrown. - * This makes the log more readable and the test faster as well. - */ - @BeforeClass - public static void reduceExcessiveLogging() { - httpRequestDispatchLoggerOriginalLevel = httpRequestDispatchLogger.getLevel(); - httpRequestDispatchLogger.setLevel(Level.SEVERE); - } - - @AfterClass - public static void restoreExcessiveLogging() { - httpRequestDispatchLogger.setLevel(httpRequestDispatchLoggerOriginalLevel); - } - @AfterClass public static void reportDiagnostics() { System.out.println( @@ -808,7 +784,7 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest { post.setProtocolVersion(client.requestVersion); request = post; } - log.fine(() -> "executorService:" + System.out.println("executorService:" + " .isShutDown()=" + executorService.isShutdown() + " .isTerminated()=" + executorService.isTerminated()); return executorService.submit(() -> client.delegate.execute(request)); diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java index 525cde9d8b3..8ddcd7f03ac 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java @@ -6,8 +6,9 @@ import com.google.inject.Module; import com.yahoo.jdisc.application.ContainerBuilder; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.http.ConnectorConfig; +import com.yahoo.jdisc.http.ssl.jks.JKSKeyStore; import com.yahoo.jdisc.http.SslContextFactory; -import com.yahoo.jdisc.http.ssl.jks.JksKeyStore; +import com.yahoo.jdisc.http.ssl.SslKeyStore; import javax.net.ssl.SSLContext; import java.io.IOException; @@ -75,9 +76,8 @@ public class TestDriver { ConnectorConfig.Ssl sslConfig = builder.getInstance(ConnectorConfig.class).ssl(); if (!sslConfig.enabled()) return null; - JksKeyStore keyStore = new JksKeyStore( - Paths.get(sslConfig.keyStorePath()), - builder.getInstance(Key.get(String.class, named("keyStorePassword")))); + SslKeyStore keyStore = new JKSKeyStore(Paths.get(sslConfig.keyStorePath())); + keyStore.setKeyStorePassword(builder.getInstance(Key.get(String.class, named("keyStorePassword")))); return SslContextFactory.newInstanceFromTrustStore(keyStore).getServerSSLContext(); } 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 baf0b41b270..d03a08a27db 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,15 +19,24 @@ 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 @@ -36,19 +45,69 @@ public class StandaloneContainerActivator implements BundleActivator { @Override public void start(BundleContext bundleContext) throws Exception { - for (ConnectorConfig config: getConnectorConfigs(getContainer())) { + 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) { ServerSocketChannel socketChannel = bindChannel(config); registerChannels(bundleContext, config.listenPort(), socketChannel); } } - static void registerChannels(BundleContext bundleContext, int listenPort, ServerSocketChannel boundChannel) { + 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); bundleContext.registerService(ServerSocketChannel.class, boundChannel, properties); } - static ServerSocketChannel bindChannel(ConnectorConfig channelInfo) throws IOException { + ServerSocketChannel bindChannel(ConnectorConfig channelInfo) throws IOException { ServerSocketChannel serverChannel = ServerSocketChannel.open(); InetSocketAddress bindAddress = new InetSocketAddress(channelInfo.listenPort()); serverChannel.socket().bind(bindAddress, channelInfo.acceptQueueSize()); @@ -58,7 +117,7 @@ public class StandaloneContainerActivator implements BundleActivator { @Override public void stop(BundleContext bundleContext) throws Exception { } - static Container getContainer(Module... modules) { + Container getContainer(Module... modules) { Module activatorModule = new ActivatorModule(); List<Module> allModules = new ArrayList<>(); allModules.addAll(Arrays.asList(modules)); @@ -68,7 +127,7 @@ public class StandaloneContainerActivator implements BundleActivator { return app.container(); } - static List<ConnectorConfig> getConnectorConfigs(Container container) { + List<ConnectorConfig> getConnectorConfigs(Container container) { Http http = container.getHttp(); return (http == null) ? diff --git a/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneContainerActivatorTest.java b/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneContainerActivatorTest.java index 8d413ade0f0..48c882c78db 100644 --- a/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneContainerActivatorTest.java +++ b/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneContainerActivatorTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.standalone; +import com.google.inject.Binder; import com.google.inject.Module; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.http.ConnectorConfig; @@ -17,7 +18,6 @@ import java.nio.file.Path; import java.util.List; import static java.util.Arrays.asList; -import static java.util.Collections.singletonList; import static java.util.stream.Collectors.toList; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.collection.IsEmptyCollection.empty; @@ -29,7 +29,7 @@ import static org.junit.Assert.assertThat; */ public class StandaloneContainerActivatorTest { - private static String getJdiscXml(String contents) throws ParserConfigurationException, IOException, SAXException { + private String getJdiscXml(String contents) throws ParserConfigurationException, IOException, SAXException { return "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" + "<services>\n" + " <jdisc version=\"1.0\" jetty=\"true\">\n" + @@ -38,7 +38,7 @@ public class StandaloneContainerActivatorTest { "</services>"; } - private static void writeApplicationPackage(String servicesXml, Path tmpDir) throws IOException { + private void writeApplicationPackage(String servicesXml, Path tmpDir) throws IOException { FileWriter fw = new FileWriter(tmpDir.resolve("services.xml").toFile(), false); fw.write(servicesXml); fw.close(); @@ -50,16 +50,16 @@ public class StandaloneContainerActivatorTest { try { writeApplicationPackage(getJdiscXml(""), applicationDir); StandaloneContainerActivator activator = new StandaloneContainerActivator(); - Container container = StandaloneContainerActivator.getContainer(newAppDirBinding(applicationDir)); + Container container = activator.getContainer(newAppDirBinding(applicationDir)); List<Integer> ports = getPorts(activator, container); - assertThat(ports, is(singletonList(Defaults.getDefaults().vespaWebServicePort()))); + assertThat(ports, is(asList(Defaults.getDefaults().vespaWebServicePort()))); } finally { IOUtils.recursiveDeleteDir(applicationDir.toFile()); } } - private static List<Integer> getPorts(StandaloneContainerActivator activator, Container container) { - return StandaloneContainerActivator.getConnectorConfigs(container).stream(). + private List<Integer> getPorts(StandaloneContainerActivator activator, Container container) { + return activator.getConnectorConfigs(container).stream(). map(ConnectorConfig::listenPort). collect(toList()); } @@ -70,7 +70,7 @@ public class StandaloneContainerActivatorTest { try { writeApplicationPackage(getJdiscXml("<http/>"), applicationDir); StandaloneContainerActivator activator = new StandaloneContainerActivator(); - Container container = StandaloneContainerActivator.getContainer(newAppDirBinding(applicationDir)); + Container container = activator.getContainer(newAppDirBinding(applicationDir)); List<Integer> ports = getPorts(activator, container); assertThat(ports, empty()); } finally { @@ -90,7 +90,7 @@ public class StandaloneContainerActivatorTest { "</http>\n"; writeApplicationPackage(getJdiscXml(contents), applicationDir); StandaloneContainerActivator activator = new StandaloneContainerActivator(); - Container container = StandaloneContainerActivator.getContainer(newAppDirBinding(applicationDir)); + Container container = activator.getContainer(newAppDirBinding(applicationDir)); List<Integer> ports = getPorts(activator, container); assertThat(ports, is(asList(123, 456, 789))); } finally { @@ -98,10 +98,15 @@ public class StandaloneContainerActivatorTest { } } - private static Module newAppDirBinding(final Path applicationDir) { - return binder -> binder.bind(Path.class) - .annotatedWith(StandaloneContainerApplication.applicationPathName()) - .toInstance(applicationDir); + private Module newAppDirBinding(final Path applicationDir) { + return new Module() { + @Override + public void configure(Binder binder) { + binder.bind(Path.class) + .annotatedWith(StandaloneContainerApplication.applicationPathName()) + .toInstance(applicationDir); + } + }; } } |