[receiver/jmxreceiver] Limit parameters for JMX receiver to reduce se…

…curity risk (open-telemetry#9721)

* Remove options considered too permissive for security in jmx receiver

* Use config file for communicating to JMX metrics gatherer

* Fix small issues with jmx receiver

* Ensure property files are generated correctly with multiline input

* Update changelog

* minor changes to receiver to support more config in properties file

* Update properties file generation

* Address PR feedback and address environment variable attack surface

* Update receiver/jmxreceiver/config.go

Co-authored-by: Alex Boten <[email protected]>

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ### 🛑 Breaking changes 🛑
 
+- `jmxreceiver`: Remove properties & groovyscript parameters from JMX Receiver. Add ResourceAttributes & LogLevel parameter to supply some of the removed functionality with reduced attack surface (#9685)
+
 ### 🚩 Deprecations 🚩
 
 ### 🚀 New components 🚀
@@ -53,6 +55,7 @@
 - `k8sattributesprocessor`: Support regex capture groups in tag_name (#9525)
 - `mongoreceiver`: Update metrics scope name from `otelcol/mongodb` to `otelcol/mongodbreceiver` (#9759)
 - `transformprocessor`: Add new `truncation` function to allow truncating string values in maps such as `attributes` or `resource.attributes` (#9546)
+- `jmxreceiver`: Communicate with JMX metrics gatherer subprocess via properties file (#9685)
 - `datadogexporter`: Add `api.fail_on_invalid_key` to fail fast if api key is invalid (#9426)
 - `transformprocessor`: Add support for functions to validate parameters (#9563)
 - `googlecloudexporter`: Add GCP cloud logging exporter (#9679)

receiver/jmxreceiver/README.md

@@ -3,8 +3,7 @@
 ### Overview
 
 The JMX Receiver will work in conjunction with the [OpenTelemetry JMX Metric Gatherer](https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/jmx-metrics/README.md)
-to report metrics from a target MBean server using a built-in or your custom `otel` helper-utilizing
-Groovy script.
+to report metrics from a target MBean server using a built-in `otel` helper-utilizing Groovy script.
 
 Status: alpha
 
@@ -35,10 +34,8 @@ receivers:
  username: my_jmx_username
  # determined by the environment variable value
  password: $MY_JMX_PASSWORD
- # will be set as system properties for the underlying java command
- properties:
- otel.resource.attributes: my.attr=my.value,my.other.attr=my.other.value
- some.system.property: some.system.property.value
+ resource_attributes: my.attr=my.value,my.other.attr=my.other.value
+ log_level: info
  additional_jars:
  - /path/to/other.jar
 ```
@@ -59,20 +56,11 @@ _Required._
 
 ### target_system
 
-The built-in target system metric gatherer script to run.
+The built-in target system (or systems) metric gatherer script to run.
+Must be a subset of: `"activemq"`, `"cassandra"`, `"hbase"`, `"hadoop"`, `"jvm"`, `"kafka"`, `"kafka-consumer"`, `"kafka-producer"`, `"solr"`, `"tomcat"`, `"wildfly"`.
 
 Corresponds to the `otel.jmx.target.system` property.
 
-One of `groovy_script` or `target_system` is _required_. Both cannot be specified at the same time.
-
-### groovy_script
-
-The path of the Groovy script the Metric Gatherer should run.
-
-Corresponds to the `otel.jmx.groovy.script` property.
-
-One of `groovy_script` or `target_system` is _required_. Both cannot be specified at the same time.
-
 ### collection_interval (default: `10s`)
 
 The interval time for the Groovy script to be run and metrics to be exported by the JMX Metric Gatherer within the persistent JRE process.
@@ -91,11 +79,6 @@ The password to use for JMX authentication.
 
 Corresponds to the `otel.jmx.password` property.
 
-### properties
-
-A map of property names to values to be used as system properties set as command line options
-(e.g. `-Dproperty.name=property.value`).
-
 ### otlp.endpoint (default: `0.0.0.0:<random open port>`)
 
 The otlp exporter endpoint to which to listen and submit metrics.
@@ -158,7 +141,18 @@ The realm, as required by remote profile SASL/DIGEST-MD5.
 
 Corresponds to the `otel.jmx.realm` property.
 
-
 ### additional_jars
 
 Additional JARs to be included in the java command classpath.
+
+### resource_attributes
+
+List of resource attributes that will be applied to any metrics emitted from the metrics gatherer.
+
+Corresponds to the `otel.resource.attributes` property.
+
+### log_level
+
+SLF4J log level for the JMX metrics gatherer. Must be one of: `"trace"`, `"debug"`, `"info"`, `"warn"`, `"error"`, `"off"`
+
+Corresponds to the `org.slf4j.simpleLogger.defaultLogLevel` property.

receiver/jmxreceiver/config.go

@@ -16,13 +16,14 @@ package jmxreceiver // import "github.com/open-telemetry/opentelemetry-collector
 
 import (
  "fmt"
- "os"
  "sort"
  "strings"
  "time"
 
  "go.opentelemetry.io/collector/config"
  "go.opentelemetry.io/collector/exporter/exporterhelper"
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
 )
 
 type Config struct {
@@ -31,10 +32,8 @@ type Config struct {
  JARPath string `mapstructure:"jar_path"`
  // The Service URL or host:port for the target coerced to one of form: service:jmx:rmi:https:///jndi/rmi:https://<host>:<port>/jmxrmi.
  Endpoint string `mapstructure:"endpoint"`
- // The target system for the metric gatherer whose built in groovy script to run. Cannot be set with GroovyScript.
+ // The target system for the metric gatherer whose built in groovy script to run.
  TargetSystem string `mapstructure:"target_system"`
- // The script for the metric gatherer to run on the configured interval. Cannot be set with TargetSystem.
- GroovyScript string `mapstructure:"groovy_script"`
  // The duration in between groovy script invocations and metric exports (10 seconds by default).
  // Will be converted to milliseconds.
  CollectionInterval time.Duration `mapstructure:"collection_interval"`
@@ -60,10 +59,13 @@ type Config struct {
  RemoteProfile string `mapstructure:"remote_profile"`
  // The SASL/DIGEST-MD5 realm
  Realm string `mapstructure:"realm"`
- // Map of property names to values to pass as system properties when running JMX Metric Gatherer
- Properties map[string]string `mapstructure:"properties"`
  // Array of additional JARs to be added to the the class path when launching the JMX Metric Gatherer JAR
  AdditionalJars []string `mapstructure:"additional_jars"`
+ // Map of resource attributes used by the Java SDK Autoconfigure to set resource attributes
+ ResourceAttributes map[string]string `mapstructure:"resource_attributes"`
+ // Log level used by the JMX metric gatherer. Should be one of:
+ // `"trace"`, `"debug"`, `"info"`, `"warn"`, `"error"`, `"off"`
+ LogLevel string `mapstructure:"log_level"`
 }
 
 // We don't embed the existing OTLP Exporter config as most fields are unsupported
@@ -94,26 +96,67 @@ func (oec otlpExporterConfig) headersToString() string {
  return headerString
 }
 
-func (c *Config) parseProperties() []string {
- parsed := make([]string, 0, len(c.Properties))
- for property, value := range c.Properties {
- parsed = append(parsed, fmt.Sprintf("-D%s=%s", property, value))
+func (c *Config) parseProperties(logger *zap.Logger) []string {
+ parsed := make([]string, 0, 1)
+
+ logLevel := "info"
+ if len(c.LogLevel) > 0 {
+ logLevel = strings.ToLower(c.LogLevel)
+ } else if logger != nil {
+ logLevel = getZapLoggerLevelEquivalent(logger)
  }
+
+ parsed = append(parsed, fmt.Sprintf("-Dorg.slf4j.simpleLogger.defaultLogLevel=%s", logLevel))
  // Sorted for testing and reproducibility
  sort.Strings(parsed)
  return parsed
 }
 
+var logLevelTranslator = map[zapcore.Level]string{
+ zap.DebugLevel: "debug",
+ zap.InfoLevel: "info",
+ zap.WarnLevel: "warn",
+ zap.ErrorLevel: "error",
+ zap.DPanicLevel: "error",
+ zap.PanicLevel: "error",
+ zap.FatalLevel: "error",
+}
+
+var zapLevels = []zapcore.Level{
+ zap.DebugLevel,
+ zap.InfoLevel,
+ zap.WarnLevel,
+ zap.ErrorLevel,
+ zap.DPanicLevel,
+ zap.PanicLevel,
+ zap.FatalLevel,
+}
+
+func getZapLoggerLevelEquivalent(logger *zap.Logger) string {
+ var loggerLevel *zapcore.Level
+ for i, level := range zapLevels {
+ if testLevel(logger, level) {
+ loggerLevel = &zapLevels[i]
+ break
+ }
+ }
+
+ // Couldn't get log level from logger default logger level to info
+ if loggerLevel == nil {
+ return "info"
+ }
+
+ return logLevelTranslator[*loggerLevel]
+}
+
+func testLevel(logger *zap.Logger, level zapcore.Level) bool {
+ return logger.Check(level, "_") != nil
+}
+
 // parseClasspath creates a classpath string with the JMX Gatherer JAR at the beginning
 func (c *Config) parseClasspath() string {
  classPathElems := make([]string, 0)
 
- // See if the CLASSPATH env exists and if so get it's current value
- currentClassPath, ok := os.LookupEnv("CLASSPATH")
- if ok && currentClassPath != "" {
- classPathElems = append(classPathElems, currentClassPath)
- }
-
  // Add JMX JAR to classpath
  classPathElems = append(classPathElems, c.JARPath)
 
@@ -124,13 +167,20 @@ func (c *Config) parseClasspath() string {
  return strings.Join(classPathElems, ":")
 }
 
+var validLogLevels = map[string]struct{}{"trace": {}, "debug": {}, "info": {}, "warn": {}, "error": {}, "off": {}}
+var validTargetSystems = map[string]struct{}{"activemq": {}, "cassandra": {}, "hbase": {}, "hadoop": {},
+ "jvm": {}, "kafka": {}, "kafka-consumer": {}, "kafka-producer": {}, "solr": {}, "tomcat": {}, "wildfly": {}}
+
 func (c *Config) validate() error {
  var missingFields []string
  if c.Endpoint == "" {
  missingFields = append(missingFields, "`endpoint`")
  }
- if c.TargetSystem == "" && c.GroovyScript == "" {
- missingFields = append(missingFields, "`target_system` or `groovy_script`")
+ if c.TargetSystem == "" {
+ missingFields = append(missingFields, "`target_system`")
+ }
+ if c.JARPath == "" {
+ missingFields = append(missingFields, "`jar_path`")
  }
  if missingFields != nil {
  baseMsg := fmt.Sprintf("%v missing required field", c.ID())
@@ -148,5 +198,26 @@ func (c *Config) validate() error {
  return fmt.Errorf("%v `otlp.timeout` must be positive: %vms", c.ID(), c.OTLPExporterConfig.Timeout.Milliseconds())
  }
 
+ if len(c.LogLevel) > 0 {
+ if _, ok := validLogLevels[strings.ToLower(c.LogLevel)]; !ok {
+ return fmt.Errorf("%v `log_level` must be one of %s", c.ID(), listKeys(validLogLevels))
+ }
+ }
+
+ for _, system := range strings.Split(c.TargetSystem, ",") {
+ if _, ok := validTargetSystems[strings.ToLower(system)]; !ok {
+ return fmt.Errorf("%v `target_system` list may only be a subset of %s", c.ID(), listKeys(validTargetSystems))
+ }
+ }
+
  return nil
 }
+
+func listKeys(presenceMap map[string]struct{}) string {
+ list := make([]string, 0, len(presenceMap))
+ for k := range presenceMap {
+ list = append(list, fmt.Sprintf("'%s'", k))
+ }
+ sort.Strings(list)
+ return strings.Join(list, ", ")
+}