Merge branch 'ticket20218_rebased_squashed' into ticket20218_merged

* ticket 32695 removed networkstatus_consensus_has_ipv6(),
  keep that change in master.
* ticket 20218 modifies the function name and comment for
  routerstatus_has_visibly_changed(), keep that change
  in ticket20218_rebased_squashed.
This commit is contained in:
teor 2020-01-20 15:50:54 +10:00
commit 3851128e88
No known key found for this signature in database
GPG Key ID: 10FEAA0E7075672A
6 changed files with 198 additions and 8 deletions

3
changes/ticket20218 Normal file
View File

@ -0,0 +1,3 @@
o Minor bugfixes (controller):
- In routerstatus_has_changed(), check all the fields that are output over the control port.
Fixes bug 20218; bugfix on 0.1.1.11-alpha

View File

@ -113,6 +113,8 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version,
if (format != NS_CONTROL_PORT) {
/* Blow up more or less nicely if we didn't get anything or not the
* thing we expected.
* This should be kept in sync with the function
* routerstatus_has_visibly_changed and the struct routerstatus_t
*/
if (!desc) {
char id[HEX_DIGEST_LEN+1];

View File

@ -102,6 +102,7 @@
#include "feature/nodelist/routerlist_st.h"
#include "feature/dirauth/vote_microdesc_hash_st.h"
#include "feature/nodelist/vote_routerstatus_st.h"
#include "routerstatus_st.h"
#ifdef HAVE_UNISTD_H
#include <unistd.h>
@ -1578,10 +1579,16 @@ networkstatus_consensus_is_already_downloading(const char *resource)
return answer;
}
/** Given two router status entries for the same router identity, return 1 if
* if the contents have changed between them. Otherwise, return 0. */
static int
routerstatus_has_changed(const routerstatus_t *a, const routerstatus_t *b)
/** Given two router status entries for the same router identity, return 1
* if the contents have changed between them. Otherwise, return 0.
* It only checks for fields that are output by control port.
* This should be kept in sync with the struct routerstatus_t
* and the printing function routerstatus_format_entry in
* NS_CONTROL_PORT mode.
**/
STATIC int
routerstatus_has_visibly_changed(const routerstatus_t *a,
const routerstatus_t *b)
{
tor_assert(tor_memeq(a->identity_digest, b->identity_digest, DIGEST_LEN));
@ -1600,9 +1607,14 @@ routerstatus_has_changed(const routerstatus_t *a, const routerstatus_t *b)
a->is_valid != b->is_valid ||
a->is_possible_guard != b->is_possible_guard ||
a->is_bad_exit != b->is_bad_exit ||
a->is_hs_dir != b->is_hs_dir;
// XXXX this function needs a huge refactoring; it has gotten out
// XXXX of sync with routerstatus_t, and it will do so again.
a->is_hs_dir != b->is_hs_dir ||
a->is_staledesc != b->is_staledesc ||
a->has_bandwidth != b->has_bandwidth ||
a->published_on != b->published_on ||
a->ipv6_orport != b->ipv6_orport ||
a->is_v2_dir != b->is_v2_dir ||
a->bandwidth_kb != b->bandwidth_kb ||
tor_addr_compare(&a->ipv6_addr, &b->ipv6_addr, CMP_EXACT);
}
/** Notify controllers of any router status entries that changed between
@ -1634,7 +1646,7 @@ notify_control_networkstatus_changed(const networkstatus_t *old_c,
tor_memcmp(rs_old->identity_digest,
rs_new->identity_digest, DIGEST_LEN),
smartlist_add(changed, (void*) rs_new)) {
if (routerstatus_has_changed(rs_old, rs_new))
if (routerstatus_has_visibly_changed(rs_old, rs_new))
smartlist_add(changed, (void*)rs_new);
} SMARTLIST_FOREACH_JOIN_END(rs_old, rs_new);

View File

@ -163,6 +163,8 @@ STATIC void warn_early_consensus(const networkstatus_t *c, const char *flavor,
extern networkstatus_t *current_ns_consensus;
extern networkstatus_t *current_md_consensus;
#endif /* defined(TOR_UNIT_TESTS) */
STATIC int routerstatus_has_visibly_changed(const routerstatus_t *a,
const routerstatus_t *b);
#endif /* defined(NETWORKSTATUS_PRIVATE) */
#endif /* !defined(TOR_NETWORKSTATUS_H) */

View File

@ -17,6 +17,10 @@
/** Contents of a single router entry in a network status object.
*/
struct routerstatus_t {
/* This should be kept in sync with the function
* routerstatus_has_visibly_changed and the printing function
* routerstatus_format_entry in NS_CONTROL_PORT mode.
*/
time_t published_on; /**< When was this router published? */
char nickname[MAX_NICKNAME_LEN+1]; /**< The nickname this router says it
* has. */

View File

@ -7,6 +7,7 @@
**/
#define NODELIST_PRIVATE
#define NETWORKSTATUS_PRIVATE
#include "core/or/or.h"
#include "lib/crypt_ops/crypto_rand.h"
@ -17,6 +18,8 @@
#include "feature/nodelist/torcert.h"
#include "core/or/extend_info_st.h"
#include "feature/dirauth/dirvote.h"
#include "feature/nodelist/fmt_routerstatus.h"
#include "feature/nodelist/microdesc_st.h"
#include "feature/nodelist/networkstatus_st.h"
#include "feature/nodelist/node_st.h"
@ -1246,6 +1249,169 @@ test_nodelist_router_get_verbose_nickname(void *arg)
return;
}
static void
test_nodelist_routerstatus_has_visibly_changed(void *arg)
{
(void)arg;
routerstatus_t rs_orig, rs;
char *fmt_orig = NULL, *fmt = NULL;
memset(&rs_orig, 0, sizeof(rs_orig));
strlcpy(rs_orig.nickname, "friendly", sizeof(rs_orig.nickname));
memcpy(rs_orig.identity_digest, "abcdefghijklmnopqrst", 20);
memcpy(rs_orig.descriptor_digest, "abcdefghijklmnopqrst", 20);
rs_orig.addr = 0x7f000001;
rs_orig.or_port = 3;
rs_orig.published_on = time(NULL);
rs_orig.has_bandwidth = 1;
rs_orig.bandwidth_kb = 20;
#define COPY() memcpy(&rs, &rs_orig, sizeof(rs))
#define FORMAT() \
STMT_BEGIN \
tor_free(fmt_orig); \
tor_free(fmt); \
fmt_orig = routerstatus_format_entry(&rs_orig, NULL, NULL, \
NS_CONTROL_PORT, \
ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD, \
NULL); \
fmt = routerstatus_format_entry(&rs, NULL, NULL, NS_CONTROL_PORT, \
ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD, \
NULL); \
tt_assert(fmt_orig); \
tt_assert(fmt); \
STMT_END
#define ASSERT_SAME() \
STMT_BEGIN \
tt_assert(! routerstatus_has_visibly_changed(&rs_orig, &rs)); \
FORMAT(); \
tt_str_op(fmt_orig, OP_EQ, fmt); \
COPY(); \
STMT_END
#define ASSERT_CHANGED() \
STMT_BEGIN \
tt_assert(routerstatus_has_visibly_changed(&rs_orig, &rs)); \
FORMAT(); \
tt_str_op(fmt_orig, OP_NE, fmt); \
COPY(); \
STMT_END
#define ASSERT_CHANGED_NO_FORMAT() \
STMT_BEGIN \
tt_assert(routerstatus_has_visibly_changed(&rs_orig, &rs)); \
COPY(); \
STMT_END
COPY();
ASSERT_SAME();
rs.addr = 0x7f000002;
ASSERT_CHANGED();
strlcpy(rs.descriptor_digest, "hello world", sizeof(rs.descriptor_digest));
ASSERT_CHANGED();
strlcpy(rs.nickname, "fr1end1y", sizeof(rs.nickname));
ASSERT_CHANGED();
rs.published_on += 3600;
ASSERT_CHANGED();
rs.or_port = 55;
ASSERT_CHANGED();
rs.dir_port = 9999;
ASSERT_CHANGED();
tor_addr_parse(&rs.ipv6_addr, "1234::56");
ASSERT_CHANGED();
tor_addr_parse(&rs_orig.ipv6_addr, "1234::56");
rs_orig.ipv6_orport = 99;
COPY();
rs.ipv6_orport = 22;
ASSERT_CHANGED();
rs.is_authority = 1;
ASSERT_CHANGED();
rs.is_exit = 1;
ASSERT_CHANGED();
rs.is_stable = 1;
ASSERT_CHANGED();
rs.is_fast = 1;
ASSERT_CHANGED();
rs.is_flagged_running = 1;
ASSERT_CHANGED();
// This option is obsolete and not actually formatted.
rs.is_named = 1;
ASSERT_CHANGED_NO_FORMAT();
// This option is obsolete and not actually formatted.
rs.is_unnamed = 1;
ASSERT_CHANGED_NO_FORMAT();
rs.is_valid = 1;
ASSERT_CHANGED();
rs.is_possible_guard = 1;
ASSERT_CHANGED();
rs.is_bad_exit = 1;
ASSERT_CHANGED();
rs.is_hs_dir = 1;
ASSERT_CHANGED();
rs.is_v2_dir = 1;
ASSERT_CHANGED();
rs.is_staledesc = 1;
ASSERT_CHANGED();
// Setting this to zero crashes us with an assertion failure in
// routerstatus_format_entry() if we don't have a descriptor.
rs.has_bandwidth = 0;
ASSERT_CHANGED_NO_FORMAT();
// Does not actually matter; not visible to controller.
rs.has_exitsummary = 1;
ASSERT_SAME();
// Does not actually matter; not visible to the controller.
rs.bw_is_unmeasured = 1;
ASSERT_SAME();
rs.bandwidth_kb = 2000;
ASSERT_CHANGED();
// not visible to the controller.
rs.has_guardfraction = 1;
rs.guardfraction_percentage = 22;
ASSERT_SAME();
// not visible to the controller.
rs_orig.has_guardfraction = 1;
rs_orig.guardfraction_percentage = 20;
COPY();
rs.guardfraction_percentage = 25;
ASSERT_SAME();
// not visible to the controller.
rs.exitsummary = (char*)"accept 1-2";
ASSERT_SAME();
done:
#undef COPY
#undef ASSERT_SAME
#undef ASSERT_CHANGED
tor_free(fmt_orig);
tor_free(fmt);
return;
}
#define NODE(name, flags) \
{ #name, test_nodelist_##name, (flags), NULL, NULL }
@ -1266,5 +1432,6 @@ struct testcase_t nodelist_tests[] = {
NODE(routerstatus_describe, 0),
NODE(extend_info_describe, 0),
NODE(router_get_verbose_nickname, 0),
NODE(routerstatus_has_visibly_changed, 0),
END_OF_TESTCASES
};