[tor-bugs] #29627 [Applications/Tor Launcher]: Moat: add support for obfsproxy's meek_lite

[Tor Bug Tracker & Wiki]

#29627: Moat: add support for obfsproxy's meek_lite
---------------------------------------+------------------------------
 Reporter:  mcs                        |          Owner:  brade
     Type:  defect                     |         Status:  needs_review
 Priority:  Medium                     |      Milestone:
Component:  Applications/Tor Launcher  |        Version:
 Severity:  Normal                     |     Resolution:
 Keywords:  TorBrowserTeam201905R      |  Actual Points:
Parent ID:  #29430                     |         Points:
 Reviewer:                             |        Sponsor:
---------------------------------------+------------------------------

Comment (by dcf):

 Replying to [comment:9 mcs]:
 > That is a good question.  We think the code is correct but the spec is
 wrong.  SOCKS5 supports up to 255 bytes in each auth field.  The
 obfs4proxy code reads a byte to get the length and does not have any other
 limitations, so up to and including 255 is supported.  Kathy and I wanted
 to maximize the space available for args, so we used <=.  Do you think
 this is OK?  Should we file a bug against the PT spec?

 There's for sure a small ambiguity in the PT spec, when the length of the
 username field is exactly 255. I almost mentioned it before but thought I
 was being too nitpicky. Suppose the PT receives a connection with a
 255-byte username `k=aaa...aaa` and a 1-byte password `\x00`. There are
 two possible strings with that encoding, and no way to distinguish between
 them:
  * the 255-byte string `k=aaa...aaa`
  * the 256-byte string `k=aaa...aaa\x00`
 Obviously in the PT context `\x00` is an unlikely byte to appear, so in
 this case both goptlib and obfs4proxy disambiguate by taking the first
 interpretation.

 To eliminate the ambiguous case, whenever the length of
 `this.mMeekClientEscapedArgs` is exactly 255, you could put 254 bytes in
 the username field and 1 byte in the password field.

 Though now that I check, technically the spec doesn't even say how the
 username and password fields are supposed to be combined, or whether a
 decoder is even required to look at the password field if the username
 field is not full. But the existing implementations work by concatenation
 without regard to the length of the fields, except that a password field
 consisting of a single `\x00` is treated as an empty string. An
 [https://gitweb.torproject.org/torspec.git/tree/pt-
 spec.txt?id=4dcd7e94f17c072e771119ec90d7cbce4a4788a4#n162 older version]
 of the spec stated the the fields should be joined by concatenation, but
 didn't say what to do with an empty password field.

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