This was pretty much just fixing the README and closing the issue. The
only other noteworthy user-facing change is that, if the Go version is
detected to be too old, we now suggest 1.16.x instead of 1.15.x.
While at it, refactor goversion.txt a bit. I wanted it to print a
clearer "mocking the go build" error if another command was used like
"go build", but I didn't want to learn BAT. So, instead use a simple Go
program and build it, which will work on all platforms. The added
"go build" step barely takes 100ms on my machine, given how simple the
program is.
The [short] line also doesn't seem necessary to me. The entire script
runs in under 200ms for me, so it's well within the realm of "short", at
least compared to many of the other test scripts.
Fixes#124.
The asm tool runs twice for a package with assembly. The second time it
does, the path given to the -p flag matters, just like in the compiler,
as we generate an object file.
We don't have a -buildid flag in the asm tool, so obtaining the action
ID to obfuscate the package path with is a bit tricky. We store it from
transformCompile, and read it from transformAsm. See the detailed docs
for more.
This was the last "skip" line in the tests due to Go 1.16. After all PRs
are merged, one last PR documenting that 1.16 is supported will be sent,
closing the issue for good.
It's unclear why this wasn't an issue in Go 1.15. My best guess is that
the ABI changes only happened in Go 1.16, and this causes exported asm
funcs to start showing up in object files with their package paths.
Updates #124.
With a few extra lines, we can keep Go 1.15 support in the table too.
Re-enables the goprivate.txt test for Go 1.16.
While at it, make the script's use of grep a bit simpler with -E, which
also uses the same syntax as Go's regexp. Its skip logic was also buggy,
resulting in the macos results always being empty.
Updates #124.
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.
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.
First, remove the shuffling of the declarations list within each file.
This is what we used at the very start to shuffle positions. Ever since
we started obfuscating positions via //line comments, that has been
entirely unnecessary.
Second, add a proper test that will fail if we don't obfuscate line
numbers well enough. Filenames were already decently covered by other
tests.
Third, simplify the line obfuscation code. It does not require
astutil.Apply, and ranging over file.Decls is easier.
Finally, also obfuscate the position of top-level vars, since we only
used to do it for top-level funcs. Without that fix, the test would fail
as varLines was unexpectedly sorted.
There was one bug keeping the command below from working:
GOPRIVATE='*' garble build std
The bug is rather obscure; I'm still working on a minimal reproducer
that I can submit upstream, and I'm not yet convinced about where the
bug lives and how it can be fixed.
In short, the command would fail with:
typecheck error: /go/src/crypto/ecdsa/ecdsa.go:122:12: cannot use asn1.SEQUENCE (constant 48 of type asn1.Tag) as asn1.Tag value in argument to b.AddASN1
Note that the error is ambiguous; there are two asn1 packages, but they
are actually mismatching. We can see that by manually adding debug
prints to go/types:
constant: asn1.SEQUENCE (constant 48 of type golang.org/x/crypto/cryptobyte/asn1.Tag)
argument type: vendor/golang.org/x/crypto/cryptobyte/asn1.Tag
It's clear that, for some reason, go/types ends up confused and loading
a vendored and non-vendored version of asn1. There also seems to be no
way to work around this with our lookup function, as it just receives an
import path as a parameter, and returns an object file reader.
For now, work around the issue by *not* using a custom lookup function
in this rare edge case involving vendored dependencies in std packages.
The added code has a lengthy comment explaining the reasoning.
I still intend to investigate this further, but there's no reason to
keep garble failing if we can work around the bug.
Fixes#223.
The added test case used to crash garble:
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x8fe71b]
goroutine 1 [running]:
golang.org/x/tools/go/ast/astutil.Apply.func1(0xc0001e8880, 0xc000221570)
/go/pkg/mod/golang.org/x/tools@v0.0.0-20210115202250-e0d201561e39/go/ast/astutil/rewrite.go:47 +0x97
panic(0x975bc0, 0xd6c610)
/sdk/go1.15.8/src/runtime/panic.go:969 +0x1b9
go/types.(*Named).Obj(...)
/sdk/go1.15.8/src/go/types/type.go:473
mvdan.cc/garble.isTestSignature(0xc0001e7080, 0xa02e84)
/src/garble/main.go:1170 +0x7b
mvdan.cc/garble.(*transformer).transformGo.func2(0xc000122df0, 0xaac301)
/src/garble/main.go:1028 +0xff1
We were assuming that the first parameter was a named type, but that
might not be the case.
This crash was found out in the wild, from which a minimal repro was
written. We add two variants of it to the test data, just in case.
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.
In 90fa325da7, the obfuscation logic was changed to use hashes for
exported names, but incremental names starting at just one letter for
unexported names. Presumably, this was done for the sake of binary size.
I argue that this is not a good idea for the default mode for a number
of reasons:
1) It makes reversing of stack traces nearly impossible for unexported
names, since replacing an obfuscated name "c" with "originalName"
would trigger too many false positives by matching single characters.
2) Exported and unexported names aren't different. We need to know how
names were obfuscated at a later time in both cases, thanks to use
cases like -ldflags=-X. Using short names for one but not the other
doesn't make a lot of sense, and makes the logic inconsistent.
3) Shaving off three bytes for unexported names doesn't seem like a huge
deal for the default mode, when we already have -tiny to optimize for
size.
This saves us a bit of work, but most importantly, simplifies the
obfuscation state as we no longer need to carry privateNameMap between
the compile and link stages.
name old time/op new time/op delta
Build-8 153ms ± 2% 150ms ± 2% ~ (p=0.065 n=6+6)
name old bin-B new bin-B delta
Build-8 7.09M ± 0% 7.08M ± 0% -0.24% (p=0.002 n=6+6)
name old sys-time/op new sys-time/op delta
Build-8 296ms ± 5% 277ms ± 6% -6.50% (p=0.026 n=6+6)
name old user-time/op new user-time/op delta
Build-8 562ms ± 1% 558ms ± 3% ~ (p=0.329 n=5+6)
Note that I do not oppose using short names for both exported and
unexported names in the future for -tiny, since reversing of stack
traces will by design not work there. The code can be resurrected from
the git history if we want to improve -tiny that way in the future, as
we'd need to store state in header files again.
Another major cleanup we can do here is to no longer use the
garbledImports map. From a look at obfuscateImports, we hash a package's
import path with its action ID, much like exported names, so we can
simply re-do that hashing for the linker's -X flag.
garbledImports does have some logic to handle duplicate package names,
but it's worth noting that should not affect package paths, as they are
always unique. That area of code could probably do with some
simplification in the future, too.
While at it, make hashWith panic if either parameter is empty.
obfuscateImports was hashing the main package path without a salt due to
a bug, so we want to catch those in the future.
Finally, make some tiny spacing and typo tweaks to the README.
For now, this only implements reversing of exported names which are
hashed with action IDs. Many other kinds of obfuscation, like positions
and private names, are not yet implemented.
Note that we don't document this new command yet on purpose, since it's
not finished.
Some other minor cleanups were done for future changes, such as making
transformLineInfo into a method that also receives the original
filename, and making header names more self-describing.
Updates #5.
Each compile and link sub-process created its own temporary directory,
to be cleaned up shortly after. Moreover, we also had the global
gob-encoded temporary file.
Instead, place all of those under a single, start-to-end temporary
directory. This is cleaner for the end user, and easier to maintain for
us.
A big plus is that we can also get rid of the confusing deferred global,
as it was mostly used to clean up these extra temp dirs. The only
remaining use was post-compile code, which is now an explicit func
returned by each "transform" func.
While at it, clean up the math/rand seeding code a bit and add a debug
log line, and stop shadowing a cmd string with a cmd *exec.Cmd.
Fixes#147.
Mimicking "go version", this tells the user garble's own version.
The code is exactly the same that is used for another tool written in
Go, shfmt. It uses runtime/debug to fetch the module version embedded in
binaries built by Go. For example:
$ go get mvdan.cc/sh/v3/cmd/shfmt@latest
$ shfmt -version
v3.2.2
$ go get mvdan.cc/sh/v3/cmd/shfmt@master
$ shfmt -version
v3.3.0-0.dev.0.20210203135509-56c9918c980d
Note that this will not work for a plain "go build" or "go install"
after a "git clone", since in that case the Go tool can't know garble's
own version via go.mod - since it's the current main module:
$ go build
$ ./garble version
(devel)
For the use case of the power user building from source directly, they
are probably clever enough to tell us what git commit they are on, so
this is not a big problem right now. It will also get better once
golang/go#37475 is fixed in the future.
Until then, if we need to do "release" builds locally, we can embed an
explicit version into the binary via ldflags:
$ go build -ldflags=-X=main.version=v1.2.3
$ ./garble version
v1.2.3
Fixes#217.
The three transformer map fields are now very well documented, which was
badly needed for anyone trying to understand the source code.
ignoreObjects is also a better field name than blacklist, as it says
what the map is indexed by (types.Object) and what we do with those:
ignore them when we obfuscate code.
The rewriting of go:linkname directives is moved to a separate func, so
that we can name that func from the docs.
Finally, the docs are overall improved a bit, as I was re-tracing all
the pieces of code that used the ambiguous "blacklist" terminology.
Fixes#169.
The point of main1 returning an int is that testscript can run code
afterwards, such as to collect coverage information when running with
-coverprofile.
We were using plain os.Exit in a couple of places: when help was
requested, and when the Go version could not be fetched.
In those cases, return an error to main1, and let it do the right thing.
For #35.
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.
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.
First, we don't need the nameSpecialDirectives list as a separate thing.
cgo types aren't obfuscated anymore, so the only item in that list that
made a difference in the tests was go:linkname, which we'll overhaul
soon. For now, keep its code around.
Second, processDetachedDirectives can be replaced by just seven lines.
Third, we don't need to separate build tag directives from the rest of
the detached directives. Their relative order (with other comments) does
not matater.
Fourth and last, ranging over a nil slice is a no-op, so a nil check
around a slice range is unnecessary.
This is some prep work to make the patch to support go:linkname smaller
and easier to review.
Previously, we were never obfuscating runtime and its direct
dependencies. Unfortunately, due to linkname, the runtime package is
actually closely related to dozens of other std packages as well.
Until we can obfuscate the runtime and properly support go:linkname
directives, obfuscating fewer std packages is a better outcome than
breaking and not producing any obfuscated code at all.
The added test case is building runtime/pprof, which used to cause
failures:
# runtime/pprof
/go/src/runtime/pprof/label.go:27:21: undefined: context.Context
/go/src/runtime/pprof/label.go:59:21: undefined: context.Context
/go/src/runtime/pprof/label.go:93:16: undefined: context.Context
/go/src/runtime/pprof/label.go:101:20: undefined: context.Context
The net package was also very close to obfuscating properly thanks to
this change, so its test is now run as well. The only other remaining
fix was to not obfuscate fields on cgo types, since those aren't
obfuscated at the moment.
The map is pretty long, but it's only a temporary solution and the
command to obtain the list again is included. Never obfuscating the
entire std library is also an option, but it's a bit unnecessary.
Fixes#134.
Previously garble heavily used env vars to share data between processes.
This also makes it easy to share complex data between processes.
The complexity of main.go is considerably reduced.
The previous globals worked, but were unnecessarily complex. For
example, we passed the fromPath variable around, but it's really a
static global, since we only compile or link a single package in each Go
process. Use such global variables instead of passing them around, which
currently include the package's import path, its build ID, and its
import config path.
Also split all the hashing and build ID code into hash.go, since that's
a relatively well contained 200 lines of code that doesn't need to make
main.go any bigger. We also split the code to alter Go's own version to
a separate function, so that it can be moved out of main.go as well.
* Use latest Binject/debug version to support importmap directives in the importcfg file
* Uncomment line in goprivate testscript to test ImportMap
* Fixed issue where a package in specified in importmap would be hashed differently in a package that imported it, due to the mapping of import paths.
Also commented out the 'net' import in the goprivate testscript (again) due to cgo compile errors
We also update the "original types importer" to support ImportMap.
The test now gets further along, no longer getting stuck on "path not
found in listed packages". Instead, we get stuck on:
error parsing importcfg: <...>/importcfg:2: unknown directive "importmap"
This bug has been filed at https://github.com/Binject/debug/issues/17.
Until it's fixed, we can't really proceed on #146, so the net import in
the test file (which triggers this case) is commented out for now.
Updates #146.
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>
Means that we no longer have to pass a dozen parameters around, mainly
to transformGo. We can also start documenting what each of the fields
actually does, and group them better.
While at it, pkgPath and pkgScope can both be replaced by a
*types.Package, since they're both accessible via trivially cheap
methods.
* 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
The test intended to use an extra module to be obfuscated, rsc.io/quote,
which we were bundling in the local proxy as well. Unfortunately, the
use of GOPRIVATE also meant that we did not actually fetch the module
from the proxy, and we would instead do a full roundtrip to the internet
to "git clone" the actual upstream repository.
To prevent that roundtrip, instead use a locally replaced module. This
fits the syntax.txt test too, since it's one more edge case that we want
to make sure works well with garble. Since rsc.io/quote is used in
another test, simply make up our own tiny module.
Reduces a 'go test -run Syntax/syntax' run with warm cache from ~5s to
~0.5s, thanks to removing the multiple roundtrips. A warm 'go test' run
still sits at ~6s, since we still need that much CPU time in total.
While at it, fix a staticcheck warning and fix inconsistent indentation
in a couple of tests.
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.
What obfuscateImports did was valid, but unfortunately made the build
cache redo work. This is because we were modifying object files in-place
in the build cache, meaning that the Go tool would think it had to
re-compile those packages.
Instead, write the modified object files in a temporary directory, and
leave the input object files untouched. We require a bit of extra code
to keep track of this and adjust the link argument as well as its
importcfg file.
The function of obfuscateImports, as well as the reasoning above, is now
summarized in its godoc as well.
This should be the last change in preparation for proper build caching
support. Rebasing the build caching branch on this commit finally makes
caching work reliably every single time.
More correct comments transformation was implemented.
Added processing of //go:linkname localname [importpath.name] directive, now localname is not renamed. This is safe and does not cause a name disclosure because the functions marked //linkname do not have a name in the resulting binary.
Added cgo directives support
Fixed filename leak protection for cgo
Part of #149
Fix for bug when a conflict occurred between generated short names
and local variables/functions/types/structs.
The already existing names are collected and if the generated short name
already exists, the package counter is increased until a free name is found.
Part of #149.
The struct type for buildInfo doesn't need to be named. Plus, the
"packageInfo" name was actually pretty misleading, because buildInfo
contains data from many packages.
Add an importCfg field, so that we don't need to fetch the flag value
many times.
Simplify reading the importCfg file; we used to also write to it, but
that's no longer the case, so we can just use ioutil.ReadFile.
Finally, give the function that fills buildInfo a better name, a godoc,
and fix the origTypesConfig godoc.
We also add a TODO to reuse goobj.ParseImportCfg in the future.
We now store how we obfuscated unexported names in the object file
itself, not a separate file. This means that the data can survive in the
build cache, whereas the separate file was being lost. Luckily, we can
just add an extra header to the archive, and other programs like the Go
linker will just ignore it.
Give the func a name that tells what the return value means.
Add missing newlines to printfs, use consistent quoting, and replace
"%s" with %q.
Document the Go 1.15 date.
Finally, fix the imports via goimports.
Many files were missing copyright, so also add a short script to add the
missing lines with the current year, and run it.
The AUTHORS file is also self-explanatory. Contributors can add
themselves there, or we can simply update it from time to time via
git-shortlog.
Since we have two scripts now, set up a directory for them.
Daniel Martí
Andrew LeFevre
lu4p
pagran