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

Support JMX SSL configuration via exporter YAML configuration #834

Open
VinceGall opened this issue Jul 3, 2023 · 20 comments
Open

Support JMX SSL configuration via exporter YAML configuration #834

VinceGall opened this issue Jul 3, 2023 · 20 comments

Comments

@VinceGall
Copy link

In Version 0.19.0, HTTPS support has been introduced. However, it is only possible to configure a keystore and not a truststore.

httpServer:
  ssl:
    keyStore:
      filename: localhost.jks
      password: changeit
    certificate:
      alias: localhost

Without a truststore I get this error:

2023-07-03 | 18:07:02.313 | main | INFO | io.prometheus.jmx.WebServer | Running
Jul 03, 2023 6:07:05 PM io.prometheus.jmx.JmxCollector collect
SEVERE: JMX scrape failed: java.rmi.ConnectIOException: error during JRMP connection establishment; nested exception is:
        javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target

I'm able to workaround the problem by configuring the HttpServer with:

httpServer:
  ssl:
    certificate:
      alias: localhost

And providing the system properties like this:
-Djavax.net.ssl.keyStore= -Djavax.net.ssl.keyStorePassword= -Djavax.net.ssl.trustStore= -Djavax.net.ssl.trustStorePassword=

However this is not ideal because the passwords can be scraped via a simple "ps -ef" command

@dhoard
Copy link
Collaborator

dhoard commented Jul 3, 2023

Please provide your full exporter YAML file content.

@dhoard
Copy link
Collaborator

dhoard commented Jul 4, 2023

Looking more closely at the information you provided above, this is working as expected.


HTTPS support only requires a keystore. The keystore for HTTPS support can be configured in two ways:

  1. Configuring the keystore in the exporter YAML configuration
httpServer:
  ssl:
    keyStore:
      filename: localhost.jks
      password: changeit
    certificate:
      alias: localhost

... or ...

  1. Configuring the keystore / keystore password as JVM arguments
-Djavax.net.ssl.keyStore= 
-Djavax.net.ssl.keyStorePassword=

JMX SSL scraping support for the standalone Webserver currently requires JVM system properties:

-Djavax.net.ssl.keyStore=
-Djavax.net.ssl.keyStorePassword=
-Djavax.net.ssl.trustStore=
-Djavax.net.ssl.trustStorePassword=

This functionality/configuration was not changed 0.19.0 and is unrelated to the HTTPS support that was added.


In most scenarios, the use of the Java agent version is recommended.

If your desire is to add the ability for the standalone Webserver to use an alternate method of injecting the keystore/keystore password and truststore/truststore password (for example via environment variables), we should track that separately as a new issue (which I can target as an enhancement request.)

@VinceGall
Copy link
Author

---
ssl: false

jmxUrl: service:jmx:rmi:///jndi/rmi://localhost:9999/appmgmt

httpServer:
  ssl:
    keyStore:
      filename: /tmp/client.keystore.key
      password: changeit
    certificate:
      alias: ciclient

rules:
- pattern: ".*"

@dhoard
Copy link
Collaborator

dhoard commented Jul 4, 2023

Please provide the command line used to start your application.

@VinceGall
Copy link
Author

This is with the JVM arguments:

export log_args="-Djava.util.logging.config.file=/jmx_exporter/logging.properties"
export ssl_args="-Djavax.net.ssl.keyStore=/tmp/client.keystore.key -Djavax.net.ssl.trustStore=/tmp/client.truststore.key -Djavax.net.ssl.keyStorePassword=changeit -Djavax.net.ssl.trustStorePassword=changeit"

export CLASSPATH=/jmx_exporter/jmx_prometheus_httpserver.jar:/jmx_exporter/mx4j-tools.jar

java $log_args $ssl_args -cp $CLASSPATH io.prometheus.jmx.WebServer 10081 /jmx_exporter/httpserver_config.yml >/jmx_exporter/logs/jmx_exporter.log 2>&1

@dhoard
Copy link
Collaborator

dhoard commented Jul 5, 2023

This is with the JVM arguments:

