Add more tests

.formatter.exs

@@ -2,5 +2,5 @@
   import_deps: [:phoenix],
   plugins: [Phoenix.LiveView.HTMLFormatter],
   inputs: ["*.{heex,ex,exs}", "{config,lib,test}/**/*.{heex,ex,exs}"],
-  line_length: 120
+  line_length: 128
 ]

lib/chronoscope/nts/key_establishment.ex

@@ -36,7 +36,7 @@ defmodule Chronoscope.NTS.KeyEstablishment do
   end
 
   # End of Message
-  defp do_parse_response([0x80, 0x00, 0x00, 0x00], acc) do
+  defp do_parse_response([0x80, 0x00, 0x00, 0x00 | _rest], acc) do
     acc
   end
 
@@ -57,7 +57,7 @@ defmodule Chronoscope.NTS.KeyEstablishment do
 
     do_parse_response(
       rest,
-      Map.put(acc, :error, Map.get(@errors, error, error))
+      Map.put(acc, :error, Map.get(@errors, error, to_string(error)))
     )
   end
 
@@ -65,7 +65,7 @@ defmodule Chronoscope.NTS.KeyEstablishment do
   defp do_parse_response([0x80, 0x03, 0x00, 0x02, warning_high, warning_low | rest], acc) do
     warning = combine_octets(warning_high, warning_low)
 
-    do_parse_response(rest, Map.put(acc, :warning, warning))
+    do_parse_response(rest, Map.put(acc, :warning, to_string(warning)))
   end
 
   # AEAD Algorithm Negotiation
@@ -81,8 +81,7 @@ defmodule Chronoscope.NTS.KeyEstablishment do
   end
 
   # New Cookie for NTPv4
-  defp do_parse_response([first, 0x05, length_high, length_low | rest], acc)
+  defp do_parse_response([first, 0x05, length_high, length_low | rest], acc) when first == 0x00 or first == 0x80 do
-       when first == 0x00 or first == 0x80 do
     length = combine_octets(length_high, length_low)
     {cookie, remaining} = Enum.split(rest, length)
 
@@ -90,13 +89,12 @@ defmodule Chronoscope.NTS.KeyEstablishment do
       remaining,
       acc
       |> Map.update(:cookies, [cookie], &[cookie | &1])
-      |> Map.put(:cookie_length, length)
+      |> Map.update(:cookie_length, length, &max(&1, length))
     )
   end
 
   # NTPv4 Server Negotiation
-  defp do_parse_response([first, 0x06, length_high, length_low | rest], acc)
+  defp do_parse_response([first, 0x06, length_high, length_low | rest], acc) when first == 0x00 or first == 0x80 do
-       when first == 0x00 or first == 0x80 do
     length = combine_octets(length_high, length_low)
     {server, remaining} = Enum.split(rest, length)
 
@@ -104,8 +102,7 @@ defmodule Chronoscope.NTS.KeyEstablishment do
   end
 
   # NTPv4 Port Negotiation
-  defp do_parse_response([first, 0x07, 0x00, 0x02, port_high, port_low | rest], acc)
+  defp do_parse_response([first, 0x07, 0x00, 0x02, port_high, port_low | rest], acc) when first == 0x00 or first == 0x80 do
-       when first == 0x00 or first == 0x80 do
     port = combine_octets(port_high, port_low)
 
     do_parse_response(rest, Map.put(acc, :port, port))

test/chronoscope/nts/key_establishment_test.exs

@@ -5,8 +5,7 @@ defmodule Chronoscope.NTS.KeyEstablishmentTest do
 
   test "builds request" do
     assert KeyEstablishment.request() ==
-             <<0x80, 0x01, 0x00, 0x02, 0x00, 0x00, 0x80, 0x04, 0x00, 0x04, 0x00, 0x1E, 0x00, 0x0F, 0x80, 0x00, 0x00,
+             <<0x80, 0x01, 0x00, 0x02, 0x00, 0x00, 0x80, 0x04, 0x00, 0x04, 0x00, 0x1E, 0x00, 0x0F, 0x80, 0x00, 0x00, 0x00>>
-               0x00>>
   end
 
   test "handles empty response" do
