commit fa04fe1674bfc786785c88fb3f49e8b8a8e0b9e5 Author: Daniel Pinto danielpinto52@gmail.com Date: Fri Apr 14 13:04:37 2017 +0100

MyFamily config string is now a list. #4998 --- doc/tor.1.txt | 13 +++---- src/or/config.c | 98 +++++++++++++++++++++++++++++++------------------- src/or/or.h | 2 +- src/or/router.c | 12 +++---- src/test/test_config.c | 38 +++++++++++++++----- 5 files changed, 103 insertions(+), 60 deletions(-)

diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 568d037..9168245 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1813,14 +1813,15 @@ is non-zero): If we have more onionskins queued for processing than we can process in this amount of time, reject new ones. (Default: 1750 msec)

-[[MyFamily]] **MyFamily** __node__,__node__,__...__:: +[[MyFamily]] **MyFamily** __node__:: Declare that this Tor server is controlled or administered by a group or organization identical or similar to that of the other servers, defined by - their identity fingerprints. When two servers both declare - that they are in the same 'family', Tor clients will not use them in the - same circuit. (Each server only needs to list the other servers in its - family; it doesn't need to list itself, but it won't hurt.) Do not list - any bridge relay as it would compromise its concealment. + + their identity fingerprints. This option can be repeated many times, for + multiple families. When two servers both declare that they are in the + same 'family', Tor clients will not use them in the same circuit. (Each + server only needs to list the other servers in its family; it doesn't need to + list itself, but it won't hurt.) Do not list any bridge relay as it would + compromise its concealment. + + When listing a node, it's better to list it by fingerprint than by nickname: fingerprints are more reliable. diff --git a/src/or/config.c b/src/or/config.c index 809ff49..46057e5 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -395,7 +395,7 @@ static config_var_t option_vars_[] = { V(MaxOnionQueueDelay, MSEC_INTERVAL, "1750 msec"), V(MaxUnparseableDescSizeToLog, MEMUNIT, "10 MB"), V(MinMeasuredBWsForAuthToIgnoreAdvertised, INT, "500"), - V(MyFamily, STRING, NULL), + V(MyFamily, LINELIST, NULL), V(NewCircuitPeriod, INTERVAL, "30 seconds"), OBSOLETE("NamingAuthoritativeDirectory"), V(NATDListenAddress, LINELIST, NULL), @@ -707,7 +707,8 @@ static int options_transition_affects_workers( const or_options_t *old_options, const or_options_t *new_options); static int options_transition_affects_descriptor( const or_options_t *old_options, const or_options_t *new_options); -static int check_nickname_list(char **lst, const char *name, char **msg); +static int normalize_nickname_list(config_line_t **lst, const char *name, + char **msg); static char *get_bindaddr_from_transport_listen_line(const char *line, const char *transport); static int parse_ports(or_options_t *options, int validate_only, @@ -3885,7 +3886,7 @@ options_validate(or_options_t *old_options, or_options_t *options, "You should also make sure you aren't listing this bridge's " "fingerprint in any other MyFamily."); } - if (check_nickname_list(&options->MyFamily, "MyFamily", msg)) + if (normalize_nickname_list(&options->MyFamily, "MyFamily", msg)) return -1; for (cl = options->NodeFamilies; cl; cl = cl->next) { routerset_t *rs = routerset_new(); @@ -4593,7 +4594,7 @@ options_transition_affects_descriptor(const or_options_t *old_options, get_effective_bwburst(old_options) != get_effective_bwburst(new_options) || !opt_streq(old_options->ContactInfo, new_options->ContactInfo) || - !opt_streq(old_options->MyFamily, new_options->MyFamily) || + !config_lines_eq(old_options->MyFamily, new_options->MyFamily) || !opt_streq(old_options->AccountingStart, new_options->AccountingStart) || old_options->AccountingMax != new_options->AccountingMax || old_options->AccountingRule != new_options->AccountingRule || @@ -4689,27 +4690,33 @@ get_default_conf_file(int defaults_file) #endif }

