tor-commits December 2018

tor-commits@lists.torproject.org

14 participants
1480 discussions

[tor/master] Emit router families in canonical form
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 05dee063c8d74437c792bdee2432293f97b6307c Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Nov 24 16:35:58 2018 -0500 Emit router families in canonical form This patch has routers use the same canonicalization logic as authorities when encoding their family lists. Additionally, they now warn if any router in their list is given by nickname, since that's error-prone. This patch also adds some long-overdue tests for family formatting. --- changes/ticket28266 | 5 ++ src/feature/relay/router.c | 135 +++++++++++++++++++++++++++++---------- src/feature/relay/router.h | 4 ++ src/test/test_router.c | 155 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 266 insertions(+), 33 deletions(-) diff --git a/changes/ticket28266 b/changes/ticket28266 index d90dc74f7..e0bc17108 100644 --- a/changes/ticket28266 +++ b/changes/ticket28266 @@ -3,3 +3,8 @@ under which microdescriptor entries are encoded in a canonical form. This improves their compressibility in transit and on the client. Closes ticket 28266; implements proposal 298. + + o Minor features (relay): + - When listing relay families, list them in canonical form including the + relay's own identity, and try to give a more useful set of warnings. + Part of ticket 28266 and proposal 298. diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 3f153e3a2..d57fcc3a4 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -30,6 +30,7 @@ #include "feature/nodelist/dirlist.h" #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/nickname.h" +#include "feature/nodelist/nodefamily.h" #include "feature/nodelist/nodelist.h" #include "feature/nodelist/routerlist.h" #include "feature/nodelist/torcert.h" @@ -340,6 +341,16 @@ set_server_identity_key(crypto_pk_t *k) } } +#ifdef TOR_UNIT_TESTS +/** Testing only -- set the server's RSA identity digest to + * be digest */ +void +set_server_identity_key_digest_testing(const uint8_t *digest) +{ + memcpy(server_identitykey_digest, digest, DIGEST_LEN); +} +#endif + /** Make sure that we have set up our identity keys to match or not match as * appropriate, and die with an assertion if we have not. */ static void @@ -1691,10 +1702,6 @@ router_get_descriptor_gen_reason(void) return desc_gen_reason; } -/** A list of nicknames that we've warned about including in our family - * declaration verbatim rather than as digests. */ -static smartlist_t *warned_nonexistent_family = NULL; - static int router_guess_address_from_dir_headers(uint32_t *guess); /** Make a current best guess at our address, either because @@ -1807,13 +1814,17 @@ router_check_descriptor_address_consistency(uint32_t ipv4h_desc_addr) CONN_TYPE_DIR_LISTENER); } +/** A list of nicknames that we've warned about including in our family, + * for one reason or another. */ +static smartlist_t *warned_family = NULL; + /** * Return a new smartlist containing the family members configured in * options. Warn about invalid or missing entries. Return NULL * if this relay should not declare a family. **/ -static smartlist_t * -get_declared_family(const or_options_t *options) +STATIC smartlist_t * +get_my_declared_family(const or_options_t *options) { if (!options->MyFamily) return NULL; @@ -1821,55 +1832,112 @@ get_declared_family(const or_options_t *options) if (options->BridgeRelay) return NULL; - if (!warned_nonexistent_family) - warned_nonexistent_family = smartlist_new(); + if (!warned_family) + warned_family = smartlist_new(); smartlist_t *declared_family = smartlist_new(); config_line_t *family; + + /* First we try to get the whole family in the form of hexdigests. */ for (family = options->MyFamily; family; family = family->next) { char *name = family->value; const node_t *member; - if (!strcasecmp(name, options->Nickname)) - continue; /* Don't list ourself, that's redundant */ + if (options->Nickname && !strcasecmp(name, options->Nickname)) + continue; /* Don't list ourself by nickname, that's redundant */ else member = node_get_by_nickname(name, 0); + if (!member) { + /* This node doesn't seem to exist, so warn about it if it is not + * a hexdigest. */ int is_legal = is_legal_nickname_or_hexdigest(name); - if (!smartlist_contains_string(warned_nonexistent_family, name) && + if (!smartlist_contains_string(warned_family, name) && !is_legal_hexdigest(name)) { if (is_legal) log_warn(LD_CONFIG, - "I have no descriptor for the router named \"%s\" in my " - "declared family; I'll use the nickname as is, but " - "this may confuse clients.", name); + "There is a router named %s in my declared family, but " + "I have no descriptor for it. I'll use the nickname " + "as is, but this may confuse clients. Please list it " + "by identity digest instead.", escaped(name)); else - log_warn(LD_CONFIG, "There is a router named \"%s\" in my " - "declared family, but that isn't a legal nickname. " + log_warn(LD_CONFIG, "There is a router named %s in my declared " + "family, but that isn't a legal digest or nickname. " "Skipping it.", escaped(name)); - smartlist_add_strdup(warned_nonexistent_family, name); + smartlist_add_strdup(warned_family, name); } if (is_legal) { smartlist_add_strdup(declared_family, name); } - } else if (router_digest_is_me(member->identity)) { - /* Don't list ourself in our own family; that's redundant */ - /* XXX shouldn't be possible */ } else { + /* List the node by digest. */ char *fp = tor_malloc(HEX_DIGEST_LEN+2); fp[0] = '$'; base16_encode(fp+1,HEX_DIGEST_LEN+1, member->identity, DIGEST_LEN); smartlist_add(declared_family, fp); - if (smartlist_contains_string(warned_nonexistent_family, name)) - smartlist_string_remove(warned_nonexistent_family, name); + + if (! is_legal_hexdigest(name) && + !smartlist_contains_string(warned_family, name)) { + /* Warn if this node was not specified by hexdigest. */ + log_warn(LD_CONFIG, "There is a router named %s in my declared " + "family, but it wasn't listed by digest. Please consider " + "saying %s instead, if that's what you meant.", + escaped(name), fp); + smartlist_add_strdup(warned_family, name); + } } } - /* remove duplicates from the list */ - smartlist_sort_strings(declared_family); - smartlist_uniq_strings(declared_family); + /* Now declared_family should have the closest we can come to the + * identities that the user wanted. + * + * Unlike older versions of Tor, we _do_ include our own identity: this + * helps microdescriptor compression, and helps in-memory compression + * on clients. */ + nodefamily_t *nf = nodefamily_from_members(declared_family, + router_get_my_id_digest(), + NF_WARN_MALFORMED, + NULL); + SMARTLIST_FOREACH(declared_family, char *, s, tor_free(s)); + smartlist_free(declared_family); + if (!nf) { + return NULL; + } + + char *s = nodefamily_format(nf); + nodefamily_free(nf); + + smartlist_t *result = smartlist_new(); + smartlist_split_string(result, s, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + tor_free(s); + + if (smartlist_len(result) == 1) { + /* This is a one-element list containing only ourself; instead return + * nothing */ + const char *singleton = smartlist_get(result, 0); + bool is_me = false; + if (singleton[0] == '$') { + char d[DIGEST_LEN]; + int n = base16_decode(d, sizeof(d), singleton+1, strlen(singleton+1)); + if (n == DIGEST_LEN && + fast_memeq(d, router_get_my_id_digest(), DIGEST_LEN)) { + is_me = true; + } + } + if (!is_me) { + // LCOV_EXCL_START + log_warn(LD_BUG, "Found a singleton family list with an element " + "that wasn't us! Element was %s", escaped(singleton)); + // LCOV_EXCL_STOP + } else { + SMARTLIST_FOREACH(result, char *, cp, tor_free(cp)); + smartlist_free(result); + return NULL; + } + } - return declared_family; + return result; } /** Build a fresh routerinfo, signed server descriptor, and extra-info document @@ -1986,7 +2054,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) tor_free(p_tmp); } - ri->declared_family = get_declared_family(options); + ri->declared_family = get_my_declared_family(options); /* Now generate the extrainfo. */ ei = tor_malloc_zero(sizeof(extrainfo_t)); @@ -3077,9 +3145,9 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, void router_reset_warnings(void) { - if (warned_nonexistent_family) { - SMARTLIST_FOREACH(warned_nonexistent_family, char *, cp, tor_free(cp)); - smartlist_clear(warned_nonexistent_family); + if (warned_family) { + SMARTLIST_FOREACH(warned_family, char *, cp, tor_free(cp)); + smartlist_clear(warned_family); } } @@ -3103,11 +3171,12 @@ router_free_all(void) memwipe(&curve25519_onion_key, 0, sizeof(curve25519_onion_key)); memwipe(&last_curve25519_onion_key, 0, sizeof(last_curve25519_onion_key)); - if (warned_nonexistent_family) { - SMARTLIST_FOREACH(warned_nonexistent_family, char *, cp, tor_free(cp)); - smartlist_free(warned_nonexistent_family); + if (warned_family) { + SMARTLIST_FOREACH(warned_family, char *, cp, tor_free(cp)); + smartlist_free(warned_family); } } + /* From the given RSA key object, convert it to ASN-1 encoded format and set * the newly allocated object in onion_pkey_out. The length of the key is set * in onion_pkey_len_out. */ diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index 4575172af..872eed1f4 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -117,6 +117,10 @@ void router_free_all(void); /* Used only by router.c and test.c */ STATIC void get_platform_str(char *platform, size_t len); STATIC int router_write_fingerprint(int hashed); +STATIC smartlist_t *get_my_declared_family(const or_options_t *options); +#ifdef TOR_UNIT_TESTS +void set_server_identity_key_digest_testing(const uint8_t *digest); +#endif #endif #endif /* !defined(TOR_ROUTER_H) */ diff --git a/src/test/test_router.c b/src/test/test_router.c index 921ec4290..b92e96ff3 100644 --- a/src/test/test_router.c +++ b/src/test/test_router.c @@ -7,16 +7,21 @@ * \brief Unittests for code in router.c **/ +#define CONFIG_PRIVATE +#define ROUTER_PRIVATE #include "core/or/or.h" #include "app/config/config.h" #include "core/mainloop/mainloop.h" #include "feature/hibernate/hibernate.h" +#include "feature/nodelist/node_st.h" +#include "feature/nodelist/nodelist.h" #include "feature/nodelist/routerinfo_st.h" #include "feature/nodelist/routerlist.h" #include "feature/relay/router.h" #include "feature/stats/rephist.h" #include "lib/crypt_ops/crypto_curve25519.h" #include "lib/crypt_ops/crypto_ed25519.h" +#include "lib/encoding/confline.h" /* Test suite stuff */ #include "test/test.h" @@ -231,11 +236,161 @@ test_router_check_descriptor_bandwidth_changed(void *arg) UNMOCK(we_are_hibernating); } +static node_t fake_node; +static const node_t * +mock_node_get_by_nickname(const char *name, unsigned flags) +{ + (void)flags; + if (!strcasecmp(name, "crumpet")) + return &fake_node; + else + return NULL; +} + +static void +test_router_get_my_family(void *arg) +{ + (void)arg; + or_options_t *options = options_new(); + smartlist_t *sl = NULL; + char *join = NULL; + // Overwrite the result of router_get_my_identity_digest(). This + // happens to be okay, but only for testing. + set_server_identity_key_digest_testing( + (const uint8_t*)"holeinthebottomofthe"); + + setup_capture_of_logs(LOG_WARN); + + // No family listed -- so there's no list. + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_EQ, NULL); + expect_no_log_entry(); + +#define CLEAR() do { \ + if (sl) { \ + SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp)); \ + smartlist_free(sl); \ + } \ + tor_free(join); \ + mock_clean_saved_logs(); \ + } while (0) + + // Add a single nice friendly hex member. This should be enough + // to have our own ID added. + tt_ptr_op(options->MyFamily, OP_EQ, NULL); + config_line_append(&options->MyFamily, "MyFamily", + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 2); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + expect_no_log_entry(); + CLEAR(); + + // Add a hex member with a ~. The ~ part should get removed. + config_line_append(&options->MyFamily, "MyFamily", + "$0123456789abcdef0123456789abcdef01234567~Muffin"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 3); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + expect_no_log_entry(); + CLEAR(); + + // Nickname lookup will fail, so a nickname will appear verbatim. + config_line_append(&options->MyFamily, "MyFamily", + "BAGEL"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 4); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "bagel"); + expect_single_log_msg_containing( + "There is a router named \"BAGEL\" in my declared family, but " + "I have no descriptor for it."); + CLEAR(); + + // A bogus digest should fail entirely. + config_line_append(&options->MyFamily, "MyFamily", + "$painauchocolat"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 4); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "bagel"); + // "BAGEL" is still there, but it won't make a warning, because we already + // warned about it. + expect_single_log_msg_containing( + "There is a router named \"$painauchocolat\" in my declared " + "family, but that isn't a legal digest or nickname. Skipping it."); + CLEAR(); + + // Let's introduce a node we can look up by nickname + memset(&fake_node, 0, sizeof(fake_node)); + memcpy(fake_node.identity, "whydoyouasknonononon", DIGEST_LEN); + MOCK(node_get_by_nickname, mock_node_get_by_nickname); + + config_line_append(&options->MyFamily, "MyFamily", + "CRUmpeT"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 5); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$776879646F796F7561736B6E6F6E6F6E6F6E6F6E " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "bagel"); + // "BAGEL" is still there, but it won't make a warning, because we already + // warned about it. Some with "$painauchocolat". + expect_single_log_msg_containing( + "There is a router named \"CRUmpeT\" in my declared " + "family, but it wasn't listed by digest. Please consider saying " + "$776879646F796F7561736B6E6F6E6F6E6F6E6F6E instead, if that's " + "what you meant."); + CLEAR(); + UNMOCK(node_get_by_nickname); + + // Try a singleton list containing only us: It should give us NULL. + config_free_lines(options->MyFamily); + config_line_append(&options->MyFamily, "MyFamily", + "$686F6C65696E746865626F74746F6D6F66746865"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_EQ, NULL); + expect_no_log_entry(); + + done: + or_options_free(options); + teardown_capture_of_logs(); + CLEAR(); + UNMOCK(node_get_by_nickname); + +#undef CLEAR +} + #define ROUTER_TEST(name, flags) \ { #name, test_router_ ## name, flags, NULL, NULL } struct testcase_t router_tests[] = { ROUTER_TEST(check_descriptor_bandwidth_changed, TT_FORK), ROUTER_TEST(dump_router_to_string_no_bridge_distribution_method, TT_FORK), + ROUTER_TEST(get_my_family, TT_FORK), END_OF_TESTCASES };

