open-telemetry / opentelemetry-collector-releases

OpenTelemetry Collector Official Releases
https://opentelemetry.io
Apache License 2.0
229 stars 144 forks source link

Document security policy for the Collector #380

Open martinjt opened 1 year ago

martinjt commented 1 year ago

I've been working with a client about using the collector, and they have a process of scanning all images with trivy.

They noticed that both the core and contrib images are showing vulnerabilities, and therefore not willing to deploy them. I highly doubt they're actually serious ones as they're likely mitigated internally with the code. The issue is that these types of security teams are policy driven, and any vulnerability is too many. I don't want to discuss the merits of whether or not vulnerabilities existing is an issue (I'm likely on your side, trust me).

What I'm wondering is, is this something that plans to be addressed? I know that container scanning isn't something you're doing right now.

I don't think it's worth me dropping them here to debate their merits, or creating individual issues for them either as I wouldn't be able to provide any guidance on whether they're worth fixing or not.

Here's what they're running.

trivy image otel/opentelemetry-collector:latest
trivy image otel/opentelemetry-collector-contrib:latest
TylerHelmuth commented 1 year ago

@martinjt can you share the vulnerabilities that trivy is finding? I'll make it easier for us to address them.

mx-psi commented 1 year ago

opentelemetry-collector shows GHSA-2w8w-qhg4-f78j for me which can't be addressed for now because of open-telemetry/opentelemetry-collector-contrib/issues/20245 (and talks about Jaeger UI, which we don't use). I don't see anything for contrib so I would also be interested in the results you get @martinjt

TylerHelmuth commented 1 year ago

For completeness:

❯ trivy image otel/opentelemetry-collector:0.82.0

2023-08-02T08:50:41.320-0600    INFO    Need to update DB
2023-08-02T08:50:41.320-0600    INFO    DB Repository: ghcr.io/aquasecurity/trivy-db
2023-08-02T08:50:41.320-0600    INFO    Downloading DB...
38.68 MiB / 38.68 MiB [-----------------------------------------------------------------------------------------------------------------------------------------] 100.00% 14.35 MiB p/s 2.9s
2023-08-02T08:50:45.444-0600    INFO    Vulnerability scanning is enabled
2023-08-02T08:50:45.444-0600    INFO    Secret scanning is enabled
2023-08-02T08:50:45.444-0600    INFO    If your scanning is slow, please try '--scanners vuln' to disable secret scanning
2023-08-02T08:50:45.444-0600    INFO    Please see also https://aquasecurity.github.io/trivy/v0.44/docs/scanner/secret/#recommendation for faster secret detection
2023-08-02T08:50:48.435-0600    INFO    Number of language-specific files: 1
2023-08-02T08:50:48.435-0600    INFO    Detecting gobinary vulnerabilities...

otelcol (gobinary)
==================
Total: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 0, CRITICAL: 0)

┌─────────────────────────────────┬─────────────────────┬──────────┬────────┬───────────────────┬───────────────┬───────────────────────────────────────────────────────┐
│             Library             │    Vulnerability    │ Severity │ Status │ Installed Version │ Fixed Version │                         Title                         │
├─────────────────────────────────┼─────────────────────┼──────────┼────────┼───────────────────┼───────────────┼───────────────────────────────────────────────────────┤
│ github.com/jaegertracing/jaeger │ GHSA-2w8w-qhg4-f78j │ MEDIUM   │ fixed  │ v1.41.0           │ 1.47.0        │ A stored XSS in jaeger UI might allow an attacker who │
│                                 │                     │          │        │                   │               │ controls...                                           │
│                                 │                     │          │        │                   │               │ https://github.com/advisories/GHSA-2w8w-qhg4-f78j     │
└─────────────────────────────────┴─────────────────────┴──────────┴────────┴───────────────────┴───────────────┴───────────────────────────────────────────────────────┘

~/projects 
❯ trivy image otel/opentelemetry-collector-contrib:0.82.0

2023-08-02T08:51:37.879-0600    INFO    Vulnerability scanning is enabled
2023-08-02T08:51:37.879-0600    INFO    Secret scanning is enabled
2023-08-02T08:51:37.879-0600    INFO    If your scanning is slow, please try '--scanners vuln' to disable secret scanning
2023-08-02T08:51:37.879-0600    INFO    Please see also https://aquasecurity.github.io/trivy/v0.44/docs/scanner/secret/#recommendation for faster secret detection
2023-08-02T08:51:42.193-0600    INFO    Number of language-specific files: 0
jpkrohling commented 1 year ago

