Skip to content

Commit

Permalink
Fix issue with failing on rejected mlines
Browse files Browse the repository at this point in the history
  • Loading branch information
LVala committed Aug 5, 2024
1 parent af1fec4 commit a52603e
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 17 deletions.
16 changes: 11 additions & 5 deletions lib/ex_webrtc/peer_connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1398,10 +1398,8 @@ defmodule ExWebRTC.PeerConnection do
defp apply_remote_description(%SessionDescription{type: type, sdp: raw_sdp}, state) do
with {:ok, next_sig_state} <- next_signaling_state(state.signaling_state, :remote, type),
{:ok, sdp} <- parse_sdp(raw_sdp),
:ok <- SDPUtils.ensure_mid(sdp),
:ok <- SDPUtils.ensure_bundle(sdp),
:ok <- SDPUtils.ensure_rtcp_mux(sdp),
{:ok, {ice_ufrag, ice_pwd}} <- SDPUtils.get_ice_credentials(sdp),
:ok <- SDPUtils.ensure_valid(sdp),
{:ok, ice_creds} <- SDPUtils.get_ice_credentials(sdp),
{:ok, {:fingerprint, {:sha256, peer_fingerprint}}} <- SDPUtils.get_cert_fingerprint(sdp),
{:ok, dtls_role} <- SDPUtils.get_dtls_role(sdp) do
config = Configuration.update(state.config, sdp)
Expand All @@ -1420,8 +1418,16 @@ defmodule ExWebRTC.PeerConnection do
dtls_role = if dtls_role in [:actpass, :passive], do: :active, else: :passive
DTLSTransport.start_dtls(state.dtls_transport, dtls_role, peer_fingerprint)

# ice_creds will be nil if all of the mlines in the description are rejected
# in such case, if this is the first remote description, connection won't be established
# TODO: this can result in ICE restart (when it should, e.g. when this is answer)
:ok = state.ice_transport.set_remote_credentials(state.ice_pid, ice_ufrag, ice_pwd)
case ice_creds do
nil ->
:ok

{ice_ufrag, ice_pwd} ->
:ok = state.ice_transport.set_remote_credentials(state.ice_pid, ice_ufrag, ice_pwd)
end

for candidate <- SDPUtils.get_ice_candidates(sdp) do
state.ice_transport.add_remote_candidate(state.ice_pid, candidate)
Expand Down
35 changes: 23 additions & 12 deletions lib/ex_webrtc/sdp_utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,22 @@ defmodule ExWebRTC.SDPUtils do

@type extension() :: {Extension.SourceDescription, atom()}

@spec ensure_mid(ExSDP.t()) :: :ok | {:error, :missing_mid | :duplicated_mid}
@spec ensure_valid(ExSDP.t()) :: :ok | {:error, term()}
def ensure_valid(sdp) do
with :ok <- ensure_non_empty(sdp),
:ok <- ensure_mid(sdp),
:ok <- ensure_bundle(sdp) do
ensure_rtcp_mux(sdp)
end
end

defp ensure_non_empty(sdp) do
case length(sdp.media) do
0 -> {:error, :empty_sdp}
_other -> :ok
end
end

