[tor-bugs] #30992 [Core Tor/Tor]: circpadding: Circsetup machines give out warnings when client-side intro gets NACKed

[Tor Bug Tracker & Wiki]

#30992: circpadding: Circsetup machines give out warnings when client-side intro
gets NACKed
----------------------------------------+----------------------------------
 Reporter:  asn                         |          Owner:  (none)
     Type:  defect                      |         Status:  new
 Priority:  Medium                      |      Milestone:  Tor:
                                        |  0.4.1.x-final
Component:  Core Tor/Tor                |        Version:  Tor:
                                        |  0.4.1.1-alpha
 Severity:  Normal                      |     Resolution:
 Keywords:  wtf-pad circpad 041-should  |  Actual Points:
Parent ID:                              |         Points:  0.4
 Reviewer:                              |        Sponsor:
----------------------------------------+----------------------------------

Comment (by mikeperry):

 Ok let me try to summarize things here. There are two bugs. The first is
 that 0.4.0.x middle nodes are listing padding support, which results in
 the log:
 {{{
 Jul 31 14:25:29.971 [warn] Middle node did not accept our padding request
 on circuit 7 (7)
 }}}

 I filed #31356 for this.

 The second bug *also* has that warn, and *also* to 0.4.0.x relays not
 supporting padding. This is what happens:

 1. We try to negotiate padding with a 0.4.0 relay that doesn't actually
 support it
 2. Before the relay can tell us "no", the introduction point fails, and
 the purpose changes from CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT back to
 CIRCUIT_PURPOSE_C_INTRODUCING.
 3. Because the client intro padding machines do not support
 CIRCUIT_PURPOSE_C_INTRODUCING purpose circuits, they shut themselves down,
 sending a STOP command to the 0.4.0 relay
 4. Then, the original padding reponse from step 1 above finally comes
 back, as an error response (since the 0.4.0.x relay did not support
 padding).
 5. Because we got an error response back, we tear down our all of our
 padding machine state from the circuit.
 6. Then, the response to the STOP finally comes back, but there is no
 padding machine anymore, so the padding is from the "wrong hop", causing
 this warn:
 {{{
 Jul 31 14:25:30.467 [warn] Padding negotiated cell from wrong hop on
 circuit 406
 }}}

 Fixing #31356 will stop this chain of events from happening.. But because
 this cell pattern is different, we're not properly concealing these failed
 intro point requests by our circuit padding, because we negotiate
 ourselves on and off, and because there are extra intro cells and extends
 here..

 Worse than this, if we fix #31356, the chain of events will become:

 1. We try to negotiate padding with a 0.4.1.x relay
 2. Before the relay can tell us "yes", the introduction point fails, and
 the purpose changes to CIRCUIT_PURPOSE_C_INTRODUCING.
 3. The client padding machines sends the STOP command
 4. Then the original padding negotiate response comes back ok. We accept
 it, but do nothing.
 5. Before the STOP response comes back, the introduction point switches
 back to CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT, causing us to try to
 negotiate padding again.
 6. The STOP response comes back, causing us to free all machine state.
 7. The negotiated OK response comes back, again from the "wrong hop".

 I didn't actually see the 0.4.1 version of the sequence in the logs,
 probably because there are not enough 0.4.1 relays yet. but it can happen.

 One fix is to add a check in circpad_handle_padding_negotiated() to ignore
 STOP commands if we're not expecting them.. But that removes the ability
 for relay-side circuit padding machines to shut down the client ones.
 Hrmm..

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