commit ccc1963890bc7448383cd4c45370139697973d64 Author: Alexander Færøy ahf@torproject.org Date: Mon Nov 26 06:14:47 2018 +0100

Move remaining code from subprocess.{h,c} to more appropriate places.

This patch moves the remaining code from subprocess.{h,c} to more appropriate places in the process.c and process_win32.c module.

We also delete the now empty subprocess module files.

See: https://bugs.torproject.org/28179 --- src/app/config/config.c | 2 +- src/feature/client/transports.c | 1 - src/lib/process/include.am | 2 - src/lib/process/process.c | 19 +++++ src/lib/process/process.h | 2 + src/lib/process/process_win32.c | 105 +++++++++++++++++++++++++- src/lib/process/process_win32.h | 3 + src/lib/process/subprocess.c | 158 ---------------------------------------- src/lib/process/subprocess.h | 20 ----- src/test/test_logging.c | 1 - src/test/test_process.c | 40 ++++++++++ src/test/test_pt.c | 2 - src/test/test_util.c | 56 +------------- 13 files changed, 171 insertions(+), 240 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c index 90eae50fd..e33c6b26e 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -142,7 +142,7 @@ #include "lib/process/pidfile.h" #include "lib/process/restrict.h" #include "lib/process/setuid.h" -#include "lib/process/subprocess.h" +#include "lib/process/process.h" #include "lib/net/gethostname.h" #include "lib/thread/numcpus.h"

diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index fd4bec780..0e326f90e 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -103,7 +103,6 @@ #include "feature/control/control.h"

#include "lib/process/process.h" -#include "lib/process/subprocess.h" #include "lib/process/env.h"

static smartlist_t * diff --git a/src/lib/process/include.am b/src/lib/process/include.am index aa7335614..a2d54b623 100644 --- a/src/lib/process/include.am +++ b/src/lib/process/include.am @@ -14,7 +14,6 @@ src_lib_libtor_process_a_SOURCES = \ src/lib/process/process_win32.c \ src/lib/process/restrict.c \ src/lib/process/setuid.c \ - src/lib/process/subprocess.c \ src/lib/process/waitpid.c \ src/lib/process/winprocess_sys.c

@@ -32,6 +31,5 @@ noinst_HEADERS += \ src/lib/process/process_win32.h \ src/lib/process/restrict.h \ src/lib/process/setuid.h \ - src/lib/process/subprocess.h \ src/lib/process/waitpid.h \ src/lib/process/winprocess_sys.h diff --git a/src/lib/process/process.c b/src/lib/process/process.c index 915217e13..7275d0a21 100644 --- a/src/lib/process/process.c +++ b/src/lib/process/process.c @@ -26,6 +26,12 @@ /** A list of all <b>process_t</b> instances currently allocated. */ static smartlist_t *processes;

+/** + * Boolean. If true, then Tor may call execve or CreateProcess via + * tor_spawn_background. + **/ +static int may_spawn_background_process = 1; + /** Structure to represent a child process. */ struct process_t { /** Process status. */ @@ -114,6 +120,16 @@ process_protocol_to_string(process_protocol_t protocol) /* LCOV_EXCL_STOP */ }

+/** + * Turn off may_spawn_background_process, so that all future calls to + * tor_spawn_background are guaranteed to fail. + **/ +void +tor_disable_spawning_background_processes(void) +{ + may_spawn_background_process = 0; +} + /** Initialize the Process subsystem. This function initializes the Process * subsystem's global state. For cleaning up, <b>process_free_all()</b> should * be called. */ @@ -234,6 +250,9 @@ process_exec(process_t *process) { tor_assert(process);

+ if (BUG(may_spawn_background_process == 0)) + return PROCESS_STATUS_ERROR; + process_status_t status = PROCESS_STATUS_NOT_RUNNING;

log_info(LD_PROCESS, "Starting new process: %s", process->command); diff --git a/src/lib/process/process.h b/src/lib/process/process.h index 6092c2da7..cb5bccbf7 100644 --- a/src/lib/process/process.h +++ b/src/lib/process/process.h @@ -45,6 +45,8 @@ typedef enum {

const char *process_protocol_to_string(process_protocol_t protocol);

+void tor_disable_spawning_background_processes(void); + struct process_t; typedef struct process_t process_t;

diff --git a/src/lib/process/process_win32.c b/src/lib/process/process_win32.c index 7b18903c7..73e19d518 100644 --- a/src/lib/process/process_win32.c +++ b/src/lib/process/process_win32.c @@ -18,13 +18,16 @@ #include "lib/log/win32err.h" #include "lib/process/process.h" #include "lib/process/process_win32.h" -#include "lib/process/subprocess.h" #include "lib/process/env.h"

#ifdef HAVE_SYS_TIME_H #include <sys/time.h> #endif

+#ifdef HAVE_STRING_H +#include <string.h> +#endif + #ifdef _WIN32

/** The size of our intermediate buffers. */ @@ -889,4 +892,104 @@ process_win32_handle_read_completion(process_win32_handle_t *handle, return false; }

+/** Format a single argument for being put on a Windows command line. + * Returns a newly allocated string */ +STATIC char * +format_win_cmdline_argument(const char *arg) +{ + char *formatted_arg; + char need_quotes; + const char *c; + int i; + int bs_counter = 0; + /* Backslash we can point to when one is inserted into the string */ + const char backslash = '\'; + + /* Smartlist of *char */ + smartlist_t *arg_chars; + arg_chars = smartlist_new(); + + /* Quote string if it contains whitespace or is empty */ + need_quotes = (strchr(arg, ' ') || strchr(arg, '\t') || '\0' == arg[0]); + + /* Build up smartlist of *chars */ + for (c=arg; *c != '\0'; c++) { + if ('"' == *c) { + /* Double up backslashes preceding a quote */ + for (i=0; i<(bs_counter*2); i++) + smartlist_add(arg_chars, (void*)&backslash); + bs_counter = 0; + /* Escape the quote */ + smartlist_add(arg_chars, (void*)&backslash); + smartlist_add(arg_chars, (void*)c); + } else if ('\' == *c) { + /* Count backslashes until we know whether to double up */ + bs_counter++; + } else { + /* Don't double up slashes preceding a non-quote */ + for (i=0; i<bs_counter; i++) + smartlist_add(arg_chars, (void*)&backslash); + bs_counter = 0; + smartlist_add(arg_chars, (void*)c); + } + } + /* Don't double up trailing backslashes */ + for (i=0; i<bs_counter; i++) + smartlist_add(arg_chars, (void*)&backslash); + + /* Allocate space for argument, quotes (if needed), and terminator */ + const size_t formatted_arg_len = smartlist_len(arg_chars) + + (need_quotes ? 2 : 0) + 1; + formatted_arg = tor_malloc_zero(formatted_arg_len); + + /* Add leading quote */ + i=0; + if (need_quotes) + formatted_arg[i++] = '"'; + + /* Add characters */ + SMARTLIST_FOREACH(arg_chars, char*, ch, + { + formatted_arg[i++] = *ch; + }); + + /* Add trailing quote */ + if (need_quotes) + formatted_arg[i++] = '"'; + formatted_arg[i] = '\0'; + + smartlist_free(arg_chars); + return formatted_arg; +} + +/** Format a command line for use on Windows, which takes the command as a + * string rather than string array. Follows the rules from "Parsing C++ + * Command-Line Arguments" in MSDN. Algorithm based on list2cmdline in the + * Python subprocess module. Returns a newly allocated string */ +STATIC char * +tor_join_win_cmdline(const char *argv[]) +{ + smartlist_t *argv_list; + char *joined_argv; + int i; + + /* Format each argument and put the result in a smartlist */ + argv_list = smartlist_new(); + for (i=0; argv[i] != NULL; i++) { + smartlist_add(argv_list, (void *)format_win_cmdline_argument(argv[i])); + } + + /* Join the arguments with whitespace */ + joined_argv = smartlist_join_strings(argv_list, " ", 0, NULL); + + /* Free the newly allocated arguments, and the smartlist */ + SMARTLIST_FOREACH(argv_list, char *, arg, + { + tor_free(arg); + }); + smartlist_free(argv_list); + + return joined_argv; +} + #endif /* ! defined(_WIN32). */ diff --git a/src/lib/process/process_win32.h b/src/lib/process/process_win32.h index 9a42e6c71..3c809dfd2 100644 --- a/src/lib/process/process_win32.h +++ b/src/lib/process/process_win32.h @@ -86,6 +86,9 @@ STATIC int process_win32_read_from_handle(process_win32_handle_t *, STATIC bool process_win32_handle_read_completion(process_win32_handle_t *, DWORD, DWORD); + +STATIC char *format_win_cmdline_argument(const char *arg); +STATIC char *tor_join_win_cmdline(const char *argv[]); #endif /* defined(PROCESS_WIN32_PRIVATE). */

#endif /* ! defined(_WIN32). */ diff --git a/src/lib/process/subprocess.c b/src/lib/process/subprocess.c deleted file mode 100644 index c16379015..000000000 --- a/src/lib/process/subprocess.c +++ /dev/null @@ -1,158 +0,0 @@ -/* Copyright (c) 2003, Roger Dingledine - * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2018, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -/** - * \file subprocess.c - * \brief Launch and monitor other processes. - **/ - -#define SUBPROCESS_PRIVATE -#include "lib/process/subprocess.h" - -#include "lib/container/smartlist.h" -#include "lib/err/torerr.h" -#include "lib/log/log.h" -#include "lib/log/util_bug.h" -#include "lib/log/win32err.h" -#include "lib/malloc/malloc.h" -#include "lib/process/env.h" -#include "lib/process/waitpid.h" -#include "lib/string/compat_ctype.h" - -#ifdef HAVE_SYS_TYPES_H -#include <sys/types.h> -#endif -#ifdef HAVE_SYS_PRCTL_H -#include <sys/prctl.h> -#endif -#ifdef HAVE_UNISTD_H -#include <unistd.h> -#endif -#ifdef HAVE_SIGNAL_H -#include <signal.h> -#endif -#ifdef HAVE_FCNTL_H -#include <fcntl.h> -#endif -#ifdef HAVE_SYS_WAIT_H -#include <sys/wait.h> -#endif -#include <errno.h> -#include <string.h> - -/** Format a single argument for being put on a Windows command line. - * Returns a newly allocated string */ -static char * -format_win_cmdline_argument(const char *arg) -{ - char *formatted_arg; - char need_quotes; - const char *c; - int i; - int bs_counter = 0; - /* Backslash we can point to when one is inserted into the string */ - const char backslash = '\'; - - /* Smartlist of *char */ - smartlist_t *arg_chars; - arg_chars = smartlist_new(); - - /* Quote string if it contains whitespace or is empty */ - need_quotes = (strchr(arg, ' ') || strchr(arg, '\t') || '\0' == arg[0]); - - /* Build up smartlist of *chars */ - for (c=arg; *c != '\0'; c++) { - if ('"' == *c) { - /* Double up backslashes preceding a quote */ - for (i=0; i<(bs_counter*2); i++) - smartlist_add(arg_chars, (void*)&backslash); - bs_counter = 0; - /* Escape the quote */ - smartlist_add(arg_chars, (void*)&backslash); - smartlist_add(arg_chars, (void*)c); - } else if ('\' == *c) { - /* Count backslashes until we know whether to double up */ - bs_counter++; - } else { - /* Don't double up slashes preceding a non-quote */ - for (i=0; i<bs_counter; i++) - smartlist_add(arg_chars, (void*)&backslash); - bs_counter = 0; - smartlist_add(arg_chars, (void*)c); - } - } - /* Don't double up trailing backslashes */ - for (i=0; i<bs_counter; i++) - smartlist_add(arg_chars, (void*)&backslash); - - /* Allocate space for argument, quotes (if needed), and terminator */ - const size_t formatted_arg_len = smartlist_len(arg_chars) + - (need_quotes ? 2 : 0) + 1; - formatted_arg = tor_malloc_zero(formatted_arg_len); - - /* Add leading quote */ - i=0; - if (need_quotes) - formatted_arg[i++] = '"'; - - /* Add characters */ - SMARTLIST_FOREACH(arg_chars, char*, ch, - { - formatted_arg[i++] = *ch; - }); - - /* Add trailing quote */ - if (need_quotes) - formatted_arg[i++] = '"'; - formatted_arg[i] = '\0'; - - smartlist_free(arg_chars); - return formatted_arg; -} - -/** Format a command line for use on Windows, which takes the command as a - * string rather than string array. Follows the rules from "Parsing C++ - * Command-Line Arguments" in MSDN. Algorithm based on list2cmdline in the - * Python subprocess module. Returns a newly allocated string */ -char * -tor_join_win_cmdline(const char *argv[]) -{ - smartlist_t *argv_list; - char *joined_argv; - int i; - - /* Format each argument and put the result in a smartlist */ - argv_list = smartlist_new(); - for (i=0; argv[i] != NULL; i++) { - smartlist_add(argv_list, (void *)format_win_cmdline_argument(argv[i])); - } - - /* Join the arguments with whitespace */ - joined_argv = smartlist_join_strings(argv_list, " ", 0, NULL); - - /* Free the newly allocated arguments, and the smartlist */ - SMARTLIST_FOREACH(argv_list, char *, arg, - { - tor_free(arg); - }); - smartlist_free(argv_list); - - return joined_argv; -} - -/** - * Boolean. If true, then Tor may call execve or CreateProcess via - * tor_spawn_background. - **/ -static int may_spawn_background_process = 1; -/** - * Turn off may_spawn_background_process, so that all future calls to - * tor_spawn_background are guaranteed to fail. - **/ -void -tor_disable_spawning_background_processes(void) -{ - may_spawn_background_process = 0; -} diff --git a/src/lib/process/subprocess.h b/src/lib/process/subprocess.h deleted file mode 100644 index eb9162634..000000000 --- a/src/lib/process/subprocess.h +++ /dev/null @@ -1,20 +0,0 @@ -/* Copyright (c) 2003-2004, Roger Dingledine - * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2018, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -/** - * \file subprocess.h - * \brief Header for subprocess.c - **/ - -#ifndef TOR_SUBPROCESS_H -#define TOR_SUBPROCESS_H - -void tor_disable_spawning_background_processes(void); - -#ifndef _WIN32 -char *tor_join_win_cmdline(const char *argv[]); -#endif - -#endif diff --git a/src/test/test_logging.c b/src/test/test_logging.c index 9d3ee5d55..2ecae461a 100644 --- a/src/test/test_logging.c +++ b/src/test/test_logging.c @@ -9,7 +9,6 @@ #include "lib/err/torerr.h" #include "lib/log/log.h" #include "test/test.h" -#include "lib/process/subprocess.h"

#ifdef HAVE_UNISTD_H #include <unistd.h> diff --git a/src/test/test_process.c b/src/test/test_process.c index 2fcb3f668..3640b8668 100644 --- a/src/test/test_process.c +++ b/src/test/test_process.c @@ -628,11 +628,51 @@ test_win32(void *arg) process_init();

process_t *process = process_new(""); + char *joined_argv = NULL;

/* On Win32 all processes should have a Win32 process handle. */ tt_ptr_op(NULL, OP_NE, process_get_win32_process(process));

+ /* Based on some test cases from "Parsing C++ Command-Line Arguments" in + * MSDN but we don't exercise all quoting rules because tor_join_win_cmdline + * will try to only generate simple cases for the child process to parse; + * i.e. we never embed quoted strings in arguments. */ + + const char *argvs[][4] = { + {"a", "bb", "CCC", NULL}, // Normal + {NULL, NULL, NULL, NULL}, // Empty argument list + {"", NULL, NULL, NULL}, // Empty argument + {""a", "b"b", "CCC"", NULL}, // Quotes + {"a\tbc", "dd dd", "E", NULL}, // Whitespace + {"a\\\b", "de fg", "H", NULL}, // Backslashes + {"a\"b", "\c", "D\", NULL}, // Backslashes before quote + {"a\\b c", "d", "E", NULL}, // Backslashes not before quote + { NULL } // Terminator + }; + + const char *cmdlines[] = { + "a bb CCC", + "", + """", + "\"a b\"b CCC\"", + ""a\tbc" "dd dd" E", + "a\\\b "de fg" H", + "a\\\"b \c D\", + ""a\\b c" d E", + NULL // Terminator + }; + + int i; + + for (i=0; cmdlines[i]!=NULL; i++) { + log_info(LD_GENERAL, "Joining argvs[%d], expecting <%s>", i, cmdlines[i]); + joined_argv = tor_join_win_cmdline(argvs[i]); + tt_str_op(cmdlines[i],OP_EQ, joined_argv); + tor_free(joined_argv); + } + done: + tor_free(joined_argv); process_free(process); process_free_all(); #endif diff --git a/src/test/test_pt.c b/src/test/test_pt.c index c980276b1..b5572ef80 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -8,7 +8,6 @@ #define UTIL_PRIVATE #define STATEFILE_PRIVATE #define CONTROL_PRIVATE -#define SUBPROCESS_PRIVATE #define PROCESS_PRIVATE #include "core/or/or.h" #include "app/config/config.h" @@ -18,7 +17,6 @@ #include "core/or/circuitbuild.h" #include "app/config/statefile.h" #include "test/test.h" -#include "lib/process/subprocess.h" #include "lib/encoding/confline.h" #include "lib/net/resolve.h" #include "lib/process/process.h" diff --git a/src/test/test_util.c b/src/test/test_util.c index 4fa67b641..a94153f2d 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -10,7 +10,7 @@ #define UTIL_PRIVATE #define UTIL_MALLOC_PRIVATE #define SOCKET_PRIVATE -#define SUBPROCESS_PRIVATE +#define PROCESS_WIN32_PRIVATE #include "lib/testsupport/testsupport.h" #include "core/or/or.h" #include "lib/container/buffers.h" @@ -22,6 +22,7 @@ #include "test/test.h" #include "lib/memarea/memarea.h" #include "lib/process/waitpid.h" +#include "lib/process/process_win32.h" #include "test/log_test_helpers.h" #include "lib/compress/compress.h" #include "lib/compress/compress_zstd.h" @@ -30,7 +31,6 @@ #include "lib/fs/winlib.h" #include "lib/process/env.h" #include "lib/process/pidfile.h" -#include "lib/process/subprocess.h" #include "lib/intmath/weakrng.h" #include "lib/thread/numcpus.h" #include "lib/math/fp.h" @@ -4395,57 +4395,6 @@ test_util_format_dec_number(void *ptr) return; }

-/** - * Test that we can properly format a Windows command line - */ -static void -test_util_join_win_cmdline(void *ptr) -{ - /* Based on some test cases from "Parsing C++ Command-Line Arguments" in - * MSDN but we don't exercise all quoting rules because tor_join_win_cmdline - * will try to only generate simple cases for the child process to parse; - * i.e. we never embed quoted strings in arguments. */ - - const char *argvs[][4] = { - {"a", "bb", "CCC", NULL}, // Normal - {NULL, NULL, NULL, NULL}, // Empty argument list - {"", NULL, NULL, NULL}, // Empty argument - {""a", "b"b", "CCC"", NULL}, // Quotes - {"a\tbc", "dd dd", "E", NULL}, // Whitespace - {"a\\\b", "de fg", "H", NULL}, // Backslashes - {"a\"b", "\c", "D\", NULL}, // Backslashes before quote - {"a\\b c", "d", "E", NULL}, // Backslashes not before quote - { NULL } // Terminator - }; - - const char *cmdlines[] = { - "a bb CCC", - "", - """", - "\"a b\"b CCC\"", - ""a\tbc" "dd dd" E", - "a\\\b "de fg" H", - "a\\\"b \c D\", - ""a\\b c" d E", - NULL // Terminator - }; - - int i; - char *joined_argv = NULL; - - (void)ptr; - - for (i=0; cmdlines[i]!=NULL; i++) { - log_info(LD_GENERAL, "Joining argvs[%d], expecting <%s>", i, cmdlines[i]); - joined_argv = tor_join_win_cmdline(argvs[i]); - tt_str_op(cmdlines[i],OP_EQ, joined_argv); - tor_free(joined_argv); - } - - done: - tor_free(joined_argv); -} - #define MAX_SPLIT_LINE_COUNT 4 struct split_lines_test_t { const char *orig_line; // Line to be split (may contain \0's) @@ -6226,7 +6175,6 @@ struct testcase_t util_tests[] = { UTIL_TEST_WIN_ONLY(load_win_lib, 0), UTIL_TEST(format_hex_number, 0), UTIL_TEST(format_dec_number, 0), - UTIL_TEST(join_win_cmdline, 0), UTIL_TEST(n_bits_set, 0), UTIL_TEST(eat_whitespace, 0), UTIL_TEST(sl_new_from_text_lines, 0),