[tor-bugs] #30992 [Core Tor/Tor]: circpadding machines have shutdown sync issues (with intro circ NACKs and other cases)

[Tor Bug Tracker & Wiki]

#30992: circpadding machines have shutdown sync issues (with intro circ NACKs and
other cases)
------------------------------------------+--------------------------------
 Reporter:  asn                           |          Owner:  mikeperry
     Type:  defect                        |         Status:  assigned
 Priority:  Medium                        |      Milestone:  Tor:
                                          |  unspecified
Component:  Core Tor/Tor                  |        Version:  Tor:
                                          |  0.4.1.1-alpha
 Severity:  Normal                        |     Resolution:
 Keywords:  wtf-pad circpad 042-proposed  |  Actual Points:
Parent ID:                                |         Points:  3
 Reviewer:                                |        Sponsor:
------------------------------------------+--------------------------------

Comment (by mikeperry):

 Replying to [comment:12 asn]:
 > Replying to [comment:11 mikeperry]:
 > I admit I don't understand exactly our "machine replace" ideal behavior
 and what goals we have for it. Can you please expand on the above and what
 other goals we have here?

 The machine replacement is meant to allow us to use the
 [https://github.com/torproject/tor/blob/maint-0.4.1/src/core/or/circuitpadding.h#L149
 machine conditions] to apply different padding machines to different
 phases of circuit lifetime.

 For example, our current padding machines only deal with circuit setup. We
 may want to add future padding machines that apply to REND circuits that
 are built and have streams on them. To do this, we would specify the
 additional circuit state condition CIRCPAD_HAS_STREAMS on the machines
 that add padding to rend circuits that already have streams on them. These
 machines would not apply during the setup phase, but would apply after we
 have padded the rend and it has been joined. So our current machines would
 shut down, and these would start up.

 Another example might be something like "We only want to pad on circuits
 that have active streams on them to port 22, to try to make SSH circuits
 look more like web circuits". For these, we would implement #29083 as a
 machine condition. Then, machines could spin up and shut down on a circuit
 only while it has port 22 streams on it (which could be opened and closed
 repeatedly).

 > > I think my preference is for 2, but maybe a protover bump is cleaner
 anyway, since we already have to mess with it for #31356, and bumping it
 to 2 would avoid the need for any 0.4.0 patching.
 >
 > I agree that '2' seems like a plausible solution here. Are the
 PADDING_NEGOTIATE(D) cells extensible enough to add such a thing? Or how
 do we go about it?

 Yes. The circpad negotiation cells have version fields, so we can add
 extra fields for them that are only checked if the version value is high
 enough for them to be present.

 > Also, can you explain how the "machine counter" logic of '2' would work
 with multiple desynched START/STOP commands flying in the air like in
 comment:7 and comment:10? Would we ignore any START/STOP commands that
 don't correspond to our latest state? And how would that work for the
 issue in comment:10? Could that create more issues?

 Yes, we would ignore any commands that did not correspond to a machine
 counter for our most recent machine. This will solve the comment:7
 sequence. For the comment:10 sequence, the key implementation detail is
 that we have to update the "current machine counter" upon shutdown, so
 that after we tear our machines down, all previous in-flight stops, etc
 will still be recognized as from the previous machine.

 A related question is if we should treat these cells as "dropped" cells
 for side channel defenses like vanguards... I think the right answer here
 is to still retain the hop number of the previous machine, as well, so we
 can verify that these invalid commands are not injected from say, the exit
 node as a side channel vector.

 Anyway, that is my current thinking.. The side channel piece of this might
 be solved with another approach, though..

 All of this complication is why it's smartest just to let this be for now,
 especially since fixing it for 0.4.1 won't change any of the
 fingerprintability of these circuits..

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