[or-cvs] r17628: {tor} Don't extend circuits over noncanonical connections with mis (in tor/trunk: . doc src/or)
nickm at seul.org
nickm at seul.org
Mon Dec 15 21:17:55 UTC 2008
Author: nickm
Date: 2008-12-15 16:17:53 -0500 (Mon, 15 Dec 2008)
New Revision: 17628
Modified:
tor/trunk/ChangeLog
tor/trunk/doc/TODO.021
tor/trunk/src/or/circuitbuild.c
Log:
Don't extend circuits over noncanonical connections with mismatched addresses.
Also, refactor the logic to check whether we will use a connection or
launch a new one into a new function.
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2008-12-15 21:17:43 UTC (rev 17627)
+++ tor/trunk/ChangeLog 2008-12-15 21:17:53 UTC (rev 17628)
@@ -1,4 +1,10 @@
Changes in version 0.2.1.9-alpha - 200?-??-??
+ o Major features:
+ - Never use a connection with a mismatched address to extend a
+ circuit, unless that connections is canonical. A canonical
+ connection is one whose address is authenticated by the router's
+ identity key, either in a NETINFO cell or in a router descriptor.
+
o Major bugfixes:
- Fix a logic error that would automatically reject all but the first
configured DNS server. Bugfix on 0.2.1.5-alpha. Possible fix for part
Modified: tor/trunk/doc/TODO.021
===================================================================
--- tor/trunk/doc/TODO.021 2008-12-15 21:17:43 UTC (rev 17627)
+++ tor/trunk/doc/TODO.021 2008-12-15 21:17:53 UTC (rev 17628)
@@ -27,7 +27,7 @@
Nick
* Look at Roger's proposal 141 discussions on or-dev, and help us
decide how to proceed.
- - Tors start believing the contents of NETINFO cells.
+ . Tors start believing the contents of NETINFO cells.
- respond to Steven's red-team TLS testing (a.k.a, look at a packet
dump and compare)
@@ -166,10 +166,10 @@
- 145: Separate "suitable from a guard" from "suitable as a new guard"
- 146: Adding new flag to reflect long-term stability
- 149: Using data from NETINFO cells
- * Don't extend a circuit over a noncanonical connection with
+ o Don't extend a circuit over a noncanonical connection with
mismatched address.
o Apply rovv's bugfixes wrt preferring canonical connections.
- - Make sure that having a non-canonical connection doesn't count
+ o Make sure that having a non-canonical connection doesn't count
as _having_ a connection for the purpose of connecting to others,
and that when no canonical connection exists, we make one.
- Learn our outgoing IP address from netinfo cells?
@@ -282,6 +282,8 @@
P - create a "make win32-bundle" for vidalia-privoxy-tor-torbutton bundle
- Refactor bad code:
+ - connection_or_get_by_identity_digest() and connection_good_enough_for
+ _extend() could be merged into a smarter variant, perhaps.
- Refactor the HTTP logic so the functions aren't so large.
- Refactor buf_read and buf_write to have sensible ways to return
error codes after partial writes
Modified: tor/trunk/src/or/circuitbuild.c
===================================================================
--- tor/trunk/src/or/circuitbuild.c 2008-12-15 21:17:43 UTC (rev 17627)
+++ tor/trunk/src/or/circuitbuild.c 2008-12-15 21:17:53 UTC (rev 17628)
@@ -334,6 +334,46 @@
return circ;
}
+/** Return true iff <b>n_conn</b> (a connection with a desired identity), is
+ * an acceptable choice for extending or launching a circuit to the address
+ * <b>target_addr</b>. If it is not, set <b>state_out</b> to a message
+ * describing the connection's state and our next action, and set
+ * <b>launch_out</b> to a boolean for whether we should launch a new
+ * connection or not. */
+static int
+connection_good_enough_for_extend(const or_connection_t *n_conn,
+ const tor_addr_t *target_addr,
+ const char **state_out,
+ int *launch_out)
+{
+ tor_assert(state_out);
+ tor_assert(launch_out);
+ tor_assert(target_addr);
+
+ if (!n_conn) {
+ *state_out = "not connected. Connecting.";
+ *launch_out = 1;
+ return 0;
+ } else if (n_conn->_base.state != OR_CONN_STATE_OPEN) {
+ *state_out = "in progress. Waiting.";
+ *launch_out = 0; /* We'll just wait till the connection finishes. */
+ return 0;
+ } else if (n_conn->_base.or_is_obsolete) {
+ *state_out = "too old. Launching a new one.";
+ *launch_out = 1;
+ return 0;
+ } else if (tor_addr_compare(&n_conn->_base.addr, target_addr, CMP_EXACT) &&
+ ! n_conn->is_canonical) {
+ *state_out = "is not from a canonical address. Launching a new one.";
+ *launch_out = 1;
+ return 0;
+ } else {
+ *state_out = "is fine; using it.";
+ *launch_out = 0;
+ return 1;
+ }
+}
+
/** Start establishing the first hop of our circuit. Figure out what
* OR we should connect to, and if necessary start the connection to
* it. If we're already connected, then send the 'create' cell.
@@ -344,6 +384,8 @@
crypt_path_t *firsthop;
or_connection_t *n_conn;
int err_reason = 0;
+ const char *msg = NULL;
+ int should_launch = 0;
firsthop = onion_next_hop_in_cpath(circ->cpath);
tor_assert(firsthop);
@@ -356,15 +398,17 @@
n_conn = connection_or_get_by_identity_digest(
firsthop->extend_info->identity_digest);
+
/* If we don't have an open conn, or the conn we have is obsolete
* (i.e. old or broken) and the other side will let us make a second
* connection without dropping it immediately... */
- if (!n_conn || n_conn->_base.state != OR_CONN_STATE_OPEN ||
- (n_conn->_base.or_is_obsolete)) {
+ if (!connection_good_enough_for_extend(n_conn, &firsthop->extend_info->addr,
+ &msg, &should_launch)) {
+ /* XXXX021 log msg, maybe. */
/* not currently connected */
circ->_base.n_hop = extend_info_dup(firsthop->extend_info);
- if (!n_conn || n_conn->_base.or_is_obsolete) { /* launch the connection */
+ if (should_launch) {
if (circ->build_state->onehop_tunnel)
control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0);
n_conn = connection_or_connect(&firsthop->extend_info->addr,
@@ -723,8 +767,11 @@
relay_header_t rh;
char *onionskin;
char *id_digest=NULL;
- uint32_t n_addr;
+ uint32_t n_addr32;
uint16_t n_port;
+ tor_addr_t n_addr;
+ const char *msg = NULL;
+ int should_launch = 0;
if (circ->n_conn) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
@@ -747,10 +794,11 @@
return -1;
}
- n_addr = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE));
+ n_addr32 = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE));
n_port = ntohs(get_uint16(cell->payload+RELAY_HEADER_SIZE+4));
onionskin = cell->payload+RELAY_HEADER_SIZE+4+2;
id_digest = cell->payload+RELAY_HEADER_SIZE+4+2+ONIONSKIN_CHALLENGE_LEN;
+ tor_addr_from_ipv4h(&n_addr, n_addr32);
/* First, check if they asked us for 0000..0000. We support using
* an empty fingerprint for the first hop (e.g. for a bridge relay),
@@ -779,26 +827,23 @@
/* If we don't have an open conn, or the conn we have is obsolete
* (i.e. old or broken) and the other side will let us make a second
* connection without dropping it immediately... */
- if (!n_conn || n_conn->_base.state != OR_CONN_STATE_OPEN ||
- n_conn->_base.or_is_obsolete) {
- tor_addr_t addr;
- tor_addr_from_ipv4h(&addr, n_addr);
+ if (!connection_good_enough_for_extend(n_conn, &n_addr, &msg,
+ &should_launch)) {
+ log_debug(LD_CIRC|LD_OR,"Next router (%s:%d) %s",
+ fmt_addr(&n_addr), (int)n_port, msg?msg:"????");
- log_debug(LD_CIRC|LD_OR,"Next router (%s:%d) not connected. Connecting.",
- fmt_addr(&addr), (int)n_port);
-
circ->n_hop = extend_info_alloc(NULL /*nickname*/,
id_digest,
NULL /*onion_key*/,
- &addr, n_port);
+ &n_addr, n_port);
circ->n_conn_onionskin = tor_malloc(ONIONSKIN_CHALLENGE_LEN);
memcpy(circ->n_conn_onionskin, onionskin, ONIONSKIN_CHALLENGE_LEN);
circuit_set_state(circ, CIRCUIT_STATE_OR_WAIT);
- if (! (n_conn && !n_conn->_base.or_is_obsolete)) {
+ if (should_launch) {
/* we should try to open a connection */
- n_conn = connection_or_connect(&addr, n_port, id_digest);
+ n_conn = connection_or_connect(&n_addr, n_port, id_digest);
if (!n_conn) {
log_info(LD_CIRC,"Launching n_conn failed. Closing circuit.");
circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED);
More information about the tor-commits
mailing list