commit 19e25d5cabd23f28044ccbddc01e5cacbde2cfcb Author: Nick Mathewson nickm@torproject.org Date: Fri Feb 3 10:14:25 2017 -0500

Prevention: never die from extend_info_from_node() failure.

Bug 21242 occurred because we asserted that extend_info_from_node() had succeeded...even though we already had the code to handle such a failure. We fixed that in 93b39c51629ed0ded2bf807cb6.

But there were four other cases in our code where we called extend_info_from_node() and either tor_assert()ed that it returned non-NULL, or [in one case] silently assumed that it returned non-NULL. That's not such a great idea. This patch makes those cases check for a bug of this kind instead.

Fixes bug 21372; bugfix on 0.2.3.1-alpha when extend_info_from_node() was introduced. --- changes/bug21372 | 4 ++++ src/or/circuitbuild.c | 5 +++-- src/or/control.c | 7 ++----- src/or/rendservice.c | 3 +++ 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/changes/bug21372 b/changes/bug21372 new file mode 100644 index 0000000..178ae3a --- /dev/null +++ b/changes/bug21372 @@ -0,0 +1,4 @@ + o Minor bugfixes (client): + - Always recover from failures in extend_info_from_node(), + in an attempt to prevent any recurrence of bug 21242. + Fixes bug 21372; bugfix on 0.2.3.1-alpha. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 88445f9..8a57d83 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2110,7 +2110,8 @@ onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit_ei) return -1; } exit_ei = extend_info_from_node(node, 0); - tor_assert(exit_ei); + if (BUG(exit_ei == NULL)) + return -1; } state->chosen_exit = exit_ei; return 0; @@ -2376,7 +2377,7 @@ onion_extend_cpath(origin_circuit_t *circ) choose_good_middle_server(purpose, state, circ->cpath, cur_len); if (r) { info = extend_info_from_node(r, 0); - tor_assert(info); + tor_assert_nonfatal(info); } }

diff --git a/src/or/control.c b/src/or/control.c index 2c71ea5..b0a6876 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -3377,7 +3377,8 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, SMARTLIST_FOREACH(nodes, const node_t *, node, { extend_info_t *info = extend_info_from_node(node, first_node); - if (first_node && !info) { + if (!info) { + tor_assert_nonfatal(first_node); log_warn(LD_CONTROL, "controller tried to connect to a node that doesn't have any " "addresses that are allowed by the firewall configuration; " @@ -3385,10 +3386,6 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, circuit_mark_for_close(TO_CIRCUIT(circ), -END_CIRC_REASON_CONNECTFAILED); connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn); goto done; - } else { - /* True, since node_has_descriptor(node) == true and we are extending - * to the node's primary address */ - tor_assert(info); } circuit_append_new_exit(circ, info); extend_info_free(info); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 4c5372c..1d6fc0f 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -4162,6 +4162,9 @@ rend_consider_services_intro_points(void) * even if we are a single onion service and intend to connect to it * directly ourselves. */ intro->extend_info = extend_info_from_node(node, 0); + if (BUG(intro->extend_info == NULL)) { + break; + } intro->intro_key = crypto_pk_new(); const int fail = crypto_pk_generate_key(intro->intro_key); tor_assert(!fail);