[tor-bugs] #7691 [Tor]: Path bias code should probe unusable circuits

[Tor Bug Tracker & Wiki]

#7691: Path bias code should probe unusable circuits
-----------------------------+----------------------------------------------
 Reporter:  mikeperry        |          Owner:  mikeperry         
     Type:  enhancement      |         Status:  needs_revision    
 Priority:  major            |      Milestone:  Tor: 0.2.4.x-final
Component:  Tor              |        Version:                    
 Keywords:  MikePerry201212  |         Parent:                    
   Points:                   |   Actualpoints:  4                 
-----------------------------+----------------------------------------------

Comment(by mikeperry):

 Replying to [comment:7 nickm]:
 > Reading the second one now.
 >
 > --
 > {{{
 >  /// XXX: Generate a random 0.a.b.c address
 > }}}
 > This is trivial to do.  It could be approximately:
 > {{{
 >   char *probe_nonce;
 >   uint32_t addr;
 >   crypto_rand(&addr, sizeof(addr));
 >   addr &= 0x00ffffff;
 >   probe_nonce = tor_dup_ip(addr);
 > }}}

 Implemented and tested. Seems to work fine. Thanks for pointing out the
 helpers. I am obviously somewhat ignorant to most of the support stuff in
 src/common.

 > (unchecked).  The port should probably be random too, right?

 I opted not to randomize the port because it is not echoed in the reply.
 It seemed not to add any actual security for this reason, and keeping it
 set at 25 saved me from worrying about potential special cases that might
 exist (like 0?).

 > -- The documentation is sparser than I would prefer.  Like, "Sends a
 probe down a circuit that wasn't usable."  Not usable how?  A probe of
 what type?  For what purpose?  (I know the answers, but a reader later on
 is going to have to figure out what's going on here.)
 >
 > -- Forcing the response length to be 9 seems wrong.  As a rule, we allow
 more bytes than expected and ignore them.
 >
 > -- No raw memcmp calls; either tor_memeq() or fast_memeq() will be
 correct.
 >
 > -- The cell_t* argument to pathbias_check_probe_response should probably
 be const.
 >
 > Other than that, it seems plausible so far.

 I think all of these should be fixed in mikeperry/bug7691-rebased. I
 rebased it to origin/master to retest it briefly, but it should be the
 same two commits plus one commit each for the review changes for each bug
 (#7341 and this one). The top commit hash is
 d05ff310a5547b15433314617d6f1b9e9ccfe5b8.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7691#comment:8>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online