[tor-bugs] #33897 [Circumvention/Snowflake]: Remove buffering from WebRTCPeer

[Tor Bug Tracker & Wiki]

#33897: Remove buffering from WebRTCPeer
-------------------------------------+------------------------------
 Reporter:  dcf                      |          Owner:  dcf
     Type:  enhancement              |         Status:  needs_review
 Priority:  Medium                   |      Milestone:
Component:  Circumvention/Snowflake  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:  turbotunnel              |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+------------------------------
Changes (by dcf):

 * status:  assigned => needs_review


Comment:

 https://gitweb.torproject.org/user/dcf/snowflake.git/log/?h=bug33897&id=6bfeabab1e0097afce941fef2de8f13a9a582826
 https://gitweb.torproject.org/user/dcf/snowflake.git/diff/?h=bug33897&id=6bfeabab1e0097afce941fef2de8f13a9a582826&id2=5b7cfafd9e64dcda88eaaa9109c28edee493b7a9

 The first few commits are refactoring changes covered by other tickets:
 #33994, #33995, #33996, #33997, #34002. The new changes for this ticket
 start with
 [https://gitweb.torproject.org/user/dcf/snowflake.git/commit/?h=bug33897&id=9e933b0083e5f9d9f99002abac9ce3e57a8c42e9
 Eliminate separate WebRTCPeer.Connect method].

 My overall approach has been to make `NewWebRTCPeer` return a ready-to-use
 connection. Formerly, you had to call `NewWebRTCPeer`, then separately
 call the `Connect` method, and then at some time in the future the
 connection would actually happen (or fail, as the case may be). There was
 no way to detect a failed connection other than by the connection closing
 itself through a `checkForStaleness` check. In contrast, now,
 `NewWebRTCPeer` waits until it is connected before returning, or else
 returns an error or timeout indicating that the data channel couldn't be
 connected.

 Unfortunately I had to delete the tests touching `WebRTCPeer`. I couldn't
 make them work, because they relied on the separate create/connect
 interface that this branch eliminates. 3 or the 5 tests test features that
 this branch removes; the value of the other 2 is arguable but I don't
 think they are critical to have.

 The simplified external interface allows simplifying the internal
 implementation. Besides `buffer`, I was able to remove or reduce the scope
 of `config`, `broker`, `offerChannel`, `answerChannel`, and `lock`.

 There's a new constant `DataChannelTimeout` that's separate from
 `SnowflakeTimeout` (the latter is what `checkForStaleness` uses). I've
 left `DataChannelTimeout` set to the same value of `SnowflakeTimeout`, 30
 seconds, but I think we can set `DataChannelTimeout` to a lower value, to
 quickly dispose of that cannot be connected to, while still being
 conservative in discarding once-working proxies. I'm testing a browser now
 with `DataChannelTimeout` set to 5 seconds.

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