Disable ProcessResourceProvider by default

[inssein]

Is your feature request related to a problem? Please describe.

By default, auto instrumentation ships process.command_args, which is very dangerous as a lot of java services pass in secrets via command line arguments (see example in additional context).

Describe the solution you'd like

Can we disable this by default? I see there was some agreement to do this in open-telemetry/opentelemetry-java#3240, but the answer in open-telemetry/opentelemetry-java#4231 doesn't quite do that.

Describe alternatives you've considered

In the interim, we are testing setting OTEL_JAVA_DISABLED_RESOURCE_PROVIDERS to io.opentelemetry.instrumentation.resources.ProcessResourceProvider for all of our java apps.

Additional context

java \
  -Dkeycloak.clientSecret="${KEYCLOAK_SECRET:-test}" \
  -jar app.jar

[inssein]

Sorry, looks like the issue got auto closed when we merged an internal PR that added the workaround mentioned in alternatives.

[trask]

I would support either not capturing the command line by default, or at least scrubbing system property values, e.g. -Dkey=****.

@jack-berg @jkwatson wdyt?

[laurit]

https://opentelemetry.io/docs/specs/semconv/resource/process/ states that process.command, process.command_args and process.command_line are required.

[jack-berg]

The resolution was to:

Every ResourceProvider should have a "name", akin to the names for exporters, samplers, etc.
You then do one of 3 things:
don't specify anything extra, get all the ResourceProviders
-Dotel.java.disabled.resource.providers=process,os to exclude the "process" and "os" named ResourceProviders
-Dotel.java.enabled.resource.providers=process,os to only include the "process" and "os" named ResourceProviders

This is the behavior today.

Maybe the answer is to make it easier to discover which resource providers are available and how to opt-in or opt-out of them? We could extend the java resources doc page to include info on this, and include a table of all the resource providers, and the FQCN needed to disable them with OTEL_JAVA_DISABLED_RESOURCE_PROVIDERS.

I'm also not opposed to some suggestions around some sort of scrubbing by default.

[inssein]

My main concern here is that we should have safe defaults, and to be fair, I don't know how big of a problem this is today.

For whatever reason, majority of the java services we have today at my current workplace passes in secrets via system properties. All of our services by default would be leaking our secrets to our telemetry provider.

In the past, java services I have worked on have either gotten a configuration file with secrets or more often used environment variables, where this is not an issue.

How common is it to pass secrets via command line args, and if it's common, we need to either scrub or make it opt-in.

Edit: Is there an easy way to make only the args opt-in, and not the whole resource provider?

[jack-berg]

How common is it to pass secrets via command line args, and if it's common, we need to either scrub or make it opt-in.

I'd say common enough that we should do something safe by default. I'm in favor of scrubbing, since Lauri points out that these are currently required. I do think we should open an issue in semantic-conventions to revise either revise the conditions of "conditionally required" to give an out for languages like java where including secrets is common, or maybe even reduce the requirement level to recommended or opt-in. Can you open an issue?

[breedx-splk]

Ugh, I think it's quite unfortunate that we're considering scrubbing (changing/masking) commandline arguments because some folks (still?) pass secrets that way. It's been a bad/insecure practice for a VERY long time. The commandline arguments are something I use ALL THE TIME when troubleshooting customer issues, especially when otel specific configuration is provided via system properties (eg. -Dotel.whatever=foo). If the default implementation masked arguments, I definitely would have missed a number of problem configurations in the past.

I'd much rather favor an opt-in approach here, where users who have gone out of their way to provide secrets insecurely can prevent them from being leaked to the o11y vendor by providing an additional argument. That's my $0.02.

[cmmcleod]

@breedx-splk The reason why I want to disable these attributes is non-security related: long strings, paths, and other irrelevant data (in terms of telemetry) are passed to our applications, and ends up being a non-trivial volume of data unnecessarily transmitted along side the telemetry data.

[bcmedeiros]

It's indeed a lot of data, just the classpath is ridiculous if the app enumerates all jars, for example.
And it's important to remember, this is sent on every single event and it's static information that will never change after the app is started. Quite a waste of resources in my opinion.

[laurit]

this is sent on every single event

with otlp exporter resource attributes are sent once per batch of exported data

[breedx-splk]

@cmmcleod @bcmedeiros Thanks for sharing your input on this. I can definitely understand the length/waste argument much more than the security argument. @laurit also makes a good point that the resource is only sent per batch, so that it's not duplicated with each piece of telemetry.

I think one way to solve this more generally (and this repo is not the place to solve it) is by standardizing some set of "startup events" (for lack of a better term right now) or "app config events" or something. The idea is that you could send the commandline (and other non-changing or infrequently changing) data at startup and then stitch it together after the fact. I think some work was started on this (maybe @jack-berg lit some kindling last year?) but there are two challenges: this approach requires some kind of unique run/instance id for correlation (and probably more work for o11y backends), and the events APIs have been in flux.

In any case, disabling it by default should be fine I suppose, and vendors and users can always choose to override if they want.

[bcmedeiros]

makes a good point that the resource is only sent per batch, so that it's not duplicated with each piece of telemetry.

batches are still quite frequent, aren't they? if not, latency would be affected and events show up pretty quickly in my experience.

I like the "startup events" idea.

[trask]

I got a report today of someone noticing that -Djavax.net.ssl.trustStorePassword was captured in process.command_args. What do folks think of special casing and scrubbing this specific arg as -Djavax.net.ssl.trustStorePassword=****?

[laurit]

I got a report today of someone noticing that -Djavax.net.ssl.trustStorePassword was captured in process.command_args. What do folks think of special casing and scrubbing this specific arg as -Djavax.net.ssl.trustStorePassword=****?

we could scrub keys that contains password or secret