From 94d03aba3a0846b9feecb7349afd47b36a620fad Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Sat, 25 Nov 2023 16:46:58 -0500 Subject: [PATCH] Update PortAssigner to set more properties from TlsContextConfiguration * PortAssigner should set new property disableSniHostCheck, which was added in kiwi 3.2.0 * PortAssigner should set additional properties from TlsContextConfiguration: keyStoreType, keyStoreProvider, trustStoreType, trustStoreProvider, jceProvider, certAlias, and supportedCipherSuites * Add implNote to javadoc of assignDynamicPorts, which explains that it replaces the HttpsConnectorFactory instances when assigning secure dynamic ports. * Add tests to verify the secure properties are correctly set on the HttpsConnectorFactory when using secure dynamic ports * Clean up existing tests by using KiwiAssertJ#assertIsExactType, and also by using assertAll when there are a lot of assertions Closes #398 Closes #401 --- .../dropwizard/util/startup/PortAssigner.java | 13 ++ .../util/startup/PortAssignerTest.java | 113 ++++++++++++++---- 2 files changed, 102 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/kiwiproject/dropwizard/util/startup/PortAssigner.java b/src/main/java/org/kiwiproject/dropwizard/util/startup/PortAssigner.java index c5f88d58..ba968e4a 100644 --- a/src/main/java/org/kiwiproject/dropwizard/util/startup/PortAssigner.java +++ b/src/main/java/org/kiwiproject/dropwizard/util/startup/PortAssigner.java @@ -74,6 +74,11 @@ private PortAssigner(LocalPortChecker localPortChecker, /** * Sets up the connectors with a dynamic available port if {@link PortAssigner} is configured for dynamic ports. + * + * @implNote When using {@link PortSecurity#SECURE}, the current implementation replaces the + * application and admin connector factories, which overrides any explicit configuration. + * Support for explicit configuration of secure connectors while still assigning dynamics ports may + * be implemented in the future if custom configuration is needed. */ public void assignDynamicPorts() { if (portAssignment == PortAssignment.STATIC) { @@ -110,9 +115,17 @@ private HttpsConnectorFactory newHttpsConnectorFactory(int port) { https.setPort(port); https.setKeyStorePath(tlsConfiguration.getKeyStorePath()); https.setKeyStorePassword(tlsConfiguration.getKeyStorePassword()); + https.setKeyStoreType(tlsConfiguration.getKeyStoreType()); + https.setKeyStoreProvider(tlsConfiguration.getKeyStoreProvider()); https.setTrustStorePath(tlsConfiguration.getTrustStorePath()); https.setTrustStorePassword(tlsConfiguration.getTrustStorePassword()); + https.setTrustStoreType(tlsConfiguration.getTrustStoreType()); + https.setTrustStoreProvider(tlsConfiguration.getTrustStoreProvider()); + https.setJceProvider(tlsConfiguration.getProvider()); + https.setCertAlias(tlsConfiguration.getCertAlias()); https.setSupportedProtocols(tlsConfiguration.getSupportedProtocols()); + https.setSupportedCipherSuites(tlsConfiguration.getSupportedCiphers()); + https.setDisableSniHostCheck(tlsConfiguration.isDisableSniHostCheck()); return https; } diff --git a/src/test/java/org/kiwiproject/dropwizard/util/startup/PortAssignerTest.java b/src/test/java/org/kiwiproject/dropwizard/util/startup/PortAssignerTest.java index 8fcd8879..52c72d4d 100644 --- a/src/test/java/org/kiwiproject/dropwizard/util/startup/PortAssignerTest.java +++ b/src/test/java/org/kiwiproject/dropwizard/util/startup/PortAssignerTest.java @@ -2,11 +2,13 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.kiwiproject.collect.KiwiLists.first; import static org.kiwiproject.dropwizard.util.startup.PortAssigner.PortAssignment.DYNAMIC; import static org.kiwiproject.dropwizard.util.startup.PortAssigner.PortAssignment.STATIC; import static org.kiwiproject.dropwizard.util.startup.PortAssigner.PortSecurity.NON_SECURE; import static org.kiwiproject.dropwizard.util.startup.PortAssigner.PortSecurity.SECURE; +import static org.kiwiproject.test.assertj.KiwiAssertJ.assertIsExactType; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -25,6 +27,7 @@ import org.kiwiproject.net.LocalPortChecker; import java.util.HashSet; +import java.util.List; import java.util.Set; @DisplayName("PortAssigner") @@ -243,13 +246,11 @@ void shouldSetupSecureDynamicPorts_WhenSecureSpecified() { assigner.assignDynamicPorts(); - var applicationConnector = first(factory.getApplicationConnectors()); - assertThat(applicationConnector).isInstanceOf(HttpsConnectorFactory.class); - assertThat(((HttpsConnectorFactory) applicationConnector).getPort()).isBetween(9_000, 9_100); + var applicationConnector = assertIsExactType(first(factory.getApplicationConnectors()), HttpsConnectorFactory.class); + assertThat(applicationConnector.getPort()).isBetween(9_000, 9_100); - var adminConnector = first(factory.getAdminConnectors()); - assertThat(adminConnector).isInstanceOf(HttpsConnectorFactory.class); - assertThat(((HttpsConnectorFactory) adminConnector).getPort()).isBetween(9_000, 9_100); + var adminConnector = assertIsExactType(first(factory.getAdminConnectors()), HttpsConnectorFactory.class); + assertThat(adminConnector.getPort()).isBetween(9_000, 9_100); } @Test @@ -266,13 +267,81 @@ void shouldSetupSecureDynamicPorts_AsZero_WhenPortRangeExcluded() { assigner.assignDynamicPorts(); - var applicationConnector = first(factory.getApplicationConnectors()); - assertThat(applicationConnector).isInstanceOf(HttpsConnectorFactory.class); - assertThat(((HttpsConnectorFactory) applicationConnector).getPort()).isZero(); + var applicationConnector = assertIsExactType(first(factory.getApplicationConnectors()), HttpsConnectorFactory.class); + assertThat(applicationConnector.getPort()).isZero(); - var adminConnector = first(factory.getAdminConnectors()); - assertThat(adminConnector).isInstanceOf(HttpsConnectorFactory.class); - assertThat(((HttpsConnectorFactory) adminConnector).getPort()).isZero(); + var adminConnector = assertIsExactType(first(factory.getAdminConnectors()), HttpsConnectorFactory.class); + assertThat(adminConnector.getPort()).isZero(); + } + + @Test + void shouldSetSecureProperties_OnApplicationConnector_WhenAssigningSecureDynamicPorts() { + var factory = new DefaultServerFactory(); + var tlsConfig = TlsContextConfiguration.builder() + .keyStorePath("/data/etc/pki/acme-ks.jks") + .keyStorePassword("R3ally-hArd-passw0rD") + .trustStorePath("/data/etc/pki/acme-ts.jks") + .supportedProtocols(List.of("TLSv1.2", "TLSv1.3")) + .disableSniHostCheck(true) + .build(); + + var assigner = PortAssigner.builder() + .portAssignment(DYNAMIC) + .serverFactory(factory) + .portSecurity(SECURE) + .tlsConfiguration(tlsConfig) + .build(); + + assigner.assignDynamicPorts(); + + var applicationConnector = assertIsExactType(first(factory.getApplicationConnectors()), HttpsConnectorFactory.class); + + assertSecureConnectorProperties(applicationConnector, tlsConfig); + } + + @Test + void shouldSetSecureProperties_OnAdminConnector_WhenAssigningSecureDynamicPorts() { + var factory = new DefaultServerFactory(); + var tlsConfig = TlsContextConfiguration.builder() + .keyStorePath("/data/etc/pki/acme-ks.jks") + .keyStorePassword("R3ally-hArd-passw0rD") + .trustStorePath("/data/etc/pki/acme-ts.jks") + .supportedProtocols(List.of("TLSv1.2", "TLSv1.3")) + .disableSniHostCheck(true) + .build(); + + var assigner = PortAssigner.builder() + .portAssignment(DYNAMIC) + .serverFactory(factory) + .portSecurity(SECURE) + .tlsConfiguration(tlsConfig) + .build(); + + assigner.assignDynamicPorts(); + + var adminConnector = assertIsExactType(first(factory.getAdminConnectors()), HttpsConnectorFactory.class); + + assertSecureConnectorProperties(adminConnector, tlsConfig); + } + + private static void assertSecureConnectorProperties(HttpsConnectorFactory httpsConnectorFactory, + TlsContextConfiguration tlsConfig) { + + assertAll( + () -> assertThat(httpsConnectorFactory.getKeyStorePath()).isEqualTo(tlsConfig.getKeyStorePath()), + () -> assertThat(httpsConnectorFactory.getKeyStorePassword()).isEqualTo(tlsConfig.getKeyStorePassword()), + () -> assertThat(httpsConnectorFactory.getKeyStoreType()).isEqualTo(tlsConfig.getKeyStoreType()), + () -> assertThat(httpsConnectorFactory.getKeyStoreProvider()).isEqualTo(tlsConfig.getKeyStoreProvider()), + () -> assertThat(httpsConnectorFactory.getTrustStorePath()).isEqualTo(tlsConfig.getTrustStorePath()), + () -> assertThat(httpsConnectorFactory.getTrustStorePassword()).isEqualTo(tlsConfig.getTrustStorePassword()), + () -> assertThat(httpsConnectorFactory.getTrustStoreType()).isEqualTo(tlsConfig.getTrustStoreType()), + () -> assertThat(httpsConnectorFactory.getTrustStoreProvider()).isEqualTo(tlsConfig.getTrustStoreProvider()), + () -> assertThat(httpsConnectorFactory.getJceProvider()).isEqualTo(tlsConfig.getProvider()), + () -> assertThat(httpsConnectorFactory.getCertAlias()).isEqualTo(tlsConfig.getCertAlias()), + () -> assertThat(httpsConnectorFactory.getSupportedProtocols()).isEqualTo(tlsConfig.getSupportedProtocols()), + () -> assertThat(httpsConnectorFactory.getSupportedCipherSuites()).isEqualTo(tlsConfig.getSupportedCiphers()), + () -> assertThat(httpsConnectorFactory.isDisableSniHostCheck()).isEqualTo(tlsConfig.isDisableSniHostCheck()) + ); } @Test @@ -289,13 +358,11 @@ void shouldSetupNonSecureDynamicPorts_WhenNonSecureSpecified() { assigner.assignDynamicPorts(); - var applicationConnector = first(factory.getApplicationConnectors()); - assertThat(applicationConnector).isInstanceOf(HttpConnectorFactory.class); - assertThat(((HttpConnectorFactory) applicationConnector).getPort()).isBetween(9_000, 9_100); + var applicationConnector = assertIsExactType(first(factory.getApplicationConnectors()), HttpConnectorFactory.class); + assertThat(applicationConnector.getPort()).isBetween(9_000, 9_100); - var adminConnector = first(factory.getAdminConnectors()); - assertThat(adminConnector).isInstanceOf(HttpConnectorFactory.class); - assertThat(((HttpConnectorFactory) adminConnector).getPort()).isBetween(9_000, 9_100); + var adminConnector = assertIsExactType(first(factory.getAdminConnectors()), HttpConnectorFactory.class); + assertThat(adminConnector.getPort()).isBetween(9_000, 9_100); } @Test @@ -310,13 +377,11 @@ void shouldSetupNonSecureDynamicPorts_AsZero_WhenPortRangeExcluded() { assigner.assignDynamicPorts(); - var applicationConnector = first(factory.getApplicationConnectors()); - assertThat(applicationConnector).isInstanceOf(HttpConnectorFactory.class); - assertThat(((HttpConnectorFactory) applicationConnector).getPort()).isZero(); + var applicationConnector = assertIsExactType(first(factory.getApplicationConnectors()), HttpConnectorFactory.class); + assertThat(applicationConnector.getPort()).isZero(); - var adminConnector = first(factory.getAdminConnectors()); - assertThat(adminConnector).isInstanceOf(HttpConnectorFactory.class); - assertThat(((HttpConnectorFactory) adminConnector).getPort()).isZero(); + var adminConnector = assertIsExactType(first(factory.getAdminConnectors()), HttpConnectorFactory.class); + assertThat(adminConnector.getPort()).isZero(); } @Test