diff --git a/changes/bug7582 b/changes/bug7582 new file mode 100644 index 0000000000..f3b0635765 --- /dev/null +++ b/changes/bug7582 @@ -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. + diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 6c7629c7ba..1903fbe2eb 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -24,6 +24,7 @@ #include "nodelist.h" #include "onion.h" #include "onion_fast.h" +#include "policies.h" #include "relay.h" #include "rendclient.h" #include "rendcommon.h" @@ -656,6 +657,7 @@ circuit_free(circuit_t *circ) memwipe(ocirc->socks_password, 0x06, ocirc->socks_password_len); tor_free(ocirc->socks_password); } + addr_policy_list_free(ocirc->prepend_policy); } else { or_circuit_t *ocirc = TO_OR_CIRCUIT(circ); /* Remember cell statistics for this circuit before deallocating. */ diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 4d1065bbd8..d48449fa81 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -109,6 +109,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, @@ -129,9 +131,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; @@ -143,6 +143,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; diff --git a/src/or/nodelist.c b/src/or/nodelist.c index ee1bc392e3..5f3b843d02 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -688,6 +688,24 @@ node_exit_policy_rejects_all(const node_t *node) return 1; } +/** Return true iff the exit policy for node is such that we can treat + * rejecting an address of type family 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 node. Caller must free all elements * using tor_free() and free the list using smartlist_free(). diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 65cf0d0284..8a4665a8bf 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -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); diff --git a/src/or/or.h b/src/or/or.h index 2b9d2b4d67..68039afdf0 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3068,6 +3068,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; diff --git a/src/or/policies.c b/src/or/policies.c index 9696b8123b..be4da55061 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -837,6 +837,24 @@ append_exit_policy_string(smartlist_t **policy, const char *more) } } +/** Add "reject addr:*" to dest, 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 *dest. */ static void exit_policy_remove_redundancies(smartlist_t *dest) diff --git a/src/or/policies.h b/src/or/policies.h index da375c4425..c0e7a9efce 100644 --- a/src/or/policies.h +++ b/src/or/policies.h @@ -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); diff --git a/src/or/relay.c b/src/or/relay.c index ef37c32810..1da993269d 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -54,6 +54,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. */ @@ -711,7 +715,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; @@ -754,10 +757,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) { @@ -809,16 +812,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) { @@ -838,6 +836,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 */ @@ -900,6 +899,47 @@ connection_ap_process_end_not_open( return 0; } +/** Called when we have gotten an END_REASON_EXITPOLICY failure on circ + * for conn, while attempting to connect via node. If the node + * told us which address it rejected, then addr 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->address field on conn to the * dotted-quad representation of new_addr, * and send an appropriate REMAP event. */