[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_information
 Priority:  Very High                            |      Milestone:  Tor:
                                                 |  0.4.1.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  wtf-pad, tor-relay, tor-cell,        |  Actual Points:  6
  padding, 041-proposed, network-team-           |
  roadmap-2019-Q1Q2                              |
Parent ID:  #28634                               |         Points:  5
 Reviewer:  asn                                  |        Sponsor:
                                                 |  Sponsor2
-------------------------------------------------+-------------------------

Comment (by asn):

 Here is a suggestion and implementation plan for an invariant we could use
 here to minimize unseen bugs:

  * Make a new soft-assert function (e.g. `assert_circuit_expiry_ok()`)
 which gets called at the end of
 `circuit_expire_old_circuits_clientside()`.
  * Also abstract the "has this circuit expired?" logic of
 `circuit_expire_old_circuits_clientside()` into its own function so that
 we can use it.
  * Go through the list of circuits: If a circuit is in
 `CIRCUIT_PURPOSE_C_CIRCUIT_PADDING` purpose, then examine it further.
  * Soft-assert that for a circuit to be in that purpose, it means that:
    * If it has no machine, then the circuit has not expired yet (using
 helper function above). With this we want to catch PADDING circuits whose
 machine got shutdown.
    * If there is a machine, then:
      * manage_circ_lifetime == 1
      * Machine has either not ENDed, or if it has ENDed the circuit has
 not expired yet (using helper function).

 I think the above should guard us from most bugs that could result in
 `PADDING` circuits staying around for ever, as long as
 `circuit_expire_old_circuits_clientside()` indeed gets called
 periodically. Perhaps we can add another safeguard to make sure that the
 expiry function indeed gets called periodically.

 Finally, the above logic is not particularly optimized for performance, as
 it does another loop over the circuit list. We could optimize it by doing
 it inline the `circuit_expire_old_circuits_clientside()` but we should
 make sure that it does not increase the tech-debt and complexity of the
 function.

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