commit adf41cd3adf6e3884626b6b7e4c7f131660343dd Author: David Fifield david@bamsoftware.com Date: Sat Feb 23 02:23:05 2019 -0700

Be more careful about terminating meek-client.

First close its stdin and send SIGTERM; and then kill it if that doesn't work. Set TOR_PT_EXIT_ON_STDIN_EOF=1 unconditionally in the subprocess. On Windows it's the same, except don't send SIGTERM.

https://bugs.torproject.org/15125 https://bugs.torproject.org/25613 --- meek-client-torbrowser/meek-client-torbrowser.go | 27 ++++++++++++++----- meek-client-torbrowser/terminate_other.go | 33 ++++++++++++++++++++++++ meek-client-torbrowser/terminate_windows.go | 30 +++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/meek-client-torbrowser/meek-client-torbrowser.go b/meek-client-torbrowser/meek-client-torbrowser.go index ce79193..0f295ff 100644 --- a/meek-client-torbrowser/meek-client-torbrowser.go +++ b/meek-client-torbrowser/meek-client-torbrowser.go @@ -35,16 +35,27 @@ import ( "regexp" "strings" "syscall" + "time" )

// This magic string is emitted by meek-http-helper. var helperAddrPattern = regexp.MustCompile(`^meek-http-helper: listen (127.0.0.1:\d+)$`)

+// How long to wait for child processes to exit gracefully before killing them. +const terminateTimeout = 2 * time.Second + func usage() { fmt.Fprintf(os.Stderr, "Usage: %s [meek-client-torbrowser args] -- meek-client [meek-client args]\n", os.Args[0]) flag.PrintDefaults() }

+// ptCmd is a *exec.Cmd augmented with an io.WriteCloser for its stdin, which we +// can close to instruct the PT subprocess to terminate. +type ptCmd struct { + *exec.Cmd + StdinCloser io.WriteCloser +} + // Log a call to os.Process.Kill. func logKill(p *os.Process) error { log.Printf("killing PID %d", p.Pid) @@ -293,14 +304,16 @@ func grepHelperAddr(r io.Reader) (string, error) { }

// Run meek-client and return its exec.Cmd. -func runMeekClient(helperAddr string, meekClientCommandLine []string) (cmd *exec.Cmd, err error) { +func runMeekClient(helperAddr string, meekClientCommandLine []string) (cmd *ptCmd, err error) { meekClientPath := meekClientCommandLine[0] args := meekClientCommandLine[1:] args = append(args, []string{"--helper", helperAddr}...) - cmd = exec.Command(meekClientPath, args...) + cmd = new(ptCmd) + cmd.Cmd = exec.Command(meekClientPath, args...) // Give the subprocess a stdin for TOR_PT_EXIT_ON_STDIN_CLOSE purposes. // https://bugs.torproject.org/24642 - _, err = cmd.StdinPipe() + cmd.Env = append(os.Environ(), "TOR_PT_EXIT_ON_STDIN_CLOSE=1") + cmd.StdinCloser, err = cmd.StdinPipe() if err != nil { return } @@ -320,7 +333,7 @@ func runMeekClient(helperAddr string, meekClientCommandLine []string) (cmd *exec // may have failed to start. If a process did not start, its corresponding // return value will be nil. The caller is responsible for terminating whatever // processes were started, whether or not err is nil. -func startProcesses(sigChan <-chan os.Signal, meekClientCommandLine []string) (firefoxCmd *exec.Cmd, meekClientCmd *exec.Cmd, err error) { +func startProcesses(sigChan <-chan os.Signal, meekClientCommandLine []string) (firefoxCmd *exec.Cmd, meekClientCmd *ptCmd, err error) { // Start firefox. var stdout io.Reader firefoxCmd, stdout, err = runFirefox() @@ -414,7 +427,6 @@ func main() { // we are instructed to stop. sig := <-sigChan log.Printf("sig %s", sig) - logSignal(meekClientCmd.Process, sig) } else { // Otherwise don't wait, go ahead and terminate whatever // processes were started. @@ -425,6 +437,9 @@ func main() { logKill(firefoxCmd.Process) } if meekClientCmd != nil { - logKill(firefoxCmd.Process) + err := terminatePTCmd(meekClientCmd) + if err != nil { + log.Printf("error terminating meek-client: %v", err) + } } } diff --git a/meek-client-torbrowser/terminate_other.go b/meek-client-torbrowser/terminate_other.go new file mode 100644 index 0000000..d38e27b --- /dev/null +++ b/meek-client-torbrowser/terminate_other.go @@ -0,0 +1,33 @@ +// +build !windows + +// Process termination code for platforms that have SIGTERM (i.e., not Windows). + +package main + +import ( + "syscall" + "time" +) + +// Terminate a PT subprocess: first close its stdin and send it SIGTERM; then +// kill it if that doesn't work. +func terminatePTCmd(cmd *ptCmd) error { + err := cmd.StdinCloser.Close() + err2 := cmd.Process.Signal(syscall.SIGTERM) + if err == nil { + err = err2 + } + ch := make(chan error, 1) + go func() { + ch <- cmd.Wait() + }() + select { + case <-time.After(terminateTimeout): + err2 = cmd.Process.Kill() + case err2 = <-ch: + } + if err == nil { + err = err2 + } + return err +} diff --git a/meek-client-torbrowser/terminate_windows.go b/meek-client-torbrowser/terminate_windows.go new file mode 100644 index 0000000..c00c44f --- /dev/null +++ b/meek-client-torbrowser/terminate_windows.go @@ -0,0 +1,30 @@ +// +build windows + +// Process termination code for platforms that don't have SIGTERM (i.e., +// Windows). + +package main + +import ( + "time" +) + +// Terminate a PT subprocess: first close its stdin; then kill it if that +// doesn't work. +func terminatePTCmd(cmd *ptCmd) error { + err := cmd.StdinCloser.Close() + ch := make(chan error, 1) + go func() { + ch <- cmd.Wait() + }() + var err2 error + select { + case <-time.After(terminateTimeout): + err2 = cmd.Process.Kill() + case err2 = <-ch: + } + if err == nil { + err = err2 + } + return err +}