From 7e3155dcd6874b1c8a415ce7659b8bf09b211581 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Fri, 9 Sep 2022 18:14:36 -0700 Subject: [PATCH] ForceTCP only for supported clients (#997) * ForceTCP only for supported clients Revert back to standard if forceRelay with TLS fails Don't force TLS unless it's configured * fix lint --- go.mod | 2 +- pkg/config/config.go | 12 ++++++++ pkg/rtc/clientinfo.go | 49 +++++++++++++++++++++++++++++- pkg/rtc/clientinfo_test.go | 59 +++++++++++++++++++++++++++++++++++++ pkg/rtc/participant.go | 2 ++ pkg/rtc/transportmanager.go | 46 +++++++++++++++++------------ pkg/service/roommanager.go | 1 + 7 files changed, 150 insertions(+), 21 deletions(-) create mode 100644 pkg/rtc/clientinfo_test.go diff --git a/go.mod b/go.mod index ad06f5d73..2e179f47f 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( github.com/urfave/negroni v1.0.0 go.uber.org/atomic v1.10.0 go.uber.org/zap v1.23.0 + golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 golang.org/x/sync v0.0.0-20220819030929-7fc1605a5dde google.golang.org/protobuf v1.28.1 gopkg.in/yaml.v3 v3.0.1 @@ -85,7 +86,6 @@ require ( github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect go.uber.org/multierr v1.6.0 // indirect golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d // indirect - golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect golang.org/x/net v0.0.0-20220728211354-c7608f3a8462 // indirect golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10 // indirect golang.org/x/text v0.3.7 // indirect diff --git a/pkg/config/config.go b/pkg/config/config.go index 0885fe411..6926bb00e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -324,6 +324,18 @@ func (conf *Config) UseSentinel() bool { return conf.Redis.SentinelAddresses != nil } +func (conf *Config) IsTURNSEnabled() bool { + if conf.TURN.Enabled && conf.TURN.TLSPort != 0 { + return true + } + for _, s := range conf.RTC.TURNServers { + if s.Protocol == "tls" { + return true + } + } + return false +} + func (conf *Config) updateFromCLI(c *cli.Context) error { if c.IsSet("dev") { conf.Development = c.Bool("dev") diff --git a/pkg/rtc/clientinfo.go b/pkg/rtc/clientinfo.go index ab8598063..35492927e 100644 --- a/pkg/rtc/clientinfo.go +++ b/pkg/rtc/clientinfo.go @@ -1,6 +1,11 @@ package rtc -import "github.com/livekit/protocol/livekit" +import ( + "strconv" + "strings" + + "github.com/livekit/protocol/livekit" +) type ClientInfo struct { *livekit.ClientInfo @@ -9,3 +14,45 @@ type ClientInfo struct { func (c ClientInfo) SupportsAudioRED() bool { return c.ClientInfo != nil && c.ClientInfo.Browser != "firefox" && c.ClientInfo.Browser != "safari" } + +// CompareVersion compares two semver versions +// returning 1 if current version is greater than version +// 0 if they are the same, and -1 if it's an earlier version +func (c ClientInfo) CompareVersion(version string) int { + if c.ClientInfo == nil { + return -1 + } + parts0 := strings.Split(c.ClientInfo.Version, ".") + parts1 := strings.Split(version, ".") + ints0 := make([]int, 3) + ints1 := make([]int, 3) + for i := 0; i < 3; i++ { + if len(parts0) > i { + ints0[i], _ = strconv.Atoi(parts0[i]) + } + if len(parts1) > i { + ints1[i], _ = strconv.Atoi(parts1[i]) + } + if ints0[i] > ints1[i] { + return 1 + } else if ints0[i] < ints1[i] { + return -1 + } + } + return 0 +} + +func (c ClientInfo) SupportsICETCP() bool { + if c.ClientInfo == nil { + return false + } + if c.ClientInfo.Sdk == livekit.ClientInfo_GO { + return false + } + if c.ClientInfo.Sdk == livekit.ClientInfo_SWIFT { + // ICE/TCP added in 1.0.5 + return c.CompareVersion("1.0.5") >= 0 + } + // most SDKs support ICE/TCP + return true +} diff --git a/pkg/rtc/clientinfo_test.go b/pkg/rtc/clientinfo_test.go new file mode 100644 index 000000000..a7d3ffec3 --- /dev/null +++ b/pkg/rtc/clientinfo_test.go @@ -0,0 +1,59 @@ +/* + * Copyright 2022 LiveKit, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package rtc + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/livekit/protocol/livekit" +) + +func TestClientInfo_CompareVersion(t *testing.T) { + c := ClientInfo{ + ClientInfo: &livekit.ClientInfo{ + Version: "1", + }, + } + require.Equal(t, 1, c.CompareVersion("0.1.0")) + require.Equal(t, 0, c.CompareVersion("1.0.0")) + require.Equal(t, -1, c.CompareVersion("1.0.5")) +} + +func TestClientInfo_SupportsICETCP(t *testing.T) { + t.Run("GO SDK cannot support TCP", func(t *testing.T) { + c := ClientInfo{ + ClientInfo: &livekit.ClientInfo{ + Sdk: livekit.ClientInfo_GO, + }, + } + require.False(t, c.SupportsICETCP()) + }) + + t.Run("Swift SDK cannot support TCP before 1.0.5", func(t *testing.T) { + c := ClientInfo{ + ClientInfo: &livekit.ClientInfo{ + Sdk: livekit.ClientInfo_SWIFT, + Version: "1.0.4", + }, + } + require.False(t, c.SupportsICETCP()) + c.Version = "1.0.5" + require.True(t, c.SupportsICETCP()) + }) +} diff --git a/pkg/rtc/participant.go b/pkg/rtc/participant.go index 5e67d5f1d..0c0e5f9e5 100644 --- a/pkg/rtc/participant.go +++ b/pkg/rtc/participant.go @@ -84,6 +84,7 @@ type ParticipantParams struct { Migration bool AdaptiveStream bool AllowTCPFallback bool + TURNSEnabled bool } type ParticipantImpl struct { @@ -1107,6 +1108,7 @@ func (p *ParticipantImpl) setupTransportManager() error { ClientInfo: p.params.ClientInfo, Migration: p.params.Migration, AllowTCPFallback: p.params.AllowTCPFallback, + TURNSEnabled: p.params.TURNSEnabled, Logger: p.params.Logger, }) if err != nil { diff --git a/pkg/rtc/transportmanager.go b/pkg/rtc/transportmanager.go index 7fe1fb08b..1e29ae6a1 100644 --- a/pkg/rtc/transportmanager.go +++ b/pkg/rtc/transportmanager.go @@ -32,6 +32,7 @@ type TransportManagerParams struct { ClientInfo ClientInfo Migration bool AllowTCPFallback bool + TURNSEnabled bool Logger logger.Logger } @@ -463,29 +464,36 @@ func (t *TransportManager) handleConnectionFailed(isShortLived bool) { iceConfig := t.iceConfig t.lock.RUnlock() - var nextConfig types.IceConfig - // irrespective of which one fails, force prefer candidate on both as the other one might - // fail at a different time and cause another disruption - switch iceConfig.PreferSub { - case types.PreferNone: - t.params.Logger.Infow("restricting transport to TCP on both peer connections") - nextConfig = types.IceConfig{ - PreferPub: types.PreferTcp, - PreferSub: types.PreferTcp, - } + var preferNext types.PreferCandidateType + if iceConfig.PreferSub == types.PreferNone && t.params.ClientInfo.SupportsICETCP() { + preferNext = types.PreferTcp + } else if iceConfig.PreferSub != types.PreferTls && t.params.TURNSEnabled { + preferNext = types.PreferTls + } else { + preferNext = types.PreferNone + } - case types.PreferTcp: - t.params.Logger.Infow("prefer transport to TLS on both peer connections") - nextConfig = types.IceConfig{ - PreferPub: types.PreferTls, - PreferSub: types.PreferTls, - } - - default: + if preferNext == iceConfig.PreferSub { return } - t.SetICEConfig(nextConfig) + switch preferNext { + case types.PreferTcp: + t.params.Logger.Infow("restricting transport to TCP on both peer connections") + + case types.PreferTls: + t.params.Logger.Infow("prefer transport to TLS on both peer connections") + + case types.PreferNone: + t.params.Logger.Infow("allowing all transports on both peer connections") + } + + // irrespective of which one fails, force prefer candidate on both as the other one might + // fail at a different time and cause another disruption + t.SetICEConfig(types.IceConfig{ + PreferPub: preferNext, + PreferSub: preferNext, + }) } func (t *TransportManager) SetMigrateInfo(previousOffer, previousAnswer *webrtc.SessionDescription, dataChannels []*livekit.DataChannelInfo) { diff --git a/pkg/service/roommanager.go b/pkg/service/roommanager.go index 4034fbd72..26c1a22e3 100644 --- a/pkg/service/roommanager.go +++ b/pkg/service/roommanager.go @@ -292,6 +292,7 @@ func (r *RoomManager) StartSession( Region: pi.Region, AdaptiveStream: pi.AdaptiveStream, AllowTCPFallback: allowFallback, + TURNSEnabled: r.config.IsTURNSEnabled(), }) if err != nil { return err