[tor-bugs] #11092 [Obfsproxy]: scramblesuit should make sure that handshake padding is less than MAX_PADDING_LENGTH

[Tor Bug Tracker & Wiki]

#11092: scramblesuit should make sure that handshake padding is less than
MAX_PADDING_LENGTH
---------------------------+-----------------------------
     Reporter:  asn        |      Owner:  asn
         Type:  defect     |     Status:  needs_review
     Priority:  normal     |  Milestone:
    Component:  Obfsproxy  |    Version:
   Resolution:             |   Keywords:  pt-scramblesuit
Actual Points:             |  Parent ID:
       Points:             |
---------------------------+-----------------------------

Comment (by yawning):

 In util.locateMark(mark) change "index = payload.find(mark)" to be bounded
 like :
 {{{
     index = payload.find(mark, 0, const.MAX_PADDING_LENGTH +
                                   const.MARK_LENGTH)
 }}}

 Without the change, it is still possible to accept out-of-spec packets.

 The current code no longer buffers data forever, but it still continues to
 process the handshake till the threshold is reached.  In
 ScrambleSuitTransport.receivedDownstream consider doing something like:
 {{{
     if self.weAreServer and (self.protoState == const.ST_AUTH_FAILED):
         self.rxHandshakeLength += len.data()
         data.drain(len.data())
         if self.rxHandshakeLength > self.srvState.closingThreshold:
             log.info("Terminating connection after having received %d"
                      " bytes because client could not "
                      "authenticate." % self.rxHandshakeLength)
             self.circuit.close()
             return
     else if self.weAreServer and (self.protoState ==
 const.ST_WAIT_FOR_AUTH):

         # blah blah blah

         else:
             if len(data) > const.MAX_HANDSHAKE_LENGTH:
                 self.rxHandshakeLength = len(data)
                 data.drain(self.rxHandshakeLength)
                 self.protoState = const.ST_AUTH_FAILED
                 log.debug("Authentication failed after having received %d"
                           " bytes, will close after peer trips the
 threshold."
                           % self.rxHandshakeLength)
                 return
 }}}

 Add const.MAX_HANDSHAKE_LENGTH, const.ST_WAIT_FOR_AUTH,
 self.rxHandshakeLength as appropriate (NB: Dry code.  May have errors, but
 I hope the idea is clear.).  This avoids doing any data processing after
 it has been clear that the handshake will never succeed and jettisons the
 invalid data immediately.

 (At first I was like "well the changes that I'm about to suggest will be
 trivial I can just inline them in the bug", but maybe I would have been
 better off writing this as a patch.  Sorry.)

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