[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:///jndi/rmi://<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, ", ")
+}