[tor-bugs] #25429 [Circumvention/Snowflake]: Need something better than client's `checkForStaleness`

[Tor Bug Tracker & Wiki]

#25429: Need something better than client's `checkForStaleness`
--------------------------------------------+------------------------------
 Reporter:  arlolra                         |          Owner:  cohosh
     Type:  defect                          |         Status:  new
 Priority:  Medium                          |      Milestone:
Component:  Circumvention/Snowflake         |        Version:
 Severity:  Normal                          |     Resolution:
 Keywords:  anti-censorship-roadmap-2020Q1  |  Actual Points:
Parent ID:  #19001                          |         Points:
 Reviewer:                                  |        Sponsor:
                                            |  Sponsor28-must
--------------------------------------------+------------------------------

Comment (by cohosh):

 Replying to [comment:18 dcf]:
 > Replying to [ticket:25429 arlolra]:
 > > but there's no heartbeat at this level of abstraction so the
 connection is constantly being reset anytime the user pauses their
 activity (for example, to read a webpage).
 >
 > This statement is no longer true since #33745; smux sends a heartbeat
 every 10 seconds in both directions (inherited from
 [https://github.com/xtaci/smux/blob/v1.5.12/mux.go#L40 smux.DefaultConfig]
 [https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/tree/client/lib/snowflake.go?id=7043a055f9fb0680281ecffd7d458a43f2ce65b5#n81
 here] and [https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/tree/server/server.go?id=7043a055f9fb0680281ecffd7d458a43f2ce65b5#n300
 here]).
 >
 > The problem is also mitigated by #34042, which uses a different
 (shorter) timeout to discard a non-working proxy originally. It still
 takes 30 seconds via `checkForStaleness` to give up on a proxy that was
 once working.

 Cool.

 It looks like there's another value for `KeepAliveTimeout` in the
 [https://github.com/xtaci/smux/blob/v1.5.12/mux.go#L45 defaultConfig] and
 the session will close itself if it hasn't received any data during this
 timeout
 [https://github.com/xtaci/smux/blob/c6969d8a76874342611f4b544d05b1590f1a76b1/session.go#L393
 here]. This defaults to 30 seconds (which is the same as our current check
 for staleness).

 Although if the session closes, it should cause the main copy loop to
 break [https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/tree/client/lib/snowflake.go#n161 here]. It seems
 like we don't want this to happen if our snowflake stops working, and
 instead we want to try to get new snowflakes before our session fails.

 So now that we have keep-alive pings, we should be able to shorten this
 staleness check. Does it make sense to shorten the pings to every 5s and
 make the staleness check 10s? This would allow for 2 additional snowflake
 tries before our session breaks.

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