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

Endpoint authentication in jmx_exporter #664

Closed
dhoard opened this issue Dec 15, 2021 · 15 comments
Closed

Endpoint authentication in jmx_exporter #664

dhoard opened this issue Dec 15, 2021 · 15 comments

Comments

@dhoard
Copy link
Collaborator

dhoard commented Dec 15, 2021

Given the merge of prometheus/client_java#682, authentication to endpoints in jmx_exporter can now be implemented.

Ideally, I think we should make the authentication pluggable, with a default BasicAuthentication implementation built-in.

Thoughts on pluggability/configuration parameters?

@fstab
Copy link
Member

fstab commented Dec 15, 2021

I'm currently playing around with a new config format, my current sketch looks like this:

2021-12-16: Updated with the current contents of my sketch file

startDelaySeconds: 0

collector:
  hostPort: 127.0.0.1:1234
  jmxUrl: service:jmx:rmi:///jndi/rmi://127.0.0.1:1234/jmxrmi
  username:
  password:
  sslEnabled: true
  sslClientAuth: true # Default is taken from the System property: com.sun.management.jmxremote.ssl.need.client.auth
  sslKeyAlias: jmx # see Tomcat's keyAlias

httpServer:
  address: 0.0.0.0
  port: 8080
  username: admin
  password: admin
  sslEnabled: true
  sslClientAuth: true # Note: In Tomcat, possible values are 'true', 'false', and 'want'.
  sslKeyAlias: server # see Tomcat's keyAlias

# The SSL section is an alternative to the standard System properties.
# Defaults are taken from the System properties.
ssl:
  sslKeyStore: /etc/my-keystore # System property: javax.net.ssl.keyStore
  sslKeyStorePassword: changeit # System property: javax.net.ssl.keyStorePassword
  sslTrustStore: /etc/my-trust-store # javax.net.ssl.trustStore
  sslTrustStorePassword: changeit # System property: javax.net.ssl.trustStorePassword

jmxBeanFilter:
  include:
  - org.apache.cassandra.metrics:*
  exclude:
  - org.apache.cassandra.metrics:type=ColumnFamily,*

metricFilter:
  nameMustNotBeEqualTo:
    - jvm_threads_deadlocked
    - io_prometheus_jmx_tabularData_Server_1_Disk_Usage_Table_avail
    - java_lang_Memory_HeapMemoryUsage_used
    - jmx_config_reload_success_total
    - java_specification_version

lowercaseOutputName: false
lowercaseOutputLabelNames: false

rules:
  - pattern: 'org.apache.cassandra.metrics<type=(\w+), name=(\w+)><>Value: (\d+)'
    name: cassandra_$1_$2
    value: $3
    valueFactor: 0.001
    labels: {}
    help: "Cassandra metric $1 $2"
    cache: false
    type: GAUGE
    attrNameSnakeCase: false

It's still early stages though, I'm extending the Config class and then we'll see if that format makes sense.

The implementation would be backwards compatible, so if anyone uses the old config jmx_exporter would still be able to read it.

What do you think?

@dhoard
Copy link
Collaborator Author

dhoard commented Dec 15, 2021

@fstab I think it looks really good.

I would like the authentication to be pluggable by an end-user if possible. A pluggable model would allow an end-user to do more than would ever be provided by packaged jmx_exporter code. Given a potential change in configuration format, this would be the best time to implement such a model.

httpServer:
  address: 0.0.0.0
  port: 8080
  ssl: true
  authenticator:
    factory: io.prometheus.jmx.BasicAuthenticatorFactory
    username: admin
    password: admin
  keyAlias: server

Example AuthenticatorFactory interface...

package io.prometheus.jmx;

import com.sun.net.httpserver.Authenticator;

public interface AuthenticatorFactory {

    void initialize(Config config);

    Authenticator getAuthenticator() throws Exception;
}

Example BasicAuthenticatorFactory class...

package io.prometheus.jmx;

import com.sun.net.httpserver.Authenticator;

