commit f478080bd08033473dfedcf0f51f0be767b985f5 Author: Nick Mathewson <nickm@torproject.org> Date: Tue Jul 21 12:34:56 2020 -0400 Combine router_get_advertised_or_port{,by_af_}() functions --- src/core/mainloop/connection.c | 9 ++++----- src/feature/dirauth/dirvote.c | 2 +- src/feature/relay/router.c | 31 +++++++++++-------------------- src/feature/relay/router.h | 5 ++--- src/test/test_router.c | 23 +++++++++-------------- 5 files changed, 27 insertions(+), 43 deletions(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index b3c5e6f51c..0fc5e6a88b 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -3168,9 +3168,9 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) smartlist_t *replacements = smartlist_new(); const or_options_t *options = get_options(); int retval = 0; - const uint16_t old_or_port = router_get_advertised_or_port(options); + const uint16_t old_or_port = router_get_advertised_or_port(options, AF_INET); const uint16_t old_or_port_ipv6 = - router_get_advertised_or_port_by_af(options,AF_INET6); + router_get_advertised_or_port(options,AF_INET6); const uint16_t old_dir_port = router_get_advertised_dir_port(options, 0); SMARTLIST_FOREACH_BEGIN(get_connection_array(), connection_t *, conn) { @@ -3241,9 +3241,8 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) SMARTLIST_FOREACH(replacements, listener_replacement_t *, r, tor_free(r)); smartlist_free(replacements); - if (old_or_port != router_get_advertised_or_port(options) || - old_or_port_ipv6 != router_get_advertised_or_port_by_af(options, - AF_INET6) || + if (old_or_port != router_get_advertised_or_port(options, AF_INET) || + old_or_port_ipv6 != router_get_advertised_or_port(options, AF_INET6) || old_dir_port != router_get_advertised_dir_port(options, 0)) { /* Our chosen ORPort or DirPort is not what it used to be: the * descriptor we had (if any) should be regenerated. (We won't diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index c067c9b85e..b0583a42f7 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -4725,7 +4725,7 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, voter->address = hostname; tor_addr_copy(&voter->ipv4_addr, &addr); voter->ipv4_dirport = router_get_advertised_dir_port(options, 0); - voter->ipv4_orport = router_get_advertised_or_port(options); + voter->ipv4_orport = router_get_advertised_or_port(options, AF_INET); voter->contact = tor_strdup(contact); if (options->V3AuthUseLegacyKey) { authority_cert_t *c = get_my_v3_legacy_cert(); diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 1ad5cbf5da..0f85db44d5 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -1154,7 +1154,7 @@ init_keys(void) router_get_advertised_ipv6_or_ap(options, &ipv6_orport); ds = trusted_dir_server_new(options->Nickname, NULL, router_get_advertised_dir_port(options, 0), - router_get_advertised_or_port(options), + router_get_advertised_or_port(options,AF_INET), &ipv6_orport, digest, v3_digest, @@ -1309,7 +1309,7 @@ decide_to_advertise_dir_impl(const or_options_t *options, if (dir_port && !router_get_advertised_dir_port(options, dir_port)) return 0; if (supports_tunnelled_dir_requests && - !router_get_advertised_or_port(options)) + !router_get_advertised_or_port(options, AF_INET)) return 0; /* Part two: consider config options that could make us choose to @@ -1390,7 +1390,7 @@ decide_if_publishable_server(void) return 0; if (authdir_mode(options)) return 1; - if (!router_get_advertised_or_port(options)) + if (!router_get_advertised_or_port(options, AF_INET)) return 0; if (!router_orport_seems_reachable(options, AF_INET)) { // We have an ipv4 orport, and it doesn't seem reachable. @@ -1459,20 +1459,12 @@ router_get_active_listener_port_by_type_af(int listener_type, return 0; } -/** Return the port that we should advertise as our ORPort; this is either - * the one configured in the ORPort option, or the one we actually bound to - * if ORPort is "auto". Returns 0 if no port is found. */ +/** Return the port that we should advertise as our ORPort in a given address + * family; this is either the one configured in the ORPort option, or the one + * we actually bound to if ORPort is "auto". Returns 0 if no port is found. */ uint16_t -router_get_advertised_or_port(const or_options_t *options) -{ - return router_get_advertised_or_port_by_af(options, AF_INET); -} - -/** As router_get_advertised_or_port(), but allows an address family argument. - */ -uint16_t -router_get_advertised_or_port_by_af(const or_options_t *options, - sa_family_t family) +router_get_advertised_or_port(const or_options_t *options, + sa_family_t family) { int port = portconf_get_first_advertised_port(CONN_TYPE_OR_LISTENER, family); @@ -1505,8 +1497,7 @@ router_get_advertised_ipv6_or_ap(const or_options_t *options, const tor_addr_t *addr = portconf_get_first_advertised_addr( CONN_TYPE_OR_LISTENER, AF_INET6); - const uint16_t port = router_get_advertised_or_port_by_af( - options, + const uint16_t port = router_get_advertised_or_port(options, AF_INET6); if (!addr || port == 0) { @@ -2065,13 +2056,13 @@ router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out)) /* IPv4. */ tor_addr_copy(&ri->ipv4_addr, &ipv4_addr); - ri->ipv4_orport = router_get_advertised_or_port_by_af(options, AF_INET); + ri->ipv4_orport = router_get_advertised_or_port(options, AF_INET); ri->ipv4_dirport = router_get_advertised_dir_port(options, 0); /* IPv6. Do not publish an IPv6 if we don't have an ORPort that can be used * with the address. This is possible for instance if the ORPort is * IPv4Only. */ - ipv6_orport = router_get_advertised_or_port_by_af(options, AF_INET6); + ipv6_orport = router_get_advertised_or_port(options, AF_INET6); if (have_v6 && ipv6_orport != 0) { tor_addr_copy(&ri->ipv6_addr, &ipv6_addr); ri->ipv6_orport = ipv6_orport; diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index fab109be7c..e53f1dd3e2 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -65,13 +65,12 @@ int init_keys_client(void); uint16_t router_get_active_listener_port_by_type_af(int listener_type, sa_family_t family); -uint16_t router_get_advertised_or_port(const or_options_t *options); void router_get_advertised_ipv6_or_ap(const or_options_t *options, tor_addr_port_t *ipv6_ap_out); bool router_has_advertised_ipv6_orport(const or_options_t *options); MOCK_DECL(bool, router_can_extend_over_ipv6,(const or_options_t *options)); -uint16_t router_get_advertised_or_port_by_af(const or_options_t *options, - sa_family_t family); +uint16_t router_get_advertised_or_port(const or_options_t *options, + sa_family_t family); uint16_t router_get_advertised_dir_port(const or_options_t *options, uint16_t dirport); diff --git a/src/test/test_router.c b/src/test/test_router.c index 6980a62e5e..d87f9f8acf 100644 --- a/src/test/test_router.c +++ b/src/test/test_router.c @@ -512,8 +512,7 @@ test_router_get_advertised_or_port(void *arg) 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)); + tt_int_op(0, OP_EQ, router_get_advertised_or_port(opts, AF_INET)); // Set up a couple of configured ports. config_line_append(&opts->ORPort_lines, "ORPort", "[1234::5678]:auto"); @@ -522,13 +521,12 @@ test_router_get_advertised_or_port(void *arg) 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)); + tt_int_op(0, OP_EQ, router_get_advertised_or_port(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)); + tt_int_op(9999, OP_EQ, router_get_advertised_or_port(opts, AF_INET)); // Now set up a dummy listener. MOCK(get_connection_array, mock_get_connection_array); @@ -538,7 +536,7 @@ test_router_get_advertised_or_port(void *arg) 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)); + tt_int_op(54321, OP_EQ, router_get_advertised_or_port(opts, AF_INET6)); // Test one succeeding case of router_get_advertised_ipv6_or_ap(). router_get_advertised_ipv6_or_ap(opts, &ipv6); @@ -546,8 +544,7 @@ test_router_get_advertised_or_port(void *arg) "[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)); + tt_int_op(9999, OP_EQ, router_get_advertised_or_port(opts, AF_INET)); done: or_options_free(opts); @@ -576,25 +573,23 @@ test_router_get_advertised_or_port_localhost(void *arg) 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)); + tt_int_op(9999, OP_EQ, router_get_advertised_or_port(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)); + tt_int_op(8888, OP_EQ, router_get_advertised_or_port(opts, AF_INET)); // 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)); + tt_int_op(9999, OP_EQ, router_get_advertised_or_port(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)); + tt_int_op(8888, OP_EQ, router_get_advertised_or_port(opts, AF_INET)); done: or_options_free(opts);