From 4d15593d630c3fea96509a7a60a061fe3855754e Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 31 Oct 2022 14:37:56 +1100 Subject: [PATCH 1/4] Issue #8786 - add configuration for KeyStoreScanner to not resolve aliases Signed-off-by: Lachlan Roberts --- .../jetty/util/ssl/KeyStoreScanner.java | 11 +++-- .../jetty/test/KeyStoreScannerTest.java | 40 +++++++++++++++---- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index c9c06b1293e..cf2fcc6b141 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -43,6 +43,11 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr private final Scanner _scanner; public KeyStoreScanner(SslContextFactory sslContextFactory) + { + this(sslContextFactory, false); + } + + public KeyStoreScanner(SslContextFactory sslContextFactory, boolean resolveAlias) { this.sslContextFactory = sslContextFactory; try @@ -54,9 +59,9 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr if (monitoredFile.isDirectory()) throw new IllegalArgumentException("expected keystore file not directory"); - if (keystoreResource.getAlias() != null) + if (resolveAlias && keystoreResource.isAlias()) { - // this resource has an alias, use the alias, as that's what's returned in the Scanner + // This resource has an alias, so monitor the target of the alias. monitoredFile = new File(keystoreResource.getAlias()); } @@ -73,7 +78,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr if (!parentFile.exists() || !parentFile.isDirectory()) throw new IllegalArgumentException("error obtaining keystore dir"); - _scanner = new Scanner(); + _scanner = new Scanner(null, resolveAlias); _scanner.addDirectory(parentFile.toPath()); _scanner.setScanInterval(1); _scanner.setReportDirs(false); diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java index bd53e52a416..94df0ebeea8 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java @@ -87,6 +87,11 @@ 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); @@ -100,7 +105,7 @@ public class KeyStoreScannerTest server.addConnector(connector); // Configure Keystore Reload. - keystoreScanner = new KeyStoreScanner(sslContextFactory); + keystoreScanner = new KeyStoreScanner(sslContextFactory, resolveAlias); keystoreScanner.setScanInterval(0); server.addBean(keystoreScanner); @@ -182,22 +187,25 @@ public class KeyStoreScannerTest public void testReloadChangingSymbolicLink() throws Exception { assumeFileSystemSupportsSymlink(); - Path keystorePath = keystoreDir.resolve("symlinkKeystore"); + Path newKeystore = useKeystore("newKeystore", "newKeystore"); + Path oldKeystore = useKeystore("oldKeystore", "oldKeystore"); + + Path symlinkKeystorePath = keystoreDir.resolve("symlinkKeystore"); start(sslContextFactory -> { - Files.createSymbolicLink(keystorePath, useKeystore("oldKeystore")); - sslContextFactory.setKeyStorePath(keystorePath.toString()); + Files.createSymbolicLink(symlinkKeystorePath, oldKeystore); + sslContextFactory.setKeyStorePath(symlinkKeystorePath.toString()); sslContextFactory.setKeyStorePassword("storepwd"); sslContextFactory.setKeyManagerPassword("keypwd"); - }); + }, false); // Check the original certificate expiry. X509Certificate cert1 = getCertificateFromServer(); assertThat(getExpiryYear(cert1), is(2015)); // Change the symlink to point to the newKeystore file location which has a later expiry date. - Files.delete(keystorePath); - Files.createSymbolicLink(keystorePath, useKeystore("newKeystore")); + Files.delete(symlinkKeystorePath); + Files.createSymbolicLink(symlinkKeystorePath, newKeystore); keystoreScanner.scan(5000); // The scanner should have detected the updated keystore, expiry should be renewed. @@ -237,6 +245,24 @@ public class KeyStoreScannerTest assertThat(getExpiryYear(cert2), is(2020)); } + public Path useKeystore(String keystoreToUse, String keystorePath) throws Exception + { + return useKeystore(MavenTestingUtils.getTestResourcePath(keystoreToUse), keystoreDir.resolve(keystorePath)); + } + + public Path useKeystore(Path keystoreToUse, Path keystorePath) throws Exception + { + if (Files.exists(keystorePath)) + Files.delete(keystorePath); + + Files.copy(keystoreToUse, keystorePath); + + if (!Files.exists(keystorePath)) + throw new IllegalStateException("keystore file was not created"); + + return keystorePath.toAbsolutePath(); + } + public Path useKeystore(String keystore) throws Exception { Path keystorePath = keystoreDir.resolve("keystore"); From 8607e3ef155227a75d075d45f8c11725d3101830 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 3 Nov 2022 18:54:53 +1100 Subject: [PATCH 2/4] changes to ssl-reload module & documentation from review Signed-off-by: Lachlan Roberts --- .../modules/module-ssl-reload.adoc | 2 ++ .../config/etc/jetty-ssl-context-reload.xml | 1 + .../src/main/config/modules/ssl-reload.mod | 3 +++ .../jetty/util/ssl/KeyStoreScanner.java | 2 +- .../jetty/test/KeyStoreScannerTest.java | 20 +++++++++---------- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc b/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc index b8621b8a5cc..acccddf61e4 100644 --- a/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc +++ b/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc @@ -22,3 +22,5 @@ The module properties are: ---- include::{JETTY_HOME}/modules/ssl-reload.mod[tags=documentation] ---- + +The `resolveAlias` property is used to specify whether aliases 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. diff --git a/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml b/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml index 46346359ed7..43b28187877 100644 --- a/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml +++ b/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml @@ -5,6 +5,7 @@ + diff --git a/jetty-server/src/main/config/modules/ssl-reload.mod b/jetty-server/src/main/config/modules/ssl-reload.mod index 9dca8473d55..27191fd0fae 100644 --- a/jetty-server/src/main/config/modules/ssl-reload.mod +++ b/jetty-server/src/main/config/modules/ssl-reload.mod @@ -15,4 +15,7 @@ etc/jetty-ssl-context-reload.xml # tag::documentation[] # Monitored directory scan period, in seconds. # jetty.sslContext.reload.scanInterval=1 + +# Whether to resolve aliases in the KeyStore path. +# jetty.sslContext.reload.resolveAlias=true # end::documentation[] diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index cf2fcc6b141..56966c9b6cb 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -44,7 +44,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr public KeyStoreScanner(SslContextFactory sslContextFactory) { - this(sslContextFactory, false); + this(sslContextFactory, true); } public KeyStoreScanner(SslContextFactory sslContextFactory, boolean resolveAlias) diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java index 94df0ebeea8..6b94e2d85c1 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java @@ -61,7 +61,7 @@ public class KeyStoreScannerTest public WorkDir testdir; private Server server; private Path keystoreDir; - private KeyStoreScanner keystoreScanner; + private KeyStoreScanner keyStoreScanner; @BeforeEach public void before() @@ -105,9 +105,9 @@ public class KeyStoreScannerTest server.addConnector(connector); // Configure Keystore Reload. - keystoreScanner = new KeyStoreScanner(sslContextFactory, resolveAlias); - keystoreScanner.setScanInterval(0); - server.addBean(keystoreScanner); + keyStoreScanner = new KeyStoreScanner(sslContextFactory, resolveAlias); + keyStoreScanner.setScanInterval(0); + server.addBean(keyStoreScanner); server.start(); } @@ -129,7 +129,7 @@ public class KeyStoreScannerTest // Switch to use newKeystore which has a later expiry date. useKeystore("newKeystore"); - assertTrue(keystoreScanner.scan(5000)); + assertTrue(keyStoreScanner.scan(5000)); // The scanner should have detected the updated keystore, expiry should be renewed. X509Certificate cert2 = getCertificateFromServer(); @@ -149,7 +149,7 @@ public class KeyStoreScannerTest try (StacklessLogging ignored = new StacklessLogging(KeyStoreScanner.class)) { useKeystore("badKeystore"); - keystoreScanner.scan(5000); + keyStoreScanner.scan(5000); } // The good keystore is removed, now the bad keystore now causes an exception. @@ -170,7 +170,7 @@ public class KeyStoreScannerTest { Path keystorePath = keystoreDir.resolve("keystore"); assertTrue(Files.deleteIfExists(keystorePath)); - keystoreScanner.scan(5000); + keyStoreScanner.scan(5000); } // The good keystore is removed, having no keystore causes an exception. @@ -178,7 +178,7 @@ public class KeyStoreScannerTest // Switch to use keystore2 which has a later expiry date. useKeystore("newKeystore"); - keystoreScanner.scan(5000); + keyStoreScanner.scan(5000); X509Certificate cert2 = getCertificateFromServer(); assertThat(getExpiryYear(cert2), is(2020)); } @@ -206,7 +206,7 @@ public class KeyStoreScannerTest // Change the symlink to point to the newKeystore file location which has a later expiry date. Files.delete(symlinkKeystorePath); Files.createSymbolicLink(symlinkKeystorePath, newKeystore); - keystoreScanner.scan(5000); + keyStoreScanner.scan(5000); // The scanner should have detected the updated keystore, expiry should be renewed. X509Certificate cert2 = getCertificateFromServer(); @@ -238,7 +238,7 @@ public class KeyStoreScannerTest // Change the target file of the symlink to the newKeystore which has a later expiry date. Files.copy(newKeystoreSrc, target, StandardCopyOption.REPLACE_EXISTING); System.err.println("### Triggering scan"); - keystoreScanner.scan(5000); + keyStoreScanner.scan(5000); // The scanner should have detected the updated keystore, expiry should be renewed. X509Certificate cert2 = getCertificateFromServer(); From 3b7ea99780939de27c8ec8a1ee59db66159a16a2 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 3 Nov 2022 19:26:00 +1100 Subject: [PATCH 3/4] set type of arg in jetty-ssl-context-reload.xml Signed-off-by: Lachlan Roberts --- jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml b/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml index 43b28187877..25d22acabce 100644 --- a/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml +++ b/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml @@ -5,7 +5,7 @@ - + From 0a14cca3077f1720f96e9efb8149c26aadc250bc Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 3 Nov 2022 20:37:55 +1100 Subject: [PATCH 4/4] changes from review - rename resolveAlias to followLinks Signed-off-by: Lachlan Roberts --- .../modules/module-ssl-reload.adoc | 3 +- .../config/etc/jetty-ssl-context-reload.xml | 2 +- .../src/main/config/modules/ssl-reload.mod | 4 +-- .../jetty/util/ssl/KeyStoreScanner.java | 6 ++-- .../jetty/test/KeyStoreScannerTest.java | 28 +++++++++--------- .../resources/{newKeystore => newKeyStore} | Bin .../resources/{oldKeystore => oldKeyStore} | Bin 7 files changed, 22 insertions(+), 21 deletions(-) rename tests/test-integration/src/test/resources/{newKeystore => newKeyStore} (100%) rename tests/test-integration/src/test/resources/{oldKeystore => oldKeyStore} (100%) diff --git a/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc b/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc index acccddf61e4..96ff3d6270c 100644 --- a/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc +++ b/documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/module-ssl-reload.adoc @@ -23,4 +23,5 @@ The module properties are: include::{JETTY_HOME}/modules/ssl-reload.mod[tags=documentation] ---- -The `resolveAlias` property is used to specify whether aliases 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. +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. diff --git a/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml b/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml index 25d22acabce..e1de1cc08fe 100644 --- a/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml +++ b/jetty-server/src/main/config/etc/jetty-ssl-context-reload.xml @@ -5,7 +5,7 @@ - + diff --git a/jetty-server/src/main/config/modules/ssl-reload.mod b/jetty-server/src/main/config/modules/ssl-reload.mod index 27191fd0fae..68ff5d5d933 100644 --- a/jetty-server/src/main/config/modules/ssl-reload.mod +++ b/jetty-server/src/main/config/modules/ssl-reload.mod @@ -16,6 +16,6 @@ etc/jetty-ssl-context-reload.xml # Monitored directory scan period, in seconds. # jetty.sslContext.reload.scanInterval=1 -# Whether to resolve aliases in the KeyStore path. -# jetty.sslContext.reload.resolveAlias=true +# Whether to resolve symbolic links in the KeyStore path. +# jetty.sslContext.reload.followLinks=true # end::documentation[] diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index 56966c9b6cb..5bc750ea6f7 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -47,7 +47,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr this(sslContextFactory, true); } - public KeyStoreScanner(SslContextFactory sslContextFactory, boolean resolveAlias) + public KeyStoreScanner(SslContextFactory sslContextFactory, boolean followLinks) { this.sslContextFactory = sslContextFactory; try @@ -59,7 +59,7 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr if (monitoredFile.isDirectory()) throw new IllegalArgumentException("expected keystore file not directory"); - if (resolveAlias && keystoreResource.isAlias()) + if (followLinks && keystoreResource.isAlias()) { // This resource has an alias, so monitor the target of the alias. monitoredFile = new File(keystoreResource.getAlias()); @@ -78,7 +78,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, resolveAlias); + _scanner = new Scanner(null, followLinks); _scanner.addDirectory(parentFile.toPath()); _scanner.setScanInterval(1); _scanner.setReportDirs(false); diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java index 6b94e2d85c1..323ec38ec82 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java @@ -79,7 +79,7 @@ public class KeyStoreScannerTest { start(sslContextFactory -> { - String keystorePath = useKeystore("oldKeystore").toString(); + String keystorePath = useKeystore("oldKeyStore").toString(); sslContextFactory.setKeyStorePath(keystorePath); sslContextFactory.setKeyStorePassword("storepwd"); sslContextFactory.setKeyManagerPassword("keypwd"); @@ -127,8 +127,8 @@ public class KeyStoreScannerTest X509Certificate cert1 = getCertificateFromServer(); assertThat(getExpiryYear(cert1), is(2015)); - // Switch to use newKeystore which has a later expiry date. - useKeystore("newKeystore"); + // Switch to use newKeyStore which has a later expiry date. + useKeystore("newKeyStore"); assertTrue(keyStoreScanner.scan(5000)); // The scanner should have detected the updated keystore, expiry should be renewed. @@ -177,7 +177,7 @@ public class KeyStoreScannerTest assertThrows(Throwable.class, this::getCertificateFromServer); // Switch to use keystore2 which has a later expiry date. - useKeystore("newKeystore"); + useKeystore("newKeyStore"); keyStoreScanner.scan(5000); X509Certificate cert2 = getCertificateFromServer(); assertThat(getExpiryYear(cert2), is(2020)); @@ -187,13 +187,13 @@ public class KeyStoreScannerTest public void testReloadChangingSymbolicLink() throws Exception { assumeFileSystemSupportsSymlink(); - Path newKeystore = useKeystore("newKeystore", "newKeystore"); - Path oldKeystore = useKeystore("oldKeystore", "oldKeystore"); + Path newKeyStore = useKeystore("newKeyStore", "newKeyStore"); + Path oldKeyStore = useKeystore("oldKeyStore", "oldKeyStore"); Path symlinkKeystorePath = keystoreDir.resolve("symlinkKeystore"); start(sslContextFactory -> { - Files.createSymbolicLink(symlinkKeystorePath, oldKeystore); + Files.createSymbolicLink(symlinkKeystorePath, oldKeyStore); sslContextFactory.setKeyStorePath(symlinkKeystorePath.toString()); sslContextFactory.setKeyStorePassword("storepwd"); sslContextFactory.setKeyManagerPassword("keypwd"); @@ -203,9 +203,9 @@ public class KeyStoreScannerTest X509Certificate cert1 = getCertificateFromServer(); assertThat(getExpiryYear(cert1), is(2015)); - // Change the symlink to point to the newKeystore file location which has a later expiry date. + // Change the symlink to point to the newKeyStore file location which has a later expiry date. Files.delete(symlinkKeystorePath); - Files.createSymbolicLink(symlinkKeystorePath, newKeystore); + Files.createSymbolicLink(symlinkKeystorePath, newKeyStore); keyStoreScanner.scan(5000); // The scanner should have detected the updated keystore, expiry should be renewed. @@ -218,13 +218,13 @@ public class KeyStoreScannerTest { assumeFileSystemSupportsSymlink(); Path keystoreLink = keystoreDir.resolve("symlinkKeystore"); - Path oldKeystoreSrc = MavenTestingUtils.getTestResourcePathFile("oldKeystore"); - Path newKeystoreSrc = MavenTestingUtils.getTestResourcePathFile("newKeystore"); + Path oldKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("oldKeyStore"); + Path newKeyStoreSrc = MavenTestingUtils.getTestResourcePathFile("newKeyStore"); Path target = keystoreDir.resolve("keystore"); start(sslContextFactory -> { - Files.copy(oldKeystoreSrc, target); + Files.copy(oldKeyStoreSrc, target); Files.createSymbolicLink(keystoreLink, target); sslContextFactory.setKeyStorePath(keystoreLink.toString()); sslContextFactory.setKeyStorePassword("storepwd"); @@ -235,8 +235,8 @@ public class KeyStoreScannerTest X509Certificate cert1 = getCertificateFromServer(); assertThat(getExpiryYear(cert1), is(2015)); - // Change the target file of the symlink to the newKeystore which has a later expiry date. - Files.copy(newKeystoreSrc, target, StandardCopyOption.REPLACE_EXISTING); + // Change the target file of the symlink to the newKeyStore which has a later expiry date. + Files.copy(newKeyStoreSrc, target, StandardCopyOption.REPLACE_EXISTING); System.err.println("### Triggering scan"); keyStoreScanner.scan(5000); diff --git a/tests/test-integration/src/test/resources/newKeystore b/tests/test-integration/src/test/resources/newKeyStore similarity index 100% rename from tests/test-integration/src/test/resources/newKeystore rename to tests/test-integration/src/test/resources/newKeyStore diff --git a/tests/test-integration/src/test/resources/oldKeystore b/tests/test-integration/src/test/resources/oldKeyStore similarity index 100% rename from tests/test-integration/src/test/resources/oldKeystore rename to tests/test-integration/src/test/resources/oldKeyStore