From 40c13240c08349b94d39a1af320f61a1f7a4fda0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 4 Feb 2013 10:19:26 -0500 Subject: [PATCH 1/5] When computing performance thresholds, ignore omitted-as-sybil nodes. Fixes bug 8146. --- changes/bug8146_etc | 5 +++++ src/or/dirserv.c | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 changes/bug8146_etc diff --git a/changes/bug8146_etc b/changes/bug8146_etc new file mode 100644 index 0000000000..d59c4c1af8 --- /dev/null +++ b/changes/bug8146_etc @@ -0,0 +1,5 @@ + o Major bugfixes (security, directory authority): + - When computing directory thresholds, ignore any rejected-as-sybil + nodes during the computation so that they can't influence Fast, + Guard, etc. Fixes bug 8146. + diff --git a/src/or/dirserv.c b/src/or/dirserv.c index e2cd7cfd26..320b8e00d3 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1893,7 +1893,8 @@ dirserv_thinks_router_is_hs_dir(const routerinfo_t *router, * * Also, set the is_exit flag of each router appropriately. */ static void -dirserv_compute_performance_thresholds(routerlist_t *rl) +dirserv_compute_performance_thresholds(routerlist_t *rl, + digestmap_t *omit_as_sybil) { int n_active, n_active_nonexit, n_familiar; uint32_t *uptimes, *bandwidths, *bandwidths_excluding_exits; @@ -1935,7 +1936,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl) /* Now, fill in the arrays. */ SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) { routerinfo_t *ri = node->ri; - if (ri && router_is_active(ri, node, now)) { + if (ri && router_is_active(ri, node, now) && + !digestmap_get(omit_as_sybil, ri->cache_info.identity_digest)) { const char *id = ri->cache_info.identity_digest; uint32_t bw; node->is_exit = (!router_exit_policy_rejects_all(ri) && @@ -1997,7 +1999,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl) SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) { routerinfo_t *ri = node->ri; - if (ri && router_is_active(ri, node, now)) { + if (ri && router_is_active(ri, node, now) && + !digestmap_get(omit_as_sybil, ri->cache_info.identity_digest)) { const char *id = ri->cache_info.identity_digest; long tk = rep_hist_get_weighted_time_known(id, now); if (tk < guard_tk) @@ -2751,13 +2754,13 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, dirserv_set_router_is_running(ri, now); }); - dirserv_compute_performance_thresholds(rl); - routers = smartlist_new(); smartlist_add_all(routers, rl->routers); routers_sort_by_identity(routers); omit_as_sybil = get_possible_sybil_list(routers); + dirserv_compute_performance_thresholds(rl, omit_as_sybil); + routerstatuses = smartlist_new(); microdescriptors = smartlist_new(); @@ -3008,14 +3011,13 @@ generate_v2_networkstatus_opinion(void) dirserv_set_router_is_running(ri, now); }); - dirserv_compute_performance_thresholds(rl); - routers = smartlist_new(); smartlist_add_all(routers, rl->routers); routers_sort_by_identity(routers); - omit_as_sybil = get_possible_sybil_list(routers); + dirserv_compute_performance_thresholds(rl, omit_as_sybil); + SMARTLIST_FOREACH_BEGIN(routers, routerinfo_t *, ri) { if (ri->cache_info.published_on >= cutoff) { routerstatus_t rs; From 8be7f69f8d2bdf074e5e90279dd42e182562ba7d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 4 Feb 2013 10:22:45 -0500 Subject: [PATCH 2/5] Refactor should-count-towards-thresholds test into new function --- src/or/dirserv.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 320b8e00d3..6c4b119e43 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1884,6 +1884,16 @@ dirserv_thinks_router_is_hs_dir(const routerinfo_t *router, node->is_running); } +/** Helper for dirserv_compute_performance_thresholds(): Decide whether to + * include a router in our calculations, and return true iff we should. */ +static int +router_counts_toward_thresholds(const node_t *node, time_t now, + const digestmap_t *omit_as_sybil) +{ + return node->ri && router_is_active(node->ri, node, now) && + !digestmap_get(omit_as_sybil, node->ri->cache_info.identity_digest); +} + /** Look through the routerlist, the Mean Time Between Failure history, and * the Weighted Fractional Uptime history, and use them to set thresholds for * the Stable, Fast, and Guard flags. Update the fields stable_uptime, @@ -1935,9 +1945,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, /* Now, fill in the arrays. */ SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) { - routerinfo_t *ri = node->ri; - if (ri && router_is_active(ri, node, now) && - !digestmap_get(omit_as_sybil, ri->cache_info.identity_digest)) { + if (router_counts_toward_thresholds(node, now, omit_as_sybil)) { + routerinfo_t *ri = node->ri; const char *id = ri->cache_info.identity_digest; uint32_t bw; node->is_exit = (!router_exit_policy_rejects_all(ri) && @@ -1998,9 +2007,8 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, n_familiar = 0; SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) { - routerinfo_t *ri = node->ri; - if (ri && router_is_active(ri, node, now) && - !digestmap_get(omit_as_sybil, ri->cache_info.identity_digest)) { + if (router_counts_toward_thresholds(node, now, omit_as_sybil)) { + routerinfo_t *ri = node->ri; const char *id = ri->cache_info.identity_digest; long tk = rep_hist_get_weighted_time_known(id, now); if (tk < guard_tk) From 317d16de04ef9f2fa827b3bea2d858069a721e24 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 4 Feb 2013 10:41:11 -0500 Subject: [PATCH 3/5] Increase the minimum value for the Fast flag to 4096. Fix for 8145. --- changes/bug8146_etc | 4 +++- src/or/dirserv.c | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/changes/bug8146_etc b/changes/bug8146_etc index d59c4c1af8..3775aa5059 100644 --- a/changes/bug8146_etc +++ b/changes/bug8146_etc @@ -1,5 +1,7 @@ o Major bugfixes (security, directory authority): - When computing directory thresholds, ignore any rejected-as-sybil nodes during the computation so that they can't influence Fast, - Guard, etc. Fixes bug 8146. + Guard, etc. Fixes bug 8146. + - When computing thresholds for flags, never let the threshold for + the Fast flag to 4096 bytes. Fixes bug 8145. diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 6c4b119e43..f3cb2de918 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1986,9 +1986,12 @@ dirserv_compute_performance_thresholds(routerlist_t *rl, { /* We can vote on a parameter for the minimum and maximum. */ +#define ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG 4096 int32_t min_fast, max_fast; min_fast = networkstatus_get_param(NULL, "FastFlagMinThreshold", - 0, 0, INT32_MAX); + ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG, + ABSOLUTE_MIN_VALUE_FOR_FAST_FLAG, + INT32_MAX); max_fast = networkstatus_get_param(NULL, "FastFlagMaxThreshold", INT32_MAX, min_fast, INT32_MAX); if (fast_bandwidth < (uint32_t)min_fast) From 61995d3e2cd7631df1fcb9fbdf9333dee24566b4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 4 Feb 2013 10:47:08 -0500 Subject: [PATCH 4/5] Ignore tiny bandwidths entirely when computing thresholds Another bug 8145 fix. --- changes/bug8146_etc | 3 +++ src/or/dirserv.c | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/changes/bug8146_etc b/changes/bug8146_etc index 3775aa5059..274e2abe48 100644 --- a/changes/bug8146_etc +++ b/changes/bug8146_etc @@ -5,3 +5,6 @@ - When computing thresholds for flags, never let the threshold for the Fast flag to 4096 bytes. Fixes bug 8145. + - Do not consider nodes with extremely low bandwidths when deciding + thresholds for various directory flags. Another fix for 8145. + diff --git a/src/or/dirserv.c b/src/or/dirserv.c index f3cb2de918..0c3e72f3a3 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1884,6 +1884,10 @@ dirserv_thinks_router_is_hs_dir(const routerinfo_t *router, node->is_running); } +/** Don't consider routers with less bandwidth than this when computing + * thresholds. */ +#define ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER 4096 + /** Helper for dirserv_compute_performance_thresholds(): Decide whether to * include a router in our calculations, and return true iff we should. */ static int @@ -1891,7 +1895,9 @@ router_counts_toward_thresholds(const node_t *node, time_t now, const digestmap_t *omit_as_sybil) { return node->ri && router_is_active(node->ri, node, now) && - !digestmap_get(omit_as_sybil, node->ri->cache_info.identity_digest); + !digestmap_get(omit_as_sybil, node->ri->cache_info.identity_digest) && + (router_get_advertised_bandwidth(node->ri) >= + ABSOLUTE_MIN_BW_VALUE_TO_CONSIDER); } /** Look through the routerlist, the Mean Time Between Failure history, and From 4eff8b65305248b1e7a57d1efdf8f55039719bd7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 4 Feb 2013 11:11:54 -0500 Subject: [PATCH 5/5] When we mark a node as a sybil, mark it down and reset its uptime to 0 This prevents bug 8147, where such nodes would accrue points towards Guard, Fast, HSDir, and so on. Fixes bug 8147. --- changes/bug8146_etc | 3 +++ src/or/dirserv.c | 5 +++++ src/or/rephist.c | 15 +++++++++++++++ src/or/rephist.h | 2 ++ 4 files changed, 25 insertions(+) diff --git a/changes/bug8146_etc b/changes/bug8146_etc index 274e2abe48..173ea3b58d 100644 --- a/changes/bug8146_etc +++ b/changes/bug8146_etc @@ -8,3 +8,6 @@ - Do not consider nodes with extremely low bandwidths when deciding thresholds for various directory flags. Another fix for 8145. + - When marking a node as a likely sybil, reset its uptime metrics + to zero, so that it cannot time towards getting marked as Guard, + Stable, or HSDir. Fix for bug 8147. diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 0c3e72f3a3..b59478e17d 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2776,6 +2776,11 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, routers_sort_by_identity(routers); omit_as_sybil = get_possible_sybil_list(routers); + DIGESTMAP_FOREACH(omit_as_sybil, sybil_id, void *, ignore) { + (void) ignore; + rep_hist_make_router_pessimal(sybil_id, now); + } DIGESTMAP_FOREACH_END; + dirserv_compute_performance_thresholds(rl, omit_as_sybil); routerstatuses = smartlist_new(); diff --git a/src/or/rephist.c b/src/or/rephist.c index 925ca88153..34caa4b518 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -422,6 +422,21 @@ rep_hist_note_router_unreachable(const char *id, time_t when) } } +/** Mark a router with ID id as non-Running, and retroactively declare + * that it has never been running: give it no stability and no WFU. */ +void +rep_hist_make_router_pessimal(const char *id, time_t when) +{ + or_history_t *hist = get_or_history(id); + tor_assert(hist); + + rep_hist_note_router_unreachable(id, when); + mark_or_down(hist, when, 1); + + hist->weighted_run_length = 0; + hist->weighted_uptime = 0; +} + /** Helper: Discount all old MTBF data, if it is time to do so. Return * the time at which we should next discount MTBF data. */ time_t diff --git a/src/or/rephist.h b/src/or/rephist.h index 5568330dd7..811cd8d450 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -24,6 +24,8 @@ void rep_hist_dump_stats(time_t now, int severity); void rep_hist_note_bytes_read(size_t num_bytes, time_t when); void rep_hist_note_bytes_written(size_t num_bytes, time_t when); +void rep_hist_make_router_pessimal(const char *id, time_t when); + void rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when); void rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when);