Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -596,18 +596,16 @@ private ServerConnector createHttpsChannelConnector(
private void setEnabledProtocols(SslContextFactory sslContextFactory) {
String enabledProtocols = conf.get(OzoneConfigKeys.OZONE_SSL_ENABLED_PROTOCOLS,
conf.get(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT));
if (!enabledProtocols.equals(SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT)) {
List<String> originalExcludedProtocols = Arrays.asList(sslContextFactory.getExcludeProtocols());
String[] enabledProtocolsArray = StringUtils.getTrimmedStrings(enabledProtocols);
List<String> originalExcludedProtocols = Arrays.asList(sslContextFactory.getExcludeProtocols());
String[] enabledProtocolsArray = StringUtils.getTrimmedStrings(enabledProtocols);

List<String> finalExcludedProtocols = new ArrayList<>(originalExcludedProtocols);
finalExcludedProtocols.removeAll(Arrays.asList(enabledProtocolsArray));
List<String> finalExcludedProtocols = new ArrayList<>(originalExcludedProtocols);
finalExcludedProtocols.removeAll(Arrays.asList(enabledProtocolsArray));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it mean that you want whitelisted protocols to be removed from the blacklist? Usually exclude list overrides white list.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it is a Jetty specific semantics. Yes, exclude list takes precendece over white list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change this behaviour (it's only modified because of removing the if condition and needed to adjust them). This behaviour was ported in #10111 from HADOOP-15169 and HADOOP-19546 to Ozone's HttpServer2 fork. There is a comment in Hadoop, which explains it:

      // Jetty 9.2.4.v20141103 and above excludes certain protocols by
      // default. Remove the user enabled protocols from the exclude list,
      // and add them into the include list.

SslContextFactory in Jetty applies excludes with higher priority than includes.


sslContextFactory.setExcludeProtocols(finalExcludedProtocols.toArray(new String[0]));
LOG.info("Disabled protocols: {}", finalExcludedProtocols);
sslContextFactory.setIncludeProtocols(enabledProtocolsArray);
LOG.info("Enabled protocols: {}", enabledProtocols);
}
sslContextFactory.setExcludeProtocols(finalExcludedProtocols.toArray(new String[0]));
LOG.info("Disabled protocols: {}", finalExcludedProtocols);
sslContextFactory.setIncludeProtocols(enabledProtocolsArray);
LOG.info("Enabled protocols: {}", enabledProtocols);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

package org.apache.hadoop.hdds.server.http;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.io.File;
Expand All @@ -29,6 +32,7 @@
import java.net.URI;
import java.net.URL;
import java.security.KeyStore;
import java.util.Arrays;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLHandshakeException;
Expand All @@ -41,6 +45,9 @@
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.security.ssl.KeyStoreTestUtil;
import org.apache.hadoop.security.ssl.SSLFactory;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -169,9 +176,58 @@ public void testDefaultConfigAcceptsConnection() throws Exception {
}
}

@Test
public void testEnabledProtocolAppliedWhenConfigUnset() throws Exception {
OzoneConfiguration serverConf = new OzoneConfiguration(conf);
serverConf.unset(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY);
assertServerAppliesEnabledProtocol(serverConf, SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
}

@Test
public void testEnabledProtocolAppliedWhenConfigSetToDefault() throws Exception {
OzoneConfiguration serverConf = new OzoneConfiguration(conf);
serverConf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
assertServerAppliesEnabledProtocol(serverConf, SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
}

@Test
public void testEnabledProtocolAppliedWhenConfigSetToNonDefault() throws Exception {
OzoneConfiguration serverConf = new OzoneConfiguration(conf);
serverConf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, "TLSv1.3");
assertServerAppliesEnabledProtocol(serverConf, "TLSv1.3");
}

private void assertServerAppliesEnabledProtocol(
OzoneConfiguration serverConf, String protocol) throws Exception {
HttpServer2 server = buildServer(serverConf, null, null, null);
server.start();
try {
ServerConnector listener = server.getListeners().get(0);
SslConnectionFactory connectionFactory =
listener.getConnectionFactory(SslConnectionFactory.class);
assertNotNull(connectionFactory,
"Expected HTTPS listener with an SSL connection factory");

SslContextFactory.Server sslContextFactory =
(SslContextFactory.Server) connectionFactory.getSslContextFactory();
assertArrayEquals(new String[] {protocol},
sslContextFactory.getIncludeProtocols());
assertFalse(Arrays.asList(sslContextFactory.getExcludeProtocols())
.contains(protocol),
"Configured enabled protocol should be removed from excluded protocols");
} finally {
server.stop();
}
}

private HttpServer2 buildServer(String excludeCiphers, String includeCiphers, String enabledProtocols)
throws Exception {
OzoneConfiguration serverConf = new OzoneConfiguration(conf);
return buildServer(serverConf, excludeCiphers, includeCiphers, enabledProtocols);
}

private HttpServer2 buildServer(OzoneConfiguration serverConf, String excludeCiphers,
String includeCiphers, String enabledProtocols) throws Exception {
if (enabledProtocols != null) {
serverConf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, enabledProtocols);
}
Expand Down