Commit graph

83 commits

Author SHA1 Message Date
Neel Chauhan
e22f397d84 Remove false positives 2024-10-17 19:49:55 -04:00
Neel Chauhan
f1e9f58b47 Move IP check 2024-10-17 23:25:03 +00:00
Neel Chauhan
990d165937 move IsHostnameLocal to common/util 2024-10-17 23:25:03 +00:00
Neel Chauhan
6fef2caaa8 Check error 2024-10-17 23:25:03 +00:00
Neel Chauhan
aaa59df308 Reduce network load 2024-10-17 23:25:03 +00:00
Neel Chauhan
5e4619bae1 Apply 1 suggestion(s) to 1 file(s)
Co-authored-by: Cecylia Bocovich <cohosh@torproject.org>
2024-10-17 23:25:03 +00:00
Neel Chauhan
7974463653 Revert "Add delay"
This reverts commit 97c73bcab662de0033bbaeadb76dea0a3ca274da.
2024-10-17 23:25:03 +00:00
Neel Chauhan
e983f7425c Add delay 2024-10-17 23:25:03 +00:00
Neel Chauhan
37a2570643 Block remote IPs and not just hostnames 2024-10-17 23:25:03 +00:00
Neel Chauhan
c18a1b7e69 resolve host to IP to check if it's local before connecting 2024-10-17 23:25:03 +00:00
Neel Chauhan
8792771cdc
broker and proxy must not reject client offers with no ICE candidates
Fixes #40371. Partially reverts !141.
2024-10-17 15:46:02 -04:00
Neel Chauhan
9ff205dd7f
Probetest/proxy: Set multiple comma-separated default STUN URLs
This adds the BlackBerry STUN server alongside Google's. Closes #40392.
2024-10-17 15:15:02 -04:00
WofWca
d346639eda
improvement(proxy): improve NAT check logging 2024-09-26 18:15:04 +01:00
WofWca
9b04728809
docs: improve proxy CLI param descriptions
Since the proxy component is the most dedicated for public use,
more comprehensive docs are good.
2024-09-25 16:50:18 +01:00
David Fifield
de61d7bb8d Document relayURL return in SignalingServer.pollOffer.
The second return value was added in
863a8296e8.
2024-09-21 18:28:17 +00:00
WofWca
0f0f118827 improvement(proxy): don't panic on invalid relayURL
Though prior to this change the panic could only happen
if the default relayURL set by the proxy is invalid,
since `datachannelHandler` is only called after a succesful
`checkIsRelayURLAcceptable()`, which ensures that it _is_ valid.
But in the case of invalid default relay URL, a warning is printed
already.
2024-09-21 18:20:31 +00:00
David Fifield
f752d2ab0c Spell out EphemeralMinPort and EphemeralMaxPort in comment.
For searching purposes.
2024-09-21 14:30:59 +00:00
WofWca
daff4d8913 refactor(proxy): add comment about packet size 2024-09-19 19:14:04 +00:00
WofWca
51edbbfd26
fix(proxy): maybe memory leak on failed NAT check
Maybe related: https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40243
2024-09-09 15:26:58 +01:00
WofWca
f44aa279fe
refactor(proxy): improve NAT check logging
4ed5da7f2f introduced `OnError` but it did not print
failed periodic NAT type check errors - the error was simply
ignored.
2024-09-09 15:26:55 +01:00
WofWca
7f9fea5797
fix(proxy): send answer even if ICE gathering is not complete
Otherwise the connection is guaranteed to fail, even though
we actually might have gathered enough to make a successful
connection.

Closes https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40230

This is the standalone proxy counterpart of https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake-webext/-/merge_requests/55
2024-09-09 11:58:28 +01:00
WofWca
2bbd4d0643
refactor(proxy): better RelayURL description
It's the case that it's simply "default" after
https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/merge_requests/87
Now the broker specifies the relay URL (see `ProxyPollResponse`).
2024-09-05 13:04:42 +01:00
WofWca
94c6089cdd
hardening(proxy): don't proxy private IP addresses
...by default.

