Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

change GOGARBLE default to obfuscate all packages #594

Closed
mvdan opened this issue Oct 6, 2022 · 7 comments · Fixed by #616
Closed

change GOGARBLE default to obfuscate all packages #594

mvdan opened this issue Oct 6, 2022 · 7 comments · Fixed by #616
Labels
help wanted Extra attention is needed

Comments

@mvdan
Copy link
Member

mvdan commented Oct 6, 2022

In the past, garble struggled to obfuscate some Go packages due to bugs, and it also couldn't obfuscate a good portion of the standard library. For those reasons, GOGARBLE defaulted to just obfuscating the current module.

However, it's gotten better. And, in general, obfuscating more packages is often a good thing - even when obfuscating public packages, it helps that the obfuscated private packages aren't making unobfuscated API calls to the public packages.

We should switch the default to be GOGARBLE=*. The only worry being that this might result in a regression if some garble users start seeing build errors.

So, this is where I would like some input. Does anyone get a failure if they use GOGARBLE=* on their garble build?
@mvdan mvdan added the help wanted Extra attention is needed label Oct 6, 2022
This was referenced Oct 6, 2022
Closed
Merged
mvdan added a commit to mvdan/garble-fork that referenced this issue Oct 12, 2022
@mvdan
README: suggest how to obfuscate all the packages
c3b1e79 
This mode is well tested, and may soon become the default,
but it wasn't properly documented yet.

For burrowers#594.
lu4p pushed a commit that referenced this issue Oct 12, 2022
@mvdan @lu4p
README: suggest how to obfuscate all the packages
745d089 
This mode is well tested, and may soon become the default,
but it wasn't properly documented yet.

For #594.
@Ne0nd0g
Copy link

Ne0nd0g commented Nov 11, 2022

I don't get build errors, but I do get this issue #604

@lu4p lu4p mentioned this issue Nov 13, 2022
Merged
mvdan added a commit to mvdan/garble-fork that referenced this issue Dec 5, 2022
@mvdan
default to GOGARBLE=*, stop using GOPRIVATE
86e40b7 
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 burrowers#594.
@mvdan mvdan mentioned this issue Dec 5, 2022
Merged
mvdan added a commit that referenced this issue Dec 13, 2022
@mvdan
default to GOGARBLE=*, stop using GOPRIVATE
481e3a1 
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.
@s77rt
Copy link

s77rt commented Dec 20, 2022

I think I'm getting errors caused by this change.

https://github.com/s77rt/rdpcloud

How can we set GOGARBLE to this module only? (at least to check if it's the root cause)

@mvdan
Copy link
Member Author

mvdan commented Dec 20, 2022

You can get the old behavior by setting GOGARBLE to your module path, e.g. GOGARBLE=github.com/s77rt/rdpcloud/server/go in your case.

If you're getting new errors or regressions, though, please file them as new issues. We want to fix them, because ideally we should be able to obfuscate all the packages.

@s77rt
Copy link

s77rt commented Dec 20, 2022

Thanks for the quick response, I'm not sure but I think i'm still getting the same error, I will be using 0.7.2 version for now.

@mvdan
Copy link
Member Author

mvdan commented Dec 20, 2022

Well, until you raise a new issue with details about that "same error", I don't have enough information to help, I'm afraid.

@s77rt
Copy link

s77rt commented Dec 20, 2022

I don't have much info either
b9Ll_yegT.go:2: unknown field 'EIgPolCkkmHQms' in struct literal of type OquAg8zx4q8lR8V.GECA1utYeDRz
This is the kind of error I'm getting, We need a minimal reproducible example to locate the issue

Maybe this has something to do with the use of ldflags.

@mvdan
Copy link
Member Author

mvdan commented Dec 20, 2022

Thanks, I can reproduce. It's #600 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

default to GOGARBLE=*, stop using GOPRIVATE mvdan/garble-fork
3 participants
@mvdan @Ne0nd0g @s77rt