diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5fbe70a..c74f759 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,9 +59,9 @@ run `go mod tidy` to minimize such changes. - use `./internal/atomicx` rather than `atomic/sync` -- use `./internal/fsx.OpenFile` when you need to open a file +- do not use `os/exec`, use `x/sys/execabs` or `./internal/shellx` -Do now use `os/exec`, use `x/sys/execabs`. +- use `./internal/fsx.OpenFile` when you need to open a file ## Code testing requirements diff --git a/cmd/ooniprobe/internal/autorun/autorun_darwin.go b/cmd/ooniprobe/internal/autorun/autorun_darwin.go index 4f2499b..547d586 100644 --- a/cmd/ooniprobe/internal/autorun/autorun_darwin.go +++ b/cmd/ooniprobe/internal/autorun/autorun_darwin.go @@ -80,12 +80,12 @@ func (managerDarwin) LogShow() error { if major < 20 /* macOS 11.0 Big Sur */ { return errNotImplemented } - return shellx.Run("log", "show", "--info", "--debug", + return shellx.Run(log.Log, "log", "show", "--info", "--debug", "--process", "ooniprobe", "--style", "compact") } func (managerDarwin) LogStream() error { - return shellx.Run("log", "stream", "--style", "compact", "--level", + return shellx.Run(log.Log, "log", "stream", "--style", "compact", "--level", "debug", "--process", "ooniprobe") } diff --git a/internal/cmd/jafar/iptables/iptables_integration_test.go b/internal/cmd/jafar/iptables/iptables_integration_test.go index 659fec2..12e2adb 100644 --- a/internal/cmd/jafar/iptables/iptables_integration_test.go +++ b/internal/cmd/jafar/iptables/iptables_integration_test.go @@ -291,7 +291,7 @@ func TestHijackHTTP(t *testing.T) { if err := policy.Apply(); err != nil { t.Fatal(err) } - err = shellx.Run("sudo", "-u", "nobody", "--", + err = shellx.Run(log.Log, "sudo", "-u", "nobody", "--", "curl", "-sf", "http://example.com") if err == nil { t.Fatal("expected an error here") @@ -330,7 +330,7 @@ func TestHijackHTTPS(t *testing.T) { if err := policy.Apply(); err != nil { t.Fatal(err) } - err = shellx.Run("sudo", "-u", "nobody", "--", + err = shellx.Run(log.Log, "sudo", "-u", "nobody", "--", "curl", "-sf", "https://example.com") if err == nil { t.Fatal("expected an error here") diff --git a/internal/cmd/jafar/iptables/iptables_linux.go b/internal/cmd/jafar/iptables/iptables_linux.go index 1664a66..1cd034d 100644 --- a/internal/cmd/jafar/iptables/iptables_linux.go +++ b/internal/cmd/jafar/iptables/iptables_linux.go @@ -3,6 +3,7 @@ package iptables import ( + "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/shellx" ) @@ -15,55 +16,56 @@ func (s *linuxShell) createChains() (err error) { // JUST KNOW WE'VE BEEN HERE } }() - err = shellx.Run("sudo", "iptables", "-N", "JAFAR_INPUT") + err = shellx.Run(log.Log, "sudo", "iptables", "-N", "JAFAR_INPUT") runtimex.PanicOnError(err, "cannot create JAFAR_INPUT chain") - err = shellx.Run("sudo", "iptables", "-N", "JAFAR_OUTPUT") + err = shellx.Run(log.Log, "sudo", "iptables", "-N", "JAFAR_OUTPUT") runtimex.PanicOnError(err, "cannot create JAFAR_OUTPUT chain") - err = shellx.Run("sudo", "iptables", "-t", "nat", "-N", "JAFAR_NAT_OUTPUT") + err = shellx.Run(log.Log, "sudo", "iptables", "-t", "nat", "-N", "JAFAR_NAT_OUTPUT") runtimex.PanicOnError(err, "cannot create JAFAR_NAT_OUTPUT chain") - err = shellx.Run("sudo", "iptables", "-I", "OUTPUT", "-j", "JAFAR_OUTPUT") + err = shellx.Run(log.Log, "sudo", "iptables", "-I", "OUTPUT", "-j", "JAFAR_OUTPUT") runtimex.PanicOnError(err, "cannot insert jump to JAFAR_OUTPUT") - err = shellx.Run("sudo", "iptables", "-I", "INPUT", "-j", "JAFAR_INPUT") + err = shellx.Run(log.Log, "sudo", "iptables", "-I", "INPUT", "-j", "JAFAR_INPUT") runtimex.PanicOnError(err, "cannot insert jump to JAFAR_INPUT") - err = shellx.Run("sudo", "iptables", "-t", "nat", "-I", "OUTPUT", "-j", "JAFAR_NAT_OUTPUT") + err = shellx.Run(log.Log, "sudo", "iptables", "-t", "nat", "-I", "OUTPUT", "-j", "JAFAR_NAT_OUTPUT") runtimex.PanicOnError(err, "cannot insert jump to JAFAR_NAT_OUTPUT") return nil } func (s *linuxShell) dropIfDestinationEquals(ip string) error { - return shellx.Run("sudo", "iptables", "-A", "JAFAR_OUTPUT", "-d", ip, "-j", "DROP") + return shellx.Run(log.Log, + "sudo", "iptables", "-A", "JAFAR_OUTPUT", "-d", ip, "-j", "DROP") } func (s *linuxShell) rstIfDestinationEqualsAndIsTCP(ip string) error { - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-A", "JAFAR_OUTPUT", "--proto", "tcp", "-d", ip, "-j", "REJECT", "--reject-with", "tcp-reset", ) } func (s *linuxShell) dropIfContainsKeywordHex(keyword string) error { - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-A", "JAFAR_OUTPUT", "-m", "string", "--algo", "kmp", "--hex-string", keyword, "-j", "DROP", ) } func (s *linuxShell) dropIfContainsKeyword(keyword string) error { - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-A", "JAFAR_OUTPUT", "-m", "string", "--algo", "kmp", "--string", keyword, "-j", "DROP", ) } func (s *linuxShell) rstIfContainsKeywordHexAndIsTCP(keyword string) error { - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-A", "JAFAR_OUTPUT", "-m", "string", "--proto", "tcp", "--algo", "kmp", "--hex-string", keyword, "-j", "REJECT", "--reject-with", "tcp-reset", ) } func (s *linuxShell) rstIfContainsKeywordAndIsTCP(keyword string) error { - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-A", "JAFAR_OUTPUT", "-m", "string", "--proto", "tcp", "--algo", "kmp", "--string", keyword, "-j", "REJECT", "--reject-with", "tcp-reset", ) @@ -73,7 +75,7 @@ func (s *linuxShell) hijackDNS(address string) error { // Hijack any DNS query, like the Vodafone station does when using the // secure network feature. Our transparent proxies will use DoT, in order // to bypass this restriction and avoid routing loop. - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-t", "nat", "-A", "JAFAR_NAT_OUTPUT", "-p", "udp", "--dport", "53", "-j", "DNAT", "--to", address, ) @@ -82,7 +84,7 @@ func (s *linuxShell) hijackDNS(address string) error { func (s *linuxShell) hijackHTTPS(address string) error { // We need to whitelist root otherwise the traffic sent by Jafar // itself will match the rule and loop. - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-t", "nat", "-A", "JAFAR_NAT_OUTPUT", "-p", "tcp", "--dport", "443", "-m", "owner", "!", "--uid-owner", "0", "-j", "DNAT", "--to", address, @@ -92,7 +94,7 @@ func (s *linuxShell) hijackHTTPS(address string) error { func (s *linuxShell) hijackHTTP(address string) error { // We need to whitelist root otherwise the traffic sent by Jafar // itself will match the rule and loop. - return shellx.Run( + return shellx.Run(log.Log, "sudo", "iptables", "-t", "nat", "-A", "JAFAR_NAT_OUTPUT", "-p", "tcp", "--dport", "80", "-m", "owner", "!", "--uid-owner", "0", "-j", "DNAT", "--to", address, diff --git a/internal/cmd/jafar/main.go b/internal/cmd/jafar/main.go index f7b1941..87b4776 100644 --- a/internal/cmd/jafar/main.go +++ b/internal/cmd/jafar/main.go @@ -276,7 +276,7 @@ func main() { policy := iptablesStart() var err error if *mainCommand != "" { - err = shellx.RunCommandline(fmt.Sprintf( + err = shellx.RunCommandline(log.Log, fmt.Sprintf( "sudo -u '%s' -- %s", *mainUser, *mainCommand, )) } else { diff --git a/internal/cmd/jafar/main_test.go b/internal/cmd/jafar/main_test.go index 6d4ac62..a8c3223 100644 --- a/internal/cmd/jafar/main_test.go +++ b/internal/cmd/jafar/main_test.go @@ -6,6 +6,7 @@ import ( "runtime" "testing" + "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/cmd/jafar/iptables" "github.com/ooni/probe-cli/v3/internal/shellx" ) @@ -74,7 +75,7 @@ func TestMustx(t *testing.T) { called int exitcode int ) - err := shellx.Run("curl", "-sf", "") // cause exitcode == 3 + err := shellx.Run(log.Log, "curl", "-sf", "") // cause exitcode == 3 mustx(err, "", func(ec int) { called++ exitcode = ec diff --git a/internal/shellx/shellx.go b/internal/shellx/shellx.go index 94e6cd4..50ccf13 100644 --- a/internal/shellx/shellx.go +++ b/internal/shellx/shellx.go @@ -6,11 +6,15 @@ import ( "os" "strings" - "github.com/apex/log" "github.com/google/shlex" "golang.org/x/sys/execabs" ) +// Logger is the logger expected by this package. +type Logger interface { + Infof(format string, v ...interface{}) +} + // runconfig is the configuration for run. type runconfig struct { // args contains the command line arguments. @@ -43,12 +47,12 @@ func run(config runconfig) error { return err } -// Run executes the specified command with the specified args -func Run(name string, arg ...string) error { +// Run executes the specified command with the specified args. +func Run(logger Logger, name string, arg ...string) error { return run(runconfig{ args: arg, command: name, - loginfof: log.Log.Infof, + loginfof: logger.Infof, stdout: os.Stdout, stderr: os.Stderr, }) @@ -68,15 +72,17 @@ func RunQuiet(name string, arg ...string) error { }) } -// RunCommandline is like Run but its only argument is a command -// line that will be splitted using the google/shlex package. -func RunCommandline(cmdline string) error { +// ErrNoCommandToExecute means that the command line is empty. +var ErrNoCommandToExecute = errors.New("shellx: no command to execute") + +// RunCommandline executes the given command line. +func RunCommandline(logger Logger, cmdline string) error { args, err := shlex.Split(cmdline) if err != nil { return err } if len(args) < 1 { - return errors.New("shellx: no command to execute") + return ErrNoCommandToExecute } - return Run(args[0], args[1:]...) + return Run(logger, args[0], args[1:]...) } diff --git a/internal/shellx/shellx_test.go b/internal/shellx/shellx_test.go index 607649a..42ac008 100644 --- a/internal/shellx/shellx_test.go +++ b/internal/shellx/shellx_test.go @@ -1,12 +1,16 @@ package shellx -import "testing" +import ( + "testing" + + "github.com/apex/log" +) func TestRun(t *testing.T) { - if err := Run("whoami"); err != nil { + if err := Run(log.Log, "whoami"); err != nil { t.Fatal(err) } - if err := Run("./nonexistent/command"); err == nil { + if err := Run(log.Log, "./nonexistent/command"); err == nil { t.Fatal("expected an error here") } } @@ -22,22 +26,22 @@ func TestRunQuiet(t *testing.T) { func TestRunCommandline(t *testing.T) { t.Run("when the command does not parse", func(t *testing.T) { - if err := RunCommandline(`"foobar`); err == nil { + if err := RunCommandline(log.Log, `"foobar`); err == nil { t.Fatal("expected an error here") } }) t.Run("when we have no arguments", func(t *testing.T) { - if err := RunCommandline(""); err == nil { + if err := RunCommandline(log.Log, ""); err == nil { t.Fatal("expected an error here") } }) t.Run("when we have a single argument", func(t *testing.T) { - if err := RunCommandline("ls"); err != nil { + if err := RunCommandline(log.Log, "ls"); err != nil { t.Fatal(err) } }) t.Run("when we have more than one argument", func(t *testing.T) { - if err := RunCommandline("ls ."); err != nil { + if err := RunCommandline(log.Log, "ls ."); err != nil { t.Fatal(err) } })