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

x/tools/gopls: add analyzer for vulnerability check #50577

Closed
hyangah opened this issue Jan 12, 2022 · 11 comments
Closed

x/tools/gopls: add analyzer for vulnerability check #50577

hyangah opened this issue Jan 12, 2022 · 11 comments
Assignees
Labels
FeatureRequest FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Jan 12, 2022

References:

Experimental command line tool: https://pkg.go.dev/golang.org/x/exp/vulndb/govulncheck
Experimental API: https://pkg.go.dev/golang.org/x/exp/vulncheck
Vuln DB access API: https://pkg.go.dev/golang.org/x/vuln

  • Gopls will publish the analysis results as LSP diagnostics to help easy integration in other LSP-aware integration.
  • We may end up implementing additional custom command or notification to turn on/off the analysis easier or retrieve extra information about vulnerability details.
  • However, vulncheck requires a whole-program analysis so it requires handling slightly different from typical analyzers.

cc @julieqiu @zpavlinovic

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 12, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jan 12, 2022
@suzmue suzmue modified the milestones: Unreleased, gopls/unplanned Jan 13, 2022
@hyangah hyangah modified the milestones: gopls/unplanned, gopls/v0.8.2 Mar 8, 2022
gopherbot pushed a commit to golang/tools that referenced this issue Mar 23, 2022
It is similar to the command line tool govulncheck, but takes
configuration parameters from the given snapshot and runs from
the root directory of the snapshot.

Currently we define result types in this package. When it is
wired to gopls to implement a custom command, they will be moved
to the internal/lsp/ command definition package.

This functionality will be offered only when go1.18+ is used
to build gopls.

Updates golang/go#50577
Updates golang/vscode-go#2096

Change-Id: I08ab6b408d0a40a86cfefff919ab670aa6b2859b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/392538
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/392538 mentions this issue: gopls/internal/vulncheck: add cmd that runs govulncheck-like analysis

@gopherbot
Copy link

Change https://go.dev/cl/395575 mentions this issue: internal/lsp/command: add VulncheckArgs/Result types

@gopherbot
Copy link

Change https://go.dev/cl/395574 mentions this issue: gopls/internal/vulncheck: copy x/vuln/cmd/govulncheck/cache.go

@gopherbot
Copy link

Change https://go.dev/cl/395576 mentions this issue: internal/lsp/command: add RunVulncheckExp

gopherbot pushed a commit to golang/tools that referenced this issue Mar 24, 2022
This is a utility that manages vulnerability information
local cache.

Updates golang/vscode-go#2096
Updates golang/go#50577

Change-Id: I1903a529adda499d078156c3f1ba38bfab75a958
Reviewed-on: https://go-review.googlesource.com/c/tools/+/395574
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 24, 2022
And move types defined in gopls/internal/vulncheck
to internal/lsp/command so VulncheckResult can use them.

Another approach considered is to encode Vuln as a
json raw message. However, presenting the data structure
in gopls api documentation is too nice to give up.

Updates golang/vscode-go#2096
Updates golang/go#50577

Change-Id: I8587d19f9c47cf786dacaae8cfe1727c77cda711
Reviewed-on: https://go-review.googlesource.com/c/tools/+/395575
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 24, 2022
This is a command that runs govulncheck-like analysis.
This is highly experimental and can change any time,
so we mark it with the "Exp" suffix. Once the interface
becomes stable, we will rename this command.

It returns VulncheckResult that can be encoded as
a JSON message. The result includes all potentially
affecting vulnerabilities, and sample traces.

This feature is currently available only when gopls
is compiled with go1.18. Otherwise, the command will
return an error.

Updates golang/go#50577
Updates golang/vscode-go#2096

Change-Id: Ia37b0555f7bf98760292c9f68e50fb70dd494522
Reviewed-on: https://go-review.googlesource.com/c/tools/+/395576
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/404575 mentions this issue: internal/lsp/cmd: add -cfg option to gopls vulncheck

@gopherbot
Copy link

Change https://go.dev/cl/404574 mentions this issue: internal/lsp/cmd: change vulncheck to directly call the hook