This is useful when `RelayDomainNamePattern` is lax (e.g. just "$")
(which is not the case by default, so this is simply
a hardening measure).
2024-09-02 14:59:26 +01:00
WofWca
399bda5257
refactor(proxy): tidy up isRelayURLAcceptable
Add clearer error messages
2024-09-02 14:59:26 +01:00
WofWca
0f2bdffba0
hardening(proxy): only accept ws & wss relays 2024-09-02 14:59:26 +01:00
WofWca
14f4c82ff7
test(proxy): add tests for relayURL check 2024-09-02 14:59:23 +01:00
WofWca
f4db64612c
feat: expose pollInterval in CLI
Closes https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40373
2024-08-22 09:31:37 -04:00
obble
a6d4570c23
Fix log message in CopyLoop 2024-08-21 16:06:41 +01:00
obble
1d6a2580c6 Improving Snowflake Proxy Performance by Adjusting Copy Buffer Size
TL;DR: The current implementation uses a 32K buffer size for a total of 64K of
buffers/connection, but each read/write is less than 2K according to my measurements.

# Background

The Snwoflake proxy uses as particularly hot function `copyLoop`
(proxy/lib/snowflake.go) to proxy data from a Tor relay to a connected client.
This is currently done using the `io.Copy` function to write all incoming data
both ways.

Looking at the `io.Copy` implementation, it internally uses `io.CopyBuffer`,
which in turn defaults to a buffer of size 32K for copying data (I checked and
the current implementation uses 32K every time).

Since `snowflake-proxy` is intended to be run in a very distributed manner, on
as many machines as possible, minimizing the CPU and memory footprint of each
proxied connection would be ideal, as well as maximising throughput for
clients.

# Hypothesis

There might exist a buffer size `X` that is more suitable for usage in `copyLoop` than 32K.

# Testing

## Using tcpdump

Assuming you use `-ephemeral-ports-range 50000:51000` for `snowflake-proxy`,
you can capture the UDP packets being proxied using

```sh
sudo tcpdump  -i <interface> udp portrange 50000-51000
```

which will provide a `length` value for each packet captured. One good start
value for `X` could then be slighly larger than the largest captured packet,
assuming one packet is copied at a time.

Experimentally I found this value to be 1265 bytes, which would make `X = 2K` a
possible starting point.

## Printing actual read

The following snippe was added in `proxy/lib/snowflake.go`:

```go
// Taken straight from standardlib io.copyBuffer
func copyBuffer(dst io.Writer, src io.Reader, buf []byte) (written int64, err error) {
	// If the reader has a WriteTo method, use it to do the copy.
	// Avoids an allocation and a copy.
	if wt, ok := src.(io.WriterTo); ok {
		return wt.WriteTo(dst)
	}
	// Similarly, if the writer has a ReadFrom method, use it to do the copy.
	if rt, ok := dst.(io.ReaderFrom); ok {
		return rt.ReadFrom(src)
	}
	if buf == nil {
		size := 32 * 1024
		if l, ok := src.(*io.LimitedReader); ok && int64(size) > l.N {
			if l.N < 1 {
				size = 1
			} else {
				size = int(l.N)
			}
		}
		buf = make([]byte, size)
	}
	for {
		nr, er := src.Read(buf)
		if nr > 0 {
			log.Printf("Read %d", nr) // THIS IS THE ONLY DIFFERENCE FROM io.CopyBuffer
			nw, ew := dst.Write(buf[0:nr])
			if nw < 0 || nr < nw {
				nw = 0
				if ew == nil {
					ew = errors.New("invalid write result")
				}
			}
			written += int64(nw)
			if ew != nil {
				err = ew
				break
			}
			if nr != nw {
				err = io.ErrShortWrite
				break
			}
		}
		if er != nil {
			if er != io.EOF {
				err = er
			}
			break
		}
	}
	return written, err
}
```

and `copyLoop` was amended to use this instead of `io.Copy`.

The `Read: BYTES` was saved to a file using this command

```sh
./proxy -verbose -ephemeral-ports-range 50000:50010 2>&1 >/dev/null  | awk '/Read: / { print $4 }' | tee read_sizes.txt
```

I got the result:

min: 8
max: 1402
median: 1402
average: 910.305

Suggested buffer size: 2K
Current buffer size: 32768 (32K, experimentally verified)

## Using a Snowflake Proxy in Tor browser and use Wireshark

I also used Wireshark, and concluded that all packets sent was < 2K.

# Conclusion

As per the commit I suggest changing the buffer size to 2K. Some things I have not been able to answer:

