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

[tor/master] Merge branch 'ticket23220'
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit 5ec91a3d7f767ec593fcd4dc9c646010dbd9f215 Merge: a368cadf9 efbd4a33f Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 14:08:26 2017 -0400 Merge branch 'ticket23220' changes/bug23220 | 7 +++++++ src/or/routerlist.c | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-)

1 0

[tor/master] Raise MIN_DL_PER_REQUEST to 32
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit efbd4a33f4223bd3b7b3b0de307e9854e1d72e4e Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 13:54:01 2017 -0400 Raise MIN_DL_PER_REQUEST to 32 This change should improve overhead for downloading small numbers of descriptors and microdescriptors by improving compression performance and lowering directory request overhead. Closes ticket 23220. --- changes/bug23220 | 7 +++++++ src/or/routerlist.c | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changes/bug23220 b/changes/bug23220 new file mode 100644 index 000000000..9c2efc959 --- /dev/null +++ b/changes/bug23220 @@ -0,0 +1,7 @@ + o Minor features (client directory bandwidth tuning): + + - When downloading (micro)descriptors, don't split the list of + descriptors into multiple requests unless there are at least 32 + descriptors that we want. Previously, we split at 4, not 32, which + could lead to significant overhead in HTTP request size and + degradation in compression performance. Closes ticket 23220. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 49caa875f..989401947 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -4971,8 +4971,9 @@ max_dl_per_request(const or_options_t *options, int purpose) } /** Don't split our requests so finely that we are requesting fewer than - * this number per server. */ -#define MIN_DL_PER_REQUEST 4 + * this number per server. (Grouping more than this at once leads to + * diminishing returns.) */ +#define MIN_DL_PER_REQUEST 32 /** To prevent a single screwy cache from confusing us by selective reply, * try to split our requests into at least this many requests. */ #define MIN_REQUESTS 3

1 0

[torspec/master] Add a proposal about downloading many microdescriptors at once
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit c0fd32ccf20e0d1e92c9c363bf66267bf3f68a9b Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 13:35:54 2017 -0400 Add a proposal about downloading many microdescriptors at once --- proposals/000-index.txt | 2 + proposals/281-bulk-md-download.txt | 89 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/proposals/000-index.txt b/proposals/000-index.txt index 54d5bc4..18ff949 100644 --- a/proposals/000-index.txt +++ b/proposals/000-index.txt @@ -201,6 +201,7 @@ Proposals by number: 278 Directory Compression Scheme Negotiation [FINISHED] 279 A Name System API for Tor Onion Services [DRAFT] 280 Privacy-Preseving Statistics with Privcount in Tor [DRAFT] +281 Downloading microdescriptors in bulk [DRAFT] Proposals by status: @@ -225,6 +226,7 @@ Proposals by status: 273 Exit relay pinning for web services [for n/a] 279 A Name System API for Tor Onion Services 280 Privacy-Preseving Statistics with Privcount in Tor + 281 Downloading microdescriptors in bulk NEEDS-REVISION: 190 Bridge Client Authorization Based on a Shared Secret NEEDS-RESEARCH: diff --git a/proposals/281-bulk-md-download.txt b/proposals/281-bulk-md-download.txt new file mode 100644 index 0000000..bd02aef --- /dev/null +++ b/proposals/281-bulk-md-download.txt @@ -0,0 +1,89 @@ +Filename: 281-bulk-md-download.txt +Title: Downloading microdescriptors in bulk +Author: Nick Mathewson +Created: 11-Aug-2017 +Status: Draft + +1. Introduction + + This proposal describes a ways to download more microdescriptors + at a time, using fewer bytes. + + Right now, to download N microdescriptors, the client must send + about 44*N bytes in its HTTP request. Because clients can request + microdescriptors in any combination, the directory caches cannot + pre-compress responses to these requests, and need to use less + space-efficient on-the-fly compression algorithms. + + Under this proposal, clients simply say "Send me the + microdescriptors I need", given what I know. + +2. Combined microdescriptor downloads + +2.1. By diff + + If a client has a consensus with base64 sha3-256 digest X, and it + previously had a consensus with base64 sha3-256 digests Y then + it may request all the microdescriptors listed in X but not Y, + by asking for the resource: + /tor/micro/diff/X/Y + + Clients SHOULD only ask for this resource compressed. + + Caches MUST NOT answer this request unless they recognize the + consensus with digest X, and digest Y. + digest Y. If answering, caches MUST reply with all of the + microdescriptors that the cache holds that were listed by + consensus X, and MUST omit all the microdescriptors that were + omitted listed in consensus Y. + +2.2. By consensus: + + If a client has fewer than NMNM% of the microdescriptors listed in a + consensus X, it should fetch the resource + /tor/micro/full/X + + Clients SHOULD only ask for this resource compressed. + + Caches MUST NOT answer this request unless they recognize the + consensus with digest X. They should send all the microdescriptors + they have that are listed in that consensus. + +2.3. When to make these requests + + Clients should decide to use this format in preference to the + old download-by-digest format if the consensus X lists their + preferred directory cache as using a new DirCache subprotocol + version. (See 5 below.) + +3. Performance analysis + + This is a back-of-the-envelope analysis using a month's worth of + consensus documents, and a randomly chosen sample of + microdescriptors. + + + On average, about 0.5% of the microdescriptors change between any + two consensuses. Call it 50. That means 50*43 bytes == 2150 + bytes to request the microdescriptors. It means ~24530 bytes of + microdescriptors downloaded, compressed to ~13687 bytes by zstd. + + With this proposal, we're down to 86 bytes for the request, and we + can precompute the compressed output, making it save to use lzma2, + getting a compressed result more like 13362. + + It appears that this change would save about 15% for incremental + microdescriptor downloads, most of that coming from the reduction + in request size. + + For complete downloads, a complete set of microdescriptors is about + 7700 microdesciptors long. That makes the total number of bytes + for the requests 7700*43 == 331100 bytes. The response, if + compressed with lzma instead of zstd, would fall from 1659682 to + 1587804 bytes, for a total savings of 20%. + + +5. Compatibility + + Caches supporting this download protocol need to advertise + support of a new DirCache subprotocol version.

