Use a single, shared TlsContext instance

The configuration is based on environment variables, which are effectively fixed through the life of the JVM instance.
This simplifaction removes the need for complex cleanup logic based on manual reference counting and weak references.

2 files changed, 31 insertions, 18 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..b061d2f9165 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
@@ -14,7 +14,6 @@ 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;
@@ -140,14 +139,12 @@ public class ConfigFileBasedTlsContext implements TlsContext {
         }
     }
 
-    // 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;
+        final MutableX509TrustManager trustManager;
+        final MutableX509KeyManager keyManager;
 
         CryptoMaterialReloader(Path tlsOptionsConfigFile,
                                ScheduledExecutorService scheduler,
@@ -155,25 +152,23 @@ public class ConfigFileBasedTlsContext implements TlsContext {
                                MutableX509KeyManager keyManager) {
             this.tlsOptionsConfigFile = tlsOptionsConfigFile;
             this.scheduler = scheduler;
-            this.trustManager = new WeakReference<>(trustManager);
-            this.keyManager = new WeakReference<>(keyManager);
+            this.trustManager = trustManager;
+            this.keyManager = keyManager;
         }
 
         @Override
         public void run() {
             try {
-                MutableX509TrustManager trustManager = this.trustManager.get();
-                MutableX509KeyManager keyManager = this.keyManager.get();
-                if (trustManager == null && keyManager == null) {
+                if (this.trustManager == null && this.keyManager == null) {
                     scheduler.shutdown();
                     return;
                 }
                 TransportSecurityOptions options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile);
-                if (trustManager != null) {
-                    reloadTrustManager(options, trustManager);
+                if (this.trustManager != null) {
+                    reloadTrustManager(options, this.trustManager);
                 }
-                if (keyManager != null) {
-                    reloadKeyManager(options, keyManager);
+                if (this.keyManager != null) {
+                    reloadKeyManager(options, this.keyManager);
                 }
             } catch (Throwable t) {
                 log.log(Level.SEVERE, String.format("Failed to reload crypto material (path='%s'): %s", tlsOptionsConfigFile, t.getMessage()), t);
@@ -181,7 +176,6 @@ public class ConfigFileBasedTlsContext implements TlsContext {
         }
     }
 
-    // Static class to ensure no reference to outer class is contained
     private static class ReloaderThreadFactory implements ThreadFactory {
         @Override
         public Thread newThread(Runnable r) {
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityUtils.java b/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityUtils.java
index f28cad2a071..af77827ae16 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityUtils.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityUtils.java
@@ -13,6 +13,8 @@ import java.util.Optional;
  */
 public class TransportSecurityUtils {
 
+    private static ConfigFileBasedTlsContext systemTlsContext;
+
     public static final String CONFIG_FILE_ENVIRONMENT_VARIABLE = "VESPA_TLS_CONFIG_FILE";
     public static final String INSECURE_MIXED_MODE_ENVIRONMENT_VARIABLE = "VESPA_TLS_INSECURE_MIXED_MODE";
     public static final String INSECURE_AUTHORIZATION_MODE_ENVIRONMENT_VARIABLE = "VESPA_TLS_INSECURE_AUTHORIZATION_MODE";
@@ -64,13 +66,30 @@ public class TransportSecurityUtils {
                 .map(TransportSecurityOptions::fromJsonFile);
     }
 
-    public static Optional<TlsContext> createTlsContext() {
-        return getConfigFile()
-                .map(configFile -> new ConfigFileBasedTlsContext(configFile, getInsecureAuthorizationMode()));
+    /**
+     * @return The shared {@link TlsContext} for the Vespa system environment
+     */
+    public static Optional<TlsContext> getSystemTlsContext() {
+        synchronized (TransportSecurityUtils.class) {
+            Path configFile = getConfigFile().orElse(null);
+            if (configFile == null) return Optional.empty();
+            if (systemTlsContext == null) {
+                systemTlsContext = new SystemTlsContext(configFile);
+            }
+            return Optional.of(systemTlsContext);
+        }
     }
 
     private static Optional<String> getEnvironmentVariable(Map<String, String> environmentVariables, String variableName) {
         return Optional.ofNullable(environmentVariables.get(variableName))
                 .filter(var -> !var.isEmpty());
     }
+
+    private static class SystemTlsContext extends ConfigFileBasedTlsContext {
+        SystemTlsContext(Path tlsOptionsConfigFile) {
+            super(tlsOptionsConfigFile, getInsecureAuthorizationMode());
+        }
+
+        @Override public void close() { throw new UnsupportedOperationException("Shared TLS context cannot be closed"); }
+    }
 }