-/** Verify whether lst is a string containing valid-looking comma-separated - * nicknames, or NULL. Will normalise <b>lst</b> to prefix '$' to any nickname - * or fingerprint that needs it. Return 0 on success. +/** Verify whether lst is a list of strings containing valid-looking + * comma-separated nicknames, or NULL. Will normalise <b>lst</b> to prefix '$' + * to any nickname or fingerprint that needs it. Also splits comma-separated + * list elements into multiple elements. Return 0 on success. * Warn and return -1 on failure. */ static int -check_nickname_list(char **lst, const char *name, char **msg) +normalize_nickname_list(config_line_t **lst, const char *name, char **msg) { - int r = 0; - smartlist_t *sl; - int changes = 0; - if (!*lst) return 0; - sl = smartlist_new();

- smartlist_split_string(sl, *lst, ",", - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK|SPLIT_STRIP_SPACE, 0); + config_line_t *new_nicknames = NULL; + config_line_t *new_nicknames_last = NULL; + config_line_t *cl; + for (cl = *lst; cl; cl = cl->next) { + char *line = cl->value; + if (!line) + continue;

- SMARTLIST_FOREACH_BEGIN(sl, char *, s) + int valid_line = 1; + smartlist_t *sl = smartlist_new(); + smartlist_split_string(sl, line, ",", + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK|SPLIT_STRIP_SPACE, 0); + SMARTLIST_FOREACH_BEGIN(sl, char *, s) { + char *normalized = NULL; if (!is_legal_nickname_or_hexdigest(s)) { // check if first char is dollar if (s[0] != '$') { @@ -4718,36 +4725,53 @@ check_nickname_list(char **lst, const char *name, char **msg) tor_asprintf(&prepended, "$%s", s);

if (is_legal_nickname_or_hexdigest(prepended)) { - // The nickname is valid when it's prepended, swap the current - // version with a prepended one - tor_free(s); - SMARTLIST_REPLACE_CURRENT(sl, s, prepended); - changes = 1; - continue; + // The nickname is valid when it's prepended, set it as the + // normalized version + normalized = prepended; + } else { + // Still not valid, free and fallback to error message + tor_free(prepended); } + }

- // Still not valid, free and fallback to error message - tor_free(prepended); + if (!normalized) { + tor_asprintf(msg, "Invalid nickname '%s' in %s line", s, name); + valid_line = 0; + break; } + } else { + normalized = tor_strdup(s); + }

- tor_asprintf(msg, "Invalid nickname '%s' in %s line", s, name); - r = -1; - break; + config_line_t *next = tor_malloc_zero(sizeof(*next)); + next->key = tor_strdup(cl->key); + next->value = normalized; + next->next = NULL; + + if (!new_nicknames) { + new_nicknames = next; + new_nicknames_last = next; + } else { + new_nicknames_last->next = next; + new_nicknames_last = next; } - } - SMARTLIST_FOREACH_END(s);

- // Replace the caller's nickname list with a fixed one - if (changes && r == 0) { - char *newNicknames = smartlist_join_strings(sl, ", ", 0, NULL); - tor_free(*lst); - *lst = newNicknames; + } SMARTLIST_FOREACH_END(s); + + SMARTLIST_FOREACH(sl, char *, s, tor_free(s)); + smartlist_free(sl); + + if (!valid_line) { + config_free_lines(new_nicknames); + return -1; + } }

- SMARTLIST_FOREACH(sl, char *, s, tor_free(s)); - smartlist_free(sl); + // Replace the caller's nickname list with the normalized one + config_free_lines(*lst); + *lst = new_nicknames;

- return r; + return 0; }

/** Learn config file name from command line arguments, or use the default. diff --git a/src/or/or.h b/src/or/or.h index a7b3a66..41b3622 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3951,7 +3951,7 @@ typedef struct { /** If set, use these bridge authorities and not the default one. */ config_line_t *AlternateBridgeAuthority;

- char *MyFamily; /**< Declared family for this OR. */ + config_line_t *MyFamily; /**< Declared family for this OR. */ config_line_t *NodeFamilies; /**< List of config lines for * node families */ smartlist_t *NodeFamilySets; /**< List of parsed NodeFamilies values. */ diff --git a/src/or/router.c b/src/or/router.c index 7fb49e8..c02cf53 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2278,14 +2278,12 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) }

if (options->MyFamily && ! options->BridgeRelay) { - smartlist_t *family; if (!warned_nonexistent_family) warned_nonexistent_family = smartlist_new(); - family = smartlist_new(); ri->declared_family = smartlist_new(); - smartlist_split_string(family, options->MyFamily, ",", - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK|SPLIT_STRIP_SPACE, 0); - SMARTLIST_FOREACH_BEGIN(family, char *, name) { + config_line_t *family; + for (family = options->MyFamily; family; family = family->next) { + char *name = family->value; const node_t *member; if (!strcasecmp(name, options->Nickname)) goto skip; /* Don't list ourself, that's redundant */ @@ -2324,13 +2322,11 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) } skip: tor_free(name); - } SMARTLIST_FOREACH_END(name); + }