@@ -17,12 +16,12 @@ defmodule Chronoscope.NTS.KeyEstablishmentTest do
     assert KeyEstablishment.parse_response([0x80, 0x00, 0x00, 0x00]) == {:ok, %{cookies: 0}}
   end
 
-  test "handles next protocol negotiation response" do
+  test "handles next protocol negotiation record" do
     assert KeyEstablishment.parse_response([0x80, 0x01, 0x00, 0x02, 0x00, 0x00]) ==
              {:ok, %{cookies: 0, next_protocols: ["NTPv4"]}}
   end
 
-  test "does not handle next protocol negotiation response without critical bit" do
+  test "does not handle next protocol negotiation record without critical bit" do
     assert KeyEstablishment.parse_response([0x00, 0x01, 0x00, 0x02, 0x00, 0x00]) == {:ok, %{cookies: 0}}
   end
 
@@ -35,12 +34,12 @@ defmodule Chronoscope.NTS.KeyEstablishmentTest do
              {:ok, %{cookies: 0, next_protocols: ["NTPv4", "UNASSIGNED", "NTPv4"]}}
   end
 
-  test "handles aead algorithm negotiation response" do
+  test "handles aead algorithm negotiation record" do
     assert KeyEstablishment.parse_response([0x80, 0x04, 0x00, 0x02, 0x00, 0x0F]) ==
              {:ok, %{cookies: 0, aead_algorithms: ["AEAD_AES_SIV_CMAC_256"]}}
   end
 
-  test "handles aead algorithm negotiation response without critical bit" do
+  test "handles aead algorithm negotiation record without critical bit" do
     assert KeyEstablishment.parse_response([0x00, 0x04, 0x00, 0x02, 0x00, 0x1E]) ==
              {:ok, %{cookies: 0, aead_algorithms: ["AEAD_AES_128_GCM_SIV"]}}
   end
@@ -53,4 +52,98 @@ defmodule Chronoscope.NTS.KeyEstablishmentTest do
     assert KeyEstablishment.parse_response([0x80, 0x04, 0x00, 0x06, 0x00, 0x1E, 0x00, 0x01, 0x00, 0x0F]) ==
              {:ok, %{cookies: 0, aead_algorithms: ["AEAD_AES_SIV_CMAC_256", "UNKNOWN", "AEAD_AES_128_GCM_SIV"]}}
   end
