MyFamily option will now fix fingerprints missing their leading instead of complaining. Should fix #4341.

3 files changed, 76 insertions, 6 deletions

diff --git a/changes/bug4341 b/changes/bug4341
new file mode 100644
index 000000000..8853a86a0
--- /dev/null
+++ b/changes/bug4341
@@ -0,0 +1,2 @@
+o Minor bugfix:
+    - Config now handles fingerprints which are missing their initial '$'
diff --git a/src/or/config.c b/src/or/config.c
index e746da220..2792f1477 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -521,7 +521,7 @@ 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(const char *lst, const char *name, char **msg);
+static int check_nickname_list(const char **lst, const char *name, char **msg);
 
 static int parse_client_transport_line(const char *line, int validate_only);
 
@@ -3090,7 +3090,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 (check_nickname_list((const char **)&options->MyFamily, "MyFamily", msg))
     return -1;
   for (cl = options->NodeFamilies; cl; cl = cl->next) {
     routerset_t *rs = routerset_new();
@@ -3621,31 +3621,63 @@ get_default_conf_file(int defaults_file)
 }
 
 /** Verify whether lst is a string containing valid-looking comma-separated
- * nicknames, or NULL. Return 0 on success. Warn and return -1 on failure.
+ * nicknames, or NULL. Will normalise <b>lst</b> to prefix '$' to any nickname
+ * or fingerprint that needs it. Return 0 on success.
+ * Warn and return -1 on failure.
  */
 static int
-check_nickname_list(const char *lst, const char *name, char **msg)
+check_nickname_list(const char **lst, const char *name, char **msg)
 {
   int r = 0;
   smartlist_t *sl;
+  int changes = 0;
 
-  if (!lst)
+  if (!*lst)
     return 0;
   sl = smartlist_new();
 
-  smartlist_split_string(sl, lst, ",",
+  smartlist_split_string(sl, *lst, ",",
     SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK|SPLIT_STRIP_SPACE, 0);
 
   SMARTLIST_FOREACH(sl, const char *, s,
     {
       if (!is_legal_nickname_or_hexdigest(s)) {
+        // check if first char is dollar
+        if (s[0] != '$') {
+          // Try again but with a dollar symbol prepended
+          char *prepended = tor_malloc(strlen(s)+2);
+          strcpy(prepended, "$");
+          strcat(prepended, 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;
+          }
+
+          // Still not valid, free and fallback to error message
+          tor_free(prepended);
+        }
+
         tor_asprintf(msg, "Invalid nickname '%s' in %s line", s, name);
         r = -1;
         break;
       }
     });
+
+  // Replace the caller's nickname list with a fixed one
+  if (changes && r == 0) {
+    const char *newNicknames = smartlist_join_strings(sl, ", ", 0, NULL);
+    tor_free(*lst);
+    *lst = newNicknames;
+  }
+
   SMARTLIST_FOREACH(sl, char *, s, tor_free(s));
   smartlist_free(sl);
+
   return r;
 }
 
diff --git a/src/test/test_config.c b/src/test/test_config.c
index 3848d352d..911913aa8 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -509,6 +509,41 @@ test_config_parse_transport_options_line(void *arg)
   }
 }
 
+// Tests if an options with MyFamily fingerprints missing '$' normalises
+// them correctly and also ensure it also works with multiple fingerprints
+static void
+test_config_fix_my_family(void *arg)
+{
+  char *err = NULL;
+  char *family = "$1111111111111111111111111111111111111111, "
+                 "1111111111111111111111111111111111111112, "
+                 "$1111111111111111111111111111111111111113";
+
+  or_options_t* options = options_new();
+  or_options_t* defaults = options_new();
+  options_init(options);
+  options_init(defaults);
+  options->MyFamily = tor_strdup(family);
+
+  options_validate(NULL, options, defaults, 0, &err) ;
+
+  if (err != NULL) {
+    test_fail_msg(err);
+  }
+
+  test_streq(options->MyFamily, "$1111111111111111111111111111111111111111, "
+                                "$1111111111111111111111111111111111111112, "
+                                "$1111111111111111111111111111111111111113");
+
+  done:
+    if (err != NULL) {
+      tor_free(err);
+    }
+
+    or_options_free(options);
+    or_options_free(defaults);
+}
+
 #define CONFIG_TEST(name, flags)                          \
   { #name, test_config_ ## name, flags, NULL, NULL }
 
@@ -518,6 +553,7 @@ struct testcase_t config_tests[] = {
   CONFIG_TEST(parse_transport_options_line, 0),
   CONFIG_TEST(check_or_create_data_subdir, TT_FORK),
   CONFIG_TEST(write_to_data_subdir, TT_FORK),
+  CONFIG_TEST(fix_my_family, 0),
   END_OF_TESTCASES
 };