Use a single reloader per tls config file, and not one per instance.

2 files changed, 162 insertions, 135 deletions

diff --git a/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java b/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java
index 28854c59b2c..26dfbf9fd9f 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java
@@ -1,32 +1,20 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.security.tls;
 
-import com.yahoo.security.KeyStoreBuilder;
-import com.yahoo.security.KeyStoreType;
-import com.yahoo.security.KeyUtils;
 import com.yahoo.security.SslContextBuilder;
-import com.yahoo.security.X509CertificateUtils;
 import com.yahoo.security.tls.authz.PeerAuthorizerTrustManager;
 import com.yahoo.security.tls.policy.AuthorizedPeers;
 
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
-import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.lang.ref.WeakReference;
 import java.nio.file.Path;
-import java.security.KeyStore;
-import java.time.Duration;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * A {@link TlsContext} that uses the tls configuration specified in the transport security options file.
@@ -36,12 +24,22 @@ import java.util.logging.Logger;
  */
 public class ConfigFileBasedTlsContext implements TlsContext {
 
-    private static final Duration UPDATE_PERIOD = Duration.ofHours(1);
+    private final TlsContext tlsContext;
+    private TlsManager tlsManager;
 
-    private static final Logger log = Logger.getLogger(ConfigFileBasedTlsContext.class.getName());
+    static private final Map<Path, WeakReference<TlsManager>> trustManagers = new HashMap<>();
 
-    private final TlsContext tlsContext;
-    private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(new ReloaderThreadFactory());
+    private static TlsManager getOrCreateTrustManager(Path tlsOptionsConfigFile) {
+        synchronized (trustManagers) {
+            WeakReference<TlsManager> tlsManager = trustManagers.get(tlsOptionsConfigFile);
+            if (tlsManager == null || tlsManager.get() == null) {
+                TlsManager manager = new TlsManager(tlsOptionsConfigFile);
+                trustManagers.put(tlsOptionsConfigFile, new WeakReference<>(manager));
+                return manager;
+            }
+            return tlsManager.get();
+        }
+    }
 
     public ConfigFileBasedTlsContext(Path tlsOptionsConfigFile, AuthorizationMode mode) {
         this(tlsOptionsConfigFile, mode, PeerAuthentication.NEED);
@@ -52,56 +50,15 @@ public class ConfigFileBasedTlsContext implements TlsContext {
      * the TLS peer authentication is enforced at a higher protocol or application layer (e.g with {@link PeerAuthentication#WANT}).
      */
     public ConfigFileBasedTlsContext(Path tlsOptionsConfigFile, AuthorizationMode mode, PeerAuthentication peerAuthentication) {
-        TransportSecurityOptions options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile);
-        MutableX509TrustManager trustManager = new MutableX509TrustManager();
-        MutableX509KeyManager keyManager = new MutableX509KeyManager();
-        reloadTrustManager(options, trustManager);
-        reloadKeyManager(options, keyManager);
-        this.tlsContext = createDefaultTlsContext(options, mode, trustManager, keyManager, peerAuthentication);
-        this.scheduler.scheduleAtFixedRate(new CryptoMaterialReloader(tlsOptionsConfigFile, scheduler, trustManager, keyManager),
-                                           UPDATE_PERIOD.getSeconds()/*initial delay*/,
-                                           UPDATE_PERIOD.getSeconds(),
-                                           TimeUnit.SECONDS);
-    }
-
-    private static void reloadTrustManager(TransportSecurityOptions options, MutableX509TrustManager trustManager) {
-        if (options.getCaCertificatesFile().isPresent()) {
-            trustManager.updateTruststore(loadTruststore(options.getCaCertificatesFile().get()));
-        } else {
-            trustManager.useDefaultTruststore();
-        }
-    }
-
-    private static void reloadKeyManager(TransportSecurityOptions options, MutableX509KeyManager keyManager) {
-        if (options.getPrivateKeyFile().isPresent() && options.getCertificatesFile().isPresent()) {
-            keyManager.updateKeystore(loadKeystore(options.getPrivateKeyFile().get(), options.getCertificatesFile().get()), new char[0]);
-        } else {
-            keyManager.useDefaultKeystore();
-        }
+        tlsManager = getOrCreateTrustManager(tlsOptionsConfigFile);
+        this.tlsContext = createDefaultTlsContext(tlsManager.getOptions(), mode, tlsManager.getTrustManager(), tlsManager.getKeyManager(), peerAuthentication);
     }
 
-    private static KeyStore loadTruststore(Path caCertificateFile) {
-        try {
-            return KeyStoreBuilder.withType(KeyStoreType.PKCS12)
-                    .withCertificateEntries("cert", X509CertificateUtils.certificateListFromPem(com.yahoo.vespa.jdk8compat.Files.readString(caCertificateFile)))
-                    .build();
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-    }
-
-    private static KeyStore loadKeystore(Path privateKeyFile, Path certificatesFile) {
-        try {
-            return KeyStoreBuilder.withType(KeyStoreType.PKCS12)
-                    .withKeyEntry(
-                            "default",
-                            KeyUtils.fromPemEncodedPrivateKey(com.yahoo.vespa.jdk8compat.Files.readString(privateKeyFile)),
-                            X509CertificateUtils.certificateListFromPem(com.yahoo.vespa.jdk8compat.Files.readString(certificatesFile)))
-                    .build();
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-    }
+    // Wrapped methods from TlsContext
+    @Override public SSLContext context() { return tlsContext.context(); }
+    @Override public SSLParameters parameters() { return tlsContext.parameters(); }
+    @Override public SSLEngine createSslEngine() { return tlsContext.createSslEngine(); }
+    @Override public SSLEngine createSslEngine(String peerHost, int peerPort) { return tlsContext.createSslEngine(peerHost, peerPort); }
 
     private static DefaultTlsContext createDefaultTlsContext(TransportSecurityOptions options,
                                                              AuthorizationMode mode,
@@ -122,73 +79,4 @@ public class ConfigFileBasedTlsContext implements TlsContext {
         return new DefaultTlsContext(sslContext, ciphers, peerAuthentication);
     }
 
-    // Wrapped methods from TlsContext
-    @Override public SSLContext context() { return tlsContext.context(); }
-    @Override public SSLParameters parameters() { return tlsContext.parameters(); }
-    @Override public SSLEngine createSslEngine() { return tlsContext.createSslEngine(); }
-    @Override public SSLEngine createSslEngine(String peerHost, int peerPort) { return tlsContext.createSslEngine(peerHost, peerPort); }
-
-    @Override
-    public void close() {
-        try {
-            scheduler.shutdownNow();
-            if (!scheduler.awaitTermination(10, TimeUnit.SECONDS)) {
-                throw new RuntimeException("Unable to shutdown executor before timeout");
-            }
-        } catch (InterruptedException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
-    // Note: no reference to outer class (directly or indirectly) to ensure trust/key managers are eventually GCed once
-    // there are no more use of the outer class and the underlying SSLContext
-    private static class CryptoMaterialReloader implements Runnable {
-
-        final Path tlsOptionsConfigFile;
-        final ScheduledExecutorService scheduler;
-        final WeakReference<MutableX509TrustManager> trustManager;
-        final WeakReference<MutableX509KeyManager> keyManager;
-
-        CryptoMaterialReloader(Path tlsOptionsConfigFile,
-                               ScheduledExecutorService scheduler,
-                               MutableX509TrustManager trustManager,
-                               MutableX509KeyManager keyManager) {
-            this.tlsOptionsConfigFile = tlsOptionsConfigFile;
-            this.scheduler = scheduler;
-            this.trustManager = new WeakReference<>(trustManager);
-            this.keyManager = new WeakReference<>(keyManager);
-        }
-
-        @Override
-        public void run() {
-            try {
-                MutableX509TrustManager trustManager = this.trustManager.get();
-                MutableX509KeyManager keyManager = this.keyManager.get();
-                if (trustManager == null && keyManager == null) {
-                    scheduler.shutdown();
-                    return;
-                }
-                TransportSecurityOptions options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile);
-                if (trustManager != null) {
-                    reloadTrustManager(options, trustManager);
-                }
-                if (keyManager != null) {
-                    reloadKeyManager(options, keyManager);
-                }
-            } catch (Throwable t) {
-                log.log(Level.SEVERE, String.format("Failed to reload crypto material (path='%s'): %s", tlsOptionsConfigFile, t.getMessage()), t);
-            }
-        }
-    }
-
-    // Static class to ensure no reference to outer class is contained
-    private static class ReloaderThreadFactory implements ThreadFactory {
-        @Override
-        public Thread newThread(Runnable r) {
-            Thread thread = new Thread(r, "tls-context-reloader");
-            thread.setDaemon(true);
-            return thread;
-        }
-    }
-
 }
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/TlsManager.java b/security-utils/src/main/java/com/yahoo/security/tls/TlsManager.java
new file mode 100644
index 00000000000..bade1993982
--- /dev/null
+++ b/security-utils/src/main/java/com/yahoo/security/tls/TlsManager.java
@@ -0,0 +1,139 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.security.tls;
+
+import com.yahoo.security.KeyStoreBuilder;
+import com.yahoo.security.KeyStoreType;
+import com.yahoo.security.KeyUtils;
+import com.yahoo.security.X509CertificateUtils;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.ref.WeakReference;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.time.Duration;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+class TlsManager {
+    private static final Logger log = Logger.getLogger(TlsManager.class.getName());
+    private static final Duration UPDATE_PERIOD = Duration.ofHours(1);
+
+    private final MutableX509TrustManager trustManager;
+    private final MutableX509KeyManager keyManager;
+    private final ScheduledExecutorService scheduler;
+    private TransportSecurityOptions options;
+
+    private static void reloadTrustManager(TransportSecurityOptions options, MutableX509TrustManager trustManager) {
+        if (options.getCaCertificatesFile().isPresent()) {
+            trustManager.updateTruststore(loadTruststore(options.getCaCertificatesFile().get()));
+        } else {
+            trustManager.useDefaultTruststore();
+        }
+    }
+
+    private static void reloadKeyManager(TransportSecurityOptions options, MutableX509KeyManager keyManager) {
+        if (options.getPrivateKeyFile().isPresent() && options.getCertificatesFile().isPresent()) {
+            keyManager.updateKeystore(loadKeystore(options.getPrivateKeyFile().get(), options.getCertificatesFile().get()), new char[0]);
+        } else {
+            keyManager.useDefaultKeystore();
+        }
+    }
+    private static KeyStore loadTruststore(Path caCertificateFile) {
+        try {
+            return KeyStoreBuilder.withType(KeyStoreType.PKCS12)
+                    .withCertificateEntries("cert", X509CertificateUtils.certificateListFromPem(com.yahoo.vespa.jdk8compat.Files.readString(caCertificateFile)))
+                    .build();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
+    private static KeyStore loadKeystore(Path privateKeyFile, Path certificatesFile) {
+        try {
+            return KeyStoreBuilder.withType(KeyStoreType.PKCS12)
+                    .withKeyEntry(
+                            "default",
+                            KeyUtils.fromPemEncodedPrivateKey(com.yahoo.vespa.jdk8compat.Files.readString(privateKeyFile)),
+                            X509CertificateUtils.certificateListFromPem(com.yahoo.vespa.jdk8compat.Files.readString(certificatesFile)))
+                    .build();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+    // Note: no reference to outer class (directly or indirectly) to ensure trust/key managers are eventually GCed once
+    // there are no more use of the outer class and the underlying SSLContext
+    private static class CryptoMaterialReloader implements Runnable {
+
+        private final Path tlsOptionsConfigFile;
+        private final ScheduledExecutorService scheduler;
+        private final WeakReference<TlsManager> tlsManager;
+
+        CryptoMaterialReloader(Path tlsOptionsConfigFile,
+                               ScheduledExecutorService scheduler,
+                               TlsManager tlsManager) {
+            this.tlsOptionsConfigFile = tlsOptionsConfigFile;
+            this.scheduler = scheduler;
+            this.tlsManager = new WeakReference<>(tlsManager);
+        }
+
+        @Override
+        public void run() {
+            try {
+                TlsManager tlsManager = this.tlsManager.get();
+                if (tlsManager == null) {
+                    scheduler.shutdown();
+                    return;
+                }
+                TransportSecurityOptions options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile);
+                reloadTrustManager(options, tlsManager.getTrustManager());
+                MutableX509KeyManager keyManager = tlsManager.getKeyManager();
+                reloadKeyManager(options, keyManager);
+            } catch (Throwable t) {
+                log.log(Level.SEVERE, String.format("Failed to reload crypto material (path='%s'): %s", tlsOptionsConfigFile, t.getMessage()), t);
+            }
+        }
+    }
+
+    // Static class to ensure no reference to outer class is contained
+    private static class ReloaderThreadFactory implements ThreadFactory {
+        Path fileName;
+        ReloaderThreadFactory(Path fileName) {
+            this.fileName = fileName;
+        }
+        @Override
+        public Thread newThread(Runnable r) {
+            Thread thread = new Thread(r, "tls-context-reloader:" + fileName.toString() );
+            thread.setDaemon(true);
+            return thread;
+        }
+    }
+
+    TlsManager(Path tlsOptionsConfigFile) {
+        trustManager = new MutableX509TrustManager();
+        keyManager = new MutableX509KeyManager();
+        options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile);
+        reloadTrustManager(options, trustManager);
+        reloadKeyManager(options, keyManager);
+        scheduler = Executors.newSingleThreadScheduledExecutor(new ReloaderThreadFactory(tlsOptionsConfigFile));
+        this.scheduler.scheduleAtFixedRate(new CryptoMaterialReloader(tlsOptionsConfigFile, scheduler, this),
+                UPDATE_PERIOD.getSeconds()/*initial delay*/,
+                UPDATE_PERIOD.getSeconds(),
+                TimeUnit.SECONDS);
+    }
+    MutableX509TrustManager getTrustManager() {
+        return trustManager;
+    }
+
+    MutableX509KeyManager getKeyManager() {
+        return keyManager;
+    }
+
+    TransportSecurityOptions getOptions() {
+        return options;
+    }
+}