Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-60256] Support for a FQDN as ice host candidate override #155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions build/test/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
24 changes: 13 additions & 11 deletions build/test/prepare-plugin.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#/bin/bash
set -x
set -xe

GIT_DEFAULT_BRANCH="main"
GIT_REPO="https://github.com/mattermost/mattermost-plugin-calls"
Expand All @@ -14,27 +14,29 @@ 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

# Installation
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
5 changes: 5 additions & 0 deletions config/config.sample.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions service/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions service/rtc/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
43 changes: 42 additions & 1 deletion service/rtc/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 67 additions & 0 deletions service/rtc/msg_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
110 changes: 110 additions & 0 deletions service/rtc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net"
"net/netip"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -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)
})
}
23 changes: 22 additions & 1 deletion service/rtc/sfu.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down
Loading
Loading