Commit Graph

29 Commits (b3db7d6fa721dd6f7d1755ff7b1757d78d7a25c8)

Author SHA1 Message Date
Daniel Martí ff0bea73b5
all: drop support for Go 1.15.x (#265)
This mainly cleans up the few bits of code where we explicitly kept
support for Go 1.15.x. With v0.1.0 released, we can drop support now,
since the next v0.2.0 release will only support Go 1.16.x.

Also updates all modules, including test ones, to 'go 1.16'.

Note that the TOOLEXEC_IMPORTPATH refactor is not done here, despite all
the TODOs about doing so when we drop 1.15 support. This is because that
refactor needs to be done carefully and might have side effects, so it's
best to keep it to a separate commit.

Finally, update the deps.
3 years ago
Daniel Martí 180d64236a
properly fix obfuscated imports with their package names (#245)
The TODO I left there didn't take long to surface as a bug. If the
package path ends with a word containing a hyphen, that's not a valid
identifier, so we end up with invalid Go syntax.

Add that test case, as well as one where an import was already named.

To fix the issue, we just need to use the package name we got from
'go list -json'.

Fixes #243.
3 years ago
Daniel Martí 05d0dd1801
reimplement import path obfuscation without goobj2 (#242)
We used to rely on a parallel implementation of an object file parser
and writer to be able to obfuscate import paths. After compiling each
package, we would parse the object file, replace the import paths, and
write the updated object file in-place.

That worked well, in most cases. Unfortunately, it had some flaws:

* Complexity. Even when most of the code is maintained in a separate
  module, the import_obfuscation.go file was still close to a thousand
  lines of code.

* Go compatibility. The object file format changes between Go releases,
  so we were supporting Go 1.15, but not 1.16. Fixing the object file
  package to work with 1.16 would probably break 1.15 support.

* Bugs. For example, we recently had to add a workaround for #224, since
  import paths containing dots after the domain would end up escaped.
  Another example is #190, which seems to be caused by the object file
  parser or writer corrupting the compiled code and causing segfaults in
  some rare edge cases.

Instead, let's drop that method entirely, and force the compiler and
linker to do the work for us. The steps necessary when compiling a
package to obfuscate are:

1) Replace its "package foo" lines with the obfuscated package path. No
   need to separate the package path and name, since the obfuscated path
   does not contain slashes.

2) Replace the "-p pkg/foo" flag with the obfuscated path.

3) Replace the "import" spec lines with the obfuscated package paths,
   for those dependencies which were obfuscated.

4) Replace the "-importcfg [...]" file with a version that uses the
   obfuscated paths instead.

The linker also needs that last step, since it also uses an importcfg
file to find object files.

There are three noteworthy drawbacks to this new method:

1) Since we no longer write object files, we can't use them to store
   data to be cached. As such, the -debugdir flag goes back to using the
   "-a" build flag to always rebuild all packages. On the plus side,
   that caching didn't work very well; see #176.

2) The package name "main" remains in all declarations under it, not
   just "func main", since we can only rename entire packages. This
   seems fine, as it gives little information to the end user.

3) The -tiny mode no longer sets all lines to 0, since it did that by
   modifying object files. As a temporary measure, we instead set all
   top-level declarations to be on line 1. A TODO is added to hopefully
   improve this again in the near future.

The upside is that we get rid of all the issues mentioned before. Plus,
garble now nearly works with Go 1.16, with the exception of two very
minor bugs that look fixable. A follow-up PR will take care of that and
start testing on 1.16.

Fixes #176.
Fixes #190.
3 years ago
Daniel Martí af517a20f8 make the handling of import paths more robust
First, make isPrivate panic on malformed import paths, since that should
never happen. This catches the errors that some users had run into with
packages like gopkg.in/yaml.v2 and github.com/satori/go.uuid:

	panic: malformed import path "gopkg.in/garbletest%2ev2": invalid char '%'

This seems to trigger when a module path contains a dot after the first
element, *and* that module is fetched via the proxy. This results in the
toolchain URL-encoding the second dot, and garble ends up seeing that
encoded path.

We reproduce this behavior with a fake gopkg.in module added to the test
module proxy. Using yaml.v2 directly would have been easier, but it's
pretty large. Note that we tried a replace directive, but that does not
trigger the URL-encoding bug.

Also note that we do not obfuscate the gopkg.in package; that's fine, as
the isPrivate path validity check catches the bug either way.

For now, make initImport use url.PathUnescape to work around this issue.
The underlying bug is likely in either the goobj2 fork, or in the
upstream Go toolchain itself.

hashImport also gives a better error if it cannot find a package now,
rather than just an "empty seed" panic.

Finally, the sanity check in isPrivate unearthed the fact that we do not
support garbling test packages at all, since they were invalid paths
which never matched GOPRIVATE. Add an explicit check and TODO about
that.

