[tor-bugs] #25552 [Core Tor/Tor]: prop224: Onion service rev counters are useless and actually harmful for scalability

[Tor Bug Tracker & Wiki]

#25552: prop224: Onion service rev counters are useless and actually harmful for
scalability
-----------------------------------------------+---------------------------
 Reporter:  asn                                |          Owner:  dgoulet
     Type:  defect                             |         Status:
                                               |  needs_review
 Priority:  Medium                             |      Milestone:  Tor:
                                               |  0.3.4.x-final
Component:  Core Tor/Tor                       |        Version:  Tor:
                                               |  0.3.1.9
 Severity:  Normal                             |     Resolution:
 Keywords:  tor-hs prop224 034-roadmap-master  |  Actual Points:
Parent ID:                                     |         Points:  4
 Reviewer:  asn                                |        Sponsor:
-----------------------------------------------+---------------------------
Changes (by dgoulet):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:26 nickm]:
 > I've reviewed the PR.  The biggest issue is related to the use of "\n"
 signature_str, which I believe should be "\n" signature_str " " instead.

 Yes. Fixed on the PR with fixup `23a4c09edb2c9d46`.

 > 1. How hard is it to DoS this into an OOM condition?  Do we need to tie
 it into the OOM system?  And by doing so, do we subject ourselves to
 replay attacks once again?

 Right so hooking it up to the OOM could affect the replay attack
 protection. Thus, this cache can potentially grow to infinite. However,
 entries are in bytes (32 byte hash + some extra byte). For example, one
 would need to upload this amount of descriptors in a 36 hour period to
 reach the size:

 {{{
 100k - ~3MB
 1M - ~32MB
 5M - ~153MB
 }}}

 What if we add a warning when a certain threshold is reached so we can
 maybe learn if it is happening in the wild at some point?

 Usually, when the OOM goes off on HSDir descriptor, we learn about it
 pretty quickly.

 > 2. On point 1: perhaps the replay cache should be thought of, not as a
 complete replacement for revision counters, but as an  alternative to them
 when they cannot be used?  That is, we could enforce the rule that
 revision counters are ''non-decreasing'', but allow revision counters to
 remain equal, and use the replay cache to handle only the "equal counter"
 case.

 This won't work with the use case we are trying to fix with this patch.

 Basically, multiple services competing (for instance what FB does), the
 last one uploading needs to be the true one winner. If we still support
 revision counter, the service that restarted the last will always be the
 winner.

 So we need to dump the rev. counter usage both at the HSDir and Client
 side for caches all together.

 >
 > That way if we need to rip out the cache because of OOM issues in the
 future, or if we solve the problem of coordinating revision counters
 between distributed HS providers, we aren't stuck with the cache forever.

 Other problem with revision counter. We can't simply dump them because it
 is also used in the encryption key computation by service/client...

 So we need this transition period were (1) we make all client/hsdir make
 it optional in the descriptor and set it to 0 which means if not present,
 client and service uses 0. Then (2) at some point far in time, we can make
 service stop putting it because we believe all client/hsdir have
 updated......

 Really not ideal situation but the best we came up with.

 >
 > 3. As above, the unit tests need fixing.

 Done in fixup `fc45a82b7bceb4ca`

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