cmd/go: go get ignores go.mod from remote repo, can escalate into a security vuln

[karalabe]

Happens with all Go versions ever supporting modules.

---

Repro: run outside of a Go module, I just want to install a Go binary:

go install -v github.com/ethereum/go-ethereum/cmd/geth

What happens:

go/src/github.com/ethereum/go-ethereum/metrics/influxdb/influxdb.go:10:2: cannot find package "github.com/influxdata/influxdb/client" in any of:
	/opt/google/go/src/github.com/influxdata/influxdb/client (from $GOROOT)
	/tmp/go/src/github.com/influxdata/influxdb/client (from $GOPATH)

Why? Because Go flat out ignores the module file distributed in that repo; and upstream nuked their repository to pieces so there's no code there any more.

---

Doing it manually works:

$ git clone https://github.com/ethereum/go-ethereum
$ cd go-ethereum
$ go install ./cmd/geth

---

This is apparently the same bug that was a release blocker in Go 1.12, and then just ignored and locked: #24250

[karalabe]

Here's a way to escalate this bug into a security vulnerability: https://github.com/karalabe/go-issue-38196-keygen

[seankhliao]

I believe this is go in gopath mode working as it always has been

if you want to install in module mode GO111MODULE=on go get github.com/karalabe/go-issue-38196-keygen

related #30515

[ALTree]

This is apparently the same bug that was a release blocker in Go 1.12, and then just ignored and locked: #24250

The issue you linked (#24250) was marked as a 1.12 release blocker, fixed by CL 148517, which was included in 1.12, and thus closed by the gopherbot when the commit was merged.

This is the opposite of "ignored and locked". Did you link the wrong issue?

[karalabe]

I believe this is go in gopath mode working as it always has been

if you want to install in module mode GO111MODULE=on go get github.com/karalabe/go-issue-38196-keygen

@seankhliao

$ GO111MODULE=on go get github.com/karalabe/go-issue-38196-keygen
go: downloading github.com/karalabe/go-issue-38196-keygen v0.0.0-20200401121913-5c894df7c1c5
go: github.com/karalabe/go-issue-38196-keygen upgrade => v0.0.0-20200401121913-5c894df7c1c5

$ go-issue-38196-keygen
Generating crypto key: insecure key

Also as far as I know as of Go 1.13, the default is on, so that should not make much of a difference.

The issue you linked (#24250) was marked as a 1.12 release blocker, fixed by CL 148517, which was included in 1.12, and thus closed by the gopherbot when the commit was merged.
This is the opposite of "ignored and locked". Did you link the wrong issue?

Hmm, I did not see that part, only the last lines in the issue:

@golang golang locked and limited conversation to collaborators on Feb 26
@gopherbot gopherbot added the FrozenDueToAge label on Feb 26

The last comments from that issue seem to be exactly this: installing a binary will not use the proper go modules.

[karalabe]

@sean-jc Hmm, scratch that, apparently go get without GOBIN stashed the binary somewhere weird:

$ GOBIN=`pwd` GO111MODULE=on go get github.com/karalabe/go-issue-38196-keygen
go: github.com/karalabe/go-issue-38196-keygen upgrade => v0.0.0-20200401121913-5c894df7c1c5

$ ./go-issue-38196-keygen 
Generating crypto key: secure key

[seankhliao]

Also as far as I know as of Go 1.13, the default is on, so that should not make much of a difference.

GO111MODULE continues to default to auto as of go1.14

[myitcv]

Please can I humbly make a suggestion:

Go flat out ignores

then just ignored and locked

Well, shit.

(last one quoted from the linked repro repository).

There is no need to resort to such hyperbole, not least when you might not be in possession of all the facts.

Instead, asking questions is more powerful way of conveying the same point:

Go appears to ignore... if I've understood this correctly?

appears to have been closed without a solution?

This is somewhat surprising

[karalabe]

@myitcv Fair points. It's just a bit frustrating when I see the following timelines:

rsc added NeedsFix release-blocker labels on Apr 9, 2019
rsc modified the milestones: Go1.13, Go1.14 on May 9, 2019
jayconrod modified the milestones: Go1.14, Go1.15 on Nov 11, 2019

It's fine to keep pushing a feature/fix if it doesn't make it into a release. It's just strange that something is important enough to be tagged a release-blocker, and then pushed 3 releases and counting. It seems to indicate that there is a lack of interest to solve the issue, even though everyone agrees on its severity. Hence why my strong language.

[myitcv]

Understood, but I think the first step should be asking questions to confirm you have the correct understanding/interpretation; that costs nothing and builds rapport with people, rather than potentially putting people's backs up.

Your frustration is shared by a number of people through #30515 and friends. This particular topic has been raised on the golang-tools calls many times. Unfortunately more pressing issues have pushed this down the priority list.

[andybons]

@jayconrod @bcmills @matloob

[bcmills]

@karalabe, issue #24250 was fixed in Go 1.12. But read the issue title carefully (emphasis mine):

cmd/go: allow "go get" when outside a module in module mode

You have to be in module mode for it to take effect. The way to activate module mode is to either invoke the go command within the working directory of a module, or explicitly set GO111MODULE=on. (This behavior is documented in https://golang.org/cmd/go/#hdr-Module_support. Improving discoverability for go command documentation is #33637.)

You can use go env GOMOD to confirm whether you are in module mode: it will report a non-empty value if (and only if) the go command is in module mode. (We ask for the output of go env in the issue template in part so that we can figure out whether the go command was in module mode — please take care to fill out the whole template next time.)

[bcmills]

Here's a way to escalate this bug into a security vulnerability

That is GOPATH mode working as it has always worked. It is only a security vulnerability to the extent that GOPATH mode is less secure than we would like it to be.

If you want to ensure that your users install your binary in module mode, you can ask them to set GO111MODULE=on in your install instructions. Otherwise, this issue is essentially the same as #30228 (default GO111MODULE to on), which was replaced by #31857 (revert to auto with changes) for Go 1.13.

At some point we should make another run at defaulting GO111MODULE to on, but that will need another proposal.

[karalabe]

That is GOPATH mode working as it has always worked. It is only a security vulnerability to the extent that GOPATH mode is less secure than we would like it to be.

Technically you are right. Practically I'd argue a bit against it, because whilst it is exactly as secure as it was before, most people (myself included) don't know about this silent revertal to the old behavior in certain cases. Since the go.mod file is in the repo, I would have never considered that there's an easy way to go around it.

If you want to ensure that your users install your binary in module mode, you can ask them to set GO111MODULE=on in your install instructions.

That is certainly doable, but my guess would be that most Go devs don't read the manual on how to install a binary, since Go made it so simple since forever. Maybe it's just me getting too used to it.

Otherwise, this issue is essentially the same as #30228 (default GO111MODULE to on), which was replaced by #31857 (revert to auto with changes) for Go 1.13.

Feel free to close this issue.

[jayconrod]

Closing. This will be resolved eventually when GO111MODULE defaults to on. Until then, we can't make significant changes to GOPATH mode without breaking people who rely on the current behavior.