[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] Merge branch 'bug32588_035' into bug32588_041

To: tor-commits@xxxxxxxxxxxxxxxxxxxx
Subject: [tor-commits] [tor/master] Merge branch 'bug32588_035' into bug32588_041
From: nickm@xxxxxxxxxxxxxx
Date: Mon, 23 Mar 2020 13:03:28 +0000 (UTC)
Delivered-to: archiver@xxxxxxxx
Delivery-date: Mon, 23 Mar 2020 09:03:54 -0400
List-archive: <http://lists.torproject.org/pipermail/tor-commits/>
List-help: <mailto:tor-commits-request@lists.torproject.org?subject=help>
List-id: "auto: code repository commits" <tor-commits.lists.torproject.org>
List-post: <mailto:tor-commits@lists.torproject.org>
List-subscribe: <https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits>, <mailto:tor-commits-request@lists.torproject.org?subject=subscribe>
List-unsubscribe: <https://lists.torproject.org/cgi-bin/mailman/options/tor-commits>, <mailto:tor-commits-request@lists.torproject.org?subject=unsubscribe>
Patch-author: teor <teor@xxxxxxxxxxxxxx>
Sender: "tor-commits" <tor-commits-bounces@xxxxxxxxxxxxxxxxxxxx>
commit 2962c32b7a60b78f4d470bf063f8b1454b56506b
Merge: 42ea03eb7 96ca14d98
Author: teor <teor@xxxxxxxxxxxxxx>
Date:   Sat Mar 21 03:50:36 2020 +1000

    Merge branch 'bug32588_035' into bug32588_041
    
    Merge tests from maint-0.4.1 with new tests from bug32588_035
    in test_router.c.

 changes/bug32588           |   4 ++
 src/app/config/config.c    |   5 +-
 src/app/config/config.h    |   4 ++
 src/feature/relay/router.c |  77 ++++++++++++++++++-----------
 src/feature/relay/router.h |   2 +
 src/test/test_config.c     |  66 +++++++++++++++++++++++++
 src/test/test_router.c     | 117 +++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 243 insertions(+), 32 deletions(-)

diff --cc src/test/test_router.c
index 5477ab51e,e0d3adfdb..e0dd607b8
--- a/src/test/test_router.c
+++ b/src/test/test_router.c
@@@ -8,19 -8,14 +8,21 @@@
   **/
  
  #define CONFIG_PRIVATE
+ #define CONNECTION_PRIVATE
 +#define ROUTER_PRIVATE
 +
  #include "core/or/or.h"
  #include "app/config/config.h"
  #include "core/mainloop/mainloop.h"
+ #include "core/mainloop/connection.h"
  #include "feature/hibernate/hibernate.h"
 +#include "feature/nodelist/networkstatus.h"
 +#include "feature/nodelist/networkstatus_st.h"
 +#include "feature/nodelist/node_st.h"
 +#include "feature/nodelist/nodelist.h"
  #include "feature/nodelist/routerinfo_st.h"
  #include "feature/nodelist/routerlist.h"
 +#include "feature/nodelist/routerstatus_st.h"
  #include "feature/relay/router.h"
  #include "feature/stats/rephist.h"
  #include "lib/crypt_ops/crypto_curve25519.h"
