From 816b701e8eb3a98e84c30abaaef1e4f9972f7dd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20=C5=9Aled=C5=BA?= <michalsledz34@gmail.com>
Date: Wed, 2 Aug 2023 17:38:47 +0200
Subject: [PATCH] Handle keepalives on selected pair

---
 lib/ice_agent.ex | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/lib/ice_agent.ex b/lib/ice_agent.ex
index 6e94d14..3143de2 100644
--- a/lib/ice_agent.ex
+++ b/lib/ice_agent.ex
@@ -456,21 +456,30 @@ defmodule ExICE.ICEAgent do
     with {:ok, key} <- authenticate_msg(msg, state.local_pwd),
          {:ok, prio_attr} <- get_prio_attribute(msg),
          {:ok, role_attr} <- get_role_attribute(msg),
+         use_cand_attr when use_cand_attr in [nil, %UseCandidate{}] <-
+           get_use_cand_attribute(msg),
          {{:ok, state}, _} <- {check_req_role_conflict(role_attr, state), key} do
       case find_host_cand(state.local_cands, socket) do
         nil ->
           # keepalive on pair selected before ice restart
+          # TODO can we reach this? Won't we use incorrect local_pwd for auth?
           send_binding_success_response(socket, src_ip, src_port, msg, key)
           state
 
         %Candidate{} = local_cand ->
           send_binding_success_response(socket, src_ip, src_port, msg, key)
           {remote_cand, state} = get_or_create_remote_cand(src_ip, src_port, prio_attr, state)
-          use_candidate = Message.get_attribute(msg, UseCandidate)
-          handle_conn_check_req(local_cand, remote_cand, use_candidate, state)
+          handle_conn_check_req(local_cand, remote_cand, use_cand_attr, state)
       end
     else
-      {:error, reason} when reason in [:invalid_priority_attribute, :no_priority_attribute] ->
+      {:error, reason}
+      when reason in [
+             :invalid_priority_attribute,
+             :no_priority_attribute,
+             :invalid_use_candidate_attribute
+           ] ->
+        # TODO should we reply with 400 bad request when
+        # attributes are invalid (they are present but invalid)
         # TODO should we authenticate?
         # chrome does not authenticate but section 6.3.1.1 suggests
         # we should add message-integrity
@@ -512,6 +521,11 @@ defmodule ExICE.ICEAgent do
           put_in(state, [:checklist, pair.id], pair)
         end
 
+      %CandidatePair{} = pair when pair == state.selected_pair ->
+        # keepalive - do nothing, we already replied 
+        Logger.debug("Received keepalive on selected pair: #{pair.id}")
+        state
+
       %CandidatePair{} = pair ->
         if use_candidate do
           if pair.state == :succeeded do
@@ -562,6 +576,15 @@ defmodule ExICE.ICEAgent do
     end
   end
 
+  defp get_use_cand_attribute(msg) do
+    # this function breaks the convention...
+    case Message.get_attribute(msg, UseCandidate) do
+      {:ok, attr} -> attr
+      {:error, _} -> {:error, :invalid_use_candidate_attribute}
+      nil -> nil
+    end
+  end
+
   defp check_req_role_conflict(
          %ICEControlling{tiebreaker: tiebreaker},
          %{role: :controlling} = state