From e6b3c4752d623b028a36a5796e269de9c3d3b14a Mon Sep 17 00:00:00 2001 From: Pim Kunis Date: Tue, 10 Aug 2021 18:02:53 +0200 Subject: [PATCH] Fix server signature verification --- lib/matrix_server.ex | 10 ++ lib/matrix_server/signing_server.ex | 41 +++---- lib/matrix_server_web/authenticate_server.ex | 102 +++++++----------- .../federation/query_controller.ex | 55 ++++++++++ .../federation/test_controller.ex | 10 -- lib/matrix_server_web/federation_client.ex | 25 ++--- lib/matrix_server_web/plug/error.ex | 3 +- lib/matrix_server_web/router.ex | 2 +- 8 files changed, 138 insertions(+), 110 deletions(-) create mode 100644 lib/matrix_server_web/federation/query_controller.ex delete mode 100644 lib/matrix_server_web/federation/test_controller.ex diff --git a/lib/matrix_server.ex b/lib/matrix_server.ex index f389b35..a497a71 100644 --- a/lib/matrix_server.ex +++ b/lib/matrix_server.ex @@ -41,6 +41,16 @@ defmodule MatrixServer do end end + # TODO Eventually move to regex with named captures. + def get_localpart(id) do + with [part, _] <- String.split(id, ":", parts: 2), + {_, localpart} <- String.split_at(part, 1) do + localpart + else + _ -> nil + end + end + # https://elixirforum.com/t/22709/9 def has_duplicates?(list) do list diff --git a/lib/matrix_server/signing_server.ex b/lib/matrix_server/signing_server.ex index ea166a7..44b07d5 100644 --- a/lib/matrix_server/signing_server.ex +++ b/lib/matrix_server/signing_server.ex @@ -26,32 +26,11 @@ defmodule MatrixServer.SigningServer do {:ok, %{public_key: public_key, private_key: private_key}} end - # https://blog.swwomm.com/2020/09/elixir-ed25519-signatures-with-enacl.html @impl true - def handle_call( - {:sign_object, object}, - _from, - %{private_key: private_key} = state - ) do - case MatrixServer.encode_canonical_json(object) do - {:ok, json} -> - signature = - json - |> :enacl.sign_detached(private_key) - |> MatrixServer.encode_unpadded_base64() - - signature_map = %{@signing_key_id => signature} - servername = MatrixServer.server_name() - - signed_object = - Map.update(object, :signatures, %{servername => signature_map}, fn signatures -> - Map.put(signatures, servername, signature_map) - end) - - {:reply, {:ok, signed_object}, state} - - {:error, _msg} -> - {:reply, {:error, :json_encode}, state} + def handle_call({:sign_object, object}, _from, %{private_key: private_key} = state) do + case sign_object(object, private_key) do + {:ok, signature} -> {:reply, {:ok, signature, @signing_key_id}, state} + {:error, _reason} -> {:reply, :error, state} end end @@ -61,6 +40,18 @@ defmodule MatrixServer.SigningServer do {:reply, [{@signing_key_id, result}], state} end + # https://blog.swwomm.com/2020/09/elixir-ed25519-signatures-with-enacl.html + defp sign_object(object, private_key) do + with {:ok, json} <- MatrixServer.encode_canonical_json(object) do + signature = + json + |> :enacl.sign_detached(private_key) + |> MatrixServer.encode_unpadded_base64() + + {:ok, signature} + end + end + # TODO: not sure if there is a better way to do this... defp get_keys do raw_priv_key = diff --git a/lib/matrix_server_web/authenticate_server.ex b/lib/matrix_server_web/authenticate_server.ex index ca58acc..2b16bb2 100644 --- a/lib/matrix_server_web/authenticate_server.ex +++ b/lib/matrix_server_web/authenticate_server.ex @@ -1,75 +1,50 @@ defmodule MatrixServerWeb.AuthenticateServer do - import Ecto.Changeset + import MatrixServerWeb.Plug.Error alias MatrixServer.SigningServer - alias Ecto.Changeset @auth_header_regex ~r/^X-Matrix origin=(?.*),key="(?.*)",sig="(?.*)"$/ - defmodule SignedJSON do - use Ecto.Schema + def authenticate( + %Plug.Conn{ + body_params: body_params, + req_headers: headers, + request_path: path, + method: method, + query_string: query_string + } + ) do + object_to_sign = %{ + uri: path <> "?" <> URI.decode_www_form(query_string), + method: method, + destination: MatrixServer.server_name() + } - import Ecto.Changeset + object_to_sign = + if method != "GET", do: Map.put(object_to_sign, :content, body_params), else: object_to_sign - @primary_key false - embedded_schema do - field :method, :string - field :uri, :string - field :origin, :string - field :destination, :string - field :content, :map - field :signatures, :map - end + object_fun = &Map.put(object_to_sign, :origin, &1) - def changeset(params) do - %__MODULE__{} - |> cast(params, [:method, :uri, :origin, :destination, :content, :signatures]) - |> validate_required([:method, :uri, :origin, :destination]) - end + authenticate_with_headers(headers, object_fun) end - def authenticated?(%Plug.Conn{body_params: params}) do - with %Changeset{valid?: true} = cs <- SignedJSON.changeset(params), - input <- apply_changes(cs) do - verify_signature(input) - else - _ -> false - end - end + defp authenticate_with_headers(headers, object_fun) do + headers + |> parse_authorization_headers() + |> Enum.find(:error, fn {origin, _, sig} -> + # TODO: fetch actual signing keys for origin from cache/key store. + {_, signing_key} = SigningServer.get_signing_keys() |> hd() + object = object_fun.(origin) - defp verify_signature(%SignedJSON{signatures: signatures, origin: origin} = input) do - if Map.has_key?(signatures, origin) do - # TODO: fetch actual signing keys from cache/key store. - signing_keys = SigningServer.get_signing_keys() |> Enum.into(%{}) - - found_signatures = - Enum.filter(signatures[origin], fn {key, _} -> - case String.split(key, ":", parts: 2) do - [algorithm, _] -> algorithm == "ed25519" - _ -> false - end - end) - |> Enum.map(fn {key_id, sig} -> - if Map.has_key?(signing_keys, key_id) do - {key_id, sig, signing_keys[key_id]} - end - end) - |> Enum.reject(&Kernel.is_nil/1) - - with [{_, sig, signing_key} | _] <- found_signatures, - {:ok, raw_sig} <- MatrixServer.decode_base64(sig), - serializable_input <- MatrixServer.to_serializable_map(input), - {:ok, encoded_input} <- MatrixServer.encode_canonical_json(serializable_input) do - :enacl.sign_verify_detached(raw_sig, encoded_input, signing_key) + with {:ok, raw_sig} <- MatrixServer.decode_base64(sig), + {:ok, encoded_object} <- MatrixServer.encode_canonical_json(object) do + :enacl.sign_verify_detached(raw_sig, encoded_object, signing_key) else _ -> false end - else - false - end + end) end - # TODO: Not actually needed? def parse_authorization_headers(headers) do headers |> Enum.filter(&(elem(&1, 0) == "authorization")) @@ -77,9 +52,10 @@ defmodule MatrixServerWeb.AuthenticateServer do Regex.named_captures(@auth_header_regex, auth_header) end) |> Enum.reject(&Kernel.is_nil/1) - |> Enum.reduce(%{}, fn %{"origin" => origin, "key" => key, "sig" => sig}, acc -> - Map.update(acc, origin, %{key => sig}, &Map.put(&1, key, sig)) + |> Enum.map(fn %{"origin" => origin, "key" => key, "sig" => sig} -> + {origin, key, sig} end) + |> Enum.filter(fn {_, key, _} -> String.starts_with?(key, "ed25519:") end) end defmacro __using__(opts) do @@ -89,10 +65,14 @@ defmodule MatrixServerWeb.AuthenticateServer do def action(conn, _) do action = action_name(conn) - if action not in unquote(except) and - not MatrixServerWeb.AuthenticateServer.authenticated?(conn) do - IO.puts("Not authorized!") - apply(__MODULE__, action, [conn, conn.params]) + if action not in unquote(except) do + case MatrixServerWeb.AuthenticateServer.authenticate(conn) do + {origin, _key, _sig} -> + conn = Plug.Conn.assign(conn, :origin, origin) + apply(__MODULE__, action, [conn, conn.params]) + :error -> + put_error(conn, :unauthorized, "Signature verification failed.") + end else apply(__MODULE__, action, [conn, conn.params]) end diff --git a/lib/matrix_server_web/federation/query_controller.ex b/lib/matrix_server_web/federation/query_controller.ex new file mode 100644 index 0000000..0568356 --- /dev/null +++ b/lib/matrix_server_web/federation/query_controller.ex @@ -0,0 +1,55 @@ +defmodule MatrixServerWeb.Federation.QueryController do + use MatrixServerWeb, :controller + use MatrixServerWeb.AuthenticateServer + + import MatrixServerWeb.Plug.Error + import Ecto.Query + + alias MatrixServer.{Repo, Account} + + defmodule ProfileRequest do + use Ecto.Schema + + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :user_id, :string + field :field, :string + end + + def validate(params) do + %__MODULE__{} + |> cast(params, [:user_id, :field]) + |> validate_required([:user_id]) + |> validate_inclusion(:field, ["displayname", "avatar_url"]) + |> tap(fn + %Ecto.Changeset{valid?: true} = cs -> {:ok, apply_changes(cs)} + _ -> :error + end) + end + end + + def profile(conn, params) do + with %ProfileRequest{user_id: user_id} <- ProfileRequest.validate(params) do + if MatrixServer.get_domain(user_id) == MatrixServer.server_name() do + localpart = MatrixServer.get_localpart(user_id) + + case Repo.one(from a in Account, where: a.localpart == ^localpart) do + %Account{} -> + # TODO: Return displayname and avatar_url when we implement them. + conn + |> put_status(200) + |> json(%{}) + + nil -> + put_error(:not_found, "User does not exist.") + end + else + put_error(:not_found, "Wrong server name.") + end + else + _ -> put_error(conn, :bad_json) + end + end +end diff --git a/lib/matrix_server_web/federation/test_controller.ex b/lib/matrix_server_web/federation/test_controller.ex deleted file mode 100644 index 37d7e8b..0000000 --- a/lib/matrix_server_web/federation/test_controller.ex +++ /dev/null @@ -1,10 +0,0 @@ -defmodule MatrixServerWeb.Federation.TestController do - use MatrixServerWeb, :controller - use MatrixServerWeb.AuthenticateServer - - def test(conn, _params) do - conn - |> put_status(200) - |> json(%{}) - end -end diff --git a/lib/matrix_server_web/federation_client.ex b/lib/matrix_server_web/federation_client.ex index 5036544..26f07c2 100644 --- a/lib/matrix_server_web/federation_client.ex +++ b/lib/matrix_server_web/federation_client.ex @@ -20,26 +20,27 @@ defmodule MatrixServerWeb.FederationClient do Tesla.get(client, RouteHelpers.key_path(Endpoint, :get_signing_keys)) end - def test_server_auth(client) do - origin = "localhost:4001" - destination = "localhost:4000" - path = RouteHelpers.test_path(Endpoint, :test) + # TODO: Create tesla middleware to add signature and headers. + def query_profile(client, server_name, user_id, field \\ nil) do + origin = MatrixServer.server_name() + path = RouteHelpers.query_path(Endpoint, :profile) |> Tesla.build_url(user_id: user_id) + path = if field, do: Tesla.build_url(path, field: field), else: path - params = %{ - method: "POST", + object_to_sign = %{ + method: "GET", uri: path, origin: origin, - destination: destination, - content: %{"hi" => "hello"} + destination: server_name } - {:ok, signed_object} = MatrixServer.SigningServer.sign_object(params) - auth_headers = create_signature_authorization_headers(signed_object, origin) + {:ok, signature, key_id} = MatrixServer.SigningServer.sign_object(object_to_sign) + signatures = %{origin => %{key_id => signature}} + auth_headers = create_signature_authorization_headers(signatures, origin) - Tesla.post(client, path, signed_object, headers: auth_headers) + Tesla.get(client, path, headers: auth_headers) end - defp create_signature_authorization_headers(%{signatures: signatures}, origin) do + defp create_signature_authorization_headers(signatures, origin) do Enum.map(signatures[origin], fn {key, sig} -> {"Authorization", "X-Matrix origin=#{origin},key=\"#{key}\",sig=\"#{sig}\""} end) diff --git a/lib/matrix_server_web/plug/error.ex b/lib/matrix_server_web/plug/error.ex index d65abad..3fb7b12 100644 --- a/lib/matrix_server_web/plug/error.ex +++ b/lib/matrix_server_web/plug/error.ex @@ -11,10 +11,11 @@ defmodule MatrixServerWeb.Plug.Error do unknown: {400, "M_UNKNOWN", "An unknown error occurred."}, invalid_room_state: {400, "M_INVALID_ROOM_STATE", "The request would leave the room in an invalid state."}, + unauthorized: {400, "M_UNAUTHORIZED", "The request was unauthorized."}, unknown_token: {401, "M_UNKNOWN_TOKEN", "Invalid access token."}, missing_token: {401, "M_MISSING_TOKEN", "Access token required."}, not_found: {404, "M_NOT_FOUND", "The requested resource was not found."}, - room_alias_exists: {409, "M.UNKNOWN", "The given room alias already exists."} + room_alias_exists: {409, "M_UNKNOWN", "The given room alias already exists."} } def put_error(conn, {error, msg}), do: put_error(conn, error, msg) diff --git a/lib/matrix_server_web/router.ex b/lib/matrix_server_web/router.ex index ff7ac8a..3ebc8b6 100644 --- a/lib/matrix_server_web/router.ex +++ b/lib/matrix_server_web/router.ex @@ -56,7 +56,7 @@ defmodule MatrixServerWeb.Router do scope "/_matrix", MatrixServerWeb.Federation do pipe_through :authenticate_server - post "/test", TestController, :test + get "/federation/v1/query/profile", QueryController, :profile end scope "/", MatrixServerWeb.Client do