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

This reverts commit c58415566e23dcac5f0daa352f39f567a4d7b44f.

2 files changed, 135 insertions, 162 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 26dfbf9fd9f..28854c59b2c 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,20 +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.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.util.HashMap;
+import java.security.KeyStore;
+import java.time.Duration;
 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.
@@ -24,22 +36,12 @@ import java.util.Set;
  */
 public class ConfigFileBasedTlsContext implements TlsContext {
 
-    private final TlsContext tlsContext;
-    private TlsManager tlsManager;
+    private static final Duration UPDATE_PERIOD = Duration.ofHours(1);
 
-    static private final Map<Path, WeakReference<TlsManager>> trustManagers = new HashMap<>();
+    private static final Logger log = Logger.getLogger(ConfigFileBasedTlsContext.class.getName());
 
-    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();
-        }
-    }
+    private final TlsContext tlsContext;
+    private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(new ReloaderThreadFactory());
 
     public ConfigFileBasedTlsContext(Path tlsOptionsConfigFile, AuthorizationMode mode) {
         this(tlsOptionsConfigFile, mode, PeerAuthentication.NEED);
@@ -50,15 +52,56 @@ 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) {
-        tlsManager = getOrCreateTrustManager(tlsOptionsConfigFile);
-        this.tlsContext = createDefaultTlsContext(tlsManager.getOptions(), mode, tlsManager.getTrustManager(), tlsManager.getKeyManager(), 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);
     }
 
-    // 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 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);
+        }
+    }
 
     private static DefaultTlsContext createDefaultTlsContext(TransportSecurityOptions options,
                                                              AuthorizationMode mode,
@@ -79,4 +122,73 @@ 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
deleted file mode 100644
index bade1993982..00000000000
--- a/security-utils/src/main/java/com/yahoo/security/tls/TlsManager.java
+++ /dev/null
@@ -1,139 +0,0 @@
-// 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;
-    }
-}