Merge remote-tracking branch 'rl1987/bug25852_take2'

5 files changed, 191 insertions, 19 deletions

diff --git a/changes/bug25852 b/changes/bug25852
new file mode 100644
index 0000000000..8b15638410
--- /dev/null
+++ b/changes/bug25852
@@ -0,0 +1,5 @@
+  o Minor bugfixes (control interface):
+    - Respond with more human readable error messages to GETINFO
+      exit-policy/* requests. Also, let controller know if error
+      is transient (response code 551) or not (response code 552).
+      Fixes bug 25852; bugfix on 0.2.8.1-alpha.
diff --git a/src/or/policies.c b/src/or/policies.c
index e0dbb021c6..1210ca687d 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -2999,11 +2999,12 @@ getinfo_helper_policies(control_connection_t *conn,
     smartlist_free(private_policy_strings);
   } else if (!strcmp(question, "exit-policy/reject-private/relay")) {
     const or_options_t *options = get_options();
-    const routerinfo_t *me = router_get_my_routerinfo();
+    int err = 0;
+    const routerinfo_t *me = router_get_my_routerinfo_with_err(&err);
 
     if (!me) {
-      *errmsg = "router_get_my_routerinfo returned NULL";
-      return -1;
+      *errmsg = routerinfo_err_to_string(err);
+      return routerinfo_err_is_transient(err) ? -1 : 0;
     }
 
     if (!options->ExitPolicyRejectPrivate &&
@@ -3038,11 +3039,17 @@ getinfo_helper_policies(control_connection_t *conn,
     SMARTLIST_FOREACH(configured_addresses, tor_addr_t *, a, tor_free(a));
     smartlist_free(configured_addresses);
   } else if (!strcmpstart(question, "exit-policy/")) {
-    const routerinfo_t *me = router_get_my_routerinfo();
-
     int include_ipv4 = 0;
     int include_ipv6 = 0;
 
+    int err = 0;
+    const routerinfo_t *me = router_get_my_routerinfo_with_err(&err);
+
+    if (!me) {
+      *errmsg = routerinfo_err_to_string(err);
+      return routerinfo_err_is_transient(err) ? -1 : 0;
+    }
+
     if (!strcmp(question, "exit-policy/ipv4")) {
       include_ipv4 = 1;
     } else if (!strcmp(question, "exit-policy/ipv6")) {
@@ -3053,13 +3060,10 @@ getinfo_helper_policies(control_connection_t *conn,
       return 0; /* No such key. */
     }
 
-    if (!me) {
-      *errmsg = "router_get_my_routerinfo returned NULL";
-      return -1;
-    }
-
-    *answer = router_dump_exit_policy_to_string(me,include_ipv4,include_ipv6);
+    *answer = router_dump_exit_policy_to_string(me,include_ipv4,
+                                                include_ipv6);
   }
+
   return 0;
 }
 
diff --git a/src/or/router.c b/src/or/router.c
index 07abf1f8d5..47416640d9 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -114,6 +114,57 @@ const char *format_node_description(char *buf,
                                     const tor_addr_t *addr,
                                     uint32_t addr32h);
 
+/** Return a readonly string with human readable description
+ * of <b>err</b>.
+ */
+const char *
+routerinfo_err_to_string(int err)
+{
+  switch (err) {
+    case TOR_ROUTERINFO_ERROR_NO_EXT_ADDR:
+      return "No known exit address yet";
+    case TOR_ROUTERINFO_ERROR_CANNOT_PARSE:
+      return "Cannot parse descriptor";
+    case TOR_ROUTERINFO_ERROR_NOT_A_SERVER:
+      return "Not running in server mode";
+    case TOR_ROUTERINFO_ERROR_DIGEST_FAILED:
+      return "Key digest failed";
+    case TOR_ROUTERINFO_ERROR_CANNOT_GENERATE:
+      return "Cannot generate descriptor";
+    case TOR_ROUTERINFO_ERROR_DESC_REBUILDING:
+      return "Descriptor still rebuilding - not ready yet";
+  }
+
+  log_warn(LD_BUG, "unknown routerinfo error %d - shouldn't happen", err);
+  tor_assert_unreached();
+
+  return "Unknown error";
+}
+
+/** Return true if we expect given error to be transient.
+ * Return false otherwise.
+ */
+int
+routerinfo_err_is_transient(int err)
+{
+  switch (err) {
+    case TOR_ROUTERINFO_ERROR_NO_EXT_ADDR:
+      return 1;
+    case TOR_ROUTERINFO_ERROR_CANNOT_PARSE:
+      return 1;
+    case TOR_ROUTERINFO_ERROR_NOT_A_SERVER:
+      return 0;
+    case TOR_ROUTERINFO_ERROR_DIGEST_FAILED:
+      return 0; // XXX: bug?
+    case TOR_ROUTERINFO_ERROR_CANNOT_GENERATE:
+      return 1;
+    case TOR_ROUTERINFO_ERROR_DESC_REBUILDING:
+      return 1;
+  }
+
+  return 0;
+}
+
 /** Replace the current onion key with <b>k</b>.  Does not affect
  * lastonionkey; to update lastonionkey correctly, call rotate_onion_key().
  */
