The legacy code does case matching on these exact strings so it's better
to ensure they're constant.

Move the logic for the legacy version into the http handlers and use a
shim when doing ipc.

Introduces an IPC struct and moves the logic out of the http handlers
and into methods on that.

So the assignment of proxies is based on the load. The number of clients
is ronded down to 8. Existing proxies that doesn't report the number
of clients will be distributed equaly to new proxies until they get 8
clients, that is okish as the existing proxies do have a maximum
capacity of 10.

Fixes #40048

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.

Refactor out a separate handleStream function and ensure that all
connections are closed and the references are out of scope.

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.

Using the boolean value was unnecessary since we already have a channel
we can check for closure.

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.

sbs authored 3 years ago and Cecylia Bocovich committed 3 years ago

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.

This fixes a stats collection bug where we were converting client
addresses between a string and net.Addr using the clientAddr function
multiple times, resulting in an empty string for all addresses.

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.

To be used by the debian package (#19409)

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.

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.

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:
!31 (comment 2733279)

David Fifield authored 3 years ago and meskio committed 3 years ago

If we don't stop them explicitly, the timers will not get garbage collected
until they timeout:
https://medium.com/@oboturov/golang-time-after-is-not-garbage-collected-4cbc94740082

Related to #40039

Follow up to 160ae2dd

Analysis by @dcf,

> I don't think the sync.Once around logMetrics is necessary anymore.
Its original purpose was to inhibit logging on later file handles of
metrics.log, if there were more than one opened. See 171c55a9 and #29734
(comment 2593039) "Making a singleton *Metrics variable causes problems
with how Convey does tests. It shouldn't be called more than once, but
for now I'm using sync.Once on the logging at least so it's explicit."
Commit ba4fe1a7 changed it so that metrics.log is opened in main, used
to create a *log.Logger, and that same instance of *log.Logger is passed
to both NewMetrics and NewBrokerContext. It's safe to share the same
*log.Logger across multiple BrokerContext.

Doesn't seem like it needs to exist outside of the metrics struct.

Also, the call to logMetrics is moved to the constructor.  A metrics
instance is only created when a BrokerContext is created, which only
happens at startup.  The sync of only doing that once is left for
documentation purposes, since it doesn't hurt, but also seems redundant.

Introduce a waitgroup and done channel to ensure that both the read and
write gorouting for turbotunnel connections terminate when the
connection is closed.

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

The default prometheus registry exports data that may be useful for
side-channel attacks. This removes all of the default metrics and makes
sure we are only reporting snowflake metrics from the broker.

This change adds a prometheus exporter for our existing snowflake broker
metrics. Current values for the metrics can be fetched by sending a GET
request to /prometheus.

This fixes a vulnerability in the library: CVE-2021-28681

Snowflake copies data between the OR connection and the KCP stream,
meaning that in most cases the copy loops will only terminate once the
OR connection times out. In this case the OR connection is already
closed and so calls to CloseRead and CloseWrite will generate errors.

These errors are triggered in three places when the OR connection times
out. They don't tell us anything useful and are filling up our logs.

This will increase transparency for people running standalone proxies
and help us debug any potential issues with proxies behaving unreliably.