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.

Sign up for GitHub

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

Verify peer cert fingerprint #22

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Verify peer cert fingerprint #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
885f861
Verify peer cert fingerprint
mickel8 Nov 24, 2023
66e9c67
Convert PeerConnection internal state into map (#24)
mickel8 Nov 28, 2023
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
48 changes: 32 additions & 16 deletions lib/ex_webrtc/dtls_transport.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule ExWebRTC.DTLSTransport do
require Logger

alias ExICE.ICEAgent
alias ExWebRTC.Utils

@type dtls_transport() :: GenServer.server()

Expand Down Expand Up @@ -48,9 +49,10 @@ defmodule ExWebRTC.DTLSTransport do
end

@doc false
@spec start_dtls(dtls_transport(), :active | :passive) :: :ok | {:error, :already_started}
def start_dtls(dtls_transport, mode) do
GenServer.call(dtls_transport, {:start_dtls, mode})
@spec start_dtls(dtls_transport(), :active | :passive, binary()) ::
:ok | {:error, :already_started}
def start_dtls(dtls_transport, mode, peer_fingerprint) do
GenServer.call(dtls_transport, {:start_dtls, mode, peer_fingerprint})
end

@doc false
Expand All @@ -61,11 +63,8 @@ defmodule ExWebRTC.DTLSTransport do

@impl true
def init([ice_config, ice_module, owner]) do
# temporary hack to generate certs
dtls = ExDTLS.init(client_mode: true, dtls_srtp: true)
cert = ExDTLS.get_cert(dtls)
pkey = ExDTLS.get_pkey(dtls)
fingerprint = ExDTLS.get_cert_fingerprint(dtls)
{pkey, cert} = ExDTLS.generate_key_cert()
fingerprint = ExDTLS.get_cert_fingerprint(cert)

{:ok, ice_agent} = ice_module.start_link(:controlled, ice_config)
srtp = ExLibSRTP.new()
Expand All @@ -78,6 +77,8 @@ defmodule ExWebRTC.DTLSTransport do
cert: cert,
pkey: pkey,
fingerprint: fingerprint,
# sha256 hex dump
peer_fingerprint: nil,
srtp: srtp,
dtls_state: :new,
dtls: nil,
Expand All @@ -100,22 +101,25 @@ defmodule ExWebRTC.DTLSTransport do
end

@impl true
def handle_call({:start_dtls, mode}, _from, %{dtls: nil} = state)
def handle_call({:start_dtls, mode, peer_fingerprint}, _from, %{dtls: nil} = state)
when mode in [:active, :passive] do
ex_dtls_mode = if mode == :active, do: :client, else: :server

dtls =
ExDTLS.init(
client_mode: mode == :active,
mode: ex_dtls_mode,
dtls_srtp: true,
pkey: state.pkey,
cert: state.cert
cert: state.cert,
verify_peer: true
)

state = %{state | dtls: dtls, mode: mode}
state = %{state | dtls: dtls, mode: mode, peer_fingerprint: peer_fingerprint}
{:reply, :ok, state}
end

@impl true
def handle_call({:start_dtls, _mode}, _from, state) do
def handle_call({:start_dtls, _mode, _peer_fingerprint}, _from, state) do
# is there a case when mode will change and new handshake will be needed?
{:reply, {:error, :already_started}, state}
end
Expand Down Expand Up @@ -198,9 +202,20 @@ defmodule ExWebRTC.DTLSTransport do
{:handshake_finished, _, remote_keying_material, profile, packets} ->
Logger.debug("DTLS handshake finished")
ICEAgent.send_data(state.ice_agent, packets)
# TODO: validate fingerprint
state = setup_srtp(state, remote_keying_material, profile)
update_dtls_state(state, :connected)

peer_fingerprint =
state.dtls
|> ExDTLS.get_peer_cert()
|> ExDTLS.get_cert_fingerprint()
|> Utils.hex_dump()

if peer_fingerprint == state.peer_fingerprint do
state = setup_srtp(state, remote_keying_material, profile)
update_dtls_state(state, :connected)
else
Logger.debug("Non-matching peer cert fingerprint.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably a rare error, isn't it? Maybe Logger.debug is too lax for the severity? State change to :failed doesn't really tell much about the reason, so it's gonna be easy to miss.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. PeerConnection state failed means that either ICE failed or DTLS failed. Because user has access to ICE state it can easily know that the problem is failing DTLS.

I would say that all our logs should be on debug as we create a library. It's API role to give user sufficient information on what's going on

update_dtls_state(state, :failed)
end

{:handshake_finished, _, remote_keying_material, profile} ->
Logger.debug("DTLS handshake finished")
Expand Down Expand Up @@ -283,6 +298,7 @@ defmodule ExWebRTC.DTLSTransport do
defp update_dtls_state(%{dtls_state: dtls_state} = state, dtls_state), do: state

defp update_dtls_state(state, new_dtls_state) do
Logger.debug("Changing DTLS state: #{state.dtls_state} -> #{new_dtls_state}")
notify(state.owner, {:state_change, new_dtls_state})
%{state | dtls_state: new_dtls_state}
end
Expand Down
72 changes: 42 additions & 30 deletions lib/ex_webrtc/peer_connection.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,6 @@ defmodule ExWebRTC.PeerConnection do
"""
@type connection_state() :: :closed | :failed | :disconnected | :new | :connecting | :connected

@enforce_keys [:config, :owner]
defstruct @enforce_keys ++
[
:current_local_desc,
:pending_local_desc,
:current_remote_desc,
:pending_remote_desc,
:ice_agent,
:dtls_transport,
demuxer: %Demuxer{},
transceivers: [],
ice_state: nil,
dtls_state: nil,
signaling_state: :stable,
conn_state: :new,
last_offer: nil,
last_answer: nil
]

#### API ####
@spec start_link(Configuration.options()) :: GenServer.on_start()
def start_link(options \\ []) do
Expand Down Expand Up @@ -131,13 +112,24 @@ defmodule ExWebRTC.PeerConnection do
{:ok, dtls_transport} = DTLSTransport.start_link(ice_config)
ice_agent = DTLSTransport.get_ice_agent(dtls_transport)

state = %__MODULE__{
state = %{
owner: owner,
config: config,
current_local_desc: nil,
pending_local_desc: nil,
current_remote_desc: nil,
pending_remote_desc: nil,
ice_agent: ice_agent,
dtls_transport: dtls_transport,
demuxer: %Demuxer{},
transceivers: [],
ice_state: :new,
dtls_state: :new
dtls_state: :new,
signaling_state: :stable,
conn_state: :new,
last_offer: nil,
last_answer: nil,
peer_fingerprint: nil
}

notify(state.owner, {:connection_state_change, :new})
Expand Down Expand Up @@ -280,7 +272,7 @@ defmodule ExWebRTC.PeerConnection do
{:ok, state} <- apply_local_description(other_type, sdp, state) do
{:reply, :ok, %{state | signaling_state: next_state}}
else
error -> {:reply, error, state}
{:error, _reason} = error -> {:reply, error, state}
end
end
end
Expand All @@ -299,7 +291,7 @@ defmodule ExWebRTC.PeerConnection do
{:ok, state} <- apply_remote_description(other_type, sdp, state) do
{:reply, :ok, %{state | signaling_state: next_state}}
else
error -> {:reply, error, state}
{:error, _reason} = error -> {:reply, error, state}
end
end
end
Expand Down Expand Up @@ -348,10 +340,16 @@ defmodule ExWebRTC.PeerConnection do

@impl true
def handle_info({:ex_ice, _from, {:connection_state_change, new_ice_state}}, state) do
state = %__MODULE__{state | ice_state: new_ice_state}
state = %{state | ice_state: new_ice_state}
next_conn_state = next_conn_state(new_ice_state, state.dtls_state)
state = update_conn_state(state, next_conn_state)
{:noreply, state}

if next_conn_state == :failed do
Logger.debug("Stopping PeerConnection")
{:stop, {:shutdown, :conn_state_failed}, state}
else
{:noreply, state}
end
end

@impl true
Expand All @@ -370,7 +368,7 @@ defmodule ExWebRTC.PeerConnection do

@impl true
def handle_info({:dtls_transport, _pid, {:state_change, new_dtls_state}}, state) do
state = %__MODULE__{state | dtls_state: new_dtls_state}
state = %{state | dtls_state: new_dtls_state}
next_conn_state = next_conn_state(state.ice_state, new_dtls_state)
state = update_conn_state(state, next_conn_state)
{:noreply, state}
Expand All @@ -381,7 +379,7 @@ defmodule ExWebRTC.PeerConnection do
case Demuxer.demux(state.demuxer, data) do
{:ok, demuxer, mid, packet} ->
notify(state.owner, {:data, {mid, packet}})
{:noreply, %__MODULE__{state | demuxer: demuxer}}
{:noreply, %{state | demuxer: demuxer}}

{:error, reason} ->
Logger.error("Unable to demux RTP, reason: #{inspect(reason)}")
Expand All @@ -408,7 +406,7 @@ defmodule ExWebRTC.PeerConnection do
dtls =
if type == :answer do
{:setup, setup} = ExSDP.Media.get_attribute(hd(sdp.media), :setup)
:ok = DTLSTransport.start_dtls(state.dtls_transport, setup)
:ok = DTLSTransport.start_dtls(state.dtls_transport, setup, state.peer_fingerprint)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it an issue that the fingerprint here might be nil as far as I understand?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never happen as we can't set SDP answer before receiving SDP offer. When we receive SDP offer we save peer_fingerprint in the state

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right

else
state.dtls_transport
end
Expand All @@ -430,6 +428,7 @@ defmodule ExWebRTC.PeerConnection do
with :ok <- SDPUtils.ensure_mid(sdp),
:ok <- SDPUtils.ensure_bundle(sdp),
{:ok, {ice_ufrag, ice_pwd}} <- SDPUtils.get_ice_credentials(sdp),
{:ok, {:fingerprint, {:sha256, peer_fingerprint}}} <- SDPUtils.get_cert_fingerprint(sdp),
{:ok, new_transceivers} <-
update_remote_transceivers(state.transceivers, sdp, state.config) do
:ok = ICEAgent.set_remote_credentials(state.ice_agent, ice_ufrag, ice_pwd)
Expand Down Expand Up @@ -460,12 +459,24 @@ defmodule ExWebRTC.PeerConnection do
:passive -> :active
end

:ok = DTLSTransport.start_dtls(state.dtls_transport, setup)
:ok = DTLSTransport.start_dtls(state.dtls_transport, setup, peer_fingerprint)
else
state.dtls_transport
end

{:ok, %{state | transceivers: new_transceivers, dtls_transport: dtls}}
{:ok,
%{
state
| transceivers: new_transceivers,
dtls_transport: dtls,
peer_fingerprint: peer_fingerprint
}}
else
{:ok, {:fingerprint, {_hash_function, _fingerprint}}} ->
{:error, :unsupported_cert_fingerprint_hash_function}

{:error, _reason} = error ->
error
end
end

Expand Down Expand Up @@ -569,6 +580,7 @@ defmodule ExWebRTC.PeerConnection do
defp update_conn_state(%{conn_state: conn_state} = state, conn_state), do: state

defp update_conn_state(state, new_conn_state) do
Logger.debug("Changing PeerConnection state: #{state.conn_state} -> #{new_conn_state}")
notify(state.owner, {:connection_state_change, new_conn_state})
%{state | conn_state: new_conn_state}
end
Expand Down
73 changes: 73 additions & 0 deletions lib/ex_webrtc/sdp_utils.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,26 @@ defmodule ExWebRTC.SDPUtils do
end)
end

@spec delete_attribute(ExSDP.t() | ExSDP.Media.t(), module() | atom() | binary()) ::
ExSDP.t() | ExSDP.Media.t()
def delete_attribute(sdp_or_mline, key) do
delete_attributes(sdp_or_mline, [key])
end

@spec delete_attributes(ExSDP.t() | ExSDP.Media.t(), [module() | atom() | binary()]) ::
ExSDP.t() | ExSDP.Media.t()
def delete_attributes(sdp_or_mline, attributes) when is_list(attributes) do
new_attrs =
Enum.reject(sdp_or_mline.attributes, fn
%module{} -> module in attributes
{k, _v} -> k in attributes
# flag attributes
k -> k in attributes
end)

Map.put(sdp_or_mline, :attributes, new_attrs)
end

@spec ensure_mid(ExSDP.t()) :: :ok | {:error, :missing_mid | :duplicated_mid}
def ensure_mid(sdp) do
sdp.media
Expand Down Expand Up @@ -101,6 +121,34 @@ defmodule ExWebRTC.SDPUtils do
end
end

@spec get_cert_fingerprint(ExSDP.t()) ::
{:ok, {:fingerprint, {:sha256, binary()}}}
| {:error, :missing_cert_fingerprint | :conflicting_cert_fingerprints}
def get_cert_fingerprint(sdp) do
session_fingerprint = do_get_cert_fingerprint(sdp)
mline_fingerprints = Enum.map(sdp.media, fn mline -> do_get_cert_fingerprint(mline) end)

case {session_fingerprint, mline_fingerprints} do
{nil, []} ->
{:error, :missing_cert_fingerprint}

{session_fingerprint, []} ->
{:ok, session_fingerprint}

{nil, mline_fingerprints} ->
with :ok <- ensure_fingerprints_present(mline_fingerprints),
:ok <- ensure_fingerprints_unique(mline_fingerprints) do
{:ok, List.first(mline_fingerprints)}
end

{session_fingerprint, mline_fingerprints} ->
with :ok <- ensure_fingerprints_present(mline_fingerprints),
:ok <- ensure_fingerprints_unique([session_fingerprint | mline_fingerprints]) do
{:ok, session_fingerprint}
end
end
end

@spec get_extensions(ExSDP.t()) ::
%{(id :: non_neg_integer()) => extension :: module() | {SourceDescription, atom()}}
def get_extensions(sdp) do
Expand Down Expand Up @@ -156,6 +204,16 @@ defmodule ExWebRTC.SDPUtils do
{ice_ufrag, ice_pwd}
end

defp do_get_cert_fingerprint(sdp_or_mline) do
get_attr =
case sdp_or_mline do
%ExSDP{} -> &ExSDP.get_attribute/2
%ExSDP.Media{} -> &ExSDP.Media.get_attribute/2
end

get_attr.(sdp_or_mline, :fingerprint)
end

defp ensure_ice_credentials_present(creds) do
creds
|> Enum.find(fn {ice_ufrag, ice_pwd} -> ice_ufrag == nil or ice_pwd == nil end)
Expand All @@ -180,4 +238,19 @@ defmodule ExWebRTC.SDPUtils do
_ -> {:error, :conflicting_ice_credentials}
end
end

defp ensure_fingerprints_present(fingerprints) do
if Enum.all?(fingerprints, &(&1 != nil)) do
:ok
else
{:error, :missing_cert_fingerprint}
end
end

defp ensure_fingerprints_unique(fingerprints) do
case Enum.uniq(fingerprints) do
[_] -> :ok
_ -> {:error, :conflicting_cert_fingerprints}
end
end
end
2 changes: 1 addition & 1 deletion mix.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ defmodule ExWebRTC.MixProject do
[
{:ex_sdp, "~> 0.13.0"},
{:ex_ice, "~> 0.2.0"},
{:ex_dtls, "~> 0.14.0"},
{:ex_dtls, github: "elixir-webrtc/ex_dtls"},
{:ex_libsrtp, "~> 0.6.0"},
{:ex_rtp, "~> 0.2.0"},
{:ex_rtcp, "~> 0.1.0"},
Expand Down
Loading
Loading