fix: reload auth key on change

development/.scripts/build.sh

@@ -1,14 +1,14 @@
 #!/bin/bash -e
 set -e
 
+SCRIPT_DIR=$(dirname "$(realpath "${BASH_SOURCE[0]}")")
 origin="$(pwd)"
 
 gradleModule=""
-gradleArgs="clean build -xtest -xcheckstyleMain -xcheckstyleTest "
+gradleArgs="clean build -x check "
 case $1 in
 "proxy")
   gradleModule="service/proxy"
-  gradleArgs+="-xintTest"
   ;;
 "configuration-client")
   gradleModule="service/configuration-client"
@@ -21,15 +21,13 @@ case $1 in
   ;;
 "cs-admin-service")
   gradleModule="central-server/admin-service"
-  gradleArgs+="-xintTest"
   ;;
 "cs-management-service")
   gradleModule="central-server/management-service"
-  gradleArgs+="-xintTest"
   ;;
 esac
 
-cd ../../src/
+cd "$SCRIPT_DIR"/../../src/
 set -o xtrace
 ./gradlew $gradleArgs -p $gradleModule
 set +o xtrace

src/common/common-core/src/main/java/ee/ria/xroad/common/util/filewatcher/FileWatcherRunner.java

@@ -40,7 +40,7 @@
 /**
  * A class that runs {@link FileWatcher} in a thread and stops is upon request.
  */
