You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by rpahli <gi...@git.apache.org> on 2018/02/01 09:37:35 UTC
[GitHub] activemq-artemis pull request #1834: ARTEMIS-1649 - enable openssl provider ...
GitHub user rpahli opened a pull request:
https://github.com/apache/activemq-artemis/pull/1834
ARTEMIS-1649 - enable openssl provider for Netty
We want to use the native Openssl Provider for netty to use the native openssl.
Added the supprt to switch between JDK and OpenSSL Provider.
see #1833
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kiwigrid/activemq-artemis enable-openssl-provider
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1834.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 #1834
----
commit bb0bea377fb89e5b44bbe03818e2c78602448fc0
Author: rico.pahlisch <ri...@...>
Date: 2018-02-01T09:35:01Z
code formating
----
---
[GitHub] activemq-artemis pull request #1834: ARTEMIS-1649 - enable openssl provider ...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1834#discussion_r165308928
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java ---
@@ -530,48 +539,34 @@ public synchronized void start() {
if (System.getProperty(ACTIVEMQ_TRUSTSTORE_PASSWORD_PROP_NAME) != null) {
realTrustStorePassword = System.getProperty(ACTIVEMQ_TRUSTSTORE_PASSWORD_PROP_NAME);
}
- context = SSLSupport.createContext(realKeyStoreProvider, realKeyStorePath, realKeyStorePassword, realTrustStoreProvider, realTrustStorePath, realTrustStorePassword, trustAll, crlPath);
+
}
} catch (Exception e) {
close();
IllegalStateException ise = new IllegalStateException("Unable to create NettyConnector for " + host + ":" + port);
ise.initCause(e);
throw ise;
}
- } else {
- context = null; // Unused
}
- if (context != null && useServlet) {
- // TODO: Fix me
- //bootstrap.setOption("sslContext", context);
- }
+ //if (context != null && useServlet) {
--- End diff --
If isn't needed anymore please remove it
---
[GitHub] activemq-artemis pull request #1834: ARTEMIS-1649 - enable openssl provider ...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1834#discussion_r165305348
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java ---
@@ -530,48 +539,34 @@ public synchronized void start() {
if (System.getProperty(ACTIVEMQ_TRUSTSTORE_PASSWORD_PROP_NAME) != null) {
realTrustStorePassword = System.getProperty(ACTIVEMQ_TRUSTSTORE_PASSWORD_PROP_NAME);
}
- context = SSLSupport.createContext(realKeyStoreProvider, realKeyStorePath, realKeyStorePassword, realTrustStoreProvider, realTrustStorePath, realTrustStorePassword, trustAll, crlPath);
+
}
} catch (Exception e) {
close();
IllegalStateException ise = new IllegalStateException("Unable to create NettyConnector for " + host + ":" + port);
ise.initCause(e);
throw ise;
}
- } else {
- context = null; // Unused
}
- if (context != null && useServlet) {
- // TODO: Fix me
- //bootstrap.setOption("sslContext", context);
- }
+ //if (context != null && useServlet) {
+ // TODO: Fix me
+ //bootstrap.setOption("sslContext", context);
+ //}
bootstrap.handler(new ChannelInitializer<Channel>() {
--- End diff --
this probably could be made a lamda.
---
[GitHub] activemq-artemis issue #1834: ARTEMIS-1649 - enable openssl provider for Net...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1834
Could you update the SSL documentation in docs/user-manual/en/configuring-transports.md to include the new "sslProvider" parameter including a short explanation of why the JDK provider might be useful vs. the OpenSSL provider and vice-versa? It's not clear from your commit what the functional benefit of this change actually is.
---
[GitHub] activemq-artemis pull request #1834: ARTEMIS-1649 - enable openssl provider ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1834
---
[GitHub] activemq-artemis issue #1834: ARTEMIS-1649 - enable openssl provider for Net...
Posted by rpahli <gi...@git.apache.org>.
Github user rpahli commented on the issue:
https://github.com/apache/activemq-artemis/pull/1834
Ok I'll add the new parameter to the documentation.
We need the openssl provider to use some special combinations of ciphersuites and elliptic curves.
see https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations
Should I add these link to the documentation too?
---
[GitHub] activemq-artemis pull request #1834: ARTEMIS-1649 - enable openssl provider ...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1834#discussion_r165335555
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java ---
@@ -841,7 +864,7 @@ private HttpUpgradeHandler(ChannelPipeline pipeline, HttpClientCodec httpClientC
* HTTP upgrade response will be decode by Netty as 2 objects:
* - 1 HttpObject corresponding to the 101 SWITCHING PROTOCOL headers
* - 1 EMPTY_LAST_CONTENT
- *
+ * <p>
--- End diff --
Un-needed change, please remove.
---
[GitHub] activemq-artemis pull request #1834: ARTEMIS-1649 - enable openssl provider ...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1834#discussion_r165304543
--- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java ---
@@ -416,6 +421,13 @@ private String getHttpUpgradeInfo() {
return ", activemqServerName=" + serverName + ", httpUpgradeEndpoint=" + acceptor;
}
+ private String realKeyStorePath;
--- End diff --
copying comment from old PR, please can we keep these method scope.
---
[GitHub] activemq-artemis issue #1834: ARTEMIS-1649 - enable openssl provider for Net...
Posted by rpahli <gi...@git.apache.org>.
Github user rpahli commented on the issue:
https://github.com/apache/activemq-artemis/pull/1834
I closed the PR #1833 unintentionally
---
[GitHub] activemq-artemis issue #1834: ARTEMIS-1649 - enable openssl provider for Net...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1834
@rpahli, it would be appropriate to add the link to the documentation.
---
[GitHub] activemq-artemis issue #1834: ARTEMIS-1649 - enable openssl provider for Net...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1834
Couple of things...
- All changes should be squashed into a single commit
- The commit message should follow the pattern described in the [Hacking Guide](https://github.com/apache/activemq-artemis/blob/master/docs/hacking-guide/en/maintainers.md#commit-messages).
---