date:20200312

Re: [tor-bugs] #32938 [Circumvention/Snowflake]: Have a way to test throughput of snowflake proxy

2020-03-12 Thread Tor Bug Tracker & Wiki

* status: needs_review => needs_revision

Comment:

The refactoring in proxy-go, allowing proxy-go to treat the broker and the
bridgestrap mostly equivalently, looks reasonable. I don't see this as
something meant to be secure against adversarial proxies, only
psychological reassurance for honest proxy operators. I really don't think
this function should be rolled into the broker; actually I think the
broker should be more compartmentalized overall. Even if it's running on
the same host, I feel it should be a separate process.

I'm having trouble understanding the control flow between `Snowflake` and
`ThroughputLogger` in bridgestrap. `Snowflake.runTest` calls
`Snowflake.MarkWritten` to store a timestamp in `ThroughputLogger`, then
does `Snowflake.Write` which results in calls to `Snowflake.dc.OnMessage`,
which calls `ThroughputLogger.AddBytes`, which writes into a channel read
by `Snowflake.Log`, which then looks at the previously stored timestamp. I
wonder if there's more of a straight-line way to write it.

The high latency you mentioned in comment:9 seems to be a bug. Even in my
localhost test, I get a latency of around 5 seconds. In the
`time.Since(start)` computation, `time.Now()` is increasing faster than
`start` is. In my local test, the difference increased smoothly and
monotonically from 0.04 to 9.66 seconds over 940 iterations in `runTest`.
Maybe there is some kind of buffering happening where packets are "sent"
much faster than they are received; like maybe I can send in 10,000
iterations almost instantly, while really those are being buffered by the
OS and not really sent immediately.

https://gitlab.torproject.org/cohosh/bridgestrap/blob/c76fb1c24eacdeefddab699aa7ac2bf111c5e63f/snowflake.go#L153-160
`AverageLatency` will panic with a division by zero if for whatever reason
`count` is 0 (if there were no messages received). Why round the average
latency to 1 second?

https://gitlab.torproject.org/cohosh/bridgestrap/blob/c76fb1c24eacdeefddab699aa7ac2bf111c5e63f/snowflake.go#L59
The `OnMessage` callback assumes it has at least 4 bytes to work with and
will panic if it does not. The design relies on the blocks sent by
`runTest` retaining message boundaries when they come back into
`OnMessage`, which isn't guaranteed. It's worth thinking about what a
malicious proxy could do by falsifying the `count` value at the beginning
of each buffer. It may be better to do something simpler like: send 10 KB,
receive 10 KB, and only do another iteration once you've received the same
number of bytes that you sent.

I think there's a memory leak in `idToSnowflake` if the test never
completes. A proxy could hit the /api/snowflake-poll route to add entries
to the map and never hit /api/snowflake-test to remove them.

https://gitlab.torproject.org/cohosh/bridgestrap/blob/c76fb1c24eacdeefddab699aa7ac2bf111c5e63f/snowflake.go#L192
[https://golang.org/pkg/math/rand/#Read Rand.Read] is documented never to
return an error, so I would prefer a panic rather than an error return
here.

`APISnowflakeRequest` and `APISnowflakeTest` need a byte limit to prevent
someone sending an infinite JSON object and using up all memory. A read
deadline would make sense, too.

An alternative design would be to reverse the direction of traffic flow.
Let the proxy send data and bridgestrap reflect it. The proxy can compute
its own throughput and latency locally. The bridgestrap part could then be
made stateless except for the offer–answer matching.

Like you, I'm not sure of the long-term utility of the throughput test
feature. Maybe we'll soon see enough organic client use to cause proxies
to actually be used, but that's hard to predict. "Give me a fake client on
demand" could be a useful diagnostic feature to have. Conceivably we could
do the same thing probabilistically in the normal course of operation of
proxies: sometimes you get a real client, sometimes you get a "canary"
client whose only purpose is to allow the proxy to assess its own health.

1 2 >

1 - 100 of 120 matches

Mail list logo