1 0

[tor/master] Add a function to canonicalize nodefamilies per prop298
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit d29e3a02d57aef402a1aaf9747ef44393b043d98 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Nov 24 10:53:38 2018 -0500 Add a function to canonicalize nodefamilies per prop298 This is the same as the regular canonical nodefamily format, except that unrecognized elements are preserved. --- src/feature/nodelist/nodefamily.c | 45 +++++++++++++++++++++++++++++++++++++-- src/feature/nodelist/nodefamily.h | 5 ++++- src/test/test_nodelist.c | 31 +++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/feature/nodelist/nodefamily.c b/src/feature/nodelist/nodefamily.c index 29659ed93..944ad5475 100644 --- a/src/feature/nodelist/nodefamily.c +++ b/src/feature/nodelist/nodefamily.c @@ -93,13 +93,47 @@ nodefamily_parse(const char *s, const uint8_t *rsa_id_self, { smartlist_t *sl = smartlist_new(); smartlist_split_string(sl, s, NULL, SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); - nodefamily_t *result = nodefamily_from_members(sl, rsa_id_self, flags); + nodefamily_t *result = nodefamily_from_members(sl, rsa_id_self, flags, NULL); SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp)); smartlist_free(sl); return result; } /** + * Canonicalize the family list s, returning a newly allocated string. + * + * The canonicalization rules are fully specified in dir-spec.txt, but, + * briefly: $hexid entries are put in caps, $hexid[=~]foo entries are + * truncated, nicknames are put into lowercase, unrecognized entries are left + * alone, and everything is sorted. + **/ +char * +nodefamily_canonicalize(const char *s, const uint8_t *rsa_id_self, + unsigned flags) +{ + smartlist_t *sl = smartlist_new(); + smartlist_t *result_members = smartlist_new(); + smartlist_split_string(sl, s, NULL, SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + nodefamily_t *nf = nodefamily_from_members(sl, rsa_id_self, flags, + result_members); + + char *formatted = nodefamily_format(nf); + smartlist_split_string(result_members, formatted, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + smartlist_sort_strings(result_members); + char *combined = smartlist_join_strings(result_members, " ", 0, NULL); + + nodefamily_free(nf); + SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp)); + smartlist_free(sl); + SMARTLIST_FOREACH(result_members, char *, cp, tor_free(cp)); + smartlist_free(result_members); + tor_free(formatted); + + return combined; +} + +/** * qsort helper for encoded nodefamily elements. **/ static int @@ -117,11 +151,15 @@ compare_members(const void *a, const void *b) * family declaration if it is not there already. * * The flags element is interpreted as in nodefamily_parse(). + * + * If unrecognized is provided, fill it copies of any unrecognized + * members. (Note that malformed $hexids are not considered unrecognized.) **/ nodefamily_t * nodefamily_from_members(const smartlist_t *members, const uint8_t *rsa_id_self, - unsigned flags) + unsigned flags, + smartlist_t *unrecognized_out) { const int n_self = rsa_id_self ? 1 : 0; int n_bad_elements = 0; @@ -146,6 +184,9 @@ nodefamily_from_members(const smartlist_t *members, ptr[0] = NODEFAMILY_BY_RSA_ID; memcpy(ptr+1, digest_buf, DIGEST_LEN); } + } else { + if (unrecognized_out) + smartlist_add_strdup(unrecognized_out, cp); } if (bad_element) { diff --git a/src/feature/nodelist/nodefamily.h b/src/feature/nodelist/nodefamily.h index 342f161a0..ea1076876 100644 --- a/src/feature/nodelist/nodefamily.h +++ b/src/feature/nodelist/nodefamily.h @@ -27,7 +27,8 @@ nodefamily_t *nodefamily_parse(const char *s, unsigned flags); nodefamily_t *nodefamily_from_members(const struct smartlist_t *members, const uint8_t *rsa_id_self, - unsigned flags); + unsigned flags, + smartlist_t *unrecognized_out); void nodefamily_free_(nodefamily_t *family); #define nodefamily_free(family) \ FREE_AND_NULL(nodefamily_t, nodefamily_free_, (family)) @@ -41,6 +42,8 @@ bool nodefamily_contains_node(const nodefamily_t *family, void nodefamily_add_nodes_to_smartlist(const nodefamily_t *family, struct smartlist_t *out); char *nodefamily_format(const nodefamily_t *family); +char *nodefamily_canonicalize(const char *s, const uint8_t *rsa_id_self, + unsigned flags); void nodefamily_free_all(void); diff --git a/src/test/test_nodelist.c b/src/test/test_nodelist.c index afbcc60ac..ed919f4ed 100644 --- a/src/test/test_nodelist.c +++ b/src/test/test_nodelist.c @@ -610,6 +610,36 @@ test_nodelist_node_nodefamily(void *arg) smartlist_free(nodes); } +static void +test_nodelist_nodefamily_canonicalize(void *arg) +{ + (void)arg; + char *c = NULL; + + c = nodefamily_canonicalize("", NULL, 0); + tt_str_op(c, OP_EQ, ""); + tor_free(c); + + uint8_t own_id[20]; + memset(own_id, 0, sizeof(own_id)); + c = nodefamily_canonicalize( + "alice BOB caroL %potrzebie !!!@#@# " + "$bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb=fred " + "ffffffffffffffffffffffffffffffffffffffff " + "$cccccccccccccccccccccccccccccccccccccccc ", own_id, 0); + tt_str_op(c, OP_EQ, + "!!!@#@# " + "$0000000000000000000000000000000000000000 " + "$BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB " + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC " + "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF " + "%potrzebie " + "alice bob carol"); + + done: + tor_free(c); +} + #define NODE(name, flags) \ { #name, test_nodelist_##name, (flags), NULL, NULL } @@ -623,5 +653,6 @@ struct testcase_t nodelist_tests[] = { NODE(nodefamily_lookup, TT_FORK), NODE(nickname_matches, 0), NODE(node_nodefamily, TT_FORK), + NODE(nodefamily_canonicalize, 0), END_OF_TESTCASES };

