November 2016 - tor-commits - lists.torproject.org

[tor/master] Treat bacoff/schedule mismatch as a bug.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 89edef6afbfc701fab5a8ce1f3d7399f7c3261c7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 11:05:57 2016 -0500 Treat bacoff/schedule mismatch as a bug. --- changes/bug20593.2 | 4 ++++ src/or/directory.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/changes/bug20593.2 b/changes/bug20593.2 new file mode 100644 index 0000000..bdaaf00 --- /dev/null +++ b/changes/bug20593.2 @@ -0,0 +1,4 @@ + o Minor features (diagnostic, directory client): + - Warn if we find an unexpected inconsistency in directory download + status objects. Prevents some negative consequences of bug 20593. + diff --git a/src/or/directory.c b/src/or/directory.c index 452571f..6a681ef 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -4055,7 +4055,7 @@ download_status_schedule_get_delay(download_status_t *dls, delay = *(int *)smartlist_get(schedule, smartlist_len(schedule) - 1); } else if (dls->backoff == DL_SCHED_RANDOM_EXPONENTIAL) { /* Check if we missed a reset somehow */ - if (dls->last_backoff_position > dls_schedule_position) { + IF_BUG_ONCE(dls->last_backoff_position > dls_schedule_position) { dls->last_backoff_position = 0; dls->last_delay_used = 0; }

1 0

[tor/maint-0.2.9] Adjust download schedules per teor's #20534 recommendataions
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 667ba776b1d91c8d1229319be88191008975d6eb Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 09:38:49 2016 -0500 Adjust download schedules per teor's #20534 recommendataions --- changes/bug20534 | 6 ++++-- src/or/config.c | 4 ++-- src/or/directory.c | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/changes/bug20534 b/changes/bug20534 index 1ffa1f3..49db433 100644 --- a/changes/bug20534 +++ b/changes/bug20534 @@ -2,5 +2,7 @@ - Remove the maximum delay on exponential-backoff scheduling. Since we now allow an infinite number of failures (see ticket 20536), we must now allow the time to grow longer on each failure. - Fixes bug 20534; bugfix on 0.2.9.1-alpha. - + Fixes part of bug 20534; bugfix on 0.2.9.1-alpha. + - Use initial delays and decrements in download scheduling closer to + those from 0.2.8. Fixes another part of bug 20534; bugfix on + 0.2.9.1-alpha. diff --git a/src/or/config.c b/src/or/config.c index 08c576e..9ad5d63 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -501,7 +501,7 @@ static config_var_t option_vars_[] = { * When clients have authorities and fallbacks available, they use these * schedules: (we stagger the times to avoid thundering herds) */ V(ClientBootstrapConsensusAuthorityDownloadSchedule, CSV_INTERVAL, - "10, 11, 3600, 10800, 25200, 54000, 111600, 262800" /* 3 days + 1 hour */), + "6, 11, 3600, 10800, 25200, 54000, 111600, 262800" /* 3 days + 1 hour */), V(ClientBootstrapConsensusFallbackDownloadSchedule, CSV_INTERVAL, "0, 1, 4, 11, 3600, 10800, 25200, 54000, 111600, 262800"), /* When clients only have authorities available, they use this schedule: */ @@ -512,7 +512,7 @@ static config_var_t option_vars_[] = { * blackholed. Clients will try 3 directories simultaneously. * (Relays never use simultaneous connections.) */ V(ClientBootstrapConsensusMaxInProgressTries, UINT, "3"), - V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "3600, 900, 900, 3600"), + V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "1200, 900, 900, 3600"), V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "10 minutes"), V(TestingDirConnectionMaxStall, INTERVAL, "5 minutes"), V(TestingConsensusMaxDownloadTries, UINT, "8"), diff --git a/src/or/directory.c b/src/or/directory.c index 5fc1572..02f1438 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3798,7 +3798,7 @@ next_random_exponential_delay(int delay, int max_delay) int max_increment; if (delay) - max_increment = delay; /* no more than double. */ + max_increment = delay * 4; /* no more than quintuple. */ else max_increment = 1; /* we're always willing to slow down a little. */

1 0

[tor/maint-0.2.9] Avoid integer overflow in delay calculation.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 1fdf6e5814ae50ed93338644f97c65b497463141 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 09:58:29 2016 -0500 Avoid integer overflow in delay calculation. --- src/or/directory.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index d1333a8..f83f622 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3796,11 +3796,15 @@ next_random_exponential_delay(int delay, int max_delay) /* How much are we willing to add to the delay? */ int max_increment; + const int multiplier = 4; /* no more than quintuple. */ - if (delay) - max_increment = delay * 4; /* no more than quintuple. */ - else + if (delay && delay < (INT_MAX-1) / multiplier) { + max_increment = delay * multiplier; + } else if (delay) { + max_increment = INT_MAX-1; + } else { max_increment = 1; /* we're always willing to slow down a little. */ + } /* the + 1 here is so that we include the end of the interval */ int increment = crypto_rand_int(max_increment+1);

1 0

[tor/maint-0.2.9] Merge branch 'bug20534_029_squashed' into maint-0.2.9
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 293aca9929cfa0205d16899191c16855a607ac77 Merge: e4b793f e51f105 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 11:01:31 2016 -0500 Merge branch 'bug20534_029_squashed' into maint-0.2.9 changes/bug20534 | 6 ++++-- changes/bug20593 | 6 ++++++ src/or/config.c | 4 ++-- src/or/directory.c | 28 ++++++++++++++++++---------- 4 files changed, 30 insertions(+), 14 deletions(-)

1 0

[tor/maint-0.2.9] Count HTTP 503 as a download failure.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 864c42f4d66641028005f8d11868368260a37b84 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 09:43:12 2016 -0500 Count HTTP 503 as a download failure. Because as Teor puts it: "[Resetting on 503] is exactly what we don't want when relays are busy - imagine clients doing an automatic reset every time they DoS a relay..." Fixes bug 20593. --- changes/bug20593 | 6 ++++++ src/or/directory.c | 9 +++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/changes/bug20593 b/changes/bug20593 new file mode 100644 index 0000000..e9f54d3 --- /dev/null +++ b/changes/bug20593 @@ -0,0 +1,6 @@ + o Minor bugfixes (client directory scheduling): + - Treat "relay too busy to answer request" as a failed request and a + reason to back off on our retry frequency. This is safe now that + exponential backups retry indefinitely, and avoids a bug where we would + reset our download schedule erroneously. + Fixes bug 20593; bugfix on 0.2.9.1-alpha. diff --git a/src/or/directory.c b/src/or/directory.c index 02f1438..d1333a8 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3933,15 +3933,16 @@ time_t download_status_increment_failure(download_status_t *dls, int status_code, const char *item, int server, time_t now) { + (void) status_code; // XXXX no longer used. + (void) server; // XXXX no longer used. int increment = -1; int min_delay = 0, max_delay = INT_MAX; tor_assert(dls); - /* only count the failure if it's permanent, or we're a server */ - if (status_code != 503 || server) { - if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1) - ++dls->n_download_failures; + /* count the failure */ + if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1) { + ++dls->n_download_failures; } if (dls->increment_on == DL_SCHED_INCREMENT_FAILURE) {

1 0

[tor/maint-0.2.9] Reduce multiplier to 3, per teor's recommendation on #20534
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit e51f105c417aff1840126fddd11875fec84c86a9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 10:55:33 2016 -0500 Reduce multiplier to 3, per teor's recommendation on #20534 (Three _is_ a good number for anonymity!) --- src/or/directory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/directory.c b/src/or/directory.c index ee42e2c..1f39981 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3796,7 +3796,7 @@ next_random_exponential_delay(int delay, int max_delay) /* How much are we willing to add to the delay? */ int max_increment; - const int multiplier = 4; /* no more than quintuple. */ + const int multiplier = 3; /* no more than quadruple the previous delay */ if (delay && delay < (INT_MAX-1) / multiplier) { max_increment = delay * multiplier;

1 0

[tor/master] Avoid integer overflow in delay calculation.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 1fdf6e5814ae50ed93338644f97c65b497463141 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 09:58:29 2016 -0500 Avoid integer overflow in delay calculation. --- src/or/directory.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index d1333a8..f83f622 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3796,11 +3796,15 @@ next_random_exponential_delay(int delay, int max_delay) /* How much are we willing to add to the delay? */ int max_increment; + const int multiplier = 4; /* no more than quintuple. */ - if (delay) - max_increment = delay * 4; /* no more than quintuple. */ - else + if (delay && delay < (INT_MAX-1) / multiplier) { + max_increment = delay * multiplier; + } else if (delay) { + max_increment = INT_MAX-1; + } else { max_increment = 1; /* we're always willing to slow down a little. */ + } /* the + 1 here is so that we include the end of the interval */ int increment = crypto_rand_int(max_increment+1);

1 0

[tor/master] Count HTTP 503 as a download failure.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 864c42f4d66641028005f8d11868368260a37b84 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 09:43:12 2016 -0500 Count HTTP 503 as a download failure. Because as Teor puts it: "[Resetting on 503] is exactly what we don't want when relays are busy - imagine clients doing an automatic reset every time they DoS a relay..." Fixes bug 20593. --- changes/bug20593 | 6 ++++++ src/or/directory.c | 9 +++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/changes/bug20593 b/changes/bug20593 new file mode 100644 index 0000000..e9f54d3 --- /dev/null +++ b/changes/bug20593 @@ -0,0 +1,6 @@ + o Minor bugfixes (client directory scheduling): + - Treat "relay too busy to answer request" as a failed request and a + reason to back off on our retry frequency. This is safe now that + exponential backups retry indefinitely, and avoids a bug where we would + reset our download schedule erroneously. + Fixes bug 20593; bugfix on 0.2.9.1-alpha. diff --git a/src/or/directory.c b/src/or/directory.c index 02f1438..d1333a8 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3933,15 +3933,16 @@ time_t download_status_increment_failure(download_status_t *dls, int status_code, const char *item, int server, time_t now) { + (void) status_code; // XXXX no longer used. + (void) server; // XXXX no longer used. int increment = -1; int min_delay = 0, max_delay = INT_MAX; tor_assert(dls); - /* only count the failure if it's permanent, or we're a server */ - if (status_code != 503 || server) { - if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1) - ++dls->n_download_failures; + /* count the failure */ + if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1) { + ++dls->n_download_failures; } if (dls->increment_on == DL_SCHED_INCREMENT_FAILURE) {

1 0

[tor/master] Always increment delays by at least 1.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 85970f70475af7f8bd6666cbdebf5bfa3095625d Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 10:17:13 2016 -0500 Always increment delays by at least 1. --- 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 f83f622..ee42e2c 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3803,11 +3803,14 @@ next_random_exponential_delay(int delay, int max_delay) } else if (delay) { max_increment = INT_MAX-1; } else { - max_increment = 1; /* we're always willing to slow down a little. */ + max_increment = 1; } - /* the + 1 here is so that we include the end of the interval */ - int increment = crypto_rand_int(max_increment+1); + if (BUG(max_increment < 1)) + max_increment = 1; + + /* the + 1 here is so that we always wait longer than last time. */ + int increment = crypto_rand_int(max_increment)+1; if (increment < max_delay - delay) return delay + increment;

1 0

[tor/maint-0.2.9] Always increment delays by at least 1.
by nickm＠torproject.org 07 Nov '16

07 Nov '16

commit 85970f70475af7f8bd6666cbdebf5bfa3095625d Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Nov 7 10:17:13 2016 -0500 Always increment delays by at least 1. --- 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 f83f622..ee42e2c 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3803,11 +3803,14 @@ next_random_exponential_delay(int delay, int max_delay) } else if (delay) { max_increment = INT_MAX-1; } else { - max_increment = 1; /* we're always willing to slow down a little. */ + max_increment = 1; } - /* the + 1 here is so that we include the end of the interval */ - int increment = crypto_rand_int(max_increment+1); + if (BUG(max_increment < 1)) + max_increment = 1; + + /* the + 1 here is so that we always wait longer than last time. */ + int increment = crypto_rand_int(max_increment)+1; if (increment < max_delay - delay) return delay + increment;

1 0