deprecate using GOPRIVATE in favor of GOGARBLE

Piggybacking off of GOPRIVATE is great for a number of reasons:

* People tend to obfuscate private code, whose package paths will
  generally be in GOPRIVATE already

* Its meaning and syntax are well understood

* It allows all the flexibility we need without adding our own env var
  or config option

However, using GOPRIVATE directly has one main drawback.
It's fairly common to also want to obfuscate public dependencies,
to make the code in private packages even harder to follow.
However, using "GOPRIVATE=*" will result in two main downsides:

* GONOPROXY defaults to GOPRIVATE, so the proxy would be entirely disabled.
  Downloading modules, such as when adding or updating dependencies,
  or when the local cache is cold, can be less reliable.

* GONOSUMDB defaults to GOPRIVATE, so the sumdb would be entirely disabled.
  Adding entries to go.sum, such as when adding or updating dependencies,
  can be less secure.

We will continue to consume GOPRIVATE as a fallback,
but we now expect users to set GOGARBLE instead.
The new logic is documented in the README.

While here, rewrite some uses of "private" with "to obfuscate",
to make the code easier to follow and harder to misunderstand.

Fixes burrowers#276.

README.md

@@ -34,9 +34,11 @@ order to:
 * [Obfuscate literals](#literal-obfuscation), if the `-literals` flag is given
 * Remove [extra information](#tiny-mode), if the `-tiny` flag is given
 
-By default, the tool obfuscates the packages under the current module. If not
-running in module mode, then only the main package is obfuscated. To specify
-what packages to obfuscate, set `GOPRIVATE`, documented at `go help private`.
+The tool obfuscates the packages matching `GOGARBLE`, a comma-separated list of
+glob patterns of module path prefixes, as documented in `go help private`.
+When `GOGARBLE` is empty, it assumes the value of `GOPRIVATE`.
+When `GOPRIVATE` is also empty, then `GOGARBLE` assumes the value of the current
+module path, to obfuscate all packages under the current module.
 
 Note that commands like `garble build` will use the `go` version found in your
 `$PATH`. To use different versions of Go, you can

hash.go

@@ -80,7 +80,7 @@ func alterToolVersion(tool string, args []string) error {
 // and returns a new hash which also contains garble's own deterministic inputs.
 //
 // This includes garble's own version, obtained via its own binary's content ID,
-// as well as any other options which affect a build, such as GOPRIVATE and -tiny.
+// as well as any other options which affect a build, such as GOGARBLE and -tiny.
 func addGarbleToHash(inputHash []byte) []byte {
 	// Join the two content IDs together into a single base64-encoded sha256
 	// sum. This includes the original tool's content ID, and garble's own
@@ -95,8 +95,8 @@ func addGarbleToHash(inputHash []byte) []byte {
 	// We also need to add the selected options to the full version string,
 	// because all of them result in different output. We use spaces to
 	// separate the env vars and flags, to reduce the chances of collisions.
-	if cache.GoEnv.GOPRIVATE != "" {
-		fmt.Fprintf(h, " GOPRIVATE=%s", cache.GoEnv.GOPRIVATE)
+	if cache.GOGARBLE != "" {
+		fmt.Fprintf(h, " GOGARBLE=%s", cache.GOGARBLE)
 	}
 	if opts.ObfuscateLiterals {
 		fmt.Fprintf(h, " -literals")

main.go

@@ -399,16 +399,15 @@ var transformFuncs = map[string]func([]string) (args []string, _ error){
 }
 
 func transformAsm(args []string) ([]string, error) {
-	// If the current package isn't private, we have nothing to do.
-	if !curPkg.Private {
-		return args, nil
+	if !curPkg.ToObfuscate {
+		return args, nil // we're not obfuscating this package
 	}
 
 	flags, paths := splitFlagsFromFiles(args, ".s")
 
 	// When assembling, the import path can make its way into the output
 	// object file.
-	if curPkg.Name != "main" && curPkg.Private {
+	if curPkg.Name != "main" && curPkg.ToObfuscate {
 		flags = flagSetValue(flags, "-p", curPkg.obfuscatedImportPath())
 	}
 
@@ -509,7 +508,7 @@ func transformAsm(args []string) ([]string, error) {
 		}
 
 		// Uncomment for some quick debugging. Do not delete.
-		// if curPkg.Private {
+		// if curPkg.ToObfuscate {
 		// 	fmt.Fprintf(os.Stderr, "\n-- %s --\n%s", path, buf.Bytes())
 		// }
 
@@ -613,7 +612,7 @@ func transformCompile(args []string) ([]string, error) {
 	// If this is a package to obfuscate, swap the -p flag with the new
 	// package path.
 	newPkgPath := ""
-	if curPkg.Name != "main" && curPkg.Private {
+	if curPkg.Name != "main" && curPkg.ToObfuscate {
 		newPkgPath = curPkg.obfuscatedImportPath()
 		flags = flagSetValue(flags, "-p", newPkgPath)
 	}
@@ -642,7 +641,7 @@ func transformCompile(args []string) ([]string, error) {
 		}
 
 		// Uncomment for some quick debugging. Do not delete.
-		// if curPkg.Private {
+		// if curPkg.ToObfuscate {
 		// 	fmt.Fprintf(os.Stderr, "\n-- %s/%s --\n%s", curPkg.ImportPath, name, src)
 		// }
 
@@ -688,7 +687,7 @@ func (tf *transformer) handleDirectives(comments []*ast.CommentGroup) {
 			// This directive has two arguments: "go:linkname localName newName"
 
 			// obfuscate the local name, if the current package is obfuscated
-			if curPkg.Private {
+			if curPkg.ToObfuscate {
 				fields[1] = hashWith(curPkg.GarbleActionID, fields[1])
 			}
 
@@ -714,7 +713,7 @@ func (tf *transformer) handleDirectives(comments []*ast.CommentGroup) {
 				comment.Text = strings.Join(fields, " ")
 				continue
 			}
-			if lpkg.Private {
+			if lpkg.ToObfuscate {
 				// The name exists and was obfuscated; obfuscate
 				// the new name.
 				newName := hashWith(lpkg.GarbleActionID, name)
@@ -781,9 +780,9 @@ var runtimeAndDeps = map[string]bool{
 	"runtime":                 true,
 }
 
-// isPrivate checks if a package import path should be considered private,
-// meaning that it should be obfuscated.
-func isPrivate(path string) bool {
+// toObfuscate checks if a package should be obfuscated given its import path.
+// If you are holding a listedPackage, reuse its ToObfuscate field instead.
+func toObfuscate(path string) bool {
 	// We don't support obfuscating these yet.
 	if cannotObfuscate[path] || runtimeAndDeps[path] {
 		return false
@@ -792,7 +791,7 @@ func isPrivate(path string) bool {
 	if path == "command-line-arguments" || strings.HasPrefix(path, "plugin/unnamed") {
 		return true
 	}
-	return module.MatchPrefixPatterns(cache.GoEnv.GOPRIVATE, path)
+	return module.MatchPrefixPatterns(cache.GOGARBLE, path)
 }
 
 // processImportCfg parses the importcfg file passed to a compile or link step,
@@ -866,7 +865,7 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
 	}
 	for _, pair := range importmaps {
 		beforePath, afterPath := pair[0], pair[1]
-		if isPrivate(afterPath) {
+		if toObfuscate(afterPath) {
 			lpkg, err := listPackage(beforePath)
 			if err != nil {
 				panic(err) // shouldn't happen
@@ -884,7 +883,7 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
 	}
 	for _, pair := range packagefiles {
 		impPath, pkgfile := pair[0], pair[1]
-		if isPrivate(impPath) {
+		if toObfuscate(impPath) {
 			lpkg, err := listPackage(impPath)
 			if err != nil {
 				panic(err) // shouldn't happen
@@ -913,7 +912,7 @@ type (
 var cachedOutput = struct {
 	// KnownObjectFiles is filled from -importcfg in the current obfuscated build.
 	// As such, it records export data for the dependencies which might be
-	// themselves obfuscated, depending on GOPRIVATE.
+	// themselves obfuscated, depending on GOGARBLE.
 	//
 	// TODO: We rely on obfuscated type information to know what names we didn't
 	// obfuscate. Instead, directly record what names we chose not to obfuscate,
@@ -1337,8 +1336,8 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 		if err != nil {
 			panic(err) // shouldn't happen
 		}
-		if !lpkg.Private {
-			return true // only private packages are transformed
+		if !lpkg.ToObfuscate {
+			return true // we're not obfuscating this package
 		}
 		hashToUse := lpkg.GarbleActionID
 
@@ -1449,7 +1448,7 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 		if err != nil {
 			panic(err) // should never happen
 		}
-		if !lpkg.Private {
+		if !lpkg.ToObfuscate {
 			return true
 		}
 		newPath := lpkg.obfuscatedImportPath()
@@ -1829,15 +1828,22 @@ How to install Go: https://golang.org/doc/install
 	if err := json.Unmarshal(out, &cache.GoEnv); err != nil {
 		return err
 	}
-	// If GOPRIVATE isn't set and we're in a module, use its module
-	// path as a GOPRIVATE default. Include a _test variant too.
-	// TODO(mvdan): we shouldn't need the _test variant here,
-	// as the import path should not include it; only the package name.
-	if cache.GoEnv.GOPRIVATE == "" {
+	cache.GOGARBLE = os.Getenv("GOGARBLE")
+	if cache.GOGARBLE != "" {
+		// GOGARBLE is non-empty; nothing to do.
+	} else if cache.GoEnv.GOPRIVATE != "" {
+		// GOGARBLE is empty and GOPRIVATE is non-empty.
+		// Set GOGARBLE to GOPRIVATE's value.
+		cache.GOGARBLE = cache.GoEnv.GOPRIVATE
+	} else {
+		// If GOPRIVATE isn't set and we're in a module, use its module
+		// path as a GOPRIVATE default. Include a _test variant too.
+		// TODO(mvdan): we shouldn't need the _test variant here,
+		// as the import path should not include it; only the package name.
 		if mod, err := ioutil.ReadFile(cache.GoEnv.GOMOD); err == nil {
 			modpath := modfile.ModulePath(mod)
 			if modpath != "" {
-				cache.GoEnv.GOPRIVATE = modpath + "," + modpath + "_test"
+				cache.GOGARBLE = modpath + "," + modpath + "_test"
 			}
 		}
 	}

position.go

@@ -30,8 +30,8 @@ func printFile(file1 *ast.File) ([]byte, error) {
 	}
 	src := buf1.Bytes()
 
-	if !curPkg.Private {
-		// TODO(mvdan): make transformCompile handle non-private
+	if !curPkg.ToObfuscate {
+		// TODO(mvdan): make transformCompile handle untouched
 		// packages like runtime earlier on, to remove these checks.
 		return src, nil
 	}

reverse.go

@@ -65,7 +65,7 @@ One can reverse a captured panic stack trace as follows:
 	var replaces []string
 
 	for _, lpkg := range cache.ListedPackages {
-		if !lpkg.Private {
+		if !lpkg.ToObfuscate {
 			continue
 		}
 		curPkg = lpkg

shared.go

@@ -40,9 +40,11 @@ type sharedCache struct {
 	// we can likely just use that.
 	BinaryContentID []byte
 
+	GOGARBLE string
+
 	// From "go env", primarily.
 	GoEnv struct {
-		GOPRIVATE string // Set to the module path as a fallback.
+		GOPRIVATE string
 		GOMOD     string
 		GOVERSION string
 		GOCACHE   string
@@ -207,11 +209,11 @@ type listedPackage struct {
 
 	GarbleActionID []byte
 
-	Private bool
+	ToObfuscate bool
 }
 
 func (p *listedPackage) obfuscatedImportPath() string {
-	if p.Name == "main" || p.ImportPath == "embed" || !p.Private {
+	if p.Name == "main" || p.ImportPath == "embed" || !p.ToObfuscate {
 		return p.ImportPath
 	}
 	newPath := hashWith(p.GarbleActionID, p.ImportPath)
@@ -263,21 +265,22 @@ func setListedPackages(patterns []string) error {
 		return fmt.Errorf("go list error: %v: %s", err, stderr.Bytes())
 	}
 
-	anyPrivate := false
+	anyToObfuscate := false
 	for path, pkg := range cache.ListedPackages {
-		// If "GOPRIVATE=foo/bar", "foo/bar_test" is also private.
+		// If "GOGARBLE=foo/bar", "foo/bar_test" should also match.
 		if pkg.ForTest != "" {
 			path = pkg.ForTest
 		}
-		// Test main packages like "foo/bar.test" are always private.
-		if (pkg.Name == "main" && strings.HasSuffix(path, ".test")) || isPrivate(path) {
-			pkg.Private = true
-			anyPrivate = true
+		// Test main packages like "foo/bar.test" are always obfuscated,
+		// just like main packages.
+		if (pkg.Name == "main" && strings.HasSuffix(path, ".test")) || toObfuscate(path) {
+			pkg.ToObfuscate = true
+			anyToObfuscate = true
 		}
 	}
 
-	if !anyPrivate {
-		return fmt.Errorf("GOPRIVATE=%q does not match any packages to be built", os.Getenv("GOPRIVATE"))
+	if !anyToObfuscate {
+		return fmt.Errorf("GOGARBLE=%q does not match any packages to be built", cache.GOGARBLE)
 	}
 
 	return nil

testdata/scripts/asm.txt

@@ -1,4 +1,4 @@
-env GOPRIVATE=test/main
+env GOGARBLE=test/main
 
 garble build
 exec ./main

testdata/scripts/basic.txt

@@ -1,4 +1,4 @@
-# Check that the simplest use of garble works. Note the lack of a module or GOPRIVATE.
+# Check that the simplest use of garble works. Note the lack of a module or GOGARBLE.
 garble build -o=main$exe garble_main.go
 exec ./main
 cmp stderr main.stderr

testdata/scripts/cgo.txt

@@ -1,4 +1,4 @@
-env GOPRIVATE=test/main
+env GOGARBLE=test/main
 
 garble build
 exec ./main
@@ -7,12 +7,12 @@ binsubstr main$exe 'privateAdd'
 
 [short] stop # no need to verify this with -short
 
-env GOPRIVATE=*
+env GOGARBLE=*
 garble build
 exec ./main
 cmp stdout main.stdout
 binsubstr main$exe 'privateAdd'
-env GOPRIVATE=test/main
+env GOGARBLE=test/main
 
 garble -tiny build
 exec ./main

testdata/scripts/crossbuild.txt

@@ -8,7 +8,7 @@
 [!arm] env GOARCH=arm
 [arm] env GOARCH=arm64
 
-env GOPRIVATE='*'
+env GOGARBLE='*'
 
 # Link a binary importing net/http, which will catch whether or not we
 # support ImportMap when linking.

testdata/scripts/debugdir.txt

@@ -1,4 +1,4 @@
-env GOPRIVATE=test/main
+env GOGARBLE=test/main
 
 garble -debugdir ./debug1 build
 exists 'debug1/test/main/imported/imported.go' 'debug1/test/main/main.go'

testdata/scripts/embed.txt

@@ -1,4 +1,4 @@
-env GOPRIVATE=*
+env GOGARBLE=*
 
 garble build
 

testdata/scripts/goprivate.txt → testdata/scripts/gogarble.txt

@@ -1,19 +1,25 @@
+# Ensure that "does not match any packages" works with GOPRIVATE and GOGARBLE.
+env GOGARBLE=match-absolutely/nothing
+! garble build -o=out ./standalone
+stderr '^GOGARBLE="match-absolutely/nothing" does not match any packages to be built$'
+
+env GOGARBLE=
 env GOPRIVATE=match-absolutely/nothing
 ! garble build -o=out ./standalone
-stderr '^GOPRIVATE="match-absolutely/nothing" does not match any packages to be built$'
+stderr '^GOGARBLE="match-absolutely/nothing" does not match any packages to be built$'
 
-env GOPRIVATE=test/main/imported
+env GOGARBLE=test/main/imported
 garble build ./importer
 
 # Obfuscated packages which import non-obfuscated std packages.
 # Some of the imported std packages use "import maps" due to vendoring,
 # and a past bug made this case fail for "garble build".
-env GOPRIVATE=test/main
+env GOGARBLE=test/main
 garble build -o=out ./stdimporter
 
 [short] stop # rebuilding std is slow
 
-env GOPRIVATE='*'
+env GOGARBLE='*'
 
 # Try garbling all of std, given some std packages.
 # No need for a main package here; building the std packages directly works the

testdata/scripts/implement.txt

@@ -1,4 +1,4 @@
-env GOPRIVATE=test/main
+env GOGARBLE=test/main
 
 garble build
 exec ./main

testdata/scripts/imports.txt

@@ -1,5 +1,5 @@
 # Note that this is the only test with a module where we rely on the detection
-# of GOPRIVATE.
+# of GOGARBLE.
 # Also note that, since this is the only test using "real" external modules
 # fetched via GOPROXY, go.mod and go.sum should declare the dependencies.
 

testdata/scripts/ldflags.txt

@@ -1,5 +1,5 @@
 # Note the proper domain, since the dot adds an edge case.
-env GOPRIVATE=domain.test/main
+env GOGARBLE=domain.test/main
 
 env LDFLAGS='-X=main.unexportedVersion=v1.0.0 -X=domain.test/main/imported.ExportedVar=replaced -X=domain.test/missing/path.missingVar=value'
 

testdata/scripts/linkname.txt

@@ -1,4 +1,4 @@
-env GOPRIVATE=test/main,big.chungus/meme
+env GOGARBLE=test/main,big.chungus/meme
 
 garble build
 exec ./main