Skip to content

Commit

Permalink
warn when a public package imports a private package (#182)
Browse files Browse the repository at this point in the history
That is, a package that is built without obfuscation imports an
obfuscated package. This will result in confusing compilation error
messages, because the importer can't find the exported names from the
imported package by their non-obfuscated names:

	> ! garble build ./importer
	[stderr]
	# test/main/importer
	importer/importer.go:5:9: undefined: imported.Name
	exit status 2

Instead, detect this bad input case and provide a nice error:

	public package "test/main/importer" can't depend on obfuscated package "test/main/imported" (matched via GOPRIVATE="test/main/imported")

For now, this is by design. It also makes little sense for a public
package to import an obfuscated package in general, because the public
package would have to leak details about the private package's API and
behavior.

While at it, fix a quirk where we thought the unsafe package could be
private. It can't be, because the runtime package is always public and
it imports the runtime package:

	public package "internal/bytealg" can't depend on obfuscated package "unsafe" (matched via GOPRIVATE="*")

Instead of trying to obfuscate "unsafe" and doing nothing, simply add it
to the neverPrivate list, which is also a better name than
"privateBlacklist" (for #169).

Fixes #164.

Co-authored-by: lu4p <lu4p@pm.me>
  • Loading branch information
mvdan and lu4p authored Nov 5, 2020
1 parent 523cc44 commit 4e79bfc
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
31 changes: 22 additions & 9 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ type listedPackage struct {
ImportPath string
Export string
Deps []string

// TODO(mvdan): reuse this field once TOOLEXEC_IMPORTPATH is used
private bool
}

func listPackage(path string) (*listedPackage, error) {
Expand Down Expand Up @@ -412,14 +415,25 @@ func mainErr(args []string) error {
return err
}
anyPrivate := false
for path := range listedPackages {
for path, pkg := range listedPackages {
if isPrivate(path) {
pkg.private = true
anyPrivate = true
break
}
}
if !anyPrivate {
return fmt.Errorf("GOPRIVATE %q does not match any packages to be built", envGoPrivate)
return fmt.Errorf("GOPRIVATE=%q does not match any packages to be built", envGoPrivate)
}
for path, pkg := range listedPackages {
if pkg.private {
continue
}
for _, depPath := range pkg.Deps {
if listedPackages[depPath].private {
return fmt.Errorf("public package %q can't depend on obfuscated package %q (matched via GOPRIVATE=%q)",
path, depPath, envGoPrivate)
}
}
}

execPath, err := os.Executable()
Expand Down Expand Up @@ -631,10 +645,6 @@ func transformCompile(args []string) ([]string, error) {
tf.existingNames = collectNames(files)
tf.buildBlacklist(files)

// unsafe.Pointer is a special type that doesn't exist as a plain Go
// type definition, so we can't change its name.
tf.blacklist[types.Unsafe.Scope().Lookup("Pointer")] = struct{}{}

if envGarbleLiterals {
// TODO: use transformer here?
files = literals.Obfuscate(files, tf.info, fset, tf.blacklist)
Expand Down Expand Up @@ -792,14 +802,17 @@ func transformCompile(args []string) ([]string, error) {
return append(flags, newPaths...), nil
}

const privateBlacklist = "runtime,internal/cpu,internal/bytealg"
// neverPrivate is a core set of std packages which we cannot obfuscate.
// At the moment, this is the runtime package and its (few) dependencies.
// This might change in the future.
const neverPrivate = "runtime,internal/cpu,internal/bytealg,unsafe"

// isPrivate checks if GOPRIVATE matches path.
//
// To allow using garble without GOPRIVATE for standalone main packages, it will
// default to not matching standard library packages.
func isPrivate(path string) bool {
if module.MatchPrefixPatterns(privateBlacklist, path) {
if module.MatchPrefixPatterns(neverPrivate, path) {
return false
}
if path == "main" || path == "command-line-arguments" || strings.HasPrefix(path, "plugin/unnamed") {
Expand Down
20 changes: 17 additions & 3 deletions testdata/scripts/goprivate.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
env GOPRIVATE=match-absolutely/nothing
! garble build -o bin ./standalone
stderr 'does not match any packages'
! garble build -o=out ./standalone
stderr '^GOPRIVATE="match-absolutely/nothing" does not match any packages to be built$'

env GOPRIVATE=test/main/imported
! garble build ./importer
stderr '^public package "test/main/importer" can''t depend on obfuscated package "test/main/imported" \(matched via GOPRIVATE="test/main/imported"\)$'

[short] stop

env GOPRIVATE='*'
garble build -o bin ./standalone
garble build -o=out ./standalone

-- go.mod --
module test/main
-- standalone/main.go --
package main

func main() {}
-- importer/importer.go --
package importer

import "test/main/imported"

var _ = imported.Name
-- imported/imported.go --
package imported

var Name = "value"

0 comments on commit 4e79bfc

Please sign in to comment.