As @mx-psi said, this affects the UI only, which we don't ship. I wouldn't worry too much about it right now, but I would still love to get this dependency updated.

@frzifus, could you work on updating the Jaeger dependencies there?

martinjt commented 1 year ago

This is the reason I didn't want to put the specific vulnerability, the question isn't about the specific vulnerability.

Your response was pretty much what I said, but it's not really an OK response for the security teams in large orgs.

jpkrohling commented 1 year ago

A more appropriate answer would then be that we have a newly formed SIG Security, which will be helping establish best practices and general security awareness across SIGs.

cc @open-telemetry/sig-security-maintainers

mx-psi commented 1 year ago

could you work on updating the Jaeger dependencies there?

To be clear, we would have to drop support for Go 1.19 on components that use this dependency in order to bump it, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/20245

mx-psi commented 1 year ago

Go 1.21 is out, filed open-telemetry/opentelemetry-collector-contrib/issues/25116 to drop support for Go 1.19. This will unblock https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/20245 and allow us to make trivy happy.

I am still not sure what action is expected out of this issue; our policy is to upgrade all dependencies weekly, there are sometimes blockers for this (notably Go version compatibility in this case), but I think our general policy to upgrade all dependencies and we also have wording here as to what our stance is towards security issues.

Do we consider the issue fixed once the CVEs listed by trivy are fixed? Do we expect something else from this issue? cc @martinjt

martinjt commented 1 year ago

This was about where the written policy is.

Where does it say that dependencies are upgraded? What about monitoring and deprecating previous versions with vulnerabilities? What about release cycles, hot fix releases, and documented vulnerabilities? What's the process?

That's the reason I didn't want to put in specific vulnerabilities in here as it's not about them. This is about security teams' confidence when they do their own scan and find an undisclosed vulnerability.

I'm not, at all, saying that what is being done isn't enough, that everyone isn't doing a thoroughly amazing job. My goal here is/was to raise the fact that the security teams finding those things is a blocker to adoption.

Ultimately, in this scenario, I worked around the issue by telling them to build their own collector using the chainguard base images. So it's not about those vulnerabilities.

Please don't take this as criticism. That isn't the way it's intended. I'm hoping that some of those will be covered by the security SIG.

What would have helped this scenario is an easier place that I could send the security teams that documented the security posture and specifically called out that they should build their own collector, that isn't in github (security teams want web pages, no idea why). To that end, I've got "create page about deploying production collectors safely" on my list to do for docs (for you all to review).

mx-psi commented 1 year ago

Thanks for the writeup @martinjt! I personally didn't take this as anything other than constructive criticism, I just want to know what actionable stuff you intended to get from this since, at least in terms of process, I think it's hard to do much more given the resources we have (documentation about such processes is an area where I agree we can improve).

AIUI these are the things you would like to see:

Am I missing something? I can help create issues to document these if the summary makes sense to you.

This is about security teams' confidence when they do their own scan and find an undisclosed vulnerability.

I want to push back here a bit, while taking into account I am speaking as myself and I don't know what other Collector leads may think here. My stance is that given our limited resources, we should do our best to fix vulnerabilities and make scanners happy, but we generally should invest our limited time into "fixing" vulnerabilities that are not really applicable to our project.

In my opinion, the fact that a commercial vulnerability scanner is giving incorrect results with our binary should be on the company behind it to fix, not on us. We should of course:

but we shouldn't put all our efforts into fixing something that does not need to be fixed just because the company behind this tool has decided that fixing such incorrect results is not worth their time.

mx-psi commented 1 year ago

I just remembered I had filed open-telemetry/opentelemetry-collector-contrib/issues/23372 for the specific Jaeger issue. It's not a trivial amount of work to make it happen, but it would have helped here.

mx-psi commented 9 months ago

Trivy shows no issues for either contrib or core now. I think we should rename the issue title to avoid confusion since it is no longer true.

mx-psi commented 6 months ago

I have renamed this issue to make it more in line with what its contents are. @martinjt if you disagree feel free to change the name back :)

martinjt commented 6 months ago

I think you can probably just close it to be honest.

I'd love to review whether there is now a documented process for security, but I'm not going to get time and I trust that it's on the security SIG's Agenda.