1 0

[tor/master] Add # to "hop N" messages to disambiguate from old messages.
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit c3a0cdeaab5aa4e4f60df7c6a798fd8c3a2f251d Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 12:11:27 2017 -0400 Add # to "hop N" messages to disambiguate from old messages. --- src/or/circuitbuild.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 48e4b75ff..0c57b0b25 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2111,7 +2111,7 @@ choose_good_middle_server(uint8_t purpose, tor_assert(CIRCUIT_PURPOSE_MIN_ <= purpose && purpose <= CIRCUIT_PURPOSE_MAX_); - log_debug(LD_CIRC, "Contemplating intermediate hop %d: random choice.", + log_debug(LD_CIRC, "Contemplating intermediate hop #%d: random choice.", cur_len+1); excluded = smartlist_new(); if ((r = build_state_get_exit_node(state))) { @@ -2262,12 +2262,12 @@ onion_extend_cpath(origin_circuit_t *circ) } if (!info) { - log_warn(LD_CIRC,"Failed to find node for hop %d of our path. Discarding " + log_warn(LD_CIRC,"Failed to find node for hop #%d of our path. Discarding " "this circuit.", cur_len+1); return -1; } - log_debug(LD_CIRC,"Chose router %s for hop %d (exit is %s)", + log_debug(LD_CIRC,"Chose router %s for hop #%d (exit is %s)", extend_info_describe(info), cur_len+1, build_state_get_exit_nickname(state));

1 0

[tor/master] Fix two log messages that mistakenly log 0-based hop counts
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit 35217f0f61c633efe1f6e5ecab18d1700b9d570b Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Sat May 7 21:03:31 2016 +1000 Fix two log messages that mistakenly log 0-based hop counts (torspec says hop counts are 1-based.) Closes ticket 18982, bugfix on 0275b6876 in tor 0.2.6.2-alpha and 907db008a in tor 0.2.4.5-alpha. Credit to Xiaofan Li for reporting this issue. --- changes/bug18982 | 6 ++++++ src/or/circuitbuild.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/changes/bug18982 b/changes/bug18982 new file mode 100644 index 000000000..bb0383d13 --- /dev/null +++ b/changes/bug18982 @@ -0,0 +1,6 @@ + o Minor bugfixes (circuit logging): + - torspec says hop counts are 1-based, so fix two log messages + that mistakenly logged 0-based hop counts. + Closes ticket 18982, bugfix on 0275b6876 in tor 0.2.6.2-alpha + and 907db008a in tor 0.2.4.5-alpha. Patch by teor. + Credit to Xiaofan Li for reporting this issue. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index e6fe3f0c3..48e4b75ff 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2112,7 +2112,7 @@ choose_good_middle_server(uint8_t purpose, purpose <= CIRCUIT_PURPOSE_MAX_); log_debug(LD_CIRC, "Contemplating intermediate hop %d: random choice.", - cur_len); + cur_len+1); excluded = smartlist_new(); if ((r = build_state_get_exit_node(state))) { nodelist_add_node_and_family(excluded, r); @@ -2263,7 +2263,7 @@ onion_extend_cpath(origin_circuit_t *circ) if (!info) { log_warn(LD_CIRC,"Failed to find node for hop %d of our path. Discarding " - "this circuit.", cur_len); + "this circuit.", cur_len+1); return -1; }

1 0

[tor/master] Merge branch 'bug18982'
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit a368cadf9514cf31deebed8613c4fd833b1448bf Merge: 6247f5a5b c3a0cdeaa Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 12:11:42 2017 -0400 Merge branch 'bug18982' changes/bug18982 | 6 ++++++ src/or/circuitbuild.c | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-)

