From cb0fb02cd5c95ac1e8087e3d8fb2925f8e6fd17c Mon Sep 17 00:00:00 2001 From: WofWca Date: Tue, 11 Feb 2025 19:14:33 +0400 Subject: [PATCH] fix(proxy): not answering before client timeout This is related to https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40230. The initial MR that closed that issue, https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/merge_requests/391, was not semantically correct, because `DataChannelTimeout` starts after the client has already received the answer. After https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/merge_requests/498#note_3156256 the code became not only semantically incorrect, but also functionally incorrect because now if this timeout is hit by the proxy, the client is guaranteed to be gone already. This commit fixes it, by lowering the timeout. This addresses a suggestion in https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40447. This also closes https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40381 and supersedes https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/merge_requests/415. --- broker/ipc.go | 3 ++- common/constants/constants.go | 10 ++++++++++ proxy/lib/snowflake.go | 8 ++++---- 3 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 common/constants/constants.go diff --git a/broker/ipc.go b/broker/ipc.go index 709974b..d0b4d47 100644 --- a/broker/ipc.go +++ b/broker/ipc.go @@ -8,13 +8,14 @@ import ( "time" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/bridgefingerprint" + "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/constants" "github.com/prometheus/client_golang/prometheus" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/messages" ) const ( - ClientTimeout = 5 // this is calibrated to match the timeout of the CDNs we use for rendezvous + ClientTimeout = constants.BrokerClientTimeout ProxyTimeout = 10 NATUnknown = "unknown" diff --git a/common/constants/constants.go b/common/constants/constants.go new file mode 100644 index 0000000..ded1659 --- /dev/null +++ b/common/constants/constants.go @@ -0,0 +1,10 @@ +package constants + +const ( + // If the broker does not receive the proxy answer within this many seconds + // after the broker received the client offer, + // the broker will respond with an error to the client. + // + // this is calibrated to match the timeout of the CDNs we use for rendezvous + BrokerClientTimeout = 5 +) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 7bd1aaf..7e761af 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -45,14 +45,13 @@ import ( "github.com/pion/transport/v3/stdnet" "github.com/pion/webrtc/v4" + "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/constants" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/event" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/messages" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/namematcher" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/task" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/util" "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/common/websocketconn" - - snowflakeClient "gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2/client/lib" ) const ( @@ -540,11 +539,12 @@ func (sf *SnowflakeProxy) makePeerConnectionFromOffer( } // Wait for ICE candidate gathering to complete, - // or for whatever we managed to gather before the client times out. + // or for whatever we managed to gather before the broker + // responds with an error to the client offer. // See https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40230 select { case <-done: - case <-time.After(snowflakeClient.DataChannelTimeout / 2): + case <-time.After(constants.BrokerClientTimeout * time.Second * 3 / 4): log.Print("ICE gathering is not yet complete, but let's send the answer" + " before the client times out") }