1 0

[tor/master] Merge remote-tracking branch 'public/prop298'
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 1eb3719a62074baef97ec1aa5c144096706fb97f Merge: 1f95e8035 05dee063c Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 5 09:43:03 2018 -0500 Merge remote-tracking branch 'public/prop298' changes/ticket28266 | 10 ++ src/feature/dirauth/dirvote.c | 18 +++- src/feature/dirauth/dirvote.h | 8 +- src/feature/nodelist/nodefamily.c | 47 +++++++- src/feature/nodelist/nodefamily.h | 5 +- src/feature/nodelist/nodefamily_st.h | 8 +- src/feature/relay/router.c | 203 +++++++++++++++++++++++++---------- src/feature/relay/router.h | 4 + src/test/test_microdesc.c | 35 +++++- src/test/test_nodelist.c | 49 +++++++-- src/test/test_router.c | 154 ++++++++++++++++++++++++++ 11 files changed, 462 insertions(+), 79 deletions(-) diff --cc src/feature/relay/router.h index 217511df9,872eed1f4..7a63c33d3 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@@ -117,10 -117,10 +117,14 @@@ void router_free_all(void) /* Used only by router.c and test.c */ STATIC void get_platform_str(char *platform, size_t len); STATIC int router_write_fingerprint(int hashed); + STATIC smartlist_t *get_my_declared_family(const or_options_t *options); ++ #ifdef TOR_UNIT_TESTS +extern time_t desc_clean_since; +extern const char *desc_dirty_reason; + void set_server_identity_key_digest_testing(const uint8_t *digest); #endif ++ #endif #endif /* !defined(TOR_ROUTER_H) */ diff --cc src/test/test_router.c index 18740dcb8,b92e96ff3..91cdd2c06 --- a/src/test/test_router.c +++ b/src/test/test_router.c @@@ -7,17 -7,16 +7,20 @@@ * \brief Unittests for code in router.c **/ + #define CONFIG_PRIVATE #define ROUTER_PRIVATE + #include "core/or/or.h" #include "app/config/config.h" #include "core/mainloop/mainloop.h" #include "feature/hibernate/hibernate.h" +#include "feature/nodelist/networkstatus.h" +#include "feature/nodelist/networkstatus_st.h" + #include "feature/nodelist/node_st.h" + #include "feature/nodelist/nodelist.h" #include "feature/nodelist/routerinfo_st.h" #include "feature/nodelist/routerlist.h" +#include "feature/nodelist/routerstatus_st.h" #include "feature/relay/router.h" #include "feature/stats/rephist.h" #include "lib/crypt_ops/crypto_curve25519.h" @@@ -236,104 -236,161 +240,254 @@@ test_router_check_descriptor_bandwidth_ UNMOCK(we_are_hibernating); } +static networkstatus_t *mock_ns = NULL; +static networkstatus_t * +mock_networkstatus_get_live_consensus(time_t now) +{ + (void)now; + return mock_ns; +} + +static routerstatus_t *mock_rs = NULL; +static const routerstatus_t * +mock_networkstatus_vote_find_entry(networkstatus_t *ns, const char *digest) +{ + (void)ns; + (void)digest; + return mock_rs; +} + +static void +test_router_mark_if_too_old(void *arg) +{ + (void)arg; + time_t now = approx_time(); + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + MOCK(networkstatus_vote_find_entry, mock_networkstatus_vote_find_entry); + + routerstatus_t rs; + networkstatus_t ns; + memset(&rs, 0, sizeof(rs)); + memset(&ns, 0, sizeof(ns)); + mock_ns = &ns; + mock_ns->valid_after = now-3600; + mock_rs = &rs; + mock_rs->published_on = now - 10; + + // no reason to mark this time. + desc_clean_since = now-10; + desc_dirty_reason = NULL; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, now-10); + + // Doesn't appear in consensus? Still don't mark it. + mock_ns = NULL; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, now-10); + mock_ns = &ns; + + // No new descriptor in a long time? Mark it. + desc_clean_since = now - 3600 * 96; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, 0); + tt_str_op(desc_dirty_reason, OP_EQ, "time for new descriptor"); + + // Version in consensus published a long time ago? We won't mark it + // if it's been clean for only a short time. + desc_clean_since = now - 10; + desc_dirty_reason = NULL; + mock_rs->published_on = now - 3600 * 96; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, now - 10); + + // ... but if it's been clean a while, we mark. + desc_clean_since = now - 2 * 3600; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, 0); + tt_str_op(desc_dirty_reason, OP_EQ, + "version listed in consensus is quite old"); + + // same deal if we're marked stale. + desc_clean_since = now - 2 * 3600; + desc_dirty_reason = NULL; + mock_rs->published_on = now - 10; + mock_rs->is_staledesc = 1; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, 0); + tt_str_op(desc_dirty_reason, OP_EQ, + "listed as stale in consensus"); + + // same deal if we're absent from the consensus. + desc_clean_since = now - 2 * 3600; + desc_dirty_reason = NULL; + mock_rs = NULL; + mark_my_descriptor_dirty_if_too_old(now); + tt_i64_op(desc_clean_since, OP_EQ, 0); + tt_str_op(desc_dirty_reason, OP_EQ, + "not listed in consensus"); + + done: + UNMOCK(networkstatus_get_live_consensus); + UNMOCK(networkstatus_vote_find_entry); +} + + static node_t fake_node; + static const node_t * + mock_node_get_by_nickname(const char *name, unsigned flags) + { + (void)flags; + if (!strcasecmp(name, "crumpet")) + return &fake_node; + else + return NULL; + } + + static void + test_router_get_my_family(void *arg) + { + (void)arg; + or_options_t *options = options_new(); + smartlist_t *sl = NULL; + char *join = NULL; + // Overwrite the result of router_get_my_identity_digest(). This + // happens to be okay, but only for testing. + set_server_identity_key_digest_testing( + (const uint8_t*)"holeinthebottomofthe"); + + setup_capture_of_logs(LOG_WARN); + + // No family listed -- so there's no list. + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_EQ, NULL); + expect_no_log_entry(); + + #define CLEAR() do { \ + if (sl) { \ + SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp)); \ + smartlist_free(sl); \ + } \ + tor_free(join); \ + mock_clean_saved_logs(); \ + } while (0) + + // Add a single nice friendly hex member. This should be enough + // to have our own ID added. + tt_ptr_op(options->MyFamily, OP_EQ, NULL); + config_line_append(&options->MyFamily, "MyFamily", + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 2); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + expect_no_log_entry(); + CLEAR(); + + // Add a hex member with a ~. The ~ part should get removed. + config_line_append(&options->MyFamily, "MyFamily", + "$0123456789abcdef0123456789abcdef01234567~Muffin"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 3); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + expect_no_log_entry(); + CLEAR(); + + // Nickname lookup will fail, so a nickname will appear verbatim. + config_line_append(&options->MyFamily, "MyFamily", + "BAGEL"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 4); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "bagel"); + expect_single_log_msg_containing( + "There is a router named \"BAGEL\" in my declared family, but " + "I have no descriptor for it."); + CLEAR(); + + // A bogus digest should fail entirely. + config_line_append(&options->MyFamily, "MyFamily", + "$painauchocolat"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 4); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "bagel"); + // "BAGEL" is still there, but it won't make a warning, because we already + // warned about it. + expect_single_log_msg_containing( + "There is a router named \"$painauchocolat\" in my declared " + "family, but that isn't a legal digest or nickname. Skipping it."); + CLEAR(); + + // Let's introduce a node we can look up by nickname + memset(&fake_node, 0, sizeof(fake_node)); + memcpy(fake_node.identity, "whydoyouasknonononon", DIGEST_LEN); + MOCK(node_get_by_nickname, mock_node_get_by_nickname); + + config_line_append(&options->MyFamily, "MyFamily", + "CRUmpeT"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_NE, NULL); + tt_int_op(smartlist_len(sl), OP_EQ, 5); + join = smartlist_join_strings(sl, " ", 0, NULL); + tt_str_op(join, OP_EQ, + "$0123456789ABCDEF0123456789ABCDEF01234567 " + "$686F6C65696E746865626F74746F6D6F66746865 " + "$776879646F796F7561736B6E6F6E6F6E6F6E6F6E " + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA " + "bagel"); + // "BAGEL" is still there, but it won't make a warning, because we already + // warned about it. Some with "$painauchocolat". + expect_single_log_msg_containing( + "There is a router named \"CRUmpeT\" in my declared " + "family, but it wasn't listed by digest. Please consider saying " + "$776879646F796F7561736B6E6F6E6F6E6F6E6F6E instead, if that's " + "what you meant."); + CLEAR(); + UNMOCK(node_get_by_nickname); + + // Try a singleton list containing only us: It should give us NULL. + config_free_lines(options->MyFamily); + config_line_append(&options->MyFamily, "MyFamily", + "$686F6C65696E746865626F74746F6D6F66746865"); + sl = get_my_declared_family(options); + tt_ptr_op(sl, OP_EQ, NULL); + expect_no_log_entry(); + + done: + or_options_free(options); + teardown_capture_of_logs(); + CLEAR(); + UNMOCK(node_get_by_nickname); + + #undef CLEAR + } + #define ROUTER_TEST(name, flags) \ { #name, test_router_ ## name, flags, NULL, NULL } struct testcase_t router_tests[] = { ROUTER_TEST(check_descriptor_bandwidth_changed, TT_FORK), ROUTER_TEST(dump_router_to_string_no_bridge_distribution_method, TT_FORK), + ROUTER_TEST(mark_if_too_old, TT_FORK), + ROUTER_TEST(get_my_family, TT_FORK), END_OF_TESTCASES };

