Issue #8973 - Rework KeyStoreScanner handling for symlink related changes (#9014)

* Issue #8973 - Rework KeyStoreScanner handling for symlink related changes

+ Removed changes from #8786 and #8787
+ More test cases
+ revert jetty.sslContext.reload.followLinks boolean
+ Scanner should follow its own linkOptions setting
+ remove bad documentation in module-ssl-reload.adoc

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Co-authored-by: Lachlan Roberts <lachlan@webtide.com>

documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc

@@ -22,6 +22,3 @@ The module properties are:
 ----
 include::{JETTY_HOME}/modules/ssl-reload.mod[tags=documentation]
 ----
-
-The `followLinks` property is used to specify whether symlinks should be resolved in the path of the KeyStore.
-If set to false and the path of the KeyStore is a symbolic link, the scanner will monitor the symbolic link file for changes instead of its target.

jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml

@@ -5,7 +5,6 @@
     <Arg>
       <New id="keyStoreScanner" class="org.eclipse.jetty.util.ssl.KeyStoreScanner">
         <Arg><Ref refid="sslContextFactory"/></Arg>
-        <Arg type="boolean"><Property name="jetty.sslContext.reload.followLinks" default="true"/></Arg>
         <Set name="scanInterval"><Property name="jetty.sslContext.reload.scanInterval" default="1"/></Set>
       </New>
     </Arg>

jetty-server/src/main/config/modules/ssl-reload.mod

@@ -15,7 +15,4 @@ etc/jetty-ssl-context-reload.xml
 # tag::documentation[]
 # Monitored directory scan period, in seconds.
 # jetty.sslContext.reload.scanInterval=1
-
-# Whether to resolve symbolic links in the KeyStore path.
-# jetty.sslContext.reload.followLinks=true
 # end::documentation[]

jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java

@@ -422,8 +422,8 @@ public class Scanner extends ContainerLifeCycle
 
         try
         {
-            // Always follow links when check ultimate type of the path
+            // Check status of the real path
-            Path real = path.toRealPath();
+            Path real = path.toRealPath(_linkOptions);
             if (!Files.exists(real) || Files.isDirectory(real))
                 throw new IllegalStateException("Not file or doesn't exist: " + path);
 
@@ -452,7 +452,7 @@ public class Scanner extends ContainerLifeCycle
         try
         {
             // Check status of the real path
-            Path real = p.toRealPath();
+            Path real = p.toRealPath(_linkOptions);
             if (!Files.exists(real) || !Files.isDirectory(real))
                 throw new IllegalStateException("Not directory or doesn't exist: " + p);
 

jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java

@@ -15,6 +15,7 @@ package org.eclipse.jetty.util.ssl;
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
@@ -43,11 +44,6 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
     private final Scanner _scanner;
 
     public KeyStoreScanner(SslContextFactory sslContextFactory)
-    {
-        this(sslContextFactory, true);
-    }
-
-    public KeyStoreScanner(SslContextFactory sslContextFactory, boolean followLinks)
     {
         this.sslContextFactory = sslContextFactory;
         try
@@ -59,12 +55,6 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
             if (monitoredFile.isDirectory())
                 throw new IllegalArgumentException("expected keystore file not directory");
 
-            if (followLinks && keystoreResource.isAlias())
-            {
-                // This resource has an alias, so monitor the target of the alias.
-                monitoredFile = new File(keystoreResource.getAlias());
-            }
-
             keystoreFile = monitoredFile;
             if (LOG.isDebugEnabled())
                 LOG.debug("Monitored Keystore File: {}", monitoredFile);
@@ -78,7 +68,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
         if (!parentFile.exists() || !parentFile.isDirectory())
             throw new IllegalArgumentException("error obtaining keystore dir");
 
-        _scanner = new Scanner(null, followLinks);
+        _scanner = new Scanner(null, false);
         _scanner.addDirectory(parentFile.toPath());
         _scanner.setScanInterval(1);
         _scanner.setReportDirs(false);
@@ -88,11 +78,23 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
         addBean(_scanner);
     }
 
+    private Path getRealKeyStorePath()
+    {
+        try
+        {
+            return keystoreFile.toPath().toRealPath();
+        }
+        catch (IOException e)
+        {
+            return keystoreFile.toPath();
+        }
+    }
+
     @Override
     public void fileAdded(String filename)
     {
         if (LOG.isDebugEnabled())
-            LOG.debug("added {}", filename);
+            LOG.debug("fileAdded {} - keystoreFile.toReal {}", filename, getRealKeyStorePath());
 
         if (keystoreFile.toPath().toString().equals(filename))
             reload();
@@ -102,7 +104,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
     public void fileChanged(String filename)
     {
         if (LOG.isDebugEnabled())
-            LOG.debug("changed {}", filename);
+            LOG.debug("fileChanged {} - keystoreFile.toReal {}", filename, getRealKeyStorePath());
 
         if (keystoreFile.toPath().toString().equals(filename))
             reload();
@@ -112,7 +114,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
     public void fileRemoved(String filename)
     {
         if (LOG.isDebugEnabled())
-            LOG.debug("removed {}", filename);
+            LOG.debug("fileRemoved {} - keystoreFile.toReal {}", filename, getRealKeyStorePath());
 
         if (keystoreFile.toPath().toString().equals(filename))
             reload();

tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java

@@ -18,6 +18,7 @@ import java.net.URL;
 import java.nio.file.FileSystemException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.security.SecureRandom;
 import java.security.cert.Certificate;
@@ -87,11 +88,6 @@ public class KeyStoreScannerTest
     }
 
     public void start(Configuration configuration) throws Exception
-    {
-        start(configuration, true);
-    }
-
-    public void start(Configuration configuration, boolean resolveAlias) throws Exception
     {
         SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
         configuration.configure(sslContextFactory);
@@ -105,7 +101,7 @@ public class KeyStoreScannerTest
         server.addConnector(connector);
 
         // Configure Keystore Reload.
-        keyStoreScanner = new KeyStoreScanner(sslContextFactory, resolveAlias);
+        keyStoreScanner = new KeyStoreScanner(sslContextFactory);
         keyStoreScanner.setScanInterval(0);
         server.addBean(keyStoreScanner);
 
@@ -197,7 +193,7 @@ public class KeyStoreScannerTest
             sslContextFactory.setKeyStorePath(symlinkKeystorePath.toString());
             sslContextFactory.setKeyStorePassword("storepwd");
             sslContextFactory.setKeyManagerPassword("keypwd");
-        }, false);
+        });
 
         // Check the original certificate expiry.
         X509Certificate cert1 = getCertificateFromServer();
@@ -245,6 +241,163 @@ public class KeyStoreScannerTest
         assertThat(getExpiryYear(cert2), is(2020));
     }
 
