Simplify PemSslKeyStore interface

5 files changed, 61 insertions, 88 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 097d0f6970e..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,12 +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.ServerSocketChannel;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.KeyStore;
@@ -206,33 +203,13 @@ public class ConnectorFactory {
     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 {
-            public final ReaderForPath readerForPath;
-
-            KeyStoreReaderForPath(String pathString) {
-                Path path = Paths.get(pathString);
-                readerForPath = new ReaderForPath(getReader(path), path);
-            }
-
-            private Reader getReader(Path path) {
-                try {
-                    return Files.newBufferedReader(path);
-                } catch (IOException e) {
-                    throw new RuntimeException("Failed opening " + path, e);
-                }
-            }
-
-            @Override
-            public void close() {}
-        }
-
-        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/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/pem/PemKeyStore.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/pem/PemKeyStore.java
index 21272f202ea..b52e923662f 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;
@@ -58,10 +61,6 @@ public class PemKeyStore extends KeyStoreSpi {
     @GuardedBy("this")
     private final Map<String, Certificate> aliasToCertificate = new LinkedHashMap<>();
 
-
-    public PemKeyStore() {}
-
-
     /**
      * The user is responsible for closing any readers given in the parameter.
      */
@@ -287,30 +286,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;
 
-        public KeyStoreLoadParameter(ReaderForPath certificateReader, ReaderForPath keyReader) {
-            this.certificateReader = certificateReader;
-            this.keyReader = keyReader;
+        TrustStoreLoadParameter(Path certificateReader) {
+            this.certificateReader = ReaderForPath.of(certificateReader);
+        }
+
+        @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..9cede37caaa 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,6 +19,7 @@ import java.security.cert.CertificateException;
  * Responsible for creating pem key stores.
  *
  * @author Tony Vaagenes
+ * @author bjorncs
  */
 public class PemSslKeyStore extends SslKeyStore {
 
@@ -25,16 +27,17 @@ public class PemSslKeyStore extends SslKeyStore {
         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
@@ -45,7 +48,7 @@ public class PemSslKeyStore extends SslKeyStore {
 
         //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 +64,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/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 dfa30a3b7db..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
@@ -4,11 +4,6 @@ package com.yahoo.jdisc.http.server.jetty;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.http.ConnectorConfig;
 import com.yahoo.jdisc.http.SecretStore;
-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;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.AbstractHandler;