Recommended Go 1.16 Workflow

[marten-seemann]

The Go setup-up guide (https://google.github.io/oss-fuzz/getting-started/new-project-guide/go-lang/#dockerfile) recommends using go get to install a project. Since Go 1.16 this doesn't work any more, since packages are not installed in the GOPATH any more.

On the other hand, compile_go_fuzzer expects the code to be located in /root/src/go/<DIRECTORY>, and doesn't even allow using an absolute path.

What's the recommended way to set up a Go project now? Should we manually create the directory structure that Go with GOPATH enabled would have created? Or are some changes to compile_go_fuzzer coming?

[inferno-chromium]

@catenacyber - can you help us on this.

[catenacyber]

Looking into it

[mvdan]

@catenacyber happy to help if you run into any issues making compile_go_fuzzer work with modules :)

[catenacyber]

Thanks @mvdan help is welcome indeed.

Here is what I have.
Quick and dirty fix (will work until go 1.17) is to set env GO111MODULE=off in infra/base-images/base-builder/Dockerfile
It can then be overridden by projects as it was before...

Other fix is to run go mod init ossfuzz.com/dummy in infra/base-images/base-builder/Dockerfile
This way, go get in projects Dockerfiles will use this dummy module and go-fuzz will find them
This looks fragile to me.

The problem is go get (when there is no go.mod) and go-fuzz are now incompatible.
go-fuzz uses "golang.org/x/tools/go/packages" with packages.Load to find the package, which needs a go.mod file in. the current directory, when go get does not need this go.mod file
go get without go.mod "downloads source code into the module cache", but I do not see how ce can use this source code then cf https://golang.org/ref/mod#commands-outside

Last fix would be to change the projects builds scripts so that they use git clone un the right directory instead of go get

[mvdan]

Quick and dirty fix

I think that's fine as a temporary workaround, if nothing else to get a fix to master as soon as possible. But, like you say, it's only a temporary workaround.

The problem is go get (when there is no go.mod) and go-fuzz are now incompatible.

I think go-fuzz is meant to be used from inside the project's module, not outside. For example, here's how I use it directly in module mode: https://github.com/mvdan/sh/blob/master/fuzz

So I personally think that initializing a dummy module should be avoided. We should just use the project's own go.mod directly.

The issue then becomes "how to download the module source code to run go-fuzz in it", like you say below:

go get without go.mod "downloads source code into the module cache", but I do not see how ce can use this source code then

One option, like you say, is to directly use a VCS command like git clone. In practice I think that's going to work well, since the oss-fuzz users are usually the maintainers of the module too, so they'll be well aware of what command to use. You can stop reading here if that solution is good enough :)

--

Another option, if you want to avoid this disconnect between module path and VCS URL, is to ask Go to download the module into its cache, and use that:

$ # note that we don't need a temporary module here
$ dir=$(go list -m -f {{.Dir}} mvdan.cc/sh/v3@latest)
$ echo $dir
/home/mvdan/go/pkg/mod/mvdan.cc/sh/[email protected]
$ go env GOMOD
/dev/null
$ cd $dir
$ go env GOMOD
/home/mvdan/go/pkg/mod/mvdan.cc/sh/[email protected]/go.mod

The above is just an example. You could use any other module, as well as any other version like a git ref (@master, @commit, etc) or a semver tag.

Note that this directory contains the entire module at the version one chooses, but no VCS information like the .git directory. If you want that, you do have to git clone.

That example with go list -m is aimed at a quick command-line demo. You can see golang/go#18387 for ideas on how that might improve in the future. In particular, Go code can call https://pkg.go.dev/golang.org/x/tools/go/vcs#RepoRootForImportPath today to ask "what VCS repository provides this Go package?".

[inferno-chromium]

