From bfecb3a7cd270c76bd5e02a5cd8b85c2e7a691c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 3 Dec 2021 15:45:33 +0000 Subject: [PATCH] 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 #276. --- README.md | 8 ++- hash.go | 6 +- main.go | 56 ++++++++++--------- position.go | 4 +- reverse.go | 2 +- shared.go | 25 +++++---- testdata/scripts/asm.txt | 2 +- testdata/scripts/basic.txt | 2 +- testdata/scripts/cgo.txt | 6 +- testdata/scripts/crossbuild.txt | 2 +- testdata/scripts/debugdir.txt | 2 +- testdata/scripts/embed.txt | 2 +- .../scripts/{goprivate.txt => gogarble.txt} | 14 +++-- testdata/scripts/implement.txt | 2 +- testdata/scripts/imports.txt | 2 +- testdata/scripts/ldflags.txt | 2 +- testdata/scripts/linkname.txt | 2 +- testdata/scripts/literals.txt | 4 +- testdata/scripts/modinfo.txt | 2 +- testdata/scripts/plugin.txt | 2 +- testdata/scripts/position.txt | 2 +- testdata/scripts/reflect.txt | 2 +- testdata/scripts/reverse.txt | 2 +- testdata/scripts/seed.txt | 2 +- testdata/scripts/syntax.txt | 2 +- testdata/scripts/tiny.txt | 2 +- 26 files changed, 88 insertions(+), 71 deletions(-) rename testdata/scripts/{goprivate.txt => gogarble.txt} (79%) diff --git a/README.md b/README.md index 366ba400..c6990628 100644 --- a/README.md +++ b/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 diff --git a/hash.go b/hash.go index f4356e26..2e29c225 100644 --- a/hash.go +++ b/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") diff --git a/main.go b/main.go index 624e087e..0e3443d2 100644 --- a/main.go +++ b/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" } } } diff --git a/position.go b/position.go index d13048b2..06da04c6 100644 --- a/position.go +++ b/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 } diff --git a/reverse.go b/reverse.go index e9926d97..b1feab5a 100644 --- a/reverse.go +++ b/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 diff --git a/shared.go b/shared.go index 3bccd388..e21f1ce3 100644 --- a/shared.go +++ b/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 diff --git a/testdata/scripts/asm.txt b/testdata/scripts/asm.txt index 3584a199..33efd5a3 100644 --- a/testdata/scripts/asm.txt +++ b/testdata/scripts/asm.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main garble build exec ./main diff --git a/testdata/scripts/basic.txt b/testdata/scripts/basic.txt index 32062ebd..9b4e7b22 100644 --- a/testdata/scripts/basic.txt +++ b/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 diff --git a/testdata/scripts/cgo.txt b/testdata/scripts/cgo.txt index 00b5bde5..e14c9698 100644 --- a/testdata/scripts/cgo.txt +++ b/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 diff --git a/testdata/scripts/crossbuild.txt b/testdata/scripts/crossbuild.txt index 1ad0b143..68ae66dd 100644 --- a/testdata/scripts/crossbuild.txt +++ b/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. diff --git a/testdata/scripts/debugdir.txt b/testdata/scripts/debugdir.txt index e21758d8..98d82df6 100644 --- a/testdata/scripts/debugdir.txt +++ b/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' diff --git a/testdata/scripts/embed.txt b/testdata/scripts/embed.txt index 2c885f82..71450a3f 100644 --- a/testdata/scripts/embed.txt +++ b/testdata/scripts/embed.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=* +env GOGARBLE=* garble build diff --git a/testdata/scripts/goprivate.txt b/testdata/scripts/gogarble.txt similarity index 79% rename from testdata/scripts/goprivate.txt rename to testdata/scripts/gogarble.txt index c3ac592c..6acdbf53 100644 --- a/testdata/scripts/goprivate.txt +++ b/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 diff --git a/testdata/scripts/implement.txt b/testdata/scripts/implement.txt index 1695600a..45df43cc 100644 --- a/testdata/scripts/implement.txt +++ b/testdata/scripts/implement.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main garble build exec ./main diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index 90e614a9..e21051ac 100644 --- a/testdata/scripts/imports.txt +++ b/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. diff --git a/testdata/scripts/ldflags.txt b/testdata/scripts/ldflags.txt index bc2b3470..8ee057ee 100644 --- a/testdata/scripts/ldflags.txt +++ b/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' diff --git a/testdata/scripts/linkname.txt b/testdata/scripts/linkname.txt index abe5ff0b..a0b550ff 100644 --- a/testdata/scripts/linkname.txt +++ b/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 diff --git a/testdata/scripts/literals.txt b/testdata/scripts/literals.txt index cb7f6024..bd484689 100644 --- a/testdata/scripts/literals.txt +++ b/testdata/scripts/literals.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main garble -literals build exec ./main$exe @@ -51,7 +51,7 @@ grep '^\s+type \w+ func\(byte\) \w+$' debug1/test/main/extra_literals.go # Finally, sanity check that we can build all of std with -literals. # Analogous to goprivate.txt. -env GOPRIVATE='*' +env GOGARBLE='*' garble -literals build std -- go.mod -- diff --git a/testdata/scripts/modinfo.txt b/testdata/scripts/modinfo.txt index 3a432fcb..e5f0f1ef 100644 --- a/testdata/scripts/modinfo.txt +++ b/testdata/scripts/modinfo.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main [exec:git] exec git init -q [exec:git] exec git config user.name "name" diff --git a/testdata/scripts/plugin.txt b/testdata/scripts/plugin.txt index a41ca204..c7bbdd25 100644 --- a/testdata/scripts/plugin.txt +++ b/testdata/scripts/plugin.txt @@ -2,7 +2,7 @@ skip # TODO: get plugins working properly. See issue #87 [windows] skip 'Go plugins are not supported on Windows' -env GOPRIVATE=test/main +env GOGARBLE=test/main garble build -buildmode=plugin ./plugin binsubstr plugin.so 'PublicVar' 'PublicFunc' diff --git a/testdata/scripts/position.txt b/testdata/scripts/position.txt index 571ce272..492a86f0 100644 --- a/testdata/scripts/position.txt +++ b/testdata/scripts/position.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main garble build exec ./main diff --git a/testdata/scripts/reflect.txt b/testdata/scripts/reflect.txt index 77255b6e..805c3f6a 100644 --- a/testdata/scripts/reflect.txt +++ b/testdata/scripts/reflect.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main garble build exec ./main diff --git a/testdata/scripts/reverse.txt b/testdata/scripts/reverse.txt index 4040f839..6e34ee3e 100644 --- a/testdata/scripts/reverse.txt +++ b/testdata/scripts/reverse.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main # Unknown build flags should result in errors. ! garble reverse -badflag=foo . diff --git a/testdata/scripts/seed.txt b/testdata/scripts/seed.txt index 363ab88c..3a3c1a36 100644 --- a/testdata/scripts/seed.txt +++ b/testdata/scripts/seed.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main env SEED1=OQg9kACEECQ env SEED2=NruiDmVz6/s diff --git a/testdata/scripts/syntax.txt b/testdata/scripts/syntax.txt index 74c1da34..32f35807 100644 --- a/testdata/scripts/syntax.txt +++ b/testdata/scripts/syntax.txt @@ -1,4 +1,4 @@ -env GOPRIVATE='test/main,private.source' +env GOGARBLE='test/main,private.source' garble build exec ./main$exe diff --git a/testdata/scripts/tiny.txt b/testdata/scripts/tiny.txt index f017cbc3..34b0c418 100644 --- a/testdata/scripts/tiny.txt +++ b/testdata/scripts/tiny.txt @@ -1,4 +1,4 @@ -env GOPRIVATE=test/main +env GOGARBLE=test/main # Tiny mode garble -tiny build