conversion between struct types from different packages breaks

[dsnet]

Reproduction:

$ git clone https://go.googlesource.com/protobuf
$ cd protobuf/cmd/protoc-gen-go
$ garble build
DkrLcCpi.go:1: cannot use m.ProtoMethods() (type *struct { jumseyKf.HPJylc36; Ua060WY0 uint64; LEKZ3V0R func(struct { jumseyKf.HPJylc36; S620HKVo Uay7rngJ.S620HKVo; Ua060WY0 uint8 }) struct { jumseyKf.HPJylc36; LEKZ3V0R int }; JNbzrZiu func(struct { jumseyKf.HPJylc36; S620HKVo Uay7rngJ.S620HKVo; L4MzpBON []byte; Ua060WY0 uint8 }) (struct { jumseyKf.HPJylc36; L4MzpBON []byte }, error); J5afwwtO func(struct { jumseyKf.HPJylc36; S620HKVo Uay7rngJ.S620HKVo; L4MzpBON []byte; Ua060WY0 uint8; EePY1yG1 interface { FindExtensionByName(Uay7rngJ.DRlssGtN) (Uay7rngJ.MsEDzXI9, error); FindExtensionByNumber(Uay7rngJ.DRlssGtN, Dv5Rwe3w.Y3l8zppN) (Uay7rngJ.MsEDzXI9, error) } }) (struct { jumseyKf.HPJylc36; Ua060WY0 uint8 }, error); ZLLLzJFW func(struct { jumseyKf.HPJylc36; NTu9CQ_i Uay7rngJ.S620HKVo; OkxSwWgX Uay7rngJ.S620HKVo }) struct { jumseyKf.HPJylc36; Ua060WY0 uint8 }; T6X4Da5i func(struct { jumseyKf.HPJylc36; S620HKVo Uay7rngJ.S620HKVo }) (struct { jumseyKf.HPJylc36 }, error) }) as type *struct { jumseyKf.HPJylc36; Pn4K9ozd uint64; Zovpk5js func(struct { jumseyKf.HPJylc36; ASrqYrjP Uay7rngJ.S620HKVo; Pn4K9ozd uint8 }) struct { jumseyKf.HPJylc36; Zovpk5js int }; N3mROQGv func(struct { jumseyKf.HPJylc36; ASrqYrjP Uay7rngJ.S620HKVo; DUmLpA_6 []byte; Pn4K9ozd uint8 }) (struct { jumseyKf.HPJylc36; DUmLpA_6 []byte }, error); CufZlXhn func(struct { jumseyKf.HPJylc36; ASrqYrjP Uay7rngJ.S620HKVo; DUmLpA_6 []byte; Pn4K9ozd uint8; L6pqAGXN interface { FindExtensionByName(Uay7rngJ.DRlssGtN) (Uay7rngJ.MsEDzXI9, error); FindExtensionByNumber(Uay7rngJ.DRlssGtN, Dv5Rwe3w.Y3l8zppN) (Uay7rngJ.MsEDzXI9, error) } }) (struct { jumseyKf.HPJylc36; Pn4K9ozd uint8 }, error); ZegXs_lp func(struct { jumseyKf.HPJylc36; XjQ_oB6v Uay7rngJ.S620HKVo; YWo0sUBi Uay7rngJ.S620HKVo }) struct { jumseyKf.HPJylc36; Pn4K9ozd uint8 }; QCHUzV4r func(struct { jumseyKf.HPJylc36; ASrqYrjP Uay7rngJ.S620HKVo }) (struct { jumseyKf.HPJylc36 }, error) } in return argument

This occurs on protobuf@v1.26.0, but I doubt the exact version matters.

[mvdan]

Tricky one. We hash field names depending on the package defining the type. Conversions from T1 to T2 will then break, if the two types are defined in different packages, as fields with the same name will end up having different names after obfuscation.

It was obvious we'd have this issue with exported method names, but I never thought about this edge case with exported field names.

[mvdan]

Simpler repro:

$ cat f.go
package main

import (
	"fmt"
	"go/token"
)

type MyPosition struct {
	Filename string
	Offset   int
	Line     int
	Column   int
}

func main() {
	pos := token.Position{Filename: "foo.go"}
	fmt.Printf("%T\n", pos)

	myPos := MyPosition(pos)
	fmt.Printf("%T\n", myPos)
}
$ go run f.go
token.Position
main.MyPosition
$ garble build f.go
# command-line-arguments
qFYM5dnF.go:1: cannot convert pos (type token.Position) to type MyPosition

This example is even worse, because token.Position is not obfuscated yet MyPosition is.

[capnspacehook]

We could treat this like we treat reflected types, obfuscate exported struct fields by default unless the parent struct type is converted to/from another type...

[mvdan]

We could, but it wouldn't catch all cases. A package A can convert from exported types B1.T1 and B2.T2, for example. The protobuf case is an example of what we wouldn't catch naively, because one of the types is declared in a dependency.

I think a first step here could be to use a global salt for exported field names, similar to what is proposed for methods in #3. Then the only remaining issue would be converting from a non-obfuscated type to an obfuscated type, but that's a bit of an open question in itself.

[dsnet]

Could the field name be a hash of information from the field and the type itself? If the entropy there is too low, perhaps you can include the other fields in the struct since they need to be identical in order for a conversion to work, right?

[mvdan]

you can include the other fields in the struct since they need to be identical in order for a conversion to work

That's a good point :) I'll give that a try.