Tor

Comment by Nick Mathewson - Monday, 3 Nov 2008, 3:34pm

Hm.  Perhaps our usual methods for noticing that directories aren't working and excluding them don't work
when we're using one-hop tunnels.  This needs more investigation.

Comment by Karsten Loesing - Wednesday, 5 Nov 2008, 11:46am

r17189 should be part of the solution. Tunneled directory requests and client-side
introduction circuits should time out after the 30 seconds, which they didn't. This
should at least mitigate the problem.

Comment by Karsten Loesing - Sunday, 16 Nov 2008, 1:36pm

Mystery resolved. The bad values are simply the result of directory
downloads that take a very long time. Tor waits for a directory download to
either finish or fail before making another one. That attempt might then be
directed to the same slow authority. The problem occurs when downloading
the consensus or authority certificates (but is more serious for the
latter, because it's bigger).

This becomes visible in the log files by large gaps between two log
statements (all pairs originate from different test runs):

Nov 15 00:34:35.667 [info] _connection_free(): Freeing linked Directory
connection [client reading] with 0 bytes on inbuf, 0 on outbuf.
Nov 15 00:40:11.906 [info] update_consensus_networkstatus_downloads():
Launching networkstatus consensus download.
--
Nov 14 23:06:07.497 [notice] Bootstrapped 25%: Loading networkstatus
consensus.
Nov 14 23:12:59.818 [info] run_connection_housekeeping(): Expiring wedged
directory conn (fd -1, purpose 14)
--
Nov 14 05:18:50.407 [notice] Bootstrapped 25%: Loading networkstatus
consensus.
Nov 14 05:25:01.475 [info] connection_edge_process_relay_cell(): -1: end
cell (closed normally) for stream 28178. Removing stream.
--
Nov 15 09:45:03.075 [notice] We're missing a certificate from authority
with signing key 8C4CA02AA1AA71FCC56F9F4A46E09A4719DD75C3: launching
request.
Nov 15 09:46:04.294 [notice] We're missing a certificate from authority
with signing key D6D2325E1511B23A825DBE1CFD3DF9285AAE4DEB: launching
request.

Comment by Sebastian Hahn - Friday, 21 Nov 2008, 11:26am

Following patch is from mortalcombat.

--- connection.c    Wed Nov 12 10:47:36 2008
+++ connection.dir_linked.c Fri Nov 21 10:02:40 2008
@@ -1552,7 +1552,16 @@
   if (authdir_mode(get_options()) && priority>1)
     return 0; /* there's always room to answer v2 if we're an auth dir */

-  if (!connection_is_rate_limited(conn))
+  tor_assert(conn->_base.type == CONN_TYPE_DIR);
+  /* connection_is_rate_limited() helping with exclude moved bytes of
+   * buffers of linked conns from extra decrements of our buckets.
+   * But this call is exception, only local conns (why? whose it's conns?)
+   * not rate limited for directory operations. Tunneled directory conns 
+   * should be limitted.
+   * Else answered bytes just server's memory and client's time consume  
+   *  if not enought bandwith present.
+   */
+  if (!connection_is_rate_limited(conn) && !conn->linked)
     return 0; /* local conns don't get limited */

   if (smaller_bucket < (int)attempt)

Comment by Roger Dingledine - Friday, 21 Nov 2008, 3:41pm

This patch is also from mortalkombat. I'm worried that it might return 1
for authdirs in the !old_dir_conn case, but I will have to review it
once I've slept to be sure.

--- connection.c    Wed Nov 12 10:47:36 2008
+++ connection.dir_linked.c Fri Nov 21 12:06:38 2008
@@ -1549,16 +1549,21 @@
 {
   int smaller_bucket = global_write_bucket < global_relayed_write_bucket ?
                        global_write_bucket : global_relayed_write_bucket;
-  if (authdir_mode(get_options()) && priority>1)
+  int old_dir_conn = !conn->linked;
+  tor_assert(conn->type == CONN_TYPE_DIR);
+  /* tunneled conns for low bandwith authorities too slow, check for bucket */
+  if (authdir_mode(get_options()) && priority > 1 && old_dir_conn)
     return 0; /* there's always room to answer v2 if we're an auth dir */

-  if (!connection_is_rate_limited(conn))
+  /* tunneled conns for limited bandwith too slow, better 503 */
+  if (!connection_is_rate_limited(conn) && old_dir_conn)
     return 0; /* local conns don't get limited */

   if (smaller_bucket < (int)attempt)
     return 1; /* not enough space no matter the priority */

-  if (write_buckets_empty_last_second)
+  /* tunneled conns it's part of Tor's traffic, limited already */
+  if (write_buckets_empty_last_second && old_dir_conn)
     return 1; /* we're already hitting our limits, no more please */

   if (priority == 1) { /* old-style v1 query */

Comment by [Deleted] - Friday, 21 Nov 2008, 9:20pm

Limiting of service from a limited number of authdirs 
can be a convenient target for an attack on a denial of service.
(new clients with empty the cache of documents as potential victims
 for the proposed patch ) 
However, current versions have such documents also, it's a certificates 
(while other unlimited, such as consensus), and attack might 
be directed against all recipients - as mirrors and clients. 
For authdir perhaps better rule: slowly but distributed among all, as quickly but 
only for the attacker. 
There are two ways to solve the problem of congestion
while receiving of consensus from authdirs (and/or): 
1) Encourage the owners of authdirs with congestion to increase the ratio of 
  bandwidthburst / bandwithrate (with reachable bandwidthburst in practice); 
2) clients reduce allowable idletime in obtaining data from the directory,
and after trigger timer to avoid re-use this directory while receiving data. 

(The remaining two items of patch is tasks to integrate affordable of
 global write bucket  on the side of mirrors for all documents or
 certificates from authdirs, with intent distribute documents
 through tunneled dir conns)

Comment by Roger Dingledine - Monday, 22 Dec 2008, 11:34am

Another component to fixing this is that authorities with low bandwidth should set
MaxAdvertisedBandwidth 10 KB
so they attract less traffic.

But the other fixes above should be considered too.

Comment by Nick Mathewson - Monday, 9 Feb 2009, 6:33pm

The basis of the patches above seems like a good idea, though we should probably refactor some to make it cleaner.
(The big thing is that connection_is_rate_limited() probably wants to allow unlimited reads from a linked connection,
but limit writes to it so as not to clog our ram.  This might mean that we pull the linked_conn check higher, or such.)

Roger, I think you're right that the second version can return 1 for authdirs.

I think that for 0.2.1.x we should:
  - Fix the possibility if any of linked connection buffers getting enormous.
  - Fix the interaction of linked_conn and connection_is_rate_limited.

Idea 2 above (reduce timeout on fetches and treat a timeout as making a directory less desirable) is a fine one,
but it seems tricky to get right, and should maybe wait for 0.2.2.x and get a proposal.  The case I'm worried
about is the one in which a user has a really slow connection such that they can't get _any_ consensus under
the default timeout, and so they fail entirely.

Comment by Sebastian Hahn - Saturday, 19 Sep 2009, 7:12pm

Does Mike's work affect this in any way?