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.

Sign up for GitHub

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

Jump to bottom

cmd/go: describe difference in go mod verify vs verification during 'go mod tidy' #36130

Open
hyangah opened this issue Dec 13, 2019 · 3 comments
Open

cmd/go: describe difference in go mod verify vs verification during 'go mod tidy' #36130

hyangah opened this issue Dec 13, 2019 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@hyangah
Copy link
Contributor

hyangah commented Dec 13, 2019

What version of Go are you using (go version)?

$ gotip version
go version devel +ef3ef8fcdf Wed Dec 11 15:43:50 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ gotip env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="$HOME/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="$HOME/golang/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="$HOME/golang/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/s/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build777528967=/tmp/go-build -gno-record-gcc-switches"

What did you do?

tainted the hash in the go.sum file in workspace (e.g. modifying the hash values manually) and ran go mod tidy and go mod verify

What did you expect to see?

go mod verify reports the problem.

What did you see instead?

go mod verify reports everything is all good.
Thankfully, go mod tidy detects the problem.

$ go mod verify
all modules verified
$  go mod tidy
verifying golang.org/x/text@v0.0.0-20170915032832-14c0d48ead0c: checksum mismatch
	downloaded: h1:qgOY6WgZOaTkIIMiVjBQcw93ERBE4m30iBm00nkL0i8=
	go.sum:     h1:qgOY6WgZOaTkIIMiVjBQcw93ERBE4m30iBm00nkl0i8=

SECURITY ERROR
...

I guess it's because go mod verify checks only whether the version in the cache is valid. go mod help verify implies that already, but I found this behavior somewhat surprising.
@toothrot toothrot added this to the Backlog milestone Dec 13, 2019
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 13, 2019
@mvdan
Copy link
Member

mvdan commented Dec 14, 2019

Hm, it sounds like a bug to me if go mod tidy verifies more stuff than go mod verify. Why not make verify exhaustive?

@jayconrod
Copy link
Contributor

@mvdan Agreed that verify should be exhaustive.

All commands that download modules will check sums when those modules are ingested into the cache.

go mod verify checks the sums of modules that are already in the cache. It doesn't download anything that's not already there though. I don't think it looks at go.sum. Its purpose is to detect tampering with the cache after modules are downloaded.

@mvdan
Copy link
Member

mvdan commented Dec 17, 2019

I'm not sure if I follow the entire puzzle. To feel entirely safe, I'd assume that go mod verify would do all the consistency checks that go mod tidy does, and would also:

  1. Verify that go.sum agrees with the module proxy, e.g. proxy.golang.org
  2. Verify that the module cache agrees with go.sum

Part 1 would probably need to be best-effort, due to env vars like GOPROXY and GONOSUMDB, but I think that's fine.

If that's what you mean, all good. When you said "go mod verify doesn't look at go.sum", what I understood is that there isn't a bug to fix here, as it's working as intended.

@thxCode thxCode mentioned this issue Jul 24, 2020
Merged
@atc0005 atc0005 mentioned this issue Jan 1, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@toothrot @jayconrod @mvdan @hyangah