Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn the directory authority operator if their versions list is bogus #228

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions changes/bug26485
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 8 additions & 1 deletion src/or/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -3098,6 +3098,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.");
Expand Down Expand Up @@ -8003,4 +8011,3 @@ init_cookie_authentication(const char *fname, const char *header,
tor_free(cookie_file_str);
return retval;
}

43 changes: 37 additions & 6 deletions src/or/dirserv.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
static int routers_with_measured_bw = 0;

static void directory_remove_invalid(void);
static char *format_versions_list(config_line_t *ln);
struct authdir_config_t;
static uint32_t
dirserv_get_status_impl(const char *fp, const char *nickname,
Expand Down Expand Up @@ -1032,8 +1031,8 @@ list_server_status_v1(smartlist_t *routers, char **router_status_out,
* 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;
Expand All @@ -1042,6 +1041,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 comma-separated instead of space-separated. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems backward. The code looks like it checks for and warns about spaces.

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));
Expand Down Expand Up @@ -2860,8 +2890,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;
Expand Down Expand Up @@ -3879,4 +3911,3 @@ dirserv_free_all(void)

dirserv_clear_measured_bw_cache();
}

3 changes: 1 addition & 2 deletions src/or/dirserv.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,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);

#ifdef DIRSERV_PRIVATE
Expand Down Expand Up @@ -141,4 +141,3 @@ int dirserv_read_guardfraction_file(const char *fname,
smartlist_t *vote_routerstatuses);

#endif