[or-cvs] r16558: {tor} Switch global_identifier on connections to a 64-bit field an (in tor/trunk: . src/or)
nickm at seul.org
nickm at seul.org
Fri Aug 15 13:55:03 UTC 2008
Author: nickm
Date: 2008-08-15 09:55:01 -0400 (Fri, 15 Aug 2008)
New Revision: 16558
Modified:
tor/trunk/ChangeLog
tor/trunk/src/or/connection.c
tor/trunk/src/or/control.c
tor/trunk/src/or/cpuworker.c
tor/trunk/src/or/or.h
Log:
Switch global_identifier on connections to a 64-bit field and move it to connection_t. When procession onionskins, look up the connection by this field rather than by addr:port. This will keep us from dropping onionskins. How many dropped circuits are dropped because of this bug?
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2008-08-15 06:39:04 UTC (rev 16557)
+++ tor/trunk/ChangeLog 2008-08-15 13:55:01 UTC (rev 16558)
@@ -7,6 +7,13 @@
hidden services): associate keys, client lists, and authorization
types with hidden services.
+ o Major bugfixes:
+ - When sending CREATED cells back for a given circuit, use a 64-bit
+ connection ID to find the right connection, rather than an addr:port
+ combination. Now that we can have multiple OR connections between the
+ same ORs, it is no longer possible to use addr:port to uniquely
+ identify a connection.
+
o Minor bugfixes:
- Recover 3-7 bytes that were wasted per memory chunk. Fixes bug
794; bug spotted by rovv. Bugfix on 0.2.0.1-alpha.
@@ -17,6 +24,8 @@
- Correctly detect the presence of the linux/netfilter_ipv4.h header
when building against recent kernels. Bugfix on 0.1.2.1-alpha.
- Add a missing safe_str() call for a debug log message.
+ - Use 64 bits instead of 32 bits for connection identifiers used with
+ the controller protocol, to greatly reduce risk of identifier reuse.
o Minor features
- Rate-limit too-many-sockets messages: when they happen, they
Modified: tor/trunk/src/or/connection.c
===================================================================
--- tor/trunk/src/or/connection.c 2008-08-15 06:39:04 UTC (rev 16557)
+++ tor/trunk/src/or/connection.c 2008-08-15 13:55:01 UTC (rev 16558)
@@ -166,7 +166,8 @@
connection_t *
connection_new(int type, int socket_family)
{
- static uint32_t n_connections_allocated = 1;
+ static uint64_t n_connections_allocated = 1;
+
connection_t *conn;
time_t now = time(NULL);
size_t length;
@@ -200,6 +201,7 @@
conn->magic = magic;
conn->s = -1; /* give it a default of 'not used' */
conn->conn_array_index = -1; /* also default to 'not used' */
+ conn->global_identifier = n_connections_allocated++;
conn->type = type;
conn->socket_family = socket_family;
@@ -211,9 +213,6 @@
TO_EDGE_CONN(conn)->socks_request =
tor_malloc_zero(sizeof(socks_request_t));
}
- if (CONN_IS_EDGE(conn)) {
- TO_EDGE_CONN(conn)->global_identifier = n_connections_allocated++;
- }
if (type == CONN_TYPE_OR) {
TO_OR_CONN(conn)->timestamp_last_added_nonpadding = now;
TO_OR_CONN(conn)->next_circ_id = crypto_rand_int(1<<15);
@@ -2437,28 +2436,6 @@
}
}
-/** Return the conn to addr/port that has the most recent
- * timestamp_created, or NULL if no such conn exists. */
-or_connection_t *
-connection_or_exact_get_by_addr_port(uint32_t addr, uint16_t port)
-{
- /* XXXX021 IP6 make this take a tor_addr_t, or deprecate it. */
-
- or_connection_t *best=NULL;
- smartlist_t *conns = get_connection_array();
-
- SMARTLIST_FOREACH(conns, connection_t *, conn,
- {
- if (conn->type == CONN_TYPE_OR &&
- tor_addr_eq_ipv4h(&conn->addr, addr) &&
- conn->port == port &&
- !conn->marked_for_close &&
- (!best || best->_base.timestamp_created < conn->timestamp_created))
- best = TO_OR_CONN(conn);
- });
- return best;
-}
-
/** Return a connection with given type, address, port, and purpose;
* or NULL if no such connection exists. */
connection_t *
@@ -2482,18 +2459,14 @@
/** Return the stream with id <b>id</b> if it is not already marked for
* close.
*/
-edge_connection_t *
-connection_get_by_global_id(uint32_t id)
+connection_t *
+connection_get_by_global_id(uint64_t id)
{
smartlist_t *conns = get_connection_array();
SMARTLIST_FOREACH(conns, connection_t *, conn,
{
- if (CONN_IS_EDGE(conn) && TO_EDGE_CONN(conn)->global_identifier == id) {
- if (!conn->marked_for_close)
- return TO_EDGE_CONN(conn);
- else
- return NULL;
- }
+ if (conn->global_identifier == id)
+ return conn;
});
return NULL;
}
Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c 2008-08-15 06:39:04 UTC (rev 16557)
+++ tor/trunk/src/or/control.c 2008-08-15 13:55:01 UTC (rev 16558)
@@ -656,16 +656,16 @@
static edge_connection_t *
get_stream(const char *id)
{
- uint32_t n_id;
+ uint64_t n_id;
int ok;
- edge_connection_t *conn;
- n_id = (uint32_t) tor_parse_ulong(id, 10, 0, UINT32_MAX, &ok, NULL);
+ connection_t *conn;
+ n_id = tor_parse_uint64(id, 10, 0, UINT64_MAX, &ok, NULL);
if (!ok)
return NULL;
conn = connection_get_by_global_id(n_id);
- if (!conn || conn->_base.type != CONN_TYPE_AP)
+ if (!conn || conn->type != CONN_TYPE_AP || conn->marked_for_close)
return NULL;
- return conn;
+ return TO_EDGE_CONN(conn);
}
/** Helper for setconf and resetconf. Acts like setconf, except
@@ -1648,8 +1648,7 @@
smartlist_t *conns = get_connection_array();
smartlist_t *status = smartlist_create();
char buf[256];
- SMARTLIST_FOREACH(conns, connection_t *, base_conn,
- {
+ SMARTLIST_FOREACH_BEGIN(conns, connection_t *, base_conn) {
const char *state;
edge_connection_t *conn;
char *s;
@@ -1691,12 +1690,12 @@
slen = strlen(buf)+strlen(state)+32;
s = tor_malloc(slen+1);
tor_snprintf(s, slen, "%lu %s %lu %s",
- (unsigned long) conn->global_identifier,state,
+ (unsigned long) conn->_base.global_identifier,state,
origin_circ?
(unsigned long)origin_circ->global_identifier : 0ul,
buf);
smartlist_add(status, s);
- });
+ } SMARTLIST_FOREACH_END(base_conn);
*answer = smartlist_join_strings(status, "\r\n", 0, NULL);
SMARTLIST_FOREACH(status, char *, cp, tor_free(cp));
smartlist_free(status);
@@ -3136,8 +3135,8 @@
if (circ && CIRCUIT_IS_ORIGIN(circ))
origin_circ = TO_ORIGIN_CIRCUIT(circ);
send_control_event_extended(EVENT_STREAM_STATUS, ALL_NAMES,
- "650 STREAM %lu %s %lu %s@%s%s%s\r\n",
- (unsigned long)conn->global_identifier, status,
+ "650 STREAM "U64_FORMAT" %s %lu %s@%s%s%s\r\n",
+ U64_PRINTF_ARG(conn->_base.global_identifier), status,
origin_circ?
(unsigned long)origin_circ->global_identifier : 0ul,
buf, reason_buf, addrport_buf, purpose);
@@ -3242,7 +3241,7 @@
smartlist_t *conns = get_connection_array();
edge_connection_t *edge_conn;
- SMARTLIST_FOREACH(conns, connection_t *, conn,
+ SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn)
{
if (conn->type != CONN_TYPE_AP)
continue;
@@ -3251,13 +3250,14 @@
continue;
send_control_event(EVENT_STREAM_BANDWIDTH_USED, ALL_NAMES,
- "650 STREAM_BW %lu %lu %lu\r\n",
- (unsigned long)edge_conn->global_identifier,
+ "650 STREAM_BW "U64_FORMAT" %lu %lu\r\n",
+ U64_PRINTF_ARG(edge_conn->_base.global_identifier),
(unsigned long)edge_conn->n_read,
(unsigned long)edge_conn->n_written);
edge_conn->n_written = edge_conn->n_read = 0;
- });
+ }
+ SMARTLIST_FOREACH_END(conn);
}
return 0;
Modified: tor/trunk/src/or/cpuworker.c
===================================================================
--- tor/trunk/src/or/cpuworker.c 2008-08-15 06:39:04 UTC (rev 16557)
+++ tor/trunk/src/or/cpuworker.c 2008-08-15 13:55:01 UTC (rev 16558)
@@ -23,7 +23,7 @@
#define MIN_CPUWORKERS 1
/** The tag specifies which circuit this onionskin was from. */
-#define TAG_LEN 8
+#define TAG_LEN 10
/** How many bytes are sent from the cpuworker back to tor? */
#define LEN_ONION_RESPONSE \
(1+TAG_LEN+ONIONSKIN_REPLY_LEN+CPATH_KEY_MATERIAL_LEN)
@@ -60,33 +60,23 @@
return 0;
}
-/** Pack addr,port,and circ_id; set *tag to the result. (See note on
+/** Pack global_id and circ_id; set *tag to the result. (See note on
* cpuworker_main for wire format.) */
static void
-tag_pack(char *tag, const tor_addr_t *addr, uint16_t port, circid_t circ_id)
+tag_pack(char *tag, uint64_t conn_id, circid_t circ_id)
{
/*XXXX RETHINK THIS WHOLE MESS !!!! !NM NM NM NM*/
- *(uint32_t *)tag = tor_addr_to_ipv4h(addr);
- *(uint16_t *)(tag+4) = port;
- *(uint16_t *)(tag+6) = circ_id;
+ *(uint64_t *)tag = conn_id;
+ *(uint16_t *)(tag+8) = circ_id;
}
/** Unpack <b>tag</b> into addr, port, and circ_id.
*/
static void
-tag_unpack(const char *tag, uint32_t *addr, uint16_t *port, circid_t *circ_id)
+tag_unpack(const char *tag, uint64_t *conn_id, circid_t *circ_id)
{
- struct in_addr in;
- char addrbuf[INET_NTOA_BUF_LEN];
-
- *addr = *(const uint32_t *)tag;
- *port = *(const uint16_t *)(tag+4);
- *circ_id = *(const uint16_t *)(tag+6);
-
- in.s_addr = htonl(*addr);
- tor_inet_ntoa(&in, addrbuf, sizeof(addrbuf));
- log_debug(LD_OR,
- "onion was from %s:%d, circ_id %d.", addrbuf, *port, *circ_id);
+ *conn_id = *(const uint64_t *)tag;
+ *circ_id = *(const uint16_t *)(tag+8);
}
/** Called when the onion key has changed and we need to spawn new
@@ -136,10 +126,10 @@
{
char success;
char buf[LEN_ONION_RESPONSE];
- uint32_t addr;
- uint16_t port;
+ uint64_t conn_id;
circid_t circ_id;
- or_connection_t *p_conn;
+ connection_t *tmp_conn;
+ or_connection_t *p_conn = NULL;
circuit_t *circ;
tor_assert(conn);
@@ -157,14 +147,13 @@
connection_fetch_from_buf(buf,LEN_ONION_RESPONSE-1,conn);
/* parse out the circ it was talking about */
- tag_unpack(buf, &addr, &port, &circ_id);
+ tag_unpack(buf, &conn_id, &circ_id);
circ = NULL;
- /* (Here we use connection_or_exact_get_by_addr_port rather than
- * get_by_identity_digest: we want a specific port here in
- * case there are multiple connections.) */
- /* XXXX021 This is dumb. We don't want just any connection with a matching
- * IP and port: we want the exact one that sent us this CREATE cell. */
- p_conn = connection_or_exact_get_by_addr_port(addr,port);
+ tmp_conn = connection_get_by_global_id(conn_id);
+ if (tmp_conn && !tmp_conn->marked_for_close &&
+ tmp_conn->type == CONN_TYPE_OR)
+ p_conn = TO_OR_CONN(tmp_conn);
+
if (p_conn)
circ = circuit_get_by_circid_orconn(circ_id, p_conn);
@@ -471,7 +460,7 @@
tor_free(onionskin);
return -1;
}
- tag_pack(tag, &circ->p_conn->_base.addr, circ->p_conn->_base.port,
+ tag_pack(tag, circ->p_conn->_base.global_identifier,
circ->p_circ_id);
cpuworker->state = CPUWORKER_STATE_BUSY_ONION;
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2008-08-15 06:39:04 UTC (rev 16557)
+++ tor/trunk/src/or/or.h 2008-08-15 13:55:01 UTC (rev 16558)
@@ -904,6 +904,9 @@
/** Another connection that's connected to this one in lieu of a socket. */
struct connection_t *linked_conn;
+ /** Unique identifier for this connection. */
+ uint64_t global_identifier;
+
/* XXXX021 move this into a subtype. */
struct evdns_server_port *dns_server_port;
@@ -1014,10 +1017,6 @@
/** The reason why this connection is closing; passed to the controller. */
uint16_t end_reason;
- /** Quasi-global identifier for this connection; used for control.c */
- /* XXXX NM This can get re-used after 2**32 streams */
- uint32_t global_identifier;
-
/** Bytes read since last call to control_event_stream_bandwidth_used() */
uint32_t n_read;
@@ -2858,9 +2857,7 @@
_connection_write_to_buf_impl(string, len, TO_CONN(conn), done ? -1 : 1);
}
-or_connection_t *connection_or_exact_get_by_addr_port(uint32_t addr,
- uint16_t port);
-edge_connection_t *connection_get_by_global_id(uint32_t id);
+connection_t *connection_get_by_global_id(uint64_t id);
connection_t *connection_get_by_type(int type);
connection_t *connection_get_by_type_purpose(int type, int purpose);
More information about the tor-commits
mailing list