1 0

[torspec/master] Merge prop293 and mark it closed.
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit d14164d8b9d7bf53c63f4720d2361a18e2d45e0e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 5 09:28:00 2018 -0500 Merge prop293 and mark it closed. --- dir-spec.txt | 11 +++++++++++ proposals/000-index.txt | 4 ++-- proposals/293-know-when-to-publish.txt | 6 +++++- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/dir-spec.txt b/dir-spec.txt index a6547ce..e9f201c 100644 --- a/dir-spec.txt +++ b/dir-spec.txt @@ -358,6 +358,11 @@ - Its uptime has been reset (by restarting). + - It receives a networkstatus consensus in which it is not listed. + + - It receives a networkstatus consensus in which it is listed + with the StaleDesc flag. + [XXX this list is incomplete; see router_differences_are_cosmetic() in routerlist.c for others] @@ -2293,6 +2298,8 @@ "NoEdConsensus" if any Ed25519 key in the router's descriptor or microdesriptor does not reflect authority consensus. "Stable" if the router is suitable for long-lived circuits. + "StaleDesc" if the router should upload a new descriptor because + the old one is too old. "Running" if the router is currently usable over all its published ORPorts. (Authorities ignore IPv6 ORPorts unless configured to check IPv6 reachability.) Relays without this flag are omitted @@ -2576,6 +2583,10 @@ "NoEdConsensus" -- authorities should not vote on this flag; it is produced as part of the consensus for consensus method 22 or later. + "StaleDesc" -- authorities should vote to assign this flag if the + published time on the descriptor is over 18 hours in the past. (This flag + was added in 0.4.0.1-alpha.) + Directory server administrators may label some relays or IPs as blacklisted, and elect not to include them in their network-status lists. diff --git a/proposals/000-index.txt b/proposals/000-index.txt index 51f56e5..7960f95 100644 --- a/proposals/000-index.txt +++ b/proposals/000-index.txt @@ -213,7 +213,7 @@ Proposals by number: 290 Continuously update consensus methods [META] 291 The move to two guard nodes [NEEDS-REVISION] 292 Mesh-based vanguards [ACCEPTED] -293 Other ways for relays to know when to publish [OPEN] +293 Other ways for relays to know when to publish [CLOSED] 294 TLS 1.3 Migration [DRAFT] 295 Using ADL-GCM for relay cryptography (solving the crypto-tagging attack) [OPEN] 296 Have Directory Authorities expose raw bandwidth list files [OPEN] @@ -247,7 +247,6 @@ Proposals by status: 285 Directory documents should be standardized as UTF-8 287 Reduce circuit lifetime without overloading the network 289 Authenticating sendme cells to mitigate bandwidth attacks - 293 Other ways for relays to know when to publish [for 0.3.5] 295 Using ADL-GCM for relay cryptography (solving the crypto-tagging attack) 296 Have Directory Authorities expose raw bandwidth list files 297 Relaxing the protover-based shutdown rules [for 0.3.5.x] @@ -356,6 +355,7 @@ Proposals by status: 278 Directory Compression Scheme Negotiation [in 0.3.1.1-alpha] 283 Move IPv6 ORPorts from microdescriptors to the microdesc consensus [for 0.3.3.x] [in 0.3.3.1-alpha] 284 Hidden Service v3 Control Port + 293 Other ways for relays to know when to publish [for 0.3.5] [in 0.4.0.1-alpha] SUPERSEDED: 112 Bring Back Pathlen Coin Weight 113 Simplifying directory authority administration diff --git a/proposals/293-know-when-to-publish.txt b/proposals/293-know-when-to-publish.txt index f2d96e0..c6fd1aa 100644 --- a/proposals/293-know-when-to-publish.txt +++ b/proposals/293-know-when-to-publish.txt @@ -2,8 +2,12 @@ Filename: 293-know-when-to-publish.txt Title: Other ways for relays to know when to publish Author: Nick Mathewson Created: 30-May-2018 -Status: Open +Status: Closed Target: 0.3.5 +Implemented-In: 0.4.0.1-alpha + + [IMPLEMENTATION NOTES: Mechanism one is implemented; mechanism two is + rejected.] 1. Motivation

