[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #27319 [Core Tor/Tor]: remove buf_get_oldest_chunk_timestamp()



#27319: remove buf_get_oldest_chunk_timestamp()
-------------------------------------+----------------------------------
 Reporter:  cypherpunks3             |          Owner:  (none)
     Type:  defect                   |         Status:  new
 Priority:  Medium                   |      Milestone:  Tor: unspecified
Component:  Core Tor/Tor             |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:  datatypes, buffers, oom  |  Actual Points:
Parent ID:  #23878                   |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+----------------------------------

Comment (by nickm):

 Replying to [comment:3 cypherpunks3]:
 > Replying to [comment:2 nickm]:
 > > we need to track the age of the data in the buffer, to implement the
 defense for this: https://www.nrl.navy.mil/itd/chacs/sites/edit-
 www.nrl.navy.mil.itd.chacs/files/pdfs/13-1231-3743.pdf
 > >
 >
 > Thanks for the additional context.
 >
 > Could we
 >
 > - replace the per-chunk timestamp with a timestamp stored in `struct
 connection_t`, which is updated with the last point in time that `outbuf`
 had been empty? Move age-calculation logic there.

 I would worry about this one -- the authors found that the methodology in
 their paper was effective against the attack they found, but some other
 methodologies were not.  The algorithm you suggest here would change the
 behavior -- instead of killing the connections that had been stalled for
 the longest, it would kill connections that had been making steady
 progress if they had never flushed fully.

 > - Or if not that, keep a uint32_t field per chunk, but have it be an
 opaque tag kept at zero normally, but callers that care about timestamps
 like connection.c can call `buf_tag_new_chunks(buf_t, uint32_t)` after
 writing to the buffer, assigning that uint32_t to any chunk that currently
 has a tag of zero (ie, any newly created chunks). Then OOM can call
 `buf_get_head_tag()` to retrieve the number and calculate its age.

 Hm.  So, "tag_new_chunks" sounds like it would require us to be able to
 walk the buffer's chunks in both directions, when currently it's only a
 singly-linked queue.  Either that, or it would be O(N) to reach the end.

 Instead, we could have something where we call
 "buf_set_tag_for_new_chunks" preemptively, and then have the buffer
 backend copy that tag into any new chunks it allocates.

 Or (maybe) better still, there could be some opaque
 "get_tag_for_new_chunk()" callback that only got called as needed.  That
 way, we wouldn't be doing extra timestamp calls, and the users of the
 buffer API wouldn't need to worry about whether they had called
 buf_set_tag_... before or after all their possible read operations.

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