1 0

[tor/maint-0.3.1] Use a single free-and-exit strategy in config_process_include.
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit 72832086e2144158308b30133aa15b39f0d2eb5d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 8 20:07:39 2017 -0400 Use a single free-and-exit strategy in config_process_include. This avoids a double-free when a pointer already freed with tor_free(config_line) is freed again in the cleanup-and-exit code. Fixes bug 23155. --- changes/bug23155 | 4 ++++ src/common/confline.c | 16 +++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/changes/bug23155 b/changes/bug23155 new file mode 100644 index 000000000..4c24ab136 --- /dev/null +++ b/changes/bug23155 @@ -0,0 +1,4 @@ + o Minor bugfixes (stability): + - Avoid crashing on double-free when unable to load or process + an included file. Fixes bug 23155; bugfix on 0.3.1.1-alpha. + Found with the clang static analyzer. diff --git a/src/common/confline.c b/src/common/confline.c index 691cbf8c6..15fd96bf3 100644 --- a/src/common/confline.c +++ b/src/common/confline.c @@ -294,24 +294,26 @@ config_process_include(const char *path, int recursion_level, int extended, return -1; } - SMARTLIST_FOREACH_BEGIN(config_files, char *, config_file) { + int rv = -1; + SMARTLIST_FOREACH_BEGIN(config_files, const char *, config_file) { config_line_t *included_list = NULL; if (config_get_included_list(config_file, recursion_level, extended, &included_list, list_last) < 0) { - SMARTLIST_FOREACH(config_files, char *, f, tor_free(f)); - smartlist_free(config_files); - return -1; + goto done; } - tor_free(config_file); *next = included_list; if (*list_last) next = &(*list_last)->next; } SMARTLIST_FOREACH_END(config_file); - smartlist_free(config_files); *list = ret_list; - return 0; + rv = 0; + + done: + SMARTLIST_FOREACH(config_files, char *, f, tor_free(f)); + smartlist_free(config_files); + return rv; } /**

1 0

[tor/master] Merge branch 'maint-0.3.1'
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit 6247f5a5ba6d300cd64a2639296c3514dcb70066 Merge: 133e1e870 72832086e Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 11:51:06 2017 -0400 Merge branch 'maint-0.3.1' changes/bug23155 | 4 ++++ src/common/confline.c | 16 +++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-)

1 0

[tor/release-0.3.1] Merge branch 'maint-0.3.1' into release-0.3.1
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit 4d5e8378be3848c07a9b1026924e879e5642903f Merge: 47e7a167d 72832086e Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 11 11:51:06 2017 -0400 Merge branch 'maint-0.3.1' into release-0.3.1 changes/bug23155 | 4 ++++ src/common/confline.c | 16 +++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-)

1 0

[tor/release-0.3.1] Use a single free-and-exit strategy in config_process_include.
by nickm＠torproject.org 11 Aug '17

11 Aug '17

commit 72832086e2144158308b30133aa15b39f0d2eb5d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 8 20:07:39 2017 -0400 Use a single free-and-exit strategy in config_process_include. This avoids a double-free when a pointer already freed with tor_free(config_line) is freed again in the cleanup-and-exit code. Fixes bug 23155. --- changes/bug23155 | 4 ++++ src/common/confline.c | 16 +++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/changes/bug23155 b/changes/bug23155 new file mode 100644 index 000000000..4c24ab136 --- /dev/null +++ b/changes/bug23155 @@ -0,0 +1,4 @@ + o Minor bugfixes (stability): + - Avoid crashing on double-free when unable to load or process + an included file. Fixes bug 23155; bugfix on 0.3.1.1-alpha. + Found with the clang static analyzer. diff --git a/src/common/confline.c b/src/common/confline.c index 691cbf8c6..15fd96bf3 100644 --- a/src/common/confline.c +++ b/src/common/confline.c @@ -294,24 +294,26 @@ config_process_include(const char *path, int recursion_level, int extended, return -1; } - SMARTLIST_FOREACH_BEGIN(config_files, char *, config_file) { + int rv = -1; + SMARTLIST_FOREACH_BEGIN(config_files, const char *, config_file) { config_line_t *included_list = NULL; if (config_get_included_list(config_file, recursion_level, extended, &included_list, list_last) < 0) { - SMARTLIST_FOREACH(config_files, char *, f, tor_free(f)); - smartlist_free(config_files); - return -1; + goto done; } - tor_free(config_file); *next = included_list; if (*list_last) next = &(*list_last)->next; } SMARTLIST_FOREACH_END(config_file); - smartlist_free(config_files); *list = ret_list; - return 0; + rv = 0; + + done: + SMARTLIST_FOREACH(config_files, char *, f, tor_free(f)); + smartlist_free(config_files); + return rv; } /**

1 0