Merge branch 'bug26485_029_squashed' into maint-0.2.9

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.

src/or/config.c

@@ -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.");
@@ -8003,4 +8011,3 @@ init_cookie_authentication(const char *fname, const char *header,
   tor_free(cookie_file_str);
   return retval;
 }
-

src/or/dirserv.c

@@ -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,
@@ -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 *
+char *
-format_versions_list(config_line_t *ln)
+format_recommended_version_list(const config_line_t *ln, int warn)
 {
   smartlist_t *versions;
   char *result;
@@ -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 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));
@@ -2860,8 +2890,10 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key,
   }
 
   if (options->VersioningAuthoritativeDir) {
-    client_versions = format_versions_list(options->RecommendedClientVersions);
+    client_versions =
-    server_versions = format_versions_list(options->RecommendedServerVersions);
+      format_recommended_version_list(options->RecommendedClientVersions, 0);
+    server_versions =
+      format_recommended_version_list(options->RecommendedServerVersions, 0);
   }
 
   contact = get_options()->ContactInfo;
@@ -3879,4 +3911,3 @@ dirserv_free_all(void)
 
   dirserv_clear_measured_bw_cache();
 }
-

src/or/dirserv.h

@@ -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
@@ -141,4 +141,3 @@ int dirserv_read_guardfraction_file(const char *fname,
                                  smartlist_t *vote_routerstatuses);
 
 #endif
-

src/or/dirvote.c

@@ -673,6 +673,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);
@@ -4002,4 +4010,3 @@ vote_routerstatus_find_microdesc_hash(char *digest256_out,
   }
   return -1;
 }
-

src/test/test_dir.c

@@ -5456,6 +5456,57 @@ test_dir_assumed_flags(void *arg)
   routerstatus_free(rs);
 }
 
+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 }
 
@@ -5511,6 +5562,6 @@ struct testcase_t dir_tests[] = {
   DIR_ARG(find_dl_schedule, TT_FORK, "cf"),
   DIR_ARG(find_dl_schedule, TT_FORK, "ca"),
   DIR(assumed_flags, 0),
+  DIR(format_versions_list, TT_FORK),
   END_OF_TESTCASES
 };
-