Skip to content
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

Open
martinjt opened this issue Aug 2, 2023 · 14 comments
Open

Document security policy for the Collector #380

martinjt opened this issue Aug 2, 2023 · 14 comments
Assignees
Labels
area:security Security and integrity issues

Comments

@martinjt
Copy link
Member

martinjt commented Aug 2, 2023

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
Copy link
Member

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

@mx-psi
Copy link
Member

mx-psi commented Aug 2, 2023

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

martinjt commented Aug 2, 2023

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
Copy link
Member

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
Copy link
Member

mx-psi commented Aug 2, 2023

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 open-telemetry/opentelemetry-collector-contrib#20245

@mx-psi
Copy link
Member

mx-psi commented Aug 9, 2023

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 trivy are fixed? Do we expect something else from this issue? cc @martinjt

@martinjt
Copy link
Member Author

martinjt commented Aug 9, 2023

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
Copy link
Member

mx-psi commented Aug 9, 2023

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:

  • Document policy regarding dependencies upgrade
  • Document stance regarding support for older versions
    • I think the summary for upstream is "zero support for anything other than the latest minor version" (downstream commercial Collector distros may have a different stance), but still it would be good to have this written somewhere
  • Document release cycles, hotfix releases, documented vulnerabilities and the processes related to it
    • I think this is done and documented here and more generally on the release.md document. Is there anything missing here?
  • Create page about deploying production collectors safely
    • We have some wording here about this that amounts to "please don't deploy contrib directly to prod, use your own distro since you reduce the chance of running into a vulnerability". Writing this somewhere more prominent would be good.

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:

  • adhere to standards that make it easier for tools like this to work
  • evaluate the vulnerabilities that get reported on downstream dependencies
  • address applicable vulnerabilities
  • address non-applicable vulnerabilities if it's easy for us to do/does not conflict with other commitments
  • have docs on all of the above

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
Copy link
Member

mx-psi commented Aug 14, 2023

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.

@frzifus frzifus self-assigned this Aug 15, 2023
@mx-psi
Copy link
Member

mx-psi commented Dec 11, 2023

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 mx-psi added the area:security Security and integrity issues label Jan 10, 2024
@mx-psi mx-psi changed the title Docker image scanning showing vulnerabilities (trivy) Document security policy for the Collector Mar 5, 2024
@mx-psi
Copy link
Member

mx-psi commented Mar 5, 2024

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
Copy link
Member Author

martinjt commented Mar 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:security Security and integrity issues
Projects
None yet
Development

No branches or pull requests

5 participants