-
Notifications
You must be signed in to change notification settings - Fork 165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document security policy for the Collector #380
Comments
@martinjt can you share the vulnerabilities that trivy is finding? I'll make it easier for us to address them. |
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 |
For completeness:
|
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. |
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 |
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 open-telemetry/opentelemetry-collector-contrib#20245 |
Go 1.21 is out, filed open-telemetry/opentelemetry-collector-contrib/issues/25116 to drop support for Go 1.19. This will unblock open-telemetry/opentelemetry-collector-contrib#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 |
This was about where the written policy is. Where does it say that dependencies are upgraded? 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). |
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.
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. |
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. |
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. |
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 :) |
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. |
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.
The text was updated successfully, but these errors were encountered: