- tor-commits - lists.torproject.org

[tor/master] Don't warn of stray Bridges if managed proxies are still unconfigured.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit d0416ce3ec67ca87448153c8b2416dd7901013c6 Author: George Kadianakis <desnacked(a)gmail.com> Date: Sun Sep 11 23:51:29 2011 +0200 Don't warn of stray Bridges if managed proxies are still unconfigured. With managed proxies you would always get the error message: "You have a Bridge line using the X pluggable transport, but there doesn't seem to be a corresponding ClientTransportPlugin line." because the check happened directly after parse_client_transport_line() when managed proxies were not fully configured and their transports were not registered. The fix is to move the validation to run_scheduled_events() and make sure that all managed proxies are configured first. --- src/or/circuitbuild.c | 13 ++++++++++--- src/or/circuitbuild.h | 2 +- src/or/config.c | 5 ----- src/or/main.c | 9 +++++++++ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index b4c2343..ed81304 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4769,11 +4769,14 @@ transport_add_from_config(const tor_addr_t *addr, uint16_t port, } } -/** Warns the user of possible pluggable transport misconfiguration. */ -void +/** Warn the user of possible pluggable transport misconfiguration. + * Return 0 if the validation happened, -1 if we should postpone the + * validation. */ +int validate_pluggable_transports_config(void) { - if (bridge_list) { + /* Don't validate if managed proxies are not yet fully configured. */ + if (bridge_list && !pt_proxies_configuration_pending()) { SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, b) { /* Skip bridges without transports. */ if (!b->transport_name) @@ -4787,6 +4790,10 @@ validate_pluggable_transports_config(void) "corresponding ClientTransportPlugin line.", b->transport_name); } SMARTLIST_FOREACH_END(b); + + return 0; + } else { + return -1; } } diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 10e7287..dca541d 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -157,7 +157,7 @@ int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, const transport_t **transport); transport_t *transport_get_by_name(const char *name); -void validate_pluggable_transports_config(void); +int validate_pluggable_transports_config(void); #endif diff --git a/src/or/config.c b/src/or/config.c index f2cb6cd..8bd47b5 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1271,11 +1271,6 @@ options_act(or_options_t *old_options) sweep_transport_list(); sweep_proxy_list(); - /* If we have pluggable transport related options enabled, see if we - should warn the user about potential configuration problems. */ - if (options->Bridges || options->ClientTransportPlugin) - validate_pluggable_transports_config(); - /* Bail out at this point if we're not going to be a client or server: * we want to not fork, and to log stuff to stderr. */ if (!running_tor) diff --git a/src/or/main.c b/src/or/main.c index 5294e4a..6297f0f 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1068,6 +1068,7 @@ run_scheduled_events(time_t now) static int should_init_bridge_stats = 1; static time_t time_to_retry_dns_init = 0; static time_t time_to_next_heartbeat = 0; + static int has_validated_pt = 0; or_options_t *options = get_options(); int is_server = server_mode(options); int i; @@ -1472,6 +1473,14 @@ run_scheduled_events(time_t now) if (pt_proxies_configuration_pending()) pt_configure_remaining_proxies(); + /** 11c. validate pluggable transports configuration if we need to */ + if (!has_validated_pt && + (options->Bridges || options->ClientTransportPlugin)) { + if (validate_pluggable_transports_config() == 0) { + has_validated_pt = 1; + } + } + /** 12. write the heartbeat message */ if (options->HeartbeatPeriod && time_to_next_heartbeat < now) {

1 0