gopherbot pushed a commit to golang/tools that referenced this issue May 6, 2022
Instead of invoking the command through the LSP custom command,
call the vulncheck command hook directly. That reduces the extra
overhead of bringing up the full gopls server & package loading.
The vulncheck hook loads packages again any way, so the benefit
of running the check inside gopls's custom command framework
is not huge any more.

Still `gopls vulncheck` is useful - editors don't need to install
another binary for vulncheck feature, and it will output the
result in the format easier to handle than what `govulncheck`
currently offers.

Updates golang/go#50577

Change-Id: Ia21e6d7e0c37c4a1b02dc8bbca860143524c3d1b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404574
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopherbot pushed a commit to golang/tools that referenced this issue May 12, 2022
If -config=true, gopls vulncheck reads the package load
configuration JSON (build flags, env, tests, and later
maybe overlay info) from stdin.

And, add some logging to gopls/internal/vulncheck/command.go
that helps measuring the package loading overhead.

Update golang/go#50577

Change-Id: I5c1ce145b07f2bed03911613f42c09a3d6be6c28
Reviewed-on: https://go-review.googlesource.com/c/tools/+/404575
Reviewed-by: Robert Findley <rfindley@google.com>
@hyangah
Copy link
Contributor Author

hyangah commented May 18, 2022

We realized two different analysis types using the Vuln DB are feasible and useful.

One is a real-time analysis of the package import paths and surfacing the fact in real time using diagnostics.

Another is a full callgraph analysis (golang.org/x/vuln/cmd/govulncheck) that performs the whole program analysis and reports only the vulnerabilities that actually affect the analyzed packages. That can be expensive and it's suitable as one-off analysis command run. Gopls will export that interface as a custom gopls command and gopls vulncheck command (~= govulncheck packaged inside gopls). #52972

This issue is tracking the progress for the first one.

@hyangah hyangah modified the milestones: gopls/v0.8.4, gopls/v0.9.0 May 18, 2022
@nightlyone
Copy link
Contributor

@hyangah may I suggest contributing the second analysis to golangci-lint instead? Or make it available as a goanalysis pass that is easy to integrate into golangci-lint?

The reason is that I am not aware of any CI/CD pipeline that runs gopls commands. But I am aware of dozens of golangci-lint integrations in CI/CD pipelines. Integrating most linter work actually happens primarily there.

@hyangah
Copy link
Contributor Author

hyangah commented May 18, 2022

Thanks for the suggestion. From our experiment - the second analysis takes easily a couple of minutes for a good size repo & accesses the network to retrieve the data. Is it really a good idea to add to a linter? But, I think - adding gopls vulncheck or even govulncheck to CI/CD is a good idea.

@hyangah hyangah modified the milestones: gopls/v0.9.0, gopls/later Jun 9, 2022
@hyangah
Copy link
Contributor Author

hyangah commented Jun 9, 2022

We learned gopls does not compute/hold analysis facts for all packages in the workspace (#48738), so there is some ground work to be done before integrating to gopls.

For short term, we are currently working on the analyzer that is pluggable to a linter that uses the go/analysis framework. (moving off of gopls/v0.9.0 milestone)

@julieqiu julieqiu added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Sep 8, 2022
@hyangah hyangah modified the milestones: gopls/later, gopls/v0.11.0 Dec 5, 2022
@hyangah
Copy link
Contributor Author

hyangah commented Dec 13, 2022

We ended up with a slightly different approach which supports a hybrid mode of vulnerability scanning.

  • Import-based analysis - that reports known vulnerabilities on the analyzed packages and the packages they import transitively. This can result in false positives if the project imports a vulnerable package but doesn't call the vulnerable part of the package.

  • Govulncheck command integration - that reports vulnerabilities that are actually used by doing call graph analysis.

For CI uses, govulncheck and govulncheck -json seems to be working well already.

For more details - https://github.com/golang/vscode-go/wiki/features#analyze-vulnerabilities-in-dependencies

@hyangah hyangah closed this as completed Dec 13, 2022
@golang golang locked and limited conversation to collaborators Dec 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
Status: Done
Development

No branches or pull requests

6 participants