Verify reply

README.md

@@ -1,6 +1,10 @@
 elixir-radius
 =============
 
+[![CI](https://github.com/bearice/elixir-radius/actions/workflows/elixir.yml/badge.svg)](https://github.com/bearice/elixir-radius/actions/workflows/elixir.yml)
+
+[![Hex.pm version](https://img.shields.io/hexpm/v/elixir_radius.svg?style=flat)](https://hex.pm/packages/elixir_radius)
+
 RADIUS protocol encoding and decoding
 
 example
@@ -16,8 +20,8 @@ loop = fn(loop)->
 
   IO.puts "From #{inspect host} : \n#{inspect p, pretty: true}"
 
-  resp = %Radius.Packet{code: "Access-Reject", id: p.id, auth: p.auth, secret: p.secret}
-  Radius.send sk,host,resp
+  resp = %Radius.Packet{code: "Access-Reject", id: p.id, secret: p.secret}
+  Radius.send_reply(sk, host, resp, p.auth)
 
   loop.(loop)
 end

example.exs

@@ -38,21 +38,21 @@ attrs = [
   {255, "123456"}
 ]
 
-# for request packets, leave auth=nil will generate with random bytes
-p = %Radius.Packet{code: "Access-Request", id: 12, auth: nil, secret: secret, attrs: attrs}
+# for request packets, authenticator will generate with random bytes
+p = %Radius.Packet{code: "Access-Request", id: 12, secret: secret, attrs: attrs}
 # will return an iolist
-data = Radius.Packet.encode(p)
-Logger.debug("data=#{inspect(data)}")
+%{raw: data} = Radius.Packet.encode_request(p)
+Logger.debug("data=#{inspect(packet.raw)}")
 
 p = Radius.Packet.decode(:erlang.iolist_to_binary(data), secret)
 Logger.debug(inspect(p, pretty: true))
 
-# for response packets, set auth=request.auth to generate new HMAC-hash with it.
-p = %Radius.Packet{code: "Access-Accept", id: 12, auth: p.auth, secret: secret, attrs: p.attrs}
-data = Radius.Packet.encode(p)
+# for response packets, provide request.auth to generate new HMAC-hash with it.
+p2 = %Radius.Packet{code: "Access-Accept", id: 12, secret: secret, attrs: p.attrs}
+%{raw: data} = Radius.Packet.encode_reply(p2, p.auth)
 Logger.debug("data=#{inspect(data)}")
 # password decoding SHOULD FAIL here, guess why?
-p = Radius.Packet.decode(:erlang.iolist_to_binary(data), p.secret)
+p = Radius.Packet.decode(:erlang.iolist_to_binary(data), p2.secret)
 Logger.debug(inspect(p, pretty: true))
 
 # wrapper of gen_udp
@@ -65,8 +65,8 @@ loop = fn loop ->
 
   IO.puts("From #{inspect(host)} : \n#{inspect(p, pretty: true)}")
 
-  resp = %Radius.Packet{code: "Access-Reject", id: p.id, auth: p.auth, secret: p.secret}
-  Radius.send(sk, host, resp)
+  resp = %Radius.Packet{code: "Access-Reject", id: p.id, secret: p.secret}
+  Radius.send_reply(sk, host, resp, p.auth)
 
   loop.(loop)
 end

lib/radius.ex

@@ -31,8 +31,35 @@ defmodule Radius do
         sk :: socket
         packet:: %Radius.Packet{}
   """
+  @deprecated "Use send_reply/4 or send_request/3"
   def send(sk, {host, port}, packet) do
-    data = Packet.encode(packet)
+    send_reply(sk, {host, port}, packet, packet.auth)
+  end
+
+  @doc """
+  encode and send reply packet
+  """
+  @spec send_reply(
+          socket :: port(),
+          {host :: :inet.ip_address(), port :: :inet.port_number()},
+          packet :: Packet.t(),
+          request_authenticator :: binary()
+        ) :: :ok | {:error, any()}
+  def send_reply(sk, {host, port}, packet, request_authenticator) do
+    %{raw: data} = Packet.encode_reply(packet, request_authenticator)
+    :gen_udp.send(sk, host, port, data)
+  end
+
+  @doc """
+  encode and send request packet
+  """
+  @spec send_request(
+          socket :: port(),
+          {host :: :inet.ip_address(), port :: :inet.port_number()},
+          packet :: Packet.t()
+        ) :: :ok | {:error, any()}
+  def send_request(sk, {host, port}, packet) do
+    %{raw: data} = Packet.encode_request(packet)
     :gen_udp.send(sk, host, port, data)
   end
 end

lib/radius/dict.ex

@@ -215,7 +215,7 @@ defmodule Radius.Dict do
       |> String.to_charlist()
       |> tokenlize!
       |> parse!
-      |> (&process_dict(ctx, &1)).()
+      |> then(&process_dict(ctx, &1))
     rescue
       e in ParserError -> reraise %{e | file: path}, __STACKTRACE__
       e -> reraise e, __STACKTRACE__

lib/radius/packet.ex

@@ -1,11 +1,21 @@
 defmodule Radius.Packet do
   require Logger
 
+  alias __MODULE__
   alias Radius.Dict.Attribute
   alias Radius.Dict.Vendor
   alias Radius.Dict.Value
   alias Radius.Dict.EntryNotFoundError
 
+  @type t :: %{
+          code: String.t(),
+          id: integer(),
+          length: integer(),
+          auth: binary(),
+          attrs: keyword(),
+          raw: iolist(),
+          secret: binary()
+        }
   defstruct code: nil,
             id: nil,
             length: nil,
@@ -196,26 +206,56 @@ defmodule Radius.Packet do
         ipaddr :: {a,b,c,d} | {a,b,c,d,e,f,g,h}
 
   """
+  @deprecated "Use encode_request/1-2 or encode_reply/1-2 instead"
   def encode(packet, options \\ []) do
-    sign? = options |> Keyword.get(:sign, false)
-    raw? = options |> Keyword.get(:raw, false)
-
-    {auth, reply?} =
+    packet =
       if packet.auth == nil do
-        {:crypto.strong_rand_bytes(16), false}
+        encode_request(packet, options)
       else
-        {packet.auth, true}
+        encode_reply(packet, packet.auth, options)
       end
 
-    packet = %{packet | auth: auth}
+    packet.raw
+  end
+
+  @doc """
+  Encode the request packet into an iolist and put the result in the `:raw` key. The `:auth` key will contain
+  the authenticator used on the request.
+  """
+  @spec encode_request(packet :: Packet.t(), options :: keyword()) :: Packet.t()
+  def encode_request(packet, options \\ []) do
+    packet = %{packet | auth: :crypto.strong_rand_bytes(16)}
+    {header, attrs} = encode_packet(packet, options)
+
+    %{packet | raw: [header, attrs]}
+  end
+
+  @doc """
+  Encode the reply packet into an iolist and put the result in the `:raw` key. The `:auth` key needs
+  to be filled with the authenticator of the request packet.
+  """
+  @spec encode_reply(
+          packet :: Packet.t(),
+          request_authenticator :: binary(),
+          options :: keyword()
+        ) :: Packet.t()
+  def encode_reply(packet, request_authenticator, options \\ []) do
+    packet = %{packet | auth: request_authenticator}
+    {header, attrs} = encode_packet(packet, options)
+
+    resp_auth = :crypto.hash(:md5, [header, attrs, packet.secret])
+
+    header = <<header::bytes-size(4), resp_auth::binary>>
+
+    %{packet | auth: resp_auth, raw: [header, attrs]}
+  end
+
+  defp encode_packet(packet, options) do
+    sign? = options |> Keyword.get(:sign, false)
 
     packet =
       if sign? do
-        attrs =
-          packet.attrs ++
-            [
-              {"Message-Authenticator", <<0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0>>}
-            ]
+        attrs = [{"Message-Authenticator", <<0::size(128)>>} | packet.attrs]
 
         %{packet | attrs: attrs}
       else
@@ -225,35 +265,23 @@ defmodule Radius.Packet do
     attrs = encode_attrs(packet)
 
     code = encode_code(packet.code)
-    length = 20 + :erlang.iolist_size(attrs)
-    header = <<code, packet.id, length::size(16), auth::binary>>
+    length = 20 + IO.iodata_length(attrs)
+    header = <<code, packet.id, length::size(16), packet.auth::binary>>
 
     attrs =
       if sign? do
-        signature = :crypto.mac(:hmac, :md5, packet.secret, [header, attrs])
-        [last | attrs] = attrs |> Enum.reverse()
-        crop_len = byte_size(last) - 16
-        last = <<last::bytes-size(crop_len), signature::binary>>
-        [last | attrs] |> Enum.reverse()
+        signature = message_authenticator(packet.secret, [header, attrs])
+        [<<t, l, _::binary>> | rest_attrs] = attrs
+        [<<t, l, signature::binary>> | rest_attrs]
       else
         attrs
       end
 
-    header =
-      if reply? and raw? == false do
-        resp_auth =
-          :crypto.hash_init(:md5)
-          |> :crypto.hash_update(header)
-          |> :crypto.hash_update(attrs)
-          |> :crypto.hash_update(packet.secret)
-          |> :crypto.hash_final()
-
-        <<header::bytes-size(4), resp_auth::binary>>
-      else
-        header
-      end
+    {header, attrs}
+  end
 
-    [header, attrs]
+  defp message_authenticator(secret, msg) do
+    :crypto.mac(:hmac, :md5, secret, msg)
   end
 
   defp encode_attrs(%{attrs: a} = ctx) do
@@ -422,47 +450,45 @@ defmodule Radius.Packet do
   @doc """
   Return the value of a given attribute, if found, or default otherwise.
   """
-  def get_attr(packet, attr_name, default \\ nil) do
-    result =
-      packet.attrs
-      |> Enum.find(default, fn
-        {^attr_name, _} -> true
-        _ -> false
-      end)
-
-    case result do
-      {_, value} -> value
-      _ -> nil
-    end
+  def get_attr(packet, attr_name) do
+    for {^attr_name, value} <- packet.attrs, do: value
   end
 
   @doc """
   Verify if the packet signature is valid.
+
+  (https://www.ietf.org/rfc/rfc2865.txt)
+  (https://www.ietf.org/rfc/rfc2869.txt)
   """
   def verify(packet) do
-    # TODO: this code is going to fail when validating replies
-    sig1 =
-      packet
-      |> Radius.Packet.get_attr("Message-Authenticator")
+    verify(packet, packet.auth)
+  end
 
-    if sig1 != nil do
-      attrs =
-        packet.attrs
-        |> Enum.filter(fn {k, _} -> k != "Message-Authenticator" end)
+  def verify(packet, request_authenticator) do
+    case Radius.Packet.get_attr(packet, "Message-Authenticator") do
+      [sig1] ->
+        {header, attrs} = encode_packet(%{packet | auth: request_authenticator}, [])
+        resp_auth = :crypto.hash(:md5, [header, attrs, packet.secret])
 
-      raw =
-        %{packet | attrs: attrs}
-        |> Radius.Packet.encode(raw: true, sign: true)
-        |> IO.iodata_to_binary()
+        attrs =
+          Enum.map(packet.attrs, fn
+            {"Message-Authenticator", _} -> {"Message-Authenticator", <<0::size(128)>>}
+            attr -> attr
+          end)
 
-      crop_len = byte_size(raw) - 16
-      <<_::bytes-size(crop_len), sig2::binary>> = raw
+        packet = %{packet | attrs: attrs}
+        {header, attrs} = encode_packet(packet, [])
+        <<code, id, length::size(16), _resp_auth::binary>> = header
+        sign_header = <<code, id, length::size(16), request_authenticator::binary>>
+        sig2 = message_authenticator(packet.secret, [sign_header, attrs])
 
-      sig1 == sig2
-    else
-      false
+        (packet.auth == request_authenticator or packet.auth == resp_auth) and sig1 == sig2
+
+      _ ->
+        {header, attrs} = encode_packet(%{packet | auth: request_authenticator}, [])
+        resp_auth = :crypto.hash(:md5, [header, attrs, packet.secret])
+
+        packet.auth == request_authenticator or packet.auth == resp_auth
     end
   end
 end
-
-# defmodule Packet

lib/radius/util.ex

@@ -34,38 +34,31 @@ defmodule Radius.Util do
   defp hash_xor(input, hash, secret, acc, opts \\ [])
 
   defp hash_xor(<<>>, _, _, acc, _) do
-    acc |> Enum.reverse() |> :erlang.iolist_to_binary()
+    acc |> Enum.reverse() |> IO.iodata_to_binary()
   end
 
-  defp hash_xor(<<block::binary-size(16), rest::binary>>, hash, secret, acc, opts) do
+  @block_binary_size 16
+  @block_size @block_binary_size * 8
+  defp hash_xor(<<block::binary-size(@block_binary_size), rest::binary>>, hash, secret, acc, opts) do
     hash = :crypto.hash(:md5, secret <> hash)
     xor_block = binary_xor(block, hash)
     next = if(opts |> Keyword.get(:reverse, false), do: block, else: xor_block)
     hash_xor(rest, next, secret, [xor_block | acc])
   end
 
-  defp binary_xor(x, y) when byte_size(x) == byte_size(y) do
-    s = byte_size(x) * 8
-    <<x::size(s)>> = x
-    <<y::size(s)>> = y
+  defp binary_xor(<<x::size(@block_size)>>, <<y::size(@block_size)>>) do
     z = bxor(x, y)
-    <<z::size(s)>>
+    <<z::size(@block_size)>>
   end
 
   defp pad_to_16(bin) do
-    pad_to_16(bin, [])
-    |> Enum.reverse()
-    |> :erlang.iolist_to_binary()
-  end
-
-  defp pad_to_16(bin, acc) when byte_size(bin) == 16, do: [bin | acc]
+    remainder = Integer.mod(byte_size(bin), 16)
 
-  defp pad_to_16(bin, acc) when byte_size(bin) < 16 do
-    bin = <<bin::binary, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0>>
-    <<chunk::binary-size(16), _::binary>> = bin
-    [chunk | acc]
+    if remainder == 0 do
+      bin
+    else
+      padding = 16 - remainder
+      <<bin::binary, 0::size(padding * 8)>>
+    end
   end
-
-  defp pad_to_16(<<chunk::binary-size(16), rest::binary>>, acc),
-    do: pad_to_16(rest, [chunk | acc])
 end