Commit Graph

209 Commits (5f74a1c9f06ff762cac7f29d8eecc2332157be08)

Author SHA1 Message Date
Daniel Martí a144789910 only obfuscate literals in packages to obfuscate
Add a regression test in gogarble.txt,
as that test is already set up with packages to not obfuscate.

This bug manifested in the form of a build failure for GOOS=plan9
with -literals turned on:

	[...]/os/file_plan9.go:151:12: invalid operation: cannot call non-function append (variable of type bool)

In this case, the "os" package is not to be obfuscated,
but we would still obfuscate its literals as per the bug.

But, since the package's identifiers were not obfuscated,
names like "append" were not replaced as per ea2e0bdf71,
meaning that the shadowing would still affect us.

Fixes #417.
3 years ago
Daniel Martí fceb19f6da
deprecate using GOPRIVATE in favor of GOGARBLE (#427)
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.
3 years ago
lu4p a645929151
obfuscate literals via constant folding
Constants don't need to be added to ignoreObjs anymore,
because go/types now does this work for us.

Fixes #360
3 years ago
Daniel Martí d5d1131b75 keep importmap entries in the right direction
For packages that we alter, we parse and modify the importcfg file.
Parsing is necessary so we can locate obfuscated object files,
which we use to remember what identifiers were obfuscated.
Modifying the files is necessary when we obfuscate import paths,
and those import paths have entries in an importcfg file.

However, we made one crucial mistake when writing the code.
When handling importmap entries such as:

	importmap golang.org/x/net/idna=vendor/golang.org/x/net/idna

we would name the two sides beforePath and afterPath, respectively.
They were added to importMap with afterPath as the key,
but when we iterated over the map to write a modified importcfg file,
we would then assume the key is beforepath.
All in all, we would end up writing the opposite direction:

	importmap vendor/golang.org/x/net/idna=golang.org/x/net/idna

This would ultimately result in the importmap never being useful,
and some rather confusing error messages such as:

	cannot find package golang.org/x/net/idna (using -importcfg)

Add a test case that reproduces this error,
and fix the code so it always uses beforePath as the key.
Note that we were also updating importCfgEntries with such entries.
I could not reproduce any failure when just removing that code,
nor could I explain why it was added there in the first place.
As such, remove that bit of code as well.

Finally, a reasonable question might be why we never noticed the bug.
In practice, such "importmap"s, represented as ImportMap by "go list",
only currently appear for packages vendored into the standard library.
Until very recently, we didn't support obfuscating most of std,
so we would usually not alter the affected importcfg files.
Now that we do parse and modify them, the bug surfaced.

Fixes #408.
3 years ago
Daniel Martí ea2e0bdf71
obfuscate all variable names, even local ones (#420)
In the added test case, "garble -literals build" would fail:

	--- FAIL: TestScripts/literals (8.29s)
		testscript.go:397:
			> env GOPRIVATE=test/main
			> garble -literals build
			[stderr]
			# test/main
			Usz1FmFm.go:1: cannot call non-function string (type int), declared at Usz1FmFm.go:1
			Usz1FmFm.go:1: string is not a type
			Usz1FmFm.go:1: cannot call non-function append (type int), declared at Usz1FmFm.go:1

That is, for input code such as:

	var append int
	println("foo")
	_ = append

We'd end up with obfuscated code like:

	var append int
	println(func() string {
		// obfuscation...
		x = append(x, ...)
		// obfuscation...
		return string(x)
	})
	_ = append

Which would then break, as the code is shadowing the "append" builtin.
To work around this, always obfuscate variable names, so we end up with:

	var mwu1xuNz int
	println(func() string {
		// obfuscation...
		x = append(x, ...)
		// obfuscation...
		return string(x)
	})
	_ = mwu1xuNz

This change shouldn't make the quality of our obfuscation stronger,
as local variable names do not currently end up in Go binaries.
However, this does make garble more consistent in treating identifiers,
and it completely avoids any issues related to shadowing builtins.

Moreover, this also paves the way for publishing obfuscated source code,
such as #369.

Fixes #417.
3 years ago
Daniel Martí 34e190c7a0 testdata: tweak typechecking error for Go 1.18 again
Upstream tweaked the error format once again.
3 years ago
Daniel Martí 64cbbbaa0f update modinfo.txt test for 1.18's build and VCS info
That is, use a very specific build tag and git commit,
and ensure that neither ends up in the binary.

Luckily, we have nothing to do here.
We were already removing _gomod_.go from the build entirely,
and that is still the mechanism that "go build" uses to bundle the data.

Note that the test will still work if git is not installed,
but it will simply not check the VCS side.

Finally, we use "go version -m" to check the existing fields,
which is easier than calling the Go APIs directly.

It seems like "go test" passes on yesterday's Go master, now.
So, enable test-gotip again with that commit hash.

Fixes #385.
3 years ago
Daniel Martí d2203c4cce testdata: make reverse.txt pass with Go 1.18
This makes "go test -short" pass on the current gotip.

Updates #385.
3 years ago
lu4p 3d19605782
Fix linkname directives with dots in importpath (#407)
Obfuscating newName arguments of linkname directives
with dots in the importpath didn't work before.

We had a test which covers this,
but the corresponding package wasn't actually obfuscated.
3 years ago
lu4p 88f238e558
Obfuscate more packages of the standard library (#312)
Also update linkname directives of public packages,
to allow the package where something is linknamed to to be
obfuscated regardless.

Public packages can now depend on private packages.
3 years ago
Daniel Martí e7320ec9c0 testdata: fix typo in literals.txt
Just noticed in passing that the type name was missing a "t".
While at it, make the fields consistently camelCase.
3 years ago
Daniel Martí 7ede21c981 drop support for Go 1.16.x
We can now use pruned module graphs in go.mod files,
and we no longer need to worry about runtime/internal/sys.

Note that I had to update testdata/mod slightly,
as the new pruned module graphs algorithm downloads an extra go.mod file.

This change also paves the way towards future Go 1.18 support.

Thanks to lu4p for cleaning up two TODOs as well.

Co-Authored-By: lu4p <lu4p@pm.me>
3 years ago
Daniel Martí 1682e8ee10 always require one argument for "reverse"
The "reverse" command had many levels of optional arguments:

	garble [garble flags] reverse [build flags] [package] [files]

This was pretty confusing,
and could easily lead to people running the command incorrectly:

	# note that output.txt isn't a Go package!
	garble reverse output.txt

Moreover, it made the handling of Go build flags pretty confusing.
Should the command below work?

	garble reverse -tags=mytag

It also made it easy to not notice that one must supply the main package
to properly reverse some text that it produced, like a panic message.
With the package path being implicit,
one could mistakenly provide the wrong package by running garble
in a directory containing a different package.

See #394.
3 years ago
Daniel Martí 8d162dcd04 test importing a C struct type
This seems to work so far, but we weren't testing it.

For #384.
3 years ago
lu4p aafd845418 More robust reflection detection
Functions which use reflection on one of their parameters are,
now added to knownReflectAPIs automatically.

This makes most explicit hints for reflection redundant.
Simple protobuf code now works correctly when obfuscated.

Fixes #162
Fixes #373
3 years ago
Daniel Martí 5c70681fee detect more std API calls which use reflection
Before, we would just notice direct calls to reflect's TypeOf and
ValueOf. Any other uses of reflection, such as encoding/json or
google.golang.org/protobuf, would require hints as documented by the
README.

Issue #162 outlines some ways we could fix this issue in a general way,
automatically detecting what functions use reflection on their parameters,
even for third party API funcs.

However, that goal is pretty significant in terms of code and effort.
As a temporary improvement, we can expand the list of "known" reflection
APIs via a static table.

Since this table is keyed by "func full name" strings, we could
potentially include third party APIs, such as:

	google.golang.org/protobuf/proto.Marshal

However, for now simply include all the std APIs we know about.
If we fail to do the proper fix for automatic detection in the future,
we can then fall back to expanding this global table for third parties.

Update the README's docs, to clarify that the hint is not always
necessary anymore.

Also update the reflect.txt test to stop using the hint for encoding/json,
and to also start testing text/template with a method call.
While at it, I noticed that we weren't testing the println outputs,
as they'd go to stderr - fix that too.

Updates #162.
3 years ago
lu4p 3ab59000f3 Follow up: Obfuscate more byte slice literals 3 years ago
Daniel Martí ec0bdc4012 keep cgo-exported Go names non-obfuscated
Otherwise, the added test case would fail, as we don't modify the C code
and so there would be a name mismatch.

In the far future we might start modifying Go names in C code,
similar to what we did for Go assembly,
but right now that seems out of scope and too complex.

An easier fix is to simply record those (hopefully few) names in ignoreObjects.

While at it, recordReflectArgs started to really outgrow its name, as it
also collected expressions used as constants for literal obfuscation.
Give it a better name.

Fixes #366.
3 years ago
Daniel Martí 691a44cecb avoid breaking const declarations using iotas
With the -literals flag, we try to convert some const declarations to
vars, as long as that doesn't break typechecking. We really only do that
for typed constant strings, really.

There was a quirk: if a numerical constant had a type and used iota, we
would not obfuscate its value, but we would still convert the
declaration from const to var. Since iotas only work within const
declarations, that would break compilation:

	> garble -literals build
	[stderr]
	# test/main
	FeWE3zwi.go:19: undefined: iota
	exit status 2

To fix the problem, make the logic more conservative: only obfuscate
constant declarations where the values are typed strings, meaning that
any numerical constants are left entirely untouched.

This fixes the build of google.golang.org/protobuf/runtime/protoiface
with -literals turned on.
3 years ago
lu4p c1672cdc0d Obfuscate more byte slice literals
Slices with hex, octal, binary and rune elements are now obfuscated.
3 years ago
lu4p 552a6bcfb0 Obfuscate literals in string slices and arrays
Fixes #354
3 years ago
Daniel Martí 5e3cdf89a8 update support for Go 1.17 in time for beta1
Back in early April we added initial support for Go 1.17,
working on a commit from master at that time. For that to work, we just
needed to add a couple of packages to runtimeRelated and tweak printFile
a bit to not break the new "//go:build" directives.

A significant amount of changes have landed since, though, and the tests
broke in multiple ways.

Most notably, the new register ABI is enabled by default for GOOS=amd64.
That affected garble indirectly in two ways: there's a new internal
package to add to runtimeRelated, and we must make reverse.txt more
clever in making its output constant across ABIs.

Another noticeable change is that Go 1.17 changes how its own version is
injected into the runtime package. It used to be via a constant in
runtime/internal/sys, such as:

	const TheVersion = `devel ...`

Since we couldn't override such constants via the linker's -X flag,
we had to directly alter the declaration while compiling.

Thankfully, Go 1.17 simply uses a "var buildVersion string" in the
runtime package, and its value is injected by the linker.
This means we can now override it with the linker's -X flag.

We make the code to alter TheVersion for Go 1.16 a bit more clever,
to not break the package when building with Go 1.17.

Finally, our hack to work around ambiguous TOOLEXEC_IMPORTPATH values
now only kicks in for non-test packages, since Go 1.17 includes our
upstream fix. Otherwise, some tests would end up with the ".test"
variant suffix added a second time:

	test/bar [test/bar.test] [test/bar [test/bar.test].test]

All the code to keep compatibility with Go 1.16.x remains in place.
We're still leaving TODOs to remind ourselves to remove it or simplify
it once we remove support for 1.16.x.

The 1.17 development freeze has already been in place for a month,
and beta1 is due to come this week, so it's unlikely that Go will change
in any considerable way at this point. Hence, we can say that support
for 1.17 is done.

Fixes #347.
3 years ago
Daniel Martí 1d31a139f5 support aliases as embedded fields in dependencies
Our recent work in fieldToAlias worked well when the embedded field
declaration (using an alias) was in the same package as the use of that
field. We would have the *ast.Ident for the field declaration, so
types.Info.Uses would give us the TypeName for the alias.

Unfortunately, if the declaration was in a dependency package, we did
not have that same *ast.Ident, as we weren't parsing the source code for
dependencies for type-checking. This resulted in us incorrectly
obfuscating the use of such an embedded field:

	> garble build
	[stderr]
	# test/main
	JtzmzxWf.go:4: unknown field 'ExternalForeignAlias' in struct literal of type _BdSNiEL.Vcs_smer

To fix this, look through the direct imports of the package defining the
field to find an alias under the exact same name. Not a foolproof
solution, as there's a TODO, but it should work for most cases.

Fixes the obfuscation of google.golang.org/grpc/internal/status, too.

Updates #349.
3 years ago
Daniel Martí 68b39e8195 fix a link issue when obfuscating complex cgo packages
The added test case, which is obfuscating and linking os/user, would fail
before this fix:

	> garble build
	[stderr]
	# test/main
	/usr/lib/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
	/usr/bin/ld: $WORK/.tmp/go-link-073246656/go.o: in function `Chz0Yfs2._cgo_cmalloc':
	go.go:(.text+0x993cc): undefined reference to `Chz0Yfs2.runtime_throw'
	/usr/bin/ld: $WORK/.tmp/go-link-073246656/go.o: in function `Chz0Yfs2.tDfhQ8uK':
	go.go:(.text+0x99801): undefined reference to `Chz0Yfs2._cgo_runtime_gostring'
	/usr/bin/ld: go.go:(.text+0x9982a): undefined reference to `Chz0Yfs2._cgo_runtime_gostring'
	/usr/bin/ld: go.go:(.text+0x99853): undefined reference to `Chz0Yfs2._cgo_runtime_gostring'
	collect2: error: ld returned 1 exit status

The reason is that we would alter the linkname directives of cgo-generated
code, but we would not obfuscate the code itself at all.

The generated code would end up being transformed into:

	//go:linkname zh_oKZIy runtime.throw
	func runtime_throw(string)

One can clearly see the error there; handleDirectives obfuscated the
local linkname name, but since transformGo didn't run, the actual Go
declaration was not obfuscated in the same way. Thus, the linker fails
to find a function body for runtime_throw, and fails.

The solution is simple: handleDirectives assumes that it's running on
code being obfuscated, so only run it when transformGo is running.

We can also remove the cgo skip check in handleDirectives, as it never
runs on cgo-generated code now.

Fixes a number of build errors that have been noticed since
907aebd770.
3 years ago
Daniel Martí 64317883c9 handle aliases to foreign named types properly
When such an alias name was used to define an embedded field, we handled
that case gracefully via the code using:

	tf.info.Uses[node].(*types.TypeName)

Unfortunately, when the same field name was used elsewhere, such as a
composite literal, tf.Info.Uses gave us a *types.Var, not a
*types.TypeName, meaning we could no longer tell if this was an alias,
or what it pointed to.

Thus, we failed to obfuscate the name properly in the added test case:

	> garble build
	[stderr]
	# test/main/sub
	xxWZf66u.go:36: unknown field 'foreignAlias' in struct literal of type smhWelwn

It doesn't seem like any of the go/types APIs allows us to obtain the
*types.TypeName directly in this scenario. Thus, use a trick that we
used before: after typechecking, but before obfuscating, record all
embedded struct field *types.Var which are aliases via a map, where the
value holds the *types.TypeName for the alias.

Updates #349.
3 years ago
Daniel Martí e2ddce75a7 support embedding via embed.FS
We already added support for "//go:embed" with string and []byte,
by not obfuscating the "embed" import path.

However, embed.FS was still failing:

	> garble build
	[stderr]
	# test/main
	:13: go:embed cannot apply to var of type embed.WtKNvwbN

The compiler detects the type by matching its name to exactly "embed.FS",
so don't obfuscate the name "FS" either.

While at it, ensure that the embed code behaves the same with "go build".

Updates #349.
3 years ago
Daniel Martí 7fc424ca26 make "garble command -h" give command-specific help
Before, "garble build -h" would print the same as "garble -h", which is
too much and confusing, as it doesn't tell us much about "build".

Now it's far better, and includes the output of "go build -h":

	$ garble build -h
	usage: garble [garble flags] build [arguments]

	This command wraps "go build". Below is its help:

	usage: go build [-o output] [build flags] [packages]
	Run 'go help build' for details.

We do the same for "garble reverse -h", since it doesn't wrap a Go tool
command.
3 years ago
Daniel Martí 750340db5b testdata: deduplicate GODEBUG cleanup line
I added a second one recently without noticing, since the original was
too far below and I had written commands above it.

Deduplicate them, and put the new env line as early as possible, to
prevent further issues and confusion.
3 years ago
Daniel Martí 5fa4acf580 testdata: use longer Go filenames for binsubstr
Every now and then, a CI run would fail:

	FAIL: testdata/scripts/reflect.txt:7: unexpected match for ["main.go"] in main

These were rare, and very hard to reproduce or debug.

My best guess is that, since "main.go" is a short string and we use
random eight-character obfuscated filenames ending with ".go", it was
possible that the random filename happened to end in "main" in some
cases.

Given the base64 encoding, the chances of a single suffix collision are
about 0.000006%. Note, however, that a single obfuscated build will most
likely obfuscate many filenames, especially for the tests obfuscating
multiple packages. For a single CI run with many tests across three OSs,
the chances of any collision are likely very low, but realistic.

All this has a simple fix: use longer filenames to match with. We choose
"garble_main.go" since it's long enough, but also because it's still
clear it's a "main" Go file, and it's very unlikely to cause conflicts
with filenames in upstream Go given the "garble_" prefix.
3 years ago
Daniel Martí 65ff07875b obfuscate alias names like any other objects
Before this change, we would try to never obfuscate alias names. That
was far from ideal, as they can end up in field names via anonymous
fields.

Even then, we would sometimes still fail to build, because we would
inconsistently obfuscate alias names. For example, in the added test
case:

	--- FAIL: TestScripts/syntax (0.23s)
	    testscript.go:397:
	        > env GOPRIVATE='test/main,private.source'
	        > garble build
	        [stderr]
	        # test/main/sub
	        Lv_a8gRD.go:15: undefined: KCvSpxmQ

To fix this problem, we set obj to be the TypeName corresponding to the
alias when it is used as an embedded field. We can then make the right
choice when obfuscating the name.

Right now, all aliases will be obfuscated. A TODO exists about not
obfuscating alias names when they're used as embedded fields in a struct
type in the same package, and that package is used for reflection -
since then, the alias name ends up as the field name.

With these changes, the protobuf module now builds.
3 years ago
Daniel Martí 68f07389b2 fix a number of issues involving types from indirect imports
obfuscatedTypesPackage is used to figure out if a name in a dependency
package was obfuscated or not. For example, if that package used
reflection on a named type, it wasn't obfuscated, so we must have the
same information to not obfuscate the same name downstream.

obfuscatedTypesPackage could return nil if the package was indirectly
imported, though. This can happen if a direct import has a function that
returns an indirect type, or if a direct import exposes a name that's a
type alias to an indirect type.

We sort of dealt with this in two pieces of code by checking for
obfPkg!=nil, but a third one did not have this check and caused a panic
in the added test case:

	--- FAIL: TestScripts/reflect (0.81s)
	    testscript.go:397:
	        > env GOPRIVATE=test/main
	        > garble build
	        [stderr]
	        # test/main
	        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=0x20 pc=0x8a5e39]

More importantly though, the nil check only avoids panics. It doesn't
fix the root cause of the problem: that importcfg does not contain
indirectly imported packages. The added test case would still fail, as
we would obfuscate a type in the main package, but not in the indirectly
imported package where the type is defined.

To fix this, resurrect a bit of code from earlier garble versions, which
uses "go list -toolexec=garble" to fetch a package's export file. This
lets us fill the indirect import gaps in importcfg, working around the
problem entirely.

This solution is still not particularly great, so we add a TODO about
possibly rethinking this in the future. It does add some overhead and
complexity, though thankfully indirect imports should be uncommon.

This fixes a few panics while building the protobuf module.
3 years ago
Daniel Martí 654841e1fb skip reflection detection for sibling packages
Our allPkgs boolean logic was wrong, because it could still lead to
garble obfuscating a type when used in the main package, but not in its
defining package. The added test case shows such a case.

To fix that, use a package path to only record the named objects from
the target package, which is a narrower operation without this problem,
but still makes all our tests pass.

This makes the google.golang.org/protobuf/internal/filetype package
start building.
3 years ago
Daniel Martí 4156f35570 remove tinyfmt implementation from a test script
We've had proper build caching for a while now. No need to avoid std
imports like fmt.
3 years ago
Daniel Martí adb4f44fb2 reverse lone filenames as well
I've wanted this to more easily debug build failures.

To not force a build failure in a test script, as that would require
some trickery to remain stable, we use runtime.Caller without printing
the line number. Before this patch, those filenames without line numbers
would not be reversed at all.
3 years ago
Daniel Martí b6dee63b32 improve handling of reflect on foreign unnamed types
If a package A imports package B, and uses reflect.TypeOf on an unnamed
struct type B.T (such as an alias), we don't want to record B.T's fields
as "do not obfuscate". This is for the same reason that we don't if B.T
is a named struct type: the detection only works for the package
defining the type, as otherwise it's inconsistent.

We failed to handle this case well, because we assumed all struct types
would be under some named type. This is not the case for type aliases.

Fortunately, struct fields are named, and as such they are objects.
Check their package too, just like we do for named types.

Fixes another build error when obfuscating the protobuf module.
We add a simplified version of the example above as a test case,
which originated from debugging the protobuf build failure.
3 years ago
Daniel Martí 8961e0a39a testdata: split reflection test cases into reflect.txt
The detection of reflection usage is tricky and there are plenty of edge
cases to test for. We definitely want one script for it, rather than
splitting those cases between other scripts like imports.txt and
syntax.txt.

Moreover, those two were rather generic and large, so this helps keep a
balance.
3 years ago
Daniel Martí d8de5a4306 avoid reproducibility issues with full rebuilds
We were using temporary filenames for modified Go and assembly files.
For example, an obfuscated "encoding/json/encode.go" would end up as:

	/tmp/garble-shared123/encode.go.456.go

where "123" and "456" are random numbers, usually longer.

This was usually fine for two reasons:

1) We would add "/tmp/garble-shared123/" to -trimpath, so the temporary
   directory and its random number would be invisible.

2) We would add "//line" directives to the source files, replacing
   the filename with obfuscated versions excluding any random number.

Unfortunately, this broke in multiple ways. Most notably, assembly files
do not have any line directives, and it's not clear that there's any
support for them. So the random number in their basename could end up in
the binary, breaking reproducibility.

Another issue is that the -trimpath addition described above was only
done for cmd/compile, not cmd/asm, so assembly filenames included the
randomized temporary directory.

To fix the issues above, the same "encoding/json/encode.go" would now
end up as:

	/tmp/garble-shared123/encoding/json/encode.go

Such a path is still unique even though the "456" random number is gone,
as import paths are unique within a single build.

This fixes issues with the base name of each file, so we no longer rely
on line directives as the only way to remove the second original random
number.

We still rely on -trimpath to get rid of the temporary directory in
filenames. To fix its problem with assembly files, also amend the
-trimpath flag when running the assembler tool.

Finally, add a test that reproducible builds still work when a full
rebuild is done. We choose goprivate.txt for such a test as its
stdimporter package imports a number of std packages, including uses of
assembly and cgo.

For the time being, we don't use such a "full rebuild" reproducibility
test in other test scripts, as this step is expensive, rebuilding many
packages from scratch.

This issue went unnoticed for over a year because such random numbers
"123" and "456" were created when a package was obfuscated, and that
only happened once per package version as long as the build cache was
kept intact.

When clearing the build cache, or forcing a rebuild with -a, one gets
new random numbers, and thus a different binary resulting from the same
build input. That's not something that most users would do regularly,
and our tests did not cover that edge case either, until now.

Fixes #328.
3 years ago
Daniel Martí 58c15aa680 testdata: scope GODEBUG to a single test case
I've tripped over this GODEBUG env var four or five times already.
Since it affects any Go program by making them print tons of runtime
information, it also affects "go env", breaking garble horribly.

To prevent further issues, unset the env var when done.
3 years ago
Daniel Martí c9b0b07853 hash field names equally in all packages
Packages P1 and P2 can define identical struct types T1 and T2, and one
can convert from type T1 to T2 or vice versa.

The spec defines two identical struct types as:

	Two struct types are identical if they have the same sequence of
	fields, and if corresponding fields have the same names, and
	identical types, and identical tags. Non-exported field names
	from different packages are always different.

Unfortunately, garble broke this: since we obfuscated field names
differently depending on the package, cross-package conversions like the
case above would result in typechecking errors.

To fix this, implement Joe Tsai's idea: hash struct field names with the
string representation of the entire struct. This way, identical struct
types will have their field names obfuscated in the same way in all
packages across a build.

Note that we had to refactor "reverse" a bit to start using transformer,
since now it needs to keep track of struct types as well.

This failure was affecting the build of google.golang.org/protobuf,
since it makes regular use of cross-package struct conversions.

Note that the protobuf module still fails to build, but for other
reasons. The package that used to fail now succeeds, so the build gets a
bit further than before. #240 tracks adding relevant third-party Go
modules to CI, so we'll track the other remaining failures there.

Fixes #310.
3 years ago
Andrew LeFevre b3db7d6fa7 fix obfuscating linkname directives that where the package name contained a dot 3 years ago
Andrew LeFevre 65b4692dbc hopefully fix position test on Windows by not matching on 'main.go' 3 years ago
Daniel Martí b4fc735a1e fix windows/arm cross-build linking
Obfuscating some std packages for windows/arm triggered a bug; when
encountering a call to runtime·memmove, we'd hash "memmove" with the
current package's action ID.

This is wrong on two levels: First, we aren't obfuscating the runtime
package yet. And second, if we did, we would have to hash the symbol
appropriately, with that package's action ID.

For now, only hashing the local names does the trick. That's all that
the code currently supports, anyway.
3 years ago
Daniel Martí 6ac7dce4a0 remove obsolete TODO in the tiny.txt test
Since 1a8e32227f we have been setting line numbers on call sites, and
that line number is always the minimum value for -tiny: 1.

Still not as zero-overhead as the old mechanism which entirely deleted
line numbers, but a useless and small line number is still pretty good.
3 years ago
Daniel Martí 0150aa8bb0 support reversing field names
THey don't show up in stack traces, and if they show up in regular
program output, we should in theory not obfuscate those names via the
detection of reflection.

However, there's one relatively common scenario where obfuscated field
names can appear: in build error messages, when obfuscation fails a
build.
3 years ago
Daniel Martí 05d35350cf record types into ignoreObjects more reliably
Our previous logic only took care of fairly simple types, such as a
simple struct or a pointer to a struct. If we had a struct embedding
another struct, we'd fail to record the objects for the fields in the
inner struct, and that would lead to miscompilation:

	> garble build
	[stderr]
	# test/main
	LZmt64Nm.go:7: outer.InnerField undefined (type *CcUt1wkQ.EmbeddingOuter has no field or method InnerField)

To fix this issue, make the function that records all objects under a
types.Type smarter. Since it now does more than just dealing with
structs, it's also renamed.

Since the function now walks types properly, we get to remove the extra
ast.Inspect in recordReflectArgs, which is nice.

We also make it a method, to avoid the map parameter. A boolean
parameter is also added, since we need this feature to only look at the
current package when looking at reflect calls.

Finally, we add a test case, a simplified version of the original bug
report.

Fixes #315.
3 years ago
Daniel Martí 2fad0e1583 wrap types.Importer to canonicalize import paths
The docs for go/importer.ForCompiler say:

	The lookup function is called each time the resulting importer
	needs to resolve an import path. In this mode the importer can
	only be invoked with canonical import paths (not relative or
	absolute ones); it is assumed that the translation to canonical
	import paths is being done by the client of the importer.

We use a lookup func for two reasons: first, to support modules, and
second, to be able to use our information from "go list -json -export".

However, go/types does not canonicalize import paths before calling
ImportFrom. This is somewhat understandable; it doesn't know whether an
importer was created with a lookup func, and ImportFrom only requires
the input path to be canonicalized in that scenario. When the lookup
func is nil, the importer canonicalizes by itself via go/build.Import.

Before this change, the added crossbuild test would fail:

	> garble build net/http
	[stderr]
	# vendor/golang.org/x/crypto/chacha20
	typecheck error: /usr/lib/go/src/vendor/golang.org/x/crypto/chacha20/chacha_generic.go:10:2: could not import crypto/cipher (can't find import: "crypto/cipher")
	# vendor/golang.org/x/text/secure/bidirule
	typecheck error: /usr/lib/go/src/vendor/golang.org/x/text/secure/bidirule/bidirule.go:12:2: could not import errors (can't find import: "errors")
	# vendor/golang.org/x/crypto/cryptobyte
	typecheck error: /usr/lib/go/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go:8:16: could not import encoding/asn1 (can't find import: "encoding/asn1")
	# vendor/golang.org/x/text/unicode/norm
	typecheck error: /usr/lib/go/src/vendor/golang.org/x/text/unicode/norm/composition.go:7:8: could not import unicode/utf8 (can't find import: "unicode/utf8")

This is because we'd fall back to importer.Default, which only knows how
to find packages in $GOROOT/pkg. Those are missing for cross-builds,
unsurprisingly, as those built archives end up in the build cache.

After this change, we properly support importing std-vendored packages,
so we can get rid of the importer.Default workaround. And, by extension,
cross-builds now work as well.

Note that, in the added test script, the full build of the binary fails,
as there seems to be some sort of linker problem:

	> garble build
	[stderr]
	# test/main
	d9rqJyxo.uoqIiDs5: relocation target runtime.os9A16A3 not defined

We leave that as a TODO for now, as this change is subtle enough as it
is.
3 years ago
lu4p 1a9fdb4e8e
Fix calls to linkname functions (#314) 3 years ago
Daniel Martí 3afc993266 use "go env -json" to collect env info all at once
In the worst case scenario, when GOPRIVATE isn't set at all, we would
run these three commands:

* "go env GOPRIVATE", to fetch GOPRIVATE itself
* "go list -m", for GOPRIVATE's fallback
* "go version", to check the version of Go being used

Now that we support Go 1.16 and later, all these three can be obtained
via "go env -json":

	$ go env -json GOPRIVATE GOMOD GOVERSION
	{
		"GOMOD": "/home/mvdan/src/garble/go.mod",
		"GOPRIVATE": "",
		"GOVERSION": "go1.16.3"
	}

Note that we don't get the module path directly, but we can use the
x/mod/modfile Go API to parse it from the GOMOD file cheaply.

Notably, this also simplifies our Go version checking logic, as now we
get just the version string without the "go version" prefix and
"GOOS/GOARCH" suffix we don't care about.

This makes our code a bit more maintainable and robust. When running a
short incremental build, we can also see a small speed-up, as saving two
"go" invocations can save a few milliseconds:

	name           old time/op       new time/op       delta
	Build/Cache-8        168ms ± 0%        166ms ± 1%  -1.26%  (p=0.009 n=6+6)

	name           old bin-B         new bin-B         delta
	Build/Cache-8        6.36M ± 0%        6.36M ± 0%  +0.12%  (p=0.002 n=6+6)

	name           old sys-time/op   new sys-time/op   delta
	Build/Cache-8        222ms ± 2%        219ms ± 3%    ~     (p=0.589 n=6+6)

	name           old user-time/op  new user-time/op  delta
	Build/Cache-8        857ms ± 1%        846ms ± 1%  -1.31%  (p=0.041 n=6+6)
3 years ago
Daniel Martí 24d5ff362c fix a regression involving imported linkname funcs
In ce2c45440a, we simplified the code a bit and removed one call to
obfuscatedTypesPackage.

Unfortunately, we introduced a regression; if an exported function is
linknamed to another symbol name, and it's called from an importer
package, we would have a build failure now:

	> garble build
	[stderr]
	# test/main
	ZiOACuw7.go:1: undefined: ODC0xN52.BaDqbhkj

This is because the imported package would not hash the original name,
via its ignoreObjects logic. And, since the importer package has no
access to that knowledge, it would hash the same name, and fail to find
it in the final build.

The regression happened because we used to have a types.Scope Lookup
that saved us in this scenario. Add the test, and re-add the Lookup,
this time only for this particular scenario with function names.

Thanks to Andrew LeFevre for reporting and describing the test case.

While at it, replace more uses of "garbled" to "obfuscated".
3 years ago
Daniel Martí 5de519694a CI: pin a commit when testing against Go tip
Since it changes rapidly, especially during merge cycles, and we don't
want CI to surprisingly blow up in our faces from one day to another.

Pin this to a commit from yesterday which works, since some changes
merged today moved where the Go build version is recorded and broke
garble.

While at it, replace "git clone" with a wget of a source archive. This
is much, much faster, mainly because a tarball is significantly smaller.
We now download about 20MiB instead of over 350MiB.

One downside is that, without git, make.bash can't construct a devel
version on its own. For that reason, add a pretty basic manual version
via the VERSION file.

This means that we must not reject custom devel version strings. This is
a good thing anyway, because custom devel strings are already common
when building Go in custom ways. Those people tend to be advanced users,
such as CI, so fall back to assuming they know what they are doing and
don't error.

Plus, starting last week, devel versions in Go master now contain the
major Go version like in build tags, such as "go1.17-commit...", so we
will soon start relying on that instead of parsing dates:

	$ go version
	go version devel go1.17-a7e16abb22 Thu Apr 8 07:33:58 2021 +0000 linux/amd64
3 years ago