Improve GETCONF exit-policy/* error handling

This will yield different error codes for transient and permament
errors. Furthermore, Tor will give human readable error
messages to controller.

src/or/policies.c

@@ -2999,11 +2999,13 @@ 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();
+    const routerinfo_t *me = NULL;
+
+    int err = router_get_my_routerinfo_with_err((routerinfo_t **)&me);
 
     if (!me) {
-      *errmsg = "router_get_my_routerinfo returned NULL";
+      *errmsg = routerinfo_errno_to_string(err);
-      return -1;
+      return routerinfo_err_is_transient(err) ? -1 : 0;
     }
 
     if (!options->ExitPolicyRejectPrivate &&
@@ -3038,11 +3040,12 @@ 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;
 
+    const routerinfo_t *me = NULL;
+    int err = router_get_my_routerinfo_with_err((routerinfo_t **)&me);
+
     if (!strcmp(question, "exit-policy/ipv4")) {
       include_ipv4 = 1;
     } else if (!strcmp(question, "exit-policy/ipv6")) {
@@ -3054,12 +3057,14 @@ getinfo_helper_policies(control_connection_t *conn,
     }
 
     if (!me) {
-      *errmsg = "router_get_my_routerinfo returned NULL";
+      *errmsg = routerinfo_errno_to_string(err);
-      return -1;
+      return routerinfo_err_is_transient(err) ? -1 : 0;
+    } else {
+      *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;
 }
 

src/or/router.c

@@ -114,6 +114,56 @@ 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_errno_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_NOT_SO_FAST:
+      return "Too soon; not ready yet";
+  }
+
+  log_warn(LD_BUG, "unknown errno %d", err);
+
+  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_NOT_SO_FAST:
+      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().
  */
@@ -2023,6 +2073,30 @@ router_get_my_routerinfo,(void))
   return desc_routerinfo;
 }
 
+/** Set <b>ri</b> to routerinfo of this OR. Rebuild it from
+ * scratch if needed. Return 0 on success or an appropriate
+ * TOR_ROUTERINFO_ERROR_* value on failure.
+ */
+MOCK_IMPL(int,
+router_get_my_routerinfo_with_err,(routerinfo_t **ri))
+{
+  if (!server_mode(get_options()))
+    return TOR_ROUTERINFO_ERROR_NOT_A_SERVER;
+
+  if (!desc_clean_since) {
+    int err = router_rebuild_descriptor(0);
+    if (err < 0)
+      return err;
+  }
+
+  if (!desc_routerinfo)
+    return TOR_ROUTERINFO_ERROR_NOT_SO_FAST;
+
+  if (ri)
+    *ri = desc_routerinfo;
+  return 0;
+}
+
 /** OR only: Return a signed server descriptor for this OR, rebuilding a fresh
  * one if necessary.  Return NULL on error.
  */
@@ -2196,7 +2270,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
@@ -2252,7 +2326,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());
@@ -2385,7 +2459,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);
@@ -2428,6 +2502,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;
@@ -2442,13 +2517,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_NOT_SO_FAST;
   }
 
   log_info(LD_OR, "Rebuilding relay descriptor%s", force ? " (forced)" : "");
 
-  if (router_build_fresh_descriptor(&ri, &ei) < 0) {
+  err = router_build_fresh_descriptor(&ri, &ei);
-    return -1;
+  if (err < 0) {
+    return err;
   }
 
   routerinfo_free(desc_routerinfo);

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_NOT_SO_FAST     (-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);
@@ -85,6 +92,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(int, router_get_my_routerinfo_with_err,(routerinfo_t **ri));
 extrainfo_t *router_get_my_extrainfo(void);
 const char *router_get_my_descriptor(void);
 const char *router_get_descriptor_gen_reason(void);
@@ -127,6 +135,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_errno_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);

src/test/test_policy.c

@@ -1496,10 +1496,18 @@ test_dump_exit_policy_to_string(void *arg)
 }
 
 static routerinfo_t *mock_desc_routerinfo = NULL;
-static const routerinfo_t *
+static int routerinfo_err;
-mock_router_get_my_routerinfo(void)
+
+static int
+mock_router_get_my_routerinfo_with_err(routerinfo_t **ri)
 {
-  return mock_desc_routerinfo;
+  if (routerinfo_err)
+    return routerinfo_err;
+
+  if (ri)
+    *ri = mock_desc_routerinfo;
+
+  return 0;
 }
 
 #define DEFAULT_POLICY_STRING "reject *:*"
@@ -1541,7 +1549,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 +1667,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_NOT_SO_FAST;
+  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, "Too soon; not ready yet");
+
  done:
   tor_free(answer);
   UNMOCK(get_options);