Commit graph

263 commits

Author SHA1 Message Date
Cecylia Bocovich
624750d5a8 Stop exporting code that should be internal 2021-10-04 10:17:37 -04:00
Cecylia Bocovich
8c6f0dbae7 Check error for calls to preparePeerConnection 2021-09-30 11:46:39 -04:00
Cecylia Bocovich
ace8df37ed Fix compile bug in client, caught by CI 2021-08-24 10:27:24 -04:00
Cecylia Bocovich
a39d6693e1 Call conn.Reject() if SOCKS arguments are invalid 2021-08-19 21:31:51 -04:00
Cecylia Bocovich
97175a91a5 Modify torrc example to pass client args in bridge line 2021-08-19 21:20:34 -04:00
Cecylia Bocovich
e762f58a31 Parse SOCKS arguments and prefer over command line options
Parsing the Snowflake client options from SOCKS allow us to specify
snowflake client settings in the bridge lines.
2021-08-19 21:20:34 -04:00
Cecylia Bocovich
4acc08cc60 Use a config struct for snowflake client options 2021-08-19 21:20:34 -04:00
Cecylia Bocovich
e6715cb4ee Increase smux and QueuePacketConn buffer sizes
This should increase the maximum amount of inflight data and hopefully
the performance of Snowflake, especially for clients geographically
distant from proxies and the server.
2021-08-10 15:38:11 -04:00
David Fifield
521eb4d4d6 Add info about rendezvous methods to client README. 2021-08-05 16:13:24 -06:00
David Fifield
5adb994028 Implement ampCacheRendezvous. 2021-08-05 16:13:24 -06:00
David Fifield
c13810192d Skeleton of ampCacheRendezvous.
Currently the same as httpRendezvous, but activated using the -ampcache
command-line option.
2021-08-05 16:13:24 -06:00
David Fifield
0f34a7778f Factor out httpRendezvous separate from BrokerChannel.
Makes BrokerChannel abstract over a rendezvousMethod. BrokerChannel
itself is responsible for keepLocalAddresses and the NAT type state, as
well as encoding and decoding client poll messages. rendezvousMethod is
only responsible for delivery of encoded messages.
2021-08-05 16:13:24 -06:00
David Fifield
55f4814dfb Change the representation of domain fronting in HTTP rendezvous.
Formerly, BrokerChannel represented the broker URL and possible domain
fronting as
	bc.url  *url.URL
        bc.Host string
That is, bc.url is the URL of the server which we contact directly, and
bc.Host is the Host header to use in the request. With no domain
fronting, bc.url points directly at the broker itself, and bc.Host is
blank. With domain fronting, we do the following reshuffling:
	if front != "" {
		bc.Host = bc.url.Host
		bc.url.Host = front
	}
That is, we alter bc.url to reflect that the server to which we send
requests directly is the CDN, not the broker, and store the broker's own
URL in the HTTP Host header.

The above representation was always confusing to me, because in my
mental model, we are always conceptually communicating with the broker;
but we may optionally be using a CDN proxy in the middle. The new
representation is
	bc.url   *url.URL
        bc.front string
bc.url is the URL of the broker itself, and never changes. bc.front is
the optional CDN front domain, and likewise never changes after
initialization. When domain fronting is in use, we do the swap in the
http.Request struct, not in BrokerChannel itself:
	if bc.front != "" {
		request.Host = request.URL.Host
		request.URL.Host = bc.front
	}

Compare to the representation in meek-client:

https://gitweb.torproject.org/pluggable-transports/meek.git/tree/meek-client/meek-client.go?h=v0.35.0#n94
	var options struct {
		URL       string
		Front     string
	}
https://gitweb.torproject.org/pluggable-transports/meek.git/tree/meek-client/meek-client.go?h=v0.35.0#n308
	if ok { // if front is set
		info.Host = info.URL.Host
		info.URL.Host = front
	}
2021-08-05 16:13:24 -06:00
David Fifield
191510c416 Use a URL with a Host component in BrokerChannel tests.
The tests were using a broker URL of "test.broker" (i.e., a schema-less,
host-less, relative path), and running assertions on the value of
b.url.Path. This is strange, especially in tests regarding domain
fronting, where we care about b.url.Host, not b.url.Path. This commit
changes the broker URL to "http://test.broker" and changes tests to
check b.url.Host. I also added an additional assertion for an empty
b.Host in the non-domain-fronted case.
2021-08-05 16:13:24 -06:00
Cecylia Bocovich
c1b0fdd8cf Cleaned up and reorganized READMEs 2021-07-19 10:16:26 -04:00
David Fifield
2d7cd3f2b7 Use the readLimit constant in a test.
Instead of copying the value.
2021-07-18 16:25:09 -06:00
David Fifield
d9a83e26b5 Remove unused FakePeers.
Unused since 1364d7d45b.
2021-07-18 13:11:29 -06:00
Cecylia Bocovich
74bdb85b30 Update example torrc file for client
Remove the -max 3 option because we only use one snowflake. Add
SocksPort auto because many testers have a tor process already bound to
port 9050.
2021-06-24 13:46:11 -04:00
Cecylia Bocovich
10b6075eaa Refactor checkForStaleness to take time.Duration 2021-06-24 11:20:44 -04:00
Cecylia Bocovich
e3351cb08a Fix data race for Peers.collection
We used a WaitGroup to prevent a call to Peers.End from melting
snowflakes while a new one is being collected. However, calls to
WaitGroup.Add are in a race with WaitGroup.Wait. To fix this, we use a
Mutex instead.
2021-06-24 11:16:24 -04:00
Cecylia Bocovich
95cbe36565 Add unit tests to check for webrtc peer data races 2021-06-24 11:16:24 -04:00
Cecylia Bocovich
bb7ff6180b Fix datarace for Peers.melted
Using the boolean value was unnecessary since we already have a channel
we can check for closure.
2021-06-24 11:16:24 -04:00
Cecylia Bocovich
ddcdfc4f09 Fix datarace for WebRTCPeer.closed
The race condition occurs because concurrent goroutines are intermixing
reads and writes of `WebRTCPeer.closed`.