+    @Test
+    public void testReloadChangingLinkTargetOfSymbolicLink() throws Exception
+    {
+        assumeFileSystemSupportsSymlink();
+        Path oldKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("oldKeyStore");
+        Path newKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("newKeyStore");
+
+        Path sslDir = keystoreDir.resolve("ssl");
+        Path optDir = keystoreDir.resolve("opt");
+        Path optKeystoreLink = optDir.resolve("keystore");
+        Path optKeystore1 = optDir.resolve("keystore.1");
+        Path optKeystore2 = optDir.resolve("keystore.2");
+        Path keystoreFile = sslDir.resolve("keystore");
+
+        start(sslContextFactory ->
+        {
+            // What we want is ..
+            // (link) ssl/keystore -> opt/keystore
+            // (link) opt/keystore -> opt/keystore.1
+            // (file) opt/keystore.1 (actual certificate)
+
+            FS.ensureEmpty(sslDir);
+            FS.ensureEmpty(optDir);
+            Files.copy(oldKeyStoreSrc, optKeystore1);
+            Files.createSymbolicLink(optKeystoreLink, optKeystore1);
+            Files.createSymbolicLink(keystoreFile, optKeystoreLink);
+
+            sslContextFactory.setKeyStorePath(keystoreFile.toString());
+            sslContextFactory.setKeyStorePassword("storepwd");
+            sslContextFactory.setKeyManagerPassword("keypwd");
+        });
+
+        // Check the original certificate expiry.
+        X509Certificate cert1 = getCertificateFromServer();
+        assertThat(getExpiryYear(cert1), is(2015));
+
+        // Create a new keystore file opt/keystore.2 with new expiry
+        Files.copy(newKeyStoreSrc, optKeystore2);
+        // Change (link) opt/keystore -> opt/keystore.2
+        Files.delete(optKeystoreLink);
+        Files.createSymbolicLink(optKeystoreLink, optKeystore2);
+        System.err.println("### Triggering scan");
+        keyStoreScanner.scan(5000);
+
+        // The scanner should have detected the updated keystore, expiry should be renewed.
+        X509Certificate cert2 = getCertificateFromServer();
+        assertThat(getExpiryYear(cert2), is(2020));
+    }
+
+    /**
+     * Test a keystore, where the monitored directory is a symlink.
+     */
+    @Test
+    public void testSymlinkedMonitoredDirectory() throws Exception
+    {
+        assumeFileSystemSupportsSymlink();
+        Path oldKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("oldKeyStore");
+        Path newKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("newKeyStore");
+
+        Path dataLinkDir = keystoreDir.resolve("data_symlink");
+        Path dataDir = keystoreDir.resolve("data");
+        Path etcDir = keystoreDir.resolve("etc");
+        Path dataLinkKeystore = dataLinkDir.resolve("keystore");
+        Path dataKeystore = dataDir.resolve("keystore");
+        Path etcKeystore = etcDir.resolve("keystore");
+
+        start(sslContextFactory ->
+        {
+            // What we want is ..
+            // (link) data_symlink/ -> data/
+            // (link) data/keystore -> etc/keystore
+            // (file) etc/keystore (actual certificate)
+
+            FS.ensureEmpty(etcDir);
+            FS.ensureEmpty(dataDir);
+            Files.copy(oldKeyStoreSrc, etcKeystore);
+            Files.createSymbolicLink(dataLinkDir, dataDir);
+            Files.createSymbolicLink(dataKeystore, etcKeystore);
+
+            sslContextFactory.setKeyStorePath(dataLinkKeystore.toString());
+            sslContextFactory.setKeyStorePassword("storepwd");
+            sslContextFactory.setKeyManagerPassword("keypwd");
+        });
+
+        // Check the original certificate expiry.
+        X509Certificate cert1 = getCertificateFromServer();
+        assertThat(getExpiryYear(cert1), is(2015));
+
+        // Update etc/keystore
+        Files.copy(newKeyStoreSrc, etcKeystore, StandardCopyOption.REPLACE_EXISTING);
+        System.err.println("### Triggering scan");
+        keyStoreScanner.scan(5000);
+
+        // The scanner should have detected the updated keystore, expiry should be renewed.
+        X509Certificate cert2 = getCertificateFromServer();
+        assertThat(getExpiryYear(cert2), is(2020));
+    }
+
+    /**
+     * Test a doubly-linked keystore, and refreshing by only modifying the middle symlink.
+     */
+    @Test
+    public void testDoublySymlinkedTimestampedDir() throws Exception
+    {
+        assumeFileSystemSupportsSymlink();
+        Path oldKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("oldKeyStore");
+        Path newKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("newKeyStore");
+
+        Path sslDir = keystoreDir.resolve("ssl");
+        Path dataDir = sslDir.resolve("data");
+        Path timestampNovDir = sslDir.resolve("2022-11");
+        Path timestampDecDir = sslDir.resolve("2022-12");
+        Path targetNov = timestampNovDir.resolve("keystore.p12");
+        Path targetDec = timestampDecDir.resolve("keystore.p12");
+
+        start(sslContextFactory ->
+        {
+            // What we want is ..
+            // (link) keystore.p12 -> data/keystore.p12
+            // (link) data/ -> 2022-11/
+            // (file) 2022-11/keystore.p12 (actual certificate)
+
+            FS.ensureEmpty(sslDir);
+            FS.ensureEmpty(timestampNovDir);
+            FS.ensureEmpty(timestampDecDir);
+            Files.copy(oldKeyStoreSrc, targetNov);
+            Files.copy(newKeyStoreSrc, targetDec);
+
+            // Create symlink of data/ to 2022-11/
+            Files.createSymbolicLink(dataDir, timestampNovDir.getFileName());
+
+            // Create symlink of keystore.p12 to data/keystore.p12
+            Path keystoreLink = sslDir.resolve("keystore.p12");
+            Files.createSymbolicLink(keystoreLink, Paths.get("data/keystore.p12"));
+
+            sslContextFactory.setKeyStorePath(keystoreLink.toString());
+            sslContextFactory.setKeyStorePassword("storepwd");
+            sslContextFactory.setKeyManagerPassword("keypwd");
+        });
+
+        // Check the original certificate expiry.
+        X509Certificate cert1 = getCertificateFromServer();
+        assertThat(getExpiryYear(cert1), is(2015));
+
+        // Replace keystore link
+        Files.delete(dataDir);
+        Files.createSymbolicLink(dataDir, timestampDecDir.getFileName());
+        // (link) data/ -> 2022-12/
+        // now keystore.p12 points to data/keystore.p12 which points to 2022-12/keystore.p12
+        System.err.println("### Triggering scan");
+        keyStoreScanner.scan(5000);
+
+        // The scanner should have detected the updated keystore, expiry should be renewed.
+        X509Certificate cert2 = getCertificateFromServer();
+        assertThat(getExpiryYear(cert2), is(2020));
+    }
+
     public Path useKeystore(String keystoreToUse, String keystorePath) throws Exception
     {
         return useKeystore(MavenTestingUtils.getTestResourcePath(keystoreToUse), keystoreDir.resolve(keystorePath));

tests/test-integration/src/test/resources/jetty-logging.properties

@@ -2,3 +2,4 @@
 #org.eclipse.jetty.LEVEL=DEBUG
 #org.eclipse.jetty.websocket.LEVEL=DEBUG
 #org.eclipse.jetty.util.ssl.KeyStoreScanner.LEVEL=DEBUG
+#org.eclipse.jetty.util.Scanner.LEVEL=DEBUG