fix verifying message authenticator on replies

tverlaan · bearice · commit 42718300c20f · 2023-05-10T17:22:48.000+08:00
diff --git a/example.exs b/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
diff --git a/lib/radius.ex b/lib/radius.ex
@@ -32,7 +32,7 @@ defmodule Radius do
         packet:: %Radius.Packet{}
   """
   def send(sk, {host, port}, packet) do
-    data = Packet.encode(packet)
+    %{raw: data} = Packet.encode_reply(packet, packet.auth)
     :gen_udp.send(sk, host, port, data)
   end
 end
diff --git a/lib/radius/dict.ex b/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__
diff --git a/lib/radius/packet.ex b/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,6 +206,7 @@ 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)
@@ -211,11 +222,7 @@ defmodule Radius.Packet do
 
     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 = packet.attrs ++ [{"Message-Authenticator", <<0::size(128)>>}]
 
         %{packet | attrs: attrs}
       else
@@ -256,6 +263,77 @@ defmodule Radius.Packet do
     [header, attrs]
   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_init(:md5)
+      |> :crypto.hash_update(header)
+      |> :crypto.hash_update(attrs)
+      |> :crypto.hash_update(packet.secret)
+      |> :crypto.hash_final()
+
+    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 = [{"Message-Authenticator", <<0::size(128)>>} | packet.attrs]
+
+        %{packet | attrs: attrs}
+      else
+        packet
+      end
+
+    attrs = encode_attrs(packet)
+
+    code = encode_code(packet.code)
+    length = 20 + IO.iodata_length(attrs)
+    header = <<code, packet.id, length::size(16), packet.auth::binary>>
+
+    attrs =
+      if sign? do
+        signature = message_authenticator(packet.secret, [header, attrs])
+        [<<t, l, _::binary>> | rest_attrs] = attrs
+        [<<t, l, signature::binary>> | rest_attrs]
+      else
+        attrs
+      end
+
+    {header, attrs}
+  end
+
+  defp message_authenticator(secret, msg) do
+    :crypto.mac(:hmac, :md5, secret, msg)
+  end
+
   defp encode_attrs(%{attrs: a} = ctx) do
     Enum.map(a, fn x ->
       x |> resolve_attr(ctx) |> encode_attr
@@ -422,47 +500,38 @@ 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/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] ->
+        attrs =
+          Enum.map(packet.attrs, fn
+            {"Message-Authenticator", _} -> {"Message-Authenticator", <<0::size(128)>>}
+            attr -> attr
+          end)
 
-      raw =
-        %{packet | attrs: attrs}
-        |> Radius.Packet.encode(raw: true, sign: true)
-        |> IO.iodata_to_binary()
+        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>>
 
-      crop_len = byte_size(raw) - 16
-      <<_::bytes-size(crop_len), sig2::binary>> = raw
+        sig2 = message_authenticator(packet.secret, [sign_header, attrs])
+        sig1 == sig2
 
-      sig1 == sig2
-    else
-      false
+      _ ->
+        false
     end
   end
 end
-
-# defmodule Packet
diff --git a/test/radius_packet_test.exs b/test/radius_packet_test.exs
@@ -0,0 +1,131 @@
+defmodule Radius.PacketTest do
+  use ExUnit.Case, async: true
+
+  @secret "mykey"
+  @sample_req %Radius.Packet{
+    code: "Access-Request",
+    id: 118,
+    length: 173,
+    auth: <<55, 91, 232, 245, 150, 233, 11, 207, 252, 94, 50, 146, 157, 20, 39, 91>>,
+    attrs: [
+      {"NAS-IP-Address", {10, 62, 1, 238}},
+      {"NAS-Port", 50001},
+      {"NAS-Port-Type", "Ethernet"},
+      {"User-Name", "host/drswin7tracyp.drsl.co.uk"},
+      {"Called-Station-Id", "00-12-00-E3-41-C1"},
+      {"Calling-Station-Id", "B4-99-BA-F2-8A-D6"},
+      {"Service-Type", "Framed-User"},
+      {"Framed-MTU", 1500},
+      {"EAP-Message",
+       <<2, 0, 0, 34, 1, 104, 111, 115, 116, 47, 100, 114, 115, 119, 105, 110, 55, 116, 114, 97,
+         99, 121, 112, 46, 100, 114, 115, 108, 46, 99, 111, 46, 117, 107>>},
+      {"Message-Authenticator",
+       <<201, 62, 246, 40, 105, 10, 87, 139, 49, 112, 155, 11, 188, 202, 222, 65>>}
+    ],
+    raw: nil,
+    secret: "mykey"
+  }
+  @sample_binary_req <<1, 118, 0, 173, 55, 91, 232, 245, 150, 233, 11, 207, 252, 94, 50, 146, 157,
+                       20, 39, 91, 4, 6, 10, 62, 1, 238, 5, 6, 0, 0, 195, 81, 61, 6, 0, 0, 0, 15,
+                       1, 31, 104, 111, 115, 116, 47, 100, 114, 115, 119, 105, 110, 55, 116, 114,
+                       97, 99, 121, 112, 46, 100, 114, 115, 108, 46, 99, 111, 46, 117, 107, 30,
+                       19, 48, 48, 45, 49, 50, 45, 48, 48, 45, 69, 51, 45, 52, 49, 45, 67, 49, 31,
+                       19, 66, 52, 45, 57, 57, 45, 66, 65, 45, 70, 50, 45, 56, 65, 45, 68, 54, 6,
+                       6, 0, 0, 0, 2, 12, 6, 0, 0, 5, 220, 79, 36, 2, 0, 0, 34, 1, 104, 111, 115,
+                       116, 47, 100, 114, 115, 119, 105, 110, 55, 116, 114, 97, 99, 121, 112, 46,
+                       100, 114, 115, 108, 46, 99, 111, 46, 117, 107, 80, 18, 201, 62, 246, 40,
+                       105, 10, 87, 139, 49, 112, 155, 11, 188, 202, 222, 65>>
+  @sample_rep %Radius.Packet{
+    code: "Access-Accept",
+    id: 118,
+    length: 173,
+    auth: nil,
+    attrs: [
+      {"NAS-IP-Address", {10, 62, 1, 238}},
+      {"NAS-Port", 50001},
+      {"NAS-Port-Type", "Ethernet"},
+      {"User-Name", "host/drswin7tracyp.drsl.co.uk"},
+      {"Called-Station-Id", "00-12-00-E3-41-C1"},
+      {"Calling-Station-Id", "B4-99-BA-F2-8A-D6"},
+      {"Service-Type", "Framed-User"},
+      {"Framed-MTU", 1500},
+      {"EAP-Message",
+       <<2, 0, 0, 34, 1, 104, 111, 115, 116, 47, 100, 114, 115, 119, 105, 110, 55, 116, 114, 97,
+         99, 121, 112, 46, 100, 114, 115, 108, 46, 99, 111, 46, 117, 107>>}
+    ],
+    raw: nil,
+    secret: "mykey"
+  }
+  @sample_binary_rep <<2, 118, 0, 155, 25, 149, 189, 198, 178, 14, 197, 28, 131, 240, 157, 146,
+                       150, 38, 53, 105, 4, 6, 10, 62, 1, 238, 5, 6, 0, 0, 195, 81, 61, 6, 0, 0,
+                       0, 15, 1, 31, 104, 111, 115, 116, 47, 100, 114, 115, 119, 105, 110, 55,
+                       116, 114, 97, 99, 121, 112, 46, 100, 114, 115, 108, 46, 99, 111, 46, 117,
+                       107, 30, 19, 48, 48, 45, 49, 50, 45, 48, 48, 45, 69, 51, 45, 52, 49, 45,
+                       67, 49, 31, 19, 66, 52, 45, 57, 57, 45, 66, 65, 45, 70, 50, 45, 56, 65, 45,
+                       68, 54, 6, 6, 0, 0, 0, 2, 12, 6, 0, 0, 5, 220, 79, 36, 2, 0, 0, 34, 1, 104,
+                       111, 115, 116, 47, 100, 114, 115, 119, 105, 110, 55, 116, 114, 97, 99, 121,
+                       112, 46, 100, 114, 115, 108, 46, 99, 111, 46, 117, 107>>
+  @sample_binary_rep_signed <<2, 118, 0, 173, 132, 213, 98, 44, 33, 151, 126, 7, 160, 110, 91, 18,
+                              56, 125, 67, 245, 80, 18, 27, 203, 27, 162, 52, 156, 30, 25, 241,
+                              43, 80, 77, 28, 109, 228, 77, 4, 6, 10, 62, 1, 238, 5, 6, 0, 0, 195,
+                              81, 61, 6, 0, 0, 0, 15, 1, 31, 104, 111, 115, 116, 47, 100, 114,
+                              115, 119, 105, 110, 55, 116, 114, 97, 99, 121, 112, 46, 100, 114,
+                              115, 108, 46, 99, 111, 46, 117, 107, 30, 19, 48, 48, 45, 49, 50, 45,
+                              48, 48, 45, 69, 51, 45, 52, 49, 45, 67, 49, 31, 19, 66, 52, 45, 57,
+                              57, 45, 66, 65, 45, 70, 50, 45, 56, 65, 45, 68, 54, 6, 6, 0, 0, 0,
+                              2, 12, 6, 0, 0, 5, 220, 79, 36, 2, 0, 0, 34, 1, 104, 111, 115, 116,
+                              47, 100, 114, 115, 119, 105, 110, 55, 116, 114, 97, 99, 121, 112,
+                              46, 100, 114, 115, 108, 46, 99, 111, 46, 117, 107>>
+
+  test "decode request" do
+    packet = Radius.Packet.decode(@sample_binary_req, @secret)
+    assert packet.code == @sample_req.code
+    assert packet.id == @sample_req.id
+    assert packet.length == @sample_req.length
+
+    assert packet.auth == @sample_req.auth
+  end
+
+  test "encode request" do
+    # cut authenticator as it will be generated on each encoding
+    <<before::size(32), _random::size(128), rest::binary>> =
+      @sample_req |> Radius.Packet.encode_request() |> Map.get(:raw) |> IO.iodata_to_binary()
+
+    <<sample_before::size(32), _random::size(128), sample_rest::binary>> = @sample_binary_req
+    assert <<before::size(32), rest::binary>> == <<sample_before::size(32), sample_rest::binary>>
+  end
+
+  test "encode reply" do
+    reply =
+      @sample_rep
+      |> Radius.Packet.encode_reply(@sample_req.auth)
+      |> Map.get(:raw)
+      |> IO.iodata_to_binary()
+
+    assert reply == @sample_binary_rep
+  end
+
+  test "encode and sign reply" do
+    reply =
+      @sample_rep
+      |> Radius.Packet.encode_reply(@sample_req.auth, sign: true)
+      |> Map.get(:raw)
+      |> IO.iodata_to_binary()
+
+    assert reply == @sample_binary_rep_signed
+  end
+
+  test "verify message authenticator signature on request" do
+    assert Radius.Packet.verify(@sample_req)
+    refute Radius.Packet.verify(%{@sample_req | id: 14})
+    refute Radius.Packet.verify(%{@sample_req | attrs: []})
+  end
+
+  test "verify message authenticator signature on reply" do
+    packet = Radius.Packet.decode(@sample_binary_rep_signed, @secret)
+    assert Radius.Packet.verify(packet, @sample_req.auth)
+    refute Radius.Packet.verify(packet)
+    refute Radius.Packet.verify(%{packet | id: 14}, @sample_req.auth)
+    refute Radius.Packet.verify(%{packet | attrs: []}, @sample_req.auth)
+  end
+end
diff --git a/test/radius_util_test.exs b/test/radius_util_test.exs
