Add support for remote jmx rules file.

[luigidemasi]

Add support to specify remote jmx rules files for otel.jmx.config property, for example:

java -javaagent:opentelemetry-javaagent.jar -Dotel.jmx.config=https://raw.githubusercontent.com/luigidemasi/opentelemetry-java-instrumentation/refs/heads/camel-update/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/camel.yaml,/path/to/my/customFile.yaml ..... myApp.jar

[trask]

cc @PeterF778 @SylvainJuge @robsunday @breedx-splk

[SylvainJuge]

Adding this is a great addition as it helps to have one extra config file to manage, at least while we still have to rely on a separate YAML definition file for the JMX configuration (maybe later when the declarative configuration will be ready the whole agent/sdk configuration will fit in a single file and/or can be changed at runtime).

It would be nice to add some documentation in https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/jmx-metrics/javaagent#configuration-files

One potential issue here is that the agent listener is synchronous as far as I remember, thus if the URL is slow to respond then it might delay the whole agent startup. I remember that we had similar concerns with the resource attributes providers, but here given it's optional maybe documenting this is enough.

[laurit]

Are there any security implications to parsing the configuration from a remote source?

[SylvainJuge]

Are there any security implications to parsing the configuration from a remote source?

I definitely see a few ones:

• if the remote host is compromised, we can't have any guarantees on the YAML content
• if there is a vulnerability in the yaml parser, this adds a way to propagate to the monitored app.

For example, if there is any remote code execution (RCE) in the yaml parsing library, then compromise from the remote host means RCE on the whole application. If the vulnerability is just a "bug" like an infinite loop when using fancy YAML features like references, then it could still be used to keep the CPU busy and have side-effects on the app.

However:

• the YAML format is more descriptive than imperative, and it's not usually parsed by doing an equivalent of a JS eval(...), so it's quite safe by nature. However I would not bet strongly on that assumption.

While I definitely see value in adding out-of-the box support for this as a convenience, it could also be replaced by a curl + writing to local file before the application startup as an ad-hoc alternative implementation (still not safer security-wise, but at least we could assume the efforts to implement this would make it a more intentional and deliberate choice).

[luigidemasi]

Hi @SylvainJuge

I definitely see a few ones:

if the remote host is compromised, we can't have any guarantees on the YAML content

For a mission critical app, you wan't use a public and/or untrusted URL for rules, and if someone compromises the private remote host, probably you are in a bigger trouble...

if there is a vulnerability in the yaml parser, this adds a way to propagate to the monitored app.

Also, to be vulnerable to the (un)famous "Unsafe deserialization" issue, you need to have at least a gadget class in your classpath and give the possibility to deserialize arbitrary class
(and from a security scanner point of view, you are still vulnerable, even if you are deserializing a local file)

https://snyk.io/blog/unsafe-deserialization-snakeyaml-java-cve-2022-1471/

While I definitely see value in adding out-of-the box support for this as a convenience, it could also be replaced by a curl + writing to local file before the application startup as an ad-hoc alternative implementation (still not safer security-wise, but at least we could assume the efforts to implement this would make it a more intentional and deliberate choice).

This is just moving the problem somewhere else, not solving it.

[trask]

at least while we still have to rely on a separate YAML definition file for the JMX configuration

hi @luigidemasi! once https://github.com/open-telemetry/opentelemetry-configuration is stable, we are going to move the JMX configuration inside of that file, at which point we wouldn't support reading that config file from a remote location without broader OpenTelemetry community consensus (beyond just Java), since it will be part of the specification.

as @SylvainJuge mentions, there is work in progress to support remote configuration in the OpenTelemetry ecosystem under https://github.com/open-telemetry/opamp-spec, with some initial work in Java at https://github.com/open-telemetry/opentelemetry-java-contrib/pulls.

I'm wondering if it would be better to solve this use case for now outside of OpenTelemetry (e.g. curl) until we work out (and implement) the broader remote configuration story in OpenTelemetry.

[luigidemasi]

hi @luigidemasi! once https://github.com/open-telemetry/opentelemetry-configuration is stable, we are going to move the JMX configuration inside of that file, at which point we wouldn't support reading that config file from a remote location without broader OpenTelemetry community consensus (beyond just Java), since it will be part of the specification.

Hi @trask, do you have a rough estimate on when it will happen?