May 2017 - tor-commits - lists.torproject.org

[tor/master] Remove old unused indentation from handle_get_current_consensus
by nickm＠torproject.org 04 May '17

04 May '17

commit e051c47e9823eb76706b8bf6107636120f22a8bd Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 28 13:18:32 2017 -0400 Remove old unused indentation from handle_get_current_consensus This commit removes a pair of meaningless braces, and changes whitespace only. --- src/or/directory.c | 212 ++++++++++++++++++++++++++--------------------------- 1 file changed, 105 insertions(+), 107 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 48bada2..b0c821d 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3488,129 +3488,127 @@ handle_get_current_consensus(dir_connection_t *conn, const time_t if_modified_since = args->if_modified_since; int clear_spool = 0; - { - /* v3 network status fetch. */ - long lifetime = NETWORKSTATUS_CACHE_LIFETIME; + /* v3 network status fetch. */ + long lifetime = NETWORKSTATUS_CACHE_LIFETIME; - networkstatus_t *v; - time_t now = time(NULL); - const char *want_fps = NULL; - char *flavor = NULL; - int flav = FLAV_NS; + networkstatus_t *v; + time_t now = time(NULL); + const char *want_fps = NULL; + char *flavor = NULL; + int flav = FLAV_NS; #define CONSENSUS_URL_PREFIX "/tor/status-vote/current/consensus/" #define CONSENSUS_FLAVORED_PREFIX "/tor/status-vote/current/consensus-" - /* figure out the flavor if any, and who we wanted to sign the thing */ - if (!strcmpstart(url, CONSENSUS_FLAVORED_PREFIX)) { - const char *f, *cp; - f = url + strlen(CONSENSUS_FLAVORED_PREFIX); - cp = strchr(f, '/'); - if (cp) { - want_fps = cp+1; - flavor = tor_strndup(f, cp-f); - } else { - flavor = tor_strdup(f); - } - flav = networkstatus_parse_flavor_name(flavor); - if (flav < 0) - flav = FLAV_NS; + /* figure out the flavor if any, and who we wanted to sign the thing */ + if (!strcmpstart(url, CONSENSUS_FLAVORED_PREFIX)) { + const char *f, *cp; + f = url + strlen(CONSENSUS_FLAVORED_PREFIX); + cp = strchr(f, '/'); + if (cp) { + want_fps = cp+1; + flavor = tor_strndup(f, cp-f); } else { - if (!strcmpstart(url, CONSENSUS_URL_PREFIX)) - want_fps = url+strlen(CONSENSUS_URL_PREFIX); + flavor = tor_strdup(f); } + flav = networkstatus_parse_flavor_name(flavor); + if (flav < 0) + flav = FLAV_NS; + } else { + if (!strcmpstart(url, CONSENSUS_URL_PREFIX)) + want_fps = url+strlen(CONSENSUS_URL_PREFIX); + } - v = networkstatus_get_latest_consensus_by_flavor(flav); + v = networkstatus_get_latest_consensus_by_flavor(flav); - if (v && !networkstatus_consensus_reasonably_live(v, now)) { - write_http_status_line(conn, 404, "Consensus is too old"); - warn_consensus_is_too_old(v, flavor, now); - geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND); - tor_free(flavor); - goto done; - } + if (v && !networkstatus_consensus_reasonably_live(v, now)) { + write_http_status_line(conn, 404, "Consensus is too old"); + warn_consensus_is_too_old(v, flavor, now); + geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND); + tor_free(flavor); + goto done; + } - if (v && want_fps && - !client_likes_consensus(v, want_fps)) { - write_http_status_line(conn, 404, "Consensus not signed by sufficient " - "number of requested authorities"); - geoip_note_ns_response(GEOIP_REJECT_NOT_ENOUGH_SIGS); - tor_free(flavor); - goto done; - } + if (v && want_fps && + !client_likes_consensus(v, want_fps)) { + write_http_status_line(conn, 404, "Consensus not signed by sufficient " + "number of requested authorities"); + geoip_note_ns_response(GEOIP_REJECT_NOT_ENOUGH_SIGS); + tor_free(flavor); + goto done; + } - conn->spool = smartlist_new(); - clear_spool = 1; - { - spooled_resource_t *spooled; - if (flavor) - spooled = spooled_resource_new(DIR_SPOOL_NETWORKSTATUS, - (uint8_t*)flavor, strlen(flavor)); - else - spooled = spooled_resource_new(DIR_SPOOL_NETWORKSTATUS, - NULL, 0); - tor_free(flavor); - smartlist_add(conn->spool, spooled); - } - lifetime = (v && v->fresh_until > now) ? v->fresh_until - now : 0; + conn->spool = smartlist_new(); + clear_spool = 1; + { + spooled_resource_t *spooled; + if (flavor) + spooled = spooled_resource_new(DIR_SPOOL_NETWORKSTATUS, + (uint8_t*)flavor, strlen(flavor)); + else + spooled = spooled_resource_new(DIR_SPOOL_NETWORKSTATUS, + NULL, 0); + tor_free(flavor); + smartlist_add(conn->spool, spooled); + } + lifetime = (v && v->fresh_until > now) ? v->fresh_until - now : 0; - if (!smartlist_len(conn->spool)) { /* we failed to create/cache cp */ - write_http_status_line(conn, 503, "Network status object unavailable"); - geoip_note_ns_response(GEOIP_REJECT_UNAVAILABLE); - goto done; - } + if (!smartlist_len(conn->spool)) { /* we failed to create/cache cp */ + write_http_status_line(conn, 503, "Network status object unavailable"); + geoip_note_ns_response(GEOIP_REJECT_UNAVAILABLE); + goto done; + } - size_t size_guess = 0; - int n_expired = 0; - dirserv_spool_remove_missing_and_guess_size(conn, if_modified_since, - compressed, - &size_guess, - &n_expired); + size_t size_guess = 0; + int n_expired = 0; + dirserv_spool_remove_missing_and_guess_size(conn, if_modified_since, + compressed, + &size_guess, + &n_expired); - if (!smartlist_len(conn->spool) && !n_expired) { - write_http_status_line(conn, 404, "Not found"); - geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND); - goto done; - } else if (!smartlist_len(conn->spool)) { - write_http_status_line(conn, 304, "Not modified"); - geoip_note_ns_response(GEOIP_REJECT_NOT_MODIFIED); - goto done; - } + if (!smartlist_len(conn->spool) && !n_expired) { + write_http_status_line(conn, 404, "Not found"); + geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND); + goto done; + } else if (!smartlist_len(conn->spool)) { + write_http_status_line(conn, 304, "Not modified"); + geoip_note_ns_response(GEOIP_REJECT_NOT_MODIFIED); + goto done; + } - if (global_write_bucket_low(TO_CONN(conn), size_guess, 2)) { - log_debug(LD_DIRSERV, - "Client asked for network status lists, but we've been " - "writing too many bytes lately. Sending 503 Dir busy."); - write_http_status_line(conn, 503, "Directory busy, try again later"); - geoip_note_ns_response(GEOIP_REJECT_BUSY); - goto done; - } + if (global_write_bucket_low(TO_CONN(conn), size_guess, 2)) { + log_debug(LD_DIRSERV, + "Client asked for network status lists, but we've been " + "writing too many bytes lately. Sending 503 Dir busy."); + write_http_status_line(conn, 503, "Directory busy, try again later"); + geoip_note_ns_response(GEOIP_REJECT_BUSY); + goto done; + } - tor_addr_t addr; - if (tor_addr_parse(&addr, (TO_CONN(conn))->address) >= 0) { - geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, - &addr, NULL, - time(NULL)); - geoip_note_ns_response(GEOIP_SUCCESS); - /* Note that a request for a network status has started, so that we - * can measure the download time later on. */ - if (conn->dirreq_id) - geoip_start_dirreq(conn->dirreq_id, size_guess, DIRREQ_TUNNELED); - else - geoip_start_dirreq(TO_CONN(conn)->global_identifier, size_guess, - DIRREQ_DIRECT); - } + tor_addr_t addr; + if (tor_addr_parse(&addr, (TO_CONN(conn))->address) >= 0) { + geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, + &addr, NULL, + time(NULL)); + geoip_note_ns_response(GEOIP_SUCCESS); + /* Note that a request for a network status has started, so that we + * can measure the download time later on. */ + if (conn->dirreq_id) + geoip_start_dirreq(conn->dirreq_id, size_guess, DIRREQ_TUNNELED); + else + geoip_start_dirreq(TO_CONN(conn)->global_identifier, size_guess, + DIRREQ_DIRECT); + } - clear_spool = 0; - write_http_response_header(conn, -1, compressed, - smartlist_len(conn->spool) == 1 ? lifetime : 0); - if (! compressed) - conn->compress_state = tor_compress_new(0, ZLIB_METHOD, - HIGH_COMPRESSION); + clear_spool = 0; + write_http_response_header(conn, -1, compressed, + smartlist_len(conn->spool) == 1 ? lifetime : 0); + if (! compressed) + conn->compress_state = tor_compress_new(0, ZLIB_METHOD, + HIGH_COMPRESSION); - /* Prime the connection with some data. */ - const int initial_flush_result = connection_dirserv_flushed_some(conn); - tor_assert_nonfatal(initial_flush_result == 0); - goto done; - } + /* Prime the connection with some data. */ + const int initial_flush_result = connection_dirserv_flushed_some(conn); + tor_assert_nonfatal(initial_flush_result == 0); + goto done; done: if (clear_spool) {

1 0

[tor/master] Extract the consensus-only part of directory_get_from_dirserver
by nickm＠torproject.org 04 May '17

04 May '17

commit afa39cef6c6f42cca3f2b08462f68bece8db7312 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 3 14:19:46 2017 -0400 Extract the consensus-only part of directory_get_from_dirserver Right now it just sets an if-modified-since header, but it's about to get even bigger. This patch avoids changing indentation; the next patch will be whitespace fixes. --- src/or/directory.c | 64 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 8f934bd..f430ddf 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -477,33 +477,16 @@ directory_pick_generic_dirserver(dirinfo_type_t type, int pds_flags, return rs; } -/** Start a connection to a random running directory server, using - * connection purpose dir_purpose, intending to fetch descriptors - * of purpose router_purpose, and requesting resource. - * Use pds_flags as arguments to router_pick_directory_server() - * or router_pick_trusteddirserver(). +/** + * Set the extra fields in req that are used when requesting a + * consensus of type resource. */ -MOCK_IMPL(void, directory_get_from_dirserver, ( - uint8_t dir_purpose, - uint8_t router_purpose, - const char *resource, - int pds_flags, - download_want_authority_t want_authority)) +static void +dir_consensus_request_set_additional_headers(directory_request_t *req, + const char *resource) { - const routerstatus_t *rs = NULL; - const or_options_t *options = get_options(); - int prefer_authority = (directory_fetches_from_authorities(options) - || want_authority == DL_WANT_AUTHORITY); - int require_authority = 0; - int get_via_tor = purpose_needs_anonymity(dir_purpose, router_purpose, - resource); - dirinfo_type_t type = dir_fetch_type(dir_purpose, router_purpose, resource); time_t if_modified_since = 0; - - if (type == NO_DIRINFO) - return; - - if (dir_purpose == DIR_PURPOSE_FETCH_CONSENSUS) { + { int flav = FLAV_NS; networkstatus_t *v; if (resource) @@ -538,6 +521,33 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( if_modified_since = cd->published + DEFAULT_IF_MODIFIED_SINCE_DELAY; } } + directory_request_set_if_modified_since(req, if_modified_since); +} + +/** Start a connection to a random running directory server, using + * connection purpose dir_purpose, intending to fetch descriptors + * of purpose router_purpose, and requesting resource. + * Use pds_flags as arguments to router_pick_directory_server() + * or router_pick_trusteddirserver(). + */ +MOCK_IMPL(void, directory_get_from_dirserver, ( + uint8_t dir_purpose, + uint8_t router_purpose, + const char *resource, + int pds_flags, + download_want_authority_t want_authority)) +{ + const routerstatus_t *rs = NULL; + const or_options_t *options = get_options(); + int prefer_authority = (directory_fetches_from_authorities(options) + || want_authority == DL_WANT_AUTHORITY); + int require_authority = 0; + int get_via_tor = purpose_needs_anonymity(dir_purpose, router_purpose, + resource); + dirinfo_type_t type = dir_fetch_type(dir_purpose, router_purpose, resource); + + if (type == NO_DIRINFO) + return; if (!options->FetchServerDescriptors) return; @@ -566,7 +576,8 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( ri->cache_info.identity_digest); directory_request_set_router_purpose(req, router_purpose); directory_request_set_resource(req, resource); - directory_request_set_if_modified_since(req, if_modified_since); + if (dir_purpose == DIR_PURPOSE_FETCH_CONSENSUS) + dir_consensus_request_set_additional_headers(req, resource); directory_request_set_guard_state(req, guard_state); directory_initiate_request(req); directory_request_free(req); @@ -634,7 +645,8 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( directory_request_set_router_purpose(req, router_purpose); directory_request_set_indirection(req, indirection); directory_request_set_resource(req, resource); - directory_request_set_if_modified_since(req, if_modified_since); + if (dir_purpose == DIR_PURPOSE_FETCH_CONSENSUS) + dir_consensus_request_set_additional_headers(req, resource); if (guard_state) directory_request_set_guard_state(req, guard_state); directory_initiate_request(req);

1 0

[tor/master] Grammar fix in a log message
by nickm＠torproject.org 04 May '17

04 May '17

commit c6fe65fcafa901a629170c425da4099311a84279 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 4 08:58:06 2017 -0400 Grammar fix in a log message --- src/or/directory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index d31dd2f..8b63e4d 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2468,8 +2468,8 @@ handle_response_fetch_consensus(dir_connection_t *conn, networkstatus_consensus_download_failed(0, flavname); return -1; } - log_info(LD_DIR, "Applied consensus diff (body size %d) from server " - "'%s:%d' resulted in a new consensus document (size %d).", + log_info(LD_DIR, "Applied consensus diff (size %d) from server " + "'%s:%d', resulting in a new consensus document (size %d).", (int)body_len, conn->base_.address, conn->base_.port, (int)strlen(new_consensus)); consensus = new_consensus;

1 0

[tor/master] Merge branch 'prop140_complete_rebased'
by nickm＠torproject.org 04 May '17

04 May '17

commit a61020ebd45e81dec36bcfad460953e1920a11f9 Merge: 24ba186 c6fe65f Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 4 08:58:28 2017 -0400 Merge branch 'prop140_complete_rebased' changes/prop140 | 9 + src/or/consdiff.c | 9 + src/or/consdiff.h | 2 + src/or/consdiffmgr.c | 9 +- src/or/directory.c | 536 ++++++++++++++++++++++++++++------------- src/or/directory.h | 4 +- src/or/dirserv.c | 67 +++++- src/or/dirserv.h | 15 +- src/or/networkstatus.c | 72 ++++-- src/or/networkstatus.h | 1 + src/or/or.h | 5 + src/or/routerparse.c | 5 +- src/test/test_dir_handle_get.c | 4 + 13 files changed, 536 insertions(+), 202 deletions(-)

1 0

[tor/master] Do not BUG on missing sha3-as-signed field
by nickm＠torproject.org 04 May '17

04 May '17

commit 6beb7028d839f26d31f4743a9a924b8ed7550a54 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 4 08:50:21 2017 -0400 Do not BUG on missing sha3-as-signed field This can happen if you've been running an earlier alpha on your relay. Instead, just ignore the entry. --- src/or/consdiffmgr.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 96b0bba..d478101 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -858,9 +858,12 @@ consdiffmgr_rescan_flavor_(consensus_flavor_t flavor) continue; // LCOV_EXCL_LINE uint8_t this_sha3[DIGEST256_LEN]; - if (BUG(cdm_entry_get_sha3_value(this_sha3, c, - LABEL_SHA3_DIGEST_AS_SIGNED)<0)) - continue; // LCOV_EXCL_LINE + if (cdm_entry_get_sha3_value(this_sha3, c, + LABEL_SHA3_DIGEST_AS_SIGNED)<0) { + // Not actually a bug, since we might be running with a directory + // with stale files from before the #22143 fixes. + continue; + } if (cdm_diff_ht_check_and_note_pending(flavor, this_sha3, most_recent_sha3)) { // This is already pending, or we encountered an error.

1 0

[tor/master] Fix: our directory.c code expects header constants to end with a :
by nickm＠torproject.org 04 May '17

04 May '17

commit baf489fc08c5f90b8ee0f00c15b1f6bb91c38926 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 4 08:57:34 2017 -0400 Fix: our directory.c code expects header constants to end with a : --- src/or/directory.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 917dfee..d31dd2f 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -131,7 +131,7 @@ static void directory_request_set_guard_state(directory_request_t *req, #define ALLOW_DIRECTORY_TIME_SKEW (30*60) #define X_ADDRESS_HEADER "X-Your-Address-Is: " -#define X_OR_DIFF_FROM_CONSENSUS_HEADER "X-Or-Diff-From-Consensus" +#define X_OR_DIFF_FROM_CONSENSUS_HEADER "X-Or-Diff-From-Consensus: " /** HTTP cache control: how long do we tell proxies they can cache each * kind of document we serve? */ @@ -1226,6 +1226,9 @@ directory_request_set_if_modified_since(directory_request_t *req, /** Include a header of name key with content val in the * request. Neither may include newlines or other odd characters. Their * ordering is not currently guaranteed. + * + * Note that, as elsewhere in this module, header keys include a trailing + * colon and space. */ void directory_request_add_header(directory_request_t *req, @@ -1724,7 +1727,7 @@ directory_send_command(dir_connection_t *conn, { config_line_t *h; for (h = req->additional_headers; h; h = h->next) { - smartlist_add_asprintf(headers, "%s: %s\r\n", h->key, h->value); + smartlist_add_asprintf(headers, "%s%s\r\n", h->key, h->value); } } @@ -3508,7 +3511,7 @@ directory_handle_command_get,(dir_connection_t *conn, const char *headers, url_len -= 2; } - if ((header = http_get_header(headers, "Accept-Encoding"))) { + if ((header = http_get_header(headers, "Accept-Encoding: "))) { compression_methods_supported = parse_accept_encoding_header(header); tor_free(header); } else {

1 0

[tor/master] Add a test for $ with non-delete commands.
by nickm＠torproject.org 04 May '17

04 May '17

commit df2bcaeb4ac5c8944212ed6f73b48bb026ac7cbb Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 4 08:20:46 2017 -0400 Add a test for $ with non-delete commands. --- src/test/test_consdiff.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c index cdc1e6d..23948d6 100644 --- a/src/test/test_consdiff.c +++ b/src/test/test_consdiff.c @@ -709,6 +709,16 @@ test_consdiff_apply_ed_diff(void *arg) smartlist_clear(diff); + /* $ for a non-delete command. */ + smartlist_add_linecpy(diff, area, "1,$c"); + mock_clean_saved_logs(); + cons2 = apply_ed_diff(cons1, diff, 0); + tt_ptr_op(NULL, OP_EQ, cons2); + expect_single_log_msg_containing("it wanted to use $ with a command " + "other than delete"); + + smartlist_clear(diff); + /* Script is not in reverse order. */ smartlist_add_linecpy(diff, area, "1d"); smartlist_add_linecpy(diff, area, "3d");

1 0

[tor/master] bug#22143/prop#140: Use <n>, $d commands in diffs to remove signatures
by nickm＠torproject.org 04 May '17

04 May '17

commit c8baa9b78362e372cfb5e2d3fcc6100d20d00229 Author: Nick Mathewson <nickm(a)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 lst 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 lst 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 cons, 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 cons (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 + * 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. */

1 0

[tor/master] Remove some unused digests from test_consdiffmgr.c
by nickm＠torproject.org 04 May '17

04 May '17

commit 87f6979a03fa8d539fc81b67462f2f6cfc7f2960 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 3 13:11:11 2017 -0400 Remove some unused digests from test_consdiffmgr.c These were unused before I started working on #22143 -- I just found them while I was lookinging for digests to update. --- src/test/test_consdiffmgr.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 0e44ed2..9d0c5b5 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -428,7 +428,6 @@ test_consdiffmgr_diff_rules(void *arg) #define N 6 char *md_body[N], *ns_body[N]; networkstatus_t *md_ns[N], *ns_ns[N]; - uint8_t md_ns_sha3[N][DIGEST256_LEN], ns_ns_sha3[N][DIGEST256_LEN]; int i; MOCK(cpuworker_queue_work, mock_cpuworker_queue_work); @@ -441,10 +440,6 @@ test_consdiffmgr_diff_rules(void *arg) ns_body[i] = fake_ns_body_new(FLAV_NS, when); md_ns[i] = fake_ns_new(FLAV_MICRODESC, when); ns_ns[i] = fake_ns_new(FLAV_NS, when); - crypto_digest256((char *)md_ns_sha3[i], md_body[i], strlen(md_body[i]), - DIGEST_SHA3_256); - crypto_digest256((char *)ns_ns_sha3[i], ns_body[i], strlen(ns_body[i]), - DIGEST_SHA3_256); } /* For the MD consensuses: add 4 of them, and make sure that @@ -724,7 +719,6 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) #define N 4 char *md_body[N]; networkstatus_t *md_ns[N]; - uint8_t md_ns_sha3[N][DIGEST256_LEN]; int i; consensus_cache_entry_t *hold_ent = NULL, *ent; @@ -739,8 +733,6 @@ test_consdiffmgr_cleanup_old_diffs(void *arg) time_t when = start + i * 15; md_body[i] = fake_ns_body_new(FLAV_MICRODESC, when); md_ns[i] = fake_ns_new(FLAV_MICRODESC, when); - crypto_digest256((char *)md_ns_sha3[i], md_body[i], strlen(md_body[i]), - DIGEST_SHA3_256); } /* add the first 3. */

1 0

[tor/master] bug#22143/prop#140: identify input diffs by their digest-as-signed
by nickm＠torproject.org 04 May '17

04 May '17

commit 5acddbbbf74199b46eef75cbbeb11ea6aad9326f Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 3 11:48:08 2017 -0400 bug#22143/prop#140: identify input diffs by their digest-as-signed See may 3 changes to prop140 for more background. --- src/or/consdiff.c | 16 +++++++++++-- src/or/consdiff.h | 3 +++ src/or/routerparse.c | 58 ++++++++++++++++++++++++++++++++++++++++------- src/or/routerparse.h | 5 ++++ src/test/fuzz/fuzz_diff.c | 2 ++ src/test/test_consdiff.c | 6 ++--- 6 files changed, 77 insertions(+), 13 deletions(-) diff --git a/src/or/consdiff.c b/src/or/consdiff.c index c0fe979..e4f7872 100644 --- a/src/or/consdiff.c +++ b/src/or/consdiff.c @@ -91,6 +91,18 @@ consensus_compute_digest,(const char *cons, return r; } +/** Compute the digest-as-signed of cons, and store the result in + * digest_out. Return 0 on success, -1 on failure. */ +/* This is a separate, mockable function so that we can override it when + * fuzzing. */ +MOCK_IMPL(STATIC int, +consensus_compute_digest_as_signed,(const char *cons, + consensus_digest_t *digest_out)) +{ + return router_get_networkstatus_v3_sha3_as_signed(digest_out->sha3_256, + cons); +} + /** Return true iff d1 and d2 contain the same digest */ /* This is a separate, mockable function so that we can override it when * fuzzing. */ @@ -1250,7 +1262,7 @@ consensus_diff_generate(const char *cons1, int r1, r2; char *result = NULL; - r1 = consensus_compute_digest(cons1, &d1); + r1 = consensus_compute_digest_as_signed(cons1, &d1); r2 = consensus_compute_digest(cons2, &d2); if (BUG(r1 < 0 || r2 < 0)) return NULL; // LCOV_EXCL_LINE @@ -1291,7 +1303,7 @@ consensus_diff_apply(const char *consensus, char *result = NULL; memarea_t *area = memarea_new(); - r1 = consensus_compute_digest(consensus, &d1); + r1 = consensus_compute_digest_as_signed(consensus, &d1); if (BUG(r1 < 0)) return NULL; // LCOV_EXCL_LINE diff --git a/src/or/consdiff.h b/src/or/consdiff.h index e9d1751..284a576 100644 --- a/src/or/consdiff.h +++ b/src/or/consdiff.h @@ -85,6 +85,9 @@ MOCK_DECL(STATIC int, consensus_compute_digest,(const char *cons, consensus_digest_t *digest_out)); MOCK_DECL(STATIC int, + consensus_compute_digest_as_signed,(const char *cons, + consensus_digest_t *digest_out)); +MOCK_DECL(STATIC int, consensus_digest_eq,(const uint8_t *d1, const uint8_t *d2)); #endif diff --git a/src/or/routerparse.c b/src/or/routerparse.c index fc0a4ab..f39c332 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -359,6 +359,7 @@ static addr_policy_t *router_parse_addr_policy_private(directory_token_t *tok); static int router_get_hash_impl_helper(const char *s, size_t s_len, const char *start_str, const char *end_str, char end_c, + int log_severity, const char **start_out, const char **end_out); static int router_get_hash_impl(const char *s, size_t s_len, char *digest, const char *start_str, const char *end_str, @@ -988,6 +989,41 @@ router_get_router_hash(const char *s, size_t s_len, char *digest) DIGEST_SHA1); } +/** Try to find the start and end of the signed portion of a networkstatus + * document in s. On success, set start_out to the first + * character of the document, and end_out to a position one after the + * final character of the signed document, and return 0. On failure, return + * -1. */ +int +router_get_networkstatus_v3_signed_boundaries(const char *s, + const char **start_out, + const char **end_out) +{ + return router_get_hash_impl_helper(s, strlen(s), + "network-status-version", + "\ndirectory-signature", + ' ', LOG_INFO, + start_out, end_out); +} + +/** Set digest_out to the SHA3-256 digest of the signed portion of the + * networkstatus vote in s -- or of the entirety of s if no + * signed portion can be identified. Return 0 on success, -1 on failure. */ +int +router_get_networkstatus_v3_sha3_as_signed(uint8_t *digest_out, + const char *s) +{ + const char *start, *end; + if (router_get_networkstatus_v3_signed_boundaries(s, &start, &end) < 0) { + start = s; + end = s + strlen(s); + } + tor_assert(start); + tor_assert(end); + return crypto_digest256((char*)digest_out, start, end-start, + DIGEST_SHA3_256); +} + /** Set digests to all the digests of the consensus document in * s */ int @@ -1787,7 +1823,8 @@ router_parse_entry_from_string(const char *s, const char *end, if (router_get_hash_impl_helper(s, end-s, "router ", "\nrouter-sig-ed25519", - ' ', &signed_start, &signed_end) < 0) { + ' ', LOG_WARN, + &signed_start, &signed_end) < 0) { log_warn(LD_DIR, "Can't find ed25519-signed portion of descriptor"); goto err; } @@ -2140,7 +2177,8 @@ extrainfo_parse_entry_from_string(const char *s, const char *end, if (router_get_hash_impl_helper(s, end-s, "extra-info ", "\nrouter-sig-ed25519", - ' ', &signed_start, &signed_end) < 0) { + ' ', LOG_WARN, + &signed_start, &signed_end) < 0) { log_warn(LD_DIR, "Can't find ed25519-signed portion of extrainfo"); goto err; } @@ -4471,16 +4509,18 @@ static int router_get_hash_impl_helper(const char *s, size_t s_len, const char *start_str, const char *end_str, char end_c, + int log_severity, const char **start_out, const char **end_out) { const char *start, *end; start = tor_memstr(s, s_len, start_str); if (!start) { - log_warn(LD_DIR,"couldn't find start of hashed material \"%s\"",start_str); + log_fn(log_severity,LD_DIR, + "couldn't find start of hashed material \"%s\"",start_str); return -1; } if (start != s && *(start-1) != '\n') { - log_warn(LD_DIR, + log_fn(log_severity,LD_DIR, "first occurrence of \"%s\" is not at the start of a line", start_str); return -1; @@ -4488,12 +4528,14 @@ router_get_hash_impl_helper(const char *s, size_t s_len, end = tor_memstr(start+strlen(start_str), s_len - (start-s) - strlen(start_str), end_str); if (!end) { - log_warn(LD_DIR,"couldn't find end of hashed material \"%s\"",end_str); + log_fn(log_severity,LD_DIR, + "couldn't find end of hashed material \"%s\"",end_str); return -1; } end = memchr(end+strlen(end_str), end_c, s_len - (end-s) - strlen(end_str)); if (!end) { - log_warn(LD_DIR,"couldn't find EOL"); + log_fn(log_severity,LD_DIR, + "couldn't find EOL"); return -1; } ++end; @@ -4517,7 +4559,7 @@ router_get_hash_impl(const char *s, size_t s_len, char *digest, digest_algorithm_t alg) { const char *start=NULL, *end=NULL; - if (router_get_hash_impl_helper(s,s_len,start_str,end_str,end_c, + if (router_get_hash_impl_helper(s,s_len,start_str,end_str,end_c,LOG_WARN, &start,&end)<0) return -1; @@ -4554,7 +4596,7 @@ router_get_hashes_impl(const char *s, size_t s_len, common_digests_t *digests, const char *end_str, char end_c) { const char *start=NULL, *end=NULL; - if (router_get_hash_impl_helper(s,s_len,start_str,end_str,end_c, + if (router_get_hash_impl_helper(s,s_len,start_str,end_str,end_c,LOG_WARN, &start,&end)<0) return -1; diff --git a/src/or/routerparse.h b/src/or/routerparse.h index e8d71b6..088f773 100644 --- a/src/or/routerparse.h +++ b/src/or/routerparse.h @@ -16,6 +16,11 @@ int router_get_router_hash(const char *s, size_t s_len, char *digest); int router_get_dir_hash(const char *s, char *digest); int router_get_networkstatus_v3_hashes(const char *s, common_digests_t *digests); +int router_get_networkstatus_v3_signed_boundaries(const char *s, + const char **start_out, + const char **end_out); +int router_get_networkstatus_v3_sha3_as_signed(uint8_t *digest_out, + const char *s); int router_get_extrainfo_hash(const char *s, size_t s_len, char *digest); #define DIROBJ_MAX_SIG_LEN 256 char *router_get_dirobj_signature(const char *digest, diff --git a/src/test/fuzz/fuzz_diff.c b/src/test/fuzz/fuzz_diff.c index c241f63..642380b 100644 --- a/src/test/fuzz/fuzz_diff.c +++ b/src/test/fuzz/fuzz_diff.c @@ -21,6 +21,7 @@ int fuzz_init(void) { MOCK(consensus_compute_digest, mock_consensus_compute_digest_); + MOCK(consensus_compute_digest_as_signed, mock_consensus_compute_digest_); return 0; } @@ -28,6 +29,7 @@ int fuzz_cleanup(void) { UNMOCK(consensus_compute_digest); + UNMOCK(consensus_compute_digest_as_signed); return 0; } diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c index 2f826b5..2e78672 100644 --- a/src/test/test_consdiff.c +++ b/src/test/test_consdiff.c @@ -907,7 +907,7 @@ test_consdiff_gen_diff(void *arg) ); tt_int_op(0, OP_EQ, - consensus_compute_digest(cons1_str, &digests1)); + consensus_compute_digest_as_signed(cons1_str, &digests1)); tt_int_op(0, OP_EQ, consensus_compute_digest(cons2_str, &digests2)); @@ -926,7 +926,7 @@ test_consdiff_gen_diff(void *arg) "directory-signature foo bar\nbar\n" ); tt_int_op(0, OP_EQ, - consensus_compute_digest(cons1_str, &digests1)); + consensus_compute_digest_as_signed(cons1_str, &digests1)); smartlist_clear(cons1); consensus_split_lines(cons1, cons1_str, area); diff = consdiff_gen_diff(cons1, cons2, &digests1, &digests2, area); @@ -935,7 +935,7 @@ test_consdiff_gen_diff(void *arg) tt_assert(line_str_eq(smartlist_get(diff, 0), "network-status-diff-version 1")); tt_assert(line_str_eq(smartlist_get(diff, 1), "hash " - "06646D6CF563A41869D3B02E73254372AE3140046C5E7D83C9F71E54976AF9B4 " + "95D70F5A3CC65F920AA8B44C4563D7781A082674329661884E19E94B79D539C2 " "7AFECEFA4599BA33D603653E3D2368F648DF4AC4723929B0F7CF39281596B0C1")); tt_assert(line_str_eq(smartlist_get(diff, 2), "3,4d")); tt_assert(line_str_eq(smartlist_get(diff, 3), "1a"));

1 0