Merge pull request #5297 from vespa-engine/freva/simplify-key-load/store

Simplify key load/store

5 files changed, 27 insertions, 53 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
index 4fb3049c39d..5fb619ee6e2 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
@@ -117,13 +117,11 @@ public class Environment {
         this.keyStoreOptions = createKeyStoreOptions(
                 configServerConfig.keyStoreConfig().path(),
                 configServerConfig.keyStoreConfig().password().toCharArray(),
-                configServerConfig.keyStoreConfig().type().name(),
-                "BC");
+                configServerConfig.keyStoreConfig().type().name());
         this.trustStoreOptions = createKeyStoreOptions(
                 configServerConfig.trustStoreConfig().path(),
                 configServerConfig.trustStoreConfig().password().toCharArray(),
-                configServerConfig.trustStoreConfig().type().name(),
-                null);
+                configServerConfig.trustStoreConfig().type().name());
         this.athenzIdentity = createAthenzIdentity(
                 configServerConfig.athenzDomain(),
                 configServerConfig.serviceName());
@@ -184,10 +182,10 @@ public class Environment {
         return Arrays.asList(logstashNodes.split("[,\\s]+"));
     }
 
-    private static Optional<KeyStoreOptions> createKeyStoreOptions(String pathToKeyStore, char[] password, String type, String provider) {
+    private static Optional<KeyStoreOptions> createKeyStoreOptions(String pathToKeyStore, char[] password, String type) {
         return Optional.ofNullable(pathToKeyStore)
                 .filter(path -> !Strings.isNullOrEmpty(path))
-                .map(path -> new KeyStoreOptions(Paths.get(path), password, type, provider));
+                .map(path -> new KeyStoreOptions(Paths.get(path), password, type));
     }
 
     private static Optional<AthenzIdentity> createAthenzIdentity(String athenzDomain, String serviceName) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConfigServerApiImpl.java
index 04b222875c3..110dbe9c9b3 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConfigServerApiImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConfigServerApiImpl.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.configserver;
 
 import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier;
 import com.yahoo.vespa.athenz.tls.AthenzSslContextBuilder;
-import com.yahoo.vespa.athenz.tls.KeyStoreType;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.configserver.certificate.ConfigServerKeyStoreRefresher;
 import com.yahoo.vespa.hosted.node.admin.util.KeyStoreOptions;
@@ -18,7 +17,7 @@ import java.util.Optional;
 
 /**
  * ConfigServerApi with proper keystore, truststore and hostname verifier to communicate with the
- * configserver(s). The keystore is refreshed automatically.
+ * config server(s). The keystore is refreshed automatically.
  *
  * @author freva
  */
@@ -99,16 +98,8 @@ public class SslConfigServerApiImpl implements ConfigServerApi {
 
     private SSLContext makeSslContext(Optional<KeyStoreOptions> keyStoreOptions) {
         AthenzSslContextBuilder sslContextBuilder = new AthenzSslContextBuilder();
-        environment.getTrustStoreOptions().ifPresent(
-                options -> sslContextBuilder.withTrustStore(options.path.toFile(), KeyStoreType.valueOf(options.type)));
-
-        keyStoreOptions.ifPresent(options -> {
-            try {
-                sslContextBuilder.withKeyStore(options.path.toFile(), options.password, KeyStoreType.valueOf(options.type));
-            } catch (Exception e) {
-                throw new RuntimeException("Failed to read key store", e);
-            }
-        });
+        environment.getTrustStoreOptions().map(KeyStoreOptions::loadKeyStore).ifPresent(sslContextBuilder::withTrustStore);
+        keyStoreOptions.ifPresent(options -> sslContextBuilder.withKeyStore(options.loadKeyStore(), options.password));
 
         return sslContextBuilder.build();
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresher.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresher.java
index ae725769bdb..a9db96c2a77 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresher.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresher.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.certificate;
 
 import com.yahoo.log.LogLevel;
 import com.yahoo.net.HostName;
+import com.yahoo.vespa.athenz.tls.KeyStoreBuilder;
 import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi;
 import com.yahoo.vespa.hosted.node.admin.util.KeyStoreOptions;
 import org.bouncycastle.asn1.x500.X500Name;
@@ -12,7 +13,6 @@ import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
 import org.bouncycastle.pkcs.PKCS10CertificationRequest;
 import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequestBuilder;
 
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
@@ -159,14 +159,12 @@ public class ConfigServerKeyStoreRefresher {
     private void storeCertificate(KeyPair keyPair, X509Certificate certificate)
             throws KeyStoreException, CertificateException, NoSuchAlgorithmException, IOException, NoSuchProviderException {
         keyStoreOptions.path.getParent().toFile().mkdirs();
-        X509Certificate[] certificateChain = {certificate};
 
-        try (FileOutputStream fos = new FileOutputStream(keyStoreOptions.path.toFile())) {
-            KeyStore keyStore = keyStoreOptions.getKeyStoreInstance();
-            keyStore.load(null, null);
-            keyStore.setKeyEntry(KEY_STORE_ALIAS, keyPair.getPrivate(), keyStoreOptions.password, certificateChain);
-            keyStore.store(fos, keyStoreOptions.password);
-        }
+        KeyStore keyStore = KeyStoreBuilder.withType(keyStoreOptions.keyStoreType)
+                .withKeyEntry(KEY_STORE_ALIAS, keyPair.getPrivate(), keyStoreOptions.password, certificate)
+                .build();
+
+        keyStoreOptions.storeKeyStore(keyStore);
     }
 
     private X509Certificate sendCsr(PKCS10CertificationRequest csr) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/KeyStoreOptions.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/KeyStoreOptions.java
index 1115f6dca91..03aff7f22d8 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/KeyStoreOptions.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/KeyStoreOptions.java
@@ -1,45 +1,32 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.util;
 
-import java.io.FileInputStream;
-import java.io.IOException;
+import com.yahoo.vespa.athenz.tls.KeyStoreBuilder;
+import com.yahoo.vespa.athenz.tls.KeyStoreType;
+import com.yahoo.vespa.athenz.tls.KeyStoreUtils;
+
 import java.nio.file.Path;
 import java.security.KeyStore;
-import java.security.KeyStoreException;
-import java.security.NoSuchAlgorithmException;
-import java.security.NoSuchProviderException;
-import java.security.cert.CertificateException;
-import java.util.Optional;
 
 public class KeyStoreOptions {
     public final Path path;
     public final char[] password;
-    public final String type;
-    private final Optional<String> provider;
+    public final KeyStoreType keyStoreType;
 
     public KeyStoreOptions(Path path, char[] password, String type) {
-        this(path, password, type, null);
-    }
-
-    public KeyStoreOptions(Path path, char[] password, String type, String provider) {
         this.path = path;
         this.password = password;
-        this.type = type;
-        this.provider = Optional.ofNullable(provider);
+        this.keyStoreType = KeyStoreType.valueOf(type);
     }
 
-    public KeyStore loadKeyStore()
-            throws IOException, NoSuchProviderException, KeyStoreException, CertificateException, NoSuchAlgorithmException {
-        try (FileInputStream in = new FileInputStream(path.toFile())) {
-            KeyStore keyStore = getKeyStoreInstance();
-            keyStore.load(in, password);
-            return keyStore;
-        }
+    public KeyStore loadKeyStore() {
+        return KeyStoreBuilder
+                .withType(keyStoreType)
+                .fromFile(path.toFile(), password)
+                .build();
     }
 
-    public KeyStore getKeyStoreInstance() throws NoSuchProviderException, KeyStoreException {
-        return provider.isPresent() ?
-                KeyStore.getInstance(type, provider.get()) :
-                KeyStore.getInstance(type);
+    public void storeKeyStore(KeyStore keyStore) {
+        KeyStoreUtils.writeKeyStoreToFile(keyStore, path.toFile(), password);
     }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresherTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresherTest.java
index f9f8b230154..85684ea3bd4 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresherTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/certificate/ConfigServerKeyStoreRefresherTest.java
@@ -54,7 +54,7 @@ public class ConfigServerKeyStoreRefresherTest {
     @Before
     public void setup() {
         keyStoreOptions = new KeyStoreOptions(
-                tempFolder.getRoot().toPath().resolve("some/path/keystore.p12"), new char[0], "PKCS12", null);
+                tempFolder.getRoot().toPath().resolve("some/path/keystore.p12"), new char[0], "PKCS12");
     }
 
     @Test