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/common/common-test/src/test/certs/consumer-2.pem

@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIDtDCCApygAwIBAgIBBTANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJFRTEU
+MBIGA1UEChMLQ3liZXJuZXRpY2ExDDAKBgNVBAsTA0lUTzEOMAwGA1UEAxMFQ3li
+ZXIxGjAYBgkqhkiG9w0BCQEWC2FhYUBiYmIuY2NjMB4XDTE5MDMwNjA5NDYwMFoX
+DTM1MDMwNDA3NDczN1owYDELMAkGA1UEBhMCRUUxFDASBgNVBAoTC0N5YmVybmV0
+aWNhMQwwCgYDVQQLEwNJVE8xETAPBgNVBAMTCGNvbnN1bWVyMRowGAYJKoZIhvcN
+AQkBFgthYWFAYmJiLmNjYzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
+ANhZDGeLz2a+Cly2Hmy4bOUnHCPJfQ89B8Utuk7NsWTHA6ghc1/xqeaWs2JXSsbQ
+RbD6KLJ5R06p9frnuwnr5pVyMmVDywffOoJvKitj7HjdFHvzcvJmiQVlqFigrNiv
+uhHHjC+KI0IOnd58p553fsmSiemcb4uptgWOY2ipaJqtSF8S40ZG5oxxjQ4J+R3l
+28dfz9wdwMThPJuiS1tuI+QpqItVLZSzj71cWZnm6KHwviw6otDkX4pFhA7UaGMA
+pXgskQUlXV+r0Tk+RpZGCNVkjEmZ5TmyWAufY4pPPQAo1xcbKA+gtM0oFT+6HbqP
+vBEw2W6rMNZXlhe4FSmSsf0CAwEAAaN8MHowCQYDVR0TBAIwADAOBgNVHQ8BAf8E
+BAMCA7gwHQYDVR0lBBYwFAYIKwYBBQUHAwIGCCsGAQUFBwMBMB0GA1UdDgQWBBSe
+2dgkXuH4vjqm+4G1o19z4bSmxTAfBgNVHSMEGDAWgBR+egbuTBG1XAUq40xvTvPK
+ht3y3jANBgkqhkiG9w0BAQsFAAOCAQEAUdL6faDHNnDvMFQllS1T02t0iivmpKIn
+rM+f08jjH9YdTk75/o7VGmiNkpfzVdZ/F+YCVqQRx0cB5+EC/PacA4+VkM3qpb+H
+sZNXlFvEuSWFvVx1U9lM04vvW/1Ba/ldNzkBgS/9l898BSRzziU4QIIaIhhMJPfO
+DrSUfR1Zu38L0uP4QRBWA7iR3ZUNsRlPl+S/kruspPf38qs3XlCDGiZgZjvEXKAt
+F/Hbn2w5gkwJocEuwdEswkqj5b6bIl505iHYGJzs9IxGdSp4DVY3A7Lls9ZRfd80
+7Ta9y2TbaVdNk2tjNM2OlA7qwUl/Vl2nxfc+PwOrNA8Pp3TmtGHd3w==
+-----END CERTIFICATE-----

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/lib/keyconf-impl/src/testFixtures/java/org/niis/xroad/test/keyconf/TestKeyConf.java

@@ -33,6 +33,7 @@
 import ee.ria.xroad.common.util.TimeUtils;
 
 import lombok.RequiredArgsConstructor;
+import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 import org.bouncycastle.cert.ocsp.CertificateStatus;
 import org.bouncycastle.cert.ocsp.OCSPResp;
@@ -60,7 +61,9 @@
 public class TestKeyConf extends EmptyKeyConf {
     private final GlobalConfProvider globalConfProvider;
 
-    Map<String, OCSPResp> ocspResponses = new HashMap<>();
+    private final Map<String, OCSPResp> ocspResponses = new HashMap<>();
+    @Setter
+    private PKCS12 authKey = TestCertUtil.getConsumer();
 
     @Override
     public SigningInfo getSigningInfo(ClientId clientId) {
@@ -69,9 +72,8 @@ public SigningInfo getSigningInfo(ClientId clientId) {
 
     @Override
     public AuthKey getAuthKey() {
-        PKCS12 consumer = TestCertUtil.getConsumer();
         return new AuthKey(new CertChainFactory(globalConfProvider)
-                .create("EE", consumer.certChain[0], null), consumer.key);
+                .create("EE", authKey.certChain[0], null), authKey.key);
     }
 
     @Override

src/lib/serverconf-impl/src/testFixtures/java/org/niis/xroad/test/serverconf/TestServerConf.java

@@ -40,32 +40,11 @@
  */
 public class TestServerConf extends EmptyServerConf {
 
-    private int servicePort;
-
-    public TestServerConf(int servicePort) {
-        this.servicePort = servicePort;
-    }
-
     @Override
     public SecurityServerId.Conf getIdentifier() {
         return SecurityServerId.Conf.create("EE", "BUSINESS", "consumer", "proxytest");
     }
 
-    @Override
-    public String getServiceAddress(ServiceId service) {
-        return "http://127.0.0.1:" + servicePort;
-    }
-
-    @Override
-    public boolean serviceExists(ServiceId service) {
-        return true;
-    }
-
-    @Override
-    public String getDisabledNotice(ServiceId service) {
-        return null;
-    }
-
     @Override
     public InternalSSLKey getSSLKey() {
         PKCS12 internal = TestCertUtil.getInternalKey();

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/AuthTrustVerifier.java

@@ -76,7 +76,7 @@ public class AuthTrustVerifier {
     private final CertHelper certHelper;
     private final CertChainFactory certChainFactory;
 
-    void verify(HttpContext context, SSLSession sslSession,
+    protected void verify(HttpContext context, SSLSession sslSession,
                 URI selectedAddress) {
         log.debug("verify()");