@@@ -243,254 -237,124 +247,367 @@@ test_router_check_descriptor_bandwidth_
    UNMOCK(we_are_hibernating);
  }
  
 +static networkstatus_t *mock_ns = NULL;
 +static networkstatus_t *
 +mock_networkstatus_get_live_consensus(time_t now)
 +{
 +  (void)now;
 +  return mock_ns;
 +}
 +
 +static routerstatus_t *mock_rs = NULL;
 +static const routerstatus_t *
 +mock_networkstatus_vote_find_entry(networkstatus_t *ns, const char *digest)
 +{
 +  (void)ns;
 +  (void)digest;
 +  return mock_rs;
 +}
 +
 +static void
 +test_router_mark_if_too_old(void *arg)
 +{
 +  (void)arg;
 +  time_t now = approx_time();
 +  MOCK(networkstatus_get_live_consensus,
 +       mock_networkstatus_get_live_consensus);
 +  MOCK(networkstatus_vote_find_entry, mock_networkstatus_vote_find_entry);
 +
 +  routerstatus_t rs;
 +  networkstatus_t ns;
 +  memset(&rs, 0, sizeof(rs));
 +  memset(&ns, 0, sizeof(ns));
 +  mock_ns = &ns;
 +  mock_ns->valid_after = now-3600;
 +  mock_rs = &rs;
 +  mock_rs->published_on = now - 10;
 +
 +  // no reason to mark this time.
 +  desc_clean_since = now-10;
 +  desc_dirty_reason = NULL;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, now-10);
 +
 +  // Doesn't appear in consensus?  Still don't mark it.
 +  mock_ns = NULL;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, now-10);
 +  mock_ns = &ns;
 +
 +  // No new descriptor in a long time?  Mark it.
 +  desc_clean_since = now - 3600 * 96;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, 0);
 +  tt_str_op(desc_dirty_reason, OP_EQ, "time for new descriptor");
 +
 +  // Version in consensus published a long time ago?  We won't mark it
 +  // if it's been clean for only a short time.
 +  desc_clean_since = now - 10;
 +  desc_dirty_reason = NULL;
 +  mock_rs->published_on = now - 3600 * 96;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, now - 10);
 +
 +  // ... but if it's been clean a while, we mark.
 +  desc_clean_since = now - 2 * 3600;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, 0);
 +  tt_str_op(desc_dirty_reason, OP_EQ,
 +            "version listed in consensus is quite old");
 +
 +  // same deal if we're marked stale.
 +  desc_clean_since = now - 2 * 3600;
 +  desc_dirty_reason = NULL;
 +  mock_rs->published_on = now - 10;
 +  mock_rs->is_staledesc = 1;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, 0);
 +  tt_str_op(desc_dirty_reason, OP_EQ,
 +            "listed as stale in consensus");
 +
 +  // same deal if we're absent from the consensus.
 +  desc_clean_since = now - 2 * 3600;
 +  desc_dirty_reason = NULL;
 +  mock_rs = NULL;
 +  mark_my_descriptor_dirty_if_too_old(now);
 +  tt_i64_op(desc_clean_since, OP_EQ, 0);
 +  tt_str_op(desc_dirty_reason, OP_EQ,
 +            "not listed in consensus");
 +
 + done:
 +  UNMOCK(networkstatus_get_live_consensus);
 +  UNMOCK(networkstatus_vote_find_entry);
 +}
 +
 +static node_t fake_node;
 +static const node_t *
 +mock_node_get_by_nickname(const char *name, unsigned flags)
 +{
 +  (void)flags;
 +  if (!strcasecmp(name, "crumpet"))
 +    return &fake_node;
 +  else
 +    return NULL;
 +}
 +
 +static void
 +test_router_get_my_family(void *arg)
 +{
 +  (void)arg;
 +  or_options_t *options = options_new();
 +  smartlist_t *sl = NULL;
 +  char *join = NULL;
 +  // Overwrite the result of router_get_my_identity_digest().  This
 +  // happens to be okay, but only for testing.
 +  set_server_identity_key_digest_testing(
 +                                   (const uint8_t*)"holeinthebottomofthe");
 +
 +  setup_capture_of_logs(LOG_WARN);
 +
 +  // No family listed -- so there's no list.
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_EQ, NULL);
 +  expect_no_log_entry();
 +
 +#define CLEAR() do {                                    \
 +    if (sl) {                                           \
 +      SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp));  \
 +      smartlist_free(sl);                               \
 +    }                                                   \
 +    tor_free(join);                                     \
 +    mock_clean_saved_logs();                            \
 +  } while (0)
 +
 +  // Add a single nice friendly hex member.  This should be enough
 +  // to have our own ID added.
 +  tt_ptr_op(options->MyFamily, OP_EQ, NULL);
 +  config_line_append(&options->MyFamily, "MyFamily",
 +                     "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
 +
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_NE, NULL);
 +  tt_int_op(smartlist_len(sl), OP_EQ, 2);
 +  join = smartlist_join_strings(sl, " ", 0, NULL);
 +  tt_str_op(join, OP_EQ,
 +            "$686F6C65696E746865626F74746F6D6F66746865 "
 +            "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
 +  expect_no_log_entry();
 +  CLEAR();
 +
 +  // Add a hex member with a ~.  The ~ part should get removed.
 +  config_line_append(&options->MyFamily, "MyFamily",
 +                     "$0123456789abcdef0123456789abcdef01234567~Muffin");
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_NE, NULL);
 +  tt_int_op(smartlist_len(sl), OP_EQ, 3);
 +  join = smartlist_join_strings(sl, " ", 0, NULL);
 +  tt_str_op(join, OP_EQ,
 +            "$0123456789ABCDEF0123456789ABCDEF01234567 "
 +            "$686F6C65696E746865626F74746F6D6F66746865 "
 +            "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
 +  expect_no_log_entry();
 +  CLEAR();
 +
 +  // Nickname lookup will fail, so a nickname will appear verbatim.
 +  config_line_append(&options->MyFamily, "MyFamily",
 +                     "BAGEL");
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_NE, NULL);
 +  tt_int_op(smartlist_len(sl), OP_EQ, 4);
 +  join = smartlist_join_strings(sl, " ", 0, NULL);
 +  tt_str_op(join, OP_EQ,
 +            "$0123456789ABCDEF0123456789ABCDEF01234567 "
 +            "$686F6C65696E746865626F74746F6D6F66746865 "
 +            "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA "
 +            "bagel");
 +  expect_single_log_msg_containing(
 +           "There is a router named \"BAGEL\" in my declared family, but "
 +           "I have no descriptor for it.");
 +  CLEAR();
 +
 +  // A bogus digest should fail entirely.
 +  config_line_append(&options->MyFamily, "MyFamily",
 +                     "$painauchocolat");
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_NE, NULL);
 +  tt_int_op(smartlist_len(sl), OP_EQ, 4);
 +  join = smartlist_join_strings(sl, " ", 0, NULL);
 +  tt_str_op(join, OP_EQ,
 +            "$0123456789ABCDEF0123456789ABCDEF01234567 "
 +            "$686F6C65696E746865626F74746F6D6F66746865 "
 +            "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA "
 +            "bagel");
 +  // "BAGEL" is still there, but it won't make a warning, because we already
 +  // warned about it.
 +  expect_single_log_msg_containing(
 +           "There is a router named \"$painauchocolat\" in my declared "
 +           "family, but that isn't a legal digest or nickname. Skipping it.");
 +  CLEAR();
 +
 +  // Let's introduce a node we can look up by nickname
 +  memset(&fake_node, 0, sizeof(fake_node));
 +  memcpy(fake_node.identity, "whydoyouasknonononon", DIGEST_LEN);
 +  MOCK(node_get_by_nickname, mock_node_get_by_nickname);
 +
 +  config_line_append(&options->MyFamily, "MyFamily",
 +                     "CRUmpeT");
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_NE, NULL);
 +  tt_int_op(smartlist_len(sl), OP_EQ, 5);
 +  join = smartlist_join_strings(sl, " ", 0, NULL);
 +  tt_str_op(join, OP_EQ,
 +            "$0123456789ABCDEF0123456789ABCDEF01234567 "
 +            "$686F6C65696E746865626F74746F6D6F66746865 "
 +            "$776879646F796F7561736B6E6F6E6F6E6F6E6F6E "
 +            "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA "
 +            "bagel");
 +  // "BAGEL" is still there, but it won't make a warning, because we already
 +  // warned about it.  Some with "$painauchocolat".
 +  expect_single_log_msg_containing(
 +           "There is a router named \"CRUmpeT\" in my declared "
 +           "family, but it wasn't listed by digest. Please consider saying "
 +           "$776879646F796F7561736B6E6F6E6F6E6F6E6F6E instead, if that's "
 +           "what you meant.");
 +  CLEAR();
 +  UNMOCK(node_get_by_nickname);
 +
 +  // Try a singleton list containing only us: It should give us NULL.
 +  config_free_lines(options->MyFamily);
 +  config_line_append(&options->MyFamily, "MyFamily",
 +                     "$686F6C65696E746865626F74746F6D6F66746865");
 +  sl = get_my_declared_family(options);
 +  tt_ptr_op(sl, OP_EQ, NULL);
 +  expect_no_log_entry();
 +
 + done:
 +  or_options_free(options);
 +  teardown_capture_of_logs();
 +  CLEAR();
 +  UNMOCK(node_get_by_nickname);
 +
 +#undef CLEAR
 +}
 +
