HDDS-15152. SSL protocol config is not applied to Jetty when set to default value#10165
Conversation
|
need unit tests |
|
Thanks for the review @jojochuang, in the latest commit I added unit tests (non default value/default value/no config set) |
|
@octachoron could you please review it? thanks! |
| List<String> finalExcludedProtocols = new ArrayList<>(originalExcludedProtocols); | ||
| finalExcludedProtocols.removeAll(Arrays.asList(enabledProtocolsArray)); | ||
| List<String> finalExcludedProtocols = new ArrayList<>(originalExcludedProtocols); | ||
| finalExcludedProtocols.removeAll(Arrays.asList(enabledProtocolsArray)); |
There was a problem hiding this comment.
does it mean that you want whitelisted protocols to be removed from the blacklist? Usually exclude list overrides white list.
There was a problem hiding this comment.
IIRC it is a Jetty specific semantics. Yes, exclude list takes precendece over white list.
There was a problem hiding this comment.
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.
|
Thanks @dombizita for the patch, @jojochuang, @yandrey321 for the review. |
|
Thank you for the review @adoroszlai, @jojochuang and @yandrey321! |
4 participants
What changes were proposed in this pull request?
In
HttpServer2.setEnabledProtocols(), the logic that applies SSL protocol restrictions to the Jetty SslContextFactory is gated behind a check that compares the resolved configuration value againstSSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT("TLSv1.2").This means that Jetty is not respecting the configuration, the condition check should be removed.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15152
How was this patch tested?
Tested the changes manually with
openssl s_client -connectcommands.