[tor-bugs] #25598 [Circumvention/Snowflake]: Let the broker inform proxies how often to poll
Tor Bug Tracker & Wiki
blackhole at torproject.org
Sun Aug 25 22:06:44 UTC 2019
#25598: Let the broker inform proxies how often to poll
-------------------------------------+--------------------------------
Reporter: dcf | Owner: (none)
Type: enhancement | Status: needs_revision
Priority: Medium | Milestone:
Component: Circumvention/Snowflake | Version:
Severity: Normal | Resolution:
Keywords: starter | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-------------------------------------+--------------------------------
Changes (by dcf):
* status: new => needs_revision
Comment:
Replying to [comment:5 serna]:
> Here's how it would work with a hardcoded 20sec poll rate:
https://github.com/BubuAnabelas/snowflake/pull/1
Thanks for this patch.
https://github.com/BubuAnabelas/snowflake/pull/1/files#diff-
ba4bd8a4477426567c409d66c2cf8a28R175-R176
{{{
+ nextPoll, _ := strconv.Atoi(resp.Header.Get
("Snowflake-Next-Poll"))
+ pollInterval = time.Duration(nextPoll) *
time.Second
}}}
The `Atoi` needs an error check. Otherwise a parse error or missing header
may return a value of 0 and make the proxy start trying to DoS the broker
:)
There should also be a safety minimum, so the proxy will not obey if the
broker asks it to go ''too'' fast. Cf.
[https://gitweb.torproject.org/flashproxy.git/tree/proxy/flashproxy.js?id=0c5ea1b04c4725fccc5a98a25bede5c419e07fcd#n545
the corresponding code in flashproxy].
I propose factoring out a separate function that takes a `string` and
returns a `time.Duration`. It parses the string and enforces a minimum. In
case of a parse error, it returns a default.
https://github.com/BubuAnabelas/snowflake/pull/1/files#diff-
ba4bd8a4477426567c409d66c2cf8a28L29-R30
{{{
-const pollInterval = 5 * time.Second
+var pollInterval = 20 * time.Second
}}}
Now that this is variable, it is better to limit its scope to the context
in which it is used (not a global variable if possible). The constant
default value can remain global.
https://github.com/BubuAnabelas/snowflake/pull/1/files#diff-
177f47700613253ab3ba906035e86714R49
{{{
snowflake.config.brokerPollInterval = xhr.getResponseHeader
('Snowflake-Next-Poll') * 1000 || snowflake.config.brokerPollInterval;
}}}
It looks like the check for a missing header needs to go before the
multiplication by 1000, not after. Here too I recommend factoring out a
function that checks the syntax and enforces a minimum. (Think of the
proxy trying to defend itself from a malfunctioning broker.)
It's better if the code doesn't modify things in the global `Config`
object. The more you can localize the poll interval, the better.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25598#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list