[tor-bugs] #27471 [Core Tor/Tor]: HS intermittently fails: Non-fatal assertion failed in send_introduce1

[Tor Bug Tracker & Wiki]

#27471: HS intermittently fails: Non-fatal assertion failed in send_introduce1
-------------------------------------------+-------------------------------
 Reporter:  tgragnato                      |          Owner:  dgoulet
     Type:  defect                         |         Status:
                                           |  needs_revision
 Priority:  Very High                      |      Milestone:  Tor:
                                           |  0.3.5.x-final
Component:  Core Tor/Tor                   |        Version:  Tor:
                                           |  0.3.4.7-rc
 Severity:  Minor                          |     Resolution:
 Keywords:  tor-hs, regression?, 035-must  |  Actual Points:
Parent ID:                                 |         Points:
 Reviewer:  asn                            |        Sponsor:
-------------------------------------------+-------------------------------

Comment (by dgoulet):

 Replying to [comment:12 asn]:
 > Replying to [comment:10 dgoulet]:
 > > This adds the support to close client introduction circuits when a new
 descriptor is replacing an old one so we don't end up with unusable
 circuits leading to what I think the BUG() in this ticket is showing us.
 > >
 > > Branch: `ticket27471_035_01`
 > > PR: https://github.com/torproject/tor/pull/400
 >
 > Not a huge fan of this patch. I feel like I don't understand enough to
 ACK or NACK it.
 >
 > The way I see it, is that this adds 100 non-trivial LoCs for a very
 unlikely edge-case that we think is causing the issue. Basically the
 assumption is that we just completed a rendezvous circuit, and between
 building the circuit and sending an `INTRODUCE1` we happen to have fetched
 a new HS descriptor. Sounds plausible, but I don't quite understand why
 this edge-case would happen to two people so quickly; it seems pretty
 rare. What do we think made the client fetch a new HS descriptor at that
 exact time?

 Right. Well, maybe because more people are getting means it might not be
 that of an edge-case and that HSv3 is being used more.

 The second thing, that code doesn't add "non-trivial" lines imo... The
 majority of the code added (`circuit_get_next_client_intro_circ()`) is a
 known function that we use for many things but this one is specifically
 for client intro. It is extremely well tested over time.

 >
 > Another thing I don't like here is that we are adding a whole new
 "feature" of closing these useless connections that usually don't exist so
 all this new code will be unused 99.9% of the times.

 I don't agree. It is code used every time a client replaces a descriptor
 in its cache. I really doubt this is unused... Considering a service
 changes its descriptor every 3 hours, if the RP connection is simply
 closed between service<->client, this situation will happen.

 >
 > Instead of that I would try to handle the `BUG()` in question more
 gracefully, and in the edge-case where we can't find an `ip` object for
 the circuit, we close the intro circuit, and establish a new one. Seems to
 me like this is a more natural way to do it, and less lines of code. Then
 the useless introduction circuit would just timeout at some point and
 become a measurement circuit or something.

 Doing this would go like:

 The caller (`connection_ap_handshake_attach_circuit()`) of
 `hs_client_send_introduce1()` handles 3 return code: Success, transient
 error and permanent error.

 The former is easy, the second is when the HS subsystem did some actions
 to fix the problem so the stream can go back in "wait mode". Third one
 will close the SOCKS conn.

 In this case, we would need to return a transient error meaning once we
 realize we can't send the INTRO1 cell, we will want to reextend the
 failing intro circ to a new IP and wait. Not too bad. The bad circuit
 would be reused meaning won't be looked up again.

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