From c0731921c255fa84d52283a1a3d64c7e6980704f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 18 Nov 2020 17:02:06 +0000 Subject: [PATCH] rewrite go:linkname directives with garbled names (#200) If code includes a linkname directive pointing at a name in an imported package, like: //go:linkname localName importedpackage.RemoteName func localName() We should rewrite the comment to replace "RemoteName" with its obfuscated counterpart, if the package in question was obfuscated and that name was as well. We already had some code to handle linkname directives, but only to ensure that "localName" was never obfuscated. This behavior is kept, to ensure that the directive applies to the right name. In the future, we could instead rewrite "localName" in the directive, like we do with "RemoteName". Add plenty of tests, too. The linkname directive used to be tested in imports.txt and syntax.txt, but that was hard to maintain as each file tested different edge cases. Now that we have build caching, adding one extra testscript file isn't a big problem anymoree. Add linkname.txt, which is self-explanatory. The other two scripts also get a bit less complex. Fixes #197. --- line_obfuscator.go | 23 ++---------- main.go | 68 ++++++++++++++++++++++++++--------- testdata/scripts/imports.txt | 6 ---- testdata/scripts/linkname.txt | 64 +++++++++++++++++++++++++++++++++ testdata/scripts/syntax.txt | 51 +++++++------------------- 5 files changed, 131 insertions(+), 81 deletions(-) create mode 100644 testdata/scripts/linkname.txt diff --git a/line_obfuscator.go b/line_obfuscator.go index 4972a98..77c24a3 100644 --- a/line_obfuscator.go +++ b/line_obfuscator.go @@ -39,25 +39,6 @@ func isDirective(text string, directives []string) bool { return false } -// TODO(mvdan): replace getLocalName with proper support for go:linkname - -func getLocalName(text string) (string, bool) { - if !strings.HasPrefix(text, "//go:linkname") { - return "", false - } - parts := strings.Fields(text) - if len(parts) < 2 { - return "", false - } - - name := strings.TrimSpace(parts[1]) - if len(name) == 0 { - return "", false - } - - return name, true -} - func prependComment(group *ast.CommentGroup, comment *ast.Comment) *ast.CommentGroup { if group == nil { return &ast.CommentGroup{List: []*ast.Comment{comment}} @@ -68,6 +49,8 @@ func prependComment(group *ast.CommentGroup, comment *ast.Comment) *ast.CommentG } // Remove all comments from CommentGroup except //go: directives. +// go:linkname directives are removed, since they're collected and rewritten +// separately. func clearCommentGroup(group *ast.CommentGroup) *ast.CommentGroup { if group == nil { return nil @@ -75,7 +58,7 @@ func clearCommentGroup(group *ast.CommentGroup) *ast.CommentGroup { var comments []*ast.Comment for _, comment := range group.List { - if strings.HasPrefix(comment.Text, "//go:") { + if strings.HasPrefix(comment.Text, "//go:") && !strings.HasPrefix(comment.Text, "//go:linkname") { comments = append(comments, &ast.Comment{Text: comment.Text}) } } diff --git a/main.go b/main.go index 10f7526..3fe70dc 100644 --- a/main.go +++ b/main.go @@ -438,20 +438,55 @@ func transformCompile(args []string) ([]string, error) { detachedComments := make([][]string, len(files)) for i, file := range files { - for _, group := range file.Comments { - for _, comment := range group.List { - if name, _ := getLocalName(comment.Text); name != "" { - obj := tf.pkg.Scope().Lookup(name) - if obj != nil { - tf.blacklist[obj] = struct{}{} - } - } + name := filepath.Base(filepath.Clean(paths[i])) + cgoFile := strings.HasPrefix(name, "_cgo_") + comments, file := transformLineInfo(file, cgoFile) + + for i, comment := range comments { + if !strings.HasPrefix(comment, "//go:linkname ") { + continue + } + fields := strings.Fields(comment) + if len(fields) != 3 { + continue + } + // This directive has two arguments: "go:linkname localName newName" + localName := fields[1] + + // The local name must not be obfuscated. + obj := tf.pkg.Scope().Lookup(localName) + if obj != nil { + tf.blacklist[obj] = struct{}{} } + + // If the new name is of the form "pkgpath.Name", and + // we've obfuscated "Name" in that package, rewrite the + // directive to use the obfuscated name. + newName := strings.Split(fields[2], ".") + if len(newName) != 2 { + continue + } + pkg, name := newName[0], newName[1] + if pkg == "runtime" && strings.HasPrefix(name, "cgo") { + continue // ignore cgo-generated linknames + } + listedPkg, ok := buildInfo.imports[pkg] + if !ok { + continue // probably a made up symbol name + } + garbledPkg, _ := garbledImport(pkg) + if garbledPkg != nil && garbledPkg.Scope().Lookup(name) != nil { + continue // the name exists and was not garbled + } + + // The name exists and was obfuscated; replace the + // comment with the obfuscated name. + obfName := hashWith(listedPkg.actionID, name) + fields[2] = pkg + "." + obfName + comments[i] = strings.Join(fields, " ") } + detachedComments[i], files[i] = comments, file - name := filepath.Base(filepath.Clean(paths[i])) - cgoFile := strings.HasPrefix(name, "_cgo_") - detachedComments[i], files[i] = transformLineInfo(file, cgoFile) } obfSrcArchive := &bytes.Buffer{} @@ -507,8 +542,7 @@ func transformCompile(args []string) ([]string, error) { obfSrc := &bytes.Buffer{} printWriter := io.MultiWriter(tempFile, obfSrc) - fileDetachedComments := detachedComments[i] - for _, comment := range fileDetachedComments { + for _, comment := range detachedComments[i] { if _, err := printWriter.Write([]byte(comment + "\n")); err != nil { return nil, err } @@ -896,7 +930,6 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { // if the struct of this field was not garbled, do not garble // any of that struct's fields if (parentScope != tf.pkg.Scope()) && (x.IsField() && !x.Embedded()) { - // fmt.Println(node.Name) parent, ok := cursor.Parent().(*ast.SelectorExpr) if !ok { break @@ -974,7 +1007,8 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { actionID = id } - // The exported names cannot be shortened as counter synchronization between packages is not currently implemented + // The exported names cannot be shortened as counter synchronization + // between packages is not currently implemented if token.IsExported(node.Name) { node.Name = hashWith(actionID, node.Name) return true @@ -1108,7 +1142,9 @@ func transformLink(args []string) ([]string, error) { pkgPath = buildInfo.firstImport } if id := buildInfo.imports[pkgPath].actionID; len(id) > 0 { - // If the name is not in the map file, it means that the name was not obfuscated or is public + // We use privateNameMap because unexported names are obfuscated + // to short names like "A", "B", "C" etc, which is not reproducible + // here. If the name isn't in the map, a hash will do. newName, ok := privateNameMap[pkg+"."+name] if !ok { newName = hashWith(id, name) diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index cc5163d..f8bdd97 100644 --- a/testdata/scripts/imports.txt +++ b/testdata/scripts/imports.txt @@ -62,16 +62,12 @@ import ( "fmt" "strings" "reflect" - _ "unsafe" "test/main/imported" "rsc.io/quote" ) -//go:linkname linkedPrintln fmt.Println -func linkedPrintln(a ...interface{}) (n int, err error) - func main() { fmt.Println(imported.ImportedVar) fmt.Println(imported.ImportedConst) @@ -93,7 +89,6 @@ func main() { fmt.Println("method not found") } - linkedPrintln(nil) fmt.Println(quote.Go()) } @@ -180,5 +175,4 @@ ReflectTypeOf ReflectTypeOfIndirect ReflectValueOf{ExportedField:"abc", unexportedField:""} [method: abc] - Don't communicate by sharing memory, share memory by communicating. diff --git a/testdata/scripts/linkname.txt b/testdata/scripts/linkname.txt new file mode 100644 index 0000000..c5a7914 --- /dev/null +++ b/testdata/scripts/linkname.txt @@ -0,0 +1,64 @@ +env GOPRIVATE=test/main + +garble build +exec ./main +cmp stderr main.stderr + +! binsubstr main$exe 'garbledFunc' 'GarbledFunc' + +[short] stop # no need to verify this with -short + +go build +exec ./main +cmp stderr main.stderr + +-- go.mod -- +module test/main + +go 1.15 +-- main.go -- +package main + +import ( + _ "strings" + _ "unsafe" + + _ "test/main/imported" +) + +// A linkname to an external non-garbled func. +//go:linkname byteIndex strings.IndexByte +func byteIndex(s string, c byte) int + +// A linkname to an external garbled func. +//go:linkname garbledFunc test/main/imported.GarbledFuncImpl +func garbledFunc() string + +// A linkname to an entirely made up name, implemented elsewhere. +//go:linkname renamedFunc madeup.newName +func renamedFunc() string + +func main() { + println(byteIndex("01234", '3')) + println(garbledFunc()) + println(renamedFunc()) +} +-- imported/imported.go -- +package imported + +import ( + _ "unsafe" +) + +func GarbledFuncImpl() string { + return "garbled func" +} + +//go:linkname renamedFunc madeup.newName +func renamedFunc() string { + return "renamed func" +} +-- main.stderr -- +3 +garbled func +renamed func diff --git a/testdata/scripts/syntax.txt b/testdata/scripts/syntax.txt index 56c64f7..48091e4 100644 --- a/testdata/scripts/syntax.txt +++ b/testdata/scripts/syntax.txt @@ -1,16 +1,15 @@ env GOPRIVATE='test/main,private.source/*' -garble build -tags directives +garble build exec ./main$exe cmp stderr main.stderr -! binsubstr main$exe 'localName' 'globalConst' 'globalVar' 'globalType' 'valuable information' 'private.source' 'remoteIntReturn' 'intReturn' -binsubstr main$exe 'magicFunc' +! binsubstr main$exe 'localName' 'globalConst' 'globalVar' 'globalType' 'valuable information' 'private.source' 'remoteIntReturn' 'intReturn' 'neverInlined' [short] stop # no need to verify this with -short # Check that the program works as expected without garble. -go build -tags directives +go build exec ./main$exe cmp stderr main.stderr @@ -80,6 +79,13 @@ type EmbeddingUniverseScope struct { string } +// TODO: test that go:noinline still works without using debugdir + +//go:noinline +func neverInlined() { + println("This func is never inlined.") +} + func main() { switch V := V.(type) { case int: @@ -94,7 +100,7 @@ func main() { scopesTest() println(extra.Func()) sub.Test() - sub.TestDirectives() + neverInlined() } -- scopes.go -- @@ -143,43 +149,10 @@ func Test() { } func noop(...interface{}) {} - --- sub/directives.go -- -// +build directives - -package sub - -import ( - _ "unsafe" - _ "test/main/sub/a" -) - -//go:linkname remoteIntReturn a.magicFunc - -func remoteIntReturn() int - -//go:noinline -func TestDirectives() { - if remoteIntReturn() != 42 { - panic("invalid result") - } -} --- sub/a/directives.go -- -// +build directives - -package a - -import _ "unsafe" - -//go:linkname intReturn a.magicFunc - -//go:noinline -func intReturn() int { - return 42 -} -- main.stderr -- nil case {"Foo":3} 1 1 1 1 4 5 1 input This is a separate module to obfuscate. +This func is never inlined.