[tor-commits] [tor/master] Prevention: never die from extend_info_from_node() failure.

[nickm at torproject.org]

commit 19e25d5cabd23f28044ccbddc01e5cacbde2cfcb
Author: Nick Mathewson <nickm at 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);