commit c8baa9b78362e372cfb5e2d3fcc6100d20d00229 Author: Nick Mathewson nickm@torproject.org Date: Wed May 3 12:34:30 2017 -0400

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 | 91 ++++++++++++++++++++++++++++++++++++++++++++---- src/test/test_consdiff.c | 18 ++++++---- 2 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/src/or/consdiff.c b/src/or/consdiff.c index e4f7872..3c2140b 100644 --- a/src/or/consdiff.c +++ b/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 " diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c index 2e78672..cdc1e6d 100644 --- a/src/test/test_consdiff.c +++ b/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. */