public class BasicAuthenticatorFactory implements AuthenticatorFactory {

    private Config config;
    private Authenticator authenticator;

    public void initialize(Config config) {
        this.config = config;
        this.authenticator = new BasicAuthenticator(config);
    }

    public Authenticator getAuthenticator() throws Exception {
        return authenticator;
    }
}

Example BasicAuthenticator class...

package io.prometheus.jmx;

public class BasicAuthenticator extends com.sun.net.httpserver.BasicAuthenticator {

    protected Config config;
    protected String username;
    protected String password;

    public BasicAuthenticator(Config config) {
        super("/");

        this.config = config;

        // TODO
        //
        // get username/password from Config
        //
        // username = config...
        // password = config...
    }

    public boolean checkCredentials(String username, String password) {
        boolean authenticated = false;

        if (this.username != null && this.password != null) {
            authenticated = this.username.equals(username) && this.password.equals(password);
        }

        return authenticated;
    }
}

@fstab
Copy link
Member

fstab commented Dec 16, 2021

I updated the yaml config sketch above.

I'm not sure about the pluggable authenticator. That would mean that the jmx_exporter has to load a JAR file with the authenticator implementation and execute that code. In the light of the recent log4shell vulnerability we should be extra careful introducing plugins that allow executing custom code. We will provide Basic authenitcation as well as SSL client authentication. My feeling is that this covers most needs. If anyone wants something really fancy, like authenticating against their LDAP, they will probably put an Nginx in between for authentication instead of implementing it in Java as a plugin for the jmx_exporter.

@dhoard
Copy link
Collaborator Author

dhoard commented Dec 17, 2021

Given the recent Log4J issue, I understand the concern around pluggability/dynamic classloading.

@fstab
Copy link
Member

fstab commented Dec 17, 2021

I did a bit of monkey work and created a Config class for the config file format above: https://github.com/prometheus/jmx_exporter/blob/new-config/collector/src/main/java/io/prometheus/jmx/Config.java

It can also read the old format for backwards compatibility. I didn't test it extensively though, so there might be bugs.

@dhoard
Copy link
Collaborator Author

dhoard commented Jan 2, 2022

Looks pretty good.

Thoughts on making ssl a group so it's clear that keyAlias is related to ssl?

Thoughts on adding enabled flags to toggle the functionality?

httpServer:
  address: 0.0.0.0
  port: 8080
  ssl:
    enabled: true
    keyAlias: server
  authenticator:
    enabled: true
    username: admin
    password: admin

@dhoard
Copy link
Collaborator Author

dhoard commented Jan 28, 2022

I noticed that the example above doesn't include sslContextType or sslKeystoreType. I think these should be added with defaults of TLSv1.2 and PKCS12.

@red61wjt
Copy link

Happy to report the new-config branch allows me to auth correctly with JBoss EAP and it's service:jmx:remote+http: url

@dhoard
Copy link
Collaborator Author

dhoard commented Dec 23, 2022

@fstab I have managed to find time during Christmas break to create an initial implementation of HTTP Basic authentication and HTTPS support. Please review.

https://github.com/dhoard/jmx_exporter/tree/basic-authentication-and-ssl

Notes:

I used a different configuration format...

httpServer:
  authentication:
    enabled: true
    type: Basic
    username: prometheus
    password: secret
  ssl:
    enabled: true
    keystore:
      filename: keystore.jks
      password: changeit

I like flags to enable/disable functionality.


Running the test suite

mvn clean package
cd jmx_prometheus_exporter_test_suite
mvn clean package

Java agent tests

  • 40 containers
  • 1,522 test method executions (various containers with various combinations of HTTP Basic authentication and HTTPS)
  • takes about ~6 minutes

Note: due to TLS issues on ibmjava:8 and ticketfly/java:6 containers, HTTPS is not supported with these JVMs


Java HttpServer tests

  • 40 containers
  • 6,240 test method executions (various containers with various combinations of HTTP Basic authentication, HTTPS, JMX authentication, and JMX SSL)
  • takes about ~50 minutes

