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

See also:
tpo/anti-censorship/pluggable-transports/snowflake#40084

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

This is an amendment of !30

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.

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

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.

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

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.

The difficulty here is that the whole point of turbotunnel sessions is
that they are not necessarily tied to a single WebSocket connection, nor
even a single client IP address. We use a heuristic: whenever a
WebSocket connection starts that has a new ClientID, we store a mapping
from that ClientID to the IP address attached to the WebSocket
connection in a lookup table. Later, when enough packets have arrived to
establish a turbotunnel session, we recover the ClientID associated with
the session (which kcp-go has stored in the RemoteAddr field), and look
it up in the table to get an IP address. We introduce a new data type,
clientIDMap, to store the clientID-to-IP mapping during the short time
between when a WebSocket connection starts and handleSession receives a
fully fledged KCP session.

The client opts into turbotunnel mode by sending a magic token at the
beginning of each WebSocket connection (before sending even the
ClientID). The token is just a random byte string I generated. The
server peeks at the token and, if it matches, uses turbotunnel mode.
Otherwise, it unreads the token and continues in the old
one-session-per-WebSocket mode.

Some proxies currently send ?client_ip=0.0.0.0 because of an error in
how they attempt to grep the address from the client's SDP. That's
inflating our "%d/%d connections had client_ip" logs. Instead, treat
these cases as if the IP address were absent.
https://bugs.torproject.org/33157
https://bugs.torproject.org/33385

This makes the return type satisfy the io.ReadWriteCloser interface
directly.

Rename websocketconn.WebSocketConn to websocketconn.Conn, and
       websocketconn.NewWebSocketConn to websocketconn.New

Following the guidelines at
https://blog.golang.org/package-names#TOC_3%2e

Only once, not again on every call to initServer.

This was the only case out of the three not to show it.

These functions are called for their side effect of sending a PT error
message on stdout; they also return a representation of the error
message as an error object for the caller to use if it wishes. These
functions *always* return a non-nil error object; it is not something to
be logged, any more than the return value of errors.New is.

The mistaken logging was added in
https://bugs.torproject.org/31794
b26c7a7a
3ec9dd19
ed3d42e1

Now tests the proxy and initServer functionalities. The tests use the
same websocket library as the server and proxy-go implementations.

Trac: 32465

And simplify EOF check.

Trac: 31028

The requirement to do so is obsolete and has already been removed from
other pluggable transports.

https://bugs.torproject.org/32046

Shane Howearth authored 5 years ago and Cecylia Bocovich committed 5 years ago

- Error strings are no longer capitalized nor end with punctuation
- Alias import
- Remove extraneous initilisation code (No need to provide zero value
	for variables, because the compiler does that anyway)

The server log scrubbing code from ticket #21304 is now refactored into
a safelog package, along with the appropriate tests

Modified regular expressions to not scrub fingerprints, but catch all
instances of IPv4 and IPv6 addresses. Expanded test cases with those
suggested by dcf.

Log scrubber now scrubs addresses even in the case where they are split
across calls to the scrubber's output io.Writer.

Added test cases to test that the writer behaves correctly across split
lines.

It is now more readable, and the regexp's are only compiled once

IPv6 regexes didn't need to be that precise, added more tests for
edge-cases.