1 0

[tor/master] changes file for prop293 / ticket 26770
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 439ffcefd57031153e49e605389a1c218e737180 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Nov 25 10:19:08 2018 -0500 changes file for prop293 / ticket 26770 --- changes/ticket26770 | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/changes/ticket26770 b/changes/ticket26770 new file mode 100644 index 000000000..7f3e92e9d --- /dev/null +++ b/changes/ticket26770 @@ -0,0 +1,8 @@ + o Minor features (directory authority, relay): + - Authorities now vote on a "StaleDesc" flag to indicate that a relay's + descriptor is so old that the relay should upload again soon. Relays + understand this flag, and treat it as a signal to upload a new + descriptor. This flag will eventually let us remove the 'published' + date from routerstatus entries, and save a great deal of space in our + consensus diffs. Closes ticket 26770; implements proposal 293. +

1 0

[tor/master] Keep list of dirauth flags in sync between dirvote.c and fuzz_vrs.c
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 32213fa9ad8a306e1f3bade1c95b1ad95a136bd9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Dec 3 12:18:45 2018 -0500 Keep list of dirauth flags in sync between dirvote.c and fuzz_vrs.c Suggested by Teor on PR --- src/feature/dirauth/dirvote.c | 20 ++++++++++++++++++-- src/feature/dirauth/dirvote.h | 3 +++ src/test/fuzz/fuzz_vrs.c | 8 ++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index aa4242f67..c6a219308 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -4372,6 +4372,23 @@ clear_status_flags_on_sybil(routerstatus_t *rs) * forget to add it to this clause. */ } +/** Space-separated list of all the flags that we will always vote on. */ +const char DIRVOTE_UNIVERSAL_FLAGS[] = + "Authority " + "Exit " + "Fast " + "Guard " + "HSDir " + "Stable " + "StaleDesc " + "V2Dir " + "Valid"; +/** Space-separated list of all flags that we may or may not vote on, + * depending on our configuration. */ +const char DIRVOTE_OPTIONAL_FLAGS[] = + "BadExit " + "Running"; + /** Return a new networkstatus_t* containing our current opinion. (For v3 * authorities) */ networkstatus_t * @@ -4620,8 +4637,7 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, v3_out->known_flags = smartlist_new(); smartlist_split_string(v3_out->known_flags, - "Authority Exit Fast Guard Stable V2Dir Valid HSDir " - "StaleDesc", + DIRVOTE_UNIVERSAL_FLAGS, 0, SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); if (vote_on_reachability) smartlist_add_strdup(v3_out->known_flags, "Running"); diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index a21e9f345..f99cc2d2b 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -92,6 +92,9 @@ /** Maximum size of a line in a vote. */ #define MAX_BW_FILE_HEADERS_LINE_LEN 1024 +extern const char DIRVOTE_UNIVERSAL_FLAGS[]; +extern const char DIRVOTE_OPTIONAL_FLAGS[]; + /* * Public API. Used outside of the dirauth subsystem. * diff --git a/src/test/fuzz/fuzz_vrs.c b/src/test/fuzz/fuzz_vrs.c index 3c6d205a3..f0d90d7cc 100644 --- a/src/test/fuzz/fuzz_vrs.c +++ b/src/test/fuzz/fuzz_vrs.c @@ -3,6 +3,7 @@ #define NS_PARSE_PRIVATE #define NETWORKSTATUS_PRIVATE #include "core/or/or.h" +#include "feature/dirauth/dirvote.h" #include "feature/dirparse/ns_parse.h" #include "feature/dirparse/unparseable.h" #include "lib/memarea/memarea.h" @@ -35,9 +36,12 @@ fuzz_init(void) dummy_vote = tor_malloc_zero(sizeof(*dummy_vote)); dummy_vote->known_flags = smartlist_new(); smartlist_split_string(dummy_vote->known_flags, - "Authority BadExit Exit Fast Guard HSDir " - "NoEdConsensus Running Stable V2Dir Valid", + DIRVOTE_UNIVERSAL_FLAGS, " ", 0, 0); + smartlist_split_string(dummy_vote->known_flags, + DIRVOTE_OPTIONAL_FLAGS, + " ", 0, 0); + smartlist_sort_strings(dummy_vote->known_flags); return 0; }

