Re: [tor-bugs] #16861 [Tor]: Pad Tor connections to collapse netflow records

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#16861: Pad Tor connections to collapse netflow records
-----------------------------+--------------------------
     Reporter:  mikeperry    |      Owner:  mikeperry
         Type:  enhancement  |     Status:  needs_review
     Priority:  normal       |  Milestone:
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+--------------------------
Changes (by nickm):

 * cc: asn (removed)


Comment:

 Preliminary review, attempts to answer questions, etc.  (Can't promise
 anything with an 0.2.7 merge yet, got to see what other folks think of the
 code and design. The timeframe is very tight. :)

 Preliminary review, high-level requirements stuff.  This doesn't all need
 to be on Mike, but it *does* all need to get done.
   * Needs-proposal.  There should be one in enough detail (and there may
 *already* be one in enough detail!) that somebody else could make code
 that works the same.  I bet that this would be a nice short proposal that
 wouldn't take too long.  But it needs a spec-level proposal all the same.
 The proposal also needs to get review and attention.
   * All the functions and structures need documentation.
   * All new stuff in options needs documentation in doc/tor.1.txt.in
   * Needs a changes file.
   * Needs tests.

 Nitty-gritty code stuff:
   * In C, functions that take no arguments are declared `int foo(void)`,
 not `int foo()`.
   * Why does `get_rep_hist-padding_count_lines` return NULL when read or
 write count is 0?  Should that be an && ?
   * You can't format uint64_t as %ld.  Use the magic U64_FORMAT and
 U64_PRINTF_ARG macros instead.
   * I'd suggest "has_been_used_for_nondir_circs" instead of
 "used_for_nondir_circs".  The latter could be confused to mean "should
 only be used for...".
   * connection_run_housekeeping is already huge; it would be great if we
 could extract as much as possible of the new code into a function.
   * Looks like there's a memory leak on pad_event and conn_id_ptr in
 launch_netflow_pending_callback.
   * "Scheduled a netflow padding cell, but connection already closed." --
 this probably shouldn't be notice; I bet it will trigger often.
   * Are you casting away the const on chan in send_netflow_padding?
 That's a bit scary to me.
   * DFLT_NETFLOW_INACTIVE_KEEPALIVE_MAX should probably be a function so
 it's clear that something is happening inside it.

 Medium-level design stuff:
   * How badly does this fail when we don't have monotonic time?
   * gettimeofday() is basically free on Linux, but it's more expensive
 elsewhere. We need to figure out how expensive it is and actually figure
 out whether (say) cached_
   * Libevent doesn't like it so much when we have tens of thousands of
 timers added in random order. It's O(lg n) to add or remove one, and IIRC
 the algorithm starts to get sad around that point. We'd better make sure
 this doesn't matter.
   * I think connection_get_by_global_id() does a linear search. I bet that
 won't be affordable.

 Overall comments, first impressions:
   * I dig the idea of tricking netflow hardware.  A pox upon them!
   * I wonder if we can abstract this code so that the logic of when to
 generate packets is separated a bit more from the logic that sends them,
 so we can drop in other algorithms more modularly in the future.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16861#comment:5>
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