Spotted when integrating Snowflake inside OONI in
https://github.com/ooni/probe-cli/pull/373.
2021-06-24 11:16:24 -04:00
Simone Basso
ed2d5df87d Fix datarace for WebRTCPeer.lastReceive
The race condition occurs because concurrent goroutines are
intermixing reads and writes of `WebRTCPeer.lastReceive`.

Spotted when integrating Snowflake inside OONI in
https://github.com/ooni/probe-cli/pull/373.
2021-06-24 11:16:24 -04:00
Simone Basso
aefabe683f fix(client/snowflake.go): prevent wg.Add race condition
In VSCode, the staticcheck tool emits this warning:

> should call wg.Add(1) before starting the goroutine to
> avoid a race (SA2000)go-staticcheck

To avoid this warning, just move wg.Add outside.
2021-06-14 10:10:02 +02:00
Cecylia Bocovich
270eb21803 Encode client-broker messages as json in HTTP body
Send the client poll request and response in a json-encoded format in
the HTTP request body rather than sending the data in HTTP headers. This
will pave the way for using domain-fronting alternatives for the
Snowflake rendezvous.
2021-06-02 09:52:42 -04:00
David Fifield
ae7cc478fd Release resources in client Transport.Dial on error.
Make a stack of cleanup functions to run (as with defer), but clear the
stack before returning if no error occurs.

Uselessly pushing the stream.Close() cleanup just before clearing the
stack is an intentional safeguard, for in case additional operations are
added before the return in the future.