def ensure_mid(sdp) do
sdp.media
|> Enum.reduce_while({:ok, []}, fn media, {:ok, acc} ->
Expand All @@ -24,13 +39,6 @@ defmodule ExWebRTC.SDPUtils do
end
end

@spec ensure_bundle(ExSDP.t()) ::
:ok
| {:error,
:non_exhaustive_bundle_group
| :missing_bundle_group
| :multiple_bundle_groups
| :invalid_bundle_group}
def ensure_bundle(sdp) do
groups = ExSDP.get_attributes(sdp, ExSDP.Attribute.Group)

Expand All @@ -56,7 +64,6 @@ defmodule ExWebRTC.SDPUtils do
Enum.filter(groups, fn %ExSDP.Attribute.Group{semantics: name} -> name == to_filter end)
end

@spec ensure_rtcp_mux(ExSDP.t()) :: :ok | {:error, :missing_rtcp_mux}
def ensure_rtcp_mux(sdp) do
sdp.media
|> Enum.all?(&(ExSDP.get_attribute(&1, :rtcp_mux) == :rtcp_mux))
Expand Down Expand Up @@ -116,20 +123,24 @@ defmodule ExWebRTC.SDPUtils do
end

@spec get_ice_credentials(ExSDP.t()) ::
{:ok, {binary(), binary()}}
{:ok, {binary(), binary()} | nil}
| {:error,
:missing_ice_pwd
| :missing_ice_ufrag
| :missing_ice_credentials
| :conflicting_ice_credentials}
def get_ice_credentials(sdp) do
session_creds = do_get_ice_credentials(sdp)
mline_creds = Enum.map(sdp.media, fn mline -> do_get_ice_credentials(mline) end)

mline_creds =
sdp.media
|> Enum.reject(&rejected?/1)
|> Enum.map(fn mline -> do_get_ice_credentials(mline) end)

case {session_creds, mline_creds} do
# no session creds and no mlines (empty SDP)
{{nil, nil}, []} ->
{:error, :missing_ice_credentials}
{:ok, nil}

# session creds but no mlines (empty SDP)
{session_creds, []} ->
Expand Down
56 changes: 56 additions & 0 deletions test/ex_webrtc/peer_connection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,62 @@ defmodule ExWebRTC.PeerConnectionTest do
assert expected_result == PeerConnection.set_remote_description(pc, offer)
end)
end

test "mline rejected" do
{:ok, pc} = PeerConnection.start_link()

{:ok, _sender} = PeerConnection.add_track(pc, MediaStreamTrack.new(:video))
{:ok, _sender} = PeerConnection.add_track(pc, MediaStreamTrack.new(:audio))
{:ok, offer} = PeerConnection.create_offer(pc)
:ok = PeerConnection.set_local_description(pc, offer)

# first m-line is rejected = ice ufrag and passwd are random (thats what Chromium seems to do)
sdp =
"""
v=0
o=- 4455712322662451611 2 IN IP4 127.0.0.1
s=-
t=0 0
a=group:BUNDLE 1
a=extmap-allow-mixed
a=msid-semantic: WMS
m=video 0 UDP/TLS/RTP/SAVPF 0
c=IN IP4 0.0.0.0
a=rtcp:9 IN IP4 0.0.0.0
a=ice-ufrag:oIRa
a=ice-pwd:10rPa8NrMCm602mt1OVkUHJs
a=ice-options:trickle
a=fingerprint:sha-256 C8:06:34:28:7C:5B:55:00:42:61:54:D2:84:29:B5:07:3D:9A:6C:5C:C6:79:B1:B0:A8:12:30:AD:26:36:93:45
a=setup:active
a=mid:0
a=extmap:3 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=extmap:1 urn:ietf:params:rtp-hdrext:sdes:mid
a=extmap:2 urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id
a=extmap:4 urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id
a=sendonly
a=rtcp-mux
m=audio 9 UDP/TLS/RTP/SAVPF 111
c=IN IP4 0.0.0.0
a=rtcp:9 IN IP4 0.0.0.0
a=ice-ufrag:Anyh
a=ice-pwd:cdaU0jbHlOTf98BNTRoZxMo1
a=ice-options:trickle
a=fingerprint:sha-256 C8:06:34:28:7C:5B:55:00:42:61:54:D2:84:29:B5:07:3D:9A:6C:5C:C6:79:B1:B0:A8:12:30:AD:26:36:93:45
a=setup:active
a=mid:1
a=extmap:3 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=extmap:1 urn:ietf:params:rtp-hdrext:sdes:mid
a=recvonly
a=rtcp-mux
a=rtpmap:111 opus/48000/2
a=rtcp-fb:111 transport-cc
a=fmtp:111 minptime=10;useinbandfec=1
"""

answer = %SessionDescription{type: :answer, sdp: sdp}

assert :ok = PeerConnection.set_remote_description(pc, answer)
end
end

describe "add_transceiver/3" do
Expand Down

0 comments on commit a52603e

Please sign in to comment.