-public final class FileWatcherRunner implements AutoCloseable {
+public final class FileWatcherRunner {
 
     private final ExecutorService executor;
     private final FileWatcher watcher;
@@ -161,10 +161,4 @@ public void stop() {
         // use shutdown to interrupt the thread because it's probably waiting for events.
         executor.shutdownNow();
     }
-
-    @Override
-    public void close() throws Exception {
-        stop();
-    }
-
 }

src/lib/keyconf-impl/src/main/java/org/niis/xroad/keyconf/impl/CachingKeyConfImpl.java

@@ -31,10 +31,13 @@
 import ee.ria.xroad.common.identifier.ClientId;
 import ee.ria.xroad.common.identifier.SecurityServerId;
 import ee.ria.xroad.common.util.FileContentChangeChecker;
+import ee.ria.xroad.common.util.filewatcher.FileWatchListener;
 import ee.ria.xroad.common.util.filewatcher.FileWatcherRunner;
+import ee.ria.xroad.common.util.filewatcher.FileWatcherStartupListener;
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
+import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 import org.bouncycastle.cert.ocsp.OCSPResp;
 import org.niis.xroad.globalconf.GlobalConfProvider;
@@ -45,7 +48,6 @@
 import org.niis.xroad.serverconf.ServerConfProvider;
 import org.niis.xroad.signer.client.SignerRpcClient;
 
-import java.lang.ref.WeakReference;
 import java.nio.file.Paths;
 import java.security.PrivateKey;
 import java.util.Date;
@@ -66,7 +68,7 @@ public class CachingKeyConfImpl extends KeyConfImpl {
     private final Cache<SecurityServerId, AuthKeyInfo> authKeyInfoCache;
     private FileWatcherRunner keyConfChangeWatcher;
 
-    CachingKeyConfImpl(GlobalConfProvider globalConfProvider, ServerConfProvider serverConfProvider,
+    public CachingKeyConfImpl(GlobalConfProvider globalConfProvider, ServerConfProvider serverConfProvider,
                        SignerRpcClient signerRpcClient) {
         super(globalConfProvider, serverConfProvider, signerRpcClient);
         signingInfoCache = CacheBuilder.newBuilder()
@@ -106,7 +108,7 @@ public SigningInfo getSigningInfo(ClientId clientId) {
     /**
      * Invalidates both auth key and signing info caches
      */
-    protected void invalidateCaches() {
+    public void invalidateCaches() {
         authKeyInfoCache.invalidateAll();
         signingInfoCache.invalidateAll();
     }
@@ -159,52 +161,28 @@ protected void watcherStarted() {
         //for testability
     }
 
-    /**
-     * Create a new CachingKeyConf instance and set up keyconf change watcher.
-     */
-    public static CachingKeyConfImpl newInstance(GlobalConfProvider globalConfProvider, ServerConfProvider serverConfProvider,
-                                                SignerRpcClient signerRpcClient)
-            throws Exception {
-        final FileContentChangeChecker changeChecker = new FileContentChangeChecker(SystemProperties.getKeyConfFile());
-        final CachingKeyConfImpl instance = new CachingKeyConfImpl(globalConfProvider, serverConfProvider, signerRpcClient);
-        // the change watcher can not be created in the constructor, because that would publish the
-        // instance reference to another thread before the constructor finishes.
-        instance.keyConfChangeWatcher = createChangeWatcher(new WeakReference<>(instance), changeChecker);
-        return instance;
+    @SneakyThrows
+    public static FileWatcherRunner createChangeWatcher(FileWatchListener onChange) {
+        return createChangeWatcher(() -> { }, onChange, new FileContentChangeChecker(SystemProperties.getKeyConfFile()));
     }
 
-    /* Implementation note:
-     * Weak reference for the callback is used so that CachingKeyConf instance can be garbage collected
-     * (e.g. after KeyConf reload). Otherwise, the FileWatcher background thread keeps it alive and creates a leak
-     * if one fails to call destroy.
-     */
-    static FileWatcherRunner createChangeWatcher(WeakReference<CachingKeyConfImpl> ref,
+    static FileWatcherRunner createChangeWatcher(FileWatcherStartupListener onStart,
+                                                 FileWatchListener onChange,
                                                  FileContentChangeChecker changeChecker) {
         return FileWatcherRunner.create()
                 .watchForChangesIn(Paths.get(changeChecker.getFileName()))
                 .listenToCreate()
                 .listenToModify()
                 .andOnChangeNotify(() -> {
-                    final CachingKeyConfImpl conf = ref.get();
-                    if (conf == null) {
-                        //stop watcher since the CachingKeyConf has become garbage
-                        Thread.currentThread().interrupt();
-                        return;
-                    }
                     boolean changed = true;
                     try {
                         changed = changeChecker.hasChanged();
                     } catch (Exception e) {
                         log.error("Failed to check if key conf has changed", e);
                     }
-                    if (changed) conf.invalidateCaches();
-                })
-                .andOnStartupNotify(() -> {
-                    final CachingKeyConfImpl conf = ref.get();
-                    if (conf != null) {
-                        conf.watcherStarted();
-                    }
+                    if (changed) onChange.fileModified();
                 })
+                .andOnStartupNotify(onStart)
                 .buildAndStartWatcher();
     }
 }

src/lib/keyconf-impl/src/test/java/org/niis/xroad/keyconf/impl/CachingKeyConfImplTest.java

@@ -47,7 +47,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.lang.ref.WeakReference;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -180,9 +179,13 @@ public void testAuthKeyReadsWithChangedKeyConf() throws Exception {
                 VALID_AUTH_KEY,
                 VALID_SIGNING_INFO,
                 NO_DELAY);
-        try (FileWatcherRunner unused = CachingKeyConfImpl.createChangeWatcher(
-                new WeakReference<>(testCachingKeyConf),
-                new TestChangeChecker(keyConfHasChanged))) {
+        FileWatcherRunner fileWatcherRunner = null;
+        try {
+            fileWatcherRunner = CachingKeyConfImpl.createChangeWatcher(
+                    testCachingKeyConf::watcherStarted,
+                    testCachingKeyConf::invalidateCaches,
+                    new TestChangeChecker(keyConfHasChanged)
+            );
 
             testCachingKeyConf.ready.await();
 
@@ -201,6 +204,10 @@ public void testAuthKeyReadsWithChangedKeyConf() throws Exception {
 
             expectedCacheHits++;
             assertEquals(expectedCacheHits, callsToGetAuthKeyInfo.get());
+        } finally {
+            if (fileWatcherRunner != null) {
+                fileWatcherRunner.stop();
+            }
         }
     }
 
@@ -485,7 +492,7 @@ protected void watcherStarted() {
         }
 
         @Override
-        protected void invalidateCaches() {
+        public void invalidateCaches() {
             super.invalidateCaches();
             changed.countDown();
         }

src/service/proxy/proxy-core/src/intTest/java/org/niis/xroad/proxy/test/glue/ProxyStepDefs.java

@@ -211,14 +211,14 @@ private void exec(String client, int count, int threads) throws InterruptedExcep
     private KeyConfProvider createKeyConf() {
         var globalConf = new TestGlobalConfImpl();
         var serverConf = new ServerConfImpl(globalConf);
-        return CachingKeyConfImpl.newInstance(globalConf, serverConf, signerRpcClient);
+        return new CachingKeyConfImpl(globalConf, serverConf, signerRpcClient);
     }
 
     @SneakyThrows
     private SigningCtxProvider createSigningCtxProvider() {
         var globalConf = new TestGlobalConfImpl();
         var serverConf = new ServerConfImpl(globalConf);
-        var keyconf = CachingKeyConfImpl.newInstance(globalConf, serverConf, signerRpcClient);
+        var keyconf = new CachingKeyConfImpl(globalConf, serverConf, signerRpcClient);
 
         return new SigningCtxProviderImpl(globalConf, keyconf, new BatchSigner(signerRpcClient));
     }

src/service/proxy/proxy-core/src/main/java/org/niis/xroad/proxy/core/auth/AuthKeyChangeManager.java

@@ -0,0 +1,72 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2019- Nordic Institute for Interoperability Solutions (NIIS)
+ * Copyright (c) 2018 Estonian Information System Authority (RIA),
+ * Nordic Institute for Interoperability Solutions (NIIS), Population Register Centre (VRK)
+ * Copyright (c) 2015-2017 Estonian Information System Authority (RIA), Population Register Centre (VRK)
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.niis.xroad.proxy.core.auth;
+
+import ee.ria.xroad.common.util.filewatcher.FileWatcherRunner;
+
+import lombok.extern.slf4j.Slf4j;
+import org.niis.xroad.keyconf.KeyConfProvider;
+import org.niis.xroad.keyconf.impl.CachingKeyConfImpl;
+import org.niis.xroad.proxy.core.clientproxy.ClientProxy;
+import org.niis.xroad.proxy.core.serverproxy.ServerProxy;
+import org.springframework.beans.factory.DisposableBean;
+import org.springframework.beans.factory.InitializingBean;
+
+@Slf4j
+public class AuthKeyChangeManager implements InitializingBean, DisposableBean {
+    private final KeyConfProvider keyConfProvider;
+    private final ClientProxy clientProxy;
+    private final ServerProxy serverProxy;
+    private FileWatcherRunner changeWatcher;
+
+    public AuthKeyChangeManager(KeyConfProvider keyConfProvider, ClientProxy clientProxy, ServerProxy serverProxy) {
+        this.keyConfProvider = keyConfProvider;
+        this.clientProxy = clientProxy;
+        this.serverProxy = serverProxy;
+    }
+
+    @Override
+    public void afterPropertiesSet() {
+        changeWatcher = CachingKeyConfImpl.createChangeWatcher(this::onAuthKeyChange);
+    }
+
+    private void onAuthKeyChange() {
+        log.debug("Authentication key change detected, reloading key.");
+        if (keyConfProvider instanceof CachingKeyConfImpl cachingKeyConf) {
+            cachingKeyConf.invalidateCaches();
+        }
+        clientProxy.reloadAuthKey();
+        serverProxy.reloadAuthKey();
+    }
+
+    @Override
+    public void destroy() throws Exception {
+        if (changeWatcher != null) {
+            changeWatcher.stop();
+        }
+    }
+}

src/service/proxy/proxy-core/src/main/java/org/niis/xroad/proxy/core/clientproxy/ClientProxy.java

@@ -58,7 +58,6 @@
 import org.niis.xroad.keyconf.KeyConfProvider;
 import org.niis.xroad.proxy.core.serverproxy.IdleConnectionMonitorThread;
 import org.niis.xroad.proxy.core.util.CommonBeanProxy;
-import org.niis.xroad.proxy.core.util.SSLContextUtil;
 import org.niis.xroad.serverconf.ServerConfProvider;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
@@ -105,6 +104,7 @@ public class ClientProxy implements InitializingBean, DisposableBean {
 
     private CloseableHttpClient client;
     private IdleConnectionMonitorThread connectionMonitor;
+    private ReloadingSSLSocketFactory reloadingSSLSocketFactory;
 
     /**
      * Constructs and configures a new client proxy.
@@ -199,8 +199,8 @@ private HttpClientConnectionManager getClientConnectionManager() throws Exceptio
     }
 
     private SSLConnectionSocketFactory createSSLSocketFactory() throws Exception {
-        return new FastestConnectionSelectingSSLSocketFactory(authTrustVerifier,
-                SSLContextUtil.createXroadSSLContext(globalConfProvider, keyConfProvider));
+        reloadingSSLSocketFactory = new ReloadingSSLSocketFactory(globalConfProvider, keyConfProvider);
+        return new FastestConnectionSelectingSSLSocketFactory(authTrustVerifier, reloadingSSLSocketFactory);
     }
 
     private void createConnectors() throws Exception {
@@ -331,6 +331,11 @@ public void destroy() throws Exception {
         HibernateUtil.closeSessionFactories();
     }
 
+    public void reloadAuthKey() {
+        log.trace("reloadAuthKey()");
+        reloadingSSLSocketFactory.reload();
+    }
+
     private static final class ClientSslTrustManager implements X509TrustManager {
 
         @Override

src/service/proxy/proxy-core/src/main/java/org/niis/xroad/proxy/core/clientproxy/FastestConnectionSelectingSSLSocketFactory.java

@@ -39,8 +39,8 @@
 import org.niis.xroad.proxy.core.clientproxy.FastestSocketSelector.SocketInfo;
 
 import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocket;
+import javax.net.ssl.SSLSocketFactory;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -83,15 +83,15 @@ class FastestConnectionSelectingSSLSocketFactory
     public static final int CACHE_MAXIMUM_SIZE = 10000;
 
     private final AuthTrustVerifier authTrustVerifier;
-    private final javax.net.ssl.SSLSocketFactory socketfactory;
+    private final SSLSocketFactory socketfactory;
 
     private final Cache<CacheKey, URI> selectedHosts;
     private final boolean cachingEnabled;
 
-    FastestConnectionSelectingSSLSocketFactory(AuthTrustVerifier authTrustVerifier, SSLContext sslContext) {
-        super(sslContext, null, SystemProperties.getXroadTLSCipherSuites(), (HostnameVerifier) null);
+    FastestConnectionSelectingSSLSocketFactory(AuthTrustVerifier authTrustVerifier, SSLSocketFactory socketfactory) {
+        super(socketfactory, null, SystemProperties.getXroadTLSCipherSuites(), (HostnameVerifier) null);
         this.authTrustVerifier = authTrustVerifier;
-        this.socketfactory = sslContext.getSocketFactory();
+        this.socketfactory = socketfactory;
         this.selectedHosts = CacheBuilder.newBuilder()
                 .expireAfterWrite(SystemProperties.getClientProxyFastestConnectingSslUriCachePeriod(), TimeUnit.SECONDS)
                 .maximumSize(CACHE_MAXIMUM_SIZE)