From 1682e8ee1034193359ead5c8de254c9d07eae72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 31 Aug 2021 11:52:50 +0100 Subject: [PATCH] always require one argument for "reverse" The "reverse" command had many levels of optional arguments: garble [garble flags] reverse [build flags] [package] [files] This was pretty confusing, and could easily lead to people running the command incorrectly: # note that output.txt isn't a Go package! garble reverse output.txt Moreover, it made the handling of Go build flags pretty confusing. Should the command below work? garble reverse -tags=mytag It also made it easy to not notice that one must supply the main package to properly reverse some text that it produced, like a panic message. With the package path being implicit, one could mistakenly provide the wrong package by running garble in a directory containing a different package. See #394. --- main.go | 29 +++++++++++++++++++---------- reverse.go | 23 ++++++++++++++--------- testdata/scripts/help.txt | 18 ++++++++++++++---- testdata/scripts/reverse.txt | 8 ++++---- 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/main.go b/main.go index 3ee0970..6ae9c95 100644 --- a/main.go +++ b/main.go @@ -76,10 +76,12 @@ Similarly, to combine garble flags and Go build flags: The following commands are supported: - build [packages] replace "go build" - test [packages] replace "go test" - version print Garble version - reverse [files] de-obfuscate output such as stack traces + build replace "go build" + test replace "go test" + version print Garble version + reverse de-obfuscate output such as stack traces + +To learn more about a command, run "garble help ". garble accepts the following flags before a command: @@ -307,14 +309,19 @@ func mainErr(args []string) error { // If we recognize an argument, we're not running within -toolexec. switch command, args := args[0], args[1:]; command { case "help": - if len(args) > 0 { - return fmt.Errorf("the help command does not take arguments") + if hasHelpFlag(args) || len(args) > 1 { + fmt.Fprintf(os.Stderr, "usage: garble help [command]\n") + return errJustExit(2) + } + if len(args) == 1 { + return mainErr([]string{args[0], "-h"}) } usage() return errJustExit(2) case "version": - if len(args) > 0 { - return fmt.Errorf("the version command does not take arguments") + if hasHelpFlag(args) || len(args) > 0 { + fmt.Fprintf(os.Stderr, "usage: garble version\n") + return errJustExit(2) } // don't overwrite the version if it was set by -ldflags=-X if info, ok := debug.ReadBuildInfo(); ok && version == "(devel)" { @@ -1023,8 +1030,10 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) { return newCfg.Name(), nil } -type funcFullName = string -type reflectParameterPosition = int +type ( + funcFullName = string + reflectParameterPosition = int +) // knownReflectAPIs is a static record of what std APIs use reflection on their // parameters, so we can avoid obfuscating types used with them. diff --git a/reverse.go b/reverse.go index edf502f..609b642 100644 --- a/reverse.go +++ b/reverse.go @@ -19,24 +19,29 @@ import ( // commandReverse implements "garble reverse". func commandReverse(args []string) error { flags, args := splitFlagsFromArgs(args) - if hasHelpFlag(flags) { - fmt.Fprintf(os.Stderr, "usage: garble [garble flags] reverse [files]\n") - return errJustExit(2) - } + if hasHelpFlag(flags) || len(args) == 0 { + fmt.Fprintf(os.Stderr, ` +usage: garble [garble flags] reverse [build flags] package [files] + +For example, after building an obfuscated program as follows: - mainPkg := "." - if len(args) > 0 { - mainPkg = args[0] - args = args[1:] + garble -literals build -tags=mytag ./cmd/mycmd + +One can reverse a captured panic stack trace as follows: + + garble -literals reverse -tags=mytag ./cmd/mycmd panic-output.txt +`[1:]) + return errJustExit(2) } + pkg, args := args[0], args[1:] listArgs := []string{ "-json", "-deps", "-export", } listArgs = append(listArgs, flags...) - listArgs = append(listArgs, mainPkg) + listArgs = append(listArgs, pkg) // TODO: We most likely no longer need this "list -toolexec" call, since // we use the original build IDs. if _, err := toolexecCmd("list", listArgs); err != nil { diff --git a/testdata/scripts/help.txt b/testdata/scripts/help.txt index 76c6774..916cc39 100644 --- a/testdata/scripts/help.txt +++ b/testdata/scripts/help.txt @@ -15,7 +15,12 @@ stderr 'garble \[garble flags\] command' ! stdout . ! garble help foo bar -stderr 'does not take arguments' +stderr 'usage: garble help' +! stderr 'Garble obfuscates Go code' +! stdout . + +! garble help -h +stderr 'usage: garble help' ! stdout . ! garble build -h @@ -35,7 +40,12 @@ stderr 'Run .go help test.' ! stdout . ! garble reverse -h -stderr 'garble \[garble flags\] reverse \[files\]' +stderr 'garble \[garble flags\] reverse \[build flags\] package \[files\]' +! stderr 'usage: go ' +! stdout . + +! garble help reverse +stderr 'garble \[garble flags\] reverse \[build flags\] package \[files\]' ! stderr 'usage: go ' ! stdout . @@ -59,10 +69,10 @@ garble version stdout 'devel|^v0' ! garble version -flag -stderr 'does not take arguments' +stderr 'usage: garble version' ! garble version arg -stderr 'does not take arguments' +stderr 'usage: garble version' # We need a dummy module for "garble build -badflag". -- go.mod -- diff --git a/testdata/scripts/reverse.txt b/testdata/scripts/reverse.txt index ed37534..3ee82b6 100644 --- a/testdata/scripts/reverse.txt +++ b/testdata/scripts/reverse.txt @@ -1,7 +1,7 @@ env GOPRIVATE=test/main # Unknown build flags should result in errors. -! garble reverse -badflag +! garble reverse -badflag=foo . stderr 'flag provided but not defined' garble build @@ -16,7 +16,7 @@ grep 'goroutine 1 \[running\]' main.stderr ! grep 'ExportedLib(Type|Field)|unexportedMainFunc|test/main|main\.go|lib\.go' main.stderr stdin main.stderr -garble reverse +garble reverse . cmp stdout reverse.stdout ! garble build ./build-error @@ -39,12 +39,12 @@ exec ./main cp stderr main-literals.stderr stdin main-literals.stderr -garble -literals reverse +garble -literals reverse . cmp stdout reverse.stdout # Reversing a -literals output without the flag should fail. stdin main-literals.stderr -! garble reverse +! garble reverse . cmp stdout main-literals.stderr -- go.mod --