+
+  test "handles error record" do
+    assert KeyEstablishment.parse_response([0x80, 0x02, 0x00, 0x02, 0x00, 0x01]) == {:ok, %{cookies: 0, error: "Bad Request"}}
+  end
+
+  test "handles unknown error record" do
+    assert KeyEstablishment.parse_response([0x80, 0x02, 0x00, 0x02, 0x00, 0x99]) == {:ok, %{cookies: 0, error: "153"}}
+  end
+
+  test "does not handle error record without critical bit" do
+    assert KeyEstablishment.parse_response([0x00, 0x02, 0x00, 0x02, 0x00, 0x01]) == {:ok, %{cookies: 0}}
+  end
+
+  test "handles warning record" do
+    assert KeyEstablishment.parse_response([0x80, 0x03, 0x00, 0x02, 0x00, 0x01]) == {:ok, %{cookies: 0, warning: "1"}}
+  end
+
+  test "does not handle warning record without critical bit" do
+    assert KeyEstablishment.parse_response([0x00, 0x03, 0x00, 0x02, 0x00, 0x01]) == {:ok, %{cookies: 0}}
+  end
+
+  test "handles server record" do
+    assert KeyEstablishment.parse_response([0x80, 0x06, 0x00, 0x09, ?1, ?2, ?7, ?., ?0, ?., ?0, ?., ?1]) ==
+             {:ok, %{cookies: 0, server: "127.0.0.1"}}
+  end
+
+  test "handles server record without critical bit" do
+    assert KeyEstablishment.parse_response([0x00, 0x06, 0x00, 0x09, ?1, ?2, ?7, ?., ?0, ?., ?0, ?., ?1]) ==
+             {:ok, %{cookies: 0, server: "127.0.0.1"}}
+  end
+
+  test "handles port record" do
+    assert KeyEstablishment.parse_response([0x80, 0x07, 0x00, 0x02, 0x04, 0xCE]) == {:ok, %{cookies: 0, port: 1230}}
+  end
+
+  test "handles port record without critical bit" do
+    assert KeyEstablishment.parse_response([0x00, 0x07, 0x00, 0x02, 0x04, 0xCE]) == {:ok, %{cookies: 0, port: 1230}}
+  end
+
+  test "handles cookie record" do
+    assert KeyEstablishment.parse_response([0x80, 0x05, 0x00, 0x0E, ?c, ?h, ?o, ?c, ?o, ?l, ?a, ?t, ?e, ?_, ?c, ?h, ?i, ?p]) ==
+             {:ok, %{cookies: 1, cookie_length: 14}}
+  end
+
+  test "handles cookie record without critical bit" do
+    assert KeyEstablishment.parse_response([0x00, 0x05, 0x00, 0x0E, ?c, ?h, ?o, ?c, ?o, ?l, ?a, ?t, ?e, ?_, ?c, ?h, ?i, ?p]) ==
+             {:ok, %{cookies: 1, cookie_length: 14}}
+  end
+
+  test "sets cookie length to longest cookie" do
+    assert KeyEstablishment.parse_response(
+             [0x80, 0x05, 0x00, 0x01, ?c] ++
+               [0x80, 0x05, 0x00, 0x03, ?c, ?c, ?c] ++
+               [0x80, 0x05, 0x00, 0x01, ?c]
+           ) ==
+             {:ok, %{cookies: 3, cookie_length: 3}}
+  end
+
+  test "handles full response" do
+    assert KeyEstablishment.parse_response(
+             [0x80, 0x01, 0x00, 0x02, 0x00, 0x00] ++
+               [0x80, 0x04, 0x00, 0x06, 0x00, 0x1E, 0x00, 0x01, 0x00, 0x0F] ++
+               [0x80, 0x05, 0x00, 0x01, ?c] ++
+               [0x80, 0x06, 0x00, 0x09, ?1, ?2, ?7, ?., ?0, ?., ?0, ?., ?1] ++
+               [0x80, 0x07, 0x00, 0x02, 0x04, 0xCE] ++
+               [0x80, 0x00, 0x00, 0x00]
+           ) ==
+             {:ok,
+              %{
+                cookies: 1,
+                aead_algorithms: ["AEAD_AES_SIV_CMAC_256", "UNKNOWN", "AEAD_AES_128_GCM_SIV"],
+                cookie_length: 1,
+                next_protocols: ["NTPv4"],
+                port: 1230,
+                server: "127.0.0.1"
+              }}
+  end
+
+  test "doesn't read past end of message record" do
+    assert KeyEstablishment.parse_response(
+             [0x80, 0x01, 0x00, 0x02, 0x00, 0x00] ++
+               [0x80, 0x04, 0x00, 0x02, 0x00, 0x1E] ++
+               [0x80, 0x00, 0x00, 0x00] ++
+               [0x80, 0x05, 0x00, 0x01, ?c] ++
+               [0x80, 0x06, 0x00, 0x09, ?1, ?2, ?7, ?., ?0, ?., ?0, ?., ?1] ++
+               [0x80, 0x07, 0x00, 0x02, 0x04, 0xCE]
+           ) ==
+             {:ok,
+              %{
+                cookies: 0,
+                aead_algorithms: ["AEAD_AES_128_GCM_SIV"],
+                next_protocols: ["NTPv4"]
+              }}
+  end
 end