diff --git a/changes/bug26485 b/changes/bug26485 new file mode 100644 index 0000000000..5a40b7a78e --- /dev/null +++ b/changes/bug26485 @@ -0,0 +1,4 @@ + o Minor bugfixes (directory authority): + - When voting for recommended versions, make sure that all of the + versions are well-formed and parsable. Fixes bug 26485; bugfix on + 0.1.1.6-alpha. diff --git a/src/or/config.c b/src/or/config.c index 94a58f3488..6f774b2605 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3516,6 +3516,14 @@ options_validate(or_options_t *old_options, or_options_t *options, !options->RecommendedServerVersions)) REJECT("Versioning authoritative dir servers must set " "Recommended*Versions."); + + char *t; + /* Call these functions to produce warnings only. */ + t = format_recommended_version_list(options->RecommendedClientVersions, 1); + tor_free(t); + t = format_recommended_version_list(options->RecommendedServerVersions, 1); + tor_free(t); + if (options->UseEntryGuards) { log_info(LD_CONFIG, "Authoritative directory servers can't set " "UseEntryGuards. Disabling."); @@ -8449,4 +8457,3 @@ options_any_client_port_set(const or_options_t *options) options->DNSPort_set || options->HTTPTunnelPort_set); } - diff --git a/src/or/dirauth/dirvote.c b/src/or/dirauth/dirvote.c index b097b10cf9..101826778e 100644 --- a/src/or/dirauth/dirvote.c +++ b/src/or/dirauth/dirvote.c @@ -778,6 +778,14 @@ compute_consensus_versions_list(smartlist_t *lst, int n_versioning) int min = n_versioning / 2; smartlist_t *good = smartlist_new(); char *result; + SMARTLIST_FOREACH_BEGIN(lst, const char *, v) { + if (strchr(v, ' ')) { + log_warn(LD_DIR, "At least one authority has voted for a version %s " + "that contains a space. This probably wasn't intentional, and " + "is likely to cause trouble. Please tell them to stop it.", + escaped(v)); + } + } SMARTLIST_FOREACH_END(v); sort_version_list(lst, 0); get_frequent_members(good, lst, min); result = smartlist_join_strings(good, ",", 0, NULL); @@ -4181,8 +4189,8 @@ version_from_platform(const char *platform) * allocate and return a new string containing the version numbers, in order, * separated by commas. Used to generate Recommended(Client|Server)?Versions */ -static char * -format_versions_list(config_line_t *ln) +char * +format_recommended_version_list(const config_line_t *ln, int warn) { smartlist_t *versions; char *result; @@ -4191,6 +4199,37 @@ format_versions_list(config_line_t *ln) smartlist_split_string(versions, ln->value, ",", SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); } + + /* Handle the case where a dirauth operator has accidentally made some + * versions space-separated instead of comma-separated. */ + smartlist_t *more_versions = smartlist_new(); + SMARTLIST_FOREACH_BEGIN(versions, char *, v) { + if (strchr(v, ' ')) { + if (warn) + log_warn(LD_DIRSERV, "Unexpected space in versions list member %s. " + "(These are supposed to be comma-separated; I'll pretend you " + "used commas instead.)", escaped(v)); + SMARTLIST_DEL_CURRENT(versions, v); + smartlist_split_string(more_versions, v, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + tor_free(v); + } + } SMARTLIST_FOREACH_END(v); + smartlist_add_all(versions, more_versions); + smartlist_free(more_versions); + + /* Check to make sure everything looks like a version. */ + if (warn) { + SMARTLIST_FOREACH_BEGIN(versions, const char *, v) { + tor_version_t ver; + if (tor_version_parse(v, &ver) < 0) { + log_warn(LD_DIRSERV, "Recommended version %s does not look valid. " + " (I'll include it anyway, since you told me to.)", + escaped(v)); + } + } SMARTLIST_FOREACH_END(v); + } + sort_version_list(versions, 1); result = smartlist_join_strings(versions,",",0,NULL); SMARTLIST_FOREACH(versions,char *,s,tor_free(s)); @@ -4306,8 +4345,10 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, } if (options->VersioningAuthoritativeDir) { - client_versions = format_versions_list(options->RecommendedClientVersions); - server_versions = format_versions_list(options->RecommendedServerVersions); + client_versions = + format_recommended_version_list(options->RecommendedClientVersions, 0); + server_versions = + format_recommended_version_list(options->RecommendedServerVersions, 0); } contact = get_options()->ContactInfo; @@ -4547,4 +4588,3 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, return v3_out; } - diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 2362089d32..4e09c1c65b 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3581,4 +3581,3 @@ dirserv_free_all(void) dirserv_clear_measured_bw_cache(); } - diff --git a/src/or/dirserv.h b/src/or/dirserv.h index 9026f332bc..b243439cc2 100644 --- a/src/or/dirserv.h +++ b/src/or/dirserv.h @@ -155,7 +155,7 @@ char *routerstatus_format_entry( void dirserv_free_all(void); void cached_dir_decref(cached_dir_t *d); cached_dir_t *new_cached_dir(char *s, time_t published); - +char *format_recommended_version_list(const config_line_t *line, int warn); int validate_recommended_package_line(const char *line); int dirserv_query_measured_bw_cache_kb(const char *node_id, long *bw_out, @@ -212,4 +212,3 @@ void dirserv_spool_sort(dir_connection_t *conn); void dir_conn_clear_spool(dir_connection_t *conn); #endif /* !defined(TOR_DIRSERV_H) */ - diff --git a/src/test/test_dir.c b/src/test/test_dir.c index b42755e351..37b015b72d 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -5956,6 +5956,57 @@ test_dir_networkstatus_consensus_has_ipv6(void *arg) UNMOCK(networkstatus_get_latest_consensus_by_flavor); } +static void +test_dir_format_versions_list(void *arg) +{ + (void)arg; + char *s = NULL; + config_line_t *lines = NULL; + + setup_capture_of_logs(LOG_WARN); + s = format_recommended_version_list(lines, 1); + tt_str_op(s, OP_EQ, ""); + + tor_free(s); + config_line_append(&lines, "ignored", "0.3.4.1, 0.2.9.111-alpha, 4.4.4-rc"); + s = format_recommended_version_list(lines, 1); + tt_str_op(s, OP_EQ, "0.2.9.111-alpha,0.3.4.1,4.4.4-rc"); + + tor_free(s); + config_line_append(&lines, "ignored", "0.1.2.3,0.2.9.10 "); + s = format_recommended_version_list(lines, 1); + tt_str_op(s, OP_EQ, "0.1.2.3,0.2.9.10,0.2.9.111-alpha,0.3.4.1,4.4.4-rc"); + + /* There should be no warnings so far. */ + expect_no_log_entry(); + + /* Now try a line with a space in it. */ + tor_free(s); + config_line_append(&lines, "ignored", "1.3.3.8 1.3.3.7"); + s = format_recommended_version_list(lines, 1); + tt_str_op(s, OP_EQ, "0.1.2.3,0.2.9.10,0.2.9.111-alpha,0.3.4.1," + "1.3.3.7,1.3.3.8,4.4.4-rc"); + + expect_single_log_msg_containing( + "Unexpected space in versions list member \"1.3.3.8 1.3.3.7\"." ); + + /* Start over, with a line containing a bogus version */ + config_free_lines(lines); + lines = NULL; + tor_free(s); + mock_clean_saved_logs(); + config_line_append(&lines, "ignored", "0.1.2.3, alpha-complex, 0.1.1.8-rc"); + s = format_recommended_version_list(lines,1); + tt_str_op(s, OP_EQ, "0.1.1.8-rc,0.1.2.3,alpha-complex"); + expect_single_log_msg_containing( + "Recommended version \"alpha-complex\" does not look valid."); + + done: + tor_free(s); + config_free_lines(lines); + teardown_capture_of_logs(); +} + #define DIR_LEGACY(name) \ { #name, test_dir_ ## name , TT_FORK, NULL, NULL } @@ -6026,6 +6077,6 @@ struct testcase_t dir_tests[] = { DIR(networkstatus_compute_bw_weights_v10, 0), DIR(platform_str, 0), DIR(networkstatus_consensus_has_ipv6, TT_FORK), + DIR(format_versions_list, TT_FORK), END_OF_TESTCASES }; -