From 8a10c0696da526ff29076cba536633d5c2483439 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 19 Aug 2019 16:36:58 +1000 Subject: [PATCH 1/7] test/nodelist: Fix a typo --- src/test/test_nodelist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_nodelist.c b/src/test/test_nodelist.c index 8d6d3cb974..23b015c8c6 100644 --- a/src/test/test_nodelist.c +++ b/src/test/test_nodelist.c @@ -76,7 +76,7 @@ test_nodelist_node_get_verbose_nickname_not_named(void *arg) } /** A node should be considered a directory server if it has an open dirport - * of it accepts tunnelled directory requests. + * or it accepts tunnelled directory requests. */ static void test_nodelist_node_is_dir(void *arg) From 81256c079f0939e1046aa1e905fe488f53c37ded Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 19 Aug 2019 16:26:28 +1000 Subject: [PATCH 2/7] nodelist: Move router_get_verbose_nickname() declaration The declaration was in nodelist/routerinfo.h, but the implementation was in nodelist/describe.h. Part of 21003. --- src/feature/control/control_events.c | 2 +- src/feature/control/fmt_serverstatus.c | 2 +- src/feature/nodelist/describe.c | 1 - src/feature/nodelist/describe.h | 2 ++ src/feature/nodelist/routerinfo.h | 2 -- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c index 9e0966ca54..82ea943999 100644 --- a/src/feature/control/control_events.c +++ b/src/feature/control/control_events.c @@ -26,9 +26,9 @@ #include "feature/control/control_fmt.h" #include "feature/control/control_proto.h" #include "feature/dircommon/directory.h" +#include "feature/nodelist/describe.h" #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/nodelist.h" -#include "feature/nodelist/routerinfo.h" #include "feature/control/control_connection_st.h" #include "core/or/entry_connection_st.h" diff --git a/src/feature/control/fmt_serverstatus.c b/src/feature/control/fmt_serverstatus.c index a80bf50ad9..33c5ba1336 100644 --- a/src/feature/control/fmt_serverstatus.c +++ b/src/feature/control/fmt_serverstatus.c @@ -9,8 +9,8 @@ #include "app/config/config.h" #include "feature/dirauth/authmode.h" #include "feature/dirauth/voteflags.h"// XXXX remove +#include "feature/nodelist/describe.h" #include "feature/nodelist/nodelist.h" -#include "feature/nodelist/routerinfo.h" #include "feature/nodelist/node_st.h" #include "feature/nodelist/routerinfo_st.h" diff --git a/src/feature/nodelist/describe.c b/src/feature/nodelist/describe.c index 5c376408c0..5736691857 100644 --- a/src/feature/nodelist/describe.c +++ b/src/feature/nodelist/describe.c @@ -11,7 +11,6 @@ #include "core/or/or.h" #include "feature/nodelist/describe.h" -#include "feature/nodelist/routerinfo.h" #include "core/or/extend_info_st.h" #include "feature/nodelist/node_st.h" diff --git a/src/feature/nodelist/describe.h b/src/feature/nodelist/describe.h index d29192200e..05f52c47c9 100644 --- a/src/feature/nodelist/describe.h +++ b/src/feature/nodelist/describe.h @@ -22,4 +22,6 @@ const char *node_describe(const struct node_t *node); const char *router_describe(const struct routerinfo_t *ri); const char *routerstatus_describe(const struct routerstatus_t *ri); +void router_get_verbose_nickname(char *buf, const routerinfo_t *router); + #endif /* !defined(TOR_DESCRIBE_H) */ diff --git a/src/feature/nodelist/routerinfo.h b/src/feature/nodelist/routerinfo.h index ca66e660b3..8465060f93 100644 --- a/src/feature/nodelist/routerinfo.h +++ b/src/feature/nodelist/routerinfo.h @@ -17,8 +17,6 @@ void router_get_prim_orport(const routerinfo_t *router, int router_has_orport(const routerinfo_t *router, const tor_addr_port_t *orport); -void router_get_verbose_nickname(char *buf, const routerinfo_t *router); - smartlist_t *router_get_all_orports(const routerinfo_t *ri); const char *router_purpose_to_string(uint8_t p); From 03be44e216ed9e4cdba66e120094329df71e7dce Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 19 Aug 2019 16:30:15 +1000 Subject: [PATCH 3/7] nodelist: Remove support for is_named in describe.c The Named flag is deprecated. Removing support from the formatting code simplifies the unit tests. Part of 21003. --- src/feature/nodelist/describe.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/feature/nodelist/describe.c b/src/feature/nodelist/describe.c index 5736691857..7924d9a7ae 100644 --- a/src/feature/nodelist/describe.c +++ b/src/feature/nodelist/describe.c @@ -28,8 +28,8 @@ /** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to * hold a human-readable description of a node with identity digest - * id_digest, named-status is_named, nickname nickname, - * and address addr or addr32h. + * id_digest, nickname nickname, and addresses addr32h and + * addr. * * The nickname and addr fields are optional and may be set to * NULL. The addr32h field is optional and may be set to 0. @@ -39,7 +39,6 @@ static const char * format_node_description(char *buf, const char *id_digest, - int is_named, const char *nickname, const tor_addr_t *addr, uint32_t addr32h) @@ -53,7 +52,7 @@ format_node_description(char *buf, base16_encode(buf+1, HEX_DIGEST_LEN+1, id_digest, DIGEST_LEN); cp = buf+1+HEX_DIGEST_LEN; if (nickname) { - buf[1+HEX_DIGEST_LEN] = is_named ? '=' : '~'; + buf[1+HEX_DIGEST_LEN] = '~'; strlcpy(buf+1+HEX_DIGEST_LEN+1, nickname, MAX_NICKNAME_LEN+1); cp += strlen(cp); } @@ -85,7 +84,6 @@ router_describe(const routerinfo_t *ri) return ""; return format_node_description(buf, ri->cache_info.identity_digest, - 0, ri->nickname, NULL, ri->addr); @@ -102,14 +100,12 @@ node_describe(const node_t *node) static char buf[NODE_DESC_BUF_LEN]; const char *nickname = NULL; uint32_t addr32h = 0; - int is_named = 0; if (!node) return ""; if (node->rs) { nickname = node->rs->nickname; - is_named = node->rs->is_named; addr32h = node->rs->addr; } else if (node->ri) { nickname = node->ri->nickname; @@ -118,7 +114,6 @@ node_describe(const node_t *node) return format_node_description(buf, node->identity, - is_named, nickname, NULL, addr32h); @@ -138,7 +133,6 @@ routerstatus_describe(const routerstatus_t *rs) return ""; return format_node_description(buf, rs->identity_digest, - rs->is_named, rs->nickname, NULL, rs->addr); @@ -158,7 +152,6 @@ extend_info_describe(const extend_info_t *ei) return ""; return format_node_description(buf, ei->identity_digest, - 0, ei->nickname, &ei->addr, 0); From 23844bc7399a3517af23fe7f6429333b9f66df9e Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 19 Aug 2019 16:35:24 +1000 Subject: [PATCH 4/7] nodelist: Add support for multiple addresses in describe.c Also prepare for unit tests. Part of 21003. --- src/feature/nodelist/describe.c | 58 ++++++++++++++++++++------------- src/feature/nodelist/describe.h | 30 +++++++++++++++++ 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/feature/nodelist/describe.c b/src/feature/nodelist/describe.c index 7924d9a7ae..b64d4335a6 100644 --- a/src/feature/nodelist/describe.c +++ b/src/feature/nodelist/describe.c @@ -9,6 +9,8 @@ * \brief Format short descriptions of relays. */ +#define DESCRIBE_PRIVATE + #include "core/or/or.h" #include "feature/nodelist/describe.h" @@ -16,15 +18,7 @@ #include "feature/nodelist/node_st.h" #include "feature/nodelist/routerinfo_st.h" #include "feature/nodelist/routerstatus_st.h" - -/** - * Longest allowed output of format_node_description, plus 1 character for - * NUL. This allows space for: - * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at" - * " [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]" - * plus a terminating NUL. - */ -#define NODE_DESC_BUF_LEN (MAX_VERBOSE_NICKNAME_LEN+4+TOR_ADDR_BUF_LEN) +#include "feature/nodelist/microdesc_st.h" /** Use buf (which must be at least NODE_DESC_BUF_LEN bytes long) to * hold a human-readable description of a node with identity digest @@ -32,11 +26,12 @@ * addr. * * The nickname and addr fields are optional and may be set to - * NULL. The addr32h field is optional and may be set to 0. + * NULL or the null address. The addr32h field is optional and may be + * set to 0. * - * Return a pointer to the front of buf. + * Return a pointer to the front of buf, or a string constant on error. */ -static const char * +STATIC const char * format_node_description(char *buf, const char *id_digest, const char *nickname, @@ -44,6 +39,7 @@ format_node_description(char *buf, uint32_t addr32h) { char *cp; + bool has_addr = addr && !tor_addr_is_null(addr); if (!buf) return ""; @@ -56,17 +52,24 @@ format_node_description(char *buf, strlcpy(buf+1+HEX_DIGEST_LEN+1, nickname, MAX_NICKNAME_LEN+1); cp += strlen(cp); } - if (addr32h || addr) { + if (addr32h || has_addr) { memcpy(cp, " at ", 4); cp += 4; - if (addr) { - tor_addr_to_str(cp, addr, TOR_ADDR_BUF_LEN, 0); - } else { - struct in_addr in; - in.s_addr = htonl(addr32h); - tor_inet_ntoa(&in, cp, INET_NTOA_BUF_LEN); - } } + if (addr32h) { + struct in_addr in; + in.s_addr = htonl(addr32h); + tor_inet_ntoa(&in, cp, INET_NTOA_BUF_LEN); + cp += strlen(cp); + } + if (addr32h && has_addr) { + memcpy(cp, " and ", 5); + cp += 5; + } + if (has_addr) { + tor_addr_to_str(cp, addr, TOR_ADDR_BUF_LEN, 1); + } + return buf; } @@ -85,7 +88,7 @@ router_describe(const routerinfo_t *ri) return format_node_description(buf, ri->cache_info.identity_digest, ri->nickname, - NULL, + &ri->ipv6_addr, ri->addr); } @@ -100,6 +103,7 @@ node_describe(const node_t *node) static char buf[NODE_DESC_BUF_LEN]; const char *nickname = NULL; uint32_t addr32h = 0; + const tor_addr_t *ipv6_addr = NULL; if (!node) return ""; @@ -107,15 +111,23 @@ node_describe(const node_t *node) if (node->rs) { nickname = node->rs->nickname; addr32h = node->rs->addr; + ipv6_addr = &node->rs->ipv6_addr; + /* Support consensus versions less than 28, when IPv6 addresses were in + * microdescs. This code can be removed when 0.2.9 is no longer supported, + * and the MIN_METHOD_FOR_NO_A_LINES_IN_MICRODESC macro is removed. */ + if (node->md && tor_addr_is_null(ipv6_addr)) { + ipv6_addr = &node->md->ipv6_addr; + } } else if (node->ri) { nickname = node->ri->nickname; addr32h = node->ri->addr; + ipv6_addr = &node->ri->ipv6_addr; } return format_node_description(buf, node->identity, nickname, - NULL, + ipv6_addr, addr32h); } @@ -134,7 +146,7 @@ routerstatus_describe(const routerstatus_t *rs) return format_node_description(buf, rs->identity_digest, rs->nickname, - NULL, + &rs->ipv6_addr, rs->addr); } diff --git a/src/feature/nodelist/describe.h b/src/feature/nodelist/describe.h index 05f52c47c9..6c0d6dc48d 100644 --- a/src/feature/nodelist/describe.h +++ b/src/feature/nodelist/describe.h @@ -24,4 +24,34 @@ const char *routerstatus_describe(const struct routerstatus_t *ri); void router_get_verbose_nickname(char *buf, const routerinfo_t *router); +#if defined(DESCRIBE_PRIVATE) || defined(TOR_UNIT_TESTS) + +/** + * Longest allowed output for an IPv4 address "255.255.255.255", with NO + * terminating NUL. + */ +#define IPV4_BUF_LEN_NO_NUL 15 + +/** + * Longest allowed output of format_node_description, plus 1 character for + * NUL. This allows space for: + * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at" + * " 255.255.255.255 and [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]" + * plus a terminating NUL. + */ +#define NODE_DESC_BUF_LEN \ + (MAX_VERBOSE_NICKNAME_LEN+4+IPV4_BUF_LEN_NO_NUL+5+TOR_ADDR_BUF_LEN) + +#endif /* defined(DESCRIBE_PRIVATE) || defined(TOR_UNIT_TESTS) */ + +#ifdef TOR_UNIT_TESTS + +STATIC const char *format_node_description(char *buf, + const char *id_digest, + const char *nickname, + const tor_addr_t *addr, + uint32_t addr32h); + +#endif /* defined(TOR_UNIT_TESTS) */ + #endif /* !defined(TOR_DESCRIBE_H) */ From e8ba094c3e23f332dba5f2d9794c5e4c100ba568 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 19 Aug 2019 16:36:20 +1000 Subject: [PATCH 5/7] nodelist: Clear buffers and detect more caller errors in describe.c Part of 21003. --- src/feature/nodelist/describe.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/feature/nodelist/describe.c b/src/feature/nodelist/describe.c index b64d4335a6..7ec18438c1 100644 --- a/src/feature/nodelist/describe.c +++ b/src/feature/nodelist/describe.c @@ -44,6 +44,13 @@ format_node_description(char *buf, if (!buf) return ""; + memset(buf, 0, NODE_DESC_BUF_LEN); + + if (!id_digest) { + memcpy(buf, "", 17); + return buf; + } + buf[0] = '$'; base16_encode(buf+1, HEX_DIGEST_LEN+1, id_digest, DIGEST_LEN); cp = buf+1+HEX_DIGEST_LEN; @@ -122,6 +129,8 @@ node_describe(const node_t *node) nickname = node->ri->nickname; addr32h = node->ri->addr; ipv6_addr = &node->ri->ipv6_addr; + } else { + return ""; } return format_node_description(buf, @@ -179,6 +188,16 @@ extend_info_describe(const extend_info_t *ei) void router_get_verbose_nickname(char *buf, const routerinfo_t *router) { + if (!buf) + return; + + memset(buf, 0, MAX_VERBOSE_NICKNAME_LEN+1); + + if (!router) { + memcpy(buf, "", 7); + return; + } + buf[0] = '$'; base16_encode(buf+1, HEX_DIGEST_LEN+1, router->cache_info.identity_digest, DIGEST_LEN); From 1ebff4d7f38272c9b66f0a4a631d7de4ac7a5c72 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 22 Aug 2019 23:51:53 +1000 Subject: [PATCH 6/7] test/nodelist: Add unit tests for describe.c Part of 21003. --- src/test/test_nodelist.c | 612 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 612 insertions(+) diff --git a/src/test/test_nodelist.c b/src/test/test_nodelist.c index 23b015c8c6..b8ca56bb81 100644 --- a/src/test/test_nodelist.c +++ b/src/test/test_nodelist.c @@ -10,11 +10,13 @@ #include "core/or/or.h" #include "lib/crypt_ops/crypto_rand.h" +#include "feature/nodelist/describe.h" #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/nodefamily.h" #include "feature/nodelist/nodelist.h" #include "feature/nodelist/torcert.h" +#include "core/or/extend_info_st.h" #include "feature/nodelist/microdesc_st.h" #include "feature/nodelist/networkstatus_st.h" #include "feature/nodelist/node_st.h" @@ -640,6 +642,610 @@ test_nodelist_nodefamily_canonicalize(void *arg) tor_free(c); } +/** format_node_description() should return + * "Fingerprint~Nickname at IPv4 and [IPv6]". + * The nickname and addresses are optional. + */ +static void +test_nodelist_format_node_description(void *arg) +{ + char mock_digest[DIGEST_LEN]; + char mock_nickname[MAX_NICKNAME_LEN+1]; + tor_addr_t mock_null_ip; + tor_addr_t mock_ipv4; + tor_addr_t mock_ipv6; + + char ndesc[NODE_DESC_BUF_LEN]; + const char *rv = NULL; + + (void) arg; + + /* Clear variables */ + memset(ndesc, 0, sizeof(ndesc)); + memset(mock_digest, 0, sizeof(mock_digest)); + memset(mock_nickname, 0, sizeof(mock_nickname)); + memset(&mock_null_ip, 0, sizeof(mock_null_ip)); + memset(&mock_ipv4, 0, sizeof(mock_ipv4)); + memset(&mock_ipv6, 0, sizeof(mock_ipv6)); + + /* Set variables */ + memcpy(mock_digest, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_digest)); + strlcpy(mock_nickname, "TestOR7890123456789", sizeof(mock_nickname)); + tor_addr_parse(&mock_ipv4, "111.222.233.244"); + tor_addr_parse(&mock_ipv6, "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* Test function with variables */ + rv = format_node_description(ndesc, + mock_digest, + NULL, + NULL, + 0); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(ndesc, OP_EQ, "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + + /* format node description should use ~ because named is deprecated */ + rv = format_node_description(ndesc, + mock_digest, + mock_nickname, + NULL, + 0); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(ndesc, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~""TestOR7890123456789"); + + /* Try a null IP address, rather than NULL */ + rv = format_node_description(ndesc, + mock_digest, + mock_nickname, + &mock_null_ip, + 0); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(ndesc, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789"); + + /* Try some real IP addresses */ + rv = format_node_description(ndesc, + mock_digest, + NULL, + &mock_ipv4, + 0); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(ndesc, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA at 111.222.233.244"); + + rv = format_node_description(ndesc, + mock_digest, + mock_nickname, + &mock_ipv6, + 0); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(ndesc, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + rv = format_node_description(ndesc, + mock_digest, + mock_nickname, + &mock_ipv6, + tor_addr_to_ipv4h(&mock_ipv4)); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(ndesc, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* test NULL handling */ + rv = format_node_description(NULL, NULL, NULL, NULL, 0); + tt_str_op(rv, OP_EQ, ""); + + rv = format_node_description(ndesc, NULL, NULL, NULL, 0); + tt_ptr_op(rv, OP_EQ, ndesc); + tt_str_op(rv, OP_EQ, ""); + + done: + return; +} + +/** router_describe() is a wrapper for format_node_description(), see that + * test for details. + * + * The routerinfo-only node_describe() tests are in this function, + * so we can re-use the same mocked variables. + */ +static void +test_nodelist_router_describe(void *arg) +{ + char mock_nickname[MAX_NICKNAME_LEN+1]; + tor_addr_t mock_ipv4; + routerinfo_t mock_ri_ipv4; + routerinfo_t mock_ri_ipv6; + routerinfo_t mock_ri_dual; + + const char *rv = NULL; + + (void) arg; + + /* Clear variables */ + memset(mock_nickname, 0, sizeof(mock_nickname)); + memset(&mock_ipv4, 0, sizeof(mock_ipv4)); + memset(&mock_ri_ipv4, 0, sizeof(mock_ri_ipv4)); + memset(&mock_ri_ipv6, 0, sizeof(mock_ri_ipv6)); + memset(&mock_ri_dual, 0, sizeof(mock_ri_dual)); + + /* Set up the dual-stack routerinfo */ + memcpy(mock_ri_dual.cache_info.identity_digest, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_ri_dual.cache_info.identity_digest)); + strlcpy(mock_nickname, "TestOR7890123456789", sizeof(mock_nickname)); + mock_ri_dual.nickname = mock_nickname; + tor_addr_parse(&mock_ipv4, "111.222.233.244"); + mock_ri_dual.addr = tor_addr_to_ipv4h(&mock_ipv4); + tor_addr_parse(&mock_ri_dual.ipv6_addr, + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* Create and modify the other routerinfos. + * mock_nickname is referenced from all 3 routerinfos. + * That's ok, all their memory is static. */ + memcpy(&mock_ri_ipv4, &mock_ri_dual, sizeof(mock_ri_ipv4)); + memcpy(&mock_ri_ipv6, &mock_ri_dual, sizeof(mock_ri_ipv6)); + /* Clear the unnecessary addresses */ + memset(&mock_ri_ipv4.ipv6_addr, 0, sizeof(mock_ri_ipv4.ipv6_addr)); + mock_ri_ipv6.addr = 0; + + /* We don't test the no-nickname and no-IP cases, because they're covered by + * format_node_description(), and we don't expect to see them in Tor code. */ + + /* Try some real IP addresses */ + rv = router_describe(&mock_ri_ipv4); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244"); + + rv = router_describe(&mock_ri_ipv6); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + rv = router_describe(&mock_ri_dual); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* test NULL handling */ + rv = router_describe(NULL); + tt_str_op(rv, OP_EQ, ""); + + /* Now test a node with only these routerinfos */ + node_t mock_node; + memset(&mock_node, 0, sizeof(mock_node)); + memcpy(mock_node.identity, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_node.identity)); + + /* Try some real IP addresses */ + mock_node.ri = &mock_ri_ipv4; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244"); + + mock_node.ri = &mock_ri_ipv6; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + mock_node.ri = &mock_ri_dual; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]"); + + done: + return; +} + +/** node_describe() is a wrapper for format_node_description(), see that + * test for details. + * + * The routerinfo-only and routerstatus-only node_describe() tests are in + * test_nodelist_router_describe() and test_nodelist_routerstatus_describe(), + * so we can re-use their mocked variables. + */ +static void +test_nodelist_node_describe(void *arg) +{ + char mock_nickname[MAX_NICKNAME_LEN+1]; + tor_addr_t mock_ipv4; + + const char *rv = NULL; + + (void) arg; + + /* Routerinfos */ + routerinfo_t mock_ri_dual; + + /* Clear variables */ + memset(mock_nickname, 0, sizeof(mock_nickname)); + memset(&mock_ipv4, 0, sizeof(mock_ipv4)); + memset(&mock_ri_dual, 0, sizeof(mock_ri_dual)); + + /* Set up the dual-stack routerinfo */ + memcpy(mock_ri_dual.cache_info.identity_digest, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_ri_dual.cache_info.identity_digest)); + strlcpy(mock_nickname, "TestOR7890123456789", sizeof(mock_nickname)); + mock_ri_dual.nickname = mock_nickname; + tor_addr_parse(&mock_ipv4, "111.222.233.244"); + mock_ri_dual.addr = tor_addr_to_ipv4h(&mock_ipv4); + tor_addr_parse(&mock_ri_dual.ipv6_addr, + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* Routerstatuses */ + routerstatus_t mock_rs_ipv4; + routerstatus_t mock_rs_dual; + + /* Clear variables */ + memset(&mock_ipv4, 0, sizeof(mock_ipv4)); + memset(&mock_rs_ipv4, 0, sizeof(mock_rs_ipv4)); + memset(&mock_rs_dual, 0, sizeof(mock_rs_dual)); + + /* Set up the dual-stack routerstatus */ + memcpy(mock_rs_dual.identity_digest, + "\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB" + "\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB", + sizeof(mock_rs_dual.identity_digest)); + strlcpy(mock_rs_dual.nickname, "Bbb", + sizeof(mock_rs_dual.nickname)); + tor_addr_parse(&mock_ipv4, "2.2.2.2"); + mock_rs_dual.addr = tor_addr_to_ipv4h(&mock_ipv4); + tor_addr_parse(&mock_rs_dual.ipv6_addr, + "[bbbb::bbbb]"); + + /* Create and modify the other routerstatus. */ + memcpy(&mock_rs_ipv4, &mock_rs_dual, sizeof(mock_rs_ipv4)); + /* Clear the unnecessary IPv6 address */ + memset(&mock_rs_ipv4.ipv6_addr, 0, sizeof(mock_rs_ipv4.ipv6_addr)); + + /* Microdescs */ + microdesc_t mock_md_null; + microdesc_t mock_md_ipv6; + + /* Clear variables */ + memset(&mock_md_null, 0, sizeof(mock_md_null)); + memset(&mock_md_ipv6, 0, sizeof(mock_md_ipv6)); + + /* Set up the microdesc */ + tor_addr_parse(&mock_md_ipv6.ipv6_addr, + "[eeee::6000:6000]"); + + /* Set up the node */ + node_t mock_node; + memset(&mock_node, 0, sizeof(mock_node)); + memcpy(mock_node.identity, + "\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC" + "\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC", + sizeof(mock_node.identity)); + + /* Test that the routerinfo and routerstatus work separately, but the + * identity comes from the node */ + mock_node.ri = &mock_ri_dual; + mock_node.rs = NULL; + mock_node.md = NULL; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~TestOR7890123456789 at " + "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]"); + + mock_node.ri = NULL; + mock_node.rs = &mock_rs_ipv4; + mock_node.md = NULL; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2"); + + mock_node.ri = NULL; + mock_node.rs = &mock_rs_dual; + mock_node.md = NULL; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [bbbb::bbbb]"); + + /* Test that the routerstatus overrides the routerinfo */ + mock_node.ri = &mock_ri_dual; + mock_node.rs = &mock_rs_ipv4; + mock_node.md = NULL; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2"); + + mock_node.ri = &mock_ri_dual; + mock_node.rs = &mock_rs_dual; + mock_node.md = NULL; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [bbbb::bbbb]"); + + /* Test that the microdesc IPv6 is used if the routerinfo doesn't have IPv6 + */ + mock_node.ri = NULL; + mock_node.rs = &mock_rs_ipv4; + mock_node.md = &mock_md_ipv6; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [eeee::6000:6000]"); + + mock_node.ri = NULL; + mock_node.rs = &mock_rs_ipv4; + mock_node.md = &mock_md_null; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2"); + + mock_node.ri = NULL; + mock_node.rs = &mock_rs_dual; + mock_node.md = &mock_md_ipv6; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [bbbb::bbbb]"); + + mock_node.ri = NULL; + mock_node.rs = &mock_rs_dual; + mock_node.md = &mock_md_null; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [bbbb::bbbb]"); + + /* Test that the routerinfo doesn't change the results above + */ + mock_node.ri = &mock_ri_dual; + mock_node.rs = &mock_rs_ipv4; + mock_node.md = &mock_md_ipv6; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [eeee::6000:6000]"); + + mock_node.ri = &mock_ri_dual; + mock_node.rs = &mock_rs_ipv4; + mock_node.md = &mock_md_null; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2"); + + mock_node.ri = &mock_ri_dual; + mock_node.rs = &mock_rs_dual; + mock_node.md = &mock_md_ipv6; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [bbbb::bbbb]"); + + mock_node.ri = &mock_ri_dual; + mock_node.rs = &mock_rs_dual; + mock_node.md = &mock_md_null; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC~Bbb at " + "2.2.2.2 and [bbbb::bbbb]"); + + /* test NULL handling */ + rv = node_describe(NULL); + tt_str_op(rv, OP_EQ, ""); + + mock_node.ri = NULL; + mock_node.rs = NULL; + mock_node.md = NULL; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + ""); + + done: + return; +} + +/** routerstatus_describe() is a wrapper for format_node_description(), see + * that test for details. + * + * The routerstatus-only node_describe() tests are in this function, + * so we can re-use the same mocked variables. + */ +static void +test_nodelist_routerstatus_describe(void *arg) +{ + tor_addr_t mock_ipv4; + routerstatus_t mock_rs_ipv4; + routerstatus_t mock_rs_ipv6; + routerstatus_t mock_rs_dual; + + const char *rv = NULL; + + (void) arg; + + /* Clear variables */ + memset(&mock_ipv4, 0, sizeof(mock_ipv4)); + memset(&mock_rs_ipv4, 0, sizeof(mock_rs_ipv4)); + memset(&mock_rs_ipv6, 0, sizeof(mock_rs_ipv6)); + memset(&mock_rs_dual, 0, sizeof(mock_rs_dual)); + + /* Set up the dual-stack routerstatus */ + memcpy(mock_rs_dual.identity_digest, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_rs_dual.identity_digest)); + strlcpy(mock_rs_dual.nickname, "TestOR7890123456789", + sizeof(mock_rs_dual.nickname)); + tor_addr_parse(&mock_ipv4, "111.222.233.244"); + mock_rs_dual.addr = tor_addr_to_ipv4h(&mock_ipv4); + tor_addr_parse(&mock_rs_dual.ipv6_addr, + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* Create and modify the other routerstatuses. */ + memcpy(&mock_rs_ipv4, &mock_rs_dual, sizeof(mock_rs_ipv4)); + memcpy(&mock_rs_ipv6, &mock_rs_dual, sizeof(mock_rs_ipv6)); + /* Clear the unnecessary addresses */ + memset(&mock_rs_ipv4.ipv6_addr, 0, sizeof(mock_rs_ipv4.ipv6_addr)); + mock_rs_ipv6.addr = 0; + + /* We don't test the no-nickname and no-IP cases, because they're covered by + * format_node_description(), and we don't expect to see them in Tor code. */ + + /* Try some real IP addresses */ + rv = routerstatus_describe(&mock_rs_ipv4); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244"); + + rv = routerstatus_describe(&mock_rs_ipv6); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + rv = routerstatus_describe(&mock_rs_dual); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* test NULL handling */ + rv = routerstatus_describe(NULL); + tt_str_op(rv, OP_EQ, ""); + + /* Now test a node with only these routerstatuses */ + node_t mock_node; + memset(&mock_node, 0, sizeof(mock_node)); + memcpy(mock_node.identity, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_node.identity)); + + /* Try some real IP addresses */ + mock_node.rs = &mock_rs_ipv4; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244"); + + mock_node.rs = &mock_rs_ipv6; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + mock_node.rs = &mock_rs_dual; + rv = node_describe(&mock_node); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]"); + + done: + return; +} + +/** extend_info_describe() is a wrapper for format_node_description(), see + * that test for details. + */ +static void +test_nodelist_extend_info_describe(void *arg) +{ + extend_info_t mock_ei_ipv4; + extend_info_t mock_ei_ipv6; + + const char *rv = NULL; + + (void) arg; + + /* Clear variables */ + memset(&mock_ei_ipv4, 0, sizeof(mock_ei_ipv4)); + memset(&mock_ei_ipv6, 0, sizeof(mock_ei_ipv6)); + + /* Set up the IPv4 extend info */ + memcpy(mock_ei_ipv4.identity_digest, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + sizeof(mock_ei_ipv4.identity_digest)); + strlcpy(mock_ei_ipv4.nickname, "TestOR7890123456789", + sizeof(mock_ei_ipv4.nickname)); + tor_addr_parse(&mock_ei_ipv4.addr, "111.222.233.244"); + + /* Create and modify the other extend info. */ + memcpy(&mock_ei_ipv6, &mock_ei_ipv4, sizeof(mock_ei_ipv6)); + tor_addr_parse(&mock_ei_ipv6.addr, + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* We don't test the no-nickname and no-IP cases, because they're covered by + * format_node_description(), and we don't expect to see them in Tor code. */ + + /* Try some real IP addresses */ + rv = extend_info_describe(&mock_ei_ipv4); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "111.222.233.244"); + + rv = extend_info_describe(&mock_ei_ipv6); + tt_str_op(rv, OP_EQ, + "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at " + "[1111:2222:3333:4444:5555:6666:7777:8888]"); + + /* Extend infos only have one IP address, so there is no dual case */ + + /* test NULL handling */ + rv = extend_info_describe(NULL); + tt_str_op(rv, OP_EQ, ""); + + done: + return; +} + +/** router_get_verbose_nickname() should return "Fingerprint~Nickname" + */ +static void +test_nodelist_router_get_verbose_nickname(void *arg) +{ + routerinfo_t mock_ri; + char mock_nickname[MAX_NICKNAME_LEN+1]; + + char vname[MAX_VERBOSE_NICKNAME_LEN+1]; + + (void) arg; + + memset(&mock_ri, 0, sizeof(routerinfo_t)); + memset(mock_nickname, 0, sizeof(mock_nickname)); + mock_ri.nickname = mock_nickname; + + /* verbose nickname should use ~ because named is deprecated */ + strlcpy(mock_nickname, "TestOR", sizeof(mock_nickname)); + memcpy(mock_ri.cache_info.identity_digest, + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" + "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", + DIGEST_LEN); + router_get_verbose_nickname(vname, &mock_ri); + tt_str_op(vname, OP_EQ, "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR"); + + /* test NULL router handling */ + router_get_verbose_nickname(vname, NULL); + tt_str_op(vname, OP_EQ, ""); + + router_get_verbose_nickname(NULL, &mock_ri); + router_get_verbose_nickname(NULL, NULL); + + done: + return; +} + #define NODE(name, flags) \ { #name, test_nodelist_##name, (flags), NULL, NULL } @@ -654,5 +1260,11 @@ struct testcase_t nodelist_tests[] = { NODE(nickname_matches, 0), NODE(node_nodefamily, TT_FORK), NODE(nodefamily_canonicalize, 0), + NODE(format_node_description, 0), + NODE(router_describe, 0), + NODE(node_describe, 0), + NODE(routerstatus_describe, 0), + NODE(extend_info_describe, 0), + NODE(router_get_verbose_nickname, 0), END_OF_TESTCASES }; From 5794523f800a2693f0b4a192821f5ac6dfbeb27c Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 23 Aug 2019 00:06:55 +1000 Subject: [PATCH 7/7] changes: file for 21003 Log IPv6 addresses as well as IPv4 addresses, when describing routerinfos, routerstatuses, and nodes. Closes ticket 21003. --- changes/ticket21003 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket21003 diff --git a/changes/ticket21003 b/changes/ticket21003 new file mode 100644 index 0000000000..896d7493eb --- /dev/null +++ b/changes/ticket21003 @@ -0,0 +1,3 @@ + o Minor features (IPv6, logging): + - Log IPv6 addresses as well as IPv4 addresses, when describing + routerinfos, routerstatuses, and nodes. Closes ticket 21003.