bug#22143/prop#140: Use <n>,$d commands in diffs to remove signatures

In this patch I add support for "delete through end of file" in our
ed diff handler, and generate our diffs so that they remove
everything after in the consensus after the signatures begin.

src/or/consdiff.c

@@ -65,17 +65,35 @@ line_str_eq(const cdline_t *a, const char *b)
   return lines_eq(a, &bline);
 }
 
-/** Add a cdline_t to <b>lst</b> holding as its contents the nul-terminated
- * string s.  Use the provided memory area for storage. */
-STATIC void
-smartlist_add_linecpy(smartlist_t *lst, memarea_t *area, const char *s)
+/** Return true iff a begins with the same contents as the nul-terminated
+ * string b. */
+static int
+line_starts_with_str(const cdline_t *a, const char *b)
+{
+  const size_t len = strlen(b);
+  tor_assert(len <= UINT32_MAX);
+  return a->len >= len && fast_memeq(a->s, b, len);
+}
+
+/** Return a new cdline_t holding as its contents the nul-terminated
+ * string s. Use the provided memory area for storage. */
+static cdline_t *
+cdline_linecpy(memarea_t *area, const char *s)
 {
   size_t len = strlen(s);
   const char *ss = memarea_memdup(area, s, len);
   cdline_t *line = memarea_alloc(area, sizeof(cdline_t));
   line->s = ss;
   line->len = (uint32_t)len;
-  smartlist_add(lst, line);
+  return line;
+}
+
+/** Add a cdline_t to <b>lst</b> holding as its contents the nul-terminated
+ * string s.  Use the provided memory area for storage. */
+STATIC void
+smartlist_add_linecpy(smartlist_t *lst, memarea_t *area, const char *s)
+{
+  smartlist_add(lst, cdline_linecpy(area, s));
 }
 
 /** Compute the digest of <b>cons</b>, and store the result in
@@ -545,6 +563,42 @@ find_next_router_line(const smartlist_t *cons,
   return 0;
 }
 
+/** Line-prefix indicating the beginning of the signatures section that we
+ * intend to delete. */
+#define START_OF_SIGNATURES_SECTION "directory-signature "
+
+/** Pre-process a consensus in <b>cons</b> (represented as a list of cdline_t)
+ * to remove the signatures from it.  If the footer is removed, return a
+ * cdline_t containing a delete command to delete the footer, allocated in
+ * <b>area</>.  If no footer is removed, return NULL.
+ *
+ * We remove the signatures here because they are not themselves signed, and
+ * as such there might be different encodings for them.
+ */
+static cdline_t *
+preprocess_consensus(memarea_t *area,
+                     smartlist_t *cons)
+{
+  int idx;
+  int dirsig_idx = -1;
+  for (idx = 0; idx < smartlist_len(cons); ++idx) {
+    cdline_t *line = smartlist_get(cons, idx);
+    if (line_starts_with_str(line, START_OF_SIGNATURES_SECTION)) {
+      dirsig_idx = idx;
+      break;
+    }
+  }
+  if (dirsig_idx >= 0) {
+    char buf[64];
+    while (smartlist_len(cons) > dirsig_idx)
+      smartlist_del(cons, dirsig_idx);
+    tor_snprintf(buf, sizeof(buf), "%d,$d", dirsig_idx+1);
+    return cdline_linecpy(area, buf);
+  } else {
+    return NULL;
+  }
+}
+
 /** Generate an ed diff as a smartlist from two consensuses, also given as
  * smartlists. Will return NULL if the diff could not be generated, which can
  * happen if any lines the script had to add matched "." or if the routers
@@ -566,13 +620,22 @@ find_next_router_line(const smartlist_t *cons,
  *   calc_changes(cons1_sl, cons2_sl, changed1, changed2);
  */
 STATIC smartlist_t *
