diff --git a/README.md b/README.md index 7164096..e800482 100644 --- a/README.md +++ b/README.md @@ -107,12 +107,14 @@ Most of these can improve with time and effort. The purpose of this section is to document the current shortcomings of this tool. * Exported methods are never obfuscated at the moment, since they could - be required by interfaces and reflection. This area is a work in progress. + be required by interfaces and reflection. This area is a work in progress; see + [#3](https://github.com/burrowers/garble/issues/3). * It can be hard for garble to know what types will be used with - [reflection](https://golang.org/pkg/reflect), including JSON encoding or - decoding. If your program breaks because a type's names are obfuscated when - they should not be, you can add an explicit hint: + [reflection](https://golang.org/pkg/reflect). + Its detection will improve over time with [#162](https://github.com/burrowers/garble/issues/162) + Until then, if your program breaks due to the obfuscation of field names, + you can add an explicit hint: ```go type Message struct { Command string diff --git a/main.go b/main.go index 4dbe987..7ea087d 100644 --- a/main.go +++ b/main.go @@ -1014,6 +1014,60 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) { return newCfg.Name(), nil } +type knownReflect struct { + pkgPath string + name string +} + +type funcFullName = string + +// knownReflectAPIs is a static record of what std APIs use reflection on their +// parameters, so we can avoid obfuscating types used with them. +// +// For now, this table is manually maintained, until we automatically detect and +// propagate the uses of reflect in std and third party APIs. +// If we end up maintaining this table for a long time, we should probably +// improve the process via either code generation or sanity checks. +// +// TODO: record which parameters get used with reflection, as it's often not all +// of them. +// +// TODO: we're not including fmt.Printf, as it would have many false positives, +// unless we were smart enough to detect which arguments get used as %#v or %T. +// +// TODO: this should also support third party APIs; see +// https://github.com/burrowers/garble/issues/162 +var knownReflectAPIs = map[funcFullName]bool{ + "reflect.TypeOf": true, + "reflect.ValueOf": true, + + "encoding/json.Marshal": true, + "encoding/json.MarshalIndent": true, + "encoding/json.Unmarshal": true, + "(*encoding/json.Decoder).Decode": true, + "(*encoding/json.Encoder).Encode": true, + + "encoding/gob.Register": true, + "encoding/gob.RegisterName": true, + "(*encoding/gob.Decoder).Decode": true, + "(*encoding/gob.Encoder).Encode": true, + + "encoding/xml.Marshal": true, + "encoding/xml.MarshalIndent": true, + "encoding/xml.Unmarshal": true, + "(*encoding/xml.Decoder).Decode": true, + "(*encoding/xml.Encoder).Encode": true, + "(*encoding/xml.Decoder).DecodeElement": true, + "(*encoding/xml.Encoder).EncodeElement": true, + + "(*text/template.Template).Execute": true, + + "(*html/template.Template).Execute": true, + + "net/rpc.Register": true, + "net/rpc.RegisterName": true, +} + // prefillIgnoreObjects collects objects which should not be obfuscated, // such as those used as arguments to reflect.TypeOf or reflect.ValueOf. // Since we obfuscate one package at a time, we only detect those if the type @@ -1034,13 +1088,14 @@ func (tf *transformer) prefillIgnoreObjects(files []*ast.File) { if !ok { return true } - fnType := tf.info.ObjectOf(sel.Sel) - - if fnType.Pkg() == nil { + fnType, _ := tf.info.ObjectOf(sel.Sel).(*types.Func) + if fnType == nil || fnType.Pkg() == nil { return true } - if fnType.Pkg().Path() == "reflect" && (fnType.Name() == "TypeOf" || fnType.Name() == "ValueOf") { + fullName := fnType.FullName() + // log.Printf("%s: %s", fset.Position(node.Pos()), fullName) + if knownReflectAPIs[fullName] { for _, arg := range call.Args { argType := tf.info.TypeOf(arg) tf.recordIgnore(argType, tf.pkg.Path()) diff --git a/testdata/scripts/reflect.txt b/testdata/scripts/reflect.txt index 90ce4a6..78cd629 100644 --- a/testdata/scripts/reflect.txt +++ b/testdata/scripts/reflect.txt @@ -24,8 +24,10 @@ package main import ( "encoding/json" "fmt" + "os" "reflect" "strings" + "text/template" "test/main/importedpkg" "test/main/importedpkg2" @@ -51,16 +53,19 @@ func main() { } // Use of a common library using reflect, encoding/json. - // TODO: remove the TypeOf hint once we detect json.Marshal. - _ = reflect.TypeOf(EncodingT{}) enc, _ := json.Marshal(EncodingT{Foo: 3}) - println(string(enc)) + fmt.Println(string(enc)) + + // Another common library, text/template. + tmpl := template.Must(template.New("").Parse("Hello {{.Name}}.")) + _ = tmpl.Execute(os.Stdout, struct{Name string}{Name: "Dave"}) + fmt.Println() // Always print a newline. // Another complex case, involving embedding and another package. outer := &importedpkg.EmbeddingOuter{} outer.InnerField = 3 enc, _ = json.Marshal(outer) - println(string(enc)) + fmt.Println(string(enc)) // An edge case; the struct type is defined in a different package. // Note that the struct type is unnamed, but it still has named fields. @@ -224,6 +229,9 @@ ReflectTypeOf ReflectTypeOfIndirect ReflectValueOf{ExportedField:"abc", unexportedField:""} [method: abc] +{"Foo":3} +Hello Dave. +{"InnerField":3,"Anon":{"AnonField":0}} {downstream} {sibling} {{indirect-with} {indirect-without} {}}