[tor-commits] [tor/master] On END_REASON_EXITPOLICY, mark circuit as unusable for that address.
nickm at torproject.org
nickm at torproject.org
Tue Mar 19 18:17:00 UTC 2013
commit 2b22c0aeef6e71d56b12411d10484aaece769178
Author: Nick Mathewson <nickm at torproject.org>
Date: Mon Mar 11 23:37:47 2013 -0400
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.
---
changes/bug7582 | 9 ++++++
src/or/circuituse.c | 13 +++++++--
src/or/nodelist.c | 18 +++++++++++++
src/or/nodelist.h | 1 +
src/or/or.h | 4 +++
src/or/policies.c | 18 +++++++++++++
src/or/policies.h | 2 +
src/or/relay.c | 68 ++++++++++++++++++++++++++++++++++++++++----------
8 files changed, 116 insertions(+), 17 deletions(-)
diff --git a/changes/bug7582 b/changes/bug7582
new file mode 100644
index 0000000..f3b0635
--- /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/circuituse.c b/src/or/circuituse.c
index 51d8716..6b5ca90 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -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;
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index ee1bc39..5f3b843 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 <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().
diff --git a/src/or/nodelist.h b/src/or/nodelist.h
index 65cf0d0..8a4665a 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 c7d2598..c893fba 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -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;
diff --git a/src/or/policies.c b/src/or/policies.c
index 9696b81..be4da55 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 <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)
diff --git a/src/or/policies.h b/src/or/policies.h
index da375c4..c0e7a9e 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 9ff9e1e..a4f402d 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -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->address field on conn to the
* dotted-quad representation of <b>new_addr</b>,
* and send an appropriate REMAP event. */
More information about the tor-commits
mailing list