@@ -2032,6 +2083,43 @@ router_get_my_routerinfo,(void))
   return desc_routerinfo;
 }
 
+/** Return routerinfo of this OR. Rebuild it from
+ * scratch if needed. Set <b>*err</b> to 0 on success or to
+ * appropriate TOR_ROUTERINFO_ERROR_* value on failure.
+ */
+MOCK_IMPL(const routerinfo_t *,
+router_get_my_routerinfo_with_err,(int *err))
+{
+  if (!server_mode(get_options())) {
+    if (err)
+      *err = TOR_ROUTERINFO_ERROR_NOT_A_SERVER;
+
+    return NULL;
+  }
+
+  if (!desc_clean_since) {
+    int rebuild_err = router_rebuild_descriptor(0);
+    if (rebuild_err < 0) {
+      if (err)
+        *err = rebuild_err;
+
+      return NULL;
+    }
+  }
+
+  if (!desc_routerinfo) {
+    if (err)
+      *err = TOR_ROUTERINFO_ERROR_DESC_REBUILDING;
+
+    return NULL;
+  }
+
+  if (err)
+    *err = 0;
+
+  return desc_routerinfo;
+}
+
 /** OR only: Return a signed server descriptor for this OR, rebuilding a fresh
  * one if necessary.  Return NULL on error.
  */
@@ -2205,7 +2293,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
 
   if (router_pick_published_address(options, &addr, 0) < 0) {
     log_warn(LD_CONFIG, "Don't know my address while generating descriptor");
-    return -1;
+    return TOR_ROUTERINFO_ERROR_NO_EXT_ADDR;
   }
 
   /* Log a message if the address in the descriptor doesn't match the ORPort
@@ -2261,7 +2349,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
   if (crypto_pk_get_digest(ri->identity_pkey,
                            ri->cache_info.identity_digest)<0) {
     routerinfo_free(ri);
-    return -1;
+    return TOR_ROUTERINFO_ERROR_DIGEST_FAILED;
   }
   ri->cache_info.signing_key_cert =
     tor_cert_dup(get_master_signing_key_cert());
@@ -2395,7 +2483,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
     log_warn(LD_BUG, "Couldn't generate router descriptor.");
     routerinfo_free(ri);
     extrainfo_free(ei);
-    return -1;
+    return TOR_ROUTERINFO_ERROR_CANNOT_GENERATE;
   }
   ri->cache_info.signed_descriptor_len =
     strlen(ri->cache_info.signed_descriptor_body);
@@ -2438,6 +2526,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
 int
 router_rebuild_descriptor(int force)
 {
+  int err = 0;
   routerinfo_t *ri;
   extrainfo_t *ei;
   uint32_t addr;
@@ -2452,13 +2541,14 @@ router_rebuild_descriptor(int force)
      * learn that it's time to try again when ip_address_changed()
      * marks it dirty. */
     desc_clean_since = time(NULL);
-    return -1;
+    return TOR_ROUTERINFO_ERROR_DESC_REBUILDING;
   }
 
   log_info(LD_OR, "Rebuilding relay descriptor%s", force ? " (forced)" : "");
 