@catenacyber - i like the use of git clone. we have 40 of these, any easy way you can please help with a cl to migrate these. all of these go projects are broken which is pretty bad :(

[catenacyber]

I think go-fuzz is meant to be used from inside the project's module, not outside.

It used to work in both cases (inside and outside) and different projects used the different ways

We should just use the project's own go.mod directly.

Not every project has a a go.mod (gonids for instance)

So I would like to avoid a solution where there is a need to change many projects build.sh
But it looks like it cannot be avoided...
So, I will try converting go get to git clone for every project...

go list does not work outside a go module directory (so it needs a dummy go module)

> go list -m -f {{.Dir}} github.com/google/gonids
go list -m: module github.com/google/gonids: not a known dependency

[mvdan]

go list does not work outside a go module directory (so it needs a dummy go module)

You need the @latest that I included in my example.

Not every project has a a go.mod (gonids for instance)

Those can keep using GO111MODULE=off with your existing method; they'll need to add a go.mod when the GOPATH mode gets fully removed anyway. And when they do, they can use the new method based on modules.

It used to work in both cases (inside and outside) and different projects used the different ways
So I would like to avoid a solution where there is a need to change many projects build.sh

Hopefully that kind of variance should no longer be necessary with modules. git clone, cd, and go build ./... should work for virtually every Go module hosted on GitHub, for example.

[catenacyber]

Thanks @latest indeed works for gonids... but not for gopacket (downloaded with go get), and it seems to need the network and not use the cache only :

go list -m -f {{.Dir}} github.com/google/gopacket/layers@latest
go list -m: module github.com/google/gopacket/layers: no matching versions for query "latest"

@mvdan can you review #5221 ?

[mvdan]

I should have clarified; the go list -m -f ... method expects a module path, not a package path. I assume that's a problem, since some projects like gopacket want to fuzz a sub-package within the module instead of the root package.

You do need a temporary module to download the module, in that case, because go list without -m doesn't allow being used in module mode outside of a module. golang/go#44203 would address that, but we can't rely on that yet.

We can work around that with a temporary module and a mix of go get and go list:

$ cd $(mktemp -d)
$ go mod init test

$ go get -d github.com/google/gopacket/layers@latest
$ dir=$(go list -f {{.Dir}} github.com/google/gopacket/layers)
$ echo $dir
/home/mvdan/go/pkg/mod/github.com/google/[email protected]/layers
$ cd $dir
$ go env GOMOD
/home/mvdan/go/pkg/mod/github.com/google/[email protected]/go.mod

By the way, note that @latest will use the latest stable semver tag, and if none exists, the default branch. In the GOPATH world, go get would always use the latest default branch, e.g. as per git clone.

[catenacyber]

go list -m -f ... method expects a module path, not a package path.

Indeed it works for gopacket, thanks for the explanation...
But another drawback remains that we need network connectivity (and oss-fuzz kind of expects the fuzzers built out of a docker image to remain the same)

go list -m -f {{.Dir}} github.com/google/gopacket@latest
/root/go/pkg/mod/github.com/google/[email protected]
root@a8fbff87eda2:/src# go list -m -f {{.Dir}} github.com/google/gopacket@latest
go list -m: module github.com/google/gopacket: Get "https://proxy.golang.org/github.com/google/gopacket/@v/list": dial tcp: lookup proxy.golang.org on 192.168.65.1:53: no such host

So, not sure what is best here...

[mvdan]

Ah, right, I was not aware that you want to run the script entirely offline.

The module cache can store many versions for each module, unlike GOPATH, which only supports storing a single version per module/package. This is why the second go list -m call tries to resolve the latest version again, because we're not telling it exactly what version we want - i.e. what version we fetched in the previous step.

I see two options; I'm going to use github.com/google/gopacket/layers as an example, as it uses a sub-package.

1. Go back to git clone, and run compile_go_fuzzer inside the module:

git clone --depth 1 https://github.com/google/gopacket
cd coredns

compile_go_fuzzer ./layers FuzzNewRR fuzz_newrr fuzz

Note that the first argument to compile_go_fuzzer can be a relative package path, like . or ./foo. It can use something like cd $(go list -f {{.Dir}} $path) to enter the directory containing the package, which should work in both GOPATH and module modes.

2. Use a temporary module again, so that the first go get -d records what version was selected in go.mod, and we can then use cd $(go list -f {{.Dir}} $path) directly to enter the right directory for the package inside the module cache. Note that omitting the @version part inside a module will mean using the version described in go.mod. For example:

go mod init test
go get -d github.com/google/gopacket/layers@master

compile_go_fuzzer github.com/google/gopacket/layers FuzzNewRR fuzz_newrr fuzz

Here, the first argument to compiel_go_fuzzer cannot be a relative package path, because we're in the temporary module which has no packages. But the above cd $(go list -f {{.Dir}} $path) would work exactly the same.

[mvdan]

I think I'd pick the first option, just for the sake of simplicity; I think adding an extra temporary module could confuse some users. Plus, having the entire VCS clone is closer to what the old GOPATH method did, so it's less likely to break users.

[roidelapluie]

Prometheus maintainer here - to be clear, do we need to do something to fix our build?

[catenacyber]

@roidelapluie this should get fixed with #5228 without changing anything to prometheus (nor its definition in oss-fuzz)
You are welcome to test it :-)

