diff --git a/ChangeLog b/ChangeLog index d42206c63b..9ed50e577b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,12 +7,16 @@ Changes in version 0.2.1.5-alpha - 2008-08-?? hidden services): associate keys, client lists, and authorization types with hidden services. - o Major bugfixes: + o Major bugfixes (on 0.2.0.x and before): - 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. + - Relays now reject risky extend cells: if the extend cell includes + a digest of all zeroes, or asks to extend back to the relay that + sent the extend cell, tear down the circuit. Ideas suggested + by rovv. o Minor bugfixes: - Recover 3-7 bytes that were wasted per memory chunk. Fixes bug diff --git a/doc/spec/tor-spec.txt b/doc/spec/tor-spec.txt index 74b3a250b6..e0a10a32d6 100644 --- a/doc/spec/tor-spec.txt +++ b/doc/spec/tor-spec.txt @@ -398,9 +398,9 @@ see tor-design.pdf. The port and address field denote the IPV4 address and port of the next onion router in the circuit; the public key hash is the hash of the PKCS#1 ASN1 encoding of the next onion router's identity (signing) key. (See 0.3 - above.) (Including this hash allows the extending OR verify that it is + above.) Including this hash allows the extending OR verify that it is indeed connected to the correct target OR, and prevents certain - man-in-the-middle attacks.) + man-in-the-middle attacks. The payload for a CREATED cell, or the relay payload for an EXTENDED cell, contains: @@ -525,10 +525,12 @@ see tor-design.pdf. When an onion router receives an EXTEND relay cell, it sends a CREATE cell to the next onion router, with the enclosed onion skin as its - payload. The initiating onion router chooses some circID not yet - used on the connection between the two onion routers. (But see - section 5.1. above, concerning choosing circIDs based on - lexicographic order of nicknames.) + payload. As special cases, if the extend cell includes a digest of + all zeroes, or asks to extend back to the relay that sent the extend + cell, the circuit will fail and be torn down. The initiating onion + router chooses some circID not yet used on the connection between the + two onion routers. (But see section 5.1. above, concerning choosing + circIDs based on lexicographic order of nicknames.) When an onion router receives a CREATE cell, if it already has a circuit on the given connection with the given circID, it drops the diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 842ecbd1d9..66f2a95c0b 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -712,10 +712,13 @@ circuit_note_clock_jumped(int seconds_elapsed) circuit_expire_all_dirty_circs(); } -/** Take the 'extend' cell, pull out addr/port plus the onion skin. Make - * sure we're connected to the next hop, and pass it the onion skin using - * a create cell. Return -1 if we want to warn and tear down the circuit, - * else return 0. +/** Take the 'extend' cell, pull out addr/port plus the onion + * skin and identity digest for the next hop. If we're already connected, + * pass the onion skin to the next hop using a create cell; otherwise + * launch a new OR connection, and circ will notice when the + * connection succeeds or fails. + * + * Return -1 if we want to warn and tear down the circuit, else return 0. */ int circuit_extend(cell_t *cell, circuit_t *circ) @@ -753,6 +756,28 @@ circuit_extend(cell_t *cell, circuit_t *circ) onionskin = cell->payload+RELAY_HEADER_SIZE+4+2; id_digest = cell->payload+RELAY_HEADER_SIZE+4+2+ONIONSKIN_CHALLENGE_LEN; + /* 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), + * but we don't want to let people send us extend cells for empty + * fingerprints -- a) because it opens the user up to a mitm attack, + * and b) because it lets an attacker force the relay to hold open a + * new TLS connection for each extend request. */ + if (tor_digest_is_zero(id_digest)) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Client asked me to extend without specifying an id_digest."); + return -1; + } + + /* Next, check if we're being asked to connect to the hop that the + * extend cell came from. There isn't any reason for that, and it can + * assist circular-path attacks. */ + if (!memcmp(id_digest, TO_OR_CIRCUIT(circ)->p_conn->identity_digest, + DIGEST_LEN)) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Client asked me to extend back to the previous hop."); + return -1; + } + n_conn = connection_or_get_by_identity_digest(id_digest); /* If we don't have an open conn, or the conn we have is obsolete