[tor-commits] [tor/master] Improve GETCONF exit-policy/* error handling
nickm at torproject.org
nickm at torproject.org
Mon May 14 18:14:26 UTC 2018
commit b00d17aa9e312e37fa05a4e3f05fedb28b67155d
Author: rl1987 <rl1987 at sdf.lonestar.org>
Date: Thu May 3 17:07:29 2018 +0200
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 | 23 +++++++------
src/or/router.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++----
src/or/router.h | 11 +++++++
src/test/test_policy.c | 66 ++++++++++++++++++++++++++++++++++---
4 files changed, 169 insertions(+), 19 deletions(-)
diff --git a/src/or/policies.c b/src/or/policies.c
index e0dbb021c..15176a97e 100644
--- a/src/or/policies.c
+++ b/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";
- return -1;
+ *errmsg = routerinfo_errno_to_string(err);
+ 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";
- return -1;
+ *errmsg = routerinfo_errno_to_string(err);
+ 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;
}
diff --git a/src/or/router.c b/src/or/router.c
index 996a28a91..612e23b3a 100644
--- a/src/or/router.c
+++ b/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) {
- 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 03eca9c65..bf0267b77 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_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);
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index f18058586..aeddc1417 100644
--- a/src/test/test_policy.c
+++ b/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 *
-mock_router_get_my_routerinfo(void)
+static int routerinfo_err;
+
+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);
More information about the tor-commits
mailing list