Merge remote-tracking branch 'public/exit_carefully'

This commit is contained in:
Nick Mathewson 2017-10-27 11:13:05 -04:00
commit 30a681553f
10 changed files with 278 additions and 44 deletions

8
changes/bug23848 Normal file
View File

@ -0,0 +1,8 @@
o Minor features (embedding):
- On most errors that would cause Tor to exit, it now tries to return
from the tor_main() function, rather than calling the system exit()
function. Most users won't notice a difference here, but it should
make a significant difference on platforms that try to run Tor inside
a separate thread: they should now be able to survive Tor's exit
conditions rather than having Tor shut down the entire process.
Closes ticket 23848.

View File

@ -30,6 +30,7 @@ periodic_timer_t *periodic_timer_new(struct event_base *base,
void periodic_timer_free(periodic_timer_t *);
#define tor_event_base_loopexit event_base_loopexit
#define tor_event_base_loopbreak event_base_loopbreak
/** Defines a configuration for using libevent with Tor: passed as an argument
* to tor_libevent_initialize() to describe how we want to set up. */

View File

@ -835,9 +835,12 @@ set_options(or_options_t *new_val, char **msg)
return -1;
}
if (options_act(old_options) < 0) { /* acting on the options failed. die. */
log_err(LD_BUG,
"Acting on config options left us in a broken state. Dying.");
exit(1); // XXXX bad exit
if (! tor_event_loop_shutdown_is_pending()) {
log_err(LD_BUG,
"Acting on config options left us in a broken state. Dying.");
tor_shutdown_event_loop_and_exit(1);
}
return -1;
}
/* Issues a CONF_CHANGED event to notify controller of the change. If Tor is
* just starting up then the old_options will be undefined. */
@ -1689,8 +1692,11 @@ options_act(const or_options_t *old_options)
else
protocol_warning_severity_level = LOG_INFO;
if (consider_adding_dir_servers(options, old_options) < 0)
if (consider_adding_dir_servers(options, old_options) < 0) {
// XXXX This should get validated earlier, and committed here, to
// XXXX lower opportunities for reaching an error case.
return -1;
}
if (rend_non_anonymous_mode_enabled(options)) {
log_warn(LD_GENERAL, "This copy of Tor was compiled or configured to run "
@ -1699,6 +1705,7 @@ options_act(const or_options_t *old_options)
#ifdef ENABLE_TOR2WEB_MODE
/* LCOV_EXCL_START */
// XXXX This should move into options_validate()
if (!options->Tor2webMode) {
log_err(LD_CONFIG, "This copy of Tor was compiled to run in "
"'tor2web mode'. It can only be run with the Tor2webMode torrc "
@ -1707,6 +1714,7 @@ options_act(const or_options_t *old_options)
}
/* LCOV_EXCL_STOP */
#else /* !(defined(ENABLE_TOR2WEB_MODE)) */
// XXXX This should move into options_validate()
if (options->Tor2webMode) {
log_err(LD_CONFIG, "This copy of Tor was not compiled to run in "
"'tor2web mode'. It cannot be run with the Tor2webMode torrc "
@ -1734,9 +1742,11 @@ options_act(const or_options_t *old_options)
for (cl = options->Bridges; cl; cl = cl->next) {
bridge_line_t *bridge_line = parse_bridge_line(cl->value);
if (!bridge_line) {
// LCOV_EXCL_START
log_warn(LD_BUG,
"Previously validated Bridge line could not be added!");
return -1;
// LCOV_EXCL_STOP
}
bridge_add_from_config(bridge_line);
}
@ -1744,15 +1754,19 @@ options_act(const or_options_t *old_options)
}
if (running_tor && hs_config_service_all(options, 0)<0) {
// LCOV_EXCL_START
log_warn(LD_BUG,
"Previously validated hidden services line could not be added!");
return -1;
// LCOV_EXCL_STOP
}
if (running_tor && rend_parse_service_authorization(options, 0) < 0) {
// LCOV_EXCL_START
log_warn(LD_BUG, "Previously validated client authorization for "
"hidden services could not be added!");
return -1;
// LCOV_EXCL_STOP
}
/* Load state */
@ -1775,10 +1789,12 @@ options_act(const or_options_t *old_options)
if (options->ClientTransportPlugin) {
for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
if (parse_transport_line(options, cl->value, 0, 0) < 0) {
// LCOV_EXCL_START
log_warn(LD_BUG,
"Previously validated ClientTransportPlugin line "
"could not be added!");
return -1;
// LCOV_EXCL_STOP
}
}
}
@ -1786,10 +1802,12 @@ options_act(const or_options_t *old_options)
if (options->ServerTransportPlugin && server_mode(options)) {
for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
if (parse_transport_line(options, cl->value, 0, 1) < 0) {
// LCOV_EXCL_START
log_warn(LD_BUG,
"Previously validated ServerTransportPlugin line "
"could not be added!");
return -1;
// LCOV_EXCL_STOP
}
}
}
@ -1875,8 +1893,10 @@ options_act(const or_options_t *old_options)
/* Set up accounting */
if (accounting_parse_options(options, 0)<0) {
log_warn(LD_CONFIG,"Error in accounting options");
// LCOV_EXCL_START
log_warn(LD_BUG,"Error in previously validated accounting options");
return -1;
// LCOV_EXCL_STOP
}
if (accounting_is_enabled(options))
configure_accounting(time(NULL));
@ -1899,6 +1919,7 @@ options_act(const or_options_t *old_options)
char *http_authenticator;
http_authenticator = alloc_http_authenticator(options->BridgePassword);
if (!http_authenticator) {
// XXXX This should get validated in options_validate().
log_warn(LD_BUG, "Unable to allocate HTTP authenticator. Not setting "
"BridgePassword.");
return -1;
@ -1911,9 +1932,12 @@ options_act(const or_options_t *old_options)
}
if (parse_outbound_addresses(options, 0, &msg) < 0) {
log_warn(LD_BUG, "Failed parsing outbound bind addresses: %s", msg);
// LCOV_EXCL_START
log_warn(LD_BUG, "Failed parsing previously validated outbound "
"bind addresses: %s", msg);
tor_free(msg);
return -1;
// LCOV_EXCL_STOP
}
config_maybe_load_geoip_files_(options, old_options);
@ -5017,7 +5041,8 @@ load_torrc_from_disk(config_line_t *cmd_arg, int defaults_file)
/** Read a configuration file into <b>options</b>, finding the configuration
* file location based on the command line. After loading the file
* call options_init_from_string() to load the config.
* Return 0 if success, -1 if failure. */
* Return 0 if success, -1 if failure, and 1 if we succeeded but should exit
* anyway. */
int
options_init_from_torrc(int argc, char **argv)
{
@ -5044,22 +5069,22 @@ options_init_from_torrc(int argc, char **argv)
if (config_line_find(cmdline_only_options, "-h") ||
config_line_find(cmdline_only_options, "--help")) {
print_usage();
exit(0); // XXXX bad exit, though probably harmless
return 1;
}
if (config_line_find(cmdline_only_options, "--list-torrc-options")) {
/* For validating whether we've documented everything. */
list_torrc_options();
exit(0); // XXXX bad exit, though probably harmless
return 1;
}
if (config_line_find(cmdline_only_options, "--list-deprecated-options")) {
/* For validating whether what we have deprecated really exists. */
list_deprecated_options();
exit(0); // XXXX bad exit, though probably harmless
return 1;
}
if (config_line_find(cmdline_only_options, "--version")) {
printf("Tor version %s.\n",get_version());
exit(0); // XXXX bad exit, though probably harmless
return 1;
}
if (config_line_find(cmdline_only_options, "--library-versions")) {
@ -5087,7 +5112,7 @@ options_init_from_torrc(int argc, char **argv)
tor_compress_header_version_str(ZSTD_METHOD));
}
//TODO: Hex versions?
exit(0); // XXXX bad exit, though probably harmless
return 1;
}
command = CMD_RUN_TOR;
@ -5148,7 +5173,8 @@ options_init_from_torrc(int argc, char **argv)
get_options_mutable()->keygen_force_passphrase = FORCE_PASSPHRASE_OFF;
} else {
log_err(LD_CONFIG, "--no-passphrase specified without --keygen!");
exit(1); // XXXX bad exit
retval = -1;
goto err;
}
}
@ -5157,7 +5183,8 @@ options_init_from_torrc(int argc, char **argv)
get_options_mutable()->change_key_passphrase = 1;
} else {
log_err(LD_CONFIG, "--newpass specified without --keygen!");
exit(1); // XXXX bad exit
retval = -1;
goto err;
}
}
@ -5167,17 +5194,20 @@ options_init_from_torrc(int argc, char **argv)
if (fd_line) {
if (get_options()->keygen_force_passphrase == FORCE_PASSPHRASE_OFF) {
log_err(LD_CONFIG, "--no-passphrase specified with --passphrase-fd!");
exit(1); // XXXX bad exit
retval = -1;
goto err;
} else if (command != CMD_KEYGEN) {
log_err(LD_CONFIG, "--passphrase-fd specified without --keygen!");
exit(1); // XXXX bad exit
retval = -1;
goto err;
} else {
const char *v = fd_line->value;
int ok = 1;
long fd = tor_parse_long(v, 10, 0, INT_MAX, &ok, NULL);
if (fd < 0 || ok == 0) {
log_err(LD_CONFIG, "Invalid --passphrase-fd value %s", escaped(v));
exit(1); // XXXX bad exit
retval = -1;
goto err;
}
get_options_mutable()->keygen_passphrase_fd = (int)fd;
get_options_mutable()->use_keygen_passphrase_fd = 1;
@ -5192,7 +5222,8 @@ options_init_from_torrc(int argc, char **argv)
if (key_line) {
if (command != CMD_KEYGEN) {
log_err(LD_CONFIG, "--master-key without --keygen!");
exit(1); // XXXX bad exit
retval = -1;
goto err;
} else {
get_options_mutable()->master_key_fname = tor_strdup(key_line->value);
}

View File

@ -999,7 +999,7 @@ connection_ap_mark_as_pending_circuit_(entry_connection_t *entry_conn,
* So the fix is to tell it right now that it ought to finish its loop at
* its next available opportunity.
*/
tell_event_loop_to_finish();
tell_event_loop_to_run_external_code();
}
/** Mark <b>entry_conn</b> as no longer waiting for a circuit. */

View File

@ -6571,8 +6571,7 @@ monitor_owning_controller_process(const char *process_spec)
"owning controller: %s. Exiting.",
msg);
owning_controller_process_spec = NULL;
tor_cleanup();
exit(1); // XXXX bad exit: or questionable, at least.
tor_shutdown_event_loop_and_exit(1);
}
}

