commit d46c1b49a459f1249ef358b3751ef656d9e19038 Author: rubiate cb@viennan.net Date: Thu Nov 10 10:16:18 2016 +1300
Do not serve a consensus if it is too old
Closes ticket 20511. --- changes/20511 | 3 +++ src/or/directory.c | 31 ++++++++++++++++++++++ src/or/networkstatus.c | 21 +++++++++++++-- src/or/networkstatus.h | 2 ++ src/test/test_dir_handle_get.c | 60 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 2 deletions(-)
diff --git a/changes/20511 b/changes/20511 new file mode 100644 index 0000000..d6e962e --- /dev/null +++ b/changes/20511 @@ -0,0 +1,3 @@ + o Minor feature: + - Relays and bridges will now refuse to serve the consensus they have if + they know it is too old for a client to use. Closes ticket 20511. diff --git a/src/or/directory.c b/src/or/directory.c index ba6d38c..b5c9d49 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2939,6 +2939,28 @@ handle_get_frontpage(dir_connection_t *conn, const get_handler_args_t *args) return 0; }
+/** Warn that the consensus <b>v</b> of type <b>flavor</b> is too old and will + * not be served to clients. Rate-limit the warning to avoid logging an entry + * on every request. + */ +static void +warn_consensus_is_too_old(networkstatus_t *v, const char *flavor, time_t now) +{ +#define TOO_OLD_WARNING_INTERVAL (60*60) + static ratelim_t warned = RATELIM_INIT(TOO_OLD_WARNING_INTERVAL); + char timestamp[ISO_TIME_LEN+1]; + char *dupes; + + if ((dupes = rate_limit_log(&warned, now))) { + format_local_iso_time(timestamp, v->valid_until); + log_warn(LD_DIRSERV, "Our %s%sconsensus is too old, so we will not " + "serve it to clients. It was valid until %s local time and we " + "continued to serve it for up to 24 hours after it expired.%s", + flavor ? flavor : "", flavor ? " " : "", timestamp, dupes); + tor_free(dupes); + } +} + /** Helper function for GET /tor/status-vote/current/consensus */ static int @@ -2983,6 +3005,15 @@ handle_get_current_consensus(dir_connection_t *conn,
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); + smartlist_free(dir_fps); + 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 " diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index ed888fb..fde0b18 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1342,6 +1342,24 @@ networkstatus_get_live_consensus,(time_t now)) return NULL; }
+/** Determine if <b>consensus</b> is valid or expired recently enough that + * we can still use it. + * + * Return 1 if the consensus is reasonably live, or 0 if it is too old. + */ +int +networkstatus_consensus_reasonably_live(networkstatus_t *consensus, time_t now) +{ +#define REASONABLY_LIVE_TIME (24*60*60) + if (BUG(!consensus)) + return 0; + + if (now <= consensus->valid_until + REASONABLY_LIVE_TIME) + return 1; + + return 0; +} + /* XXXX remove this in favor of get_live_consensus. But actually, * leave something like it for bridge users, who need to not totally * lose if they spend a while fetching a new consensus. */ @@ -1350,12 +1368,11 @@ networkstatus_get_live_consensus,(time_t now)) networkstatus_t * networkstatus_get_reasonably_live_consensus(time_t now, int flavor) { -#define REASONABLY_LIVE_TIME (24*60*60) networkstatus_t *consensus = networkstatus_get_latest_consensus_by_flavor(flavor); if (consensus && consensus->valid_after <= now && - now <= consensus->valid_until+REASONABLY_LIVE_TIME) + networkstatus_consensus_reasonably_live(consensus, now)) return consensus; else return NULL; diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h index 71f36b6..172c0ea 100644 --- a/src/or/networkstatus.h +++ b/src/or/networkstatus.h @@ -79,6 +79,8 @@ MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus,(void)); MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor, (consensus_flavor_t f)); MOCK_DECL(networkstatus_t *, networkstatus_get_live_consensus,(time_t now)); +int networkstatus_consensus_reasonably_live(networkstatus_t *consensus, + time_t now); networkstatus_t *networkstatus_get_reasonably_live_consensus(time_t now, int flavor); MOCK_DECL(int, networkstatus_consensus_is_bootstrapping,(time_t now)); diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c index c215fee..a0868f9 100644 --- a/src/test/test_dir_handle_get.c +++ b/src/test/test_dir_handle_get.c @@ -30,6 +30,7 @@ #include "dirserv.h" #include "torgzip.h" #include "dirvote.h" +#include "log_test_helpers.h"
#ifdef _WIN32 /* For mkdir() */ @@ -53,6 +54,7 @@ ENABLE_GCC_WARNING(overlength-strings) #define NOT_FOUND "HTTP/1.0 404 Not found\r\n\r\n" #define BAD_REQUEST "HTTP/1.0 400 Bad request\r\n\r\n" #define SERVER_BUSY "HTTP/1.0 503 Directory busy, try again later\r\n\r\n" +#define TOO_OLD "HTTP/1.0 404 Consensus is too old\r\n\r\n" #define NOT_ENOUGH_CONSENSUS_SIGNATURES "HTTP/1.0 404 " \ "Consensus not signed by sufficient number of requested authorities\r\n\r\n"
@@ -1617,6 +1619,7 @@ test_dir_handle_get_status_vote_current_consensus_ns_not_enough_sigs(void* d) mock_ns_val = tor_malloc_zero(sizeof(networkstatus_t)); mock_ns_val->flavor = FLAV_NS; mock_ns_val->voters = smartlist_new(); + mock_ns_val->valid_until = time(NULL);
/* init mock */ init_mock_options(); @@ -1696,6 +1699,62 @@ test_dir_handle_get_status_vote_current_consensus_ns_not_found(void* data) or_options_free(mock_options); mock_options = NULL; }
+static void +test_dir_handle_get_status_vote_current_consensus_too_old(void *data) +{ + dir_connection_t *conn = NULL; + char *header = NULL; + (void)data; + + mock_ns_val = tor_malloc_zero(sizeof(networkstatus_t)); + mock_ns_val->flavor = FLAV_MICRODESC; + mock_ns_val->valid_until = time(NULL) - (60 * 60 * 24) - 1; + + init_mock_options(); + MOCK(get_options, mock_get_options); + MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock); + MOCK(networkstatus_get_latest_consensus_by_flavor, mock_ns_get_by_flavor); + + conn = new_dir_conn(); + + setup_capture_of_logs(LOG_WARN); + + tt_int_op(0, OP_EQ, directory_handle_command_get(conn, + GET("/tor/status-vote/current/consensus-microdesc"), NULL, 0)); + + fetch_from_buf_http(TO_CONN(conn)->outbuf, &header, MAX_HEADERS_SIZE, + NULL, NULL, 1, 0); + tt_assert(header); + tt_str_op(TOO_OLD, OP_EQ, header); + + expect_log_msg_containing("too old"); + + tor_free(header); + teardown_capture_of_logs(); + + setup_capture_of_logs(LOG_WARN); + + tt_int_op(0, OP_EQ, directory_handle_command_get(conn, + GET("/tor/status-vote/current/consensus"), NULL, 0)); + + fetch_from_buf_http(TO_CONN(conn)->outbuf, &header, MAX_HEADERS_SIZE, + NULL, NULL, 1, 0); + tt_assert(header); + tt_str_op(TOO_OLD, OP_EQ, header); + + expect_no_log_entry(); + + done: + teardown_capture_of_logs(); + UNMOCK(networkstatus_get_latest_consensus_by_flavor); + UNMOCK(connection_write_to_buf_impl_); + UNMOCK(get_options); + connection_free_(TO_CONN(conn)); + tor_free(header); + tor_free(mock_ns_val); + or_options_free(mock_options); mock_options = NULL; +} + NS_DECL(int, geoip_get_country_by_addr, (const tor_addr_t *addr));
int @@ -2481,6 +2540,7 @@ struct testcase_t dir_handle_get_tests[] = { DIR_HANDLE_CMD(status_vote_next_authority, 0), DIR_HANDLE_CMD(status_vote_current_consensus_ns_not_enough_sigs, 0), DIR_HANDLE_CMD(status_vote_current_consensus_ns_not_found, 0), + DIR_HANDLE_CMD(status_vote_current_consensus_too_old, 0), DIR_HANDLE_CMD(status_vote_current_consensus_ns_busy, 0), DIR_HANDLE_CMD(status_vote_current_consensus_ns, 0), DIR_HANDLE_CMD(status_vote_current_d_not_found, 0),