[tor-bugs] #28780 [Core Tor/Tor]: circpadding: Add machine flag for not closing circuit if machine is active

[Tor Bug Tracker & Wiki]

#28780: circpadding: Add machine flag for not closing circuit if machine is active
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  (none)
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Very High                            |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  wtf-pad, tor-relay, tor-cell,        |  Actual Points:
  padding, 041-proposed, network-team-           |
  roadmap-2019-Q1Q2                              |
Parent ID:  #28632                               |         Points:  5
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor2
-------------------------------------------------+-------------------------
Changes (by mikeperry):

 * status:  needs_revision => needs_review


Comment:

 Ok I think the best way to handle comment:8 is to not mark circuit for
 close upon transition to the end state. Then,
 circuit_expire_old_circuits_clientside() should cull them as per existing
 dirtyness timeouts. This is done in an extra fixup commit on
 https://github.com/torproject/tor/pull/644.

 However, I am still going back and forth between purpose vs flag. Here's
 the Pro/Con:

 Purpose:
 * Pros:
   1. We don't have to worry about hidden service code and circuit
 prediction code getting confused by outstanding circuits of a particular
 type, with particular state, that they thought they closed.
   2. In logs, controller handling, and everywhere else in the code, it is
 very clear what this circuit is doing and what it is for. Code is very
 unlikely to accidentally use it for something else.
 * Cons:
   1. State machines must remember to include CIRCUIT_PURPOSE_C_PADDING in
 their purpose masks, or they will shut down as soon as the purpose changes
 and cause chaos when other machines pick up this purpose.
   2. Changing purposes of things is a little scary. But we already do it;
 the code should support it (but: #29034 is one major outstanding instance
 where it does not properly do so).


 Flag:
 * Pros:
   1. Our padding machine conditions are simpler. They can apply to just
 the purpose they are for, and not worrying about adding this extra padding
 purpose if they happen to use the "please keep circ open" feature.
 * Cons:
   1. Lots of wide-ranging code can be confused because it tried to close a
 circuit, but then that circuit stays around with the same purpose, and
 does not have its marked for close flag set. It may try to re-use these
 circuits in surprising places.

 After this analysis, I am inclined to say that the purpose-based way is
 the way to go here. So I'm going to set this to needs_review. Please do
 let me know if there are other pros/cons.

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