Fixes #224.
Fixes #228.
3 years ago
Daniel Martí 249501b5e9
fix garbling names belonging to indirect imports (#203)
main.go includes a lengthy comment that documents this edge case, why it
happened, and how we are fixing it. To summarize, we should no longer
error with a build error in those cases. Read the comment for details.

A few other minor changes were done to allow writing this patch.

First, the actionID and contentID funcs were renamed, since they started
to collide with variable names.

Second, the logging has been improved a bit, which allowed me to debug
the issue.

Third, the "cache" global shared by all garble sub-processes now
includes the necessary parameters to run "go list -toolexec", including
the path to garble and the build flags being used.

Thanks to lu4p for writing a test case, which also applied gofmt to that
testdata Go file.

Fixes #180.
Closes #181, since it includes its test case.
4 years ago
Daniel Martí c0731921c2
rewrite go:linkname directives with garbled names (#200)
If code includes a linkname directive pointing at a name in an imported
package, like:

	//go:linkname localName importedpackage.RemoteName
	func localName()

We should rewrite the comment to replace "RemoteName" with its
obfuscated counterpart, if the package in question was obfuscated and
that name was as well.

We already had some code to handle linkname directives, but only to
ensure that "localName" was never obfuscated. This behavior is kept, to
ensure that the directive applies to the right name. In the future, we
could instead rewrite "localName" in the directive, like we do with
"RemoteName".

Add plenty of tests, too. The linkname directive used to be tested in
imports.txt and syntax.txt, but that was hard to maintain as each file
tested different edge cases.

Now that we have build caching, adding one extra testscript file isn't a
big problem anymoree. Add linkname.txt, which is self-explanatory. The
other two scripts also get a bit less complex.

Fixes #197.
4 years ago
Daniel Martí 4f65e6f99c
testdata: avoid messing with the host's mod cache (#198)
It turns out that the modules we include in testdata/mod via
txtar-addmod don't result in the same h1 hash that one gets when using
proxy.golang.org.

As proof:

	$ go clean -modcache && go get -d rsc.io/quote@v1.5.2 && go test -short
	[...]
	--- FAIL: TestScripts/imports (0.06s)
		> garble build -tags buildtag
		go list error: exit status 1: verifying rsc.io/quote@v1.5.2: checksum mismatch
		    downloaded: h1:w5fcysjrx7yqtD/aO+QwRjYZOKnaM9Uh2b40tElTs3Y=
		    go.sum:     h1:3fEykkD9k7lYzXqCYrwGAf7iNhbk4yCjHmKBN9td4L0=

The added comment explains the situation in detail.

For now, simply work around the issue by not sharing GOMODCACHE with the
host.
4 years ago
Daniel Martí 39372a8c9b testdata: don't let tests rely on rewriting mod files
In Go 1.15, if a dependency is required but not listed in go.mod/go.sum,
it's resolved and added automatically.

This is changing in 1.16. From that release, one will have to explicitly
update the mod files via 'go mod tidy' or 'go get'.

To get ahead of the curve, start using -mod=readonly to get the same
behavior in 1.15, and fix all existing tests.

The only tests that failed were imports.txt and syntax.txt, the only
ones to require other modules. But since we're here, let's add the 'go'
line to all go.mod files as well.
4 years ago
Andrew LeFevre 1fc990dcf8
Fix bug where structs would get garbled in some packages but not in others (#161)
* fix bug where structs would get garbled in some packages but not in others

* only check if struct/field was not defined in current package

* fix a related bug when two objects share the same name in the same package and one is garbled but the other one is not

* renamed parameter for clarity
4 years ago
Andrew LeFevre 0e0a9fc594
Allow struct fields to be garbled, fixes #48 (#159)
* Allow struct fields to be garbled, fixes #48

* fix syntax test script

* simplified code according to review
4 years ago
Daniel Martí 2a0ac434fb
initial support for build caching (#142)
As per the discussion in https://github.com/golang/go/issues/41145, it
turns out that we don't need special support for build caching in
-toolexec. We can simply modify the behavior of "[...]/compile -V=full"
and "[...]/link -V=full" so that they include garble's own version and
options in the printed build ID.

The part of the build ID that matters is the last, since it's the
"content ID" which is used to work out whether there is a need to redo
the action (build) or not. Since cmd/go parses the last word in the
output as "buildID=...", we simply add "+garble buildID=_/_/_/${hash}".
The slashes let us imitate a full binary build ID, but we assume that
the other components such as the action ID are not necessary, since the
only reader here is cmd/go and it only consumes the content ID.

The reported content ID includes the tool's original content ID,
garble's own content ID from the built binary, and the garble options
which modify how we obfuscate code. If any of the three changes, we
should use a different build cache key. GOPRIVATE also affects caching,
since a different GOPRIVATE value means that we might have to garble a
different set of packages.

Include tests, which mainly check that 'garble build -v' prints package
lines when we expect to always need to rebuild packages, and that it
prints nothing when we should be reusing the build cache even when the
built binary is missing.

After this change, 'go test' on Go 1.15.2 stabilizes at about 8s on my
machine, whereas it used to be at around 25s before.
4 years ago
Andrew LeFevre c8d61c772f
Garble imports and package paths in GOPRIVATE (#116)
Finally, finally this is done. This allows import paths to be obfuscated by modifying
object/archive files and garbling import paths contained within. The bulk of the
code that makes parsing and writing Go object/archive files possible lives at
https://github.com/Binject/debug/tree/master/goobj2, which I wrote as well.

I have tested by garbling and checking for import paths via strings and grep
(in order of difficulty) https://github.com/lu4p/binclude, garble itself, and
https://github.com/dominikh/go-tools/tree/master/cmd/staticcheck.

This only supports object/archive files produced from the Go 1.15 compiler.
The object file format changed at 1.15, and 1.14 and earlier is not supported.

Fixes #13.
4 years ago
Daniel Martí 511779d8ff testdata: set GOPRIVATE in all but two tests (#104)
basic.txt just builds main.go without a module. Similarly, we leave
imports.txt without a GOPRIVATE, to test the 'go list -m' fallback.

For all other tests, explicitly set GOPRIVATE, to avoid two exec calls -
both 'go env GOPRIVATE' as well as 'go list -m'. Each of those calls
takes in the order of 10ms, so saving ~26 exec calls should easily add
to 200-300ms saved from 'go test -short'.
4 years ago
lu4p 7df14ad860 Fix reflect detection if -literals is passed.
Fixes #93.

The second typecheck lead to the creation of different type objects,
which didn't match the types in the blacklist anymore.

It turns out we don't need the second typecheck,
therfore it is now removed.
4 years ago
Daniel Martí d0e01478f0 keep build flags when calling 'go list'
Otherwise any build flags like -tags won't be used, and we might easily
end up with errors or incorrect packages.

The common case with -tags is covered by one of the integration test
scripts. On top of that, we add a table-driven unit test to cover all
edge cases, since there are many we can do quickly in a unit test.

Fixes #82.
4 years ago
Daniel Martí 80538f19c7 blacklist struct fields with reflection too
In the added test, the unexported field used to be garbled.

Reflection can only reach exported methods, exported fields, and
unexported fields. Exported methods and fields are currently never
garbled, so unexported fields was the only missing piece.
4 years ago
Daniel Martí 4bc64ef8fb make detection of reflect more robust
It now works with variables and composite type expressions too.
4 years ago
lu4p 8b898ad0d2
exclude identifiers used via reflection
If reflect.TypeOf or reflect.ValueOf are used on a type declared in the same package,
don't garble that type name or any of its fields.

Fixes #15.
4 years ago
Daniel Martí 19e4c098cd make selection of packages configurable via GOPRIVATE
Carefully select a default that will do the right thing when inside a
module, as well as when building ad-hoc packages.

This means we no longer need to look at the compiler's -std flag, which
is nice.

Also replace foo.com/ with test/, as per golang/go#37641.

Fixes #7.
4 years ago
Daniel Martí 5aaa086e5d don't remove "//go:" compile directives
For example, this broke cgo, since it uses go:linkname.

Updates #12.
4 years ago
Daniel Martí d72c00eafd support building modules which require other modules
We use 'go list -json -export' to locate required modules. This works
fine to locate direct module dependencies; since we're building in the
current module, we run 'go list' in the correct directory.

However, if we're building one of those module dependencies, and it has
other module dependencies of its own, we would fail with cryptic errors
like:

	typecheck error: [...] go list error: updates to go.sum needed, disabled by -mod=readonly

This is because we would try to run 'go list' outside of the main
module, probably inside the module cache. Instead, use a $GARBLE_DIR env
var from the top-level 'garble build' call to always run 'go list' in
the original directory.

We add a few small modules to properly test this.

Updates #9.
4 years ago
Daniel Martí 302cc137b6 test that filenames are garbled
We've done this for a while, but we weren't testing it.

Fixes #1.
4 years ago
Daniel Martí 308e984293 don't use regexes when searching binaries for strings
This is a bit simpler, and saves us a small amount of CPU work in the
tests.
4 years ago
Daniel Martí 30524ea282 shorten 'go test -short' run time
Down from ~11s to ~7s, as we can skip some extra checks.

While at it, avoid duplicate 'go test' builds in test.txt.
5 years ago
Daniel Martí 5556be7402 make the tool work on Windows, enable tests
The tests required a few last tweaks to work on Windows.
5 years ago
Daniel Martí 0058dfc12a make output binaries deterministic
We were leaking temporary file paths, which is no longer the case.
5 years ago
Daniel Martí e08dd99c1e introduce a binary grep command for the tests
The problem with the "grep" built-in command is that it prints the
entire data if there is an error. We don't want megabytes of binary
output for a test.
5 years ago
Daniel Martí 766bb47b82 support std imports 5 years ago
Daniel Martí 1fe0351517 garbling imported packages starts being supported 5 years ago