View File

@ -818,8 +818,8 @@ hibernate_begin(hibernate_state_t new_state, time_t now)
log_notice(LD_GENERAL,"SIGINT received %s; exiting now.",
hibernate_state == HIBERNATE_STATE_EXITING ?
"a second time" : "while hibernating");
tor_cleanup();
exit(0); // XXXX bad exit
tor_shutdown_event_loop_and_exit(0);
return;
}
if (new_state == HIBERNATE_STATE_LOWBANDWIDTH &&
@ -980,8 +980,7 @@ consider_hibernation(time_t now)
tor_assert(shutdown_time);
if (shutdown_time <= now) {
log_notice(LD_GENERAL, "Clean shutdown finished. Exiting.");
tor_cleanup();
exit(0); // XXXX bad exit
tor_shutdown_event_loop_and_exit(0);
}
return; /* if exiting soon, don't worry about bandwidth limits */
}

View File

@ -195,6 +195,14 @@ static smartlist_t *active_linked_connection_lst = NULL;
* <b>loop_once</b>. If so, there's no need to trigger a loopexit in order
* to handle linked connections. */
static int called_loop_once = 0;
/** Flag: if true, it's time to shut down, so the main loop should exit as
* soon as possible.
*/
static int main_loop_should_exit = 0;
/** The return value that the main loop should yield when it exits, if
* main_loop_should_exit is true.
*/
static int main_loop_exit_value = 0;
/** We set this to 1 when we've opened a circuit, so we can print a log
* entry to inform the user that Tor is working. We set it to 0 when
@ -642,7 +650,7 @@ connection_should_read_from_linked_conn(connection_t *conn)
* runs out of events, now we've changed our mind: tell it we want it to
* finish. */
void
tell_event_loop_to_finish(void)
tell_event_loop_to_run_external_code(void)
{
if (!called_loop_once) {
struct timeval tv = { 0, 0 };
@ -651,6 +659,53 @@ tell_event_loop_to_finish(void)
}
}
/** Failsafe measure that should never actually be necessary: If
* tor_shutdown_event_loop_and_exit() somehow doesn't successfully exit the
* event loop, then this callback will kill Tor with an assertion failure
* seconds later
*/
static void
shutdown_did_not_work_callback(evutil_socket_t fd, short event, void *arg)
{
// LCOV_EXCL_START
(void) fd;
(void) event;
(void) arg;
tor_assert_unreached();
// LCOV_EXCL_STOP
}
/**
* After finishing the current callback (if any), shut down the main loop,
* clean up the process, and exit with <b>exitcode</b>.
*/
void
tor_shutdown_event_loop_and_exit(int exitcode)
{
if (main_loop_should_exit)
return; /* Ignore multiple calls to this function. */
main_loop_should_exit = 1;
main_loop_exit_value = exitcode;
/* Die with an assertion failure in ten seconds, if for some reason we don't
* exit normally. */
struct timeval ten_seconds = { 10, 0 };
event_base_once(tor_libevent_get_base(), -1, EV_TIMEOUT,
shutdown_did_not_work_callback, NULL,
&ten_seconds);
/* Unlike loopexit, loopbreak prevents other callbacks from running. */
tor_event_base_loopbreak(tor_libevent_get_base());
}
/** Return true iff tor_shutdown_event_loop_and_exit() has been called. */
int
tor_event_loop_shutdown_is_pending(void)
{
return main_loop_should_exit;
}
/** Helper: Tell the main loop to begin reading bytes into <b>conn</b> from
* its linked connection, if it is not doing so already. Called by
* connection_start_reading and connection_start_writing as appropriate. */
@ -666,7 +721,7 @@ connection_start_reading_from_linked_conn(connection_t *conn)
/* make sure that the event_base_loop() function exits at
* the end of its run through the current connections, so we can
* activate read events for linked connections. */
tell_event_loop_to_finish();
tell_event_loop_to_run_external_code();
} else {
tor_assert(smartlist_contains(active_linked_connection_lst, conn));
}
@ -1557,8 +1612,7 @@ check_ed_keys_callback(time_t now, const or_options_t *options)
if (new_signing_key < 0 ||
generate_ed_link_cert(options, now, new_signing_key > 0)) {
log_err(LD_OR, "Unable to update Ed25519 keys! Exiting.");
tor_cleanup();
exit(1); // XXXX bad exit
tor_shutdown_event_loop_and_exit(1);
}
}
return 30;
@ -2369,10 +2423,18 @@ do_hup(void)
/* first, reload config variables, in case they've changed */
if (options->ReloadTorrcOnSIGHUP) {
/* no need to provide argc/v, they've been cached in init_from_config */
if (options_init_from_torrc(0, NULL) < 0) {
int init_rv = options_init_from_torrc(0, NULL);
if (init_rv < 0) {
log_err(LD_CONFIG,"Reading config failed--see warnings above. "
"For usage, try -h.");
return -1;
} else if (BUG(init_rv > 0)) {
// LCOV_EXCL_START
/* This should be impossible: the only "return 1" cases in
* options_init_from_torrc are ones caused by command-line arguments;
* but they can't change while Tor is running. */
return -1;
// LCOV_EXCL_STOP
}
options = get_options(); /* they have changed now */
/* Logs are only truncated the first time they are opened, but were
@ -2598,6 +2660,9 @@ do_main_loop(void)
}
#endif /* defined(HAVE_SYSTEMD) */
main_loop_should_exit = 0;
main_loop_exit_value = 0;
return run_main_loop_until_done();
}
@ -2613,6 +2678,9 @@ run_main_loop_once(void)
if (nt_service_is_stopping())
return 0;
if (main_loop_should_exit)
return 0;
#ifndef _WIN32
/* Make it easier to tell whether libevent failure is our fault or not. */
errno = 0;
@ -2659,6 +2727,9 @@ run_main_loop_once(void)
}
}
if (main_loop_should_exit)
return 0;
/* And here is where we put callbacks that happen "every time the event loop
* runs." They must be very fast, or else the whole Tor process will get
* slowed down.
@ -2687,7 +2758,11 @@ run_main_loop_until_done(void)
do {
loop_result = run_main_loop_once();
} while (loop_result == 1);
return loop_result;
if (main_loop_should_exit)
return main_loop_exit_value;
else
return loop_result;
}
/** Libevent callback: invoked when we get a signal.
@ -2711,14 +2786,13 @@ process_signal(int sig)
{
case SIGTERM:
log_notice(LD_GENERAL,"Catching signal TERM, exiting cleanly.");
tor_cleanup();
exit(0); // XXXX bad exit
tor_shutdown_event_loop_and_exit(0);
break;
case SIGINT:
if (!server_mode(get_options())) { /* do it now */
log_notice(LD_GENERAL,"Interrupt: exiting cleanly.");
tor_cleanup();
exit(0); // XXXX bad exit
tor_shutdown_event_loop_and_exit(0);
return;
}
#ifdef HAVE_SYSTEMD
sd_notify(0, "STOPPING=1");
@ -2747,8 +2821,8 @@ process_signal(int sig)
#endif
if (do_hup() < 0) {
log_warn(LD_CONFIG,"Restart failed (config error?). Exiting.");
tor_cleanup();
exit(1); // XXXX bad exit
tor_shutdown_event_loop_and_exit(1);
return;
}
#ifdef HAVE_SYSTEMD
sd_notify(0, "READY=1");
@ -3022,7 +3096,8 @@ activate_signal(int signal_num)
}
}
/** Main entry point for the Tor command-line client.
/** Main entry point for the Tor command-line client. Return 0 on "success",
* negative on "failure", and positive on "success and exit".
*/
int
tor_init(int argc, char *argv[])
@ -3128,9 +3203,14 @@ tor_init(int argc, char *argv[])
}
atexit(exit_function);
if (options_init_from_torrc(argc,argv) < 0) {
int init_rv = options_init_from_torrc(argc,argv);
if (init_rv < 0) {
log_err(LD_CONFIG,"Reading config failed--see warnings above.");
return -1;
} else if (init_rv > 0) {
// We succeeded, and should exit anyway -- probably the user just said
// "--version" or something like that.
return 1;
}
/* The options are now initialised */
@ -3200,7 +3280,7 @@ try_locking(const or_options_t *options, int err_if_locked)
r = try_locking(options, 0);
if (r<0) {
log_err(LD_GENERAL, "No, it's still there. Exiting.");
exit(1); // XXXX bad exit
return -1;
}
return r;
}
@ -3756,8 +3836,13 @@ tor_main(int argc, char *argv[])
if (done) return result;
}
#endif /* defined(NT_SERVICE) */
if (tor_init(argc, argv)<0)
return -1;
{
int init_rv = tor_init(argc, argv);
if (init_rv < 0)
return -1;
else if (init_rv > 0)
return 0;
}
if (get_options()->Sandbox && get_options()->command == CMD_RUN_TOR) {
sandbox_cfg_t* cfg = sandbox_init_filter();

View File

@ -45,7 +45,9 @@ int connection_is_writing(connection_t *conn);
MOCK_DECL(void,connection_stop_writing,(connection_t *conn));
MOCK_DECL(void,connection_start_writing,(connection_t *conn));
void tell_event_loop_to_finish(void);
void tell_event_loop_to_run_external_code(void);
void tor_shutdown_event_loop_and_exit(int exitcode);
int tor_event_loop_shutdown_is_pending(void);
void connection_stop_reading_from_linked_conn(connection_t *conn);

View File

@ -318,7 +318,7 @@ nt_service_main(void)
printf("Service error %d : %s\n", (int) result, errmsg);
tor_free(errmsg);
if (result == ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) {
if (tor_init(backup_argc, backup_argv) < 0)
if (tor_init(backup_argc, backup_argv))
return;
switch (get_options()->command) {
case CMD_RUN_TOR:

View File

@ -40,6 +40,12 @@ fi
CASE8=$dflt
CASE9=$dflt
CASE10=$dflt
CASE11A=$dflt
CASE11B=$dflt
CASE11C=$dflt
CASE11D=$dflt
CASE11E=$dflt
CASE11F=$dflt
if [ $# -ge 1 ]; then
eval "CASE${1}"=1
@ -363,6 +369,109 @@ echo "==== Case 10 ok"
fi
# Case 11a: -passphrase-fd without --keygen
if [ "$CASE11A" = 1 ]; then
ME="${DATA_DIR}/case11a"
mkdir -p "${ME}/keys"
${TOR} --DataDirectory "${ME}" --passphrase-fd 1 > "${ME}/stdout" && die "Successfully started with passphrase-fd but no keygen?" || true
grep "passphrase-fd specified without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments."
echo "==== Case 11A ok"
fi
# Case 11b: --no-passphrase without --keygen
if [ "$CASE11B" = 1 ]; then
ME="${DATA_DIR}/case11b"
mkdir -p "${ME}/keys"
${TOR} --DataDirectory "${ME}" --no-passphrase > "${ME}/stdout" && die "Successfully started with no-passphrase but no keygen?" || true
grep "no-passphrase specified without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments."
echo "==== Case 11B ok"
fi
# Case 11c: --newpass without --keygen
if [ "$CASE11C" = 1 ]; then
ME="${DATA_DIR}/case11C"
mkdir -p "${ME}/keys"
${TOR} --DataDirectory "${ME}" --newpass > "${ME}/stdout" && die "Successfully started with newpass but no keygen?" || true
grep "newpass specified without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments."
echo "==== Case 11C ok"
fi
######## --master-key does not work yet, but this will test the error case
######## when it does.
#
# Case 11d: --master-key without --keygen
#
if [ "$CASE11D" = 1 ]; then
#
# ME="${DATA_DIR}/case11d"
#
# mkdir -p "${ME}/keys"
#
# ${TOR} --DataDirectory "${ME}" --master-key "${ME}/foobar" > "${ME}/stdout" && die "Successfully started with master-key but no keygen?" || true
#
# cat "${ME}/stdout"
#
# grep "master-key without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments."
echo "==== Case 11D skipped"
fi
# Case 11E: Silly passphrase-fd
if [ "$CASE11E" = 1 ]; then
ME="${DATA_DIR}/case11E"
mkdir -p "${ME}/keys"
${TOR} --DataDirectory "${ME}" --keygen --passphrase-fd ewigeblumenkraft > "${ME}/stdout" && die "Successfully started with bogus passphrase-fd?" || true
grep "Invalid --passphrase-fd value" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments."
echo "==== Case 11E ok"
fi
# Case 11F: --no-passphrase with --passphrase-fd
if [ "$CASE11F" = 1 ]; then
ME="${DATA_DIR}/case11F"
mkdir -p "${ME}/keys"
${TOR} --DataDirectory "${ME}" --keygen --passphrase-fd 1 --no-passphrase > "${ME}/stdout" && die "Successfully started with bogus passphrase-fd combination?" || true
grep "no-passphrase specified with --passphrase-fd" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments."
echo "==== Case 11F ok"
fi
# Check cert-only.