commit 4460feaf2850ef0fb027a2d01786a5bbaee056dc Author: teor (Tim Wilson-Brown) <teor2345@gmail.com> Date: Tue Dec 22 10:42:09 2015 +1100 Fix *_get_all_orports to use ipv6_orport node_get_all_orports and router_get_all_orports incorrectly used or_port with IPv6 addresses. They now use ipv6_orport. Also refactor and remove duplicated code. --- src/common/address.c | 53 ++++++++++++++++++++++++++++++++++++ src/common/address.h | 21 +++++++++++++++ src/or/nodelist.c | 76 ++++++++++++++++++++++++++++++++++++++++------------ src/or/router.c | 26 +++++------------- 4 files changed, 140 insertions(+), 36 deletions(-) diff --git a/src/common/address.c b/src/common/address.c index 69a8098..19e9fdd 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -908,6 +908,59 @@ tor_addr_is_loopback(const tor_addr_t *addr) } } +/* Is addr valid? + * Checks that addr is non-NULL and not tor_addr_is_null(). + * If for_listening is true, IPv4 addr 0.0.0.0 is allowed. + * It means "bind to all addresses on the local machine". */ +int +tor_addr_is_valid(const tor_addr_t *addr, int for_listening) +{ + /* NULL addresses are invalid regardless of for_listening */ + if (addr == NULL) { + return 0; + } + + /* Only allow IPv4 0.0.0.0 for_listening. */ + if (for_listening && addr->family == AF_INET + && tor_addr_to_ipv4h(addr) == 0) { + return 1; + } + + /* Otherwise, the address is valid if it's not tor_addr_is_null() */ + return !tor_addr_is_null(addr); +} + +/* Is the network-order IPv4 address v4n_addr valid? + * Checks that addr is not zero. + * Except if for_listening is true, where IPv4 addr 0.0.0.0 is allowed. */ +int +tor_addr_is_valid_ipv4n(uint32_t v4n_addr, int for_listening) +{ + /* Any IPv4 address is valid with for_listening. */ + if (for_listening) { + return 1; + } + + /* Otherwise, zero addresses are invalid. */ + return v4n_addr != 0; +} + +/* Is port valid? + * Checks that port is not 0. + * Except if for_listening is true, where port 0 is allowed. + * It means "OS chooses a port". */ +int +tor_port_is_valid(uint16_t port, int for_listening) +{ + /* Any port value is valid with for_listening. */ + if (for_listening) { + return 1; + } + + /* Otherwise, zero ports are invalid. */ + return port != 0; +} + /** Set <b>dest</b> to equal the IPv4 address in <b>v4addr</b> (given in * network order). */ void diff --git a/src/common/address.h b/src/common/address.h index 684ba65..918b024 100644 --- a/src/common/address.h +++ b/src/common/address.h @@ -267,6 +267,27 @@ void tor_addr_from_in6(tor_addr_t *dest, const struct in6_addr *in6); int tor_addr_is_null(const tor_addr_t *addr); int tor_addr_is_loopback(const tor_addr_t *addr); +int tor_addr_is_valid(const tor_addr_t *addr, int for_listening); +int tor_addr_is_valid_ipv4n(uint32_t v4n_addr, int for_listening); +#define tor_addr_is_valid_ipv4h(v4h_addr, for_listening) \ + tor_addr_is_valid_ipv4n(htonl(v4h_addr), (for_listening)) +int tor_port_is_valid(uint16_t port, int for_listening); +/* Are addr and port both valid? */ +#define tor_addr_port_is_valid(addr, port, for_listening) \ + (tor_addr_is_valid((addr), (for_listening)) && \ + tor_port_is_valid((port), (for_listening))) +/* Are ap->addr and ap->port both valid? */ +#define tor_addr_port_is_valid_ap(ap, for_listening) \ + tor_addr_port_is_valid(&(ap)->addr, (ap)->port, (for_listening)) +/* Are the network-order v4addr and port both valid? */ +#define tor_addr_port_is_valid_ipv4n(v4n_addr, port, for_listening) \ + (tor_addr_is_valid_ipv4n((v4n_addr), (for_listening)) && \ + tor_port_is_valid((port), (for_listening))) +/* Are the host-order v4addr and port both valid? */ +#define tor_addr_port_is_valid_ipv4h(v4h_addr, port, for_listening) \ + (tor_addr_is_valid_ipv4h((v4h_addr), (for_listening)) && \ + tor_port_is_valid((port), (for_listening))) + int tor_addr_port_split(int severity, const char *addrport, char **address_out, uint16_t *port_out); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index fc27207..a1d99e9 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -754,6 +754,40 @@ node_exit_policy_is_exact(const node_t *node, sa_family_t family) return 1; } +/* Check if the "addr" and port_field fields from r are a valid non-listening + * address/port. If so, set valid to true and add a newly allocated + * tor_addr_port_t containing "addr" and port_field to sl. + * "addr" is an IPv4 host-order address and port_field is a uint16_t. + * r is typically a routerinfo_t or routerstatus_t. + */ +#define SL_ADD_NEW_IPV4_AP(r, port_field, sl, valid) \ + STMT_BEGIN \ + if (tor_addr_port_is_valid_ipv4h((r)->addr, (r)->port_field, 0)) { \ + valid = 1; \ + tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); \ + tor_addr_from_ipv4h(&ap->addr, (r)->addr); \ + ap->port = (r)->port_field; \ + smartlist_add((sl), ap); \ + } \ + STMT_END + +/* Check if the "addr" and port_field fields from r are a valid non-listening + * address/port. If so, set valid to true and add a newly allocated + * tor_addr_port_t containing "addr" and port_field to sl. + * "addr" is a tor_addr_t and port_field is a uint16_t. + * r is typically a routerinfo_t or routerstatus_t. + */ +#define SL_ADD_NEW_IPV6_AP(r, port_field, sl, valid) \ + STMT_BEGIN \ + if (tor_addr_port_is_valid(&(r)->ipv6_addr, (r)->port_field, 0)) { \ + valid = 1; \ + tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); \ + tor_addr_copy(&ap->addr, &(r)->ipv6_addr); \ + ap->port = (r)->port_field; \ + smartlist_add((sl), ap); \ + } \ + STMT_END + /** Return list of tor_addr_port_t with all OR ports (in the sense IP * addr + TCP port) for <b>node</b>. Caller must free all elements * using tor_free() and free the list using smartlist_free(). @@ -766,30 +800,38 @@ smartlist_t * node_get_all_orports(const node_t *node) { smartlist_t *sl = smartlist_new(); + int valid = 0; + /* Find a valid IPv4 address and port */ if (node->ri != NULL) { - if (node->ri->addr != 0) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_from_ipv4h(&ap->addr, node->ri->addr); - ap->port = node->ri->or_port; - smartlist_add(sl, ap); - } - if (!tor_addr_is_null(&node->ri->ipv6_addr)) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_copy(&ap->addr, &node->ri->ipv6_addr); - ap->port = node->ri->or_port; - smartlist_add(sl, ap); - } - } else if (node->rs != NULL) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_from_ipv4h(&ap->addr, node->rs->addr); - ap->port = node->rs->or_port; - smartlist_add(sl, ap); + SL_ADD_NEW_IPV4_AP(node->ri, or_port, sl, valid); + } + + /* If we didn't find a valid address/port in the ri, try the rs */ + if (!valid && node->rs != NULL) { + SL_ADD_NEW_IPV4_AP(node->rs, or_port, sl, valid); + } + + /* Find a valid IPv6 address and port */ + valid = 0; + if (node->ri != NULL) { + SL_ADD_NEW_IPV6_AP(node->ri, ipv6_orport, sl, valid); + } + + if (!valid && node->rs != NULL) { + SL_ADD_NEW_IPV6_AP(node->rs, ipv6_orport, sl, valid); + } + + if (!valid && node->md != NULL) { + SL_ADD_NEW_IPV6_AP(node->md, ipv6_orport, sl, valid); } return sl; } +#undef SL_ADD_NEW_IPV4_AP +#undef SL_ADD_NEW_IPV6_AP + /** Wrapper around node_get_prim_orport for backward compatibility. */ void diff --git a/src/or/router.c b/src/or/router.c index c35f629..ba32d77 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -3350,28 +3350,16 @@ router_free_all(void) /** Return a smartlist of tor_addr_port_t's with all the OR ports of <b>ri</b>. Note that freeing of the items in the list as well as - the smartlist itself is the callers responsibility. - - XXX duplicating code from node_get_all_orports(). */ + the smartlist itself is the callers responsibility. */ smartlist_t * router_get_all_orports(const routerinfo_t *ri) { - smartlist_t *sl = smartlist_new(); tor_assert(ri); - - if (ri->addr != 0) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_from_ipv4h(&ap->addr, ri->addr); - ap->port = ri->or_port; - smartlist_add(sl, ap); - } - if (!tor_addr_is_null(&ri->ipv6_addr)) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_copy(&ap->addr, &ri->ipv6_addr); - ap->port = ri->or_port; - smartlist_add(sl, ap); - } - - return sl; + node_t fake_node; + memset(&fake_node, 0, sizeof(fake_node)); + /* we don't modify ri, fake_node is passed as a const node_t * + */ + fake_node.ri = (routerinfo_t *)ri; + return node_get_all_orports(&fake_node); }