On END_REASON_EXITPOLICY, mark circuit as unusable for that address.

Also, don't call the exit node 'reject *' unless our decision to pick
that node was based on a non-summarized version of that node's exit
policy.

rransom and arma came up with the ideas for this fix.

Fix for 7582; the summary-related part is a bugfix on 0.2.3.2-alpha.
This commit is contained in:
Nick Mathewson 2013-03-11 23:37:47 -04:00
parent 051b1e8ac4
commit 2b22c0aeef
8 changed files with 116 additions and 17 deletions

9
changes/bug7582 Normal file
View File

@ -0,0 +1,9 @@
o Major bugfixes:
- When an exit node tells us that it is rejecting because of its
exit policy a stream we expected it to accept (because of its exit
policy), do not mark the node as useless for exiting if our
expectation was only based on an exit policy summary. Instead,
mark the circuit as unsuitable for that particular address. Fixes
part of bug 7582; bugfix on 0.2.3.2-alpha.

View File

@ -105,6 +105,8 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ,
return 0;
if (purpose == CIRCUIT_PURPOSE_C_GENERAL) {
tor_addr_t addr;
const int family = tor_addr_parse(&addr, conn->socks_request->address);
if (!exitnode && !build_state->onehop_tunnel) {
log_debug(LD_CIRC,"Not considering circuit with unknown router.");
return 0; /* this circuit is screwed and doesn't know it yet,
@ -125,9 +127,7 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ,
return 0; /* this is a circuit to somewhere else */
if (tor_digest_is_zero(digest)) {
/* we don't know the digest; have to compare addr:port */
tor_addr_t addr;
int r = tor_addr_parse(&addr, conn->socks_request->address);
if (r < 0 ||
if (family < 0 ||
!tor_addr_eq(&build_state->chosen_exit->addr, &addr) ||
build_state->chosen_exit->port != conn->socks_request->port)
return 0;
@ -139,6 +139,13 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ,
return 0;
}
}
if (origin_circ->prepend_policy && family != -1) {
int r = compare_tor_addr_to_addr_policy(&addr,
conn->socks_request->port,
origin_circ->prepend_policy);
if (r == ADDR_POLICY_REJECTED)
return 0;
}
if (exitnode && !connection_ap_can_use_exit(conn, exitnode)) {
/* can't exit from this router */
return 0;

View File

@ -688,6 +688,24 @@ node_exit_policy_rejects_all(const node_t *node)
return 1;
}
/** Return true iff the exit policy for <b>node</b> is such that we can treat
* rejecting an address of type <b>family</b> unexpectedly as a sign of that
* node's failure. */
int
node_exit_policy_is_exact(const node_t *node, sa_family_t family)
{
if (family == AF_UNSPEC) {
return 1; /* Rejecting an address but not telling us what address
* is a bad sign. */
} else if (family == AF_INET) {
return node->ri != NULL;
} else if (family == AF_INET6) {
return 0;
}
tor_fragile_assert();
return 1;
}
/** 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().

View File

@ -41,6 +41,7 @@ int node_get_purpose(const node_t *node);
(node_get_purpose((node)) == ROUTER_PURPOSE_BRIDGE)
int node_is_me(const node_t *node);
int node_exit_policy_rejects_all(const node_t *node);
int node_exit_policy_is_exact(const node_t *node, sa_family_t family);
smartlist_t *node_get_all_orports(const node_t *node);
int node_allows_single_hop_exits(const node_t *node);
const char *node_get_nickname(const node_t *node);

View File

@ -3059,6 +3059,10 @@ typedef struct origin_circuit_t {
* ISO_STREAM. */
uint64_t associated_isolated_stream_global_id;
/**@}*/
/** A list of addr_policy_t for this circuit in particular. Used by
* adjust_exit_policy_from_exitpolicy_failure.
*/
smartlist_t *prepend_policy;
} origin_circuit_t;
struct onion_queue_t;

View File

@ -837,6 +837,24 @@ append_exit_policy_string(smartlist_t **policy, const char *more)
}
}
/** Add "reject <b>addr</b>:*" to <b>dest</b>, creating the list as needed. */
void
addr_policy_append_reject_addr(smartlist_t **dest, const tor_addr_t *addr)
{
addr_policy_t p, *add;
memset(&p, 0, sizeof(p));
p.policy_type = ADDR_POLICY_REJECT;
p.maskbits = tor_addr_family(addr) == AF_INET6 ? 128 : 32;
tor_addr_copy(&p.addr, addr);
p.prt_min = 1;
p.prt_max = 65535;
add = addr_policy_get_canonical_entry(&p);
if (!*dest)
*dest = smartlist_new();
smartlist_add(*dest, add);
}
/** Detect and excise "dead code" from the policy *<b>dest</b>. */
static void
exit_policy_remove_redundancies(smartlist_t *dest)

