diff options
15 files changed, 157 insertions, 366 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 d2b58090665..e7039e85e5e 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,7 +24,6 @@ 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; @@ -35,14 +34,18 @@ 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 <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a> + * @author Simon Thoresen Hult */ @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; /** @@ -2790,7 +2793,7 @@ public abstract class ServerProviderConformanceTest { serverProvider.release(); for (int i = 0; i < NUM_RUNS_EACH_TEST; ++i) { - System.out.println("Test run #" + i); + log.fine("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 96180f48229..54338c64c1e 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,8 +8,6 @@ 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; @@ -24,15 +22,11 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import javax.servlet.ServletRequest; import java.io.IOException; -import java.io.Reader; +import java.io.UncheckedIOException; 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; @@ -43,10 +37,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 +76,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 +117,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 +167,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,54 +200,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); - } - - } - - 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(); + 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); } 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 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/main/java/com/yahoo/jdisc/http/ssl/ReaderForPath.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/ReaderForPath.java deleted file mode 100644 index b04d91d7403..00000000000 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/ReaderForPath.java +++ /dev/null @@ -1,22 +0,0 @@ -// 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 1201bb08afc..c282c94c1bd 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,29 +1,12 @@ // 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 <a href="mailto:charlesk@yahoo-inc.com">Charles Kim</a> + * @author bjorncs */ -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; - +public interface SslKeyStore { + KeyStore loadJavaKeyStore() throws Exception; } 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 2ca53b731c3..9cb040fb97d 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,22 +13,33 @@ import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; /** - * @author tonytv + * @author Tony Vaagenes + * @author bjorncs */ -public class JKSKeyStore extends SslKeyStore { +public class JksKeyStore implements SslKeyStore { - private static final String keyStoreType = "JKS"; + private static final String KEY_STORE_TYPE = "JKS"; private final Path keyStoreFile; + private final String keyStorePassword; - public JKSKeyStore(Path keyStoreFile) { + public JksKeyStore(Path keyStoreFile) { + this(keyStoreFile, null); + } + + public JksKeyStore(Path keyStoreFile, String keyStorePassword) { 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(keyStoreType); - keystore.load(stream, getKeyStorePassword().map(String::toCharArray).orElse(null)); + KeyStore keystore = KeyStore.getInstance(KEY_STORE_TYPE); + keystore.load(stream, keyStorePassword != null ? keyStorePassword.toCharArray() : 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 21272f202ea..787c976f6a0 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,7 +2,6 @@ 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; @@ -16,9 +15,13 @@ 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; @@ -43,7 +46,7 @@ import static com.yahoo.jdisc.http.server.jetty.Exceptions.throwUnchecked; * @author Tony Vaagenes * @author bjorncs */ -public class PemKeyStore extends KeyStoreSpi { +class PemKeyStore extends KeyStoreSpi { private static String KEY_ALIAS = "KEY"; @@ -58,9 +61,7 @@ public class PemKeyStore extends KeyStoreSpi { @GuardedBy("this") private final Map<String, Certificate> aliasToCertificate = new LinkedHashMap<>(); - - public PemKeyStore() {} - + PemKeyStore() {} /** * The user is responsible for closing any readers given in the parameter. @@ -287,30 +288,51 @@ public class PemKeyStore extends KeyStoreSpi { } } - public static class PemLoadStoreParameter implements LoadStoreParameter { - private PemLoadStoreParameter() {} + // A reader along with the path used to construct it. + private static class ReaderForPath { + final Reader reader; + final Path path; - @Override - public ProtectionParameter getProtectionParameter() { - return null; + 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); + } } } - public static final class KeyStoreLoadParameter extends PemLoadStoreParameter { - public final ReaderForPath certificateReader; - public final ReaderForPath keyReader; + static class TrustStoreLoadParameter implements KeyStore.LoadStoreParameter { + final ReaderForPath certificateReader; + + TrustStoreLoadParameter(Path certificateReader) { + this.certificateReader = ReaderForPath.of(certificateReader); + } - 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; + static class KeyStoreLoadParameter implements KeyStore.LoadStoreParameter { + final ReaderForPath certificateReader; + final ReaderForPath keyReader; + + KeyStoreLoadParameter(Path certificateReader, Path keyReader) { + this.certificateReader = ReaderForPath.of(certificateReader); + this.keyReader = ReaderForPath.of(keyReader); + } - public TrustStoreLoadParameter(ReaderForPath certificateReader) { - this.certificateReader = certificateReader; + @Override + public KeyStore.ProtectionParameter getProtectionParameter() { + return null; } } + } 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 bbb8232f78e..9f0a635f7c1 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,11 +3,12 @@ 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; @@ -18,34 +19,32 @@ import java.security.cert.CertificateException; * Responsible for creating pem key stores. * * @author Tony Vaagenes + * @author bjorncs */ -public class PemSslKeyStore extends SslKeyStore { +public class PemSslKeyStore implements SslKeyStore { static { Security.addProvider(new PemKeyStoreProvider()); } - private static final String keyStoreType = "PEM"; - private final PemLoadStoreParameter loadParameter; + private static final String KEY_STORE_TYPE = "PEM"; + + private final LoadStoreParameter loadParameter; private KeyStore keyStore; - public PemSslKeyStore(KeyStoreLoadParameter loadParameter) { - this.loadParameter = loadParameter; + public PemSslKeyStore(Path certificatePath, Path keyPath) { + this.loadParameter = new KeyStoreLoadParameter(certificatePath, keyPath); } - public PemSslKeyStore(TrustStoreLoadParameter loadParameter) { - this.loadParameter = loadParameter; + public PemSslKeyStore(Path certificatePath) { + this.loadParameter = new TrustStoreLoadParameter(certificatePath); } @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(keyStoreType); + keyStore = KeyStore.getInstance(KEY_STORE_TYPE); keyStore.load(loadParameter); } return keyStore; @@ -61,6 +60,6 @@ public class PemSslKeyStore extends 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 8d709cb8ab1..7a13ec2485f 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,6 +77,7 @@ 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 e71bd190a37..5dd5dca1667 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,16 +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; -import com.yahoo.jdisc.http.ssl.SslKeyStore; +import com.yahoo.jdisc.http.ssl.jks.JksKeyStore; 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; @@ -32,16 +27,16 @@ public class SslContextFactory { return this.sslContext; } - public static SslContextFactory newInstanceFromTrustStore(SslKeyStore trustStore) { + public static SslContextFactory newInstanceFromTrustStore(JksKeyStore trustStore) { return newInstance(DEFAULT_ALGORITHM, DEFAULT_PROTOCOL, null, trustStore); } - public static SslContextFactory newInstance(SslKeyStore trustStore, SslKeyStore keyStore) { + public static SslContextFactory newInstance(JksKeyStore trustStore, JksKeyStore keyStore) { return newInstance(DEFAULT_ALGORITHM, DEFAULT_PROTOCOL, keyStore, trustStore); } public static SslContextFactory newInstance(String sslAlgorithm, String sslProtocol, - SslKeyStore keyStore, SslKeyStore trustStore) { + JksKeyStore keyStore, JksKeyStore trustStore) { log.fine("Configuring SSLContext..."); log.fine("Using " + sslAlgorithm + " algorithm."); try { @@ -60,15 +55,14 @@ public class SslContextFactory { /** * Used for the key store, which contains the SSL cert and private key. */ - public static javax.net.ssl.KeyManager[] getKeyManagers(SslKeyStore keyStore, - String sslAlgorithm) - throws NoSuchAlgorithmException, CertificateException, IOException, UnrecoverableKeyException, - KeyStoreException { + public static javax.net.ssl.KeyManager[] getKeyManagers(JksKeyStore keyStore, + String sslAlgorithm) throws Exception { KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(sslAlgorithm); + String keyStorePassword = keyStore.getKeyStorePassword(); keyManagerFactory.init( keyStore.loadJavaKeyStore(), - keyStore.getKeyStorePassword().map(String::toCharArray).orElse(null)); + keyStorePassword != null ? keyStorePassword.toCharArray() : null); log.fine("KeyManagerFactory initialized with keystore"); return keyManagerFactory.getKeyManagers(); } @@ -77,9 +71,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(SslKeyStore trustStore, + public static javax.net.ssl.TrustManager[] getTrustManagers(JksKeyStore trustStore, String sslAlgorithm) - throws NoSuchAlgorithmException, CertificateException, IOException, KeyStoreException { + throws Exception { 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 25457c0c6c6..1380abc03f3 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,33 +1,20 @@ // 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; @@ -73,7 +60,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 +86,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 +104,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/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 d588ace8268..cc7095dadda 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,6 +24,7 @@ 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; @@ -33,6 +34,8 @@ 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; @@ -49,13 +52,34 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; /** - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a> + * @author Simon Thoresen Hult */ 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( @@ -784,7 +808,7 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest { post.setProtocolVersion(client.requestVersion); request = post; } - System.out.println("executorService:" + log.fine(() -> "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 8ddcd7f03ac..525cde9d8b3 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,9 +6,8 @@ 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.SslKeyStore; +import com.yahoo.jdisc.http.ssl.jks.JksKeyStore; import javax.net.ssl.SSLContext; import java.io.IOException; @@ -76,8 +75,9 @@ public class TestDriver { ConnectorConfig.Ssl sslConfig = builder.getInstance(ConnectorConfig.class).ssl(); if (!sslConfig.enabled()) return null; - SslKeyStore keyStore = new JKSKeyStore(Paths.get(sslConfig.keyStorePath())); - keyStore.setKeyStorePassword(builder.getInstance(Key.get(String.class, named("keyStorePassword")))); + JksKeyStore keyStore = new JksKeyStore( + Paths.get(sslConfig.keyStorePath()), + 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 d03a08a27db..baf0b41b270 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,69 +36,19 @@ 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) { + static void registerChannels(BundleContext bundleContext, int listenPort, ServerSocketChannel boundChannel) { Hashtable<String, Integer> properties = new Hashtable<>(); properties.put("port", listenPort); bundleContext.registerService(ServerSocketChannel.class, boundChannel, properties); } - ServerSocketChannel bindChannel(ConnectorConfig channelInfo) throws IOException { + static ServerSocketChannel bindChannel(ConnectorConfig channelInfo) throws IOException { ServerSocketChannel serverChannel = ServerSocketChannel.open(); InetSocketAddress bindAddress = new InetSocketAddress(channelInfo.listenPort()); serverChannel.socket().bind(bindAddress, channelInfo.acceptQueueSize()); @@ -117,7 +58,7 @@ public class StandaloneContainerActivator implements BundleActivator { @Override public void stop(BundleContext bundleContext) throws Exception { } - Container getContainer(Module... modules) { + static Container getContainer(Module... modules) { Module activatorModule = new ActivatorModule(); List<Module> allModules = new ArrayList<>(); allModules.addAll(Arrays.asList(modules)); @@ -127,7 +68,7 @@ public class StandaloneContainerActivator implements BundleActivator { return app.container(); } - List<ConnectorConfig> getConnectorConfigs(Container container) { + static 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 48c882c78db..8d413ade0f0 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,7 +1,6 @@ // 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; @@ -18,6 +17,7 @@ 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 String getJdiscXml(String contents) throws ParserConfigurationException, IOException, SAXException { + private static 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 void writeApplicationPackage(String servicesXml, Path tmpDir) throws IOException { + private static 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 = activator.getContainer(newAppDirBinding(applicationDir)); + Container container = StandaloneContainerActivator.getContainer(newAppDirBinding(applicationDir)); List<Integer> ports = getPorts(activator, container); - assertThat(ports, is(asList(Defaults.getDefaults().vespaWebServicePort()))); + assertThat(ports, is(singletonList(Defaults.getDefaults().vespaWebServicePort()))); } finally { IOUtils.recursiveDeleteDir(applicationDir.toFile()); } } - private List<Integer> getPorts(StandaloneContainerActivator activator, Container container) { - return activator.getConnectorConfigs(container).stream(). + private static List<Integer> getPorts(StandaloneContainerActivator activator, Container container) { + return StandaloneContainerActivator.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 = activator.getContainer(newAppDirBinding(applicationDir)); + Container container = StandaloneContainerActivator.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 = activator.getContainer(newAppDirBinding(applicationDir)); + Container container = StandaloneContainerActivator.getContainer(newAppDirBinding(applicationDir)); List<Integer> ports = getPorts(activator, container); assertThat(ports, is(asList(123, 456, 789))); } finally { @@ -98,15 +98,10 @@ public class StandaloneContainerActivatorTest { } } - private Module newAppDirBinding(final Path applicationDir) { - return new Module() { - @Override - public void configure(Binder binder) { - binder.bind(Path.class) - .annotatedWith(StandaloneContainerApplication.applicationPathName()) - .toInstance(applicationDir); - } - }; + private static Module newAppDirBinding(final Path applicationDir) { + return binder -> binder.bind(Path.class) + .annotatedWith(StandaloneContainerApplication.applicationPathName()) + .toInstance(applicationDir); } } |