Note: due to TLS issues on ibmjava:8 and ticketfly/java:6 containers, HTTPS is not supported with these JVMs

@fstab
Copy link
Member

fstab commented Dec 26, 2022

Hi @dhoard, thanks for your work on this. I didn't look at the code yet, but looking at the config sample, I see a difference between yours and the one in the new-config branch:

collector:
hostPort: 127.0.0.1:1234
#jmxUrl: service:jmx:rmi:///jndi/rmi://127.0.0.1:1234/jmxrmi
username: prometheus
password: secret
sslEnabled: true
sslClientAuth: true
sslKeyAlias: jmx
httpServer:
address: 127.0.0.1
port: 9012
username: scraper
password: changeMe
sslEnabled: true
sslClientAuth: true
sslKeyAlias: server
ssl:
sslKeyStore: /etc/my-key-store
sslKeyStorePassword: keyStorePwd
sslTrustStore: /etc/my-trust-store
sslTrustStorePassword: trustStorePwd

There are two places where SSL can be used:

  1. When exporting metrics via the HTTPServer.
  2. When scraping metrics via JMX remote.

However, I think you can only configure one keystore in Java applications. If you want different keys for exporting and scraping, you need to add both keys to the same keystore and make them accessible with different aliases.

So in the config in the new-config branch I have a spearate ssl section, and both the collector (scraping) and the httpServer can have ssl enabled individually. In case they use different keys, they both have a sslKeyAlias config option.

Does this make sense?

@dhoard
Copy link
Collaborator Author

dhoard commented Dec 26, 2022

@fstab the goal of the new-config format makes sense, though Java allows multiple keystores / truststores.

The code, as currently written, allows two separate keystores to prevent existing installations from changing JMX SSL configuration. If the goal is a single keystore / truststore... I can go down that path.

With the current version in my branch, all you have to do to get Basic authentication and HTTPS is add...

httpServer:
  authentication:
    enabled: true
    type: Basic
    username: prometheus
    password: secret
  ssl:
    enabled: true
    certificateAlias: localhost
    keystore:
      filename: keystore.jks
      password: changeit

JMX uses the keystore defined as Java properties:

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

The HTTP server uses the values in the configuration YAML to create its own SSLContext

Relevant test...

https://github.com/dhoard/jmx_exporter/blob/basic-authentication-and-ssl/jmx_prometheus_exporter_test_suite/src/test/java/io/prometheus/jmx/httpserver/AllAuthenticationAndSSLWithIndividualKeystoresTruststores_Test.java

and configuration...

https://github.com/dhoard/jmx_exporter/tree/basic-authentication-and-ssl/jmx_prometheus_exporter_test_suite/src/test/resources/io/prometheus/jmx/httpserver/AllAuthenticationAndSSLWithIndividualKeystoresTruststores_Test


If this conversation is best moved to the mailing list, let me know. I know that a lot of users / requesters of this feature watch GitHub issues, but may not follow the mailing list.

@dhoard
Copy link
Collaborator Author

dhoard commented Dec 28, 2022

Added more test classes.

  • 31 test classes
  • 43 containers
  • 15,310 test method executions (across all test classes/containers/test methods)

Notes

  • HTTPS configuration appears to work for Java 6, but is not tested
  • The full integration test suite takes ~1.5 hours

@dhoard
Copy link
Collaborator Author

dhoard commented Jan 8, 2023

@red61wjt I have been working on the initial code to support HTTPS and HTTP Basic authentication.

A potential pre-release version of the code can be found at...

https://github.com/dhoard/jmx_exporter/releases/tag/jmx_exporter_enhanced-snapshot.

Due to the vast number of changes, it will take time to be merged and released.

@dhoard
Copy link
Collaborator Author

dhoard commented Jan 26, 2023

see #688

@dhoard
Copy link
Collaborator Author

dhoard commented May 7, 2023

This is targeted for the next (post 0.18.0) release

@dhoard dhoard closed this as completed Jun 24, 2023
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

No branches or pull requests

3 participants