Commit bb2145b4 authored by Nick Mathewson's avatar Nick Mathewson 🐻
Browse files

Fix a bug in policy_is_reject_star() that was making IPv4 exits break

IPv4-only exits have an implicit "reject [::]/0", which was making
policy_is_reject_star() return 1 for them, making us refuse to do
hostname lookups.

This fix chanes policy_is_reject_star() to ask about which family we meant.
parent 85e8d35f
...@@ -799,7 +799,6 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, ...@@ -799,7 +799,6 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
cached_resolve_t search; cached_resolve_t search;
pending_connection_t *pending_connection; pending_connection_t *pending_connection;
int is_reverse = 0; int is_reverse = 0;
const routerinfo_t *me;
tor_addr_t addr; tor_addr_t addr;
time_t now = time(NULL); time_t now = time(NULL);
int r; int r;
...@@ -824,10 +823,9 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve, ...@@ -824,10 +823,9 @@ dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
} }
/* If we're a non-exit, don't even do DNS lookups. */ /* If we're a non-exit, don't even do DNS lookups. */
if (!(me = router_get_my_routerinfo()) || if (router_my_exit_policy_is_reject_star())
policy_is_reject_star(me->exit_policy)) {
return -1; return -1;
}
if (address_is_invalid_destination(exitconn->base_.address, 0)) { if (address_is_invalid_destination(exitconn->base_.address, 0)) {
log(LOG_PROTOCOL_WARN, LD_EXIT, log(LOG_PROTOCOL_WARN, LD_EXIT,
"Rejecting invalid destination address %s", "Rejecting invalid destination address %s",
......
...@@ -1049,18 +1049,23 @@ exit_policy_is_general_exit(smartlist_t *policy) ...@@ -1049,18 +1049,23 @@ exit_policy_is_general_exit(smartlist_t *policy)
/** Return false if <b>policy</b> might permit access to some addr:port; /** Return false if <b>policy</b> might permit access to some addr:port;
* otherwise if we are certain it rejects everything, return true. */ * otherwise if we are certain it rejects everything, return true. */
int int
policy_is_reject_star(const smartlist_t *policy) policy_is_reject_star(const smartlist_t *policy, sa_family_t family)
{ {
if (!policy) /*XXXX disallow NULL policies? */ if (!policy) /*XXXX disallow NULL policies? */
return 1; return 1;
SMARTLIST_FOREACH(policy, addr_policy_t *, p, { SMARTLIST_FOREACH_BEGIN(policy, addr_policy_t *, p) {
if (p->policy_type == ADDR_POLICY_ACCEPT) if (p->policy_type == ADDR_POLICY_ACCEPT &&
(tor_addr_family(&p->addr) == family ||
tor_addr_family(&p->addr) == AF_UNSPEC)) {
return 0; return 0;
else if (p->policy_type == ADDR_POLICY_REJECT && } else if (p->policy_type == ADDR_POLICY_REJECT &&
p->prt_min <= 1 && p->prt_max == 65535 && p->prt_min <= 1 && p->prt_max == 65535 &&
p->maskbits == 0) p->maskbits == 0 &&
(tor_addr_family(&p->addr) == family ||
tor_addr_family(&p->addr) == AF_UNSPEC)) {
return 1; return 1;
}); }
} SMARTLIST_FOREACH_END(p);
return 1; return 1;
} }
......
...@@ -49,7 +49,7 @@ int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest, ...@@ -49,7 +49,7 @@ int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest,
void policies_exit_policy_append_reject_star(smartlist_t **dest); void policies_exit_policy_append_reject_star(smartlist_t **dest);
void policies_set_node_exitpolicy_to_reject_all(node_t *exitrouter); void policies_set_node_exitpolicy_to_reject_all(node_t *exitrouter);
int exit_policy_is_general_exit(smartlist_t *policy); int exit_policy_is_general_exit(smartlist_t *policy);
int policy_is_reject_star(const smartlist_t *policy); int policy_is_reject_star(const smartlist_t *policy, sa_family_t family);
int getinfo_helper_policies(control_connection_t *conn, int getinfo_helper_policies(control_connection_t *conn,
const char *question, char **answer, const char *question, char **answer,
const char **errmsg); const char **errmsg);
......
...@@ -1621,7 +1621,8 @@ router_rebuild_descriptor(int force) ...@@ -1621,7 +1621,8 @@ router_rebuild_descriptor(int force)
ri->address, !options->BridgeRelay); ri->address, !options->BridgeRelay);
} }
ri->policy_is_reject_star = ri->policy_is_reject_star =
policy_is_reject_star(ri->exit_policy); policy_is_reject_star(ri->exit_policy, AF_INET) &&
policy_is_reject_star(ri->exit_policy, AF_INET6);
if (options->IPv6Exit) { if (options->IPv6Exit) {
char *p_tmp = policy_summarize(ri->exit_policy, AF_INET6); char *p_tmp = policy_summarize(ri->exit_policy, AF_INET6);
......
...@@ -1574,8 +1574,6 @@ router_parse_entry_from_string(const char *s, const char *end, ...@@ -1574,8 +1574,6 @@ router_parse_entry_from_string(const char *s, const char *end,
goto err; goto err;
}); });
policy_expand_private(&router->exit_policy); policy_expand_private(&router->exit_policy);
if (policy_is_reject_star(router->exit_policy))
router->policy_is_reject_star = 1;
if ((tok = find_opt_by_keyword(tokens, K_IPV6_POLICY)) && tok->n_args) { if ((tok = find_opt_by_keyword(tokens, K_IPV6_POLICY)) && tok->n_args) {
router->ipv6_exit_policy = parse_short_policy(tok->args[0]); router->ipv6_exit_policy = parse_short_policy(tok->args[0]);
...@@ -1585,6 +1583,11 @@ router_parse_entry_from_string(const char *s, const char *end, ...@@ -1585,6 +1583,11 @@ router_parse_entry_from_string(const char *s, const char *end,
} }
} }
if (policy_is_reject_star(router->exit_policy, AF_INET) &&
(!router->ipv6_exit_policy ||
short_policy_is_reject_star(router->ipv6_exit_policy)))
router->policy_is_reject_star = 1;
if ((tok = find_opt_by_keyword(tokens, K_FAMILY)) && tok->n_args) { if ((tok = find_opt_by_keyword(tokens, K_FAMILY)) && tok->n_args) {
int i; int i;
router->declared_family = smartlist_new(); router->declared_family = smartlist_new();
......
...@@ -1176,9 +1176,9 @@ test_policies(void) ...@@ -1176,9 +1176,9 @@ test_policies(void)
test_assert(!cmp_addr_policies(policy2, policy2)); test_assert(!cmp_addr_policies(policy2, policy2));
test_assert(!cmp_addr_policies(NULL, NULL)); test_assert(!cmp_addr_policies(NULL, NULL));
test_assert(!policy_is_reject_star(policy2)); test_assert(!policy_is_reject_star(policy2, AF_INET));
test_assert(policy_is_reject_star(policy)); test_assert(policy_is_reject_star(policy, AF_INET));
test_assert(policy_is_reject_star(NULL)); test_assert(policy_is_reject_star(NULL, AF_INET));
addr_policy_list_free(policy); addr_policy_list_free(policy);
policy = NULL; policy = NULL;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment