[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_revision
 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
-------------------------------------------------+-------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:35 mikeperry]:
 > Ok, in addition to the pathbias fix and log improvements above, I pushed
 two more additional commits to the PR:
 >
 > 1.
 https://github.com/torproject/tor/pull/966/commits/54bc9f59c0b52f75de76872db7fc872dc4f8f7f4
 - Check for liveness while holding open padding circuits.
 > 2.
 https://github.com/torproject/tor/pull/966/commits/d03035c8a68f1c0201167d106de703a9ebf2f64a
 - Refactor and clarify hold open logic.
 >
 > With these two commits, it should be much easier to verify that it is
 not possible for circpad to hold open a circuit if more than
 CIRCPAD_DELAY_INFINITE==UNIT32_MAX microseconds (or about 1.25 hours) pass
 with no circuit cell delivery events happening. I have not written tests
 for this yet, but if we like this approach, I can.
 >
 > I am open to adding additional checks to
 circuit_expire_old_circuits_clientside(), but I want to temperature check
 how people felt about this handbrake-style lifespan check in the first
 place, because that's the type of thing I'd add to
 circuit_expire_old_circuit_clientside() first.
 >
 > I can still be persuaded to eliminate circuit_mark_for_close() and make
 it super clear for callers that they must pick between a new error-close
 version and a differently-named normal-close version, and have each
 version assert if they are called with reason codes that should be used
 with the other version, but that change will be invasive and I am not sure
 that will actually save us from circuit-leak errors (which will actually
 arise from circuit_expire_old_circuits_clientside()).

 Hey Mike,

 I like this approach but I would like to hear Nick's opinion too.

 I think the general concept here is general enough to detect all types of
 "circuit left open for ever" failures. Unittests on this specific
 functionality would be useful to get more confidence, and we should also
 test it on the real net.

 I also left some comments on the PR.

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