cmd/go: disable server fetch by full hash

[FiloSottile]

Unfortunately, the two major code hosts used in the Go ecosystem—GitHub and Gerrit—allow anyone to inject commits into any git repository. On GitHub PRs show up in refs/pull/ in the main repo, and in Gerrit CLs show up as refs/changes/. This regularly causes confusion.

This means that they are reachable across the wire. The git policy is that anything reachable from a ref can be fetched remotely. It also means they can be used as preudoversions.

For example, golang.org/x/crypto@v0.0.0-20190330071201-e19e1a02deae is a valid pseudoversion for the unmerged https://go-review.googlesource.com/c/crypto/+/169037/3. It also ranks higher than master, which is older.

The problem is that anyone would take a PR that updates golang.org/x/crypto from v0.0.0-20190325154230-a5d413f7728c to v0.0.0-20190330071201-e19e1a02deae, and the chances that they would review the diff are none. In modules world in particular, using a module delegates trust to its author, but allowing anyone to effectively publish a pseudoversion for any module breaks that trust system.

We need to make sure anything we fetch is reachable from refs/heads or refs/tags.

Besides performance, there is a major drawback to that: if something was reachable but isn't anymore, it won't be possible to fetch it directly anymore. I'm afraid there is no way around it. (Hopefully a proxy might still have it cached.)

This is a superset of #30434: fixing this fixes the unexpected behavior there, too.

/cc @jayconrod @bcmills

[FiloSottile]

@gopherbot please open backport issues for this. It is a security issue and we want to make sure the entire modules ecosystem will reject the external pseudoversions.

[gopherbot]

Backport issue(s) opened: #31195 (for 1.11), #31196 (for 1.12).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

[FiloSottile]

By the way, this came up in a few discussions, but I think the first one to notice the issue might have been @dmitshur.

[rsc]

We need to make sure anything we fetch is reachable from refs/heads or refs/tags.

The go command mostly already does this, and has since Go 1.11, for exactly the reasons you give.

$ go clean -modcache
$ go list -m golang.org/x/crypto@v0.0.0-20190330071201-e19e1a02deae
go: finding golang.org/x/crypto v0.0.0-20190330071201-e19e1a02deae
go list -m golang.org/x/crypto: unknown revision e19e1a02deae
$ go list -m golang.org/x/crypto@e19e1a02deae
go: finding golang.org/x/crypto e19e1a02deae
go list -m golang.org/x/crypto: unknown revision e19e1a02deae
$ 

It does this by ignoring refs that are not named refs/heads/* or refs/tags/* in the ref list and by only downloading those patterns when fetching the full repo. The bad commit should never enter the cached repo.

There is one exception that can pollute the module cache, and that's naming the full hash on the command line, when the remote server supports naming a hash directly in the Git fetch, as Gerrit does:

$ go list -m golang.org/x/crypto@e19e1a02deae137265a17f61a8319ad7e8c0ca13
go: finding golang.org/x/crypto e19e1a02deae137265a17f61a8319ad7e8c0ca13
golang.org/x/crypto v0.0.0-20190330071201-e19e1a02deae
$ go list -m golang.org/x/crypto@e19e1a02deae
golang.org/x/crypto v0.0.0-20190330071201-e19e1a02deae
$ go list -m golang.org/x/crypto@v0.0.0-20190330071201-e19e1a02deae
golang.org/x/crypto v0.0.0-20190330071201-e19e1a02deae
$

At this point, the shorter versions start working:

$ go list -m golang.org/x/crypto@v0.0.0-20190330071201-e19e1a02deae
golang.org/x/crypto v0.0.0-20190330071201-e19e1a02deae
$ go list -m golang.org/x/crypto@e19e1a02deae
golang.org/x/crypto v0.0.0-20190330071201-e19e1a02deae
$ 

There's no way to trigger that fetch remotely. In particular, go.mod files from dependencies do not auto-resolve non-semver versions to semver equivalents, and so if a malicious dependency said

require golang.org/x/crypto e19e1a02deae137265a17f61a8319ad7e8c0ca13

that would be rejected as a syntax error before even getting to the git code.

If you can convince someone to type golang.org/x/crypto@e19e1a02deae137265a17f61a8319ad7e8c0ca13 into their go command line, then you can get them to poison their cache, but otherwise there is no problem here. And if people are blindly accepting go commands from you, I think that's outside our threat model.

I can disable “fetch hash with unknown ref directly from server” for Go 1.13, but I don't think this is a security issue that warrants backporting (there is no real problem to correct, weighed against the always-non-zero chance of introducing a real problem).

[bcmills]

The main place where this is likely to occur today, I think, is when we convert a go.mod file from an existing legacy lockfile.

It's also relatively easy to smuggle in such a commit though a `go.mod` file in a transitive dependency, since it's not clearly marked as out-of-tree.

I agree that we should at least fix this for 1.13.

[rsc]

It's also relatively easy to smuggle in such a commit though a go.mod file in a transitive dependency, since it's not clearly marked as out-of-tree.

I don't understand what you mean by "not clearly marked as out-of-tree". When reading a go.mod from a dependency, a 40-byte hex hash will be rejected as a syntax error.

[bcmills]

Hmm... I think I was just confused. Sorry for the noise.