[tor-bugs] #936 [Tor - Relay]: Race in picking connections for create cell
Tor Bug Tracker & Wiki
torproject-admin at torproject.org
Tue Aug 3 20:17:10 UTC 2010
#936: Race in picking connections for create cell
--------------------------------+-------------------------------------------
Reporter: arma | Type: defect
Status: new | Priority: minor
Milestone: Tor: 0.2.2.x-final | Component: Tor - Relay
Version: 0.2.0.31 | Resolution: None
Keywords: | Parent:
--------------------------------+-------------------------------------------
Old description:
> Reported by lark.
>
> I possible found edge case for doubling conns between two relays, the
> one inbound and the one outbound for the node. It's can trigger if conns
> launched by relays to each other at the same time, very precisely. Which
> can
> happens for two relays with high uptime and fast speed, after
> TIME_BEFORE_OR_CONN_IS_TOO_OLD.
>
> --- connection_or.c.orig Thu Feb 5 00:42:22 2009
> +++ connection_or.c Fri Mar 6 19:15:04 2009
> -452,7 +452,7 @@
> const or_connection_t *b,
> int forgive_new_connections)
> {
> - int newer;
> + int newer, delta, inbound_a, outbound_b, prefer_conn_a;
> /** Do not definitively deprecate a new connection with no circuits on
> it
> * until this much time has passed. */
> #define NEW_CONN_GRACE_PERIOD (15*60)
> -461,15 +461,32 @@
> return 0; /* A canonical connection is better than a non-canonical
> * one, no matter how new it is or which has circuits. */
>
> - newer = b->_base.timestamp_created < a->_base.timestamp_created;
> + inbound_a = tor_tls_is_server(a->tls);
> + outbound_b = !tor_tls_is_server(b->tls);
>
> + delta=(int)(a->_base.timestamp_created - b->_base.timestamp_created);
> + newer= delta > 0;
> +
> + /* We prefer the "a" conn at equal conditions:
> + *
> + * if "a" is inbound from a node with higher identity key
> + * and created at the same time or later than outbound "b" conn;
> + *
> + * or
> + *
> + * if "a" newer than "b" for any other inbound-outbound
> + * combinations */
> + prefer_conn_a = ((newer && !(inbound_a && outbound_b)) ||
> + (inbound_a && outbound_b && delta >= 0 &&
> + a->circ_id_type == CIRC_ID_TYPE_LOWER));
> +
> if (
> /* We prefer canonical connections regardless of newness. */
> (!b->is_canonical && a->is_canonical) ||
> /* If both have circuits we prefer the newer: */
> - (b->n_circuits && a->n_circuits && newer) ||
> + (b->n_circuits && a->n_circuits && prefer_conn_a) ||
> /* If neither has circuits we prefer the newer: */
> - (!b->n_circuits && !a->n_circuits && newer))
> + (!b->n_circuits && !a->n_circuits && prefer_conn_a))
> return 1;
>
> /* If one has no circuits and the other does... */
>
> [Automatically added by flyspray2trac: Operating System: All]
New description:
Reported by lark.
I possible found edge case for doubling conns between two relays, the
one inbound and the one outbound for the node. It's can trigger if conns
launched by relays to each other at the same time, very precisely. Which
can
happens for two relays with high uptime and fast speed, after
TIME_BEFORE_OR_CONN_IS_TOO_OLD.
--- connection_or.c.orig Thu Feb 5 00:42:22 2009
+++ connection_or.c Fri Mar 6 19:15:04 2009
-452,7 +452,7 @@
const or_connection_t *b,
int forgive_new_connections)
{
- int newer;
+ int newer, delta, inbound_a, outbound_b, prefer_conn_a;
/** Do not definitively deprecate a new connection with no circuits on it
* until this much time has passed. */
#define NEW_CONN_GRACE_PERIOD (15*60)
-461,15 +461,32 @@
return 0; /* A canonical connection is better than a non-canonical
* one, no matter how new it is or which has circuits. */
- newer = b->_base.timestamp_created < a->_base.timestamp_created;
+ inbound_a = tor_tls_is_server(a->tls);
+ outbound_b = !tor_tls_is_server(b->tls);
+ delta=(int)(a->_base.timestamp_created - b->_base.timestamp_created);
+ newer= delta > 0;
+
+ /* We prefer the "a" conn at equal conditions:
+ *
+ * if "a" is inbound from a node with higher identity key
+ * and created at the same time or later than outbound "b" conn;
+ *
+ * or
+ *
+ * if "a" newer than "b" for any other inbound-outbound
+ * combinations */
+ prefer_conn_a = ((newer && !(inbound_a && outbound_b)) ||
+ (inbound_a && outbound_b && delta >= 0 &&
+ a->circ_id_type == CIRC_ID_TYPE_LOWER));
+
if (
/* We prefer canonical connections regardless of newness. */
(!b->is_canonical && a->is_canonical) ||
/* If both have circuits we prefer the newer: */
- (b->n_circuits && a->n_circuits && newer) ||
+ (b->n_circuits && a->n_circuits && prefer_conn_a) ||
/* If neither has circuits we prefer the newer: */
- (!b->n_circuits && !a->n_circuits && newer))
+ (!b->n_circuits && !a->n_circuits && prefer_conn_a))
return 1;
/* If one has no circuits and the other does... */
[Automatically added by flyspray2trac: Operating System: All]
--
Comment(by nickm):
So there are two problems at work here:
* Two connections could have been created in the same second, such that
neither would seem newer than the other.
* Two ORs will not necessarily have the same view of when two
connections were created relative to another; each will think that the
connection that came from it will was created before the other's.
We could add a tie-breaker to be used only in the case where the two
connections were created in the same second, but that wouldn't help in the
case where the ORs each think that a different connection is newest. We
could add a tie-breaker to be used in the case where the connections were
created within some small time-step of one another, but there's no reason
that ORs would necessarily agree on the delta.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/936#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list