Skip to content

Commit

Permalink
Replace invalid default excludedProtocols in HttpsConnectorFactory (#…
Browse files Browse the repository at this point in the history 
…3533)

The default list of excluded protocols used in `HttpsConnectorFactory` wasn't working as expected.

Jetty currently doesn't support using regular expressions for supported 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 jetty/jetty.project#5531
Fixes #3532
  • Loading branch information
@joschi
joschi committed Oct 29, 2020
1 parent eccebcf commit 206e858
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 46 deletions.
6 changes: 3 additions & 3 deletions docs/source/manual/configuration.rst
View file
Expand Up @@ -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
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
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
Expand Down
54 changes: 27 additions & 27 deletions dropwizard-jetty/src/main/java/io/dropwizard/jetty/HttpsConnectorFactory.java
View file
Expand Up @@ -182,7 +182,7 @@
* </tr>
* <tr>
* <td>{@code excludedProtocols}</td>
* <td>["SSL.*", "TLSv1", "TLSv1\.1"]</td>
* <td>["SSLv3", "TLSv1", "TLSv1.1"]</td>
* <td>
* A list of protocols (e.g., {@code SSLv3}, {@code TLSv1}) which are excluded. These
* protocols will be refused.
Expand All @@ -193,7 +193,7 @@
* <td>JVM default</td>
* <td>
* 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.
* </td>
* </tr>
* <tr>
Expand Down Expand Up @@ -287,7 +287,7 @@ public class HttpsConnectorFactory extends HttpConnectorFactory {
private List<String> supportedProtocols;

@Nullable
private List<String> excludedProtocols = Arrays.asList("SSL.*", "TLSv1", "TLSv1\\.1");
private List<String> excludedProtocols = Arrays.asList("SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1");

@Nullable
private List<String> supportedCipherSuites;
Expand Down Expand Up @@ -317,7 +317,7 @@ public String getEndpointIdentificationAlgorithm() {
}

@JsonProperty
public void setEndpointIdentificationAlgorithm(String endpointIdentificationAlgorithm) {
public void setEndpointIdentificationAlgorithm(@Nullable String endpointIdentificationAlgorithm) {
this.endpointIdentificationAlgorithm = endpointIdentificationAlgorithm;
}

Expand All @@ -328,7 +328,7 @@ public String getKeyStorePath() {
}

@JsonProperty
public void setKeyStorePath(String keyStorePath) {
public void setKeyStorePath(@Nullable String keyStorePath) {
this.keyStorePath = keyStorePath;
}

Expand All @@ -339,7 +339,7 @@ public String getKeyStorePassword() {
}

@JsonProperty
public void setKeyStorePassword(String keyStorePassword) {
public void setKeyStorePassword(@Nullable String keyStorePassword) {
this.keyStorePassword = keyStorePassword;
}

Expand All @@ -360,7 +360,7 @@ public String getKeyStoreProvider() {
}

@JsonProperty
public void setKeyStoreProvider(String keyStoreProvider) {
public void setKeyStoreProvider(@Nullable String keyStoreProvider) {
this.keyStoreProvider = keyStoreProvider;
}

Expand All @@ -381,7 +381,7 @@ public String getTrustStoreProvider() {
}

@JsonProperty
public void setTrustStoreProvider(String trustStoreProvider) {
public void setTrustStoreProvider(@Nullable String trustStoreProvider) {
this.trustStoreProvider = trustStoreProvider;
}

Expand All @@ -392,7 +392,7 @@ public String getKeyManagerPassword() {
}

@JsonProperty
public void setKeyManagerPassword(String keyManagerPassword) {
public void setKeyManagerPassword(@Nullable String keyManagerPassword) {
this.keyManagerPassword = keyManagerPassword;
}

Expand All @@ -403,7 +403,7 @@ public String getTrustStorePath() {
}

@JsonProperty
public void setTrustStorePath(String trustStorePath) {
public void setTrustStorePath(@Nullable String trustStorePath) {
this.trustStorePath = trustStorePath;
}

Expand All @@ -425,7 +425,7 @@ public Boolean getNeedClientAuth() {
}

@JsonProperty
public void setNeedClientAuth(Boolean needClientAuth) {
public void setNeedClientAuth(@Nullable Boolean needClientAuth) {
this.needClientAuth = needClientAuth;
}

Expand All @@ -436,7 +436,7 @@ public Boolean getWantClientAuth() {
}

@JsonProperty
public void setWantClientAuth(Boolean wantClientAuth) {
public void setWantClientAuth(@Nullable Boolean wantClientAuth) {
this.wantClientAuth = wantClientAuth;
}

Expand All @@ -447,7 +447,7 @@ public String getCertAlias() {
}

@JsonProperty
public void setCertAlias(String certAlias) {
public void setCertAlias(@Nullable String certAlias) {
this.certAlias = certAlias;
}

Expand All @@ -458,7 +458,7 @@ public File getCrlPath() {
}

@JsonProperty
public void setCrlPath(File crlPath) {
public void setCrlPath(@Nullable File crlPath) {
this.crlPath = crlPath;
}

Expand All @@ -469,7 +469,7 @@ public Boolean getEnableCRLDP() {
}

@JsonProperty
public void setEnableCRLDP(Boolean enableCRLDP) {
public void setEnableCRLDP(@Nullable Boolean enableCRLDP) {
this.enableCRLDP = enableCRLDP;
}

Expand All @@ -480,7 +480,7 @@ public Boolean getEnableOCSP() {
}

@JsonProperty
public void setEnableOCSP(Boolean enableOCSP) {
public void setEnableOCSP(@Nullable Boolean enableOCSP) {
this.enableOCSP = enableOCSP;
}

Expand All @@ -491,7 +491,7 @@ public Integer getMaxCertPathLength() {
}

@JsonProperty
public void setMaxCertPathLength(Integer maxCertPathLength) {
public void setMaxCertPathLength(@Nullable Integer maxCertPathLength) {
this.maxCertPathLength = maxCertPathLength;
}

Expand All @@ -502,7 +502,7 @@ public URI getOcspResponderUrl() {
}

@JsonProperty
public void setOcspResponderUrl(URI ocspResponderUrl) {
public void setOcspResponderUrl(@Nullable URI ocspResponderUrl) {
this.ocspResponderUrl = ocspResponderUrl;
}

Expand All @@ -513,7 +513,7 @@ public String getJceProvider() {
}

@JsonProperty
public void setJceProvider(String jceProvider) {
public void setJceProvider(@Nullable String jceProvider) {
this.jceProvider = jceProvider;
}

Expand All @@ -534,7 +534,7 @@ public List<String> getSupportedProtocols() {
}

@JsonProperty
public void setSupportedProtocols(List<String> supportedProtocols) {
public void setSupportedProtocols(@Nullable List<String> supportedProtocols) {
this.supportedProtocols = supportedProtocols;
}

Expand All @@ -545,7 +545,7 @@ public List<String> getExcludedProtocols() {
}

@JsonProperty
public void setExcludedProtocols(List<String> excludedProtocols) {
public void setExcludedProtocols(@Nullable List<String> excludedProtocols) {
this.excludedProtocols = excludedProtocols;
}

Expand All @@ -562,12 +562,12 @@ public List<String> getExcludedCipherSuites() {
}

@JsonProperty
public void setExcludedCipherSuites(List<String> excludedCipherSuites) {
public void setExcludedCipherSuites(@Nullable List<String> excludedCipherSuites) {
this.excludedCipherSuites = excludedCipherSuites;
}

@JsonProperty
public void setSupportedCipherSuites(List<String> supportedCipherSuites) {
public void setSupportedCipherSuites(@Nullable List<String> supportedCipherSuites) {
this.supportedCipherSuites = supportedCipherSuites;
}

Expand Down Expand Up @@ -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) {
Expand Down
50 changes: 34 additions & 16 deletions dropwizard-jetty/src/test/java/io/dropwizard/jetty/HttpsConnectorFactoryTest.java
View file
Expand Up @@ -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()));
Expand Down Expand Up @@ -90,7 +90,7 @@ public void testParsingConfiguration() throws Exception {
}

@Test
public void testSupportedProtocols() {
void testSupportedProtocols() {
List<String> supportedProtocols = Arrays.asList("SSLv3", "TLS1");

HttpsConnectorFactory factory = new HttpsConnectorFactory();
Expand All @@ -102,7 +102,7 @@ public void testSupportedProtocols() {
}

@Test
public void testExcludedProtocols() {
void testExcludedProtocols() {
List<String> excludedProtocols = Arrays.asList("SSLv3", "TLS1");

HttpsConnectorFactory factory = new HttpsConnectorFactory();
Expand All @@ -114,15 +114,33 @@ 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<String> properties = getViolationProperties(validator.validate(factory));
assertThat(properties.contains("validKeyStorePassword")).isEqualTo(true);
assertThat(properties.contains("validKeyStorePath")).isEqualTo(true);
}

@Test
public void windowsKeyStoreValidation() throws Exception {
void windowsKeyStoreValidation() {
HttpsConnectorFactory factory = new HttpsConnectorFactory();
factory.setKeyStoreType(WINDOWS_MY_KEYSTORE_NAME);
Collection<String> properties = getViolationProperties(validator.validate(factory));
Expand All @@ -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());

Expand All @@ -142,7 +160,7 @@ public void canBuildContextFactoryWhenWindowsKeyStoreAvailable() throws Exceptio
}

@Test
public void windowsKeyStoreUnavailableThrowsException() throws Exception {
void windowsKeyStoreUnavailableThrowsException() {
assumeFalse(canAccessWindowsKeyStore());

final HttpsConnectorFactory factory = new HttpsConnectorFactory();
Expand All @@ -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);
Expand Down Expand Up @@ -196,15 +214,15 @@ 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);
assertThat(jetty93SslConnectionFacttory.getTimer()).isSameAs(
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");
Expand Down Expand Up @@ -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)
Expand All @@ -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<Boolean, List<String>> partition =
Expand All @@ -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.*"};
Expand All @@ -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*"};
Expand Down

0 comments on commit 206e858

Please sign in to comment.