+ static smartlist_t *fake_connection_array = NULL;
+ static smartlist_t *
+ mock_get_connection_array(void)
+ {
+   return fake_connection_array;
+ }
+ 
+ static void
+ test_router_get_advertised_or_port(void *arg)
+ {
+   (void)arg;
+   int r, w=0, n=0;
+   char *msg=NULL;
+   or_options_t *opts = options_new();
+   listener_connection_t *listener = NULL;
+   tor_addr_port_t ipv6;
+ 
+   // Test one failing case of router_get_advertised_ipv6_or_ap().
+   router_get_advertised_ipv6_or_ap(opts, &ipv6);
+   tt_str_op(fmt_addrport(&ipv6.addr, ipv6.port), OP_EQ, "[::]:0");
+ 
+   // And one failing case of router_get_advertised_or_port().
+   tt_int_op(0, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET));
+   tt_int_op(0, OP_EQ, router_get_advertised_or_port(opts));
+ 
+   // Set up a couple of configured ports.
+   config_line_append(&opts->ORPort_lines, "ORPort", "[1234::5678]:auto");
+   config_line_append(&opts->ORPort_lines, "ORPort", "5.6.7.8:9999");
+   r = parse_ports(opts, 0, &msg, &n, &w);
+   tt_assert(r == 0);
+ 
+   // There are no listeners, so the "auto" case will turn up no results.
+   tt_int_op(0, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET6));
+   router_get_advertised_ipv6_or_ap(opts, &ipv6);
+   tt_str_op(fmt_addrport(&ipv6.addr, ipv6.port), OP_EQ, "[::]:0");
+ 
+   // This will return the matching value from the configured port.
+   tt_int_op(9999, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET));
+   tt_int_op(9999, OP_EQ, router_get_advertised_or_port(opts));
+ 
+   // Now set up a dummy listener.
+   MOCK(get_connection_array, mock_get_connection_array);
+   fake_connection_array = smartlist_new();
+   listener = listener_connection_new(CONN_TYPE_OR_LISTENER, AF_INET6);
+   TO_CONN(listener)->port = 54321;
+   smartlist_add(fake_connection_array, TO_CONN(listener));
+ 
+   // We should get a port this time.
+   tt_int_op(54321, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET6));
+ 
+   // Test one succeeding case of router_get_advertised_ipv6_or_ap().
+   router_get_advertised_ipv6_or_ap(opts, &ipv6);
+   tt_str_op(fmt_addrport(&ipv6.addr, ipv6.port), OP_EQ,
+             "[1234::5678]:54321");
+ 
+   // This will return the matching value from the configured port.
+   tt_int_op(9999, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET));
+   tt_int_op(9999, OP_EQ, router_get_advertised_or_port(opts));
+ 
+  done:
+   or_options_free(opts);
+   config_free_all();
+   smartlist_free(fake_connection_array);
+   connection_free_minimal(TO_CONN(listener));
+   UNMOCK(get_connection_array);
+ }
+ 
+ static void
+ test_router_get_advertised_or_port_localhost(void *arg)
+ {
+   (void)arg;
+   int r, w=0, n=0;
+   char *msg=NULL;
+   or_options_t *opts = options_new();
+   tor_addr_port_t ipv6;
+ 
+   // Set up a couple of configured ports on localhost.
+   config_line_append(&opts->ORPort_lines, "ORPort", "[::1]:9999");
+   config_line_append(&opts->ORPort_lines, "ORPort", "127.0.0.1:8888");
+   r = parse_ports(opts, 0, &msg, &n, &w);
+   tt_assert(r == 0);
+ 
+   // We should refuse to advertise them, since we have default dirauths.
+   router_get_advertised_ipv6_or_ap(opts, &ipv6);
+   tt_str_op(fmt_addrport(&ipv6.addr, ipv6.port), OP_EQ, "[::]:0");
+   // But the lower-level function should still report the correct value
+   tt_int_op(9999, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET6));
+ 
+   // The IPv4 checks are done in resolve_my_address(), which doesn't use
+   // ORPorts so we can't test them here. (See #33681.) Both these lower-level
+   // functions should still report the correct value.
+   tt_int_op(8888, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET));
+   tt_int_op(8888, OP_EQ, router_get_advertised_or_port(opts));
+ 
+   // Now try with a fake authority set up.
+   config_line_append(&opts->DirAuthorities, "DirAuthority",
+                      "127.0.0.1:1066 "
+                      "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
+ 
+   tt_int_op(9999, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET6));
+   router_get_advertised_ipv6_or_ap(opts, &ipv6);
+   tt_str_op(fmt_addrport(&ipv6.addr, ipv6.port), OP_EQ, "[::1]:9999");
+ 
+   tt_int_op(8888, OP_EQ, router_get_advertised_or_port_by_af(opts, AF_INET));
+   tt_int_op(8888, OP_EQ, router_get_advertised_or_port(opts));
+ 
+  done:
+   or_options_free(opts);
+   config_free_all();
+ }
+ 
  #define ROUTER_TEST(name, flags)                          \
    { #name, test_router_ ## name, flags, NULL, NULL }
  
  struct testcase_t router_tests[] = {
    ROUTER_TEST(check_descriptor_bandwidth_changed, TT_FORK),
    ROUTER_TEST(dump_router_to_string_no_bridge_distribution_method, TT_FORK),
 +  ROUTER_TEST(mark_if_too_old, TT_FORK),
 +  ROUTER_TEST(get_my_family, TT_FORK),
+   ROUTER_TEST(get_advertised_or_port, TT_FORK),
+   ROUTER_TEST(get_advertised_or_port_localhost, TT_FORK),
    END_OF_TESTCASES
  };



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
Prev by Author: [tor-commits] [tor/release-0.4.1] Extract key length check into a new function, and check more fields.
Next by Author: [tor-commits] [tor/release-0.3.5] Use >= consistently with max_bits.
Previous by thread: [tor-commits] [tor/master] Add a test for the localhost case.
Next by thread: [tor-commits] [tor/master] Merge branch 'bug32588_042' into bug32588_043
Index(es):
- Author
- Thread