View File

@ -47,6 +47,8 @@ int policies_parse_exit_policy(config_line_t *cfg, smartlist_t **dest,
int rejectprivate, const char *local_address,
int add_default_policy);
void policies_exit_policy_append_reject_star(smartlist_t **dest);
void addr_policy_append_reject_addr(smartlist_t **dest,
const tor_addr_t *addr);
void policies_set_node_exitpolicy_to_reject_all(node_t *exitrouter);
int exit_policy_is_general_exit(smartlist_t *policy);
int policy_is_reject_star(const smartlist_t *policy, sa_family_t family);

View File

@ -53,6 +53,10 @@ static int circuit_resume_edge_reading_helper(edge_connection_t *conn,
static int circuit_consider_stop_edge_reading(circuit_t *circ,
crypt_path_t *layer_hint);
static int circuit_queue_streams_are_blocked(circuit_t *circ);
static void adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ,
entry_connection_t *conn,
node_t *node,
const tor_addr_t *addr);
/** Stop reading on edge connections when we have this many cells
* waiting on the appropriate queue. */
@ -710,7 +714,6 @@ connection_ap_process_end_not_open(
relay_header_t *rh, cell_t *cell, origin_circuit_t *circ,
entry_connection_t *conn, crypt_path_t *layer_hint)
{
struct in_addr in;
node_t *exitrouter;
int reason = *(cell->payload+RELAY_HEADER_SIZE);
int control_reason;
@ -753,10 +756,10 @@ connection_ap_process_end_not_open(
stream_end_reason_to_string(reason));
exitrouter = node_get_mutable_by_id(chosen_exit_digest);
switch (reason) {
case END_STREAM_REASON_EXITPOLICY:
case END_STREAM_REASON_EXITPOLICY: {
tor_addr_t addr;
tor_addr_make_unspec(&addr);
if (rh->length >= 5) {
tor_addr_t addr;
int ttl = -1;
tor_addr_make_unspec(&addr);
if (rh->length == 5 || rh->length == 9) {
@ -808,16 +811,11 @@ connection_ap_process_end_not_open(
}
}
/* check if he *ought* to have allowed it */
if (exitrouter &&
(rh->length < 5 ||
(tor_inet_aton(conn->socks_request->address, &in) &&
!conn->chosen_exit_name))) {
log_info(LD_APP,
"Exitrouter %s seems to be more restrictive than its exit "
"policy. Not using this router as exit for now.",
node_describe(exitrouter));
policies_set_node_exitpolicy_to_reject_all(exitrouter);
}
adjust_exit_policy_from_exitpolicy_failure(circ,
conn,
exitrouter,
&addr);
if (conn->chosen_exit_optional ||
conn->chosen_exit_retries) {
@ -837,6 +835,7 @@ connection_ap_process_end_not_open(
return 0;
/* else, conn will get closed below */
break;
}
case END_STREAM_REASON_CONNECTREFUSED:
if (!conn->chosen_exit_optional)
break; /* break means it'll close, below */
@ -901,6 +900,47 @@ connection_ap_process_end_not_open(
return 0;
}
/** Called when we have gotten an END_REASON_EXITPOLICY failure on <b>circ</b>
* for <b>conn</b>, while attempting to connect via <b>node</b>. If the node
* told us which address it rejected, then <b>addr</b> is that address;
* otherwise it is AF_UNSPEC.
*
* If we are sure the node should have allowed this address, mark the node as
* having a reject *:* exit policy. Otherwise, mark the circuit as unusable
* for this particular address.
**/
static void
adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ,
entry_connection_t *conn,
node_t *node,
const tor_addr_t *addr)
{
int make_reject_all = 0;
const sa_family_t family = tor_addr_family(addr);
if (node) {
tor_addr_t tmp;
int asked_for_family = tor_addr_parse(&tmp, conn->socks_request->address);
if (family == AF_UNSPEC) {
make_reject_all = 1;
} else if (node_exit_policy_is_exact(node, family) &&
asked_for_family != -1 && !conn->chosen_exit_name) {
make_reject_all = 1;
}
if (make_reject_all) {
log_info(LD_APP,
"Exitrouter %s seems to be more restrictive than its exit "
"policy. Not using this router as exit for now.",
node_describe(node));
policies_set_node_exitpolicy_to_reject_all(node);
}
}
if (family != AF_UNSPEC)
addr_policy_append_reject_addr(&circ->prepend_policy, addr);
}
/** Helper: change the socks_request-&gt;address field on conn to the
* dotted-quad representation of <b>new_addr</b>,
* and send an appropriate REMAP event. */