Refactor authentication

Author: mickel8

.github/workflows/ci.yml

@@ -6,8 +6,8 @@ jobs:
     name: lint OTP ${{matrix.otp}} / Elixir ${{matrix.elixir}}
     strategy:
       matrix:
-        otp: ['25']
-        elixir: ['1.13.2']
+        otp: ['26']
+        elixir: ['1.16']
     steps:
       - uses: actions/checkout@v2
       - uses: erlef/setup-beam@v1
@@ -16,6 +16,7 @@ jobs:
           elixir-version: ${{matrix.elixir}}
       - run: mix deps.get
       - run: mix credo
+      - run: mix dialyzer
       - run: mix format --check-formatted
       - run: mix docs 2>&1 | (! grep -q "warning:")
 
@@ -24,11 +25,10 @@ jobs:
     name: test OTP ${{matrix.otp}} / Elixir ${{matrix.elixir}}
     strategy:
       matrix:
-        otp: ['25']
-        elixir: ['1.13.2']
+        otp: ['26']
+        elixir: ['1.16']
     env:
       MIX_ENV: test
-      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     steps:
       - uses: actions/checkout@v2
       - uses: erlef/setup-beam@v1

README.md

@@ -14,8 +14,7 @@ alias ExSTUN.Message.Type
 # therefore they are not fully symmetric with "encode" benchmarks
 
 fix_lt_password = :crypto.mac(:hmac, :sha, "123456789", "someusername") |> :base64.encode()
-fix_lt_key = "someusername" <> ":" <> "somerealm" <> ":" <> fix_lt_password
-fix_lt_key = :crypto.hash(:md5, fix_lt_key)
+fix_lt_key = Message.lt_key("someusername", fix_lt_password, "somerealm")
 fix_st_key = "somekey"
 
 <<fix_t_id::12*8>> = :crypto.strong_rand_bytes(12)
@@ -77,15 +76,20 @@ Benchee.run(
     "binding_response.decode" => fn -> {:ok, _} = Message.decode(fix_enc_binding_response) end,
     "message_full.encode" => fn -> full_msg_enc.(fix_st_key) end,
     "message_full.decode" => fn -> {:ok, _} = Message.decode(fix_enc_full_message) end,
-    "message_full.authenticate_st" => fn ->
-      {:ok, _} = Message.authenticate_st(fix_full_message, "someusername", fix_st_key)
+    "message_full.authenticate (short-term)" => fn ->
+      # check if username is correct and authenticate
+      {:ok, username} = Message.get_attribute(fix_full_message, Username)
+      username.value == "someusername"
+      :ok = Message.authenticate(fix_full_message, fix_st_key)
     end,
-    "message_full.authenticate_lt" => fn ->
-      password = :crypto.mac(:hmac, :sha, "123456789", "someusername") |> :base64.encode()
-      {:ok, _} = Message.authenticate_lt(fix_full_lt_message, password)
+    "message_full.authenticate (long-term)" => fn ->
+      {:ok, username} = Message.get_attribute(fix_full_lt_message, Username)
+      {:ok, realm} = Message.get_attribute(fix_full_lt_message, Realm)
+      key = Message.lt_key(username.value, fix_lt_password, realm.value)
+      :ok = Message.authenticate(fix_full_lt_message, key)
     end,
     "message_full.check_fingerprint" => fn ->
-      true = Message.check_fingerprint(fix_full_message)
+      :ok = Message.check_fingerprint(fix_full_message)
     end,
     "type.to_value" => fn ->
       Type.to_value(%Type{class: :success_response, method: :binding})

bench/message.exs

@@ -0,0 +1,18 @@
+codecov:
+  require_ci_to_pass: false
+
+comment:
+  layout: "header, diff, files, footer"
+  behavior: default
+
+coverage:
+  status:
+    project:
+      default:
+        informational: true
+    patch:
+      default:
+        informational: true
+
+github_checks:
+  annotations: false

codecov.yml

@@ -6,11 +6,11 @@ defmodule ExSTUN do
   @doc """
   Checks if binary is a STUN message.
   """
-  @spec is_stun(binary()) :: boolean()
-  def is_stun(<<first_byte::8, _rem_header::binary-size(19), _rest::binary>>)
+  @spec stun?(binary()) :: boolean()
+  def stun?(<<first_byte::8, _rem_header::binary-size(19), _rest::binary>>)
       when first_byte in 0..3 do
     true
   end
 
-  def is_stun(_other), do: false
+  def stun?(_other), do: false
 end

lib/ex_stun.ex

@@ -19,8 +19,8 @@ defmodule ExSTUN.Message do
   ```
   """
   import Bitwise
-  alias ExSTUN.Message.Attribute.Fingerprint
-  alias ExSTUN.Message.Attribute.{MessageIntegrity, Realm, Username}
+
+  alias ExSTUN.Message.Attribute.{Fingerprint, MessageIntegrity}
   alias ExSTUN.Message.{RawAttribute, Type}
 
   @magic_cookie 0x2112A442
@@ -221,74 +221,40 @@ defmodule ExSTUN.Message do
   end
 
   @doc """
-  Authenticates a message long-term mechanism.
-
-  `password` depends on the STUN authentication method and has to
-  be provided from the outside.
-
-  `key` is a key used for calculating MAC and can be used
-  for adding message integrity in a response. See `with_integrity/2`.
+  Create longe-term authentication key.
   """
-  @spec authenticate_lt(t(), binary()) ::
-          {:ok, key :: binary()}
-          | {:error,
-             :no_message_integrity
-             | :no_username
-             | :no_realm
-             | :no_matching_message_integrity
-             | atom()}
-  def authenticate_lt(msg, password) do
-    with {:ok, %MessageIntegrity{} = msg_int} <- get_message_integrity(msg),
-         {:ok, %Username{value: username}} <- get_username(msg),
-         {:ok, %Realm{value: realm}} <- get_realm(msg) do
-      key = username <> ":" <> realm <> ":" <> password
-      key = :crypto.hash(:md5, key)
-
-      # + 20 for STUN message header
-      # - 24 for message integrity
-      len = msg.len_to_int + 20 - 24
-      <<msg_without_integrity::binary-size(len), _rest::binary>> = msg.raw
-      <<pre_len::binary-size(2), _len::16, post_len::binary>> = msg_without_integrity
-      msg_without_integrity = <<pre_len::binary, msg.len_to_int::16, post_len::binary>>
-
-      mac = :crypto.mac(:hmac, :sha, key, msg_without_integrity)
-
-      if mac == msg_int.value do
-        {:ok, key}
-      else
-        {:error, :no_matching_message_integrity}
-      end
-    else
-      {:error, _reason} = err -> err
-    end
+  @spec lt_key(binary(), binary(), binary()) :: binary()
+  def lt_key(username, password, realm) do
+    :crypto.hash(:md5, username <> ":" <> realm <> ":" <> password)
   end
 
   @doc """
-  Authenticates a message using short-term mechanism.
+  Authenticates a message.
 
-  It is assumed that username attribute of this message is valid.
+  `key` depends on the authentication method.
+  When authenticating using short-term mechanism, it is simply a password.
+  When authenticating using long-term mechanism, use `lt_key/3` to obtain the key.
 
-  `key` is a key used for calculating MAC and can be used
-  for adding message integrity in a response. See `with_integrity/2`.
+  Presence of username, realm and nonce attributes is not checked.
+  Depending on the authentication method and its context (client/server side),
+  user has to perform those checks on their own.
   """
-  @spec authenticate_st(t(), binary()) ::
-          {:ok, key :: binary()}
-          | {:error, :no_message_integrity | :no_matching_message_integrity | atom()}
-  def authenticate_st(msg, password) do
+  @spec authenticate(t(), binary()) ::
+          :ok | {:error, :no_message_integrity, :no_matching_message_integrity | atom()}
+  def authenticate(msg, key) do
     case get_message_integrity(msg) do
       {:ok, %MessageIntegrity{} = msg_int} ->
         # + 20 for STUN message header
         # - 24 for message integrity
-        len = msg.len_to_int + 20 - (20 + 4)
+        len = msg.len_to_int + 20 - 24
         <<msg_without_integrity::binary-size(len), _rest::binary>> = msg.raw
         <<pre_len::binary-size(2), _len::16, post_len::binary>> = msg_without_integrity
         msg_without_integrity = <<pre_len::binary, msg.len_to_int::16, post_len::binary>>
 
-        # in short-term authentication key == password
-        mac = :crypto.mac(:hmac, :sha, password, msg_without_integrity)
+        mac = :crypto.mac(:hmac, :sha, key, msg_without_integrity)
 
         if mac == msg_int.value do
-          {:ok, password}
+          :ok
         else
           {:error, :no_matching_message_integrity}
         end
@@ -404,18 +370,4 @@ defmodule ExSTUN.Message do
       other -> other
     end
   end
-
-  defp get_username(msg) do
-    case get_attribute(msg, Username) do
-      nil -> {:error, :no_username}
-      other -> other
-    end
-  end
-
-  defp get_realm(msg) do
-    case get_attribute(msg, Realm) do
-      nil -> {:error, :no_realm}
-      other -> other
-    end
-  end
 end

lib/ex_stun/message.ex

@@ -10,7 +10,7 @@ defmodule ExSTUN.MixProject do
       version: @version,
       elixir: "~> 1.13",
       start_permanent: Mix.env() == :prod,
-      description: "Implementation of STUN protocol",
+      description: "Implementation of the STUN protocol",
       package: package(),
       deps: deps(),