1. Does this make a big impact on performance?
1. Are there any unforseen consequences? What happens if a packet is > 2K (I
	 think the Go standard libary just splits the packet, but someone please confirm).
2024-08-21 15:02:15 +00:00
David Fifield
ee5f815f60 Cosmetic changes from dev-snowflake-udp-rebase-extradata.
https://gitlab.torproject.org/shelikhoo/snowflake/-/tree/dev-snowflake-udp-rebase-extradata
commit 59b76dc68d2ee0383c2acd91cb0f44edc46af939
2024-08-01 22:12:56 +00:00
itchyonion
4ed5da7f2f
Simplify proxy NAT checking logic 2024-05-28 12:30:44 -07:00
am3o
acce1f1fd9
refactor: change deprecated "io/ioutil" package to recommended "io" package 2024-02-17 12:47:22 +01:00
David Fifield
d0529141ac Cosmetic fixes taken from !219.
shelikhoo/dev-udp-performance-rebased branch
https://gitlab.torproject.org/shelikhoo/snowflake/-/commits/9dce28cfc2093490473432ffecd9abaab7ebdbdb
2024-01-16 18:43:58 +00:00
n8fr8
36a8eb487f
Add Ignore Android Restriction Workaround for Proxy 2023-12-18 12:58:48 +00:00
David Fifield
234d9cb11c Link a section in the pion/webrtc@3.0.0 release notes. 2023-11-21 01:27:09 +00:00
Cecylia Bocovich
648609dbea
Refactor disabling the stats logger
Have Snowflake proxy periodically collect throughput stats even if the
stats logger is disabled so that it can be handled by the prometheus
metrics.
2023-10-31 13:15:52 -04:00
Cecylia Bocovich
caa2b36463
Process and properly log connection closure stats 2023-10-31 10:02:31 -04:00
Cecylia Bocovich
018bbd6d65
Proxy stats log only what occurred that time interval
Modify the periodic stats output by standalone snowflake proxies to only
include the data transferred during the time interval being logged. This
is an improvement of previous behaviour that logged the total data
transferred by all proxy connections that were closed within the time
interval being logged..

Closes #40302:
https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40302
2023-10-30 12:42:45 -04:00
Cecylia Bocovich
354cb65432
Move creation of periodic stats task inside proxy library
This adds a new type of SnowflakeEvent. EventOnProxyStats is triggered
by the periodic task run at SummaryInterval and produces an event with a
proxy stats output string.
2023-10-30 12:42:45 -04:00
Cecylia Bocovich
939062c7dd
Remove ThroughputSummary from bytesLogger
This was leftover from when we used to log the total throughput of
connections when they close. It should be removed for privacy reasons as
mentioned in
https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40079
2023-10-30 12:42:45 -04:00
KokaKiwi
7142fa3ddb
fix(proxy): Correctly close connection pipe when dealing with error 2023-10-12 15:52:43 +01:00
David Fifield
8104732114 Change DefaultRelayURL back to wss://snowflake.torproject.net/.
Fixes #40283. Compare to #31522.
2023-07-29 22:33:26 +00:00
Vort
ea01c92cf1
Implement DataChannel flow control 2023-06-19 17:44:45 +01:00
meskio
82cc0f38f7
Move the development to gitlab
Related: tpo/anti-censorship/team#86
2023-05-31 10:01:47 +02:00
itchyonion
07b5f07452
Validate SDP offers and answers 2023-05-29 10:12:48 -07:00
KokaKiwi
1ef43a0dde
Use latest Pion WebRTC libs version
- webrtc and dtls libs got the "Skip Hello Verify" patches applied

Link: https://github.com/pion/dtls/pull/513
Link: https://github.com/pion/webrtc/pull/2433
2023-03-22 12:19:03 +00:00
itchyonion
5dd0a31d95
Add comments and improve logging 2023-03-14 12:43:00 -07:00
itchyonion
fb35e80b0a
Proxy: add outbound-address config 2023-03-14 12:42:59 -07:00
Cecylia Bocovich
f6fa51d749
Switch default proxy STUN server to stun.l.google.com
This is the same default that the web-based proxies use. Proxies do not
need RFC 5780 compatible STUN servers.
2022-12-31 12:23:27 -05:00
Cecylia Bocovich
6007d5e08e
Refactor creation of webRTCConn in proxy 2022-11-28 17:10:49 -05:00