[tor/master] Enforce transport names being C identifiers.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit c6811c57cb75b2c594b2a6fffaca0c5ae4c19e0a Author: George Kadianakis <desnacked(a)gmail.com> Date: Sun Sep 11 23:34:11 2011 +0200 Enforce transport names being C identifiers. Introduce string_is_C_identifier() and use it to enforce transport names according to the 180 spec. --- src/common/util.c | 28 ++++++++++++++++++++++++++++ src/common/util.h | 2 ++ src/or/config.c | 8 ++++++++ src/or/transports.c | 10 ++++++++++ 4 files changed, 48 insertions(+), 0 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 63172c3..5fc2cbe 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -719,6 +719,34 @@ find_str_at_start_of_line(const char *haystack, const char *needle) return NULL; } +/** Returns true if string could be a C identifier. + A C identifier must begin with a letter or an underscore and the + rest of its characters can be letters, numbers or underscores. No + length limit is imposed. */ +int +string_is_C_identifier(const char *string) +{ + size_t iter; + size_t length = strlen(string); + if (!length) + return 0; + + for (iter = 0; iter < length ; iter++) { + if (iter == 0) { + if (!(TOR_ISALPHA(string[iter]) || + string[iter] == '_')) + return 0; + } else { + if (!(TOR_ISALPHA(string[iter]) || + TOR_ISDIGIT(string[iter]) || + string[iter] == '_')) + return 0; + } + } + + return 1; +} + /** Return true iff the 'len' bytes at 'mem' are all zero. */ int tor_mem_is_zero(const char *mem, size_t len) diff --git a/src/common/util.h b/src/common/util.h index 7e889b1..04ae7cb 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -203,6 +203,8 @@ const char *find_whitespace(const char *s) ATTR_PURE; const char *find_whitespace_eos(const char *s, const char *eos) ATTR_PURE; const char *find_str_at_start_of_line(const char *haystack, const char *needle) ATTR_PURE; +int string_is_C_identifier(const char *string); + int tor_mem_is_zero(const char *mem, size_t len) ATTR_PURE; int tor_digest_is_zero(const char *digest) ATTR_PURE; int tor_digest256_is_zero(const char *digest) ATTR_PURE; diff --git a/src/or/config.c b/src/or/config.c index d36418b..58668b1 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4724,6 +4724,10 @@ parse_client_transport_line(const char *line, int validate_only) } name = smartlist_get(items, 0); + if (!string_is_C_identifier(name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", name); + goto err; + } /* field2 is either a SOCKS version or "exec" */ field2 = smartlist_get(items, 1); @@ -4826,6 +4830,10 @@ parse_server_transport_line(const char *line, int validate_only) } name = smartlist_get(items, 0); + if (!string_is_C_identifier(name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", name); + goto err; + } type = smartlist_get(items, 1); diff --git a/src/or/transports.c b/src/or/transports.c index 1b7249f..6d1ddeb 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -686,6 +686,11 @@ parse_smethod_line(const char *line, managed_proxy_t *mp) tor_assert(!strcmp(smartlist_get(items,0),PROTO_SMETHOD)); method_name = smartlist_get(items,1); + if (!string_is_C_identifier(method_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + method_name); + goto err; + } addrport = smartlist_get(items, 2); if (tor_addr_port_parse(addrport, &addr, &port)<0) { @@ -754,6 +759,11 @@ parse_cmethod_line(const char *line, managed_proxy_t *mp) tor_assert(!strcmp(smartlist_get(items,0),PROTO_CMETHOD)); method_name = smartlist_get(items,1); + if (!string_is_C_identifier(method_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + method_name); + goto err; + } socks_ver_str = smartlist_get(items,2);

1 0

[tor/master] Merge remote-tracking branch 'asn2/bug3656'
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit ed39621a9d97dc07063b6e9052b52a91b99d82d6 Merge: 98e5c63 1174bb9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 7 16:05:13 2011 -0400 Merge remote-tracking branch 'asn2/bug3656' Conflicts: src/common/util.c src/common/util.h src/or/config.h src/or/main.c src/test/test_util.c src/common/util.c | 206 +++++++--- src/common/util.h | 27 +- src/or/Makefile.am | 2 + src/or/circuitbuild.c | 186 ++++++++-- src/or/circuitbuild.h | 15 +- src/or/config.c | 501 +++++++++++++++++++++--- src/or/config.h | 5 + src/or/connection.c | 9 +- src/or/main.c | 21 +- src/or/or.h | 5 + src/or/transports.c | 1048 +++++++++++++++++++++++++++++++++++++++++++++++++ src/or/transports.h | 105 +++++ src/test/Makefile.am | 1 + src/test/test.c | 2 + src/test/test_pt.c | 147 +++++++ src/test/test_util.c | 8 +- 16 files changed, 2141 insertions(+), 147 deletions(-) diff --cc src/common/util.c index df77c33,a0777ea..a3716e4 --- a/src/common/util.c +++ b/src/common/util.c @@@ -3140,131 -2993,28 +3192,129 @@@ tor_terminate_process(pid_t pid #define CHILD_STATE_EXEC 8 #define CHILD_STATE_FAILEXEC 9 - #define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code " - -/** Start a program in the background. If filename contains a '/', - * then it will be treated as an absolute or relative path. Otherwise the - * system path will be searched for filename. The strings in - * argv will be passed as the command line arguments of the child - * program (following convention, argv[0] should normally be the filename of - * the executable). The last element of argv must be NULL. If the child - * program is launched, the PID will be returned and stdout_read and - * stdout_err will be set to file descriptors from which the stdout - * and stderr, respectively, output of the child program can be read, and the - * stdin of the child process shall be set to /dev/null. Otherwise returns - * -1. Some parts of this code are based on the POSIX subprocess module from - * Python. +/** Start a program in the background. If filename contains a '/', then + * it will be treated as an absolute or relative path. Otherwise, on + * non-Windows systems, the system path will be searched for filename. + * On Windows, only the current directory will be searched. Here, to search the + * system path (as well as the application directory, current working + * directory, and system directories), set filename to NULL. + * + * The strings in argv will be passed as the command line arguments of + * the child program (following convention, argv[0] should normally be the + * filename of the executable, and this must be the case if filename is + * NULL). The last element of argv must be NULL. A handle to the child process + * will be returned in process_handle (which must be non-NULL). Read + * process_handle.status to find out if the process was successfully launched. + * For convenience, process_handle.status is returned by this function. + * + * Some parts of this code are based on the POSIX subprocess module from + * Python, and example code from + * http://msdn.microsoft.com/en-us/library/ms682499%28v=vs.85%29.aspx. */ - int -tor_spawn_background(const char *const filename, int *stdout_read, - int *stderr_read, const char **argv, const char **envp) +tor_spawn_background(const char *const filename, const char **argv, ++ const char **envp, + process_handle_t *process_handle) { #ifdef MS_WINDOWS - (void) filename; (void) stdout_read; (void) stderr_read; (void) argv; - log_warn(LD_BUG, "not yet implemented on Windows."); - return -1; -#else + HANDLE stdout_pipe_read = NULL; + HANDLE stdout_pipe_write = NULL; + HANDLE stderr_pipe_read = NULL; + HANDLE stderr_pipe_write = NULL; + + STARTUPINFO siStartInfo; + BOOL retval = FALSE; + + SECURITY_ATTRIBUTES saAttr; + char *joined_argv; + + /* process_handle must not be NULL */ + tor_assert(process_handle != NULL); + + saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); + saAttr.bInheritHandle = TRUE; + /* TODO: should we set explicit security attributes? (#2046, comment 5) */ + saAttr.lpSecurityDescriptor = NULL; + + /* Assume failure to start process */ + memset(process_handle, 0, sizeof(process_handle_t)); + process_handle->status = PROCESS_STATUS_ERROR; + + /* Set up pipe for stdout */ + if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) { + log_warn(LD_GENERAL, + "Failed to create pipe for stdout communication with child process: %s", + format_win32_error(GetLastError())); + return process_handle->status; + } + if (!SetHandleInformation(stdout_pipe_read, HANDLE_FLAG_INHERIT, 0)) { + log_warn(LD_GENERAL, + "Failed to configure pipe for stdout communication with child " + "process: %s", format_win32_error(GetLastError())); + return process_handle->status; + } + + /* Set up pipe for stderr */ + if (!CreatePipe(&stderr_pipe_read, &stderr_pipe_write, &saAttr, 0)) { + log_warn(LD_GENERAL, + "Failed to create pipe for stderr communication with child process: %s", + format_win32_error(GetLastError())); + return process_handle->status; + } + if (!SetHandleInformation(stderr_pipe_read, HANDLE_FLAG_INHERIT, 0)) { + log_warn(LD_GENERAL, + "Failed to configure pipe for stderr communication with child " + "process: %s", format_win32_error(GetLastError())); + return process_handle->status; + } + + /* Create the child process */ + + /* Windows expects argv to be a whitespace delimited string, so join argv up + */ + joined_argv = tor_join_win_cmdline(argv); + + ZeroMemory(&(process_handle->pid), sizeof(PROCESS_INFORMATION)); + ZeroMemory(&siStartInfo, sizeof(STARTUPINFO)); + siStartInfo.cb = sizeof(STARTUPINFO); + siStartInfo.hStdError = stderr_pipe_write; + siStartInfo.hStdOutput = stdout_pipe_write; + siStartInfo.hStdInput = NULL; + siStartInfo.dwFlags |= STARTF_USESTDHANDLES; + + /* Create the child process */ + + retval = CreateProcess(filename, // module name + joined_argv, // command line + /* TODO: should we set explicit security attributes? (#2046, comment 5) */ + NULL, // process security attributes + NULL, // primary thread security attributes + TRUE, // handles are inherited + /*(TODO: set CREATE_NEW CONSOLE/PROCESS_GROUP to make GetExitCodeProcess() + * work?) */ + 0, // creation flags + NULL, // use parent's environment + NULL, // use parent's current directory + &siStartInfo, // STARTUPINFO pointer + &(process_handle->pid)); // receives PROCESS_INFORMATION + + tor_free(joined_argv); + + if (!retval) { + log_warn(LD_GENERAL, + "Failed to create child process %s: %s", filename?filename:argv[0], + format_win32_error(GetLastError())); + } else { + /* TODO: Close hProcess and hThread in process_handle->pid? */ + process_handle->stdout_pipe = stdout_pipe_read; + process_handle->stderr_pipe = stderr_pipe_read; + process_handle->status = PROCESS_STATUS_RUNNING; + } + + /* TODO: Close pipes on exit */ + + return process_handle->status; +#else // MS_WINDOWS pid_t pid; int stdout_pipe[2]; int stderr_pipe[2]; @@@ -3428,341 -3173,73 +3481,342 @@@ log_warn(LD_GENERAL, "Failed to close write end of stderr pipe in parent process: %s", strerror(errno)); - /* Do not return -1, because the child is running, so the parent - needs to know about the pid in order to reap it later */ } - return pid; -#endif + process_handle->status = PROCESS_STATUS_RUNNING; + /* Set stdout/stderr pipes to be non-blocking */ + fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK); + fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK); + /* Open the buffered IO streams */ + process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r"); + process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r"); + + return process_handle->status; +#endif // MS_WINDOWS +} + +/** Get the exit code of a process specified by process_handle and store + * it in exit_code, if set to a non-NULL value. If block is set + * to true, the call will block until the process has exited. Otherwise if + * the process is still running, the function will return + * PROCESS_EXIT_RUNNING, and exit_code will be left unchanged. Returns + * PROCESS_EXIT_EXITED if the process did exit. If there is a failure, + * PROCESS_EXIT_ERROR will be returned and the contents of exit_code (if + * non-NULL) will be undefined. N.B. Under *nix operating systems, this will + * probably not work in Tor, because waitpid() is called in main.c to reap any + * terminated child processes.*/ +int +tor_get_exit_code(const process_handle_t process_handle, + int block, int *exit_code) +{ +#ifdef MS_WINDOWS + DWORD retval; + BOOL success; + + if (block) { + /* Wait for the process to exit */ + retval = WaitForSingleObject(process_handle.pid.hProcess, INFINITE); + if (retval != WAIT_OBJECT_0) { + log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s", + (int)retval, format_win32_error(GetLastError())); + return PROCESS_EXIT_ERROR; + } + } else { + retval = WaitForSingleObject(process_handle.pid.hProcess, 0); + if (WAIT_TIMEOUT == retval) { + /* Process has not exited */ + return PROCESS_EXIT_RUNNING; + } else if (retval != WAIT_OBJECT_0) { + log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s", + (int)retval, format_win32_error(GetLastError())); + return PROCESS_EXIT_ERROR; + } + } + + if (exit_code != NULL) { + success = GetExitCodeProcess(process_handle.pid.hProcess, + (PDWORD)exit_code); + if (!success) { + log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s", + format_win32_error(GetLastError())); + return PROCESS_EXIT_ERROR; + } + } +#else + int stat_loc; + int retval; + + retval = waitpid(process_handle.pid, &stat_loc, block?0:WNOHANG); + if (!block && 0 == retval) { + /* Process has not exited */ + return PROCESS_EXIT_RUNNING; + } else if (retval != process_handle.pid) { + log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid, + strerror(errno)); + return PROCESS_EXIT_ERROR; + } + + if (!WIFEXITED(stat_loc)) { + log_warn(LD_GENERAL, "Process %d did not exit normally", + process_handle.pid); + return PROCESS_EXIT_ERROR; + } + + if (exit_code != NULL) + *exit_code = WEXITSTATUS(stat_loc); +#endif // MS_WINDOWS + + return PROCESS_EXIT_EXITED; } -/** Reads from stream and stores input in buf_out making - * sure it's below count bytes. - * If the string has a trailing newline, we strip it off. - * - * This function is specifically created to handle input from managed - * proxies, according to the pluggable transports spec. Make sure it - * fits your needs before using it. - * - * Returns: - * IO_STREAM_CLOSED: If the stream is closed. - * IO_STREAM_EAGAIN: If there is nothing to read and we should check back - * later. - * IO_STREAM_TERM: If something is wrong with the stream. - * IO_STREAM_OKAY: If everything went okay and we got a string - * in buf_out. */ -enum stream_status -get_string_from_pipe(FILE *stream, char *buf_out, size_t count) +#ifdef MS_WINDOWS +/** Read from a handle h into buf, up to count bytes. If + * hProcess is NULL, the function will return immediately if there is + * nothing more to read. Otherwise hProcess should be set to the handle + * to the process owning the h. In this case, the function will exit + * only once the process has exited, or count bytes are read. Returns + * the number of bytes read, or -1 on error. */ +ssize_t +tor_read_all_handle(HANDLE h, char *buf, size_t count, + const process_handle_t *process) +{ + size_t numread = 0; + BOOL retval; + DWORD byte_count; + BOOL process_exited = FALSE; + + if (count > SIZE_T_CEILING || count > SSIZE_T_MAX) + return -1; + + while (numread != count) { + /* Check if there is anything to read */ + retval = PeekNamedPipe(h, NULL, 0, NULL, &byte_count, NULL); + if (!retval) { + log_warn(LD_GENERAL, + "Failed to peek from handle: %s", + format_win32_error(GetLastError())); + return -1; + } else if (0 == byte_count) { + /* Nothing available: process exited or it is busy */ + + /* Exit if we don't know whether the process is running */ + if (NULL == process) + break; + + /* The process exited and there's nothing left to read from it */ + if (process_exited) + break; + + /* If process is not running, check for output one more time in case + it wrote something after the peek was performed. Otherwise keep on + waiting for output */ + tor_assert(process != NULL); + byte_count = WaitForSingleObject(process->pid.hProcess, 0); + if (WAIT_TIMEOUT != byte_count) + process_exited = TRUE; + + continue; + } + + /* There is data to read; read it */ + retval = ReadFile(h, buf+numread, count-numread, &byte_count, NULL); + tor_assert(byte_count + numread <= count); + if (!retval) { + log_warn(LD_GENERAL, "Failed to read from handle: %s", + format_win32_error(GetLastError())); + return -1; + } else if (0 == byte_count) { + /* End of file */ + break; + } + numread += byte_count; + } + return (ssize_t)numread; +} +#else +/** Read from a handle h into buf, up to count bytes. If + * process is NULL, the function will return immediately if there is + * nothing more to read. Otherwise data will be read until end of file, or + * count bytes are read. Returns the number of bytes read, or -1 on + * error. Sets eof to true if eof is not NULL and the end of the + * file has been reached. */ +ssize_t +tor_read_all_handle(FILE *h, char *buf, size_t count, + const process_handle_t *process, + int *eof) { + size_t numread = 0; char *retval; - size_t len; - retval = fgets(buf_out, count, stream); + if (eof) + *eof = 0; - if (!retval) { - if (feof(stream)) { - /* Program has closed stream (probably it exited) */ - /* TODO: check error */ - return IO_STREAM_CLOSED; - } else { - if (EAGAIN == errno) { - /* Nothing more to read, try again next time */ - return IO_STREAM_EAGAIN; + if (count > SIZE_T_CEILING || count > SSIZE_T_MAX) + return -1; + + while (numread != count) { + /* Use fgets because that is what we use in log_from_pipe() */ + retval = fgets(buf+numread, (int)(count-numread), h); + if (NULL == retval) { + if (feof(h)) { + log_debug(LD_GENERAL, "fgets() reached end of file"); + fclose(h); + if (eof) + *eof = 1; + break; } else { - /* There was a problem, abandon this child process */ - return IO_STREAM_TERM; + if (EAGAIN == errno) { + if (process) + continue; + else + break; + } else { + log_warn(LD_GENERAL, "fgets() from handle failed: %s", + strerror(errno)); + fclose(h); + return -1; + } } } - } else { - len = strlen(buf_out); - tor_assert(len>0); + tor_assert(retval != NULL); + tor_assert(strlen(retval) + numread <= count); + numread += strlen(retval); + } - if (buf_out[len - 1] == '\n') { - /* Remove the trailing newline */ - buf_out[len - 1] = '\0'; - } else { - /* No newline; check whether we overflowed the buffer */ - if (!feof(stream)) - log_info(LD_GENERAL, - "Line from stream was truncated: %s", buf_out); - /* TODO: What to do with this error? */ + log_debug(LD_GENERAL, "fgets() read %d bytes from handle", (int)numread); + return (ssize_t)numread; +} +#endif + +/** Read from stdout of a process until the process exits. */ +ssize_t +tor_read_all_from_process_stdout(const process_handle_t *process_handle, + char *buf, size_t count) +{ +#ifdef MS_WINDOWS + return tor_read_all_handle(process_handle->stdout_pipe, buf, count, + process_handle); +#else + return tor_read_all_handle(process_handle->stdout_handle, buf, count, + process_handle, NULL); +#endif +} + +/** Read from stdout of a process until the process exits. */ +ssize_t +tor_read_all_from_process_stderr(const process_handle_t *process_handle, + char *buf, size_t count) +{ +#ifdef MS_WINDOWS + return tor_read_all_handle(process_handle->stderr_pipe, buf, count, + process_handle); +#else + return tor_read_all_handle(process_handle->stderr_handle, buf, count, + process_handle, NULL); +#endif +} + +/** Split buf into lines, and add to smartlist. The buffer buf will be + * modified. The resulting smartlist will consist of pointers to buf, so there + * is no need to free the contents of sl. buf must be a NUL-terminated + * string. len should be set to the length of the buffer excluding the + * NUL. Non-printable characters (including NUL) will be replaced with "." */ +int +tor_split_lines(smartlist_t *sl, char *buf, int len) +{ + /* Index in buf of the start of the current line */ + int start = 0; + /* Index in buf of the current character being processed */ + int cur = 0; + /* Are we currently in a line */ + char in_line = 0; + + /* Loop over string */ + while (cur < len) { + /* Loop until end of line or end of string */ + for (; cur < len; cur++) { + if (in_line) { + if ('\r' == buf[cur] || '\n' == buf[cur]) { + /* End of line */ + buf[cur] = '\0'; + /* Point cur to the next line */ + cur++; + /* Line starts at start and ends with a nul */ + break; + } else { + if (!TOR_ISPRINT(buf[cur])) + buf[cur] = '.'; + } + } else { + if ('\r' == buf[cur] || '\n' == buf[cur]) { + /* Skip leading vertical space */ + ; + } else { + in_line = 1; + start = cur; + if (!TOR_ISPRINT(buf[cur])) + buf[cur] = '.'; + } + } } + /* We are at the end of the line or end of string. If in_line is true there + * is a line which starts at buf+start and ends at a NUL. cur points to + * the character after the NUL. */ + if (in_line) + smartlist_add(sl, (void *)(buf+start)); + in_line = 0; + } + return smartlist_len(sl); +} - return IO_STREAM_OKAY; +#ifdef MS_WINDOWS +/** Read from stream, and send lines to log at the specified log level. + * Returns -1 if there is a error reading, and 0 otherwise. + * If the generated stream is flushed more often than on new lines, or + * a read exceeds 256 bytes, lines will be truncated. This should be fixed, + * along with the corresponding problem on *nix (see bug #2045). + */ +static int +log_from_handle(HANDLE *pipe, int severity) +{ + char buf[256]; + int pos; + smartlist_t *lines; + + pos = tor_read_all_handle(pipe, buf, sizeof(buf) - 1, NULL); + if (pos < 0) { + /* Error */ + log_warn(LD_GENERAL, "Failed to read data from subprocess"); + return -1; } - /* We should never get here */ - return IO_STREAM_TERM; + if (0 == pos) { + /* There's nothing to read (process is busy or has exited) */ + log_debug(LD_GENERAL, "Subprocess had nothing to say"); + return 0; + } + + /* End with a null even if there isn't a \r\n at the end */ + /* TODO: What if this is a partial line? */ + buf[pos] = '\0'; + log_debug(LD_GENERAL, "Subprocess had %d bytes to say", pos); + + /* Split up the buffer */ + lines = smartlist_create(); + tor_split_lines(lines, buf, pos); + + /* Log each line */ + SMARTLIST_FOREACH(lines, char *, line, + { + log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", line); + }); + smartlist_free(lines); + + return 0; } +#else ++ /** Read from stream, and send lines to log at the specified log level. * Returns 1 if stream is closed normally, -1 if there is a error reading, and * 0 otherwise. Handles lines from tor-fw-helper and @@@ -3773,72 -3250,50 +3827,110 @@@ log_from_pipe(FILE *stream, int severit int *child_status) { char buf[256]; + enum stream_status r; for (;;) { - char *retval; - retval = fgets(buf, sizeof(buf), stream); + r = get_string_from_pipe(stream, buf, sizeof(buf) - 1); - if (NULL == retval) { - if (feof(stream)) { - /* Program has closed stream (probably it exited) */ - /* TODO: check error */ - fclose(stream); - return 1; + if (r == IO_STREAM_CLOSED) { + fclose(stream); + return 1; + } else if (r == IO_STREAM_EAGAIN) { + return 0; + } else if (r == IO_STREAM_TERM) { + fclose(stream); + return -1; + } + + tor_assert(r == IO_STREAM_OKAY); + + /* Check if buf starts with SPAWN_ERROR_MESSAGE */ + if (strcmpstart(buf, SPAWN_ERROR_MESSAGE) == 0) { + /* Parse error message */ + int retval, child_state, saved_errno; + retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x", + &child_state, &saved_errno); + if (retval == 2) { + log_warn(LD_GENERAL, + "Failed to start child process \"%s\" in state %d: %s", + executable, child_state, strerror(saved_errno)); + if (child_status) + *child_status = 1; } else { - if (EAGAIN == errno) { - /* Nothing more to read, try again next time */ - return 0; - } else { - /* There was a problem, abandon this child process */ - fclose(stream); - return -1; - } + /* Failed to parse message from child process, log it as a + warning */ + log_warn(LD_GENERAL, + "Unexpected message from port forwarding helper \"%s\": %s", + executable, buf); } } else { - /* We have some data, log it and keep asking for more */ - size_t len; + log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", buf); + } + } - len = strlen(buf); - if (buf[len - 1] == '\n') { - /* Remove the trailing newline */ - buf[len - 1] = '\0'; - } else { - /* No newline; check whether we overflowed the buffer */ - if (!feof(stream)) - log_warn(LD_GENERAL, - "Line from port forwarding helper was truncated: %s", buf); - /* TODO: What to do with this error? */ - } + /* We should never get here */ + return -1; + } ++#endif + - /* Check if buf starts with SPAWN_ERROR_MESSAGE */ - if (strcmpstart(buf, SPAWN_ERROR_MESSAGE) == 0) { - /* Parse error message */ - int retval, child_state, saved_errno; - retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x", - &child_state, &saved_errno); - if (retval == 2) { - log_warn(LD_GENERAL, - "Failed to start child process \"%s\" in state %d: %s", - executable, child_state, strerror(saved_errno)); - if (child_status) - *child_status = 1; - } else { - /* Failed to parse message from child process, log it as a - warning */ - log_warn(LD_GENERAL, - "Unexpected message from port forwarding helper \"%s\": %s", - executable, buf); - } ++/** Reads from stream and stores input in buf_out making ++ * sure it's below count bytes. ++ * If the string has a trailing newline, we strip it off. ++ * ++ * This function is specifically created to handle input from managed ++ * proxies, according to the pluggable transports spec. Make sure it ++ * fits your needs before using it. ++ * ++ * Returns: ++ * IO_STREAM_CLOSED: If the stream is closed. ++ * IO_STREAM_EAGAIN: If there is nothing to read and we should check back ++ * later. ++ * IO_STREAM_TERM: If something is wrong with the stream. ++ * IO_STREAM_OKAY: If everything went okay and we got a string ++ * in buf_out. */ ++enum stream_status ++get_string_from_pipe(FILE *stream, char *buf_out, size_t count) ++{ ++ char *retval; ++ size_t len; ++ ++ retval = fgets(buf_out, count, stream); ++ ++ if (!retval) { ++ if (feof(stream)) { ++ /* Program has closed stream (probably it exited) */ ++ /* TODO: check error */ ++ return IO_STREAM_CLOSED; ++ } else { ++ if (EAGAIN == errno) { ++ /* Nothing more to read, try again next time */ ++ return IO_STREAM_EAGAIN; + } else { - log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", buf); ++ /* There was a problem, abandon this child process */ ++ return IO_STREAM_TERM; + } + } ++ } else { ++ len = strlen(buf_out); ++ tor_assert(len>0); ++ ++ if (buf_out[len - 1] == '\n') { ++ /* Remove the trailing newline */ ++ buf_out[len - 1] = '\0'; ++ } else { ++ /* No newline; check whether we overflowed the buffer */ ++ if (!feof(stream)) ++ log_info(LD_GENERAL, ++ "Line from stream was truncated: %s", buf_out); ++ /* TODO: What to do with this error? */ ++ } ++ ++ return IO_STREAM_OKAY; + } + + /* We should never get here */ - return -1; ++ return IO_STREAM_TERM; +} - #endif void tor_check_port_forwarding(const char *filename, int dir_port, int or_port, @@@ -3883,26 -3343,24 +3975,26 @@@ /* Assume tor-fw-helper will succeed, start it later*/ time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS; - child_pid = tor_spawn_background(filename, &fd_out, &fd_err, argv, NULL); - if (child_pid < 0) { +#ifdef MS_WINDOWS + /* Passing NULL as lpApplicationName makes Windows search for the .exe */ - tor_spawn_background(NULL, argv, &child_handle); ++ tor_spawn_background(NULL, argv, NULL &child_handle); +#else - tor_spawn_background(filename, argv, &child_handle); ++ tor_spawn_background(filename, argv, NULL, &child_handle); +#endif + if (PROCESS_STATUS_ERROR == child_handle.status) { log_warn(LD_GENERAL, "Failed to start port forwarding helper %s", filename); - child_pid = -1; + time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL; return; } - /* Set stdout/stderr pipes to be non-blocking */ - fcntl(fd_out, F_SETFL, O_NONBLOCK); - fcntl(fd_err, F_SETFL, O_NONBLOCK); - /* Open the buffered IO streams */ - stdout_read = fdopen(fd_out, "r"); - stderr_read = fdopen(fd_err, "r"); - +#ifdef MS_WINDOWS log_info(LD_GENERAL, - "Started port forwarding helper (%s) with pid %d", filename, child_pid); + "Started port forwarding helper (%s)", filename); +#else + log_info(LD_GENERAL, + "Started port forwarding helper (%s) with pid %d", filename, + child_handle.pid); +#endif } /* If child is running, read from its stdout and stderr) */ diff --cc src/common/util.h index c8cce39,04ae7cb..77ed1ca --- a/src/common/util.h +++ b/src/common/util.h @@@ -348,57 -352,18 +360,62 @@@ void write_pidfile(char *filename) void tor_check_port_forwarding(const char *filename, int dir_port, int or_port, time_t now); + int tor_terminate_process(pid_t pid); -int tor_spawn_background(const char *const filename, int *stdout_read, - int *stderr_read, const char **argv, - const char **envp); ++typedef struct process_handle_s process_handle_t; ++int tor_spawn_background(const char *const filename, const char **argv, ++ const char **envp, process_handle_t *process_handle); ++ + #define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code " + #ifdef MS_WINDOWS HANDLE load_windows_system_library(const TCHAR *library_name); #endif #ifdef UTIL_PRIVATE /* Prototypes for private functions only used by util.c (and unit tests) */ + +/* Values of process_handle_t.status. PROCESS_STATUS_NOTRUNNING must be + * 0 because tor_check_port_forwarding depends on this being the initial + * statue of the static instance of process_handle_t */ +#define PROCESS_STATUS_NOTRUNNING 0 +#define PROCESS_STATUS_RUNNING 1 +#define PROCESS_STATUS_ERROR -1 - typedef struct process_handle_s { ++struct process_handle_s { + int status; +#ifdef MS_WINDOWS + HANDLE stdout_pipe; + HANDLE stderr_pipe; + PROCESS_INFORMATION pid; +#else + int stdout_pipe; + int stderr_pipe; + FILE *stdout_handle; + FILE *stderr_handle; + pid_t pid; +#endif // MS_WINDOWS - } process_handle_t; - - int tor_spawn_background(const char *const filename, const char **argv, - process_handle_t *process_handle); ++}; + +/* Return values of tor_get_exit_code() */ +#define PROCESS_EXIT_RUNNING 1 +#define PROCESS_EXIT_EXITED 0 +#define PROCESS_EXIT_ERROR -1 +int tor_get_exit_code(const process_handle_t process_handle, + int block, int *exit_code); +int tor_split_lines(struct smartlist_t *sl, char *buf, int len); +#ifdef MS_WINDOWS +ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count, + const process_handle_t *process); +#else +ssize_t tor_read_all_handle(FILE *h, char *buf, size_t count, + const process_handle_t *process, + int *eof); +#endif +ssize_t tor_read_all_from_process_stdout( + const process_handle_t *process_handle, char *buf, size_t count); +ssize_t tor_read_all_from_process_stderr( + const process_handle_t *process_handle, char *buf, size_t count); +char *tor_join_win_cmdline(const char *argv[]); ++ void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno); diff --cc src/or/config.c index c44b09c,536324d..07f0082 --- a/src/or/config.c +++ b/src/or/config.c @@@ -3722,11 -3699,14 +3738,16 @@@ options_validate(or_options_t *old_opti if (validate_dir_authorities(options, old_options) < 0) REJECT("Directory authority line did not parse. See logs for details."); + if (options->UseBridges && !options->Bridges) + REJECT("If you set UseBridges, you must specify at least one bridge."); if (options->UseBridges && !options->TunnelDirConns) - REJECT("TunnelDirConns set to 0 only works with UseBridges set to 0"); + REJECT("If you set UseBridges, you must set TunnelDirConns."); + for (cl = options->Bridges; cl; cl = cl->next) { + if (parse_bridge_line(cl->value, 1)<0) + REJECT("Bridge line did not parse. See logs for details."); + } + for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { if (parse_client_transport_line(cl->value, 1)<0) REJECT("Transport line did not parse. See logs for details."); diff --cc src/or/config.h index 4a5afdf,45baf45..76f6841 --- a/src/or/config.h +++ b/src/or/config.h @@@ -64,10 -63,11 +64,15 @@@ or_state_t *get_or_state(void) int did_last_state_file_write_fail(void); int or_state_save(time_t now); +const smartlist_t *get_configured_client_ports(void); + +int options_need_geoip_info(const or_options_t *options, + const char **reason_out); ++ + void save_transport_to_state(const char *transport_name, + const tor_addr_t *addr, uint16_t port); + const char *get_bindaddr_for_transport(const char *transport); + -int options_need_geoip_info(or_options_t *options, const char **reason_out); int getinfo_helper_config(control_connection_t *conn, const char *question, char **answer, const char **errmsg); diff --cc src/or/main.c index 4948d59,6297f0f..aa167e1 --- a/src/or/main.c +++ b/src/or/main.c @@@ -1086,7 -1068,8 +1087,9 @@@ run_scheduled_events(time_t now static int should_init_bridge_stats = 1; static time_t time_to_retry_dns_init = 0; static time_t time_to_next_heartbeat = 0; + static int has_validated_pt = 0; - or_options_t *options = get_options(); + const or_options_t *options = get_options(); ++ int is_server = server_mode(options); int i; int have_dir_info; diff --cc src/or/transports.c index 0000000,c531fe7..3c533cc mode 000000,100644..100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@@ -1,0 -1,1041 +1,1048 @@@ + /* Copyright (c) 2011, The Tor Project, Inc. */ + /* See LICENSE for licensing information */ + + /** + * \file transports.c + * \brief Pluggable Transports related code. + **/ + + #define PT_PRIVATE + #include "or.h" + #include "config.h" + #include "circuitbuild.h" + #include "transports.h" + #include "util.h" + + static void set_managed_proxy_environment(char ***envp, + const managed_proxy_t *mp); + static INLINE int proxy_configuration_finished(const managed_proxy_t *mp); + + static void managed_proxy_destroy(managed_proxy_t *mp); + + static void handle_finished_proxy(managed_proxy_t *mp); + static void configure_proxy(managed_proxy_t *mp); + + static void parse_method_error(const char *line, int is_server_method); + #define parse_server_method_error(l) parse_method_error(l, 1) + #define parse_client_method_error(l) parse_method_error(l, 0) + + static INLINE void free_execve_args(char **arg); + + /** Managed proxy protocol strings */ + #define PROTO_ENV_ERROR "ENV-ERROR" + #define PROTO_NEG_SUCCESS "VERSION" + #define PROTO_NEG_FAIL "VERSION-ERROR no-version" + #define PROTO_CMETHOD "CMETHOD" + #define PROTO_SMETHOD "SMETHOD" + #define PROTO_CMETHOD_ERROR "CMETHOD-ERROR" + #define PROTO_SMETHOD_ERROR "SMETHOD-ERROR" + #define PROTO_CMETHODS_DONE "CMETHODS DONE" + #define PROTO_SMETHODS_DONE "SMETHODS DONE" + + /* The smallest valid managed proxy protocol line that can + appear. It's the size of "VERSION 1" */ + #define SMALLEST_MANAGED_LINE_SIZE 9 + + /** Number of environment variables for managed proxy clients/servers. */ + #define ENVIRON_SIZE_CLIENT 5 + #define ENVIRON_SIZE_SERVER 8 + + /** The first and only supported - at the moment - configuration + protocol version. */ + #define PROTO_VERSION_ONE 1 + + /** List of unconfigured managed proxies. */ + static smartlist_t *managed_proxy_list = NULL; + /** Number of still unconfigured proxies. */ + static int unconfigured_proxies_n = 0; + + /** "The main idea is:" + + Each managed proxy is represented by a 'managed_proxy_t'. + Each managed proxy can support multiple transports. + Each managed proxy gets configured through a multistep process. + + 'managed_proxy_list' contains all the managed proxies this tor + instance is supporting. + In the 'managed_proxy_list' there are 'unconfigured_proxies_n' + managed proxies that are still unconfigured. + + In every run_scheduled_event() tick, we attempt to launch and then + configure the unconfiged managed proxies, using the configuration + protocol defined in the 180_pluggable_transport.txt proposal. A + managed proxy might need several ticks to get fully configured. + + When a managed proxy is fully configured, we register all its + transports to the circuitbuild.c subsystem. At that point the + transports are owned by the circuitbuild.c subsystem. + + When a managed proxy fails to follow the 180 configuration + protocol, it gets marked as broken and gets destroyed. + + "In a little more technical detail:" + + While we are serially parsing torrc, we store all the transports + that a proxy should spawn in its 'transports_to_launch' element. + + When we finish reading the torrc, we spawn the managed proxy and + expect {S,C}METHOD lines from its output. We add transports + described by METHOD lines to its 'transports' element, as + 'transport_t' structs. + + When the managed proxy stops spitting METHOD lines (signified by a + '{S,C}METHODS DONE' message) we register all the transports + collected to the circuitbuild.c subsystem. At this point, the + 'transport_t's can be transformed into dangling pointers at any + point by the circuitbuild.c subsystem, and so we replace all + 'transport_t's with strings describing the transport names. We + can still go from a transport name to a 'transport_t' using the + fact that transport names uniquely identify 'transport_t's. + + "In even more technical detail I shall describe what happens when + the SIGHUP bell tolls:" + + We immediately destroy all unconfigured proxies (We shouldn't have + unconfigured proxies in the first place, except when SIGHUP rings + immediately after tor is launched.). + + We mark all managed proxies and transports to signify that they + must be removed if they don't contribute by the new torrc + (marked_for_removal). + We also mark all managed proxies to signify that they might need + to be restarted so that they end up supporting all the transports + the new torrc wants them to support (got_hup). + We also clear their 'transports_to_launch' list so that we can put + there the transports we need to launch according to the new torrc. + + We then start parsing torrc again. + + Everytime we encounter a transport line using a known pre-SIGHUP + managed proxy, we cleanse that proxy from the removal mark. + + We also mark it as unconfigured so that on the next scheduled + events tick, we investigate whether we need to restart the proxy + so that it also spawns the new transports. + If the post-SIGHUP 'transports_to_launch' list is identical to the + pre-SIGHUP one, it means that no changes were introduced to this + proxy during the SIGHUP and no restart has to take place. + + During the post-SIGHUP torrc parsing, we unmark all transports + spawned by managed proxies that we find in our torrc. + We do that so that if we don't need to restart a managed proxy, we + can continue using its old transports normally. + If we end up restarting the proxy, we destroy and unregister all + old transports from the circuitbuild.c subsystem. + */ + + /** Return true if there are still unconfigured managed proxies. */ + int + pt_proxies_configuration_pending(void) + { + return !! unconfigured_proxies_n; + } + + /** Return true if mp has the same argv as proxy_argv */ + static int + managed_proxy_has_argv(const managed_proxy_t *mp, char **proxy_argv) + { + char **tmp1=proxy_argv; + char **tmp2=mp->argv; + + tor_assert(tmp1); + tor_assert(tmp2); + + while (*tmp1 && *tmp2) { + if (strcmp(*tmp1++, *tmp2++)) + return 0; + } + + if (!*tmp1 && !*tmp2) + return 1; + + return 0; + } + + /** Return a managed proxy with the same argv as proxy_argv. + * If no such managed proxy exists, return NULL. */ + static managed_proxy_t * + get_managed_proxy_by_argv_and_type(char **proxy_argv, int is_server) + { + if (!managed_proxy_list) + return NULL; + + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + if (managed_proxy_has_argv(mp, proxy_argv) && + mp->is_server == is_server) + return mp; + } SMARTLIST_FOREACH_END(mp); + + return NULL; + } + + /** Add transport to managed proxy mp. */ + static void + add_transport_to_proxy(const char *transport, managed_proxy_t *mp) + { + tor_assert(mp->transports_to_launch); + if (!smartlist_string_isin(mp->transports_to_launch, transport)) + smartlist_add(mp->transports_to_launch, tor_strdup(transport)); + } + + /** Called when a SIGHUP occurs. Returns true if managed proxy + * mp needs to be restarted after the SIGHUP, based on the new + * torrc. */ + static int + proxy_needs_restart(const managed_proxy_t *mp) + { + /* mp->transport_to_launch is populated with the names of the + transports that must be launched *after* the SIGHUP. + mp->transports is populated with the names of the transports that + were launched *before* the SIGHUP. + + If the two lists contain the same strings, we don't need to + restart the proxy, since it already does what we want. */ + + tor_assert(smartlist_len(mp->transports_to_launch) > 0); + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + + if (smartlist_len(mp->transports_to_launch) != smartlist_len(mp->transports)) + goto needs_restart; + + SMARTLIST_FOREACH_BEGIN(mp->transports_to_launch, char *, t_t_l) { + if (!smartlist_string_isin(mp->transports, t_t_l)) + goto needs_restart; + + } SMARTLIST_FOREACH_END(t_t_l); + + return 0; + + needs_restart: + return 1; + } + + /** Managed proxy mp must be restarted. Do all the necessary + * preparations and then flag its state so that it will be relaunched + * in the next tick. */ + static void + proxy_prepare_for_restart(managed_proxy_t *mp) + { + transport_t *t_tmp = NULL; + + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + tor_assert(mp->pid); + + /* kill the old obfsproxy process */ + tor_terminate_process(mp->pid); + mp->pid = 0; + fclose(mp->_stdout); + + /* destroy all its old transports. we no longer use them. */ + SMARTLIST_FOREACH_BEGIN(mp->transports, const char *, t_name) { + t_tmp = transport_get_by_name(t_name); + if (t_tmp) + t_tmp->marked_for_removal = 1; + } SMARTLIST_FOREACH_END(t_name); + sweep_transport_list(); + + /* free the transport names in mp->transports */ + SMARTLIST_FOREACH(mp->transports, char *, t_name, tor_free(t_name)); + smartlist_clear(mp->transports); + + /* flag it as an infant proxy so that it gets launched on next tick */ + mp->conf_state = PT_PROTO_INFANT; + } + + /** Launch managed proxy mp. */ + static int + launch_managed_proxy(managed_proxy_t *mp) + { ++ (void) mp; ++ (void) set_managed_proxy_environment; ++ return -1; ++#if 0 ++ /* XXXX023 we must reenable this code for managed proxies to work. ++ * "All it needs" is revision to work with the new tor_spawn_background ++ * API. */ + char **envp=NULL; + int pid; ++ process_handle_t proc; + FILE *stdout_read = NULL; + int stdout_pipe=-1, stderr_pipe=-1; + + /* prepare the environment variables for the managed proxy */ + set_managed_proxy_environment(&envp, mp); + - pid = tor_spawn_background(mp->argv[0], &stdout_pipe, - &stderr_pipe, (const char **)mp->argv, - (const char **)envp); ++ pid = tor_spawn_background(mp->argv[0], (const char **)mp->argv, ++ (const char **)envp, &proc); + if (pid < 0) { + log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", + mp->argv[0]); + return -1; + } + + /* free the memory allocated by set_managed_proxy_environment(). */ + free_execve_args(envp); + + /* Set stdout/stderr pipes to be non-blocking */ + #ifdef _WIN32 + { + u_long nonblocking = 1; + ioctlsocket(stdout_pipe, FIONBIO, &nonblocking); + } + #else + fcntl(stdout_pipe, F_SETFL, O_NONBLOCK); + #endif + + /* Open the buffered IO streams */ + stdout_read = fdopen(stdout_pipe, "r"); + + log_info(LD_CONFIG, "Managed proxy has spawned at PID %d.", pid); + + mp->conf_state = PT_PROTO_LAUNCHED; + mp->_stdout = stdout_read; + mp->pid = pid; - ++#endif + return 0; + } + + /** Check if any of the managed proxies we are currently trying to + * configure have anything new to say. This is called from + * run_scheduled_events(). */ + void + pt_configure_remaining_proxies(void) + { + log_debug(LD_CONFIG, "Configuring remaining managed proxies (%d)!", + unconfigured_proxies_n); + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + tor_assert(mp->conf_state != PT_PROTO_BROKEN); + + if (mp->got_hup) { + mp->got_hup = 0; + + /* This proxy is marked by a SIGHUP. Check whether we need to + restart it. */ + if (proxy_needs_restart(mp)) { + log_info(LD_GENERAL, "Preparing managed proxy for restart."); + proxy_prepare_for_restart(mp); + continue; + } else { /* it doesn't need to be restarted. */ + log_info(LD_GENERAL, "Nothing changed for managed proxy after HUP: " + "not restarting."); + unconfigured_proxies_n--; + tor_assert(unconfigured_proxies_n >= 0); + } + + continue; + } + + /* If the proxy is not fully configured, try to configure it + futher. */ + if (!proxy_configuration_finished(mp)) + configure_proxy(mp); + + } SMARTLIST_FOREACH_END(mp); + } + + /** Attempt to continue configuring managed proxy mp. */ + static void + configure_proxy(managed_proxy_t *mp) + { + enum stream_status r; + char stdout_buf[200]; + + /* if we haven't launched the proxy yet, do it now */ + if (mp->conf_state == PT_PROTO_INFANT) { + launch_managed_proxy(mp); + return; + } + + tor_assert(mp->conf_state != PT_PROTO_INFANT); + + while (1) { + r = get_string_from_pipe(mp->_stdout, stdout_buf, + sizeof(stdout_buf) - 1); + + if (r == IO_STREAM_OKAY) { /* got a line; handle it! */ + handle_proxy_line((const char *)stdout_buf, mp); + } else if (r == IO_STREAM_EAGAIN) { /* check back later */ + return; + } else if (r == IO_STREAM_CLOSED || r == IO_STREAM_TERM) { /* snap! */ + log_notice(LD_GENERAL, "Managed proxy stream closed. " + "Most probably application stopped running"); + mp->conf_state = PT_PROTO_BROKEN; + } else { /* unknown stream status */ + log_notice(LD_GENERAL, "Unknown stream status while configuring proxy."); + } + + /* if the proxy finished configuring, exit the loop. */ + if (proxy_configuration_finished(mp)) { + handle_finished_proxy(mp); + return; + } + } + } + + /** Register server managed proxy mp transports to state */ + static void + register_server_proxy(managed_proxy_t *mp) + { + /* After we register this proxy's transports, we switch its + mp->transports to a list containing strings of its transport + names. (See transports.h) */ + smartlist_t *sm_tmp = smartlist_create(); + + tor_assert(mp->conf_state != PT_PROTO_COMPLETED); + SMARTLIST_FOREACH_BEGIN(mp->transports, transport_t *, t) { + save_transport_to_state(t->name, &t->addr, t->port); + smartlist_add(sm_tmp, tor_strdup(t->name)); + } SMARTLIST_FOREACH_END(t); + + /* Since server proxies don't register their transports in the + circuitbuild.c subsystem, it's our duty to free them when we + switch mp->transports to strings. */ + SMARTLIST_FOREACH(mp->transports, transport_t *, t, transport_free(t)); + smartlist_free(mp->transports); + + mp->transports = sm_tmp; + } + + /** Register all the transports supported by client managed proxy + * mp to the bridge subsystem. */ + static void + register_client_proxy(managed_proxy_t *mp) + { + int r; + /* After we register this proxy's transports, we switch its + mp->transports to a list containing strings of its transport + names. (See transports.h) */ + smartlist_t *sm_tmp = smartlist_create(); + + tor_assert(mp->conf_state != PT_PROTO_COMPLETED); + SMARTLIST_FOREACH_BEGIN(mp->transports, transport_t *, t) { + r = transport_add(t); + switch (r) { + case -1: + log_notice(LD_GENERAL, "Could not add transport %s. Skipping.", t->name); + transport_free(t); + break; + case 0: + log_info(LD_GENERAL, "Succesfully registered transport %s", t->name); + smartlist_add(sm_tmp, tor_strdup(t->name)); + break; + case 1: + log_info(LD_GENERAL, "Succesfully registered transport %s", t->name); + smartlist_add(sm_tmp, tor_strdup(t->name)); + transport_free(t); + break; + } + } SMARTLIST_FOREACH_END(t); + + smartlist_free(mp->transports); + mp->transports = sm_tmp; + } + + /** Register the transports of managed proxy mp. */ + static INLINE void + register_proxy(managed_proxy_t *mp) + { + if (mp->is_server) + register_server_proxy(mp); + else + register_client_proxy(mp); + } + + /** Free memory allocated by managed proxy mp. */ + static void + managed_proxy_destroy(managed_proxy_t *mp) + { + if (mp->conf_state != PT_PROTO_COMPLETED) + SMARTLIST_FOREACH(mp->transports, transport_t *, t, transport_free(t)); + else + SMARTLIST_FOREACH(mp->transports, char *, t_name, tor_free(t_name)); + + /* free the transports smartlist */ + smartlist_free(mp->transports); + + /* free the transports_to_launch smartlist */ + SMARTLIST_FOREACH(mp->transports_to_launch, char *, t, tor_free(t)); + smartlist_free(mp->transports_to_launch); + + /* remove it from the list of managed proxies */ + smartlist_remove(managed_proxy_list, mp); + + /* close its stdout stream */ + if (mp->_stdout) + fclose(mp->_stdout); + + /* free the argv */ + free_execve_args(mp->argv); + + if (mp->pid) + tor_terminate_process(mp->pid); + + tor_free(mp); + } + + /** Handle a configured or broken managed proxy mp. */ + static void + handle_finished_proxy(managed_proxy_t *mp) + { + switch (mp->conf_state) { + case PT_PROTO_BROKEN: /* if broken: */ + managed_proxy_destroy(mp); /* annihilate it. */ + break; + case PT_PROTO_CONFIGURED: /* if configured correctly: */ + register_proxy(mp); /* register its transports */ + mp->conf_state = PT_PROTO_COMPLETED; /* and mark it as completed. */ + break; + case PT_PROTO_INFANT: + case PT_PROTO_LAUNCHED: + case PT_PROTO_ACCEPTING_METHODS: + case PT_PROTO_COMPLETED: + default: + log_warn(LD_CONFIG, "Unexpected managed proxy state in " + "handle_finished_proxy()."); + tor_assert(0); + } + + unconfigured_proxies_n--; + tor_assert(unconfigured_proxies_n >= 0); + } + + /** Return true if the configuration of the managed proxy mp is + finished. */ + static INLINE int + proxy_configuration_finished(const managed_proxy_t *mp) + { + return (mp->conf_state == PT_PROTO_CONFIGURED || + mp->conf_state == PT_PROTO_BROKEN); + } + + /** This function is called when a proxy sends an {S,C}METHODS DONE message. */ + static void + handle_methods_done(const managed_proxy_t *mp) + { + tor_assert(mp->transports); + + if (smartlist_len(mp->transports) == 0) + log_notice(LD_GENERAL, "Proxy was spawned successfully, " + "but it didn't laucn any pluggable transport listeners!"); + + log_info(LD_CONFIG, "%s managed proxy configuration completed!", + mp->is_server ? "Server" : "Client"); + } + + /** Handle a configuration protocol line received from a + * managed proxy mp. */ + void + handle_proxy_line(const char *line, managed_proxy_t *mp) + { + log_debug(LD_GENERAL, "Got a line from managed proxy: %s\n", line); + + if (strlen(line) < SMALLEST_MANAGED_LINE_SIZE) { + log_warn(LD_GENERAL, "Managed proxy configuration line is too small. " + "Discarding"); + goto err; + } + + if (!strcmpstart(line, PROTO_ENV_ERROR)) { + if (mp->conf_state != PT_PROTO_LAUNCHED) + goto err; + + parse_env_error(line); + goto err; + } else if (!strcmpstart(line, PROTO_NEG_FAIL)) { + if (mp->conf_state != PT_PROTO_LAUNCHED) + goto err; + + log_warn(LD_CONFIG, "Managed proxy could not pick a " + "configuration protocol version."); + goto err; + } else if (!strcmpstart(line, PROTO_NEG_SUCCESS)) { + if (mp->conf_state != PT_PROTO_LAUNCHED) + goto err; + + if (parse_version(line,mp) < 0) + goto err; + + tor_assert(mp->conf_protocol != 0); + mp->conf_state = PT_PROTO_ACCEPTING_METHODS; + return; + } else if (!strcmpstart(line, PROTO_CMETHODS_DONE)) { + if (mp->conf_state != PT_PROTO_ACCEPTING_METHODS) + goto err; + + handle_methods_done(mp); + + mp->conf_state = PT_PROTO_CONFIGURED; + return; + } else if (!strcmpstart(line, PROTO_SMETHODS_DONE)) { + if (mp->conf_state != PT_PROTO_ACCEPTING_METHODS) + goto err; + + handle_methods_done(mp); + + mp->conf_state = PT_PROTO_CONFIGURED; + return; + } else if (!strcmpstart(line, PROTO_CMETHOD_ERROR)) { + if (mp->conf_state != PT_PROTO_ACCEPTING_METHODS) + goto err; + + parse_client_method_error(line); + goto err; + } else if (!strcmpstart(line, PROTO_SMETHOD_ERROR)) { + if (mp->conf_state != PT_PROTO_ACCEPTING_METHODS) + goto err; + + parse_server_method_error(line); + goto err; + } else if (!strcmpstart(line, PROTO_CMETHOD)) { + if (mp->conf_state != PT_PROTO_ACCEPTING_METHODS) + goto err; + + if (parse_cmethod_line(line, mp) < 0) + goto err; + + return; + } else if (!strcmpstart(line, PROTO_SMETHOD)) { + if (mp->conf_state != PT_PROTO_ACCEPTING_METHODS) + goto err; + + if (parse_smethod_line(line, mp) < 0) + goto err; + + return; + } else if (!strcmpstart(line, SPAWN_ERROR_MESSAGE)) { + log_warn(LD_GENERAL, "Could not launch managed proxy executable!"); + goto err; + } + + log_warn(LD_CONFIG, "Unknown line received by managed proxy. (%s)", line); + + err: + mp->conf_state = PT_PROTO_BROKEN; + return; + } + + /** Parses an ENV-ERROR line and warns the user accordingly. */ + void + parse_env_error(const char *line) + { + /* (Length of the protocol string) plus (a space) and (the first char of + the error message) */ + if (strlen(line) < (strlen(PROTO_ENV_ERROR) + 2)) + log_notice(LD_CONFIG, "Managed proxy sent us an %s without an error " + "message.", PROTO_ENV_ERROR); + + log_warn(LD_CONFIG, "Managed proxy couldn't understand the " + "pluggable transport environment variables. (%s)", + line+strlen(PROTO_ENV_ERROR)+1); + } + + /** Handles a VERSION line. Updates the configuration protocol + * version in mp. */ + int + parse_version(const char *line, managed_proxy_t *mp) + { + if (strlen(line) < (strlen(PROTO_NEG_SUCCESS) + 2)) { + log_warn(LD_CONFIG, "Managed proxy sent us malformed %s line.", + PROTO_NEG_SUCCESS); + return -1; + } + + if (strcmp("1", line+strlen(PROTO_NEG_SUCCESS)+1)) { /* hardcoded temp */ + log_warn(LD_CONFIG, "Managed proxy tried to negotiate on version '%s'. " + "We only support version '1'", line+strlen(PROTO_NEG_SUCCESS)+1); + return -1; + } + + mp->conf_protocol = PROTO_VERSION_ONE; /* temp. till more versions appear */ + return 0; + } + + /** Parses {C,S}METHOD-ERROR line and warns the user + * accordingly. If is_server it is an SMETHOD-ERROR, + * otherwise it is a CMETHOD-ERROR. */ + static void + parse_method_error(const char *line, int is_server) + { + const char* error = is_server ? + PROTO_SMETHOD_ERROR : PROTO_CMETHOD_ERROR; + + /* (Length of the protocol string) plus (a space) and (the first char of + the error message) */ + if (strlen(line) < (strlen(error) + 2)) + log_warn(LD_CONFIG, "Managed proxy sent us an %s without an error " + "message.", error); + + log_warn(LD_CONFIG, "%s managed proxy encountered a method error. (%s)", + is_server ? "Server" : "Client", + line+strlen(error)+1); + } + + /** Parses an SMETHOD line and if well-formed it registers the + * new transport in mp. */ + int + parse_smethod_line(const char *line, managed_proxy_t *mp) + { + int r; + smartlist_t *items = NULL; + + char *method_name=NULL; + + char *addrport=NULL; + tor_addr_t addr; + uint16_t port = 0; + + transport_t *transport=NULL; + + items = smartlist_create(); + smartlist_split_string(items, line, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); + if (smartlist_len(items) < 3) { + log_warn(LD_CONFIG, "Server managed proxy sent us a SMETHOD line " + "with too few arguments."); + goto err; + } + + tor_assert(!strcmp(smartlist_get(items,0),PROTO_SMETHOD)); + + method_name = smartlist_get(items,1); + if (!string_is_C_identifier(method_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + method_name); + goto err; + } + + addrport = smartlist_get(items, 2); + if (tor_addr_port_parse(addrport, &addr, &port)<0) { + log_warn(LD_CONFIG, "Error parsing transport " + "address '%s'", addrport); + goto err; + } + + if (!port) { + log_warn(LD_CONFIG, + "Transport address '%s' has no port.", addrport); + goto err; + } + + transport = transport_create(&addr, port, method_name, PROXY_NONE); + if (!transport) + goto err; + + smartlist_add(mp->transports, transport); + + /* For now, notify the user so that he knows where the server + transport is listening. */ + log_info(LD_CONFIG, "Server transport %s at %s:%d.", + method_name, fmt_addr(&addr), (int)port); + + r=0; + goto done; + + err: + r = -1; + + done: + SMARTLIST_FOREACH(items, char*, s, tor_free(s)); + smartlist_free(items); + return r; + } + + /** Parses a CMETHOD line, and if well-formed it registers + * the new transport in mp. */ + int + parse_cmethod_line(const char *line, managed_proxy_t *mp) + { + int r; + smartlist_t *items = NULL; + + char *method_name=NULL; + + char *socks_ver_str=NULL; + int socks_ver=PROXY_NONE; + + char *addrport=NULL; + tor_addr_t addr; + uint16_t port = 0; + + transport_t *transport=NULL; + + items = smartlist_create(); + smartlist_split_string(items, line, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); + if (smartlist_len(items) < 4) { + log_warn(LD_CONFIG, "Client managed proxy sent us a CMETHOD line " + "with too few arguments."); + goto err; + } + + tor_assert(!strcmp(smartlist_get(items,0),PROTO_CMETHOD)); + + method_name = smartlist_get(items,1); + if (!string_is_C_identifier(method_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + method_name); + goto err; + } + + socks_ver_str = smartlist_get(items,2); + + if (!strcmp(socks_ver_str,"socks4")) { + socks_ver = PROXY_SOCKS4; + } else if (!strcmp(socks_ver_str,"socks5")) { + socks_ver = PROXY_SOCKS5; + } else { + log_warn(LD_CONFIG, "Client managed proxy sent us a proxy protocol " + "we don't recognize. (%s)", socks_ver_str); + goto err; + } + + addrport = smartlist_get(items, 3); + if (tor_addr_port_parse(addrport, &addr, &port)<0) { + log_warn(LD_CONFIG, "Error parsing transport " + "address '%s'", addrport); + goto err; + } + + if (!port) { + log_warn(LD_CONFIG, + "Transport address '%s' has no port.", addrport); + goto err; + } + + transport = transport_create(&addr, port, method_name, socks_ver); + if (!transport) + goto err; + + smartlist_add(mp->transports, transport); + + log_info(LD_CONFIG, "Transport %s at %s:%d with SOCKS %d. " + "Attached to managed proxy.", + method_name, fmt_addr(&addr), (int)port, socks_ver); + + r=0; + goto done; + + err: + r = -1; + + done: + SMARTLIST_FOREACH(items, char*, s, tor_free(s)); + smartlist_free(items); + return r; + } + + /** Return a string containing the address:port that transport + * should use. It's the responsibility of the caller to free() the + * received string. */ + static char * + get_bindaddr_for_proxy(const managed_proxy_t *mp) + { + char *bindaddr = NULL; + smartlist_t *string_tmp = smartlist_create(); + + tor_assert(mp->is_server); + + SMARTLIST_FOREACH_BEGIN(mp->transports_to_launch, char *, t) { + tor_asprintf(&bindaddr, "%s-%s", t, get_bindaddr_for_transport(t)); + smartlist_add(string_tmp, bindaddr); + } SMARTLIST_FOREACH_END(t); + + bindaddr = smartlist_join_strings(string_tmp, ",", 0, NULL); + + SMARTLIST_FOREACH(string_tmp, char *, t, tor_free(t)); + smartlist_free(string_tmp); + + return bindaddr; + } + + /** Prepare the envp of managed proxy mp */ + static void + set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) + { - or_options_t *options = get_options(); ++ const or_options_t *options = get_options(); + char **tmp=NULL; + char *state_loc=NULL; + char *transports_to_launch=NULL; + char *bindaddr=NULL; + + int n_envs = mp->is_server ? ENVIRON_SIZE_SERVER : ENVIRON_SIZE_CLIENT; + + /* allocate enough space for our env. vars and a NULL pointer */ + *envp = tor_malloc(sizeof(char*)*(n_envs+1)); + tmp = *envp; + + state_loc = get_datadir_fname("pt_state/"); /* XXX temp */ + transports_to_launch = + smartlist_join_strings(mp->transports_to_launch, ",", 0, NULL); + + tor_asprintf(tmp++, "HOME=%s", getenv("HOME")); + tor_asprintf(tmp++, "PATH=%s", getenv("PATH")); + tor_asprintf(tmp++, "TOR_PT_STATE_LOCATION=%s", state_loc); + tor_asprintf(tmp++, "TOR_PT_MANAGED_TRANSPORT_VER=1"); /* temp */ + if (mp->is_server) { + bindaddr = get_bindaddr_for_proxy(mp); + + /* XXX temp */ + tor_asprintf(tmp++, "TOR_PT_ORPORT=127.0.0.1:%d", options->ORPort); + tor_asprintf(tmp++, "TOR_PT_SERVER_BINDADDR=%s", bindaddr); + tor_asprintf(tmp++, "TOR_PT_SERVER_TRANSPORTS=%s", transports_to_launch); + /* XXX temp*/ + tor_asprintf(tmp++, "TOR_PT_EXTENDED_SERVER_PORT=127.0.0.1:4200"); + } else { + tor_asprintf(tmp++, "TOR_PT_CLIENT_TRANSPORTS=%s", transports_to_launch); + } + *tmp = NULL; + + tor_free(state_loc); + tor_free(transports_to_launch); + tor_free(bindaddr); + } + + /** Create and return a new managed proxy for transport using + * proxy_argv. If is_server is true, it's a server + * managed proxy. */ + static managed_proxy_t * + managed_proxy_create(const smartlist_t *transport_list, + char **proxy_argv, int is_server) + { + managed_proxy_t *mp = tor_malloc_zero(sizeof(managed_proxy_t)); + mp->conf_state = PT_PROTO_INFANT; + mp->is_server = is_server; + mp->argv = proxy_argv; + mp->transports = smartlist_create(); + + mp->transports_to_launch = smartlist_create(); + SMARTLIST_FOREACH(transport_list, const char *, transport, + add_transport_to_proxy(transport, mp)); + + /* register the managed proxy */ + if (!managed_proxy_list) + managed_proxy_list = smartlist_create(); + smartlist_add(managed_proxy_list, mp); + unconfigured_proxies_n++; + + return mp; + } + + /** Register transport using proxy with proxy_argv to + * the managed proxy subsystem. + * If is_server is true, then the proxy is a server proxy. */ + void + pt_kickstart_proxy(const smartlist_t *transport_list, + char **proxy_argv, int is_server) + { + managed_proxy_t *mp=NULL; + transport_t *old_transport = NULL; + + mp = get_managed_proxy_by_argv_and_type(proxy_argv, is_server); + + if (!mp) { /* we haven't seen this proxy before */ + managed_proxy_create(transport_list, proxy_argv, is_server); + + } else { /* known proxy. add its transport to its transport list */ + if (mp->got_hup) { + /* If the managed proxy we found is marked by a SIGHUP, it means + that it's not useless and should be kept. If it's marked for + removal, unmark it and increase the unconfigured proxies so + that we try to restart it if we need to. Afterwards, check if + a transport_t for 'transport' used to exist before the SIGHUP + and make sure it doesn't get deleted because we might reuse + it. */ + if (mp->marked_for_removal) { + mp->marked_for_removal = 0; + unconfigured_proxies_n++; + } + + SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport) { + old_transport = transport_get_by_name(transport); + if (old_transport) + old_transport->marked_for_removal = 0; + } SMARTLIST_FOREACH_END(transport); + } + + SMARTLIST_FOREACH(transport_list, const char *, transport, + add_transport_to_proxy(transport, mp)); + free_execve_args(proxy_argv); + } + } + + /** Frees the array of pointers in arg used as arguments to + execve(2). */ + static INLINE void + free_execve_args(char **arg) + { + char **tmp = arg; + while (*tmp) /* use the fact that the last element of the array is a + NULL pointer to know when to stop freeing */ + _tor_free(*tmp++); + + tor_free(arg); + } + + /** Tor will read its config. + * Prepare the managed proxy list so that proxies not used in the new + * config will shutdown, and proxies that need to spawn different + * transports will do so. */ + void + pt_prepare_proxy_list_for_config_read(void) + { + if (!managed_proxy_list) + return; + + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + /* Destroy unconfigured proxies. */ + if (mp->conf_state != PT_PROTO_COMPLETED) { + managed_proxy_destroy(mp); + unconfigured_proxies_n--; + continue; + } + + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + + mp->marked_for_removal = 1; + mp->got_hup = 1; + SMARTLIST_FOREACH(mp->transports_to_launch, char *, t, tor_free(t)); + smartlist_clear(mp->transports_to_launch); + } SMARTLIST_FOREACH_END(mp); + + tor_assert(unconfigured_proxies_n == 0); + } + + /** The tor config was read. + * Destroy all managed proxies that were marked by a previous call to + * prepare_proxy_list_for_config_read() and are not used by the new + * config. */ + void + sweep_proxy_list(void) + { + if (!managed_proxy_list) + return; + + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + if (mp->marked_for_removal) { + SMARTLIST_DEL_CURRENT(managed_proxy_list, mp); + managed_proxy_destroy(mp); + } + } SMARTLIST_FOREACH_END(mp); + } + + /** Release all storage held by the pluggable transports subsystem. */ + void + pt_free_all(void) + { + if (managed_proxy_list) { + /* If the proxy is in PT_PROTO_COMPLETED, it has registered its + transports and it's the duty of the circuitbuild.c subsystem to + free them. Otherwise, it hasn't registered its transports yet + and we should free them here. */ + SMARTLIST_FOREACH(managed_proxy_list, managed_proxy_t *, mp, + managed_proxy_destroy(mp)); + + smartlist_free(managed_proxy_list); + managed_proxy_list=NULL; + } + } + diff --cc src/test/test_util.c index f9672c1,c778faa..6603ab0 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@@ -1388,42 -1389,34 +1388,42 @@@ run_util_spawn_background(const char *a char stdout_buf[100], stderr_buf[100]; /* Start the program */ - retval = tor_spawn_background(argv[0], &stdout_pipe, &stderr_pipe, - argv, NULL); - tt_int_op(retval, >, 0); - tt_int_op(stdout_pipe, >, 0); - tt_int_op(stderr_pipe, >, 0); - pid = retval; +#ifdef MS_WINDOWS - tor_spawn_background(NULL, argv, &process_handle); ++ tor_spawn_background(NULL, argv, NULL, &process_handle); +#else - tor_spawn_background(argv[0], argv, &process_handle); ++ tor_spawn_background(argv[0], argv, NULL, &process_handle); +#endif + + tt_int_op(process_handle.status, ==, expected_status); + + /* If the process failed to start, don't bother continuing */ + if (process_handle.status == PROCESS_STATUS_ERROR) + return; + + tt_int_op(process_handle.stdout_pipe, >, 0); + tt_int_op(process_handle.stderr_pipe, >, 0); /* Check stdout */ - pos = read_all(stdout_pipe, stdout_buf, sizeof(stdout_buf) - 1, 0); + pos = tor_read_all_from_process_stdout(&process_handle, stdout_buf, + sizeof(stdout_buf) - 1); tt_assert(pos >= 0); stdout_buf[pos] = '\0'; - tt_int_op(pos, ==, strlen(expected_out)); tt_str_op(stdout_buf, ==, expected_out); + tt_int_op(pos, ==, strlen(expected_out)); /* Check it terminated correctly */ - retval = waitpid(pid, &stat_loc, 0); - tt_int_op(retval, ==, pid); - tt_assert(WIFEXITED(stat_loc)); - tt_int_op(WEXITSTATUS(stat_loc), ==, expected_exit); - tt_assert(!WIFSIGNALED(stat_loc)); - tt_assert(!WIFSTOPPED(stat_loc)); + retval = tor_get_exit_code(process_handle, 1, &exit_code); + tt_int_op(retval, ==, PROCESS_EXIT_EXITED); + tt_int_op(exit_code, ==, expected_exit); + // TODO: Make test-child exit with something other than 0 /* Check stderr */ - pos = read_all(stderr_pipe, stderr_buf, sizeof(stderr_buf) - 1, 0); + pos = tor_read_all_from_process_stderr(&process_handle, stderr_buf, + sizeof(stderr_buf) - 1); tt_assert(pos >= 0); stderr_buf[pos] = '\0'; - tt_int_op(pos, ==, strlen(expected_err)); tt_str_op(stderr_buf, ==, expected_err); + tt_int_op(pos, ==, strlen(expected_err)); done: ; @@@ -1471,219 -1446,9 +1471,219 @@@ test_util_spawn_background_fail(void *p (void)ptr; - run_util_spawn_background(argv, expected_out, expected_err, 255); + run_util_spawn_background(argv, expected_out, expected_err, 255, + expected_status); } + +/** Test that reading from a handle returns a partial read rather than + * blocking */ +static void +test_util_spawn_background_partial_read(void *ptr) +{ + const int expected_exit = 0; + const int expected_status = PROCESS_STATUS_RUNNING; + + int retval, exit_code; + ssize_t pos = -1; + process_handle_t process_handle; + char stdout_buf[100], stderr_buf[100]; +#ifdef MS_WINDOWS + const char *argv[] = {"test-child.exe", "--test", NULL}; + const char *expected_out[] = { "OUT\r\n--test\r\nSLEEPING\r\n", + "DONE\r\n", + NULL }; + const char *expected_err = "ERR\r\n"; +#else + const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL}; + const char *expected_out[] = { "OUT\n--test\nSLEEPING\n", + "DONE\n", + NULL }; + const char *expected_err = "ERR\n"; + int eof = 0; +#endif + int expected_out_ctr; + (void)ptr; + + /* Start the program */ +#ifdef MS_WINDOWS - tor_spawn_background(NULL, argv, &process_handle); ++ tor_spawn_background(NULL, argv, NULL, &process_handle); +#else - tor_spawn_background(argv[0], argv, &process_handle); ++ tor_spawn_background(argv[0], argv, NULL, &process_handle); #endif + tt_int_op(process_handle.status, ==, expected_status); + + /* Check stdout */ + for (expected_out_ctr =0; expected_out[expected_out_ctr] != NULL;) { +#ifdef MS_WINDOWS + pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, + sizeof(stdout_buf) - 1, NULL); +#else + /* Check that we didn't read the end of file last time */ + tt_assert(!eof); + pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, + sizeof(stdout_buf) - 1, NULL, &eof); +#endif + log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos); + + /* We would have blocked, keep on trying */ + if (0 == pos) + continue; + + tt_int_op(pos, >, 0); + stdout_buf[pos] = '\0'; + tt_str_op(stdout_buf, ==, expected_out[expected_out_ctr]); + tt_int_op(pos, ==, strlen(expected_out[expected_out_ctr])); + expected_out_ctr++; + } + + /* The process should have exited without writing more */ +#ifdef MS_WINDOWS + pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, + sizeof(stdout_buf) - 1, + &process_handle); + tt_int_op(pos, ==, 0); +#else + if (!eof) { + /* We should have got all the data, but maybe not the EOF flag */ + pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, + sizeof(stdout_buf) - 1, + &process_handle, &eof); + tt_int_op(pos, ==, 0); + tt_assert(eof); + } + /* Otherwise, we got the EOF on the last read */ +#endif + + /* Check it terminated correctly */ + retval = tor_get_exit_code(process_handle, 1, &exit_code); + tt_int_op(retval, ==, PROCESS_EXIT_EXITED); + tt_int_op(exit_code, ==, expected_exit); + + // TODO: Make test-child exit with something other than 0 + + /* Check stderr */ + pos = tor_read_all_from_process_stderr(&process_handle, stderr_buf, + sizeof(stderr_buf) - 1); + tt_assert(pos >= 0); + stderr_buf[pos] = '\0'; + tt_str_op(stderr_buf, ==, expected_err); + tt_int_op(pos, ==, strlen(expected_err)); + + done: + ; +} + +/** + * Test that we can properly format q 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 + {} // 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; + + (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(joined_argv, ==, cmdlines[i]); + tor_free(joined_argv); + } + + done: + ; +} + +#define MAX_SPLIT_LINE_COUNT 3 +struct split_lines_test_t { + const char *orig_line; // Line to be split (may contain \0's) + int orig_length; // Length of orig_line + const char *split_line[MAX_SPLIT_LINE_COUNT]; // Split lines +}; + +/** + * Test that we properly split a buffer into lines + */ +static void +test_util_split_lines(void *ptr) +{ + /* Test cases. orig_line of last test case must be NULL. + * The last element of split_line[i] must be NULL. */ + struct split_lines_test_t tests[] = { + {"", 0, {NULL}}, + {"foo", 3, {"foo", NULL}}, + {"\n\rfoo\n\rbar\r\n", 12, {"foo", "bar", NULL}}, + {"fo o\r\nb\tar", 10, {"fo o", "b.ar", NULL}}, + {"\x0f""f\0o\0\n\x01""b\0r\0\r", 12, {".f.o.", ".b.r.", NULL}}, + {NULL, 0, {}} + }; + + int i, j; + char *orig_line; + smartlist_t *sl; + + (void)ptr; + + for (i=0; tests[i].orig_line; i++) { + sl = smartlist_create(); + /* Allocate space for string and trailing NULL */ + orig_line = tor_memdup(tests[i].orig_line, tests[i].orig_length + 1); + tor_split_lines(sl, orig_line, tests[i].orig_length); + + j = 0; + log_info(LD_GENERAL, "Splitting test %d of length %d", + i, tests[i].orig_length); + SMARTLIST_FOREACH(sl, const char *, line, + { + /* Check we have not got too many lines */ + tt_int_op(j, <, MAX_SPLIT_LINE_COUNT); + /* Check that there actually should be a line here */ + tt_assert(tests[i].split_line[j] != NULL); + log_info(LD_GENERAL, "Line %d of test %d, should be <%s>", + j, i, tests[i].split_line[j]); + /* Check that the line is as expected */ + tt_str_op(tests[i].split_line[j], ==, line); + j++; + }); + /* Check that we didn't miss some lines */ + tt_assert(tests[i].split_line[j] == NULL); + tor_free(orig_line); + smartlist_free(sl); + } + + done: + ; +} static void test_util_di_ops(void)

1 0

[tor/master] Put some sense into our logging.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit 2e73f9b3eeb37d0307197e48ed62e1def40e44a8 Author: George Kadianakis <desnacked(a)gmail.com> Date: Mon Sep 12 00:10:07 2011 +0200 Put some sense into our logging. Transform our logging severities to something more sensible. Remove sneaky printf()s. --- src/common/util.c | 2 +- src/or/circuitbuild.c | 26 +++++++++++++------------- src/or/config.c | 12 ++++++------ src/or/transports.c | 46 ++++++++++++++++++++++++---------------------- 4 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 502840b..4a66e93 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3230,7 +3230,7 @@ get_string_from_pipe(FILE *stream, char *buf_out, size_t count) } else { /* No newline; check whether we overflowed the buffer */ if (!feof(stream)) - log_warn(LD_GENERAL, + log_info(LD_GENERAL, "Line from stream was truncated: %s", buf_out); /* TODO: What to do with this error? */ } diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index ed81304..86154ba 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4694,18 +4694,18 @@ transport_resolve_conflicts(transport_t *t) return 1; } else { /* same name but different addrport */ if (t_tmp->marked_for_removal) { /* marked for removal */ - log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' but " - "there was already a transport marked for deletion at " - "'%s:%u'. We deleted the old transport and registered the " - "new one.", t->name, fmt_addr(&t->addr), t->port, - fmt_addr(&t_tmp->addr), t_tmp->port); + log_notice(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' " + "but there was already a transport marked for deletion at " + "'%s:%u'. We deleted the old transport and registered the " + "new one.", t->name, fmt_addr(&t->addr), t->port, + fmt_addr(&t_tmp->addr), t_tmp->port); smartlist_remove(transport_list, t_tmp); transport_free(t_tmp); } else { /* *not* marked for removal */ - log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' " - "which already exists at '%s:%u'. Skipping.", t->name, - fmt_addr(&t->addr), t->port, - fmt_addr(&t_tmp->addr), t_tmp->port); + log_notice(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' " + "which already exists at '%s:%u'. Skipping.", t->name, + fmt_addr(&t->addr), t->port, + fmt_addr(&t_tmp->addr), t_tmp->port); return -1; } } @@ -4753,17 +4753,17 @@ transport_add_from_config(const tor_addr_t *addr, uint16_t port, switch (r) { case -1: default: - log_warn(LD_GENERAL, "Could not add transport %s at %s:%u. Skipping.", - t->name, fmt_addr(&t->addr), t->port); + log_notice(LD_GENERAL, "Could not add transport %s at %s:%u. Skipping.", + t->name, fmt_addr(&t->addr), t->port); transport_free(t); return -1; case 1: - log_warn(LD_GENERAL, "Succesfully registered transport %s at %s:%u.", + log_info(LD_GENERAL, "Succesfully registered transport %s at %s:%u.", t->name, fmt_addr(&t->addr), t->port); transport_free(t); /* falling */ return 0; case 0: - log_warn(LD_GENERAL, "Succesfully registered transport %s at %s:%u.", + log_info(LD_GENERAL, "Succesfully registered transport %s at %s:%u.", t->name, fmt_addr(&t->addr), t->port); return 0; } diff --git a/src/or/config.c b/src/or/config.c index 8bd47b5..3d83b99 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4772,8 +4772,8 @@ parse_client_transport_line(const char *line, int validate_only) if (!validate_only) { transport_add_from_config(&addr, port, name, socks_ver); - log_debug(LD_DIR, "Transport '%s' found at %s:%d", name, - fmt_addr(&addr), (int)port); + log_info(LD_DIR, "Transport '%s' found at %s:%d", name, + fmt_addr(&addr), (int)port); } } @@ -4871,7 +4871,7 @@ parse_server_transport_line(const char *line, int validate_only) } if (!validate_only) { - log_warn(LD_DIR, "Transport '%s' at %s:%d.", name, + log_info(LD_DIR, "Server transport '%s' at %s:%d.", name, fmt_addr(&addr), (int)port); } } @@ -5886,11 +5886,11 @@ save_transport_to_state(const char *transport, /* if transport in state has the same address as this one, life is good */ if (!strcmp(prev_bindaddr, transport_addrport)) { - log_warn(LD_CONFIG, "Transport seems to have spawned on its usual " + log_info(LD_CONFIG, "Transport seems to have spawned on its usual " "address:port."); goto done; } else { /* addrport in state is different than the one we got */ - log_warn(LD_CONFIG, "Transport seems to have spawned on different " + log_info(LD_CONFIG, "Transport seems to have spawned on different " "address:port. Let's update the state file with the new " "address:port"); tor_free(transport_line->value); /* free the old line */ @@ -5899,7 +5899,7 @@ save_transport_to_state(const char *transport, (int) port); /* replace old addrport line with new line */ } } else { /* never seen this one before; save it in state for next time */ - log_warn(LD_CONFIG, "It's the first time we see this transport. " + log_info(LD_CONFIG, "It's the first time we see this transport. " "Let's save its address:port"); next = &state->TransportProxies; /* find the last TransportProxy line in the state and point 'next' diff --git a/src/or/transports.c b/src/or/transports.c index a3abfed..8fafcf4 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -268,7 +268,8 @@ launch_managed_proxy(managed_proxy_t *mp) &stderr_pipe, (const char **)mp->argv, (const char **)envp); if (pid < 0) { - log_warn(LD_GENERAL, "Spawn failed"); + log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", + mp->argv[0]); return -1; } @@ -280,7 +281,7 @@ launch_managed_proxy(managed_proxy_t *mp) /* Open the buffered IO streams */ stdout_read = fdopen(stdout_pipe, "r"); - log_warn(LD_CONFIG, "The spawn is alive (%d)!", pid); + log_info(LD_CONFIG, "Managed proxy has spawned at PID %d.", pid); mp->conf_state = PT_PROTO_LAUNCHED; mp->stdout = stdout_read; @@ -295,8 +296,8 @@ launch_managed_proxy(managed_proxy_t *mp) void pt_configure_remaining_proxies(void) { - log_warn(LD_CONFIG, "We start configuring remaining managed proxies (%d)!", - unconfigured_proxies_n); + log_debug(LD_CONFIG, "Configuring remaining managed proxies (%d)!", + unconfigured_proxies_n); SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { tor_assert(mp->conf_state != PT_PROTO_BROKEN); @@ -306,10 +307,12 @@ pt_configure_remaining_proxies(void) /* This proxy is marked by a SIGHUP. Check whether we need to restart it. */ if (proxy_needs_restart(mp)) { + log_info(LD_GENERAL, "Preparing managed proxy for restart."); proxy_prepare_for_restart(mp); continue; } else { /* it doesn't need to be restarted. */ - printf("No need for restart; status quo\n"); + log_info(LD_GENERAL, "Nothing changed for managed proxy after HUP: " + "not restarting."); unconfigured_proxies_n--; tor_assert(unconfigured_proxies_n >= 0); } @@ -349,11 +352,11 @@ configure_proxy(managed_proxy_t *mp) } else if (r == IO_STREAM_EAGAIN) { /* check back later */ return; } else if (r == IO_STREAM_CLOSED || r == IO_STREAM_TERM) { /* snap! */ - log_warn(LD_GENERAL, "Managed proxy stream closed. " - "Most probably application stopped running"); + log_notice(LD_GENERAL, "Managed proxy stream closed. " + "Most probably application stopped running"); mp->conf_state = PT_PROTO_BROKEN; } else { /* unknown stream status */ - log_warn(LD_GENERAL, "Unknown stream status while configuring proxy."); + log_notice(LD_GENERAL, "Unknown stream status while configuring proxy."); } /* if the proxy finished configuring, exit the loop. */ @@ -398,15 +401,15 @@ register_client_proxy(managed_proxy_t *mp) r = transport_add(t); switch (r) { case -1: - log_warn(LD_GENERAL, "Could not add transport %s. Skipping.", t->name); + log_notice(LD_GENERAL, "Could not add transport %s. Skipping.", t->name); transport_free(t); break; case 0: - log_warn(LD_GENERAL, "Succesfully registered transport %s", t->name); + log_info(LD_GENERAL, "Succesfully registered transport %s", t->name); smartlist_add(sm_tmp, tor_strdup(t->name)); break; case 1: - log_warn(LD_GENERAL, "Succesfully registered transport %s", t->name); + log_info(LD_GENERAL, "Succesfully registered transport %s", t->name); smartlist_add(sm_tmp, tor_strdup(t->name)); transport_free(t); break; @@ -431,7 +434,6 @@ register_proxy(managed_proxy_t *mp) static void managed_proxy_destroy(managed_proxy_t *mp) { - printf("Destroying mp %p\n", mp); if (mp->conf_state != PT_PROTO_COMPLETED) SMARTLIST_FOREACH(mp->transports, transport_t *, t, transport_free(t)); else @@ -503,10 +505,10 @@ handle_methods_done(const managed_proxy_t *mp) tor_assert(mp->transports); if (smartlist_len(mp->transports) == 0) - log_warn(LD_GENERAL, "Proxy was spawned successfully, " - "but it didn't laucn any pluggable transport listeners!"); + log_notice(LD_GENERAL, "Proxy was spawned successfully, " + "but it didn't laucn any pluggable transport listeners!"); - log_warn(LD_CONFIG, "%s managed proxy configuration completed!", + log_info(LD_CONFIG, "%s managed proxy configuration completed!", mp->is_server ? "Server" : "Client"); } @@ -515,7 +517,7 @@ handle_methods_done(const managed_proxy_t *mp) void handle_proxy_line(const char *line, managed_proxy_t *mp) { - printf("Judging line: %s\n", line); + log_debug(LD_GENERAL, "Got a line from managed proxy: %s\n", line); if (strlen(line) < SMALLEST_MANAGED_LINE_SIZE) { log_warn(LD_GENERAL, "Managed proxy configuration line is too small. " @@ -609,8 +611,8 @@ parse_env_error(const char *line) /* (Length of the protocol string) plus (a space) and (the first char of the error message) */ if (strlen(line) < (strlen(PROTO_ENV_ERROR) + 2)) - log_warn(LD_CONFIG, "Managed proxy sent us an %s without an error " - "message.", PROTO_ENV_ERROR); + log_notice(LD_CONFIG, "Managed proxy sent us an %s without an error " + "message.", PROTO_ENV_ERROR); log_warn(LD_CONFIG, "Managed proxy couldn't understand the " "pluggable transport environment variables. (%s)", @@ -628,8 +630,8 @@ parse_version(const char *line, managed_proxy_t *mp) return -1; } - if (strcmp("1", line+strlen(PROTO_NEG_SUCCESS)+1)) { - log_warn(LD_CONFIG, "We don't support version '%s'. " + if (strcmp("1", line+strlen(PROTO_NEG_SUCCESS)+1)) { /* hardcoded temp */ + log_warn(LD_CONFIG, "Managed proxy tried to negotiate on version '%s'. " "We only support version '1'", line+strlen(PROTO_NEG_SUCCESS)+1); return -1; } @@ -713,7 +715,7 @@ parse_smethod_line(const char *line, managed_proxy_t *mp) /* For now, notify the user so that he knows where the server transport is listening. */ - log_warn(LD_CONFIG, "Server transport %s at %s:%d.", + log_info(LD_CONFIG, "Server transport %s at %s:%d.", method_name, fmt_addr(&addr), (int)port); r=0; @@ -796,7 +798,7 @@ parse_cmethod_line(const char *line, managed_proxy_t *mp) smartlist_add(mp->transports, transport); - log_warn(LD_CONFIG, "Transport %s at %s:%d with SOCKS %d. " + log_info(LD_CONFIG, "Transport %s at %s:%d with SOCKS %d. " "Attached to managed proxy.", method_name, fmt_addr(&addr), (int)port, socks_ver);

1 0

[tor/master] Make it compile on Windows™.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit 3be9d76fa2e56a9715e8151d9b6802da5b38512a Author: George Kadianakis <desnacked(a)gmail.com> Date: Fri Oct 7 15:44:44 2011 +0200 Make it compile on Windows™. --- src/common/util.c | 8 +++----- src/or/transports.c | 20 ++++++++++++++------ src/or/transports.h | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 4a66e93..a0777ea 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2966,20 +2966,18 @@ int tor_terminate_process(pid_t pid) { #ifdef MS_WINDOWS - DWORD pid_win = pid; - DWORD err; HANDLE handle; /* If the signal is outside of what GenerateConsoleCtrlEvent can use, attempt to open and terminate the process. */ handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid); - if (handle == NULL) + if (!handle) return -1; - if (TerminateProcess(handle, sig) == 0) + if (!TerminateProcess(handle, 0)) return -1; else return 0; -#else /* *nix */ +#else /* Unix */ return kill(pid, SIGTERM); #endif } diff --git a/src/or/transports.c b/src/or/transports.c index 465b8e1..c531fe7 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -234,7 +234,7 @@ proxy_prepare_for_restart(managed_proxy_t *mp) /* kill the old obfsproxy process */ tor_terminate_process(mp->pid); mp->pid = 0; - fclose(mp->stdout); + fclose(mp->_stdout); /* destroy all its old transports. we no longer use them. */ SMARTLIST_FOREACH_BEGIN(mp->transports, const char *, t_name) { @@ -277,14 +277,22 @@ launch_managed_proxy(managed_proxy_t *mp) free_execve_args(envp); /* Set stdout/stderr pipes to be non-blocking */ - fcntl(stdout_pipe, F_SETFL, O_NONBLOCK); +#ifdef _WIN32 + { + u_long nonblocking = 1; + ioctlsocket(stdout_pipe, FIONBIO, &nonblocking); + } +#else + fcntl(stdout_pipe, F_SETFL, O_NONBLOCK); +#endif + /* Open the buffered IO streams */ stdout_read = fdopen(stdout_pipe, "r"); log_info(LD_CONFIG, "Managed proxy has spawned at PID %d.", pid); mp->conf_state = PT_PROTO_LAUNCHED; - mp->stdout = stdout_read; + mp->_stdout = stdout_read; mp->pid = pid; return 0; @@ -344,7 +352,7 @@ configure_proxy(managed_proxy_t *mp) tor_assert(mp->conf_state != PT_PROTO_INFANT); while (1) { - r = get_string_from_pipe(mp->stdout, stdout_buf, + r = get_string_from_pipe(mp->_stdout, stdout_buf, sizeof(stdout_buf) - 1); if (r == IO_STREAM_OKAY) { /* got a line; handle it! */ @@ -456,8 +464,8 @@ managed_proxy_destroy(managed_proxy_t *mp) smartlist_remove(managed_proxy_list, mp); /* close its stdout stream */ - if (mp->stdout) - fclose(mp->stdout); + if (mp->_stdout) + fclose(mp->_stdout); /* free the argv */ free_execve_args(mp->argv); diff --git a/src/or/transports.h b/src/or/transports.h index 0b5cd5f..4a93387 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -47,7 +47,7 @@ typedef struct { int is_server; /* is it a server proxy? */ - FILE *stdout; /* a stream to its stdout + FILE *_stdout; /* a stream to its stdout (closed in managed_proxy_destroy()) */ int pid; /* The Process ID this managed proxy is using. */

1 0

[tor/master] Support multiple transports in a single transport line.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit 105cc42e96ea979b1111f3ba0d410c510cd229f0 Author: George Kadianakis <desnacked(a)gmail.com> Date: Fri Oct 7 14:13:41 2011 +0200 Support multiple transports in a single transport line. Support multiple comma-separated transpotrs in a single {Client,Server}TransportPlugin line. --- src/or/config.c | 78 ++++++++++++++++++++++++++++++++++++++------------ src/or/transports.c | 22 +++++++++----- src/or/transports.h | 10 +++--- 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 95017b3..536324d 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4693,7 +4693,8 @@ parse_client_transport_line(const char *line, int validate_only) int r; char *field2=NULL; - const char *name=NULL; + const char *transports=NULL; + smartlist_t *transport_list=NULL; char *addrport=NULL; tor_addr_t addr; uint16_t port = 0; @@ -4717,11 +4718,20 @@ parse_client_transport_line(const char *line, int validate_only) goto err; } - name = smartlist_get(items, 0); - if (!string_is_C_identifier(name)) { - log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", name); - goto err; - } + /* Get the first line element, split it to commas into + transport_list (in case it's multiple transports) and validate + the transport names. */ + transports = smartlist_get(items, 0); + transport_list = smartlist_create(); + smartlist_split_string(transport_list, transports, ",", + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport_name) { + if (!string_is_C_identifier(transport_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + transport_name); + goto err; + } + } SMARTLIST_FOREACH_END(transport_name); /* field2 is either a SOCKS version or "exec" */ field2 = smartlist_get(items, 1); @@ -4753,9 +4763,15 @@ parse_client_transport_line(const char *line, int validate_only) *tmp = NULL; /*terminated with NUL pointer, just like execve() likes it*/ /* kickstart the thing */ - pt_kickstart_client_proxy(name, proxy_argv); + pt_kickstart_client_proxy(transport_list, proxy_argv); } } else { /* external */ + if (smartlist_len(transport_list) != 1) { + log_warn(LD_CONFIG, "You can't have an external proxy with " + "more than one transports."); + goto err; + } + addrport = smartlist_get(items, 2); if (tor_addr_port_parse(addrport, &addr, &port)<0) { @@ -4770,10 +4786,11 @@ parse_client_transport_line(const char *line, int validate_only) } if (!validate_only) { - transport_add_from_config(&addr, port, name, socks_ver); + transport_add_from_config(&addr, port, smartlist_get(transport_list, 0), + socks_ver); - log_info(LD_DIR, "Transport '%s' found at %s:%d", name, - fmt_addr(&addr), (int)port); + log_info(LD_DIR, "Transport '%s' found at %s:%d", + transports, fmt_addr(&addr), (int)port); } } @@ -4786,6 +4803,9 @@ parse_client_transport_line(const char *line, int validate_only) done: SMARTLIST_FOREACH(items, char*, s, tor_free(s)); smartlist_free(items); + SMARTLIST_FOREACH(transport_list, char*, s, tor_free(s)); + smartlist_free(transport_list); + return r; } @@ -4799,7 +4819,8 @@ parse_server_transport_line(const char *line, int validate_only) { smartlist_t *items = NULL; int r; - const char *name=NULL; + const char *transports=NULL; + smartlist_t *transport_list=NULL; char *type=NULL; char *addrport=NULL; tor_addr_t addr; @@ -4823,11 +4844,20 @@ parse_server_transport_line(const char *line, int validate_only) goto err; } - name = smartlist_get(items, 0); - if (!string_is_C_identifier(name)) { - log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", name); - goto err; - } + /* Get the first line element, split it to commas into + transport_list (in case it's multiple transports) and validate + the transport names. */ + transports = smartlist_get(items, 0); + transport_list = smartlist_create(); + smartlist_split_string(transport_list, transports, ",", + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport_name) { + if (!string_is_C_identifier(transport_name)) { + log_warn(LD_CONFIG, "Transport name is not a C identifier (%s).", + transport_name); + goto err; + } + } SMARTLIST_FOREACH_END(transport_name); type = smartlist_get(items, 1); @@ -4854,9 +4884,15 @@ parse_server_transport_line(const char *line, int validate_only) *tmp = NULL; /*terminated with NUL pointer, just like execve() likes it*/ /* kickstart the thing */ - pt_kickstart_server_proxy(name, proxy_argv); + pt_kickstart_server_proxy(transport_list, proxy_argv); } } else { /* external */ + if (smartlist_len(transport_list) != 1) { + log_warn(LD_CONFIG, "You can't have an external proxy with " + "more than one transports."); + goto err; + } + addrport = smartlist_get(items, 2); if (tor_addr_port_parse(addrport, &addr, &port)<0) { @@ -4871,8 +4907,8 @@ parse_server_transport_line(const char *line, int validate_only) } if (!validate_only) { - log_info(LD_DIR, "Server transport '%s' at %s:%d.", name, - fmt_addr(&addr), (int)port); + log_info(LD_DIR, "Server transport '%s' at %s:%d.", + transports, fmt_addr(&addr), (int)port); } } @@ -4885,6 +4921,9 @@ parse_server_transport_line(const char *line, int validate_only) done: SMARTLIST_FOREACH(items, char*, s, tor_free(s)); smartlist_free(items); + SMARTLIST_FOREACH(transport_list, char*, s, tor_free(s)); + smartlist_free(transport_list); + return r; } @@ -5993,3 +6032,4 @@ getinfo_helper_config(control_connection_t *conn, } return 0; } + diff --git a/src/or/transports.c b/src/or/transports.c index c67fc68..465b8e1 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -890,7 +890,8 @@ set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) * proxy_argv. If is_server is true, it's a server * managed proxy. */ static managed_proxy_t * -managed_proxy_create(const char *transport, char **proxy_argv, int is_server) +managed_proxy_create(const smartlist_t *transport_list, + char **proxy_argv, int is_server) { managed_proxy_t *mp = tor_malloc_zero(sizeof(managed_proxy_t)); mp->conf_state = PT_PROTO_INFANT; @@ -899,7 +900,8 @@ managed_proxy_create(const char *transport, char **proxy_argv, int is_server) mp->transports = smartlist_create(); mp->transports_to_launch = smartlist_create(); - add_transport_to_proxy(transport, mp); + SMARTLIST_FOREACH(transport_list, const char *, transport, + add_transport_to_proxy(transport, mp)); /* register the managed proxy */ if (!managed_proxy_list) @@ -914,7 +916,8 @@ managed_proxy_create(const char *transport, char **proxy_argv, int is_server) * the managed proxy subsystem. * If is_server is true, then the proxy is a server proxy. */ void -pt_kickstart_proxy(const char *transport, char **proxy_argv, int is_server) +pt_kickstart_proxy(const smartlist_t *transport_list, + char **proxy_argv, int is_server) { managed_proxy_t *mp=NULL; transport_t *old_transport = NULL; @@ -922,7 +925,7 @@ pt_kickstart_proxy(const char *transport, char **proxy_argv, int is_server) mp = get_managed_proxy_by_argv_and_type(proxy_argv, is_server); if (!mp) { /* we haven't seen this proxy before */ - managed_proxy_create(transport, proxy_argv, is_server); + managed_proxy_create(transport_list, proxy_argv, is_server); } else { /* known proxy. add its transport to its transport list */ if (mp->got_hup) { @@ -938,12 +941,15 @@ pt_kickstart_proxy(const char *transport, char **proxy_argv, int is_server) unconfigured_proxies_n++; } - old_transport = transport_get_by_name(transport); - if (old_transport) - old_transport->marked_for_removal = 0; + SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport) { + old_transport = transport_get_by_name(transport); + if (old_transport) + old_transport->marked_for_removal = 0; + } SMARTLIST_FOREACH_END(transport); } - add_transport_to_proxy(transport, mp); + SMARTLIST_FOREACH(transport_list, const char *, transport, + add_transport_to_proxy(transport, mp)); free_execve_args(proxy_argv); } } diff --git a/src/or/transports.h b/src/or/transports.h index 57bfb5c..0b5cd5f 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -11,13 +11,13 @@ #ifndef TOR_TRANSPORTS_H #define TOR_TRANSPORTS_H -void pt_kickstart_proxy(const char *method, char **proxy_argv, +void pt_kickstart_proxy(const smartlist_t *transport_list, char **proxy_argv, int is_server); -#define pt_kickstart_client_proxy(m, pa) \ - pt_kickstart_proxy(m, pa, 0) -#define pt_kickstart_server_proxy(m, pa) \ - pt_kickstart_proxy(m, pa, 1) +#define pt_kickstart_client_proxy(tl, pa) \ + pt_kickstart_proxy(tl, pa, 0) +#define pt_kickstart_server_proxy(tl, pa) \ + pt_kickstart_proxy(tl, pa, 1) void pt_configure_remaining_proxies(void);

1 0

[tor/master] Make check-spaces happy.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit de7565f87fba14973038fbcd740ff9fecaa28b4e Author: George Kadianakis <desnacked(a)gmail.com> Date: Sun Sep 11 23:34:36 2011 +0200 Make check-spaces happy. --- src/common/util.c | 6 ++++-- src/or/circuitbuild.c | 10 +++++----- src/or/config.c | 9 +++++---- src/or/transports.c | 11 ++++++----- src/or/transports.h | 3 ++- 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 5fc2cbe..502840b 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3193,9 +3193,11 @@ tor_spawn_background(const char *const filename, int *stdout_read, * * Returns: * IO_STREAM_CLOSED: If the stream is closed. - * IO_STREAM_EAGAIN: If there is nothing to read and we should check back later. + * IO_STREAM_EAGAIN: If there is nothing to read and we should check back + * later. * IO_STREAM_TERM: If something is wrong with the stream. - * IO_STREAM_OKAY: If everything went okay and we got a string in buf_out. */ + * IO_STREAM_OKAY: If everything went okay and we got a string + * in buf_out. */ enum stream_status get_string_from_pipe(FILE *stream, char *buf_out, size_t count) { diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 7338e24..b4c2343 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4695,15 +4695,15 @@ transport_resolve_conflicts(transport_t *t) } else { /* same name but different addrport */ if (t_tmp->marked_for_removal) { /* marked for removal */ log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' but " - "there was already a transport marked for deletion at '%s:%u'." - "We deleted the old transport and registered the new one.", - t->name, fmt_addr(&t->addr), t->port, + "there was already a transport marked for deletion at " + "'%s:%u'. We deleted the old transport and registered the " + "new one.", t->name, fmt_addr(&t->addr), t->port, fmt_addr(&t_tmp->addr), t_tmp->port); smartlist_remove(transport_list, t_tmp); transport_free(t_tmp); } else { /* *not* marked for removal */ - log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' which " - "already exists at '%s:%u'. Skipping.", t->name, + log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' " + "which already exists at '%s:%u'. Skipping.", t->name, fmt_addr(&t->addr), t->port, fmt_addr(&t_tmp->addr), t_tmp->port); return -1; diff --git a/src/or/config.c b/src/or/config.c index 58668b1..df06739 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1245,7 +1245,6 @@ options_act(or_options_t *old_options) rep_hist_load_mtbf_data(time(NULL)); } - mark_transport_list(); pt_prepare_proxy_list_for_config_read(); if (options->ClientTransportPlugin) { @@ -5892,11 +5891,13 @@ save_transport_to_state(const char *transport, /* if transport in state has the same address as this one, life is good */ if (!strcmp(prev_bindaddr, transport_addrport)) { - log_warn(LD_CONFIG, "Transport seems to have spawned on its usual address:port."); + log_warn(LD_CONFIG, "Transport seems to have spawned on its usual " + "address:port."); goto done; } else { /* addrport in state is different than the one we got */ - log_warn(LD_CONFIG, "Transport seems to have spawned on different address:port." - "Let's update the state file with the new address:port"); + log_warn(LD_CONFIG, "Transport seems to have spawned on different " + "address:port. Let's update the state file with the new " + "address:port"); tor_free(transport_line->value); /* free the old line */ tor_asprintf(&transport_line->value, "%s %s:%d", transport, fmt_addr(addr), diff --git a/src/or/transports.c b/src/or/transports.c index 6d1ddeb..01cbfca 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -13,7 +13,8 @@ #include "transports.h" #include "util.h" -static void set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp); +static void set_managed_proxy_environment(char ***envp, + const managed_proxy_t *mp); static INLINE int proxy_configuration_finished(const managed_proxy_t *mp); static void managed_proxy_destroy(managed_proxy_t *mp); @@ -495,7 +496,6 @@ proxy_configuration_finished(const managed_proxy_t *mp) mp->conf_state == PT_PROTO_BROKEN); } - /** This function is called when a proxy sends an {S,C}METHODS DONE message. */ static void handle_methods_done(const managed_proxy_t *mp) @@ -863,10 +863,12 @@ set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) if (mp->is_server) { bindaddr = get_bindaddr_for_proxy(mp); - tor_asprintf(tmp++, "TOR_PT_ORPORT=127.0.0.1:%d", options->ORPort); /* XXX temp */ + /* XXX temp */ + tor_asprintf(tmp++, "TOR_PT_ORPORT=127.0.0.1:%d", options->ORPort); tor_asprintf(tmp++, "TOR_PT_SERVER_BINDADDR=%s", bindaddr); tor_asprintf(tmp++, "TOR_PT_SERVER_TRANSPORTS=%s", transports_to_launch); - tor_asprintf(tmp++, "TOR_PT_EXTENDED_SERVER_PORT=127.0.0.1:4200"); /* XXX temp*/ + /* XXX temp*/ + tor_asprintf(tmp++, "TOR_PT_EXTENDED_SERVER_PORT=127.0.0.1:4200"); } else { tor_asprintf(tmp++, "TOR_PT_CLIENT_TRANSPORTS=%s", transports_to_launch); } @@ -952,7 +954,6 @@ free_execve_args(char **arg) tor_free(arg); } - /** Tor will read its config, prepare the managed proxy list so that * proxies that are not used in the new config will shutdown, and * proxies that need to spawn more transports will do so. */ diff --git a/src/or/transports.h b/src/or/transports.h index 48b7839..57bfb5c 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -63,7 +63,8 @@ typedef struct { * torrc. */ unsigned int got_hup : 1; - smartlist_t *transports_to_launch; /* transports to-be-launched by this proxy */ + /* transports to-be-launched by this proxy */ + smartlist_t *transports_to_launch; /* The 'transports' list contains all the transports this proxy has launched.

1 0

[tor/master] Revive our beautiful unit tests.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit 1174bb95ce79767cfaee4f50ce70f42e7eb01b2e Author: George Kadianakis <desnacked(a)gmail.com> Date: Fri Oct 7 15:44:58 2011 +0200 Revive our beautiful unit tests. They broke when the PT_PROTO_INFANT proxy state was added. --- src/test/test_pt.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/test_pt.c b/src/test/test_pt.c index 99fc514..f97b21f 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -13,11 +13,9 @@ static void reset_mp(managed_proxy_t *mp) { - mp->conf_state = PT_PROTO_INFANT; + mp->conf_state = PT_PROTO_LAUNCHED; SMARTLIST_FOREACH(mp->transports, transport_t *, t, transport_free(t)); smartlist_clear(mp->transports); - smartlist_free(mp->transports); - mp->transports = smartlist_create(); } static void @@ -94,7 +92,7 @@ test_pt_protocol(void) char line[200]; managed_proxy_t *mp = tor_malloc(sizeof(managed_proxy_t)); - mp->conf_state = PT_PROTO_INFANT; + mp->conf_state = PT_PROTO_LAUNCHED; mp->transports = smartlist_create(); /* various wrong protocol runs: */

1 0

[tor/master] Update transports.[ch] to support SIGHUPs.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit 1e92b24889bd64ccdd568366aaf989714d130f31 Author: George Kadianakis <desnacked(a)gmail.com> Date: Sun Sep 11 20:29:12 2011 +0200 Update transports.[ch] to support SIGHUPs. --- src/or/transports.c | 323 +++++++++++++++++++++++++++++++++++++++------------ src/or/transports.h | 42 +++++++- 2 files changed, 292 insertions(+), 73 deletions(-) diff --git a/src/or/transports.c b/src/or/transports.c index 6255a56..c4391c5 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -17,10 +17,7 @@ static void set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp); static INLINE int proxy_configuration_finished(const managed_proxy_t *mp); -static void managed_proxy_destroy_impl(managed_proxy_t *mp, - int also_free_transports); -#define managed_proxy_destroy(mp) managed_proxy_destroy_impl(mp, 0) -#define managed_proxy_destroy_with_transports(mp) managed_proxy_destroy_impl(mp, 1) +static void managed_proxy_destroy(managed_proxy_t *mp); static void handle_finished_proxy(managed_proxy_t *mp); static void configure_proxy(managed_proxy_t *mp); @@ -55,14 +52,16 @@ static INLINE void free_execve_args(char **arg); #define PROTO_VERSION_ONE 1 /** List of unconfigured managed proxies. */ -static smartlist_t *unconfigured_proxy_list = NULL; +static smartlist_t *managed_proxy_list = NULL; +/** Number of still unconfigured proxies. */ +static int unconfigured_proxies_n = 0; /* The main idea here is: A managed proxy is represented by a managed_proxy_t struct and can spawn multiple transports. - unconfigured_proxy_list is a list of all the unconfigured managed + managed_proxy_list is a list of all the unconfigured managed proxies; everytime we find a managed proxy in torrc we add it in that list. In every run_scheduled_event() tick, we attempt to launch and then @@ -79,8 +78,7 @@ static smartlist_t *unconfigured_proxy_list = NULL; int pt_proxies_configuration_pending(void) { - if (!unconfigured_proxy_list) return 0; - return !!smartlist_len(unconfigured_proxy_list); + return !! unconfigured_proxies_n; } /** Return true if mp has the same argv as proxy_argv */ @@ -109,10 +107,10 @@ managed_proxy_has_argv(managed_proxy_t *mp, char **proxy_argv) static managed_proxy_t * get_managed_proxy_by_argv(char **proxy_argv) { - if (!unconfigured_proxy_list) + if (!managed_proxy_list) return NULL; - SMARTLIST_FOREACH_BEGIN(unconfigured_proxy_list, managed_proxy_t *, mp) { + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { if (managed_proxy_has_argv(mp, proxy_argv)) return mp; } SMARTLIST_FOREACH_END(mp); @@ -129,22 +127,88 @@ add_transport_to_proxy(const char *transport, managed_proxy_t *mp) smartlist_add(mp->transports_to_launch, tor_strdup(transport)); } +/** Called when a SIGHUP occurs. + * Returns true if managed proxy mp needs to be restarted + * after the SIGHUP based on the new torrc. */ +static int +proxy_needs_restart(managed_proxy_t *mp) +{ + /* mp->transport_to_launch is populated with the names of the + transports that must be launched *after* the SIGHUP. + + Since only PT_PROTO_COMPLETED proxies reach this function, + mp->transports is populated with strings of the *names of the + transports* that were launched *before* the SIGHUP. + + If the two lists contain the same strings, we don't need to + restart the proxy, since it already does what we want. */ + + tor_assert(smartlist_len(mp->transports_to_launch) > 0); + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + + if (smartlist_len(mp->transports_to_launch) != smartlist_len(mp->transports)) + goto needs_restart; + + SMARTLIST_FOREACH_BEGIN(mp->transports_to_launch, char *, t_t_l) { + if (!smartlist_string_isin(mp->transports, t_t_l)) + goto needs_restart; + + } SMARTLIST_FOREACH_END(t_t_l); + + return 0; + + needs_restart: + return 1; +} + +/** Managed proxy mp must be restarted. Do all the necessary + * preparations and then flag its state so that it will be launched + * in the next tick. */ +static void +proxy_prepare_for_restart(managed_proxy_t *mp) +{ + transport_t *t_tmp = NULL; + + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + tor_assert(mp->pid); + + /* kill the old obfsproxy process */ + tor_terminate_process(mp->pid); + mp->pid = 0; + fclose(mp->stdout); + + /* destroy all its old transports. we no longer use them. */ + SMARTLIST_FOREACH_BEGIN(mp->transports, const char *, t_name) { + t_tmp = transport_get_by_name(t_name); + if (t_tmp) + t_tmp->marked_for_removal = 1; + } SMARTLIST_FOREACH_END(t_name); + sweep_transport_list(); + + /* free the transport names in mp->transports */ + SMARTLIST_FOREACH(mp->transports, char *, t_name, tor_free(t_name)); + smartlist_clear(mp->transports); + + /* flag it as an infant proxy so that it gets launched on next tick */ + mp->conf_state = PT_PROTO_INFANT; +} + /** Launch managed proxy mp. */ static int launch_managed_proxy(managed_proxy_t *mp) { char **envp=NULL; - int retval; + int pid; FILE *stdout_read = NULL; int stdout_pipe=-1, stderr_pipe=-1; /* prepare the environment variables for the managed proxy */ set_managed_proxy_environment(&envp, mp); - retval = tor_spawn_background(mp->argv[0], &stdout_pipe, - &stderr_pipe, (const char **)mp->argv, - (const char **)envp); - if (retval < 0) { + pid = tor_spawn_background(mp->argv[0], &stdout_pipe, + &stderr_pipe, (const char **)mp->argv, + (const char **)envp); + if (pid < 0) { log_warn(LD_GENERAL, "Spawn failed"); return -1; } @@ -157,10 +221,11 @@ launch_managed_proxy(managed_proxy_t *mp) /* Open the buffered IO streams */ stdout_read = fdopen(stdout_pipe, "r"); - log_warn(LD_CONFIG, "The spawn is alive (%d)!", retval); + log_warn(LD_CONFIG, "The spawn is alive (%d)!", pid); mp->conf_state = PT_PROTO_LAUNCHED; mp->stdout = stdout_read; + mp->pid = pid; return 0; } @@ -171,12 +236,32 @@ launch_managed_proxy(managed_proxy_t *mp) void pt_configure_remaining_proxies(void) { - log_warn(LD_CONFIG, "We start configuring remaining managed proxies!"); - SMARTLIST_FOREACH_BEGIN(unconfigured_proxy_list, managed_proxy_t *, mp) { - /* configured proxies shouldn't be in unconfigured_proxy_list. */ - tor_assert(!proxy_configuration_finished(mp)); + log_warn(LD_CONFIG, "We start configuring remaining managed proxies (%d)!", + unconfigured_proxies_n); + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + tor_assert(mp->conf_state != PT_PROTO_BROKEN); + + if (mp->got_hup) { + mp->got_hup = 0; + + /* This proxy is marked by a SIGHUP. Check whether we need to + restart it. */ + if (proxy_needs_restart(mp)) { + proxy_prepare_for_restart(mp); + continue; + } else { /* it doesn't need to be restarted. */ + printf("No need for restart; status quo\n"); + unconfigured_proxies_n--; + tor_assert(unconfigured_proxies_n >= 0); + } + + continue; + } - configure_proxy(mp); + /* If the proxy is not fully configured, try to configure it + futher. */ + if (!proxy_configuration_finished(mp)) + configure_proxy(mp); } SMARTLIST_FOREACH_END(mp); } @@ -222,33 +307,55 @@ configure_proxy(managed_proxy_t *mp) /** Register server managed proxy mp transports to state */ static void -register_server_proxy(const managed_proxy_t *mp) +register_server_proxy(managed_proxy_t *mp) { - if (mp->is_server) { - SMARTLIST_FOREACH_BEGIN(mp->transports, transport_t *, t) { - save_transport_to_state(t->name,&t->addr,t->port); /* pass tor_addr_t? */ - } SMARTLIST_FOREACH_END(t); - } + smartlist_t *sm_tmp = smartlist_create(); + + tor_assert(mp->conf_state != PT_PROTO_COMPLETED); + SMARTLIST_FOREACH_BEGIN(mp->transports, transport_t *, t) { + save_transport_to_state(t->name,&t->addr,t->port); /* pass tor_addr_t? */ + smartlist_add(sm_tmp, tor_strdup(t->name)); + } SMARTLIST_FOREACH_END(t); + + smartlist_free(mp->transports); + mp->transports = sm_tmp; } /** Register all the transports supported by client managed proxy * mp to the bridge subsystem. */ static void -register_client_proxy(const managed_proxy_t *mp) +register_client_proxy(managed_proxy_t *mp) { + int r; + smartlist_t *sm_tmp = smartlist_create(); + + tor_assert(mp->conf_state != PT_PROTO_COMPLETED); SMARTLIST_FOREACH_BEGIN(mp->transports, transport_t *, t) { - if (transport_add(t)<0) { + r = transport_add(t); + switch (r) { + case -1: log_warn(LD_GENERAL, "Could not add transport %s. Skipping.", t->name); transport_free(t); - } else { + break; + case 0: log_warn(LD_GENERAL, "Succesfully registered transport %s", t->name); + smartlist_add(sm_tmp, tor_strdup(t->name)); + break; + case 1: + log_warn(LD_GENERAL, "Succesfully registered transport %s", t->name); + smartlist_add(sm_tmp, tor_strdup(t->name)); + transport_free(t); + break; } } SMARTLIST_FOREACH_END(t); + + smartlist_free(mp->transports); + mp->transports = sm_tmp; } /** Register the transports of managed proxy mp. */ static INLINE void -register_proxy(const managed_proxy_t *mp) +register_proxy(managed_proxy_t *mp) { if (mp->is_server) register_server_proxy(mp); @@ -256,18 +363,17 @@ register_proxy(const managed_proxy_t *mp) register_client_proxy(mp); } -/** Free memory allocated by managed proxy mp. - * If also_free_transports is set, also free the transports - * associated with this managed proxy. */ +/** Free memory allocated by managed proxy mp. */ static void -managed_proxy_destroy_impl(managed_proxy_t *mp, int also_free_transports) +managed_proxy_destroy(managed_proxy_t *mp) { - /* transport_free() all its transports */ - if (also_free_transports) + printf("Destroying mp %p\n", mp); + if (mp->conf_state != PT_PROTO_COMPLETED) SMARTLIST_FOREACH(mp->transports, transport_t *, t, transport_free(t)); + else + SMARTLIST_FOREACH(mp->transports, char *, t_name, tor_free(t_name)); /* free the transports smartlist */ - smartlist_clear(mp->transports); smartlist_free(mp->transports); SMARTLIST_FOREACH(mp->transports_to_launch, char *, t, tor_free(t)); @@ -277,31 +383,32 @@ managed_proxy_destroy_impl(managed_proxy_t *mp, int also_free_transports) smartlist_free(mp->transports_to_launch); /* remove it from the list of managed proxies */ - smartlist_remove(unconfigured_proxy_list, mp); + smartlist_remove(managed_proxy_list, mp); /* close its stdout stream */ - fclose(mp->stdout); + if (mp->stdout) + fclose(mp->stdout); /* free the argv */ free_execve_args(mp->argv); + if (mp->pid) + tor_terminate_process(mp->pid); + tor_free(mp); } - /** Handle a configured or broken managed proxy mp. */ static void handle_finished_proxy(managed_proxy_t *mp) { switch (mp->conf_state) { case PT_PROTO_BROKEN: /* if broken: */ - managed_proxy_destroy_with_transports(mp); /* destroy it and all its transports */ + managed_proxy_destroy(mp); /* annihilate it. */ break; case PT_PROTO_CONFIGURED: /* if configured correctly: */ register_proxy(mp); /* register transports */ - mp->conf_state = PT_PROTO_COMPLETED; /* mark it as completed, */ - managed_proxy_destroy(mp); /* destroy the managed proxy struct, - keeping the transports intact */ + mp->conf_state = PT_PROTO_COMPLETED; /* mark it as completed. */ break; default: log_warn(LD_CONFIG, "Unfinished managed proxy in " @@ -309,7 +416,8 @@ handle_finished_proxy(managed_proxy_t *mp) tor_assert(0); } - tor_assert(smartlist_len(unconfigured_proxy_list) >= 0); + unconfigured_proxies_n--; + tor_assert(unconfigured_proxies_n >= 0); } /** Return true if the configuration of the managed proxy mp is @@ -322,8 +430,7 @@ proxy_configuration_finished(const managed_proxy_t *mp) } -/** This function is called when a proxy sends an {S,C}METHODS DONE message, - */ +/** This function is called when a proxy sends an {S,C}METHODS DONE message. */ static void handle_methods_done(const managed_proxy_t *mp) { @@ -694,6 +801,30 @@ set_managed_proxy_environment(char ***envp, const managed_proxy_t *mp) tor_free(bindaddr); } +/** Create and return a new managed proxy for transport using + * proxy_argv. If is_server is true, it's a server + * managed proxy. */ +static managed_proxy_t * +managed_proxy_create(const char *transport, char **proxy_argv, int is_server) +{ + managed_proxy_t *mp = tor_malloc_zero(sizeof(managed_proxy_t)); + mp->conf_state = PT_PROTO_INFANT; + mp->is_server = is_server; + mp->argv = proxy_argv; + mp->transports = smartlist_create(); + + mp->transports_to_launch = smartlist_create(); + add_transport_to_proxy(transport, mp); + + /* register the managed proxy */ + if (!managed_proxy_list) + managed_proxy_list = smartlist_create(); + smartlist_add(managed_proxy_list, mp); + unconfigured_proxies_n++; + + return mp; +} + /** Register transport using proxy with proxy_argv to * the managed proxy subsystem. * If is_server is true, then the proxy is a server proxy. */ @@ -705,21 +836,28 @@ pt_kickstart_proxy(const char *transport, char **proxy_argv, int is_server) mp = get_managed_proxy_by_argv(proxy_argv); if (!mp) { /* we haven't seen this proxy before */ - /* create a managed proxy */ - managed_proxy_t *mp = tor_malloc_zero(sizeof(managed_proxy_t)); - mp->conf_state = PT_PROTO_INFANT; - mp->is_server = is_server; - mp->argv = proxy_argv; - mp->transports = smartlist_create(); - - mp->transports_to_launch = smartlist_create(); - add_transport_to_proxy(transport, mp); + managed_proxy_create(transport, proxy_argv, is_server); + + } else { /* known proxy. add its transport to its transport list */ + if (mp->got_hup) { + /* If the managed proxy we found is marked by a SIGHUP, it means + that it's not useless and should be kept. If it's marked for + removal, unmark it and increase the unconfigured proxies so + that we try to restart it if we need to. Afterwards, check if + a transport_t for 'transport' used to exist before the SIGHUP + and make sure it doesn't get deleted because we might reuse + it. */ + if (mp->marked_for_removal) { + mp->marked_for_removal = 0; + unconfigured_proxies_n++; + } + + transport_t *old_transport = NULL; + old_transport = transport_get_by_name(transport); + if (old_transport) + old_transport->marked_for_removal = 0; + } - /* register the managed proxy */ - if (!unconfigured_proxy_list) - unconfigured_proxy_list = smartlist_create(); - smartlist_add(unconfigured_proxy_list, mp); - } else { /* known proxy. just add transport to its transport list */ add_transport_to_proxy(transport, mp); free_execve_args(proxy_argv); } @@ -738,25 +876,66 @@ free_execve_args(char **arg) tor_free(arg); } + +/** Tor will read its config, prepare the managed proxy list so that + * proxies that are not used in the new config will shutdown, and + * proxies that need to spawn more transports will do so. */ +void +pt_prepare_proxy_list_for_config_read(void) +{ + if (!managed_proxy_list) + return; + + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + /* Destroy unconfigured proxies. */ + if (mp->conf_state != PT_PROTO_COMPLETED) { + managed_proxy_destroy(mp); + unconfigured_proxies_n--; + continue; + } + + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + + mp->marked_for_removal = 1; + mp->got_hup = 1; + SMARTLIST_FOREACH(mp->transports_to_launch, char *, t, tor_free(t)); + smartlist_clear(mp->transports_to_launch); + } SMARTLIST_FOREACH_END(mp); + + tor_assert(unconfigured_proxies_n == 0); +} + +/** The tor config was read, destroy all managed proxies that were + * marked by a previous call to prepare_proxy_list_for_config_read() + * and are not used by the new config. */ +void +sweep_proxy_list(void) +{ + if (!managed_proxy_list) + return; + + SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + if (mp->marked_for_removal) { + SMARTLIST_DEL_CURRENT(managed_proxy_list, mp); + managed_proxy_destroy(mp); + } + } SMARTLIST_FOREACH_END(mp); +} + /** Release all storage held by the pluggable transports subsystem. */ void pt_free_all(void) { - if (unconfigured_proxy_list) { + if (managed_proxy_list) { /* If the proxy is in PT_PROTO_COMPLETED, it has registered its transports and it's the duty of the circuitbuild.c subsystem to free them. Otherwise, it hasn't registered its transports yet and we should free them here. */ - SMARTLIST_FOREACH_BEGIN(unconfigured_proxy_list, managed_proxy_t *, mp) { - if (mp->conf_state == PT_PROTO_COMPLETED) - managed_proxy_destroy(mp); - else - managed_proxy_destroy_with_transports(mp); - } SMARTLIST_FOREACH_END(mp); + SMARTLIST_FOREACH(managed_proxy_list, managed_proxy_t *, mp, + managed_proxy_destroy(mp)); - smartlist_clear(unconfigured_proxy_list); - smartlist_free(unconfigured_proxy_list); - unconfigured_proxy_list=NULL; + smartlist_free(managed_proxy_list); + managed_proxy_list=NULL; } } diff --git a/src/or/transports.h b/src/or/transports.h index 6fec1dc..48b7839 100644 --- a/src/or/transports.h +++ b/src/or/transports.h @@ -25,6 +25,9 @@ int pt_proxies_configuration_pending(void); void pt_free_all(void); +void pt_prepare_proxy_list_for_config_read(void); +void sweep_proxy_list(void); + #ifdef PT_PRIVATE /** State of the managed proxy configuration protocol. */ enum pt_proto_state { @@ -47,8 +50,45 @@ typedef struct { FILE *stdout; /* a stream to its stdout (closed in managed_proxy_destroy()) */ + int pid; /* The Process ID this managed proxy is using. */ + + /** Boolean: We are re-parsing our config, and we are going to + * remove this managed proxy if we don't find it any transport + * plugins that use it. */ + unsigned int marked_for_removal : 1; + + /** Boolean: We got a SIGHUP while this proxy was running. We use + * this flag to signify that this proxy might need to be restarted + * so that it can listen for other transports according to the new + * torrc. */ + unsigned int got_hup : 1; + smartlist_t *transports_to_launch; /* transports to-be-launched by this proxy */ - smartlist_t *transports; /* list of transport_t this proxy spawned */ + + /* The 'transports' list contains all the transports this proxy has + launched. + + Before a managed_proxy_t reaches the PT_PROTO_COMPLETED phase, + this smartlist contains a 'transport_t' for every transport it + has launched. + + When the managed_proxy_t reaches the PT_PROTO_COMPLETED phase, it + registers all its transports to the circuitbuild.c subsystem. At + that point the 'transport_t's are owned by the circuitbuild.c + subsystem. + + To avoid carrying dangling 'transport_t's in this smartlist, + right before the managed_proxy_t reaches the PT_PROTO_COMPLETED + phase we replace all 'transport_t's with strings of their + transport names. + + So, tl;dr: + When (conf_state != PT_PROTO_COMPLETED) this list carries + (transport_t *). + When (conf_state == PT_PROTO_COMPLETED) this list carries + (char *). + */ + smartlist_t *transports; } managed_proxy_t; int parse_cmethod_line(const char *line, managed_proxy_t *mp);

1 0

[tor/master] Prepare circuitbuild.[ch] and config.[ch] for SIGHUPs.
by nickm＠torproject.org 07 Oct '11

07 Oct '11

commit fa514fb207f23cb6f0ade95bbd830834ea14811f Author: George Kadianakis <desnacked(a)gmail.com> Date: Sun Sep 11 20:28:47 2011 +0200 Prepare circuitbuild.[ch] and config.[ch] for SIGHUPs. * Create mark/sweep functions for transports. * Create a transport_resolve_conflicts() function that tries to resolve conflicts when registering transports. --- src/or/circuitbuild.c | 126 +++++++++++++++++++++++++++++++++++++++++++------ src/or/circuitbuild.h | 8 +++ src/or/config.c | 10 ++-- 3 files changed, 124 insertions(+), 20 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 382016e..bd06d31 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -124,7 +124,6 @@ static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); static void entry_guards_changed(void); -static const transport_t *transport_get_by_name(const char *name); static void bridge_free(bridge_info_t *bridge); /** @@ -4579,6 +4578,32 @@ bridge_free(bridge_info_t *bridge) /** A list of pluggable transports found in torrc. */ static smartlist_t *transport_list = NULL; +/** Mark every entry of the transport list to be removed on our next call to + * sweep_transport_list unless it has first been un-marked. */ +void +mark_transport_list(void) +{ + if (!transport_list) + transport_list = smartlist_create(); + SMARTLIST_FOREACH(transport_list, transport_t *, t, + t->marked_for_removal = 1); +} + +/** Remove every entry of the transport list that was marked with + * mark_transport_list if it has not subsequently been un-marked. */ +void +sweep_transport_list(void) +{ + if (!transport_list) + transport_list = smartlist_create(); + SMARTLIST_FOREACH_BEGIN(transport_list, transport_t *, t) { + if (t->marked_for_removal) { + SMARTLIST_DEL_CURRENT(transport_list, t); + transport_free(t); + } + } SMARTLIST_FOREACH_END(t); +} + /** Initialize the pluggable transports list to empty, creating it if * needed. */ void @@ -4603,7 +4628,7 @@ transport_free(transport_t *transport) /** Returns the transport in our transport list that has the name name. * Else returns NULL. */ -static const transport_t * +transport_t * transport_get_by_name(const char *name) { tor_assert(name); @@ -4611,7 +4636,7 @@ transport_get_by_name(const char *name) if (!transport_list) return NULL; - SMARTLIST_FOREACH_BEGIN(transport_list, const transport_t *, transport) { + SMARTLIST_FOREACH_BEGIN(transport_list, transport_t *, transport) { if (!strcmp(transport->name, name)) return transport; } SMARTLIST_FOREACH_END(transport); @@ -4636,23 +4661,81 @@ transport_create(const tor_addr_t *addr, uint16_t port, return t; } -/** Adds transport t to the internal list of pluggable transports. */ +/** Resolve any conflicts that the insertion of transport t + * might cause. + * Return 0 if t is OK and should be registered, 1 if there is + * a transport identical to t already registered and -1 if + * t cannot be added due to conflicts. */ +static int +transport_resolve_conflicts(transport_t *t) +{ + /* This is how we resolve transport conflicts: + + If there is already a transport with the same name and addrport, + we either have duplicate torrc lines OR we are here post-HUP and + this transport was here pre-HUP as well. In any case, mark the + old transport so that it doesn't get removed and ignore the new + one. + + If there is already a transport with the same name but different + addrport: + * if it's marked for removal, it means that it either has a lower + priority than 't' in torrc (otherwise the mark would have been + cleared by the paragraph above), or it doesn't exist at all in + the post-HUP torrc. We destroy the old transport and register 't'. + * if it's *not* marked for removal, it means that it was newly + added in the post-HUP torrc or that it's of higher priority, in + this case we ignore 't'. */ + transport_t *t_tmp = transport_get_by_name(t->name); + if (t_tmp) { /* same name */ + if (tor_addr_eq(&t->addr, &t_tmp->addr) && (t->port == t_tmp->port)) { + /* same name *and* addrport */ + t_tmp->marked_for_removal = 0; + return 1; + } else { /* same name but different addrport */ + if (t_tmp->marked_for_removal) { /* marked for removal */ + log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' but " + "there was already a transport marked for deletion at '%s:%u'." + "We deleted the old transport and registered the new one.", + t->name, fmt_addr(&t->addr), t->port, + fmt_addr(&t_tmp->addr), t_tmp->port); + smartlist_remove(transport_list, t_tmp); + transport_free(t_tmp); + } else { /* *not* marked for removal */ + log_warn(LD_GENERAL, "You tried to add transport '%s' at '%s:%u' which " + "already exists at '%s:%u'. Skipping.", t->name, + fmt_addr(&t->addr), t->port, + fmt_addr(&t_tmp->addr), t_tmp->port); + return -1; + } + } + } + + return 0; +} + +/** Add transport t to the internal list of pluggable + * transports. + * Returns 0 if the transport was added correctly, 1 if the same + * transport was already registered (in this case the caller must + * free the transport) and -1 if there was an error. */ int transport_add(transport_t *t) { tor_assert(t); - if (transport_get_by_name(t->name)) { /* check for duplicate names */ - log_notice(LD_CONFIG, "More than one transports have '%s' as " - "their name.", t->name); - return -1; - } + int r = transport_resolve_conflicts(t); - if (!transport_list) - transport_list = smartlist_create(); + switch (r) { + case 0: /* should register transport */ + if (!transport_list) + transport_list = smartlist_create(); - smartlist_add(transport_list, t); - return 0; + smartlist_add(transport_list, t); + return 0; + default: /* should let the caller know the return code */ + return r; + } } /** Remember a new pluggable transport proxy at addr:port. @@ -4664,10 +4747,23 @@ transport_add_from_config(const tor_addr_t *addr, uint16_t port, { transport_t *t = transport_create(addr, port, name, socks_ver); - if (transport_add(t) < 0) { + int r = transport_add(t); + + switch (r) { + case -1: + default: + log_warn(LD_GENERAL, "Could not add transport %s at %s:%u. Skipping.", + t->name, fmt_addr(&t->addr), t->port); transport_free(t); return -1; - } else { + case 1: + log_warn(LD_GENERAL, "Succesfully registered transport %s at %s:%u.", + t->name, fmt_addr(&t->addr), t->port); + transport_free(t); /* falling */ + return 0; + case 0: + log_warn(LD_GENERAL, "Succesfully registered transport %s at %s:%u.", + t->name, fmt_addr(&t->addr), t->port); return 0; } } diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 92449b4..10e7287 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -22,6 +22,9 @@ typedef struct { tor_addr_t addr; /** Port of proxy */ uint16_t port; + /** Boolean: We are re-parsing our transport list, and we are going to remove + * this one if we don't find it in the list of configured transports. */ + unsigned marked_for_removal : 1; } transport_t; char *circuit_list_path(origin_circuit_t *circ, int verbose); @@ -77,6 +80,9 @@ int getinfo_helper_entry_guards(control_connection_t *conn, void mark_bridge_list(void); void sweep_bridge_list(void); +void mark_transport_list(void); +void sweep_transport_list(void); + int routerinfo_is_a_configured_bridge(const routerinfo_t *ri); int node_is_a_configured_bridge(const node_t *node); void learned_router_identity(const tor_addr_t *addr, uint16_t port, @@ -149,6 +155,8 @@ transport_t *transport_create(const tor_addr_t *addr, uint16_t port, int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, const transport_t **transport); +transport_t *transport_get_by_name(const char *name); + void validate_pluggable_transports_config(void); #endif diff --git a/src/or/config.c b/src/or/config.c index 0b84f92..829d5ff 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1251,7 +1251,8 @@ options_act(or_options_t *old_options) } - clear_transport_list(); + mark_transport_list(); + pt_prepare_proxy_list_for_config_read(); if (options->ClientTransportPlugin) { for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { if (parse_client_transport_line(cl->value, 0)<0) { @@ -1273,6 +1274,8 @@ options_act(or_options_t *old_options) } } } + sweep_transport_list(); + sweep_proxy_list(); /* Bail out at this point if we're not going to be a client or server: * we want to not fork, and to log stuff to stderr. */ @@ -4769,10 +4772,7 @@ parse_client_transport_line(const char *line, int validate_only) } if (!validate_only) { - if (transport_add_from_config(&addr, port, name, - socks_ver) < 0) { - goto err; - } + transport_add_from_config(&addr, port, name, socks_ver); log_debug(LD_DIR, "Transport '%s' found at %s:%d", name, fmt_addr(&addr), (int)port);

1 0