commit c75bf388b5fc3555dd21d44b7856358f771292a4 Author: teor (Tim Wilson-Brown) <teor2345@gmail.com> Date: Sat May 7 10:18:52 2016 -0700 Warn users when addresses in ports and descriptor are inconsistent This mitigates bug 13953. --- changes/bug13953 | 3 ++ src/or/config.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/config.h | 6 ++++ src/or/router.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+) diff --git a/changes/bug13953 b/changes/bug13953 new file mode 100644 index 0000000..42b013f --- /dev/null +++ b/changes/bug13953 @@ -0,0 +1,3 @@ + o Minor bugfixes (config): + - Warn users when descriptor and port addresses are inconsistent. + Mitigates bug 13953; patch by teor. diff --git a/src/or/config.c b/src/or/config.c index 7ca9277..2fd97de 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -7037,6 +7037,87 @@ get_first_advertised_port_by_type_af(int listener_type, int address_family) return 0; } +/** Return the first advertised address of type <b>listener_type</b> in + * <b>address_family</b>. Returns NULL if there is no advertised address, + * and when passed AF_UNSPEC. */ +const tor_addr_t * +get_first_advertised_addr_by_type_af(int listener_type, int address_family) +{ + if (address_family == AF_UNSPEC) + return NULL; + if (!configured_ports) + return NULL; + SMARTLIST_FOREACH_BEGIN(configured_ports, const port_cfg_t *, cfg) { + if (cfg->type == listener_type && + !cfg->server_cfg.no_advertise) { + if ((address_family == AF_INET && port_binds_ipv4(cfg)) || + (address_family == AF_INET6 && port_binds_ipv6(cfg))) { + return &cfg->addr; + } + } + } SMARTLIST_FOREACH_END(cfg); + return NULL; +} + +/** Return 1 if a port exists of type <b>listener_type</b> on <b>addr</b> and + * <b>port</b>. If <b>check_wildcard</b> is true, INADDR[6]_ANY and AF_UNSPEC + * addresses match any address of the appropriate family; and port -1 matches + * any port. + * To match auto ports, pass CFG_PORT_AUTO. (Does not match on the actual + * automatically chosen listener ports.) */ +int +port_exists_by_type_addr_port(int listener_type, const tor_addr_t *addr, + int port, int check_wildcard) +{ + if (!configured_ports || !addr) + return 0; + SMARTLIST_FOREACH_BEGIN(configured_ports, const port_cfg_t *, cfg) { + if (cfg->type == listener_type) { + if (cfg->port == port || (check_wildcard && port == -1)) { + /* Exact match */ + if (tor_addr_eq(&cfg->addr, addr)) { + return 1; + } + /* Skip wildcard matches if we're not doing them */ + if (!check_wildcard) { + continue; + } + /* Wildcard matches IPv4 */ + const int cfg_v4 = port_binds_ipv4(cfg); + const int cfg_any_v4 = tor_addr_is_null(&cfg->addr) && cfg_v4; + const int addr_v4 = tor_addr_family(addr) == AF_INET || + tor_addr_family(addr) == AF_UNSPEC; + const int addr_any_v4 = tor_addr_is_null(&cfg->addr) && addr_v4; + if ((cfg_any_v4 && addr_v4) || (cfg_v4 && addr_any_v4)) { + return 1; + } + /* Wildcard matches IPv6 */ + const int cfg_v6 = port_binds_ipv6(cfg); + const int cfg_any_v6 = tor_addr_is_null(&cfg->addr) && cfg_v6; + const int addr_v6 = tor_addr_family(addr) == AF_INET6 || + tor_addr_family(addr) == AF_UNSPEC; + const int addr_any_v6 = tor_addr_is_null(&cfg->addr) && addr_v6; + if ((cfg_any_v6 && addr_v6) || (cfg_v6 && addr_any_v6)) { + return 1; + } + } + } + } SMARTLIST_FOREACH_END(cfg); + return 0; +} + +/* Like port_exists_by_type_addr_port, but accepts a host-order IPv4 address + * instead. */ +int +port_exists_by_type_addr32h_port(int listener_type, uint32_t addr_ipv4h, + int port, int check_wildcard) +{ + tor_addr_t ipv4; + tor_addr_from_ipv4h(&ipv4, addr_ipv4h); + return port_exists_by_type_addr_port(listener_type, &ipv4, port, + check_wildcard); +} + /** Adjust the value of options->DataDirectory, or fill it in if it's * absent. Return 0 on success, -1 on failure. */ static int diff --git a/src/or/config.h b/src/or/config.h index 02121cf..168e7d9 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -87,6 +87,12 @@ int get_first_advertised_port_by_type_af(int listener_type, (get_first_advertised_port_by_type_af(CONN_TYPE_OR_LISTENER, AF_INET)) #define get_primary_dir_port() \ (get_first_advertised_port_by_type_af(CONN_TYPE_DIR_LISTENER, AF_INET)) +const tor_addr_t *get_first_advertised_addr_by_type_af(int listener_type, + int address_family); +int port_exists_by_type_addr_port(int listener_type, const tor_addr_t *addr, + int port, int check_wildcard); +int port_exists_by_type_addr32h_port(int listener_type, uint32_t addr_ipv4h, + int port, int check_wildcard); char *get_first_listener_addrport_string(int listener_type); diff --git a/src/or/router.c b/src/or/router.c index 68bcf13..d48bd05 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1896,6 +1896,85 @@ router_pick_published_address,(const or_options_t *options, uint32_t *addr)) return 0; } +/* Tor relays only have one IPv4 address in the descriptor, which is derived + * from the Address torrc option, or guessed using various methods in + * router_pick_published_address(). + * Warn the operator if there is no ORPort on the descriptor address + * ipv4h_desc_addr. + * Warn the operator if there is no DirPort on the descriptor address. + * This catches a few common config errors: + * - operators who expect ORPorts and DirPorts to be advertised on the + * ports' listen addresses, rather than the torrc Address (or guessed + * addresses in the absence of an Address config). This includes + * operators who attempt to put their ORPort and DirPort on different + * addresses; + * - discrepancies between guessed addresses and configured listen + * addresses (when the Address option isn't set). + * If a listener is listening on all IPv4 addresses, it is assumed that it + * is listening on the configured Address, and no messages are logged. + * If an operators has specified NoAdvertise ORPorts in a NAT setting, + * no messages are logged, unless they have specified other advertised + * addresses. + * The message tells operators to configure an ORPort and DirPort that match + * the Address (using NoListen if needed). + */ +static void +router_check_descriptor_address_consistency(uint32_t ipv4h_desc_addr) +{ + + /* The first configured ORPort and DirPort, which may be CFG_AUTO_PORT. */ + int orport_v4_cfg = get_first_advertised_port_by_type_af( + CONN_TYPE_OR_LISTENER, + AF_INET); + int dirport_v4_cfg = get_first_advertised_port_by_type_af( + CONN_TYPE_DIR_LISTENER, + AF_INET); + + if (orport_v4_cfg != 0 && + !port_exists_by_type_addr32h_port(CONN_TYPE_OR_LISTENER, + ipv4h_desc_addr, orport_v4_cfg, 1)) { + const tor_addr_t *port_addr = get_first_advertised_addr_by_type_af( + CONN_TYPE_OR_LISTENER, + AF_INET); + tor_addr_t desc_addr; + char port_addr_str[TOR_ADDR_BUF_LEN]; + char desc_addr_str[TOR_ADDR_BUF_LEN]; + + tor_addr_to_str(port_addr_str, port_addr, TOR_ADDR_BUF_LEN, 0); + + tor_addr_from_ipv4h(&desc_addr, ipv4h_desc_addr); + tor_addr_to_str(desc_addr_str, &desc_addr, TOR_ADDR_BUF_LEN, 0); + + log_warn(LD_CONFIG, "The configured IPv4 ORPort address %s does not " + "match the address %s in the descriptor. Please configure " + "matching IPv4 addresses for the Address and ORPort options. " + "Use NoListen on the ORPort if you are behind a NAT.", + port_addr_str, desc_addr_str); + } + + if (dirport_v4_cfg != 0 && + !port_exists_by_type_addr32h_port(CONN_TYPE_DIR_LISTENER, + ipv4h_desc_addr, dirport_v4_cfg, 1)) { + const tor_addr_t *port_addr = get_first_advertised_addr_by_type_af( + CONN_TYPE_DIR_LISTENER, + AF_INET); + tor_addr_t desc_addr; + char port_addr_str[TOR_ADDR_BUF_LEN]; + char desc_addr_str[TOR_ADDR_BUF_LEN]; + + tor_addr_to_str(port_addr_str, port_addr, TOR_ADDR_BUF_LEN, 0); + + tor_addr_from_ipv4h(&desc_addr, ipv4h_desc_addr); + tor_addr_to_str(desc_addr_str, &desc_addr, TOR_ADDR_BUF_LEN, 0); + + log_warn(LD_CONFIG, "The configured IPv4 DirPort address %s does not " + "match the address %s in the descriptor. Please configure " + "matching IPv4 addresses for the Address and DirPort options. " + "Use NoListen on the DirPort if you are behind a NAT.", + port_addr_str, desc_addr_str); + } +} + /** Build a fresh routerinfo, signed server descriptor, and extra-info document * for this OR. Set r to the generated routerinfo, e to the generated * extra-info document. Return 0 on success, -1 on temporary error. Failure to @@ -1918,6 +1997,10 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) return -1; } + /* Log a message if the address in the descriptor doesn't match the ORPort + * and DirPort addresses configured by the operator. */ + router_check_descriptor_address_consistency(addr); + ri = tor_malloc_zero(sizeof(routerinfo_t)); ri->cache_info.routerlist_index = -1; ri->nickname = tor_strdup(options->Nickname);