aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/bug318104
-rw-r--r--changes/ticket310913
-rw-r--r--changes/ticket318415
-rw-r--r--scripts/maint/practracker/exceptions.txt2
-rw-r--r--src/feature/client/transports.c6
-rw-r--r--src/lib/net/resolve.c29
-rw-r--r--src/lib/net/resolve.h15
-rw-r--r--src/lib/process/process_unix.c9
-rw-r--r--src/lib/process/process_win32.c18
-rw-r--r--src/test/include.am2
-rw-r--r--src/test/resolve_test_helpers.c85
-rw-r--r--src/test/resolve_test_helpers.h18
-rw-r--r--src/test/test_addr.c6
-rw-r--r--src/test/test_config.c4
-rw-r--r--src/test/test_hs_config.c8
-rw-r--r--src/test/test_options.c3
-rw-r--r--src/test/test_process_slow.c30
17 files changed, 214 insertions, 33 deletions
diff --git a/changes/bug31810 b/changes/bug31810
new file mode 100644
index 0000000000..628d12f09b
--- /dev/null
+++ b/changes/bug31810
@@ -0,0 +1,4 @@
+ o Minor bugfixes (process management):
+ - Remove assertion in the Unix process backend. This assertion would trigger
+ when a new process is spawned where the executable is not found leading to
+ a stack trace from the child process. Fixes bug 31810; bugfix on 0.4.0.1-alpha.
diff --git a/changes/ticket31091 b/changes/ticket31091
new file mode 100644
index 0000000000..3cb9a2c37b
--- /dev/null
+++ b/changes/ticket31091
@@ -0,0 +1,3 @@
+ o Minor bugfixes (pluggable transports):
+ - Remove overly strict assertions that triggers when a pluggable transport
+ is spawned in an unsuccessful manner. Fixes bug 31091; bugfix on 0.4.0.1-alpha.
diff --git a/changes/ticket31841 b/changes/ticket31841
new file mode 100644
index 0000000000..6e7fbc1da1
--- /dev/null
+++ b/changes/ticket31841
@@ -0,0 +1,5 @@
+ o Minor features (testing):
+ - When running tests that attempt to look up hostname, replace the libc
+ name lookup functions with ones that do not actually touch the network.
+ This way, the tests complete more quickly in the presence of a slow or
+ missing DNS resolver. Closes ticket 31841.
diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index dcea3a3f47..7b15b37f8c 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -311,7 +311,7 @@ problem function-size /src/lib/net/inaddr.c:tor_inet_pton() 107
problem function-size /src/lib/net/socketpair.c:tor_ersatz_socketpair() 102
problem function-size /src/lib/osinfo/uname.c:get_uname() 116
problem function-size /src/lib/process/process_unix.c:process_unix_exec() 220
-problem function-size /src/lib/process/process_win32.c:process_win32_exec() 133
+problem function-size /src/lib/process/process_win32.c:process_win32_exec() 151
problem function-size /src/lib/process/process_win32.c:process_win32_create_pipe() 109
problem function-size /src/lib/process/restrict.c:set_max_file_descriptors() 102
problem function-size /src/lib/process/setuid.c:switch_id() 156
diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c
index 97bfc8ae30..3f731ac7d4 100644
--- a/src/feature/client/transports.c
+++ b/src/feature/client/transports.c
@@ -1826,15 +1826,13 @@ managed_proxy_stdout_callback(process_t *process,
managed_proxy_t *mp = process_get_data(process);
- if (BUG(mp == NULL))
+ if (mp == NULL)
return;
handle_proxy_line(line, mp);
- if (proxy_configuration_finished(mp)) {
+ if (proxy_configuration_finished(mp))
handle_finished_proxy(mp);
- tor_assert(mp->conf_state == PT_PROTO_COMPLETED);
- }
}
/** Callback function that is called when our PT process have data on its
diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c
index 78e72fba4f..442bc4a6b3 100644
--- a/src/lib/net/resolve.c
+++ b/src/lib/net/resolve.c
@@ -8,6 +8,7 @@
* \brief Use the libc DNS resolver to convert hostnames into addresses.
**/
+#define RESOLVE_PRIVATE
#include "lib/net/resolve.h"
#include "lib/net/address.h"
@@ -70,10 +71,10 @@ tor_lookup_hostname,(const char *name, uint32_t *addr))
*
* See tor_addr_lookup() for details.
*/
-static int
-tor_addr_lookup_host_getaddrinfo(const char *name,
- uint16_t family,
- tor_addr_t *addr)
+MOCK_IMPL(STATIC int,
+tor_addr_lookup_host_impl,(const char *name,
+ uint16_t family,
+ tor_addr_t *addr))
{
int err;
struct addrinfo *res=NULL, *res_p;
@@ -120,15 +121,17 @@ tor_addr_lookup_host_getaddrinfo(const char *name,
#else /* !defined(HAVE_GETADDRINFO) */
-/* Host lookup helper for tor_addr_lookup(), which calls getaddrinfo().
- * Used when gethostbyname() is not available on this system.
+/* Host lookup helper for tor_addr_lookup(), which calls gethostbyname().
+ * Used when getaddrinfo() is not available on this system.
*
* See tor_addr_lookup() for details.
*/
-static int
-tor_addr_lookup_host_gethostbyname(const char *name,
- tor_addr_t *addr)
+MOCK_IMPL(STATIC int,
+tor_addr_lookup_host_impl,(const char *name,
+ uint16_t family,
+ tor_addr_t *addr))
{
+ (void) family;
struct hostent *ent;
int err;
#ifdef HAVE_GETHOSTBYNAME_R_6_ARG
@@ -170,7 +173,6 @@ tor_addr_lookup_host_gethostbyname(const char *name,
return (err == TRY_AGAIN) ? 1 : -1;
#endif
}
-
#endif /* defined(HAVE_GETADDRINFO) */
/** Similar behavior to Unix gethostbyname: resolve <b>name</b>, and set
@@ -215,13 +217,8 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr))
} else {
/* Clear the address after a failed tor_addr_parse(). */
memset(addr, 0, sizeof(tor_addr_t));
-#ifdef HAVE_GETADDRINFO
- result = tor_addr_lookup_host_getaddrinfo(name, family, addr);
+ result = tor_addr_lookup_host_impl(name, family, addr);
goto done;
-#else /* !(defined(HAVE_GETADDRINFO)) */
- result = tor_addr_lookup_host_gethostbyname(name, addr);
- goto done;
-#endif /* defined(HAVE_GETADDRINFO) */
}
/* If we weren't successful, and haven't already set the result,
diff --git a/src/lib/net/resolve.h b/src/lib/net/resolve.h
index d7b60be917..b979b2fb41 100644
--- a/src/lib/net/resolve.h
+++ b/src/lib/net/resolve.h
@@ -24,12 +24,18 @@
struct tor_addr_t;
+/*
+ * Primary lookup functions.
+ */
MOCK_DECL(int, tor_lookup_hostname,(const char *name, uint32_t *addr));
MOCK_DECL(int, tor_addr_lookup,(const char *name, uint16_t family,
struct tor_addr_t *addr_out));
int tor_addr_port_lookup(const char *s, struct tor_addr_t *addr_out,
uint16_t *port_out);
+/*
+ * Sandbox helpers
+ */
struct addrinfo;
#ifdef USE_SANDBOX_GETADDRINFO
/** Pre-calls getaddrinfo in order to pre-record result. */
@@ -55,4 +61,13 @@ void tor_free_getaddrinfo_cache(void);
void sandbox_disable_getaddrinfo_cache(void);
void tor_make_getaddrinfo_cache_active(void);
+/*
+ * Internal resolver wrapper; exposed for mocking.
+ */
+#ifdef RESOLVE_PRIVATE
+MOCK_DECL(STATIC int, tor_addr_lookup_host_impl, (const char *name,
+ uint16_t family,
+ struct tor_addr_t *addr));
+#endif
+
#endif /* !defined(TOR_RESOLVE_H) */
diff --git a/src/lib/process/process_unix.c b/src/lib/process/process_unix.c
index 332b432c54..8191bdc1f0 100644
--- a/src/lib/process/process_unix.c
+++ b/src/lib/process/process_unix.c
@@ -253,22 +253,15 @@ process_unix_exec(process_t *process)
process_environment_t *env = process_get_environment(process);
/* Call the requested program. */
- retval = execve(argv[0], argv, env->unixoid_environment_block);
+ execve(argv[0], argv, env->unixoid_environment_block);
/* If we made it here it is because execve failed :-( */
- if (-1 == retval)
- fprintf(stderr, "Call to execve() failed: %s", strerror(errno));
-
tor_free(argv);
process_environment_free(env);
- tor_assert_unreached();
-
error:
- /* LCOV_EXCL_START */
fprintf(stderr, "Error from child process: %s", strerror(errno));
_exit(1);
- /* LCOV_EXCL_STOP */
}
/* We are in the parent process. */
diff --git a/src/lib/process/process_win32.c b/src/lib/process/process_win32.c
index 624333d4a3..7e4082ad13 100644
--- a/src/lib/process/process_win32.c
+++ b/src/lib/process/process_win32.c
@@ -234,6 +234,24 @@ process_win32_exec(process_t *process)
CloseHandle(stdin_pipe_read);
CloseHandle(stdin_pipe_write);
+ /* In the Unix backend, we do not get an error in the Tor process when a
+ * child process fails to spawn its target executable since we need to
+ * first do the fork() call in the Tor process and then the child process
+ * is responsible for doing the call to execve().
+ *
+ * This means that the user of the process_exec() API must check for
+ * whether it returns PROCESS_STATUS_ERROR, which will rarely happen on
+ * Unix, but will happen for error cases on Windows where it does not
+ * happen on Unix. For example: when the target executable does not exist
+ * on the file system.
+ *
+ * To have somewhat feature compatibility between the Unix and the Windows
+ * backend, we here notify the process_t owner that the process have exited
+ * (even though it never managed to run) to ensure that the exit callback
+ * is executed.
+ */
+ process_notify_event_exit(process, 0);
+
return PROCESS_STATUS_ERROR;
}
diff --git a/src/test/include.am b/src/test/include.am
index b3222550ca..d8e25dea9f 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -102,6 +102,7 @@ src_test_test_SOURCES += \
src/test/log_test_helpers.c \
src/test/hs_test_helpers.c \
src/test/rend_test_helpers.c \
+ src/test/resolve_test_helpers.c \
src/test/rng_test_helpers.c \
src/test/test.c \
src/test/test_accounting.c \
@@ -341,6 +342,7 @@ noinst_HEADERS+= \
src/test/hs_test_helpers.h \
src/test/log_test_helpers.h \
src/test/rend_test_helpers.h \
+ src/test/resolve_test_helpers.h \
src/test/rng_test_helpers.h \
src/test/test.h \
src/test/ptr_helpers.h \
diff --git a/src/test/resolve_test_helpers.c b/src/test/resolve_test_helpers.c
new file mode 100644
index 0000000000..73ea730149
--- /dev/null
+++ b/src/test/resolve_test_helpers.c
@@ -0,0 +1,85 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file resolve_test_helpers.c
+ * @brief Helper functions for mocking libc's blocking hostname lookup
+ * facilities.
+ **/
+
+#define RESOLVE_PRIVATE
+#include "orconfig.h"
+#include "test/resolve_test_helpers.h"
+#include "lib/net/address.h"
+#include "lib/net/resolve.h"
+#include "test/test.h"
+
+#include <stdio.h>
+#include <string.h>
+
+/**
+ * Mock replacement for our getaddrinfo/gethostbyname wrapper.
+ **/
+static int
+replacement_host_lookup(const char *name, uint16_t family, tor_addr_t *addr)
+{
+ static const struct lookup_table_ent {
+ const char *name;
+ const char *ipv4;
+ const char *ipv6;
+ } entries[] = {
+ { "localhost", "127.0.0.1", "::1" },
+ { "torproject.org", "198.51.100.6", "2001:DB8::700" },
+ { NULL, NULL, NULL },
+ };
+
+ int r = -1;
+
+ for (unsigned i = 0; entries[i].name != NULL; ++i) {
+ if (!strcasecmp(name, entries[i].name)) {
+ if (family == AF_INET6) {
+ int s = tor_addr_parse(addr, entries[i].ipv6);
+ tt_int_op(s, OP_EQ, AF_INET6);
+ } else {
+ int s = tor_addr_parse(addr, entries[i].ipv4);
+ tt_int_op(s, OP_EQ, AF_INET);
+ }
+ r = 0;
+ break;
+ }
+ }
+
+ log_debug(LD_GENERAL, "resolve(%s,%d) => %s",
+ name, family, r == 0 ? fmt_addr(addr) : "-1");
+
+ return r;
+ done:
+ return -1;
+}
+
+/**
+ * Set up a mock replacement for our wrapper on libc's resolver code.
+ *
+ * According to our replacement, only "localhost" and "torproject.org"
+ * are real addresses; everything else doesn't exist.
+ *
+ * Use this function to avoid using the DNS resolver during unit tests;
+ * call unmock_hostname_resolver() when you're done.
+ **/
+void
+mock_hostname_resolver(void)
+{
+ MOCK(tor_addr_lookup_host_impl, replacement_host_lookup);
+}
+
+/**
+ * Unmock our wrappers for libc's blocking hostname resolver code.
+ **/
+void
+unmock_hostname_resolver(void)
+{
+ UNMOCK(tor_addr_lookup_host_impl);
+}
diff --git a/src/test/resolve_test_helpers.h b/src/test/resolve_test_helpers.h
new file mode 100644
index 0000000000..e7d2e29373
--- /dev/null
+++ b/src/test/resolve_test_helpers.h
@@ -0,0 +1,18 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file resolve_test_helpers.h
+ * @brief Header for test/resolve_test_helpers.c
+ **/
+
+#ifndef TOR_TEST_RESOLVE_TEST_HELPERS_H
+#define TOR_TEST_RESOLVE_TEST_HELPERS_H
+
+void mock_hostname_resolver(void);
+void unmock_hostname_resolver(void);
+
+#endif /* !defined(TOR_TEST_RESOLVE_TEST_HELPERS_H) */
diff --git a/src/test/test_addr.c b/src/test/test_addr.c
index f99e3be8f5..c89c6e78d4 100644
--- a/src/test/test_addr.c
+++ b/src/test/test_addr.c
@@ -12,6 +12,7 @@
#include "test/log_test_helpers.h"
#include "lib/net/resolve.h"
#include "test/rng_test_helpers.h"
+#include "test/resolve_test_helpers.h"
#ifdef HAVE_SYS_UN_H
#include <sys/un.h>
@@ -1160,6 +1161,7 @@ test_addr_parse_canonical(void *arg)
static void
test_addr_parse(void *arg)
{
+
int r;
tor_addr_t addr;
uint16_t port;
@@ -1169,6 +1171,8 @@ test_addr_parse(void *arg)
(void)arg;
+ mock_hostname_resolver();
+
/* IPv6-mapped IPv4 addresses. Tor doesn't really use these. */
TEST_ADDR_V6_PARSE("11:22:33:44:55:66:1.2.3.4", 0,
"11:22:33:44:55:66:102:304");
@@ -1273,7 +1277,7 @@ test_addr_parse(void *arg)
"11:22::88",99);
done:
- ;
+ unmock_hostname_resolver();
}
static void
diff --git a/src/test/test_config.c b/src/test/test_config.c
index 1c6c913078..cbb84e4dcf 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -45,6 +45,7 @@
#include "app/config/statefile.h"
#include "test/test_helpers.h"
+#include "test/resolve_test_helpers.h"
#include "feature/dirclient/dir_server_st.h"
#include "core/or/port_cfg_st.h"
@@ -4068,6 +4069,8 @@ test_config_parse_port_config__ports__ports_given(void *data)
slout = smartlist_new();
+ mock_hostname_resolver();
+
// Test error when encounters an invalid Port specification
config_port_invalid = mock_config_line("DNSPort", "");
ret = parse_port_config(NULL, config_port_invalid, "DNS", 0, NULL,
@@ -4764,6 +4767,7 @@ test_config_parse_port_config__ports__ports_given(void *data)
#endif /* defined(_WIN32) */
done:
+ unmock_hostname_resolver();
if (slout)
SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
smartlist_free(slout);
diff --git a/src/test/test_hs_config.c b/src/test/test_hs_config.c
index 2b3afbb6e9..71e1529216 100644
--- a/src/test/test_hs_config.c
+++ b/src/test/test_hs_config.c
@@ -12,6 +12,7 @@
#include "test/test.h"
#include "test/test_helpers.h"
#include "test/log_test_helpers.h"
+#include "test/resolve_test_helpers.h"
#include "app/config/config.h"
#include "feature/hs/hs_common.h"
@@ -272,6 +273,7 @@ test_valid_service_v2(void *arg)
int ret;
(void) arg;
+ mock_hostname_resolver();
/* Valid complex configuration. Basic client authorization. */
{
@@ -314,7 +316,7 @@ test_valid_service_v2(void *arg)
}
done:
- ;
+ unmock_hostname_resolver();
}
static void
@@ -392,6 +394,7 @@ test_valid_service_v3(void *arg)
int ret;
(void) arg;
+ mock_hostname_resolver();
/* Valid complex configuration. */
{
@@ -448,7 +451,7 @@ test_valid_service_v3(void *arg)
}
done:
- ;
+ unmock_hostname_resolver();
}
static void
@@ -623,4 +626,3 @@ struct testcase_t hs_config_tests[] = {
END_OF_TESTCASES
};
-
diff --git a/src/test/test_options.c b/src/test/test_options.c
index d0e77c031d..69407a999b 100644
--- a/src/test/test_options.c
+++ b/src/test/test_options.c
@@ -14,6 +14,7 @@
#include "feature/nodelist/routerset.h"
#include "core/mainloop/mainloop.h"
#include "test/log_test_helpers.h"
+#include "test/resolve_test_helpers.h"
#include "lib/sandbox/sandbox.h"
#include "lib/memarea/memarea.h"
@@ -241,6 +242,7 @@ test_options_validate(void *arg)
(void)arg;
setup_log_callback();
sandbox_disable_getaddrinfo_cache();
+ mock_hostname_resolver();
WANT_ERR("ExtORPort 500000", "Invalid ExtORPort", PH_VALIDATE);
@@ -282,6 +284,7 @@ test_options_validate(void *arg)
close_temp_logs();
clear_log_messages();
+ unmock_hostname_resolver();
return;
}
diff --git a/src/test/test_process_slow.c b/src/test/test_process_slow.c
index 91252c725d..f311e8b293 100644
--- a/src/test/test_process_slow.c
+++ b/src/test/test_process_slow.c
@@ -328,8 +328,38 @@ test_callbacks_terminate(void *arg)
process_free(process);
}
+static void
+test_nonexistent_executable(void *arg)
+{
+ (void)arg;
+
+ /* Process callback data. */
+ process_data_t *process_data = process_data_new();
+
+ /* Setup our process. */
+ process_t *process = process_new("binary-does-not-exist");
+ process_set_data(process, process_data);
+ process_set_exit_callback(process, process_exit_callback);
+
+ /* Run our process. */
+ process_exec(process);
+
+ /* Start our main loop. */
+ run_main_loop(process_data);
+
+ /* Ensure that the exit callback was actually called even though the binary
+ * did not exist.
+ */
+ tt_assert(process_data->did_exit);
+
+ done:
+ process_data_free(process_data);
+ process_free(process);
+}
+
struct testcase_t slow_process_tests[] = {
{ "callbacks", test_callbacks, 0, NULL, NULL },
{ "callbacks_terminate", test_callbacks_terminate, 0, NULL, NULL },
+ { "nonexistent_executable", test_nonexistent_executable, 0, NULL, NULL },
END_OF_TESTCASES
};
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion