commit b00d17aa9e312e37fa05a4e3f05fedb28b67155d Author: rl1987 rl1987@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);