@mvdan git clone seems the good way to go. But it will need to take care of each project individually...

[mvdan]

@mvdan git clone seems the good way to go. But it will need to take care of each project individually...

Indeed. The old scripts were depending on GOPATH-specific behavior, so I don't think there's a way to avoid that. The good news is that the GOPATH vs modules breakage is unlikely to happen again anytime soon :)

[catenacyber]

The old scripts were depending on GOPATH-specific behavior

The problem looks to me that go get (without a dummy test module, and without specifying @master) is not fit for ossfuzz use case even if it looks nice at first with the "download all the dependencies" feature

[mvdan]

Right. In the old GOPATH world, go get downloaded packages. In the new modules world, go get is mainly meant for adding or updating dependencies in a go.mod. Adding the dummy module into the mix is a possible workaround, but not one I think is worthwhile in the long run.

[catenacyber]

@mvdan syzkaller build has a specific error :

go: inconsistent vendoring in /src/syzkaller:
40147
   mvdan.cc/[email protected]: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod

Do you know about it since your name shows up in it ?
I cannot reproduce it right now, so maybe it got fixed already...

[mvdan]

My guess is that upstream had to run go mod tidy to make go.mod and the vendor folder consistent. If it still fails, try running it there and see if you get any changes. If so, report it upstream.

[catenacyber]

go mod tidy does not seem to fix this...
go mod vendor does fix it locally for me...
@mvdan what should be done ? (and where ?)

[mvdan]

In general, go mod tidy if their go.mod or go.sum files are missing data. I'm pretty sure go mod tidy should also update vendor directories if they are out of date, so if that doesn't work, please show an example.

It should be noted that it's upstreams who should be fixing these problems, though. You having to run either of those commands should only be a temporary workaround.

[catenacyber]

so if that doesn't work, please show an example.

This is syzkaller project
run python infra/helper.py shell syzkaller
Then compile fails
If you run go mod tidy then compile still fails
If you run go mod vendor then compile works

I guess this is the same for teleport project (the error message is the same)

[mvdan]

@catenacyber yeah, as expected, the syzkaller project just has an out of date go.mod/go.sum/vendor. I've sent google/syzkaller#2459 to fix it. The same should be done with other projects, I think. The alternative is to run go mod tidy for all of them (and go mod vendor if vendor/modules.txt exists), assuming that they can't be trusted to keep it up to date.

[catenacyber]

Thanks for he syzkaller patch.
You can also make a PR to change oss-fuzz compile_go_fuzzer so that it runs go mod tidy and go mod vendor in the right conditions... (not sure what these conditions are)

[mvdan]

it should be something like:

if [[ -f go.mod ]]; then
    # Note that Go 1.16 defaults to -mod=readonly now,
    # and some projects still have untidy go.mod/go.sum files.
    # Re-run tidy in case any lines are missing.
    go mod tidy

    if [[ -f vendor/modules.txt ]]; then
        # Similar to the above, 1.16 might complain if modules.txt is out of date.
        go mod vendor
    fi
fi

Note that some projects, like syzkaller, require generating code before go mod tidy can run. I had to run make descriptions, as otherwise it (rightfully) complained about missing Go packages. So the code above should be run after any of those code generation steps.

[catenacyber]

Thanks @mvdan
Indeed, syzkaller seems specific