[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it



#20511: add a failsafe where if you're about to serve a consensus that you know is
obsolete, don't do it
--------------------------+------------------------------------
 Reporter:  arma          |          Owner:
     Type:  enhancement   |         Status:  needs_revision
 Priority:  Medium        |      Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:                |  Actual Points:
Parent ID:                |         Points:
 Reviewer:                |        Sponsor:
--------------------------+------------------------------------

Comment (by rubiate):

 Here's an updated patch using a define for the warning timeout and a NULL
 consensus check:

 {{{
 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..24393eb 100644
 --- a/src/or/directory.c
 +++ b/src/or/directory.c
 @@ -2939,6 +2939,33 @@ 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. Log the message at a lower severity if we have already
 + * warned about this recently.
 + */
 +static void
 +warn_consensus_is_too_old(networkstatus_t *v, const char *flavor, time_t
 now)
 +{
 +  static time_t warned = 0;
 +  int severity;
 +  char timestamp[ISO_TIME_LEN+1];
 +
 +#define TOO_OLD_WARNING_TIMEOUT 60*60
 +  if (now >= warned + TOO_OLD_WARNING_TIMEOUT) {
 +    severity = LOG_WARN;
 +    warned = now;
 +  } else {
 +    severity = LOG_DEBUG;
 +  }
 +
 +  format_local_iso_time(timestamp, v->valid_until);
 +  log_fn(severity, LD_DIRSERV,
 +         "Our %s%sconsensus is too old, we will not serve it to clients.
 "
 +         "It was valid until %s and we continued to serve it for up to 24
 "
 +         "hours after it expired.",
 +         flavor ? flavor : "", flavor ? " " : "", timestamp);
 +}
 +
  /** Helper function for GET /tor/status-vote/current/consensus
   */
  static int
 @@ -2983,6 +3010,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..cab3fb2 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,63 @@
 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);
 +  header = NULL;
 +  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 +2541,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),
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20511#comment:7>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs