diff --git a/Makefile b/Makefile index e158103..cca973c 100644 --- a/Makefile +++ b/Makefile @@ -329,7 +329,7 @@ endif start-mm: ## start MM server ifeq (${CI}, true) @$(INFO) starting up MM server... - docker-compose -p mmserver -f ./build/test/docker-compose.yaml up -d + docker compose -p mmserver -f ./build/test/docker-compose.yaml up -d else @$(INFO) skipping start-mm target, not on CI endif @@ -338,7 +338,7 @@ endif stop-mm: ## stop MM server ifeq (${CI}, true) @$(INFO) stopping up MM server... - docker-compose -p mmserver -f ./build/test/docker-compose.yaml down + docker compose -p mmserver -f ./build/test/docker-compose.yaml down else @$(INFO) skipping stop-mm target, not on CI endif diff --git a/build/test/docker-compose.yaml b/build/test/docker-compose.yaml index 1d4447f..ba444d2 100644 --- a/build/test/docker-compose.yaml +++ b/build/test/docker-compose.yaml @@ -39,6 +39,7 @@ services: MM_SERVICESETTINGS_ENABLEONBOARDINGFLOW: "false" MM_FEATUREFLAGS_ONBOARDINGTOURTIPS: "false" MM_SERVICEENVIRONMENT: "test" + MM_CALLS_GROUP_CALLS_ALLOWED: "true" volumes: - "server-config:/mattermost/config" depends_on: diff --git a/build/test/prepare-plugin.sh b/build/test/prepare-plugin.sh index 7b1065c..a014e0e 100755 --- a/build/test/prepare-plugin.sh +++ b/build/test/prepare-plugin.sh @@ -1,5 +1,5 @@ #/bin/bash -set -x +set -xe GIT_DEFAULT_BRANCH="main" GIT_REPO="https://github.com/mattermost/mattermost-plugin-calls" @@ -14,11 +14,12 @@ else fi # Build -cd .. && git clone -b ${GIT_BRANCH} https://github.com/mattermost/mattermost-plugin-calls --depth 1 && \ -cd mattermost-plugin-calls && +cd .. && git clone -b ${GIT_BRANCH} https://github.com/mattermost/mattermost-plugin-calls && \ +cd mattermost-plugin-calls && \ +git fetch --tags && \ cd standalone && npm ci && cd .. && \ cd webapp && npm ci && cd .. && \ -echo 'replace github.com/mattermost/rtcd => ../rtcd' >> go.mod && \ # We need to make sure we compile the plugin with the rtcd changes. +echo "replace github.com/mattermost/rtcd => ../rtcd" >> go.mod && \ go mod tidy && \ make dist MM_SERVICESETTINGS_ENABLEDEVELOPER=true @@ -26,15 +27,16 @@ make dist MM_SERVICESETTINGS_ENABLEDEVELOPER=true PLUGIN_BUILD_PATH=$(realpath dist/*.tar.gz) PLUGIN_FILE_NAME=$(basename ${PLUGIN_BUILD_PATH}) -docker cp ../rtcd/build/test/config_patch.json mmserver_server_1:/mattermost && \ -docker exec mmserver_server_1 bin/mmctl --local config patch config_patch.json && \ -docker cp ${PLUGIN_BUILD_PATH} mmserver_server_1:/mattermost && \ -docker exec mmserver_server_1 bin/mmctl --local plugin delete ${PLUGIN_ID} && \ -docker exec mmserver_server_1 bin/mmctl --local plugin add ${PLUGIN_FILE_NAME} && \ -docker exec mmserver_server_1 bin/mmctl --local plugin enable ${PLUGIN_ID} && \ +docker ps -a && \ +docker cp ../rtcd/build/test/config_patch.json mmserver-server-1:/mattermost && \ +docker exec mmserver-server-1 bin/mmctl --local config patch config_patch.json && \ +docker cp ${PLUGIN_BUILD_PATH} mmserver-server-1:/mattermost && \ +docker exec mmserver-server-1 bin/mmctl --local plugin delete ${PLUGIN_ID} && \ +docker exec mmserver-server-1 bin/mmctl --local plugin add ${PLUGIN_FILE_NAME} && \ +docker exec mmserver-server-1 bin/mmctl --local plugin enable ${PLUGIN_ID} && \ sleep 5s STATUS_CODE=$(curl --write-out '%{http_code}' --silent --output /dev/null http://localhost:8065/plugins/com.mattermost.calls/version) if [ "$STATUS_CODE" != "200" ]; then - echo "Status code check for plugin failed" && docker logs mmserver_server_1 && exit 1 + echo "Status code check for plugin failed" && docker logs mmserver-server-1 && exit 1 fi diff --git a/config/config.sample.toml b/config/config.sample.toml index 7ec1e7e..7777260 100644 --- a/config/config.sample.toml +++ b/config/config.sample.toml @@ -77,6 +77,11 @@ ice_host_override = "" # config to multiple pods in Kubernetes deployments. In that case, each pod should match against one # local (node) IP and greatly simplify load balancing across multiple nodes. +# Whether or not to perform DNS resolution of the ice_host_override. +# When set to false (Experimental), the override is forwarded to the client unchanged. +# Note: This setting only takes effect if the ice_host_override is a FQDN (i.e. not an IP address). +ice_host_override_resolution = true + # A list of ICE servers (STUN/TURN) to be used by the service. It supports # advanced configurations. # Example diff --git a/service/config.go b/service/config.go index f11c353..c61d98a 100644 --- a/service/config.go +++ b/service/config.go @@ -83,6 +83,7 @@ func (c *Config) SetDefaults() { c.RTC.ICEPortUDP = 8443 c.RTC.ICEPortTCP = 8443 c.RTC.TURNConfig.CredentialsExpirationMinutes = 1440 + c.RTC.ICEHostOverrideResolution = true c.Store.DataSource = "/tmp/rtcd_db" c.Logger.EnableConsole = true c.Logger.ConsoleJSON = false diff --git a/service/rtc/config.go b/service/rtc/config.go index fbd2282..9adfd03 100644 --- a/service/rtc/config.go +++ b/service/rtc/config.go @@ -31,6 +31,9 @@ type ServerConfig struct { TURNConfig TURNConfig `toml:"turn"` // EnableIPv6 specifies whether or not IPv6 should be used. EnableIPv6 bool `toml:"enable_ipv6"` + // ICEHostOverrideResolution controls whether or not the ICEHostOverride should + // be resolved by the server before forwarding it to the client. + ICEHostOverrideResolution bool `toml:"ice_host_override_resolution"` } func (c ServerConfig) IsValid() error { diff --git a/service/rtc/msg.go b/service/rtc/msg.go index a69a7a6..c25fc76 100644 --- a/service/rtc/msg.go +++ b/service/rtc/msg.go @@ -54,10 +54,51 @@ func newMessage(s *session, msgType MessageType, data []byte) Message { } } +func marshalHostCandidate(c *webrtc.ICECandidate) webrtc.ICECandidateInit { + val := c.Foundation + if val == " " { + val = "" + } + + val = fmt.Sprintf("%s %d %s %d %s %d typ %s", + val, + c.Component, + c.Protocol, + c.Priority, + c.Address, + c.Port, + c.Typ) + + if c.TCPType != "" { + val += fmt.Sprintf(" tcptype %s", c.TCPType) + } + + if c.RelatedAddress != "" && c.RelatedPort != 0 { + val = fmt.Sprintf("%s raddr %s rport %d", + val, + c.RelatedAddress, + c.RelatedPort) + } + + return webrtc.ICECandidateInit{ + Candidate: fmt.Sprintf("candidate:%s", val), + SDPMid: new(string), + SDPMLineIndex: new(uint16), + } +} + func newICEMessage(s *session, c *webrtc.ICECandidate) (Message, error) { data := make(map[string]interface{}) data["type"] = "candidate" - data["candidate"] = c.ToJSON() + + if c.Typ == webrtc.ICECandidateTypeHost && !isIPAddress(c.Address) { + // If the address is not an IP, we assume it's a hostname (FQDN) + // and pass it through as such. + data["candidate"] = marshalHostCandidate(c) + } else { + data["candidate"] = c.ToJSON() + } + js, err := json.Marshal(data) if err != nil { return Message{}, err diff --git a/service/rtc/msg_test.go b/service/rtc/msg_test.go new file mode 100644 index 0000000..d1f5321 --- /dev/null +++ b/service/rtc/msg_test.go @@ -0,0 +1,67 @@ +// Copyright (c) 2022-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package rtc + +import ( + "testing" + + "github.com/pion/webrtc/v3" + "github.com/stretchr/testify/require" +) + +func TestNewICEMessage(t *testing.T) { + t.Run("host candidate - ip address", func(t *testing.T) { + msg, err := newICEMessage(&session{ + cfg: SessionConfig{ + SessionID: "sessionID", + UserID: "userID", + CallID: "callID", + GroupID: "groupID", + }, + }, &webrtc.ICECandidate{ + Address: "1.1.1.1", + Port: 8443, + Priority: 45, + Typ: webrtc.ICECandidateTypeHost, + Protocol: webrtc.ICEProtocolUDP, + Foundation: "2145320272", + }) + require.NoError(t, err) + require.Equal(t, Message{ + SessionID: "sessionID", + UserID: "userID", + CallID: "callID", + GroupID: "groupID", + Type: ICEMessage, + Data: []byte(`{"candidate":{"candidate":"candidate:2145320272 0 udp 45 1.1.1.1 8443 typ host","sdpMid":"","sdpMLineIndex":0,"usernameFragment":null},"type":"candidate"}`), + }, msg) + }) + + t.Run("host candidate - fqdn", func(t *testing.T) { + msg, err := newICEMessage(&session{ + cfg: SessionConfig{ + SessionID: "sessionID", + UserID: "userID", + CallID: "callID", + GroupID: "groupID", + }, + }, &webrtc.ICECandidate{ + Address: "example.tld", + Port: 8443, + Priority: 45, + Typ: webrtc.ICECandidateTypeHost, + Protocol: webrtc.ICEProtocolUDP, + Foundation: "2145320272", + }) + require.NoError(t, err) + require.Equal(t, Message{ + SessionID: "sessionID", + UserID: "userID", + CallID: "callID", + GroupID: "groupID", + Type: ICEMessage, + Data: []byte(`{"candidate":{"candidate":"candidate:2145320272 0 udp 45 example.tld 8443 typ host","sdpMid":"","sdpMLineIndex":0,"usernameFragment":null},"type":"candidate"}`), + }, msg) + }) +} diff --git a/service/rtc/server_test.go b/service/rtc/server_test.go index 1772bba..91caaf2 100644 --- a/service/rtc/server_test.go +++ b/service/rtc/server_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/netip" + "strings" "sync" "testing" "time" @@ -735,3 +736,112 @@ func TestICEHostPortOverride(t *testing.T) { } }) } + +func TestICEHostOverrideFQDN(t *testing.T) { + log, err := logger.New(logger.Config{ + EnableConsole: true, + ConsoleLevel: "DEBUG", + }) + require.NoError(t, err) + defer func() { + err := log.Shutdown() + require.NoError(t, err) + }() + + metrics := perf.NewMetrics("rtcd", nil) + require.NotNil(t, metrics) + + gatherCandidates := func(serverCfg ServerConfig, publicAddrsMap map[netip.Addr]string) <-chan string { + s, err := NewServer(serverCfg, log, metrics) + require.NoError(t, err) + require.NotNil(t, s) + + if publicAddrsMap != nil { + s.publicAddrsMap = publicAddrsMap + } + + err = s.Start() + require.NoError(t, err) + defer func() { + err := s.Stop() + require.NoError(t, err) + }() + + cfg := SessionConfig{ + GroupID: random.NewID(), + CallID: random.NewID(), + UserID: random.NewID(), + SessionID: random.NewID(), + } + err = s.InitSession(cfg, nil) + require.NoError(t, err) + + pc, err := webrtc.NewPeerConnection(webrtc.Configuration{}) + require.NoError(t, err) + defer pc.Close() + + dc, err := pc.CreateDataChannel("calls-dc", nil) + require.NoError(t, err) + require.NotNil(t, dc) + + offer, err := pc.CreateOffer(nil) + require.NoError(t, err) + + err = pc.SetLocalDescription(offer) + require.NoError(t, err) + + offerData, err := json.Marshal(&offer) + require.NoError(t, err) + + err = s.Send(Message{ + GroupID: cfg.GroupID, + CallID: cfg.CallID, + UserID: cfg.UserID, + SessionID: cfg.SessionID, + Type: SDPMessage, + Data: offerData, + }) + require.NoError(t, err) + + candidatesCh := make(chan string, 10) + go func() { + for msg := range s.ReceiveCh() { + if msg.Type == ICEMessage { + data := make(map[string]any) + err := json.Unmarshal(msg.Data, &data) + require.NoError(t, err) + iceString := data["candidate"].(map[string]interface{})["candidate"].(string) + candidatesCh <- iceString + } + } + }() + + time.Sleep(time.Second) + + err = s.CloseSession(cfg.SessionID) + require.NoError(t, err) + + return candidatesCh + } + + t.Run("FQDN host override with no resolution", func(t *testing.T) { + serverCfg := ServerConfig{ + ICEPortUDP: 30433, + ICEPortTCP: 30433, + ICEHostPortOverride: "8443", + ICEHostOverride: "example.tld", + ICEHostOverrideResolution: false, + } + candidatesCh := gatherCandidates(serverCfg, nil) + require.NotEmpty(t, candidatesCh) + var found bool + for i := 0; i < len(candidatesCh); i++ { + candidateStr := <-candidatesCh + if strings.Contains(candidateStr, "typ host") { + require.Contains(t, candidateStr, "example.tld 8443") + found = true + } + } + require.True(t, found) + }) +} diff --git a/service/rtc/sfu.go b/service/rtc/sfu.go index cfffa22..b62d936 100644 --- a/service/rtc/sfu.go +++ b/service/rtc/sfu.go @@ -95,7 +95,8 @@ func (s *Server) initSettingEngine() (webrtc.SettingEngine, error) { sEngine.SetDTLSInsecureSkipHelloVerify(true) } - pairs, err := generateAddrsPairs(s.localIPs, s.publicAddrsMap, s.cfg.ICEHostOverride, s.cfg.EnableIPv6) + pairs, err := generateAddrsPairs(s.localIPs, s.publicAddrsMap, s.cfg.ICEHostOverride, + s.cfg.EnableIPv6, s.cfg.ICEHostOverrideResolution) if err != nil { return webrtc.SettingEngine{}, fmt.Errorf("failed to generate addresses pairs: %w", err) } else if len(pairs) > 0 { @@ -278,6 +279,26 @@ func (s *Server) InitSession(cfg SessionConfig, closeCb func() error) error { } } + // If the ICE host override is a FQDN and resolution is off, we pass it through to the client unchanged. + if candidate.Typ == webrtc.ICECandidateTypeHost && s.cfg.ICEHostOverride != "" && !isIPAddress(s.cfg.ICEHostOverride) && !s.cfg.ICEHostOverrideResolution { + s.log.Debug("overriding host address with fqdn", + mlog.String("sessionID", cfg.SessionID), + mlog.Uint("port", candidate.Port), + mlog.String("addr", candidate.Address), + mlog.Int("protocol", candidate.Protocol), + mlog.String("override", s.cfg.ICEHostOverride)) + candidate.Address = s.cfg.ICEHostOverride + if port := s.cfg.ICEHostPortOverride.SinglePort(); port != 0 { + s.log.Debug("overriding host candidate port", + mlog.String("sessionID", cfg.SessionID), + mlog.Uint("port", candidate.Port), + mlog.Int("override", port), + mlog.String("addr", candidate.Address), + mlog.Int("protocol", candidate.Protocol)) + candidate.Port = uint16(port) + } + } + msg, err := newICEMessage(us, candidate) if err != nil { s.log.Error("failed to create ICE message", mlog.Err(err), mlog.String("sessionID", cfg.SessionID)) diff --git a/service/rtc/utils.go b/service/rtc/utils.go index 7223761..83401aa 100644 --- a/service/rtc/utils.go +++ b/service/rtc/utils.go @@ -58,7 +58,7 @@ func getTrackType(kind webrtc.RTPCodecType) string { return "unknown" } -func generateAddrsPairs(localIPs []netip.Addr, publicAddrsMap map[netip.Addr]string, hostOverride string, dualStack bool) ([]string, error) { +func generateAddrsPairs(localIPs []netip.Addr, publicAddrsMap map[netip.Addr]string, hostOverride string, dualStack bool, resolveOverride bool) ([]string, error) { var err error var pairs []string var hostOverrideIP string @@ -74,12 +74,14 @@ func generateAddrsPairs(localIPs []netip.Addr, publicAddrsMap map[netip.Addr]str ipNetwork = "ip" } - // If the override is set we resolve it in case it's a hostname. - if hostOverride != "" { + // If the override is a hostname and server-side resolving is on, we try to resolve it. + if hostOverride != "" && !isIPAddress(hostOverride) && resolveOverride { hostOverrideIP, err = resolveHost(hostOverride, ipNetwork, time.Second) if err != nil { return pairs, fmt.Errorf("failed to resolve host: %w", err) } + } else if isIPAddress(hostOverride) { + hostOverrideIP = hostOverride } // Nothing to do at this point if no local IP was found. @@ -172,3 +174,10 @@ func pickRandom[S ~[]*E, E any](s S) *E { } return s[rand.Intn(len(s))] } + +func isIPAddress(addr string) bool { + if _, err := netip.ParseAddr(addr); err == nil { + return true + } + return false +} diff --git a/service/rtc/utils_test.go b/service/rtc/utils_test.go index 0eb9d7f..0fc8de0 100644 --- a/service/rtc/utils_test.go +++ b/service/rtc/utils_test.go @@ -12,11 +12,11 @@ import ( func TestGenerateAddrsPairs(t *testing.T) { t.Run("nil/empty inputs", func(t *testing.T) { - pairs, err := generateAddrsPairs(nil, nil, "", false) + pairs, err := generateAddrsPairs(nil, nil, "", false, false) require.NoError(t, err) require.Empty(t, pairs) - pairs, err = generateAddrsPairs([]netip.Addr{}, map[netip.Addr]string{}, "", false) + pairs, err = generateAddrsPairs([]netip.Addr{}, map[netip.Addr]string{}, "", false, false) require.NoError(t, err) require.Empty(t, pairs) }) @@ -28,7 +28,7 @@ func TestGenerateAddrsPairs(t *testing.T) { }, map[netip.Addr]string{ netip.MustParseAddr("127.0.0.1"): "", netip.MustParseAddr("10.1.1.1"): "", - }, "", false) + }, "", false, false) require.NoError(t, err) require.Equal(t, []string{"127.0.0.1/127.0.0.1", "10.1.1.1/10.1.1.1"}, pairs) }) @@ -37,7 +37,7 @@ func TestGenerateAddrsPairs(t *testing.T) { pairs, err := generateAddrsPairs([]netip.Addr{ netip.MustParseAddr("127.0.0.1"), netip.MustParseAddr("10.1.1.1"), - }, map[netip.Addr]string{}, "1.1.1.1/127.0.0.1,1.1.1.1/10.1.1.1", false) + }, map[netip.Addr]string{}, "1.1.1.1/127.0.0.1,1.1.1.1/10.1.1.1", false, false) require.NoError(t, err) require.Equal(t, []string{"1.1.1.1/127.0.0.1", "1.1.1.1/10.1.1.1"}, pairs) }) @@ -49,7 +49,7 @@ func TestGenerateAddrsPairs(t *testing.T) { }, map[netip.Addr]string{ netip.MustParseAddr("127.0.0.1"): "", netip.MustParseAddr("10.1.1.1"): "", - }, "1.1.1.1", false) + }, "1.1.1.1", false, false) require.NoError(t, err) require.Equal(t, []string{"127.0.0.1/127.0.0.1", "1.1.1.1/10.1.1.1"}, pairs) }) @@ -61,7 +61,7 @@ func TestGenerateAddrsPairs(t *testing.T) { }, map[netip.Addr]string{ netip.MustParseAddr("127.0.0.1"): "", netip.MustParseAddr("10.1.1.1"): "1.1.1.1", - }, "", false) + }, "", false, false) require.NoError(t, err) require.Equal(t, []string{"127.0.0.1/127.0.0.1", "1.1.1.1/10.1.1.1"}, pairs) }) @@ -73,7 +73,7 @@ func TestGenerateAddrsPairs(t *testing.T) { }, map[netip.Addr]string{ netip.MustParseAddr("127.0.0.1"): "", netip.MustParseAddr("10.1.1.1"): "1.1.1.1", - }, "", false) + }, "", false, false) require.NoError(t, err) require.Equal(t, []string{"127.0.0.1/127.0.0.1", "1.1.1.1/10.1.1.1"}, pairs) }) @@ -85,7 +85,7 @@ func TestGenerateAddrsPairs(t *testing.T) { }, map[netip.Addr]string{ netip.MustParseAddr("127.0.0.1"): "1.1.1.1", netip.MustParseAddr("10.1.1.1"): "1.1.1.2", - }, "", false) + }, "", false, false) require.NoError(t, err) require.Equal(t, []string{"1.1.1.1/127.0.0.1", "1.1.1.2/10.1.1.1"}, pairs) }) @@ -99,10 +99,34 @@ func TestGenerateAddrsPairs(t *testing.T) { }, map[netip.Addr]string{ netip.MustParseAddr("127.0.0.1"): "1.1.1.1", netip.MustParseAddr("10.1.1.1"): "1.1.1.2", - }, "8.8.8.8", false) + }, "8.8.8.8", false, false) require.NoError(t, err) require.Equal(t, []string{"127.0.0.1/127.0.0.1", "8.8.8.8/10.1.1.1"}, pairs) }) + + t.Run("ice host override is a FQDN, resolve on", func(t *testing.T) { + pairs, err := generateAddrsPairs([]netip.Addr{ + netip.MustParseAddr("127.0.0.1"), + netip.MustParseAddr("10.1.1.1"), + }, map[netip.Addr]string{ + netip.MustParseAddr("127.0.0.1"): "", + netip.MustParseAddr("10.1.1.1"): "", + }, "localhost", false, true) + require.NoError(t, err) + require.Equal(t, []string{"127.0.0.1/127.0.0.1", "127.0.0.1/10.1.1.1"}, pairs) + }) + + t.Run("ice host override is a FQDN, resolve off", func(t *testing.T) { + pairs, err := generateAddrsPairs([]netip.Addr{ + netip.MustParseAddr("127.0.0.1"), + netip.MustParseAddr("10.1.1.1"), + }, map[netip.Addr]string{ + netip.MustParseAddr("127.0.0.1"): "", + netip.MustParseAddr("10.1.1.1"): "", + }, "localhost", false, false) + require.NoError(t, err) + require.Equal(t, []string{"127.0.0.1/127.0.0.1", "10.1.1.1/10.1.1.1"}, pairs) + }) } func TestIsValidTrackID(t *testing.T) {