Use of "go mod verify" in Makefile results in unreproducible builds

[leoluk]

The go.sum make target (required by the build target) runs go mod verify:

cosmos-sdk/Makefile

Lines 113 to 115 in 93e8f46

	go.sum: tools go.mod
	@echo "--> Ensure dependencies have not been modified"
	@go mod verify

There are two concerns with this:

• The tools target pulls in unverified third party dependencies.
• go mod verify does not respect -mod=readonly and will modify go.mod and go.sum during build if the minimum version selection algorithm finds updated dependencies (cmd/go: 'mod verify' should not modify the go.mod file golang/go#31372)

Suggested fix is to remove the target. Tools aren't required for the build target and go mod verify is unnecessary.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

@leoluk thanks for creating an issue! I believe the reason why go.sum depends on the tools target is because it needs gosum. We definitely need to add -mod=readonly to the verification.

Maybe @alessio can shed some more light here.

[alessio]

@leoluk Thank you for taking the time to report this bug and helping to make Cosmos SDK better.

Few things:

• Although I was not aware of cmd/go: 'mod verify' should not modify the go.mod file golang/go#31372 when I was working on porting the repo to go mod, I kinda suspected that in certain conditions go mod verify could actually modify go.sum unpromptedly. That's why the go.sum target depends on go.mod: as long as you don't modify go.mod, make go.sum is supposedly no-op, though...
• ...it could (and it does) actually modify go.sum even when go.mod had not been changed! And yes, you have correctly identified the culprit: the tools target.
• Wrapping up go mod verify in a Makefile target should be a good mitigation to the upstream issue. In other words: if you change something in go.mod and you run make go.sum then expect changes in go.sum. If you go.mod is unchanged and you run make go.sum, go mod verify will just not run.
• Rest assured that we will add -mod=readonly to go mod verify as soon as upstream makes it available.

Furthermore, concerning the following:

The tools target pulls in unverified third party dependencies.

All tools but golangci-lint are downloaded via either go get or git clone and installed through go install. golangci-lint is the only exception: we download its installer (via curl -sfL, see contrib/devtools/install-golangci-lint.sh), compute its hashsum and make sure it matches an expected one (see contrib/devtools/Makefile), then we run it [1].
If you think something is wrong in the way we dowload/install developer tools, I'll be very happy to welcome suggestions on how we can improve it!

[1] We have enabled golangci.com Github's integration too. We are presently testing it and we may stop running golangci-lint in our CI builds at some point in the near future to then only rely on the automation with golangci.com. However, I guess that we will likely keep a lint target in the Makefile to run golangci-lint locally for developers convenience.

[leoluk]

This is only a partial fix, no? Builds are still unreproducible.

Any of the dependencies in go.mod could do a minor release and break builds.

[alessio]

@leoluk make go.sum is now provided with a mitigration to the go mod's lack of support for -mod=readonly. Running go mod verify by hand may still trigger dependencies update, thus ensuing builds may still end up being unreproducible.

We are not yet providing reproducible builds. We have an relevant issue to track work on it: #4027

We need to come up with a proper (and preferably simple) solution to achieve build reproducibility. I thought of https://gitian.org/ - do you have any recommendations?

[leoluk]

As far as I can tell, it is still executed as the build target depends on go.sum.

Aside from that, the builds are already reproducible with CGO disabled and a known build environment (paths, Go compiler version). We make use of this property internally to validate the integrity of our builds and this is how this issue was found.