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:               |
-----------------------------+--------------------------

Comment (by mikeperry):

 Ok, I just pushed fixes for Nick's review to the neflow_padding-squashed
 branch, and have a new squashed patch at
 https://gitweb.torproject.org/mikeperry/tor.git/commit/?h=netflow_padding-
 squashed2.

 Everything has been refactored and reorganized into channelpadding.{c,h},
 and the code is generally a lot more organized and properly abstracted to
 use channel_t. Channel lookup during timer invocation is now O(1), but
 this will only work for TLS-based channels (because connection_t has an
 index into the connection array that allows O(1) lookups, but channel_t
 does not provide O(1) lookups based on its global_identifier).

 The following issues remain, which I will fix later:
  * Still needs a proposal
  * Still needs tests
  * Still needs a changes file
  * Still has lots of questions for folks listed in comment:1 and in the
 XXX comments in the code.

 The following were non-issues as far as I can tell:
  * rep_hist_get_padding_count_lines() deliberately omits the extra-info
 lines if either the read *or* write cells were empty, because in either
 case we don't have enough info to safely publish stats.
  * I don't think I actually leaked conn_id_ptr. In any case, I refactored
 that code and now pass an allocated struct. I don't think I leak that one
 either.
  * I don't think I was casting away a const in send_netflow_padding().
 That function is now called channelpadding_send_padding_cell_callback().
 The cast has been replaced by proper macro usage, but there was no const
 there anyway, unless I missed something?
  * If the clock jumps, at worst we would have emitted a warn about the
 padding time being in the past. Now we only emit a notice. Should we
 double-check here anyway somehow?

 I'm still now sure what to do about the following:
  * Am I still leaking pad_event from tor_evtimer_new in
 channelpadding_schedule_padding()? I cargo-culted that from
 dns_launch_correctness_checks() in dns.c, so if I'm leaking, that function
 probably is too..
  * Is there a way to test for a slow gettimeofday()? I noticed some
 TIME_IS_FAST ifdefs in util.c, but nothing sets that define. We *can* use
 time() if we need to, and it will still work fine, but we'll end up
 sending more padding due to truncation error in that case (which is why I
 added timestamp_active_highres in the first place).
  * How should we check if there are too many libevent timers scheduled?
 Note that the code didn't (and still doesn't) schedule a callback unless
 we're within 1 second of the padding timeout. It just waits for the next
 invocation in those cases. That should mean that even if all connections
 are always idle, only 1/10 of them are scheduling timer callbacks (because
 the function is called once per second, and the timeout range is 10
 seconds wide). We can still check for to many timer callbacks anyway, and
 call directly in that case, but how do I do that? I've added an XXX in the
 code where we'd need to do this.

 There are still lots of XXX's in the code for my other questions.

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