[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #7691 [Tor]: Path bias code should probe unusable circuits
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs