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/vuln: unexpected publishing latency #66872

Closed
stapelberg opened this issue Apr 17, 2024 · 9 comments
Closed

x/vuln: unexpected publishing latency #66872

stapelberg opened this issue Apr 17, 2024 · 9 comments
Assignees
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo

Comments

@stapelberg
Copy link
Contributor

govulncheck version

% govulncheck -version
Go: go1.22.2
Scanner: govulncheck@v1.0.1
DB: https://vuln.go.dev
DB updated: 2024-04-17 15:34:19 +0000 UTC

No vulnerabilities found.

Share feedback at https://go.dev/s/govulncheck-feedback.

Does this issue reproduce at the latest version of golang.org/x/vuln?

Yes, it’s server-side

Output of go env in your module/workspace:

go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/michael/.cache/go-build'
GOENV='/home/michael/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/michael/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/michael/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/michael/sdk/go1.22.2'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/michael/sdk/go1.22.2/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/michael/go/src/github.com/stapelberg/scan2drive/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4268251989=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I ran govulncheck ./...

What did you see happen?

govulncheck would not print https://pkg.go.dev/vuln/GO-2024-2730, despite my program being affected.

What did you expect to see?

I saw golang/vulndb@cd4d4fe was merged and https://pkg.go.dev/vuln/GO-2024-2730 worked in my browser, so I was surprised to see that it took about 10 more minutes or so for govulncheck to find the vulnerability locally.

At first, I suspected govulncheck was using a locally cached version of the database. I looked for an “update” command but couldn’t find anything. Finally I read the source to see that govulncheck always seems to query the database (but doesn’t print anything about that), and that’s roughly when govulncheck invocations started showing the expected output:

Vulnerability #1: GO-2024-2730
    Directory traversal in FilesystemStore in github.com/gorilla/sessions
  More info: https://pkg.go.dev/vuln/GO-2024-2730
  Module: github.com/gorilla/sessions
    Found in: github.com/gorilla/sessions@v1.2.0
    Fixed in: N/A
    Example traces found:
      #1: internal/webui/web.go:93:30: webui.UI.indexHandler calls sessions.FilesystemStore.Get
      #2: internal/webui/web.go:76:25: webui.UI.constantsHandler calls sessions.Session.Save

What is the current publishing latency and could it be improved? Is there a cache that should be invalidated or something along these lines?

@stapelberg stapelberg added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Apr 17, 2024
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Apr 17, 2024
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 17, 2024
@cherrymui
Copy link
Member

cc @golang/vulndb

@zpavlinovic
Copy link
Contributor

Thanks for reporting this.

Yes, govulncheck does not do any type of caching. We don't print anything related to that so as to not confuse the users. They shouldn't even be aware of its (non-)existence.

It is hard to tell why you are seeing this. We'll try to dig on the server side for more information.

@stapelberg
Copy link
Contributor Author

Yes, govulncheck does not do any type of caching. We don't print anything related to that so as to not confuse the users. They shouldn't even be aware of its (non-)existence.

…but maybe you could print something about querying a remote server? That would be helpful, not just to establish the correct mental model, but also for many other issues (e.g. slow networking).

Thanks for taking a look at the server side.

@zpavlinovic
Copy link
Contributor

Would that just be a message upon successful retrieval of information from the vuln db server?

I think that if we decide to add this, then we'd likely hide this under the -show verbose option. We already have people mentioning that govulncheck is quite chatty.

@stapelberg
Copy link
Contributor Author

I suggest printing a message when starting the request, not afterwards. That way, if the program hangs, it’s very clear what’s happening: the last line will tell you exactly what it’s trying to do.

Hiding this behind a verbose flag sounds reasonable.

@gopherbot
Copy link

Change https://go.dev/cl/580175 mentions this issue: internal/scan: print progress messages only in verbose mode

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Apr 18, 2024

Govulncheck first prints out the message about the user program and then goes to fetching vulnerabilities for that program. So this message would be the second one. If govulncheck hangs, then it is very likely due to the analysis taking a long time. A message on starting the request would then be misleading.

Edit: we'll just also add an additional message when the checking phase starts.

@gopherbot
Copy link

Change https://go.dev/cl/580216 mentions this issue: internal/vulncheck: emit fetch db and vuln checking progress messages

gopherbot pushed a commit to golang/vuln that referenced this issue Apr 18, 2024
And also update the documentation.

Updates golang/go#66872

Change-Id: I73f0c7e9c1f46b66711b878748bf78571d26f66b
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/580175
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@zpavlinovic
Copy link
Contributor

We looked at the server side and we believe this has something to do with replication and eventual-consistency. We could dig deeper into that, but it is not clear if it is really worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

5 participants