-gen_ed_diff(const smartlist_t *cons1, const smartlist_t *cons2,
+gen_ed_diff(const smartlist_t *cons1_orig, const smartlist_t *cons2,
             memarea_t *area)
 {
+  smartlist_t *cons1 = smartlist_new();
+  smartlist_add_all(cons1, cons1_orig);
+  cdline_t *remove_trailer = preprocess_consensus(area, cons1);
+
   int len1 = smartlist_len(cons1);
   int len2 = smartlist_len(cons2);
   smartlist_t *result = smartlist_new();
 
+  if (remove_trailer) {
+    /* There's a delete-the-trailer line at the end, so add it here. */
+    smartlist_add(result, remove_trailer);
+  }
+
   /* Initialize the changed bitarrays to zero, so that calc_changes only needs
    * to set the ones that matter and leave the rest untouched.
    */
@@ -746,6 +809,7 @@ gen_ed_diff(const smartlist_t *cons1, const smartlist_t *cons2,
     }
   }
 
+  smartlist_free(cons1);
   bitarray_free(changed1);
   bitarray_free(changed2);
 
@@ -753,6 +817,7 @@ gen_ed_diff(const smartlist_t *cons1, const smartlist_t *cons2,
 
  error_cleanup:
 
+  smartlist_free(cons1);
   bitarray_free(changed1);
   bitarray_free(changed2);
 
@@ -811,6 +876,7 @@ apply_ed_diff(const smartlist_t *cons1, const smartlist_t *diff,
     const char *ptr = diff_line;
     int start = 0, end = 0;
     int had_range = 0;
+    int end_was_eof = 0;
     if (get_linenum(&ptr, &start) < 0) {
       log_warn(LD_CONSDIFF, "Could not apply consensus diff because "
                "an ed command was missing a line number.");
@@ -820,7 +886,11 @@ apply_ed_diff(const smartlist_t *cons1, const smartlist_t *diff,
       /* Two-item range */
       had_range = 1;
       ++ptr;
-      if (get_linenum(&ptr, &end) < 0) {
+      if (*ptr == '$') {
+        end_was_eof = 1;
+        end = smartlist_len(cons1);
+        ++ptr;
+      } else if (get_linenum(&ptr, &end) < 0) {
         log_warn(LD_CONSDIFF, "Could not apply consensus diff because "
                  "an ed command was missing a range end line number.");
         goto error_cleanup;
@@ -867,6 +937,13 @@ apply_ed_diff(const smartlist_t *cons1, const smartlist_t *diff,
         goto error_cleanup;
     }
 
+    /** $ is not allowed with non-d actions. */
+    if (end_was_eof && action != 'd') {
+      log_warn(LD_CONSDIFF, "Could not apply consensus diff because "
+               "it wanted to use $ with a command other than delete");
+      goto error_cleanup;
+    }
+
     /* 'a' commands are not allowed to have ranges. */
     if (had_range && action == 'a') {
       log_warn(LD_CONSDIFF, "Could not apply consensus diff because "

src/test/test_consdiff.c

@@ -931,18 +931,24 @@ test_consdiff_gen_diff(void *arg)
   consensus_split_lines(cons1, cons1_str, area);
   diff = consdiff_gen_diff(cons1, cons2, &digests1, &digests2, area);
   tt_ptr_op(NULL, OP_NE, diff);
-  tt_int_op(7, OP_EQ, smartlist_len(diff));
+  tt_int_op(11, OP_EQ, smartlist_len(diff));
   tt_assert(line_str_eq(smartlist_get(diff, 0),
                         "network-status-diff-version 1"));
   tt_assert(line_str_eq(smartlist_get(diff, 1), "hash "
       "95D70F5A3CC65F920AA8B44C4563D7781A082674329661884E19E94B79D539C2 "
       "7AFECEFA4599BA33D603653E3D2368F648DF4AC4723929B0F7CF39281596B0C1"));
-  tt_assert(line_str_eq(smartlist_get(diff, 2), "3,4d"));
-  tt_assert(line_str_eq(smartlist_get(diff, 3), "1a"));
-  tt_assert(line_str_eq(smartlist_get(diff, 4),
+  tt_assert(line_str_eq(smartlist_get(diff, 2), "6,$d"));
+  tt_assert(line_str_eq(smartlist_get(diff, 3), "3,4c"));
+  tt_assert(line_str_eq(smartlist_get(diff, 4), "bar"));
+  tt_assert(line_str_eq(smartlist_get(diff, 5),
+                        "directory-signature foo bar"));
+  tt_assert(line_str_eq(smartlist_get(diff, 6),
+                        "."));
+  tt_assert(line_str_eq(smartlist_get(diff, 7), "1a"));
+  tt_assert(line_str_eq(smartlist_get(diff, 8),
                         "r name aaaaaaaaaaaaaaaaa etc"));
-  tt_assert(line_str_eq(smartlist_get(diff, 5), "foo"));
-  tt_assert(line_str_eq(smartlist_get(diff, 6), "."));
+  tt_assert(line_str_eq(smartlist_get(diff, 9), "foo"));
+  tt_assert(line_str_eq(smartlist_get(diff, 10), "."));
 
   /* TODO: small real use-cases, i.e. consensuses. */