Re: Squeezing non-relays at the entry node

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Sun, Feb 21, 2010 at 11:34 PM, Roger Dingledine <arma@xxxxxxx> wrote:
> On Sun, Dec 13, 2009 at 08:23:14PM -0500, Roger Dingledine wrote:
>> +  if (r || router_get_consensus_status_by_id(id_digest)) {
>> +    /* It's in the consensus, or we have a descriptor for it meaning it
>> +     * was probably in a recent consensus. It's a recognized relay:
>> +     * give it full bandwidth. */
>> +    conn->bandwidthrate = (int)options->BandwidthRate;
>> +    conn->read_bucket = conn->bandwidthburst = (int)options->BandwidthBurst;
>> +  } else { /* Not a recognized relay. Squeeze it down based on the
>> +            * suggested bandwidth parameters in the consensus. */
> [snip]
>> As you can see, I'm making it configurable inside the consensus, so we
>> can experiment with it rather than rolling it out and then changing our
>> minds later. I don't have a good sense of whether it will be a good move,
>> but the only way I can imagine to find out is to try it.
>
> I put that feature into Tor 0.2.2.7-alpha. Now there's a followup feature
> I want to put into 0.2.2.10-alpha:
>
 [...]

The patch looks not implausbile.  Before we merge it and turn it on,
would it be possible to run it on an actual exit node for a day or two
to see how much traffic it actually blocks in practice?

[...]
> The only other question here is how to fail the stream -- that is, what
> reason to send back. We still send back END_STREAM_REASON_TORPROTOCOL in
> the case of or_circ->is_first_hop, since that's clearly against what's
> written in tor-spec.txt.
>
> Should we reject people not listed in the consensus with
> TORPROTOCOL too? The chance of false positives is higher. Check out
> edge_reason_is_retriable() in relay.c:
>
> /** Return 1 if reason is something that you should retry if you
>  * get the end cell before you've connected; else return 0. */
> static int
> edge_reason_is_retriable(int reason)
> {
>  return reason == END_STREAM_REASON_HIBERNATING ||
>         reason == END_STREAM_REASON_RESOURCELIMIT ||
>         reason == END_STREAM_REASON_EXITPOLICY ||
>         reason == END_STREAM_REASON_RESOLVEFAILED ||
>         reason == END_STREAM_REASON_MISC;
> }
>
> If we want the client to retry the stream somewhere else (to handle false
> positives more smoothly), we want to use one of these. RESOURCELIMIT,
> EXITPOLICY, and MISC are plausible choices. If we choose EXITPOLICY or
> RESOURCELIMIT (but not MISC), we'll call
>            policies_set_router_exitpolicy_to_reject_all(exitrouter);
> which on first glance seems like a good idea -- it means we will avoid
> that router in the future on the theory that one broken attempt is an
> indication of future results. (Its exit policy will get reset the next
> time a descriptor is parsed for it.) But on further thought, if the
> false positives here are randomly distributed, we don't actually want
> to avoid that router for a whole day. The ...reject_all() idea was
> introduced back in directory v1, when you got a new descriptor for the
> relay every hour. With the new microdescriptor plan, you might not
> refresh the relay's exit policy for a week.
>
> So I went with MISC.
>
> How's my logic? If you like it, I'll try to summarize it in a comment
> when I put the patch in.

Your logic seems fine, but it shows a weakness in our protocol: If I
were redesigning this from scratch, I'd want a new "you don't look
like an OR to me!" reason, but we can't introduce new reasons if we
want them to be treated as retriable things by existing clients.

Somebody should write a proposal to carve off unallocated section of
the various end-reason spaces for retriable errors and non-retriable
errors, so that future extensions here can be smarter.

-- 
Nick