Fixes #40042.
2021-05-24 15:28:13 -06:00
David Fifield
01a96c7d95 Fix error handling around transport.Dial.
The code checked for and displayed an error, but would then go on to
call copyLoop on the nil Conn returned from transport.Dial. Add a return
in that case, and put the cleanup operations in defer. Also remove an
obsolete comment about an empty address. Obsolete because:
https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/merge_requests/31#note_2733279
2021-05-24 14:40:50 -06:00
Cecylia Bocovich
e87b9175dd Implement snowflake client lib as PTv2.1 Go API
This implements a pluggable transports v2.1 compatible Go API in the
Snowflake client library, and refactors how the main Snowflake program
calls it. The Go API implements the two required client side functions:
a constructor that returns a Transport, and a Dial function for the
Transport that returns a net.Conn. See the PT specification for more
information:
https://github.com/Pluggable-Transports/Pluggable-Transports-spec/blob/master/releases/PTSpecV2.1/Pluggable%20Transport%20Specification%20v2.1%20-%20Go%20Transport%20API.pdf
2021-05-12 09:08:41 -04:00
Cecylia Bocovich
eff73c3016 Switch front domain and host to fastly 2021-04-01 11:56:52 -04:00
Cecylia Bocovich
83c01565ef Update webrtc library to v3.0.0
This update required two main changes to how we use the library. First,
we had to make sure we created the datachannel on the offering peer side
before creating the offer. Second, we had to make sure we wait for the
gathering of all candidates to complete since trickle-ice is enabled by
default. See the release notes for more details:
https://github.com/pion/webrtc/wiki/Release-WebRTC@v3.0.0.
2021-01-12 10:37:26 -05:00
Cecylia Bocovich
f908576c60 Increase the KCP maximum window size 2020-12-17 09:54:18 -05:00
Cecylia Bocovich
8ec8a7cb63 Pass lock to socksAcceptLoop by reference
This fixes a bug where we were passing the lock by value to
socksAcceptLoop.
2020-12-16 10:52:19 -05:00
Cecylia Bocovich
3e8947bfc9 Avoid double delay in client from ReconnectTimeout
Run the snowflake collection ReconnectTimeout timer in parallel to the
negotiation with the broker. This way, if the broker takes a long time
to respond the client doesn't have to wait the full timeout to respond.
2020-12-05 15:51:42 -05:00
Cecylia Bocovich
effc667544 Wait until all goroutines finish before shutdown 2020-12-05 15:50:16 -05:00
Cecylia Bocovich
b9cc54b3b7 Send shutdown signal to shutdown open connections
Normally all dangling goroutines are terminated when the main function
exits. However, for projects that use a patched version of snowflake as
a library, these goroutines continued running as long as the main function
had not yet terminated. This commit has all open SOCKS connections close
after receiving a shutdown signal.
2020-12-05 15:50:16 -05:00
Cecylia Bocovich
114df695ce Create new smux session for each SOCKS connection
Each SOCKS connection has its own set of snowflakes and broker poll
loop. Since the session manager was tied to a single set of snowflakes,
this resulted in a bug where RedialPacketConn would sometimes try to
pull snowflakes from a previously melted pool. The fix is to maintain
separate smux sessions for each SOCKS connection, tied to its own
snowflake pool.
2020-12-04 11:17:13 -05:00
Cecylia Bocovich
665d76c5b0 Remove for loop around broker.Negotiate
Instead of continuously polling the broker until the client receives a
snowflake, fail back to the Connect() loop and try again to collect more
peers after ReconnectTimeout.
2020-11-23 12:10:59 -05:00
Cecylia Bocovich
6baa3c4d5f Add synchronization to prevent post-melt collects
This fixes a race condition in which snowflakes.End() is called while
snowflakes.Collect() is in progress resulting in a write to a closed
channel. We now wait for all in-progress collections to finish and add
an extra check before proceeding with a collection.
2020-10-15 14:47:51 -04:00
Cecylia Bocovich
cc55481faf Set max number of snowflakes in the Tongue 2020-08-27 16:44:07 -04:00
Cecylia Bocovich
1364d7d45b Move snowflake ConnectLoop inside SOCKS Handler
Bug #21314: maintains a separate snowflake connect loop per SOCKS
connection. This way, if Tor decides to stop using Snowflake, Snowflake
will stop using the client's network.
2020-08-27 16:43:55 -04:00
Cecylia Bocovich
d5ae7562ac Add response header timeouts to broker transports
The client and proxy use the net/http default transport to make round
trip connecitons to the broker. These by default don't time out and can
wait indefinitely for the broker to respond if the broker hangs and
doesn't terminate the connection.
2020-07-30 17:54:28 -04:00
Cecylia Bocovich
82031289a3 Refactor subsetting of ice servers into main
This moves the subsetting of ice servers out of the parseIceServers
function and into main.
2020-07-24 14:08:09 -04:00
Cecylia Bocovich
92520f681d Choose a random subset from given STUN servers
Only chooses a subset as long as we have over 2 STUN servers to choose
from.
2020-07-23 11:30:36 -04:00
Cecylia Bocovich
bf924445e3 Implement NAT discovery (RFC 5780) at the client
Snowflake clients will now attempt NAT discovery using the provided STUN
servers and report their NAT type to the Snowflake broker for matching.
The three possibilities for NAT types are:
- unknown (the client was unable to determine their NAT type),
- restricted (the client has a restrictive NAT and can only be paired
with unrestricted NATs)
- unrestricted (the client can be paired with any other NAT).
2020-07-06 13:16:03 -04:00
Cecylia Bocovich
bbf11a97e4 Reduce SnowflakeTimeout to 20 seconds
The underlying smux layer sends a keep-alive ping every 10 seconds. This
modification will allow for one dropped/delayed ping before discarding
the snowflake
2020-05-07 09:42:09 -04:00
David Fifield
7043a055f9 Reduce DataChannelTimeout from 30s to 10s.
https://bugs.torproject.org/34042
2020-05-04 19:43:48 -06:00
David Fifield
c8293a5de3 Format the establishDataChannel error log message like other log messages.
It was sticking out in the context of other log messages.

2020/04/30 22:39:10 WebRTC: DataChannel created.
2020/04/30 22:39:20 establishDataChannel: timeout waiting for DataChannel.OnOpen
2020/04/30 22:39:20 WebRTC: closing PeerConnection
2020/04/30 22:39:20 WebRTC: Closing
2020/04/30 22:39:20 WebRTC: WebRTC: Could not establish DataChannel  Retrying in 10s...
2020-05-01 10:30:04 -06:00
David Fifield
72cfb96ede Restore check for nil writePipe in WebRTCPeer.Close.
I removed this check in 047d3214bf because
NewWebRTCPeer always initializes writePipe, and it is never reset to
nil. However tests used &WebRTCPeer{} which bypasses NewWebRTCPeer and
leaves writePipe set to nil.

https://bugs.torproject.org/34049#comment:3
https://bugs.torproject.org/34050
2020-04-28 11:47:34 -06:00
David Fifield
047d3214bf Wait for data channel OnOpen before returning from NewWebRTCPeer.
Now callers cannot call Write without there being a DataChannel to write
to. This lets us remove the internal buffer and checks for transport ==
nil.

Don't set internal fields like writePipe, transport, and pc to nil when
closing; just close them and let them return errors if further calls are
made on them.

There's now a constant DataChannelTimeout that's separate from
SnowflakeTimeout (the latter is what checkForStaleness uses). Now we can
set DataChannel timeout to a lower value, to quickly dispose of
unconnectable proxies, while still keeping the threshold for detecting
the failure of a once-working proxy at 30 seconds.

https://bugs.torproject.org/33897
2020-04-27 18:48:00 -06:00