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.

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

Build error and garble unsupport github.com/gogo/protobuf@v1.3.2 #685

Closed
uk0 opened this issue Feb 22, 2023 · 10 comments · Fixed by #783
Closed

Build error and garble unsupport github.com/gogo/protobuf@v1.3.2 #685

uk0 opened this issue Feb 22, 2023 · 10 comments · Fixed by #783

Comments

@uk0
Copy link

uk0 commented Feb 22, 2023

What version of Garble and Go are you using?

$ garble version (0.9.3)
mvdan.cc/garble v0.0.0-20230216084601-d0c8c8d844df

Build settings:
      -buildmode exe
       -compiler gc
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin
             vcs git
    vcs.revision d0c8c8d844dfd0d866aaa7706c3f77227688828a
        vcs.time 2023-02-16T08:46:01Z
    vcs.modified false

$ go version
  go version go1.20.1 darwin/arm64

What environment are you running Garble on?

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/firshme/Library/Caches/go-build"
GOENV="/Users/firshme/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/firshme/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/firshme/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.20.1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.20.1/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mm/fg4pvncn5psd0wkx0rkkhht80000gn/T/go-build3623751020=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

link ERROR GIST

@uk0 uk0 changed the title Build error and garble unsupport Build error and garble unsupport github.com/gogo/protobuf@v1.3.2 Feb 22, 2023
@lu4p
Copy link
Member

lu4p commented Feb 24, 2023

We test against  google.golang.org/protobuf@v1.28, maybe downgrade for now.

@mvdan
Copy link
Member

mvdan commented Feb 24, 2023

I think those are unrelated; gogo/protobuf is a very old, and I believe unmaintained, fork of protobuf. I seem to recall it makes pretty heavy use of unsafe or reflect, so it's not surprising that it doesn't work out of the box. At the same time, it has been deprecated for a while now, so I'm not sure we should prioritize it.

@lu4p
Copy link
Member

lu4p commented Feb 24, 2023

Oh I assumed we're talking about normal protobuf

@Techno-Fox
Copy link

gogo/protobuf is old and unmaintained, I think big libraries still use it for ease of use. Namely libp2p.

@eyevanovich
Copy link

Is there any way to ignore gabling this library? I've tried to add GOBARBLE=!github.com/gogo/protobuf with no luck.

@lu4p
Copy link
Member

lu4p commented Mar 3, 2023

You can use GOGARBLE=your/module/path to only obfuscate your module and nothing else.

@uk0 uk0 closed this as completed Apr 24, 2023
@mvdan
Copy link
Member

mvdan commented Apr 30, 2023

This is not completed; we do support the official google.golang.org/protobuf, but the gogo/protobuf does not work with garble right now. Reopening to track the fix, since otherwise we keep getting duplicate reports like #723.

@Techno-Fox
Copy link

Thank you for reopening

mvdan added a commit to mvdan/garble-fork that referenced this issue Aug 1, 2023
Up until now, the new SSA reflection detection relied on call sites
to propagate which objects (named types, struct fields) used reflection.
For example, given the code:

    json.Marshal(new(T))

we would first record that json.Marshal calls reflect.TypeOf,
and then that the user's code called json.Marshal with the type *T.

However, this would not catch a slight variation on the above:

    var t T
    reflect.TypeOf(t)
    t.foo = struct{bar int}{}

Here, type T's fields such as "foo" and "bar" are not obfuscated,
since our logic sees the call site and marks the type T recursively.
However, the unnamed `struct{bar int}` type was still obfuscated,
causing errors such as:

    cannot use struct{uKGvcJvD24 int}{} (value of type struct{uKGvcJvD24 int}) as struct{bar int} value in assignment

The solution is to teach the analysis about *ssa.Store instructions
in a similar way to how it already knows about *ssa.Call instructions.
If we see a store where the destination type is marked for reflection,
then we mark the source type as well, fixing the bug above.

This fixes obfuscating github.com/gogo/protobuf/proto.
A number of other Go modules fail with similar errors,
and they look like very similar bugs,
but this particular fix doesn't apply to them.
Future incremental fixes will try to deal with those extra cases.

Fixes burrowers#685.
@lu4p lu4p closed this as completed in #783 Aug 2, 2023
lu4p pushed a commit that referenced this issue Aug 2, 2023
Up until now, the new SSA reflection detection relied on call sites
to propagate which objects (named types, struct fields) used reflection.
For example, given the code:

    json.Marshal(new(T))

we would first record that json.Marshal calls reflect.TypeOf,
and then that the user's code called json.Marshal with the type *T.

However, this would not catch a slight variation on the above:

    var t T
    reflect.TypeOf(t)
    t.foo = struct{bar int}{}

Here, type T's fields such as "foo" and "bar" are not obfuscated,
since our logic sees the call site and marks the type T recursively.
However, the unnamed `struct{bar int}` type was still obfuscated,
causing errors such as:

    cannot use struct{uKGvcJvD24 int}{} (value of type struct{uKGvcJvD24 int}) as struct{bar int} value in assignment

The solution is to teach the analysis about *ssa.Store instructions
in a similar way to how it already knows about *ssa.Call instructions.
If we see a store where the destination type is marked for reflection,
then we mark the source type as well, fixing the bug above.

This fixes obfuscating github.com/gogo/protobuf/proto.
A number of other Go modules fail with similar errors,
and they look like very similar bugs,
but this particular fix doesn't apply to them.
Future incremental fixes will try to deal with those extra cases.

Fixes #685.
@ezequiel-aguilera-uala
Copy link

Even though it's old and deprecated, "github.com/gogo/protobuf" is a dependency of the latest version of the official Docker package as of 08/2023: github.com/docker/docker v24.0.5+incompatible.

@mvdan
Copy link
Member

mvdan commented Aug 13, 2023

This is fixed in master, for what it's worth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@mvdan @uk0 @Techno-Fox @lu4p @eyevanovich @ezequiel-aguilera-uala and others