/* remove duplicates from the list */ smartlist_sort_strings(ri->declared_family); smartlist_uniq_strings(ri->declared_family); - - smartlist_free(family); }

/* Now generate the extrainfo. */ diff --git a/src/test/test_config.c b/src/test/test_config.c index f0874e0..ccada94 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -854,9 +854,23 @@ static void test_config_fix_my_family(void *arg) { char *err = NULL; - const char *family = "$1111111111111111111111111111111111111111, " - "1111111111111111111111111111111111111112, " - "$1111111111111111111111111111111111111113"; + config_line_t *family = tor_malloc_zero(sizeof(config_line_t)); + family->key = tor_strdup("MyFamily"); + family->value = tor_strdup("$1111111111111111111111111111111111111111, " + "1111111111111111111111111111111111111112, " + "$1111111111111111111111111111111111111113"); + + config_line_t *family2 = tor_malloc_zero(sizeof(config_line_t)); + family2->key = tor_strdup("MyFamily"); + family2->value = tor_strdup("1111111111111111111111111111111111111114"); + + config_line_t *family3 = tor_malloc_zero(sizeof(config_line_t)); + family3->key = tor_strdup("MyFamily"); + family3->value = tor_strdup("$1111111111111111111111111111111111111115"); + + family->next = family2; + family2->next = family3; + family3->next = NULL;

or_options_t* options = options_new(); or_options_t* defaults = options_new(); @@ -864,7 +878,7 @@ test_config_fix_my_family(void *arg)

options_init(options); options_init(defaults); - options->MyFamily = tor_strdup(family); + options->MyFamily = family;

options_validate(NULL, options, defaults, 0, &err) ;

@@ -872,10 +886,18 @@ test_config_fix_my_family(void *arg) TT_FAIL(("options_validate failed: %s", err)); }

- tt_str_op(options->MyFamily,OP_EQ, - "$1111111111111111111111111111111111111111, " - "$1111111111111111111111111111111111111112, " - "$1111111111111111111111111111111111111113"); + const char *valid[] = { "$1111111111111111111111111111111111111111", + "$1111111111111111111111111111111111111112", + "$1111111111111111111111111111111111111113", + "$1111111111111111111111111111111111111114", + "$1111111111111111111111111111111111111115" }; + int ret_size = 0; + config_line_t *ret; + for (ret = options->MyFamily; ret && ret_size < 5; ret = ret->next) { + tt_str_op(ret->value, OP_EQ, valid[ret_size]); + ret_size++; + } + tt_int_op(ret_size, OP_EQ, 5);

done: if (err != NULL) {