When updating Garble to support Go 1.22.0, CI on MacOS spotted
that the syscall package was failing to build given that it uses
assembly code which is only allowed in some std packages.
That allowlist is based on import paths, and we were obfuscating
the syscall package's import path, so that was breaking GOOS=darwin.
As a fix, I added syscall to runtimeAndDeps to not obfuscate it.
That wasn't a great fix; it's not part of runtime and its dependencies,
and there's no reason we should avoid obfuscating the package contents.
Not obfuscating the contents in fact broke x/sys/unix,
as it contains a copy of syscall.Rlimit which it type converted with.
Undo that fix and reinstate the gogarble.txtar syscall test.
Implement the fix where we only leave syscall's import path alone.
Add a regression test, and add a note about adding x/net and x/sys
to check-third-party.sh so that we can catch these bugs earlier.
Fixes#830.
`go build ./...` does indeed compile and link main packages,
it just does not move the resulting binaries anywhere permanent
like `go install` does.
As such, the TODO isn't relevant; the fact that we build all packages
inside each module means we are already linking any binaries matched via
`./...` from the module root.
We don't run any of the binaries, which would catch panics at run-time,
but we already have a note at the top about using `garble test`.
The current garble release is able to obfuscate it with Go 1.20.
While here, re-generate all files to use "go 1.20" directives,
and add a TODO about also testing binary builds for each project.
See #600.
Since "garble build" first performs a Go build with -trimpath,
it helps to also use the flag with the "go build" step.
This way we get more build cache hits, as building a Go package with and
without the flag results in two separate builds.
Before:
$ go clean -cache && time ./scripts/check-third-party.sh
real 0m41.844s
user 2m17.791s
sys 0m35.440s
After:
$ go clean -cache && time ./scripts/check-third-party.sh
real 0m33.983s
user 1m50.596s
sys 0m28.499s
We can drop the code that kicked in when GOGARBLE was empty.
We can also add the value in addGarbleToHash unconditionally,
as we never allow it to be empty.
In the tests, remove all GOGARBLE lines where it just meant "obfuscate
everything" or "obfuscate the entire main module".
cgo.txtar had "obfuscate everything" as a separate step,
so remove it entirely.
linkname.txtar started failing because the imported package did not
import strings, so listPackage errored out. This wasn't a problem when
strings itself wasn't obfuscated, as transformLinkname silently left
strings.IndexByte untouched. It is a problem when IndexByte does get
obfuscated. Make that kind of listPackage error visible, and fix it.
reflect.txtar started failing with "unreachable method" runtime throws.
It's not clear to me why; it appears that GOGARBLE=* makes the linker
think that ExportedMethodName is suddenly unreachable.
Work around the problem by making the method explicitly reachable,
and leave a TODO as a reminder to investigate.
Finally, gogarble.txtar no longer needs to test for GOPRIVATE.
The rest of the test is left the same, as we still want the various
values for GOGARBLE to continue to work just like before.
Fixes#594.
When obfuscating the following piece of code:
func issue_573(s struct{ f int }) {
var _ *int = &s.f
/*x*/
}
the function body would roughly end up printed as:
we would roughly end up with:
var _ *int = &dZ4xYx3N
/*x*/.rbg1IM3V
Note that the /*x*/ comment got moved earlier in the source code.
This happens because the new identifiers are longer, so the printer
thinks that the selector now ends past the comment.
That would be fine - we don't really mind where comments end up,
because these non-directive comments end up being removed anyway.
However, the resulting syntax is wrong, as the period for the selector
must be on the first line rather than the second.
This is a go/printer bug that we should fix upstream,
but until then, we must work around it in Go 1.18.x and 1.19.x.
The fix is somewhat obvious in hindsight. To reduce the chances that
go/printer will trip over comments and produce invalid syntax,
get rid of most comments before we use the printer.
We still keep the removal of comments after printing,
since go/printer consumes some comments in ast.Node Doc fields.
Add the minimized unit test case above, and add the upstream project
that found this bug to check-third-party.
andybalholm/brotli helps cover a compression algorithm and ccgo code
generation from C to Go, and it's also a fairly popular module,
particular with HTTP implementations which want pure-Go brotli.
While here, fix the check-third-party script: it was setting GOFLAGS
a bit too late, so it may run `go get` on the wrong mod file.
Fixes#573.
Our tests should already be pretty extensive,
and any bug fixes should result in more regression test cases,
but testing against a few diverse and popular third party modules
will help prevent unintended regressions while developing garble.
The list is short for now. More can be added later.
This adds protobuf and wireguard from the original issue,
but not cobra and logrus, as they aren't particularly complex nor add
significant variety on top of protobuf and wireguard.
While here, we remove the job that only runs crlf-test.sh,
as we don't really need a separate job for a tiny script.
Fixes#240.
Daniel Martí