export log_args="-Djava.util.logging.config.file=/jmx_exporter/logging.properties"
export ssl_args="-Djavax.net.ssl.keyStore=/tmp/client.keystore.key -Djavax.net.ssl.trustStore=/tmp/client.truststore.key -Djavax.net.ssl.keyStorePassword=changeit -Djavax.net.ssl.trustStorePassword=changeit"

export CLASSPATH=/jmx_exporter/jmx_prometheus_httpserver.jar:/jmx_exporter/mx4j-tools.jar

java $log_args $ssl_args -cp $CLASSPATH io.prometheus.jmx.WebServer 10081 /jmx_exporter/httpserver_config.yml >/jmx_exporter/logs/jmx_exporter.log 2>&1

This is the exporter application information. Please provide the command line/configuration of the application being monitored.

@VinceGall
Copy link
Author

Hi, that is a proprietary application therefore I can't provide Intellectual Property info. But the startup would be something like this:

nohup $JAVA_HOME/bin/java -Dpython.import.site=false -Dpython.cachedir=$MYDIR/logs/jython -Dprocess.id=$PROCESS_ID -DJMX_IMPL=$JMXIMPL -Xmx256m -Duser.region=US -Dhttps.protocols=TLSv1.2 -Duser.language=en -Dlog4j.configurationFile=$MYDIR/config/EventConfig.xml -Denable.managementservice=false -Dconfig.filename=$MYDIR/config/config.properties com.test.app.mgmt.myJmxServer file://$MYDIR/config/myjmx.mlet > $MYDIR/logs/$PROCESS_ID.stdout 2>> $MYDIR/logs/$PROCESS_ID.stderr &

@dhoard
Copy link
Collaborator

dhoard commented Jul 6, 2023

Understood.

If you disable the HTTP server support (remove the configuration completely), does it work?

Have you tested with 0.18.0?

@dhoard
Copy link
Collaborator

dhoard commented Jul 6, 2023

With ssl: false (or not defined) the internal exporter JMX collector / scraper code does not use SSL when connecting to the application to be monitored.

if (yamlConfig.containsKey("ssl")) {
cfg.ssl = (Boolean)yamlConfig.get("ssl");
}

if (ssl) {
environment.put(Context.SECURITY_PROTOCOL, "ssl");
SslRMIClientSocketFactory clientSocketFactory = new SslRMIClientSocketFactory();
environment.put(RMIConnectorServer.RMI_CLIENT_SOCKET_FACTORY_ATTRIBUTE, clientSocketFactory);
environment.put("com.sun.jndi.rmi.factory.socket", clientSocketFactory);
}


This scenario has been tested successfully as part of the integration test suite.

Test:

https://github.com/prometheus/jmx_exporter/blob/main/integration_test_suite/integration_tests/src/test/java/io/prometheus/jmx/test/http/ssl/SSLWithJKSKeyStoreTest2.java

Configuration:

https://github.com/prometheus/jmx_exporter/tree/main/integration_test_suite/integration_tests/src/test/resources/io/prometheus/jmx/test/http/ssl/SSLWithJKSKeyStoreTest2/Standalone


Without a shareable working example reproducing the issue that I can debug, no more investigation is possible.

@VinceGall
Copy link
Author

Hello Doug,

With the SSL JVM arguments it works. I'm able to scrape the metrics from the JMX application. And the Http server port is listening with SSL/TLS (HTTPS).
Version 0.18.0 does not support HTTPS therefore I did not use it.

The only issue I have is that passing the passwords as JVM arguments isn't safe and I would like to able to do this via the yaml config.

Looking at the new feature introduced in 0.19.0, it is now possible to provide a key store and a password via the config, which is great but this doesn't work for me. Probably because the trust store that should contain the CA/root certificates is missing?

@VinceGall
Copy link
Author

I'll try to run the exporter with SSL debug enabled, and chekc the handshake

@VinceGall
Copy link
Author

Shouldn't the config accept both a key store (which contains the key) and a trust store (which contains the CA certs)? Looking at the new code, it seems that the same jks file is used as both keystore and truststore but it also removes, from the loaded keystore, all the certificates with aliases that don't match the input alias.
This means that if I store in the same jks file both the key and the CA certs, which have their own aliases, and the input alias is the one for the key, then the CA certs are removed...

