Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes SSL failure to connect using proxy or .war #436

Merged
merged 2 commits into from Aug 31, 2020

Conversation

jc-roman
Copy link
Contributor

@jc-roman jc-roman commented Apr 27, 2020

Adds SSL-RMI support when the javax.net.ssl.trustStore system property is defined and fixes error "java.rmi.ConnectIOException: non-JRMP server at remote endpoint. Tested with tomcat 9 by mounting build .war file.

Resolves #109 - Add SSL support for the JSR-160 proxy

Adds SSL-RMI support when the `javax.net.ssl.trustStore` system property is defined and fixes error "java.rmi.ConnectIOException: non-JRMP server at remote endpoint"
Copy link
Member

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR ! I don't know about the concrete issue but it looks reasonable to me (i.e. to switch to SSL communication when a trust store is set). Is the presence of a trustStore the only indicator to use SSL for RMI communication or is there a more direct property ?

PR looks good with some minor cosmetic suggestion.

…patcher.java

Co-authored-by: Roland Huß <rhuss@redhat.com>
@jc-roman
Copy link
Contributor Author

jc-roman commented Aug 22, 2020

@rhuss,

Thanks for the PR ! I don't know about the concrete issue but it looks reasonable to me (i.e. to switch to SSL communication when a trust store is set). Is the presence of a trustStore the only indicator to use SSL for RMI communication or is there a more direct property ?

For my specific use case with Cassandra's default JMX configs, here's the server side settings:

    -Dcom.sun.management.jmxremote.ssl=true
    -Dcom.sun.management.jmxremote.ssl.need.client.auth=true
    -Dcom.sun.management.jmxremote.registry.ssl=true
    -Djavax.net.ssl.keyStore=/etc/cassandra/certstore/client_certstore/keystore.jks
    -Djavax.net.ssl.keyStorePassword=cassandra
    -Djavax.net.ssl.trustStore=/etc/cassandra/certstore/client_certstore/truststore.jks
    -Djavax.net.ssl.trustStorePassword=cassandra

Using Jolokia in Proxy mode with tomcat, this is the minimal required system properties to make it work (notice jmxremote.ssl is disabled, as this only applies to tomcat's JMX):

-Dcom.sun.management.jmxremote.ssl=false
-Djavax.net.ssl.keyStore=/etc/cassandra/certstore/client_certstore/keystore.jks
-Djavax.net.ssl.keyStorePassword=cassandra
-Djavax.net.ssl.trustStore=/etc/cassandra/certstore/client_certstore/truststore.jks
-Djavax.net.ssl.trustStorePassword=cassandra

If one were to disable ssl.need.client.auth, only the truststore path and password are required. The truststore is then the minimal property required to enable SSL, (although in practice, disabling client auth is a really bad idea because it cancels out the security benefits of SSL)

PR looks good with some minor cosmetic suggestion.

I commited your suggestion. Thanks for the improvement!

@jc-roman
Copy link
Contributor Author

jc-roman commented Aug 22, 2020

For reference:
My setup to create a tomcat + jolokia in Proxy Mode Dockerfile:
https://github.com/TheWeatherCompany/jolokia/blob/docker-proxy/Dockerfile
https://github.com/TheWeatherCompany/jolokia/blob/docker-proxy/setenv.sh

diff (a bit outdated) that includes building the docker image with Github actions: master...TheWeatherCompany:docker-proxy

truststore and keystore volumes mounted at runtime. Truststore and keystore System properties are set in CATALINA_OPTS.

@jc-roman jc-roman requested a review from rhuss August 25, 2020 11:48
@rhuss
Copy link
Member

rhuss commented Aug 31, 2020

Thanks, that makes sense to me.

@rhuss rhuss merged commit c03861f into jolokia:master Aug 31, 2020
@jc-roman jc-roman deleted the proxy-ssl branch September 4, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SSL support for the JSR-160 proxy
2 participants