From ff1ea432b1df07af6825e4d71ba109cb8ab98f94 Mon Sep 17 00:00:00 2001 From: Jochen Schalanda Date: Wed, 28 Oct 2020 23:39:32 +0100 Subject: [PATCH 1/2] Replace invalid default excludedProtocols in HttpsConnectorFactory The default list of excluded protocols used in `HttpsConnectorFactory` wasn't working as expected. Jetty currently doesn't support using regular expressions for supporte or excluded protocols. This only works for supported and excluded cipher suites as of Jetty 9.4.33.v20201020. The default list of excluded protocols now only contains valid and complete entries: SSLv3, TLSv1, and TLSv1.1 Refs https://github.com/eclipse/jetty.project/issues/5531 Fixes #3532 --- docs/source/manual/configuration.rst | 4 +- .../jetty/HttpsConnectorFactory.java | 54 +++++++++---------- .../jetty/HttpsConnectorFactoryTest.java | 50 +++++++++++------ 3 files changed, 63 insertions(+), 45 deletions(-) diff --git a/docs/source/manual/configuration.rst b/docs/source/manual/configuration.rst index 248c2f976ff..a46cc38970a 100644 --- a/docs/source/manual/configuration.rst +++ b/docs/source/manual/configuration.rst @@ -470,10 +470,10 @@ validatePeers false Whether or not implemented. supportedProtocols (none) A list of protocols (e.g., ``SSLv3``, ``TLSv1``) which are supported. All other protocols will be refused. -excludedProtocols ["SSL.*", "TLSv1", "TLSv1\\.1"] A list of protocols (e.g., ``SSLv3``, ``TLSv1``) which are excluded. These +excludedProtocols ["SSLv3", "TLSv1", "TLSv1.1"] A list of protocols (e.g., ``SSLv3``, ``TLSv1``) which are excluded. These protocols will be refused. supportedCipherSuites (none) A list of cipher suites (e.g., ``TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256``) which - are supported. All other cipher suites will be refused + are supported. All other cipher suites will be refused. excludedCipherSuites (none) A list of cipher suites (e.g., ``TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256``) which are excluded. These cipher suites will be refused and exclusion takes higher precedence than inclusion, such that if a cipher suite is listed in diff --git a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java index 365d97bd0b6..c3e59147bf5 100644 --- a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java +++ b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java @@ -182,7 +182,7 @@ * * * {@code excludedProtocols} - * ["SSL.*", "TLSv1", "TLSv1\.1"] + * ["SSLv3", "TLSv1", "TLSv1.1"] * * A list of protocols (e.g., {@code SSLv3}, {@code TLSv1}) which are excluded. These * protocols will be refused. @@ -193,7 +193,7 @@ * JVM default * * A list of cipher suites (e.g., {@code TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256}) which - * are supported. All other cipher suites will be refused + * are supported. All other cipher suites will be refused. * * * @@ -287,7 +287,7 @@ public class HttpsConnectorFactory extends HttpConnectorFactory { private List supportedProtocols; @Nullable - private List excludedProtocols = Arrays.asList("SSL.*", "TLSv1", "TLSv1\\.1"); + private List excludedProtocols = Arrays.asList("SSLv3", "TLSv1", "TLSv1.1"); @Nullable private List supportedCipherSuites; @@ -317,7 +317,7 @@ public String getEndpointIdentificationAlgorithm() { } @JsonProperty - public void setEndpointIdentificationAlgorithm(String endpointIdentificationAlgorithm) { + public void setEndpointIdentificationAlgorithm(@Nullable String endpointIdentificationAlgorithm) { this.endpointIdentificationAlgorithm = endpointIdentificationAlgorithm; } @@ -328,7 +328,7 @@ public String getKeyStorePath() { } @JsonProperty - public void setKeyStorePath(String keyStorePath) { + public void setKeyStorePath(@Nullable String keyStorePath) { this.keyStorePath = keyStorePath; } @@ -339,7 +339,7 @@ public String getKeyStorePassword() { } @JsonProperty - public void setKeyStorePassword(String keyStorePassword) { + public void setKeyStorePassword(@Nullable String keyStorePassword) { this.keyStorePassword = keyStorePassword; } @@ -360,7 +360,7 @@ public String getKeyStoreProvider() { } @JsonProperty - public void setKeyStoreProvider(String keyStoreProvider) { + public void setKeyStoreProvider(@Nullable String keyStoreProvider) { this.keyStoreProvider = keyStoreProvider; } @@ -381,7 +381,7 @@ public String getTrustStoreProvider() { } @JsonProperty - public void setTrustStoreProvider(String trustStoreProvider) { + public void setTrustStoreProvider(@Nullable String trustStoreProvider) { this.trustStoreProvider = trustStoreProvider; } @@ -392,7 +392,7 @@ public String getKeyManagerPassword() { } @JsonProperty - public void setKeyManagerPassword(String keyManagerPassword) { + public void setKeyManagerPassword(@Nullable String keyManagerPassword) { this.keyManagerPassword = keyManagerPassword; } @@ -403,7 +403,7 @@ public String getTrustStorePath() { } @JsonProperty - public void setTrustStorePath(String trustStorePath) { + public void setTrustStorePath(@Nullable String trustStorePath) { this.trustStorePath = trustStorePath; } @@ -425,7 +425,7 @@ public Boolean getNeedClientAuth() { } @JsonProperty - public void setNeedClientAuth(Boolean needClientAuth) { + public void setNeedClientAuth(@Nullable Boolean needClientAuth) { this.needClientAuth = needClientAuth; } @@ -436,7 +436,7 @@ public Boolean getWantClientAuth() { } @JsonProperty - public void setWantClientAuth(Boolean wantClientAuth) { + public void setWantClientAuth(@Nullable Boolean wantClientAuth) { this.wantClientAuth = wantClientAuth; } @@ -447,7 +447,7 @@ public String getCertAlias() { } @JsonProperty - public void setCertAlias(String certAlias) { + public void setCertAlias(@Nullable String certAlias) { this.certAlias = certAlias; } @@ -458,7 +458,7 @@ public File getCrlPath() { } @JsonProperty - public void setCrlPath(File crlPath) { + public void setCrlPath(@Nullable File crlPath) { this.crlPath = crlPath; } @@ -469,7 +469,7 @@ public Boolean getEnableCRLDP() { } @JsonProperty - public void setEnableCRLDP(Boolean enableCRLDP) { + public void setEnableCRLDP(@Nullable Boolean enableCRLDP) { this.enableCRLDP = enableCRLDP; } @@ -480,7 +480,7 @@ public Boolean getEnableOCSP() { } @JsonProperty - public void setEnableOCSP(Boolean enableOCSP) { + public void setEnableOCSP(@Nullable Boolean enableOCSP) { this.enableOCSP = enableOCSP; } @@ -491,7 +491,7 @@ public Integer getMaxCertPathLength() { } @JsonProperty - public void setMaxCertPathLength(Integer maxCertPathLength) { + public void setMaxCertPathLength(@Nullable Integer maxCertPathLength) { this.maxCertPathLength = maxCertPathLength; } @@ -502,7 +502,7 @@ public URI getOcspResponderUrl() { } @JsonProperty - public void setOcspResponderUrl(URI ocspResponderUrl) { + public void setOcspResponderUrl(@Nullable URI ocspResponderUrl) { this.ocspResponderUrl = ocspResponderUrl; } @@ -513,7 +513,7 @@ public String getJceProvider() { } @JsonProperty - public void setJceProvider(String jceProvider) { + public void setJceProvider(@Nullable String jceProvider) { this.jceProvider = jceProvider; } @@ -534,7 +534,7 @@ public List getSupportedProtocols() { } @JsonProperty - public void setSupportedProtocols(List supportedProtocols) { + public void setSupportedProtocols(@Nullable List supportedProtocols) { this.supportedProtocols = supportedProtocols; } @@ -545,7 +545,7 @@ public List getExcludedProtocols() { } @JsonProperty - public void setExcludedProtocols(List excludedProtocols) { + public void setExcludedProtocols(@Nullable List excludedProtocols) { this.excludedProtocols = excludedProtocols; } @@ -562,12 +562,12 @@ public List getExcludedCipherSuites() { } @JsonProperty - public void setExcludedCipherSuites(List excludedCipherSuites) { + public void setExcludedCipherSuites(@Nullable List excludedCipherSuites) { this.excludedCipherSuites = excludedCipherSuites; } @JsonProperty - public void setSupportedCipherSuites(List supportedCipherSuites) { + public void setSupportedCipherSuites(@Nullable List supportedCipherSuites) { this.supportedCipherSuites = supportedCipherSuites; } @@ -762,12 +762,12 @@ protected SslContextFactory configureSslContextFactory(SslContextFactory factory factory.setKeyManagerPassword(keyManagerPassword); } - if (needClientAuth != null) { - factory.setNeedClientAuth(needClientAuth); + if (needClientAuth != null && factory instanceof SslContextFactory.Server) { + ((SslContextFactory.Server) factory).setNeedClientAuth(needClientAuth); } - if (wantClientAuth != null) { - factory.setWantClientAuth(wantClientAuth); + if (wantClientAuth != null && factory instanceof SslContextFactory.Server) { + ((SslContextFactory.Server) factory).setWantClientAuth(wantClientAuth); } if (certAlias != null) { diff --git a/dropwizard-jetty/src/test/java/io/dropwizard/jetty/HttpsConnectorFactoryTest.java b/dropwizard-jetty/src/test/java/io/dropwizard/jetty/HttpsConnectorFactoryTest.java index 732852bc503..c64b7cc63a1 100644 --- a/dropwizard-jetty/src/test/java/io/dropwizard/jetty/HttpsConnectorFactoryTest.java +++ b/dropwizard-jetty/src/test/java/io/dropwizard/jetty/HttpsConnectorFactoryTest.java @@ -44,18 +44,18 @@ import static org.junit.jupiter.api.Assumptions.assumeFalse; import static org.junit.jupiter.api.Assumptions.assumeTrue; -public class HttpsConnectorFactoryTest { +class HttpsConnectorFactoryTest { private static final String WINDOWS_MY_KEYSTORE_NAME = "Windows-MY"; private final Validator validator = BaseValidator.newValidator(); @Test - public void isDiscoverable() throws Exception { + void isDiscoverable() { assertThat(new DiscoverableSubtypeResolver().getDiscoveredSubtypes()) .contains(HttpsConnectorFactory.class); } @Test - public void testParsingConfiguration() throws Exception { + void testParsingConfiguration() throws Exception { HttpsConnectorFactory https = new YamlConfigurationFactory<>(HttpsConnectorFactory.class, validator, Jackson.newObjectMapper(), "dw-https") .build(new File(Resources.getResource("yaml/https-connector.yml").toURI())); @@ -90,7 +90,7 @@ public void testParsingConfiguration() throws Exception { } @Test - public void testSupportedProtocols() { + void testSupportedProtocols() { List supportedProtocols = Arrays.asList("SSLv3", "TLS1"); HttpsConnectorFactory factory = new HttpsConnectorFactory(); @@ -102,7 +102,7 @@ public void testSupportedProtocols() { } @Test - public void testExcludedProtocols() { + void testExcludedProtocols() { List excludedProtocols = Arrays.asList("SSLv3", "TLS1"); HttpsConnectorFactory factory = new HttpsConnectorFactory(); @@ -114,7 +114,25 @@ public void testExcludedProtocols() { } @Test - public void nonWindowsKeyStoreValidation() throws Exception { + void testDefaultExcludedProtocols() throws Exception { + HttpsConnectorFactory factory = new HttpsConnectorFactory(); + + SslContextFactory sslContextFactory = factory.configureSslContextFactory(new SslContextFactory.Server()); + assertThat(sslContextFactory.getExcludeProtocols()) + .containsExactlyElementsOf(factory.getExcludedProtocols()); + + sslContextFactory.start(); + try { + assertThat(sslContextFactory.newSSLEngine().getEnabledProtocols()) + .doesNotContainAnyElementsOf(factory.getExcludedProtocols()) + .allSatisfy(protocol -> assertThat(protocol).doesNotStartWith("SSL")); + } finally { + sslContextFactory.stop(); + } + } + + @Test + void nonWindowsKeyStoreValidation() { HttpsConnectorFactory factory = new HttpsConnectorFactory(); Collection properties = getViolationProperties(validator.validate(factory)); assertThat(properties.contains("validKeyStorePassword")).isEqualTo(true); @@ -122,7 +140,7 @@ public void nonWindowsKeyStoreValidation() throws Exception { } @Test - public void windowsKeyStoreValidation() throws Exception { + void windowsKeyStoreValidation() { HttpsConnectorFactory factory = new HttpsConnectorFactory(); factory.setKeyStoreType(WINDOWS_MY_KEYSTORE_NAME); Collection properties = getViolationProperties(validator.validate(factory)); @@ -131,7 +149,7 @@ public void windowsKeyStoreValidation() throws Exception { } @Test - public void canBuildContextFactoryWhenWindowsKeyStoreAvailable() throws Exception { + void canBuildContextFactoryWhenWindowsKeyStoreAvailable() { // ignore test when Windows Keystore unavailable assumeTrue(canAccessWindowsKeyStore()); @@ -142,7 +160,7 @@ public void canBuildContextFactoryWhenWindowsKeyStoreAvailable() throws Exceptio } @Test - public void windowsKeyStoreUnavailableThrowsException() throws Exception { + void windowsKeyStoreUnavailableThrowsException() { assumeFalse(canAccessWindowsKeyStore()); final HttpsConnectorFactory factory = new HttpsConnectorFactory(); @@ -152,7 +170,7 @@ public void windowsKeyStoreUnavailableThrowsException() throws Exception { } @Test - public void testBuild() throws Exception { + void testBuild() throws Exception { final HttpsConnectorFactory https = new HttpsConnectorFactory(); https.setBindHost("127.0.0.1"); https.setPort(8443); @@ -196,7 +214,7 @@ public void testBuild() throws Exception { assertThat(serverConnector.getServer()).isSameAs(server); assertThat(serverConnector.getScheduler()).isInstanceOf(ScheduledExecutorScheduler.class); assertThat(serverConnector.getExecutor()).isSameAs(threadPool); - + final Jetty93InstrumentedConnectionFactory jetty93SslConnectionFacttory = (Jetty93InstrumentedConnectionFactory) serverConnector.getConnectionFactory("ssl"); assertThat(jetty93SslConnectionFacttory).isInstanceOf(Jetty93InstrumentedConnectionFactory.class); @@ -204,7 +222,7 @@ public void testBuild() throws Exception { metrics.timer("org.eclipse.jetty.server.HttpConnectionFactory.127.0.0.1.8443.connections")); final SslContextFactory sslContextFactory = ((SslConnectionFactory) jetty93SslConnectionFacttory .getConnectionFactory()).getSslContextFactory(); - + assertThat(getField(SslContextFactory.class, "_keyStoreResource", true).get(sslContextFactory)) .isEqualTo(Resource.newResource("/etc/app/server.ks")); assertThat(sslContextFactory.getKeyStoreType()).isEqualTo("JKS"); @@ -235,7 +253,7 @@ public void testBuild() throws Exception { assertThat(sslContextFactory.isValidatePeerCerts()).isTrue(); assertThat(sslContextFactory.getIncludeProtocols()).containsOnly("TLSv1.1", "TLSv1.2"); assertThat(sslContextFactory.getIncludeCipherSuites()).containsOnly("TLS_DHE_RSA.*", "TLS_ECDHE.*"); - + final ConnectionFactory httpConnectionFactory = serverConnector.getConnectionFactory("http/1.1"); assertThat(httpConnectionFactory).isInstanceOf(HttpConnectionFactory.class); final HttpConfiguration httpConfiguration = ((HttpConnectionFactory) httpConnectionFactory) @@ -250,7 +268,7 @@ public void testBuild() throws Exception { } @Test - public void partitionSupportOnlyEnable() { + void partitionSupportOnlyEnable() { final String[] supported = {"SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"}; final String[] enabled = {"TLSv1", "TLSv1.1", "TLSv1.2"}; final Map> partition = @@ -264,7 +282,7 @@ public void partitionSupportOnlyEnable() { } @Test - public void partitionSupportExclude() { + void partitionSupportExclude() { final String[] supported = {"SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"}; final String[] enabled = {"SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"}; final String[] exclude = {"SSL.*"}; @@ -279,7 +297,7 @@ public void partitionSupportExclude() { } @Test - public void partitionSupportInclude() { + void partitionSupportInclude() { final String[] supported = {"SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"}; final String[] enabled = {"SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"}; final String[] exclude = {"SSL*"}; From 711c9ce2380c6148dd5ddd978439cffa1d9424d0 Mon Sep 17 00:00:00 2001 From: Jochen Schalanda Date: Wed, 28 Oct 2020 23:50:42 +0100 Subject: [PATCH 2/2] Let's add "SSLv2Hello" to be sure it's excluded --- docs/source/manual/configuration.rst | 4 ++-- .../main/java/io/dropwizard/jetty/HttpsConnectorFactory.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/source/manual/configuration.rst b/docs/source/manual/configuration.rst index a46cc38970a..0c840728377 100644 --- a/docs/source/manual/configuration.rst +++ b/docs/source/manual/configuration.rst @@ -470,8 +470,8 @@ validatePeers false Whether or not implemented. supportedProtocols (none) A list of protocols (e.g., ``SSLv3``, ``TLSv1``) which are supported. All other protocols will be refused. -excludedProtocols ["SSLv3", "TLSv1", "TLSv1.1"] A list of protocols (e.g., ``SSLv3``, ``TLSv1``) which are excluded. These - protocols will be refused. +excludedProtocols ["SSLv2Hello", "SSLv3", A list of protocols (e.g., ``SSLv3``, ``TLSv1``) which are excluded. These + "TLSv1", "TLSv1.1"] protocols will be refused. supportedCipherSuites (none) A list of cipher suites (e.g., ``TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256``) which are supported. All other cipher suites will be refused. excludedCipherSuites (none) A list of cipher suites (e.g., ``TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256``) which diff --git a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java index c3e59147bf5..1aa01291e97 100644 --- a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java +++ b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java @@ -287,7 +287,7 @@ public class HttpsConnectorFactory extends HttpConnectorFactory { private List supportedProtocols; @Nullable - private List excludedProtocols = Arrays.asList("SSLv3", "TLSv1", "TLSv1.1"); + private List excludedProtocols = Arrays.asList("SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1"); @Nullable private List supportedCipherSuites;