jmx_prometheus_common/src/main/java/io/prometheus/jmx/common/http/ssl/SSLContextFactory.java

@VinceGall
Copy link
Author

Ok, Just run a test without the JVM SSL args and with debug enabled and I see the following, which means that the keyStore I pass in the YAML config isn't picked up, while the trustStore is defaulting to the Java default one (cacerts):

javax.net.ssl|DEBUG|0E|prometheus-http-1-1|2023-07-06 10:07:48.908 CEST|TrustStoreManager.java:112|trustStore is: /usr/lib/jvm/java-11-openjdk-11.0.19.0.7-1.el7_9.x86_64/lib/security/cacerts

javax.net.ssl|DEBUG|0E|prometheus-http-1-1|2023-07-06 10:07:49.102 CEST|SSLContextImpl.java:1071|keyStore is :

While when passing the JVM args:

javax.net.ssl|DEBUG|0E|prometheus-http-1-1|2023-07-06 10:16:03.809 CEST|TrustStoreManager.java:112|trustStore is: /tmp/client.truststore.jks

javax.net.ssl|DEBUG|0E|prometheus-http-1-1|2023-07-06 10:16:03.937 CEST|SSLContextImpl.java:1071|keyStore is : /tmp/client.keystore.jks

@dhoard
Copy link
Collaborator

dhoard commented Jul 6, 2023

Based on your comments and tests, this is working as expected.

The configuration section ...

  ssl:
    keyStore:
      filename: /tmp/client.keystore.key
      password: changeit

... is only used for HTTPS support to expose metrics to Prometheus - it does not control the JMX SSL scraping configuration.

#834 (comment)


Your security concern is valid, but is unrelated to HTTPS support that was added in 0.19.0.

I'll change the issue description and mark it as an enhancement request.

@dhoard dhoard changed the title HTTPS - Add support to TrustStore Support JMX SSL configuration via exporter YAML configuration Jul 6, 2023
@VinceGall
Copy link
Author

Hello Doug,
I apologize but I'm confused. Indeed I also thought that those new properties introduced in 0.19.0 are used to add TLS support for the HTTP port used by Prometheus to scrape metrics.
This is actually what I need and by adding the httpServer/ssl portion to the config file, I get this behaviour.
I don't need SSL between the HttpServer exporter and my JMX application, that's why 'ssl: false'

The only problem I have is that it seems that the keystore passed via the new Param isn't used. And there is no support for Trust Store.

@dhoard
Copy link
Collaborator

dhoard commented Jul 6, 2023

@VinceGall I could be totally wrong.

Jul 03, 2023 6:07:05 PM io.prometheus.jmx.JmxCollector collect
SEVERE: JMX scrape failed: java.rmi.ConnectIOException: error during JRMP connection establishment; nested exception is:
        javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target

Should only happen if JMX is trying to use SSL to scrape metrics.


Attached is a test package zip derived from the integration test (HTTPS support enabled and JMX SSL disabled) that you can run on your machine.

Instructions:

  1. Download the test package zip
  2. Unzip the test package zip into a directory
  3. In one console, run ./application.sh
  4. In a second console, run ./exporter.sh
  5. Access the URL https://localhost:8888 (Because it's a self-signed certificate you will have to accept the browser warning.)

test-package.zip

@VinceGall
Copy link
Author

Thanks for the support so far. I think you are right. It seems my HttpServer exporter tries to establish an SSL connection with the JMX Application via RMI.
I'll make a tcpdump to confirm this

@dhoard
Copy link
Collaborator

dhoard commented Jul 17, 2023

@VinceGall any update on your testing?

@Janardhan78
Copy link

i am trying to use 0.19.0 trying to setup SSL connection between prometheus server and JMX server , i tried to setup the configuration currently , but it seems that the below configuration doesnt setup SSL between prometheus server and JMX Server , please correct me in this case

httpServer:
ssl:
certificate:
alias: localhost
keyStore:
filename: localhost.pkcs12
password: changeit

@dhoard
Copy link
Collaborator

dhoard commented Aug 2, 2023

@Janardhan78 please open a new issue since it seems unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants