community
community copied to clipboard
open-telemetry
Security response in OpenTelemetry
Hey everyone -- as many have seen, there is a major security vulnerability in log4j 2.0 - 2.14.1. If you search for log4j in the opentelemetry-java-instrumentation repository it may appear that the instrumentation is vulnerable. Turns out, the instrumentation instruments log4j but does not pull in log4j as a dependency even though from the build.gradle it looks like it does (my understanding is that the dependency is handled via muzzle and only uses what is available on the application if anything -- it is not pulled in by the instrumentation itself).
This issue is interesting for a couple of reasons:
- It raises the concern of security response in OTel -- clearly, a SECURITY.md exists, but what is the response? For example, Kubernetes)
- It raises the concern of how issues like this should be handled. Should the minimum version be increased so scanners do not report a false-positive, should an issue be created to track per repo (thus something publicly referenceable by others), should a note be added to some README (again something public to reference), should something else be done?
- It raises the concern of security response in OTel -- clearly, a SECURITY.md exists, but what is the response? For example, Kubernetes)
The vulnerabilities reported to cncf-opentelemetry-tc are received by TC. The TC typically creates an appropriate private issue in the affected repo and asks the maintainers to work on it. We can document this process if we want.
For log4j we have not received any report. I guess everyone was busy patching their own stuff.
It raises the concern of how issues like this should be handled. Should the minimum version be increased so scanners do not report a false-positive, should an issue be created to track per repo (thus something publicly referenceable by others), should a note be added to some README (again something public to reference), should something else be done?
@open-telemetry/java-instrumentation-approvers please comment on this.
Should the minimum version be increased so scanners do not report a false-positive?
I'd prefer not to do this, as the instrumentation intentionally supports old versions of libraries (but does not ship or transitively pull in those old versions).
Should an issue be created to track per repo (thus something publicly referenceable by others)?
This seems reasonable given the severity and breadth of this particular CVE, but I don't think we would generally proactively comment on CVEs that do not affect us.
I pinned an announcement to the opentelemetry-java-instrumentation repo discussions: https://github.com/open-telemetry/opentelemetry-java-instrumentation/discussions. Will plan to unpin it in a month or two once things calm down.
Should the minimum version be increased so scanners do not report a false-positive?
I'd prefer not to do this, as the instrumentation intentionally supports old versions of libraries (but does not ship or transitively pull in those old versions).
Here's the downstream impact of that for some users: It's hard to have a list of issue suppressions to manage in the SBOM/SCA tool when the dependencies are nuanced. Yes, you can point to the tools and say, "They should figure that out." But, not all do. The net is we need to do periodic review in case you decide to pull in log4J in the future. I assume the burden of that feels incremental to you, probably because it is. However, the aggregate impact of all projects that make these decisions is non-trivial. I get it if that doesn't change your mind. You're balancing one aspect of DX vs another. But, I had to get that off my chest. Now, I feel better. :-)
hi @bobharwood! we definitely understand this pain, which is (partly) why:
does not ship or transitively pull in those old versions
How is your security tool picking up this dependency (and version), since it is not pulled in transitively? We are happy to revisit if we can understand this part better.
Tools, some are home-grown, that are in the editor are just looking at POM entries (since it's pre-build). The build tools and repo scanners - especially the commercial ones - are able to handle it. Our shift left mentality is trying to get warnings to the developer as early as possible. I know there are commercial plug-ins for editors that do a better job, but $, labor, etc leads to a long-tail of conversion. I'm not sure how wide-spread this issue is. Maybe I'm unique. (My mom said I am, but that's another story... )
Thanks, which specific opentelemetry dependency are you using that is getting flagged?
I'm told it's here: instrumentation/log4j/log4j-context-data/log4j-context-data-2.16/javaagent/build.gradle.kts
the tool scans the dependency section and sees: dependencies { library("org.apache.logging.log4j:log4j-core:2.16.0") and flags it.
They also said it might be happening with the java appender, too.. not sure.
I looked through the file and saw it was listed in muzzle, so maybe that's on us to update the tool.. or we can up it to 2.17 and see.
Eh... before you spend too much time on this, let me see why they are doing things the way they are... if it's a larger issue, I'll repost.
ah, if it's just an issue of 2.16 vs 2.17, can you open an issue over in https://github.com/open-telemetry/opentelemetry-java-instrumentation? that sounds like something we can probably address
hey @flands! is there anything remaining that you'd like to see addressed as part of this issue?