[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544037#comment-16544037 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202506811 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -163,80 +163,157 @@ public static void setSSLVerifyHostname(Configuration sslConfig, SSLParameters s } /** -* Creates the SSL Context for the client if SSL is configured. +* SSL engine provider. +*/ + public enum SSLProvider { + JDK, + /** +* OpenSSL with fallback to JDK if not available. +*/ + OPENSSL; + + public static SSLProvider fromString(String value) { + Preconditions.checkNotNull(value); + if (value.equalsIgnoreCase("OPENSSL")) { + return OPENSSL; + } else if (value.equalsIgnoreCase("JDK")) { + return JDK; + } else { + throw new IllegalArgumentException("Unknown SSL provider: " + value); + } + } + } + + /** +* Instances needed to set up an SSL client connection. +*/ + public static class SSLClientTools { --- End diff -- the class name use singular looks better to me > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544033#comment-16544033 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202506694 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -249,14 +326,40 @@ public static SSLContext createSSLServerContext(Configuration sslConfig) throws // Set up key manager factory to use the server key store KeyManagerFactory kmf = KeyManagerFactory.getInstance( - KeyManagerFactory.getDefaultAlgorithm()); + KeyManagerFactory.getDefaultAlgorithm()); kmf.init(ks, certPassword.toCharArray()); + return new SSLServerTools(sslProvider, sslProtocolVersion, sslCipherSuites, kmf); + } + + return null; + } + + /** +* Creates the SSL Context for the server if SSL is configured. +* +* @param sslConfig +*The application configuration --- End diff -- the description of the param and throws do not need linefeed > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544031#comment-16544031 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202505334 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -249,14 +326,40 @@ public static SSLContext createSSLServerContext(Configuration sslConfig) throws // Set up key manager factory to use the server key store KeyManagerFactory kmf = KeyManagerFactory.getInstance( - KeyManagerFactory.getDefaultAlgorithm()); + KeyManagerFactory.getDefaultAlgorithm()); kmf.init(ks, certPassword.toCharArray()); + return new SSLServerTools(sslProvider, sslProtocolVersion, sslCipherSuites, kmf); + } + + return null; + } + + /** +* Creates the SSL Context for the server if SSL is configured. +* +* @param sslConfig +*The application configuration +* @return The SSLContext object which can be used by the ssl transport server +* Returns null if SSL is disabled +* @throws Exception +* Thrown if there is any misconfiguration +*/ + @Nullable + public static SSLContext createSSLServerContext(Configuration sslConfig) throws Exception { + --- End diff -- this empty line is useless, can be removed > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544030#comment-16544030 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202505321 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/io/network/netty/NettyClientServerSslTest.java --- @@ -160,6 +160,7 @@ private Configuration createSslConfig() throws Exception { flinkConfig.setString(SecurityOptions.SSL_KEY_PASSWORD, "password"); flinkConfig.setString(SecurityOptions.SSL_TRUSTSTORE, "src/test/resources/local127.truststore"); flinkConfig.setString(SecurityOptions.SSL_TRUSTSTORE_PASSWORD, "password"); +// flinkConfig.setString(SecurityOptions.SSL_PROVIDER, "OPENSSL"); --- End diff -- if this is a useless dead code, can be removed > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544036#comment-16544036 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202506868 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -163,80 +163,157 @@ public static void setSSLVerifyHostname(Configuration sslConfig, SSLParameters s } /** -* Creates the SSL Context for the client if SSL is configured. +* SSL engine provider. +*/ + public enum SSLProvider { + JDK, + /** +* OpenSSL with fallback to JDK if not available. +*/ + OPENSSL; + + public static SSLProvider fromString(String value) { + Preconditions.checkNotNull(value); + if (value.equalsIgnoreCase("OPENSSL")) { --- End diff -- provide a constructor like `SSLProvider(String provider)` to give the enum's string representation looks better than hard code. > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544034#comment-16544034 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202506769 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -163,80 +163,157 @@ public static void setSSLVerifyHostname(Configuration sslConfig, SSLParameters s } /** -* Creates the SSL Context for the client if SSL is configured. +* SSL engine provider. +*/ + public enum SSLProvider { + JDK, + /** +* OpenSSL with fallback to JDK if not available. +*/ + OPENSSL; + + public static SSLProvider fromString(String value) { + Preconditions.checkNotNull(value); + if (value.equalsIgnoreCase("OPENSSL")) { + return OPENSSL; + } else if (value.equalsIgnoreCase("JDK")) { + return JDK; + } else { + throw new IllegalArgumentException("Unknown SSL provider: " + value); + } + } + } + + /** +* Instances needed to set up an SSL client connection. +*/ + public static class SSLClientTools { + public final SSLProvider preferredSslProvider; + public final String sslProtocolVersion; + public final TrustManagerFactory trustManagerFactory; + + public SSLClientTools( + SSLProvider preferredSslProvider, + String sslProtocolVersion, + TrustManagerFactory trustManagerFactory) { + this.preferredSslProvider = preferredSslProvider; + this.sslProtocolVersion = sslProtocolVersion; + this.trustManagerFactory = trustManagerFactory; + } + } + + /** +* Creates necessary helper objects to use for creating an SSL Context for the client if SSL is +* configured. * * @param sslConfig *The application configuration -* @return The SSLContext object which can be used by the ssl transport client -* Returns null if SSL is disabled +* @return The SSLClientTools object which can be used for creating some SSL context object; +* returns null if SSL is disabled. * @throws Exception * Thrown if there is any misconfiguration */ @Nullable - public static SSLContext createSSLClientContext(Configuration sslConfig) throws Exception { - + public static SSLClientTools createSSLClientTools(Configuration sslConfig) throws Exception { Preconditions.checkNotNull(sslConfig); - SSLContext clientSSLContext = null; if (getSSLEnabled(sslConfig)) { LOG.debug("Creating client SSL context from configuration"); String trustStoreFilePath = sslConfig.getString(SecurityOptions.SSL_TRUSTSTORE); String trustStorePassword = sslConfig.getString(SecurityOptions.SSL_TRUSTSTORE_PASSWORD); String sslProtocolVersion = sslConfig.getString(SecurityOptions.SSL_PROTOCOL); + SSLProvider sslProvider = SSLProvider.fromString(sslConfig.getString(SecurityOptions.SSL_PROVIDER)); Preconditions.checkNotNull(trustStoreFilePath, SecurityOptions.SSL_TRUSTSTORE.key() + " was not configured."); Preconditions.checkNotNull(trustStorePassword, SecurityOptions.SSL_TRUSTSTORE_PASSWORD.key() + " was not configured."); KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); - FileInputStream trustStoreFile = null; - try { - trustStoreFile = new FileInputStream(new File(trustStoreFilePath)); + try (FileInputStream trustStoreFile = new FileInputStream(new File(trustStoreFilePath))) { trustStore.load(trustStoreFile, trustStorePassword.toCharArray()); - } finally { - if (trustStoreFile != null) { - trustStoreFile.close(); - } } TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544032#comment-16544032 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202506709 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -163,80 +163,157 @@ public static void setSSLVerifyHostname(Configuration sslConfig, SSLParameters s } /** -* Creates the SSL Context for the client if SSL is configured. +* SSL engine provider. +*/ + public enum SSLProvider { + JDK, + /** +* OpenSSL with fallback to JDK if not available. +*/ + OPENSSL; + + public static SSLProvider fromString(String value) { + Preconditions.checkNotNull(value); + if (value.equalsIgnoreCase("OPENSSL")) { + return OPENSSL; + } else if (value.equalsIgnoreCase("JDK")) { + return JDK; + } else { + throw new IllegalArgumentException("Unknown SSL provider: " + value); + } + } + } + + /** +* Instances needed to set up an SSL client connection. +*/ + public static class SSLClientTools { + public final SSLProvider preferredSslProvider; + public final String sslProtocolVersion; + public final TrustManagerFactory trustManagerFactory; + + public SSLClientTools( + SSLProvider preferredSslProvider, + String sslProtocolVersion, + TrustManagerFactory trustManagerFactory) { + this.preferredSslProvider = preferredSslProvider; + this.sslProtocolVersion = sslProtocolVersion; + this.trustManagerFactory = trustManagerFactory; + } + } + + /** +* Creates necessary helper objects to use for creating an SSL Context for the client if SSL is +* configured. * * @param sslConfig *The application configuration -* @return The SSLContext object which can be used by the ssl transport client -* Returns null if SSL is disabled +* @return The SSLClientTools object which can be used for creating some SSL context object; +* returns null if SSL is disabled. * @throws Exception * Thrown if there is any misconfiguration */ @Nullable - public static SSLContext createSSLClientContext(Configuration sslConfig) throws Exception { - + public static SSLClientTools createSSLClientTools(Configuration sslConfig) throws Exception { Preconditions.checkNotNull(sslConfig); - SSLContext clientSSLContext = null; if (getSSLEnabled(sslConfig)) { LOG.debug("Creating client SSL context from configuration"); String trustStoreFilePath = sslConfig.getString(SecurityOptions.SSL_TRUSTSTORE); String trustStorePassword = sslConfig.getString(SecurityOptions.SSL_TRUSTSTORE_PASSWORD); String sslProtocolVersion = sslConfig.getString(SecurityOptions.SSL_PROTOCOL); + SSLProvider sslProvider = SSLProvider.fromString(sslConfig.getString(SecurityOptions.SSL_PROVIDER)); Preconditions.checkNotNull(trustStoreFilePath, SecurityOptions.SSL_TRUSTSTORE.key() + " was not configured."); Preconditions.checkNotNull(trustStorePassword, SecurityOptions.SSL_TRUSTSTORE_PASSWORD.key() + " was not configured."); KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); - FileInputStream trustStoreFile = null; - try { - trustStoreFile = new FileInputStream(new File(trustStoreFilePath)); + try (FileInputStream trustStoreFile = new FileInputStream(new File(trustStoreFilePath))) { trustStore.load(trustStoreFile, trustStorePassword.toCharArray()); - } finally { - if (trustStoreFile != null) { - trustStoreFile.close(); - } } TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16544035#comment-16544035 ] ASF GitHub Bot commented on FLINK-9816: --- Github user yanghua commented on a diff in the pull request: https://github.com/apache/flink/pull/6328#discussion_r202506820 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java --- @@ -163,80 +163,157 @@ public static void setSSLVerifyHostname(Configuration sslConfig, SSLParameters s } /** -* Creates the SSL Context for the client if SSL is configured. +* SSL engine provider. +*/ + public enum SSLProvider { + JDK, + /** +* OpenSSL with fallback to JDK if not available. +*/ + OPENSSL; + + public static SSLProvider fromString(String value) { + Preconditions.checkNotNull(value); + if (value.equalsIgnoreCase("OPENSSL")) { + return OPENSSL; + } else if (value.equalsIgnoreCase("JDK")) { + return JDK; + } else { + throw new IllegalArgumentException("Unknown SSL provider: " + value); + } + } + } + + /** +* Instances needed to set up an SSL client connection. +*/ + public static class SSLClientTools { + public final SSLProvider preferredSslProvider; + public final String sslProtocolVersion; + public final TrustManagerFactory trustManagerFactory; --- End diff -- mark these fields as `private` as provide `getter/setter` looks better to me > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16542783#comment-16542783 ] ASF GitHub Bot commented on FLINK-9816: --- Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/6328 Could you rebase this on top of #6326 ? That PR makes sure SSLEngine factories are used everywhere, giving a single point to integrate the provider such that it is available for all SSL endpoints. > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-9816) Support Netty SslEngine based on openSSL
[ https://issues.apache.org/jira/browse/FLINK-9816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16542748#comment-16542748 ] ASF GitHub Bot commented on FLINK-9816: --- GitHub user NicoK opened a pull request: https://github.com/apache/flink/pull/6328 [FLINK-9816][network] add option to configure SSL engine provider for TM communication ## What is the purpose of the change Netty has the ability to run with different `SSLEngine` implementations but with our current setup, we are fixed to the JDK implementation, although one based on OpenSSL is expected to be faster [1]. We should make this configurable and ideally also provide everything needed to run with OpenSSL in the future (the last part is not part of this PR). [1] https://netty.io/wiki/requirements-for-4.x.html#benefits-of-using-openssl ## Brief change log - allow selecting the SSL engine provider via `security.ssl.provider` - set up Netty SSL handler with its `SslContextBuilder` (in `NettyConfig`) to have this flexibility ## Verifying this change This change can be verified as follows: - I verified by running an SSL setup with 2 TMs and submitting a job through the WebUI with the default `JDK` SSL engine and `OPENSSL` using a custom build using `netty-tcnative` with statically linked boringssl libraries from http://netty.io/wiki/forked-tomcat-native.html - there is an end-to-end test in #6327 which is currently blocked on the CLI submission not working with SSL ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): **no** - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: **no** - The serializers: **no** - The runtime per-record code paths (performance sensitive): **no** - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: **no** - The S3 file system connector: **no** ## Documentation - Does this pull request introduce a new feature? **no** - If yes, how is the feature documented? **docs, JavaDocs** You can merge this pull request into a Git repository by running: $ git pull https://github.com/NicoK/flink flink-9816 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/6328.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6328 commit d6fb90a0c4cb1b105c482d982f8ab84744a80dc8 Author: Nico Kruber Date: 2018-07-11T20:10:22Z [hotfix][typo] fix a deprecation message commit 83c00b9cd5e1178a1152feac04bbc7a68213deb3 Author: Nico Kruber Date: 2018-07-11T21:06:04Z [hotfix][checkstyle] fix a warning in NettyConfig commit af8486d587a5dbc553fec42d80180a1f6ecc1571 Author: Nico Kruber Date: 2018-07-11T21:05:01Z [FLINK-9816][network] add option to configure SSL engine provider for TM communication This prepares Flink to use OpenSSL for TM communication channels via netty. Currently, there is no easy way to provide the required native libraries, though. We'll either include these in a future version of flink-shaded or update instructions on how to include/build them manually. > Support Netty SslEngine based on openSSL > > > Key: FLINK-9816 > URL: https://issues.apache.org/jira/browse/FLINK-9816 > Project: Flink > Issue Type: Improvement > Components: Network >Reporter: Nico Kruber >Assignee: Nico Kruber >Priority: Major > Labels: pull-request-available > > Since a while now, Netty does not only support the JDK's {{SSLEngine}} but > also implements one based on openSSL which, according to > https://netty.io/wiki/requirements-for-4.x.html#wiki-h4-4 is significantly > faster. We should add support for using that engine instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)