[tor-commits] [tor/maint-0.3.4] Warn the directory authority operator if their versions list is bogus

[nickm at torproject.org]

commit dcee4d4c9cc7d98e5ae8e913c73a3afc41753c64
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Jul 11 11:14:48 2018 -0400

    Warn the directory authority operator if their versions list is bogus
    
    Prevents bug 26485; bugfix on 0.1.1.6-alpha.
---
 changes/bug26485 |  4 ++++
 src/or/config.c  |  9 ++++++++-
 src/or/dirserv.c | 43 +++++++++++++++++++++++++++++++++++++------
 src/or/dirserv.h |  3 +--
 4 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/changes/bug26485 b/changes/bug26485
new file mode 100644
index 000000000..5a40b7a78
--- /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 75e406585..810f1e9a7 100644
--- a/src/or/config.c
+++ b/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;
 }
-
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 94290d5dd..177009208 100644
--- a/src/or/dirserv.c
+++ b/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 *
-format_versions_list(config_line_t *ln)
+char *
+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);
-    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;
@@ -3879,4 +3911,3 @@ dirserv_free_all(void)
 
   dirserv_clear_measured_bw_cache();
 }
-
diff --git a/src/or/dirserv.h b/src/or/dirserv.h
index 1e4f27e3d..624cd7e0b 100644
--- a/src/or/dirserv.h
+++ b/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
-