Skip to content

Commit

Permalink
Fix set_remote_description failing on description with rejected m-l…
Browse files Browse the repository at this point in the history 
…ines (#145)
  • Loading branch information
@LVala
LVala authored Aug 6, 2024
1 parent af1fec4 commit 15eee61
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 22 deletions.
20 changes: 13 additions & 7 deletions lib/ex_webrtc/peer_connection.ex
View file
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,11 +1418,19 @@ 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)

for candidate <- SDPUtils.get_ice_candidates(sdp) do
state.ice_transport.add_remote_candidate(state.ice_pid, candidate)
for candidate <- SDPUtils.get_ice_candidates(sdp) do
state.ice_transport.add_remote_candidate(state.ice_pid, candidate)
end
end

state =
Expand Down
42 changes: 27 additions & 15 deletions lib/ex_webrtc/sdp_utils.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,24 @@ defmodule ExWebRTC.SDPUtils do

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

@spec ensure_mid(ExSDP.t()) :: :ok | {:error, :missing_mid | :duplicated_mid}
def ensure_mid(sdp) do
@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
if Enum.empty?(sdp.media) do
{:error, :empty_sdp}
else
:ok
end
end

defp ensure_mid(sdp) do
sdp.media
|> Enum.reduce_while({:ok, []}, fn media, {:ok, acc} ->
case ExSDP.get_attributes(media, :mid) do
Expand All @@ -24,14 +40,7 @@ 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
defp ensure_bundle(sdp) do
groups = ExSDP.get_attributes(sdp, ExSDP.Attribute.Group)

mline_mids = get_bundle_mids(sdp.media)
Expand All @@ -56,8 +65,7 @@ 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
defp ensure_rtcp_mux(sdp) do
sdp.media
|> Enum.all?(&(ExSDP.get_attribute(&1, :rtcp_mux) == :rtcp_mux))
|> case do
Expand Down Expand Up @@ -116,20 +124,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(&do_get_ice_credentials/1)

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
50 changes: 50 additions & 0 deletions test/ex_webrtc/peer_connection_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,56 @@ 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=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=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 15eee61

Please sign in to comment.