You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/12/14 19:46:43 UTC

[GitHub] [nifi] exceptionfactory opened a new pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

exceptionfactory opened a new pull request #4673:
URL: https://github.com/apache/nifi/pull/4673


   #### Description of PR
   
   NIFI-8019 Added TlsPlatform utility class and updated multiple unit tests to use the getDefaultProtocols() and getLatestProtocol() methods.  Fedora 33 introduced a default Java Security configuration that disabled TLSv1 and TLSv1.1, which results in build failures for tests that expected those TLS protocols.  This problem can be duplicated on other platforms by adding TLSv1 and TLSv1.1 to jdk.tls.disabledAlgorithms in the java.security configuration.  The TlsPlatform class uses the SSLContext.getDefaultSSLParameters().getProtocols() method to determine supported protocols at runtime.  This approach removes the need for checking Java versions in some tests.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [X] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [X] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [X] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
   
   - [X] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
   
   ### For code changes:
   - [X] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
   - [X] Have you written or updated unit tests to verify your changes?
   - [X] Have you verified that the full build is successful on JDK 8?
   - [X] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] mtien-apache commented on pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#issuecomment-740193497


   Reviewing.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on a change in pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#discussion_r541266376



##########
File path: nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/TlsPlatform.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.security.util;
+
+import static java.util.Collections.unmodifiableSet;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.security.NoSuchAlgorithmException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Transport Layer Security Platform provides runtime protocol configuration information
+ */
+public class TlsPlatform {
+    private static final Pattern PROTOCOL_VERSION = Pattern.compile("TLSv(\\d+\\.?\\d*)");
+
+    private static final int FIRST_GROUP = 1;
+
+    private static final List<String> LEGACY_PROTOCOLS = Arrays.asList("TLSv1", "TLSv1.1");
+
+    private static final SortedMap<Float, String> SORTED_PROTOCOLS = getDefaultSslContextProtocols();
+
+    private static final Set<String> DEFAULT_PROTOCOLS = unmodifiableSet(new TreeSet<>(SORTED_PROTOCOLS.values()).descendingSet());
+
+    private static final Set<String> PREFERRED_PROTOCOLS = unmodifiableSet(
+            DEFAULT_PROTOCOLS.stream()
+            .filter(protocol -> !LEGACY_PROTOCOLS.contains(protocol))
+            .collect(Collectors.toSet())
+    );
+
+    /**
+     * Get Default Protocols based on Java Security configuration
+     *
+     * @return Set of Transport Layer Security Protocol names
+     */
+    public static Set<String> getDefaultProtocols() {

Review comment:
       This method returns all available protocols, and not necessarily the protocol we will/want to 'default' to. In what circumstances should this method be called? Situations where we allow legacy protocols (outgoing processors I believe)? I think the naming and comment can be more clear about when we expect this to be used and when it should be used over the other methods. getAllAvailableProtocols() is more accurate. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] exceptionfactory commented on a change in pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#discussion_r541336803



##########
File path: nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/TlsPlatform.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.security.util;
+
+import static java.util.Collections.unmodifiableSet;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.security.NoSuchAlgorithmException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Transport Layer Security Platform provides runtime protocol configuration information
+ */
+public class TlsPlatform {
+    private static final Pattern PROTOCOL_VERSION = Pattern.compile("TLSv(\\d+\\.?\\d*)");
+
+    private static final int FIRST_GROUP = 1;
+
+    private static final List<String> LEGACY_PROTOCOLS = Arrays.asList("TLSv1", "TLSv1.1");
+
+    private static final SortedMap<Float, String> SORTED_PROTOCOLS = getDefaultSslContextProtocols();
+
+    private static final Set<String> DEFAULT_PROTOCOLS = unmodifiableSet(new TreeSet<>(SORTED_PROTOCOLS.values()).descendingSet());
+
+    private static final Set<String> PREFERRED_PROTOCOLS = unmodifiableSet(
+            DEFAULT_PROTOCOLS.stream()
+            .filter(protocol -> !LEGACY_PROTOCOLS.contains(protocol))
+            .collect(Collectors.toSet())
+    );
+
+    /**
+     * Get Default Protocols based on Java Security configuration
+     *
+     * @return Set of Transport Layer Security Protocol names
+     */
+    public static Set<String> getDefaultProtocols() {
+        return DEFAULT_PROTOCOLS;
+    }
+
+    /**
+     * Get Preferred Protocols based on default protocols with legacy protocols removed
+     *
+     * @return Set of Preferred Transport Layer Security Protocol names
+     */
+    public static Set<String> getPreferredProtocols() {
+        return PREFERRED_PROTOCOLS;
+    }
+
+    /**
+     * Get Latest Protocol based on high version number from default protocols in Java Security configuration
+     *
+     * @return Latest Transport Layer Security Protocol
+     */
+    public static String getLatestProtocol() {
+        return SORTED_PROTOCOLS.get(SORTED_PROTOCOLS.lastKey());

Review comment:
       This is somewhat complicated since the Java runtime behavior is can be controlled using the `jdk.tls.disabledAlgorithms` property in the `java.security` configuration.  It is theoretically possible to disable modern protocols such as TLSv1.2 and TLSv1.3 using that configuration property, which would influence that this method returns.  The primary purpose of this class is to provide a _descriptive_ approach to what is running.  The response of this method could in fact return TLSv1 or TLSv1.1, but it seems like it should be the responsibility of another NiFi class to determine whether to flag that as problem.
   
   For that reason, it is theoretically possible for PREFERRED_PROTOCOLS to be empty, should TLSv1.2 and TLSv1.3 be disabled.  As a class that should reflect how the JVM is currently running, it seems better to maintain this behavior and consider other approaches if it is necessary for NiFi to require TLSv1.2 or greater for acceptable operation.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] mtien-apache commented on pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#issuecomment-741499574


   +1 LGTM. I ran a full build and ran each test class that was changed with:
   
   - JDK 8u231
   - AdoptOpenJDK 8 Update 275
   - JDK 11.0.5
   
   I debugged through`TlsPlatform` and verified it gets the correct TLS protocols during runtime according to the Java versions. I also verified the tests were able to build when disabling legacy TLS protocols of TLSv1 and TLSv1.1 by configuring the `java.security` file. Thank you. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on a change in pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#discussion_r541302184



##########
File path: nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/TlsPlatform.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.security.util;
+
+import static java.util.Collections.unmodifiableSet;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.security.NoSuchAlgorithmException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Transport Layer Security Platform provides runtime protocol configuration information
+ */
+public class TlsPlatform {
+    private static final Pattern PROTOCOL_VERSION = Pattern.compile("TLSv(\\d+\\.?\\d*)");
+
+    private static final int FIRST_GROUP = 1;
+
+    private static final List<String> LEGACY_PROTOCOLS = Arrays.asList("TLSv1", "TLSv1.1");
+
+    private static final SortedMap<Float, String> SORTED_PROTOCOLS = getDefaultSslContextProtocols();
+
+    private static final Set<String> DEFAULT_PROTOCOLS = unmodifiableSet(new TreeSet<>(SORTED_PROTOCOLS.values()).descendingSet());
+
+    private static final Set<String> PREFERRED_PROTOCOLS = unmodifiableSet(
+            DEFAULT_PROTOCOLS.stream()
+            .filter(protocol -> !LEGACY_PROTOCOLS.contains(protocol))
+            .collect(Collectors.toSet())
+    );
+
+    /**
+     * Get Default Protocols based on Java Security configuration
+     *
+     * @return Set of Transport Layer Security Protocol names
+     */
+    public static Set<String> getDefaultProtocols() {
+        return DEFAULT_PROTOCOLS;
+    }
+
+    /**
+     * Get Preferred Protocols based on default protocols with legacy protocols removed
+     *
+     * @return Set of Preferred Transport Layer Security Protocol names
+     */
+    public static Set<String> getPreferredProtocols() {
+        return PREFERRED_PROTOCOLS;
+    }
+
+    /**
+     * Get Latest Protocol based on high version number from default protocols in Java Security configuration
+     *
+     * @return Latest Transport Layer Security Protocol
+     */
+    public static String getLatestProtocol() {
+        return SORTED_PROTOCOLS.get(SORTED_PROTOCOLS.lastKey());

Review comment:
       Because this uses SORTED_PROTOCOLS which is a sorted set of all available protocols, this can allow use of protocols less than TLSv1.2 if TLSv1.2+ are not found to be available at the JVM level. In other words, there is no explicit control of protocol versions from NiFi code.
   
   Is it reasonable to allow NiFi to operate for example on TLSv1.0/TLSv1.1 because it's the 'latest' available protocol attained, or should we explicitly deny this situation? Is it possible code could modify the available protocols at the JVM level to create a TLS downgrade attack? Should getLatestProtocol() only use PREFERRED_PROTOCOLS?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] thenatog closed pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
thenatog closed pull request #4673:
URL: https://github.com/apache/nifi/pull/4673


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] exceptionfactory commented on a change in pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#discussion_r541328922



##########
File path: nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/TlsPlatform.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.security.util;
+
+import static java.util.Collections.unmodifiableSet;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.security.NoSuchAlgorithmException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Transport Layer Security Platform provides runtime protocol configuration information
+ */
+public class TlsPlatform {
+    private static final Pattern PROTOCOL_VERSION = Pattern.compile("TLSv(\\d+\\.?\\d*)");
+
+    private static final int FIRST_GROUP = 1;
+
+    private static final List<String> LEGACY_PROTOCOLS = Arrays.asList("TLSv1", "TLSv1.1");
+
+    private static final SortedMap<Float, String> SORTED_PROTOCOLS = getDefaultSslContextProtocols();
+
+    private static final Set<String> DEFAULT_PROTOCOLS = unmodifiableSet(new TreeSet<>(SORTED_PROTOCOLS.values()).descendingSet());
+
+    private static final Set<String> PREFERRED_PROTOCOLS = unmodifiableSet(
+            DEFAULT_PROTOCOLS.stream()
+            .filter(protocol -> !LEGACY_PROTOCOLS.contains(protocol))
+            .collect(Collectors.toSet())
+    );
+
+    /**
+     * Get Default Protocols based on Java Security configuration
+     *
+     * @return Set of Transport Layer Security Protocol names
+     */
+    public static Set<String> getDefaultProtocols() {

Review comment:
       Thanks for the feedback.  The initial naming approach was based on these protocols being derived from the SSLContext.getDefault() instance, as indicated by the comment mentioning the Java Security configuration and the fact of this class being named TlsPlatform.  This method is referenced in several unit tests which verify that the Default SSL Parameters were not changed as a result of setting enabled protocols.  This method could also be used later to support enumerating the list of protocols available in the StandardSSLContextService.  The method response is intended to indicate what the current JVM supports at runtime.  What do you think about naming it getSupportedProtocols?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] exceptionfactory commented on pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#issuecomment-743303160


   @mtien-apache Thanks for the review and testing. I reverted one change to TestListenSMTP since that was conflicting due to the merge of NIFI-7913.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] exceptionfactory commented on a change in pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#discussion_r542822592



##########
File path: nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/TlsPlatform.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.security.util;
+
+import static java.util.Collections.unmodifiableSet;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.security.NoSuchAlgorithmException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Transport Layer Security Platform provides runtime protocol configuration information
+ */
+public class TlsPlatform {
+    private static final Pattern PROTOCOL_VERSION = Pattern.compile("TLSv(\\d+\\.?\\d*)");
+
+    private static final int FIRST_GROUP = 1;
+
+    private static final List<String> LEGACY_PROTOCOLS = Arrays.asList("TLSv1", "TLSv1.1");
+
+    private static final SortedMap<Float, String> SORTED_PROTOCOLS = getDefaultSslContextProtocols();
+
+    private static final Set<String> DEFAULT_PROTOCOLS = unmodifiableSet(new TreeSet<>(SORTED_PROTOCOLS.values()).descendingSet());
+
+    private static final Set<String> PREFERRED_PROTOCOLS = unmodifiableSet(
+            DEFAULT_PROTOCOLS.stream()
+            .filter(protocol -> !LEGACY_PROTOCOLS.contains(protocol))
+            .collect(Collectors.toSet())
+    );
+
+    /**
+     * Get Default Protocols based on Java Security configuration
+     *
+     * @return Set of Transport Layer Security Protocol names
+     */
+    public static Set<String> getDefaultProtocols() {

Review comment:
       That sounds good, I pushed an update renaming the method to getSupportedProtocols.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#issuecomment-744887599


   +1 LGTM, will merge.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] thenatog closed pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
thenatog closed pull request #4673:
URL: https://github.com/apache/nifi/pull/4673


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [nifi] thenatog commented on a change in pull request #4673: NIFI-8019 Added TlsPlatform to provide runtime default TLS Protocols

Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4673:
URL: https://github.com/apache/nifi/pull/4673#discussion_r542779005



##########
File path: nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/TlsPlatform.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.security.util;
+
+import static java.util.Collections.unmodifiableSet;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.security.NoSuchAlgorithmException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Transport Layer Security Platform provides runtime protocol configuration information
+ */
+public class TlsPlatform {
+    private static final Pattern PROTOCOL_VERSION = Pattern.compile("TLSv(\\d+\\.?\\d*)");
+
+    private static final int FIRST_GROUP = 1;
+
+    private static final List<String> LEGACY_PROTOCOLS = Arrays.asList("TLSv1", "TLSv1.1");
+
+    private static final SortedMap<Float, String> SORTED_PROTOCOLS = getDefaultSslContextProtocols();
+
+    private static final Set<String> DEFAULT_PROTOCOLS = unmodifiableSet(new TreeSet<>(SORTED_PROTOCOLS.values()).descendingSet());
+
+    private static final Set<String> PREFERRED_PROTOCOLS = unmodifiableSet(
+            DEFAULT_PROTOCOLS.stream()
+            .filter(protocol -> !LEGACY_PROTOCOLS.contains(protocol))
+            .collect(Collectors.toSet())
+    );
+
+    /**
+     * Get Default Protocols based on Java Security configuration
+     *
+     * @return Set of Transport Layer Security Protocol names
+     */
+    public static Set<String> getDefaultProtocols() {

Review comment:
       Yeah I think getSupportedProtocols works, with a comment that the protocols returned are _all_ the protocols supported by the JVM.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org