1 0

[tor/master] Make input argument const in set_routerstatus_from_routerinfo.
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 417a324a8577b0c61185c9d06eb72fbc483d984b Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Dec 3 12:34:29 2018 -0500 Make input argument const in set_routerstatus_from_routerinfo. --- src/feature/dirauth/voteflags.c | 4 ++-- src/feature/dirauth/voteflags.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/feature/dirauth/voteflags.c b/src/feature/dirauth/voteflags.c index 5adf21ad1..aab322d96 100644 --- a/src/feature/dirauth/voteflags.c +++ b/src/feature/dirauth/voteflags.c @@ -95,7 +95,7 @@ real_uptime(const routerinfo_t *router, time_t now) */ static int dirserv_thinks_router_is_unreliable(time_t now, - routerinfo_t *router, + const routerinfo_t *router, int need_uptime, int need_capacity) { if (need_uptime) { @@ -541,7 +541,7 @@ dirserv_set_router_is_running(routerinfo_t *router, time_t now) void set_routerstatus_from_routerinfo(routerstatus_t *rs, node_t *node, - routerinfo_t *ri, + const routerinfo_t *ri, time_t now, int listbadexits) { diff --git a/src/feature/dirauth/voteflags.h b/src/feature/dirauth/voteflags.h index 743a666cc..8dce9fbb0 100644 --- a/src/feature/dirauth/voteflags.h +++ b/src/feature/dirauth/voteflags.h @@ -19,7 +19,8 @@ int running_long_enough_to_decide_unreachable(void); void set_routerstatus_from_routerinfo(routerstatus_t *rs, node_t *node, - routerinfo_t *ri, time_t now, + const routerinfo_t *ri, + time_t now, int listbadexits); void dirserv_compute_performance_thresholds(digestmap_t *omit_as_sybil);

1 0

[tor/master] Treat the StaleDesc flag as making our descriptor dirty.
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 881b85cf328b5fbcd0108ecb0320374f638b9e47 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Nov 25 10:12:20 2018 -0500 Treat the StaleDesc flag as making our descriptor dirty. Relay side of prop293. --- src/feature/relay/router.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 9d61ced11..2d4ab9b0a 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -59,6 +59,7 @@ #include "feature/dircommon/dir_connection_st.h" #include "feature/nodelist/authority_cert_st.h" #include "feature/nodelist/extrainfo_st.h" +#include "feature/nodelist/networkstatus_st.h" #include "feature/nodelist/node_st.h" #include "feature/nodelist/routerinfo_st.h" #include "feature/nodelist/routerstatus_st.h" @@ -2134,7 +2135,9 @@ mark_my_descriptor_dirty_if_too_old(time_t now) /* Now we see whether we want to be retrying frequently or no. The * rule here is that we'll retry frequently if we aren't listed in the * live consensus we have, or if the publication time of the - * descriptor listed for us in the consensus is very old. */ + * descriptor listed for us in the consensus is very old, or if the + * consensus lists us as "stale" and we haven't regenerated since the + * consensus was published. */ ns = networkstatus_get_live_consensus(now); if (ns) { rs = networkstatus_vote_find_entry(ns, server_identitykey_digest); @@ -2142,6 +2145,8 @@ mark_my_descriptor_dirty_if_too_old(time_t now) retry_fast_reason = "not listed in consensus"; else if (rs->published_on < slow_cutoff) retry_fast_reason = "version listed in consensus is quite old"; + else if (rs->is_staledesc && ns->valid_after > desc_clean_since) + retry_fast_reason = "listed as stale in consensus"; } if (retry_fast_reason && desc_clean_since < fast_cutoff)

1 0

[tor/master] Vote on the StaleDesc flag from prop293
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 36f808c9364bb2e6781e555697eac977d754e905 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Nov 25 10:05:13 2018 -0500 Vote on the StaleDesc flag from prop293 The StaleDesc flag tells relays that they need to upload a new descriptor soon, or they will drop out of the consensus. --- src/feature/dirauth/dirvote.c | 3 ++- src/feature/dirauth/voteflags.c | 4 ++++ src/feature/dirauth/voteflags.h | 3 +++ src/feature/dirparse/ns_parse.c | 2 ++ src/feature/nodelist/fmt_routerstatus.c | 3 ++- src/feature/nodelist/routerstatus_st.h | 2 ++ 6 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 066a9e6e8..aa4242f67 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -4620,7 +4620,8 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, v3_out->known_flags = smartlist_new(); smartlist_split_string(v3_out->known_flags, - "Authority Exit Fast Guard Stable V2Dir Valid HSDir", + "Authority Exit Fast Guard Stable V2Dir Valid HSDir " + "StaleDesc", 0, SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); if (vote_on_reachability) smartlist_add_strdup(v3_out->known_flags, "Running"); diff --git a/src/feature/dirauth/voteflags.c b/src/feature/dirauth/voteflags.c index 4a24dcb50..5adf21ad1 100644 --- a/src/feature/dirauth/voteflags.c +++ b/src/feature/dirauth/voteflags.c @@ -593,6 +593,10 @@ set_routerstatus_from_routerinfo(routerstatus_t *rs, rs->or_port = ri->or_port; rs->dir_port = ri->dir_port; rs->is_v2_dir = ri->supports_tunnelled_dir_requests; + + rs->is_staledesc = + (ri->cache_info.published_on + DESC_IS_STALE_INTERVAL) < now; + if (options->AuthDirHasIPv6Connectivity == 1 && !tor_addr_is_null(&ri->ipv6_addr) && node->last_reachable6 >= now - REACHABLE_TIMEOUT) { diff --git a/src/feature/dirauth/voteflags.h b/src/feature/dirauth/voteflags.h index 2f0e061ea..743a666cc 100644 --- a/src/feature/dirauth/voteflags.h +++ b/src/feature/dirauth/voteflags.h @@ -25,6 +25,9 @@ void set_routerstatus_from_routerinfo(routerstatus_t *rs, void dirserv_compute_performance_thresholds(digestmap_t *omit_as_sybil); #ifdef VOTEFLAGS_PRIVATE +/** Any descriptor older than this age causes the authorities to set the + * StaleDesc flag. */ +#define DESC_IS_STALE_INTERVAL (18*60*60) STATIC void dirserv_set_routerstatus_testing(routerstatus_t *rs); #endif diff --git a/src/feature/dirparse/ns_parse.c b/src/feature/dirparse/ns_parse.c index 3fccec154..e0cdb2d46 100644 --- a/src/feature/dirparse/ns_parse.c +++ b/src/feature/dirparse/ns_parse.c @@ -434,6 +434,8 @@ routerstatus_parse_entry_from_string(memarea_t *area, rs->is_hs_dir = 1; } else if (!strcmp(tok->args[i], "V2Dir")) { rs->is_v2_dir = 1; + } else if (!strcmp(tok->args[i], "StaleDesc")) { + rs->is_staledesc = 1; } } /* These are implied true by having been included in a consensus made diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_routerstatus.c index e70aeb295..b1d4a4803 100644 --- a/src/feature/nodelist/fmt_routerstatus.c +++ b/src/feature/nodelist/fmt_routerstatus.c @@ -135,7 +135,7 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version, goto done; smartlist_add_asprintf(chunks, - "s%s%s%s%s%s%s%s%s%s%s\n", + "s%s%s%s%s%s%s%s%s%s%s%s\n", /* These must stay in alphabetical order. */ rs->is_authority?" Authority":"", rs->is_bad_exit?" BadExit":"", @@ -145,6 +145,7 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version, rs->is_hs_dir?" HSDir":"", rs->is_flagged_running?" Running":"", rs->is_stable?" Stable":"", + rs->is_staledesc?" StaleDesc":"", rs->is_v2_dir?" V2Dir":"", rs->is_valid?" Valid":""); diff --git a/src/feature/nodelist/routerstatus_st.h b/src/feature/nodelist/routerstatus_st.h index 714aa2743..ea0658779 100644 --- a/src/feature/nodelist/routerstatus_st.h +++ b/src/feature/nodelist/routerstatus_st.h @@ -47,6 +47,8 @@ struct routerstatus_t { unsigned int is_v2_dir:1; /** True iff this router publishes an open DirPort * or it claims to accept tunnelled dir requests. */ + unsigned int is_staledesc:1; /** True iff the authorities think this router + * should upload a new descriptor soon. */ unsigned int has_bandwidth:1; /**< The vote/consensus had bw info */ unsigned int has_exitsummary:1; /**< The vote/consensus had exit summaries */

1 0

[tor/master] Add a framework for testing set_routerstatus_from_routerinfo().
by nickm＠torproject.org 05 Dec '18

05 Dec '18

commit 92af8e511394685f7a39224ec1f73ce79fcf2c42 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Dec 3 13:22:23 2018 -0500 Add a framework for testing set_routerstatus_from_routerinfo(). Additionally, use it to test that is_staledesc is set correctly. Eventually we'll want to test all the other flags, but I'm aiming for only adding coverage on the changed code here. --- src/test/include.am | 1 + src/test/test.c | 3 +- src/test/test.h | 1 + src/test/test_voting_flags.c | 192 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 1 deletion(-) diff --git a/src/test/include.am b/src/test/include.am index e5eae56e2..d0f71fa66 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -177,6 +177,7 @@ src_test_test_SOURCES += \ src/test/test_util.c \ src/test/test_util_format.c \ src/test/test_util_process.c \ + src/test/test_voting_flags.c \ src/test/test_voting_schedule.c \ src/test/test_x509.c \ src/test/test_helpers.c \ diff --git a/src/test/test.c b/src/test/test.c index 17b736d30..bbb00da78 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -866,7 +866,8 @@ struct testgroup_t testgroups[] = { { "crypto/pem/", pem_tests }, { "dir/", dir_tests }, { "dir/md/", microdesc_tests }, - { "dir/voting-schedule/", voting_schedule_tests }, + { "dir/voting/flags/", voting_flags_tests }, + { "dir/voting/schedule/", voting_schedule_tests }, { "dir_handle_get/", dir_handle_get_tests }, { "dns/", dns_tests }, { "dos/", dos_tests }, diff --git a/src/test/test.h b/src/test/test.h index 092356f0f..1d8f79717 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -265,6 +265,7 @@ extern struct testcase_t tortls_tests[]; extern struct testcase_t util_format_tests[]; extern struct testcase_t util_process_tests[]; extern struct testcase_t util_tests[]; +extern struct testcase_t voting_flags_tests[]; extern struct testcase_t voting_schedule_tests[]; extern struct testcase_t x509_tests[]; diff --git a/src/test/test_voting_flags.c b/src/test/test_voting_flags.c new file mode 100644 index 000000000..0c4cedb51 --- /dev/null +++ b/src/test/test_voting_flags.c @@ -0,0 +1,192 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "orconfig.h" + +#define VOTEFLAGS_PRIVATE + +#include "core/or/or.h" + +#include "feature/dirauth/voteflags.h" +#include "feature/nodelist/node_st.h" +#include "feature/nodelist/routerstatus_st.h" +#include "feature/nodelist/routerinfo_st.h" + +#include "app/config/config.h" + +#include "test/test.h" + +typedef struct { + time_t now; + routerinfo_t ri; + node_t node; + + routerstatus_t expected; +} flag_vote_test_cfg_t; + +static void +setup_cfg(flag_vote_test_cfg_t *c) +{ + memset(c, 0, sizeof(*c)); + + c->now = approx_time(); + + c->ri.nickname = (char *) "testing100"; + strlcpy(c->expected.nickname, "testing100", sizeof(c->expected.nickname)); + + memset(c->ri.cache_info.identity_digest, 0xff, DIGEST_LEN); + memset(c->ri.cache_info.signed_descriptor_digest, 0xee, DIGEST256_LEN); + + c->ri.cache_info.published_on = c->now - 100; + c->expected.published_on = c->now - 100; + + c->ri.addr = 0x7f010105; + c->expected.addr = 0x7f010105; + c->ri.or_port = 9090; + c->expected.or_port = 9090; + + tor_addr_make_null(&c->ri.ipv6_addr, AF_INET6); + tor_addr_make_null(&c->expected.ipv6_addr, AF_INET6); + + // By default we have no loaded information about stability or speed, + // so we'll default to voting "yeah sure." on these two. + c->expected.is_fast = 1; + c->expected.is_stable = 1; +} + +static bool +check_result(flag_vote_test_cfg_t *c) +{ + bool result = false; + routerstatus_t rs; + memset(&rs, 0, sizeof(rs)); + set_routerstatus_from_routerinfo(&rs, &c->node, &c->ri, c->now, 0); + + tt_i64_op(rs.published_on, OP_EQ, c->expected.published_on); + tt_str_op(rs.nickname, OP_EQ, c->expected.nickname); + + // identity_digest and descriptor_digest are not set here. + + tt_uint_op(rs.addr, OP_EQ, c->expected.addr); + tt_uint_op(rs.or_port, OP_EQ, c->expected.or_port); + tt_uint_op(rs.dir_port, OP_EQ, c->expected.dir_port); + + tt_assert(tor_addr_eq(&rs.ipv6_addr, &c->expected.ipv6_addr)); + tt_uint_op(rs.ipv6_orport, OP_EQ, c->expected.ipv6_orport); + +#define FLAG(flagname) \ + tt_uint_op(rs.flagname, OP_EQ, c->expected.flagname) + + FLAG(is_authority); + FLAG(is_exit); + FLAG(is_stable); + FLAG(is_fast); + FLAG(is_flagged_running); + FLAG(is_named); + FLAG(is_unnamed); + FLAG(is_valid); + FLAG(is_possible_guard); + FLAG(is_bad_exit); + FLAG(is_hs_dir); + FLAG(is_v2_dir); + FLAG(is_staledesc); + FLAG(has_bandwidth); + FLAG(has_exitsummary); + FLAG(bw_is_unmeasured); + + result = true; + + done: + return result; +} + +static void +test_voting_flags_minimal(void *arg) +{ + flag_vote_test_cfg_t *cfg = arg; + check_result(cfg); +} + +static void +test_voting_flags_ipv6(void *arg) +{ + flag_vote_test_cfg_t *cfg = arg; + + tt_assert(tor_addr_parse(&cfg->ri.ipv6_addr, "f00::b42") == AF_INET6); + cfg->ri.ipv6_orport = 9091; + // no change in expected results, since we aren't set up with ipv6 + // connectivity. + if (!check_result(cfg)) + goto done; + + get_options_mutable()->AuthDirHasIPv6Connectivity = 1; + // no change in expected results, since last_reachable6 won't be set. + if (!check_result(cfg)) + goto done; + + cfg->node.last_reachable6 = cfg->now - 10; + // now that lastreachable6 is set, we expect to see the result. + tt_assert(tor_addr_parse(&cfg->expected.ipv6_addr, "f00::b42") == AF_INET6); + cfg->expected.ipv6_orport = 9091; + if (!check_result(cfg)) + goto done; + done: + ; +} + +static void +test_voting_flags_staledesc(void *arg) +{ + flag_vote_test_cfg_t *cfg = arg; + time_t now = cfg->now; + + cfg->ri.cache_info.published_on = now - DESC_IS_STALE_INTERVAL + 10; + cfg->expected.published_on = now - DESC_IS_STALE_INTERVAL + 10; + // no change in expectations for is_staledesc + if (!check_result(cfg)) + goto done; + + cfg->ri.cache_info.published_on = now - DESC_IS_STALE_INTERVAL - 10; + cfg->expected.published_on = now - DESC_IS_STALE_INTERVAL - 10; + cfg->expected.is_staledesc = 1; + if (!check_result(cfg)) + goto done; + + done: + ; +} + +static void * +setup_voting_flags_test(const struct testcase_t *testcase) +{ + (void)testcase; + flag_vote_test_cfg_t *cfg = tor_malloc_zero(sizeof(*cfg)); + setup_cfg(cfg); + return cfg; +} + +static int +teardown_voting_flags_test(const struct testcase_t *testcase, void *arg) +{ + (void)testcase; + flag_vote_test_cfg_t *cfg = arg; + tor_free(cfg); + return 1; +} + +static const struct testcase_setup_t voting_flags_setup = { + .setup_fn = setup_voting_flags_test, + .cleanup_fn = teardown_voting_flags_test, +}; + +#define T(name,flags) \ + { #name, test_voting_flags_##name, (flags), &voting_flags_setup, NULL } + +struct testcase_t voting_flags_tests[] = { + T(minimal, 0), + T(ipv6, TT_FORK), + // TODO: Add more of these tests. + T(staledesc, TT_FORK), + END_OF_TESTCASES +}; +

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits December 2018