-  if (router_build_fresh_descriptor(&ri, &ei) < 0) {
-    return -1;
+  err = router_build_fresh_descriptor(&ri, &ei);
+  if (err < 0) {
+    return err;
   }
 
   routerinfo_free(desc_routerinfo);
diff --git a/src/or/router.h b/src/or/router.h
index 0db2c1cfb2..752f2f2dbe 100644
--- a/src/or/router.h
+++ b/src/or/router.h
@@ -14,6 +14,13 @@
 
 #include "testsupport.h"
 
+#define TOR_ROUTERINFO_ERROR_NO_EXT_ADDR     (-1)
+#define TOR_ROUTERINFO_ERROR_CANNOT_PARSE    (-2)
+#define TOR_ROUTERINFO_ERROR_NOT_A_SERVER    (-3)
+#define TOR_ROUTERINFO_ERROR_DIGEST_FAILED   (-4)
+#define TOR_ROUTERINFO_ERROR_CANNOT_GENERATE (-5)
+#define TOR_ROUTERINFO_ERROR_DESC_REBUILDING (-6)
+
 crypto_pk_t *get_onion_key(void);
 time_t get_onion_key_set_at(void);
 void set_server_identity_key(crypto_pk_t *k);
@@ -86,6 +93,7 @@ void router_new_address_suggestion(const char *suggestion,
 int router_compare_to_my_exit_policy(const tor_addr_t *addr, uint16_t port);
 MOCK_DECL(int, router_my_exit_policy_is_reject_star,(void));
 MOCK_DECL(const routerinfo_t *, router_get_my_routerinfo, (void));
+MOCK_DECL(const routerinfo_t *, router_get_my_routerinfo_with_err,(int *err));
 extrainfo_t *router_get_my_extrainfo(void);
 const char *router_get_my_descriptor(void);
 const char *router_get_descriptor_gen_reason(void);
@@ -128,6 +136,9 @@ const char *node_describe(const node_t *node);
 const char *routerstatus_describe(const routerstatus_t *ri);
 const char *extend_info_describe(const extend_info_t *ei);
 
+const char *routerinfo_err_to_string(int err);
+int routerinfo_err_is_transient(int err);
+
 void router_get_verbose_nickname(char *buf, const routerinfo_t *router);
 void router_reset_warnings(void);
 void router_reset_reachability(void);
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index f180585861..e89d49aaf5 100644
--- a/src/test/test_policy.c
+++ b/src/test/test_policy.c
@@ -1496,9 +1496,21 @@ test_dump_exit_policy_to_string(void *arg)
 }
 
 static routerinfo_t *mock_desc_routerinfo = NULL;
+static int routerinfo_err;
+
 static const routerinfo_t *
-mock_router_get_my_routerinfo(void)
+mock_router_get_my_routerinfo_with_err(int *err)
 {
+  if (routerinfo_err) {
+    if (err)
+      *err = routerinfo_err;
+
+    return NULL;
+  }
+
+  if (err)
+    *err = 0;
+
   return mock_desc_routerinfo;
 }
 
@@ -1541,7 +1553,8 @@ test_policies_getinfo_helper_policies(void *arg)
   tor_free(answer);
 
   memset(&mock_my_routerinfo, 0, sizeof(routerinfo_t));
-  MOCK(router_get_my_routerinfo, mock_router_get_my_routerinfo);
+  MOCK(router_get_my_routerinfo_with_err,
+       mock_router_get_my_routerinfo_with_err);
   mock_my_routerinfo.exit_policy = smartlist_new();
   mock_desc_routerinfo = &mock_my_routerinfo;
 
@@ -1658,6 +1671,55 @@ test_policies_getinfo_helper_policies(void *arg)
   tt_assert(strlen(answer) == ipv4_len + ipv6_len + 1);
   tor_free(answer);
 
+  routerinfo_err = TOR_ROUTERINFO_ERROR_NO_EXT_ADDR;
+  rv = getinfo_helper_policies(NULL, "exit-policy/full", &answer,
+                               &errmsg);
+  tt_int_op(rv, OP_EQ, -1);
+  tt_ptr_op(answer, OP_EQ, NULL);
+  tt_ptr_op(errmsg, OP_NE, NULL);
+  tt_str_op(errmsg, OP_EQ, "No known exit address yet");
+
+  routerinfo_err = TOR_ROUTERINFO_ERROR_CANNOT_PARSE;
+  rv = getinfo_helper_policies(NULL, "exit-policy/full", &answer,
+                               &errmsg);
+  tt_int_op(rv, OP_EQ, -1);
+  tt_ptr_op(answer, OP_EQ, NULL);
+  tt_ptr_op(errmsg, OP_NE, NULL);
+  tt_str_op(errmsg, OP_EQ, "Cannot parse descriptor");
+
+  routerinfo_err = TOR_ROUTERINFO_ERROR_NOT_A_SERVER;
+  rv = getinfo_helper_policies(NULL, "exit-policy/full", &answer,
+                               &errmsg);
+  tt_int_op(rv, OP_EQ, 0);
+  tt_ptr_op(answer, OP_EQ, NULL);
+  tt_ptr_op(errmsg, OP_NE, NULL);
+  tt_str_op(errmsg, OP_EQ, "Not running in server mode");
+
+  routerinfo_err = TOR_ROUTERINFO_ERROR_DIGEST_FAILED;
+  rv = getinfo_helper_policies(NULL, "exit-policy/full", &answer,
+                               &errmsg);
+
+  tt_int_op(rv, OP_EQ, 0);
+  tt_ptr_op(answer, OP_EQ, NULL);
+  tt_ptr_op(errmsg, OP_NE, NULL);
+  tt_str_op(errmsg, OP_EQ, "Key digest failed");
+
+  routerinfo_err = TOR_ROUTERINFO_ERROR_CANNOT_GENERATE;
+  rv = getinfo_helper_policies(NULL, "exit-policy/full", &answer,
+                               &errmsg);
+  tt_int_op(rv, OP_EQ, -1);
+  tt_ptr_op(answer, OP_EQ, NULL);
+  tt_ptr_op(errmsg, OP_NE, NULL);
+  tt_str_op(errmsg, OP_EQ, "Cannot generate descriptor");
+
+  routerinfo_err = TOR_ROUTERINFO_ERROR_DESC_REBUILDING;
+  rv = getinfo_helper_policies(NULL, "exit-policy/full", &answer,
+                               &errmsg);
+  tt_int_op(rv, OP_EQ, -1);
+  tt_ptr_op(answer, OP_EQ, NULL);
+  tt_ptr_op(errmsg, OP_NE, NULL);
+  tt_str_op(errmsg, OP_EQ, "Descriptor still rebuilding - not ready yet");
+
  done:
   tor_free(answer);
   UNMOCK(get_options);