User: generate private keys on user creation

This fixes a race condition bug where keys could be regenerated
post-federation, causing activities and HTTP signatures from an user to
be dropped due to key differences.
This commit is contained in:
Hélène 2022-08-26 18:30:43 +02:00
parent 84a573877a
commit cd237d22f1
No known key found for this signature in database
GPG Key ID: A215F2E9F1589D62
9 changed files with 32 additions and 88 deletions

View File

@ -59,9 +59,8 @@ def refetch_public_key(conn) do
end end
end end
def sign(%User{} = user, headers) do def sign(%User{keys: keys} = user, headers) do
with {:ok, %{keys: keys}} <- User.ensure_keys_present(user), with {:ok, private_key, _} <- Keys.keys_from_pem(keys) do
{:ok, private_key, _} <- Keys.keys_from_pem(keys) do
HTTPSignatures.sign(private_key, user.ap_id <> "#main-key", headers) HTTPSignatures.sign(private_key, user.ap_id <> "#main-key", headers)
end end
end end

View File

@ -711,6 +711,7 @@ def register_changeset_ldap(struct, params = %{password: password})
|> put_ap_id() |> put_ap_id()
|> unique_constraint(:ap_id) |> unique_constraint(:ap_id)
|> put_following_and_follower_and_featured_address() |> put_following_and_follower_and_featured_address()
|> put_private_key()
end end
def register_changeset(struct, params \\ %{}, opts \\ []) do def register_changeset(struct, params \\ %{}, opts \\ []) do
@ -768,6 +769,7 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do
|> put_ap_id() |> put_ap_id()
|> unique_constraint(:ap_id) |> unique_constraint(:ap_id)
|> put_following_and_follower_and_featured_address() |> put_following_and_follower_and_featured_address()
|> put_private_key()
end end
def validate_not_restricted_nickname(changeset, field) do def validate_not_restricted_nickname(changeset, field) do
@ -846,6 +848,11 @@ defp put_following_and_follower_and_featured_address(changeset) do
|> put_change(:featured_address, featured) |> put_change(:featured_address, featured)
end end
defp put_private_key(changeset) do
{:ok, pem} = Keys.generate_rsa_pem()
put_change(changeset, :keys, pem)
end
defp autofollow_users(user) do defp autofollow_users(user) do
candidates = Config.get([:instance, :autofollowed_nicknames]) candidates = Config.get([:instance, :autofollowed_nicknames])
@ -2086,6 +2093,7 @@ defp create_service_actor(uri, nickname) do
follower_address: uri <> "/followers" follower_address: uri <> "/followers"
} }
|> change |> change
|> put_private_key()
|> unique_constraint(:nickname) |> unique_constraint(:nickname)
|> Repo.insert() |> Repo.insert()
|> set_cache() |> set_cache()
@ -2351,17 +2359,6 @@ def get_mascot(%{mascot: mascot}) when is_nil(mascot) do
} }
end end
def ensure_keys_present(%{keys: keys} = user) when not is_nil(keys), do: {:ok, user}
def ensure_keys_present(%User{} = user) do
with {:ok, pem} <- Keys.generate_rsa_pem() do
user
|> cast(%{keys: pem}, [:keys])
|> validate_required([:keys])
|> update_and_set_cache()
end
end
def get_ap_ids_by_nicknames(nicknames) do def get_ap_ids_by_nicknames(nicknames) do
from(u in User, from(u in User,
where: u.nickname in ^nicknames, where: u.nickname in ^nicknames,

View File

@ -66,8 +66,7 @@ defp relay_active?(conn, _) do
end end
def user(conn, %{"nickname" => nickname}) do def user(conn, %{"nickname" => nickname}) do
with %User{local: true} = user <- User.get_cached_by_nickname(nickname), with %User{local: true} = user <- User.get_cached_by_nickname(nickname) do
{:ok, user} <- User.ensure_keys_present(user) do
conn conn
|> put_resp_content_type("application/activity+json") |> put_resp_content_type("application/activity+json")
|> put_view(UserView) |> put_view(UserView)
@ -174,7 +173,6 @@ def relay_following(conn, _params) do
def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname),
{user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user),
{:show_follows, true} <- {:show_follows, true} <-
{:show_follows, (for_user && for_user == user) || !user.hide_follows} do {:show_follows, (for_user && for_user == user) || !user.hide_follows} do
{page, _} = Integer.parse(page) {page, _} = Integer.parse(page)
@ -192,8 +190,7 @@ def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "p
end end
def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) do def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname) do
{user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
conn conn
|> put_resp_content_type("application/activity+json") |> put_resp_content_type("application/activity+json")
|> put_view(UserView) |> put_view(UserView)
@ -213,7 +210,6 @@ def relay_followers(conn, _params) do
def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname),
{user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user),
{:show_followers, true} <- {:show_followers, true} <-
{:show_followers, (for_user && for_user == user) || !user.hide_followers} do {:show_followers, (for_user && for_user == user) || !user.hide_followers} do
{page, _} = Integer.parse(page) {page, _} = Integer.parse(page)
@ -231,8 +227,7 @@ def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "p
end end
def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) do def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname) do
{user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
conn conn
|> put_resp_content_type("application/activity+json") |> put_resp_content_type("application/activity+json")
|> put_view(UserView) |> put_view(UserView)
@ -245,8 +240,7 @@ def outbox(
%{"nickname" => nickname, "page" => page?} = params %{"nickname" => nickname, "page" => page?} = params
) )
when page? in [true, "true"] do when page? in [true, "true"] do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname) do
{:ok, user} <- User.ensure_keys_present(user) do
# "include_poll_votes" is a hack because postgres generates inefficient # "include_poll_votes" is a hack because postgres generates inefficient
# queries when filtering by 'Answer', poll votes will be hidden by the # queries when filtering by 'Answer', poll votes will be hidden by the
# visibility filter in this case anyway # visibility filter in this case anyway
@ -270,8 +264,7 @@ def outbox(
end end
def outbox(conn, %{"nickname" => nickname}) do def outbox(conn, %{"nickname" => nickname}) do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname) do
{:ok, user} <- User.ensure_keys_present(user) do
conn conn
|> put_resp_content_type("application/activity+json") |> put_resp_content_type("application/activity+json")
|> put_view(UserView) |> put_view(UserView)
@ -328,14 +321,10 @@ defp post_inbox_relayed_create(conn, params) do
end end
defp represent_service_actor(%User{} = user, conn) do defp represent_service_actor(%User{} = user, conn) do
with {:ok, user} <- User.ensure_keys_present(user) do conn
conn |> put_resp_content_type("application/activity+json")
|> put_resp_content_type("application/activity+json") |> put_view(UserView)
|> put_view(UserView) |> render("user.json", %{user: user})
|> render("user.json", %{user: user})
else
nil -> {:error, :not_found}
end
end end
defp represent_service_actor(nil, _), do: {:error, :not_found} defp represent_service_actor(nil, _), do: {:error, :not_found}
@ -388,12 +377,10 @@ def read_inbox(
def read_inbox(%{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{ def read_inbox(%{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{
"nickname" => nickname "nickname" => nickname
}) do }) do
with {:ok, user} <- User.ensure_keys_present(user) do conn
conn |> put_resp_content_type("application/activity+json")
|> put_resp_content_type("application/activity+json") |> put_view(UserView)
|> put_view(UserView) |> render("activity_collection.json", %{iri: "#{user.ap_id}/inbox"})
|> render("activity_collection.json", %{iri: "#{user.ap_id}/inbox"})
end
end end
def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{ def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{
@ -530,19 +517,6 @@ defp set_requester_reachable(%Plug.Conn{} = conn, _) do
conn conn
end end
defp ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
{:ok, new_user} = User.ensure_keys_present(user)
for_user =
if new_user != user and match?(%User{}, for_user) do
User.get_cached_by_nickname(for_user.nickname)
else
for_user
end
{new_user, for_user}
end
def upload_media(%{assigns: %{user: %User{} = user}} = conn, %{"file" => file} = data) do def upload_media(%{assigns: %{user: %User{} = user}} = conn, %{"file" => file} = data) do
with {:ok, object} <- with {:ok, object} <-
ActivityPub.upload( ActivityPub.upload(

View File

@ -34,7 +34,6 @@ def render("endpoints.json", %{user: %User{local: true} = _user}) do
def render("endpoints.json", _), do: %{} def render("endpoints.json", _), do: %{}
def render("service.json", %{user: user}) do def render("service.json", %{user: user}) do
{:ok, user} = User.ensure_keys_present(user)
{:ok, _, public_key} = Keys.keys_from_pem(user.keys) {:ok, _, public_key} = Keys.keys_from_pem(user.keys)
public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key) public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key)
public_key = :public_key.pem_encode([public_key]) public_key = :public_key.pem_encode([public_key])
@ -71,7 +70,6 @@ def render("user.json", %{user: %User{nickname: "internal." <> _} = user}),
do: render("service.json", %{user: user}) |> Map.put("preferredUsername", user.nickname) do: render("service.json", %{user: user}) |> Map.put("preferredUsername", user.nickname)
def render("user.json", %{user: user}) do def render("user.json", %{user: user}) do
{:ok, user} = User.ensure_keys_present(user)
{:ok, _, public_key} = Keys.keys_from_pem(user.keys) {:ok, _, public_key} = Keys.keys_from_pem(user.keys)
public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key) public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key)
public_key = :public_key.pem_encode([public_key]) public_key = :public_key.pem_encode([public_key])

View File

@ -61,10 +61,8 @@ def perform(:publish_one, module, params) do
def perform(:publish, activity) do def perform(:publish, activity) do
Logger.debug(fn -> "Running publish for #{activity.data["id"]}" end) Logger.debug(fn -> "Running publish for #{activity.data["id"]}" end)
with %User{} = actor <- User.get_cached_by_ap_id(activity.data["actor"]), %User{} = actor = User.get_cached_by_ap_id(activity.data["actor"])
{:ok, actor} <- User.ensure_keys_present(actor) do Publisher.publish(actor, activity)
Publisher.publish(actor, activity)
end
end end
def perform(:incoming_ap_doc, params) do def perform(:incoming_ap_doc, params) do

View File

@ -63,8 +63,6 @@ defp gather_aliases(%User{} = user) do
end end
def represent_user(user, "JSON") do def represent_user(user, "JSON") do
{:ok, user} = User.ensure_keys_present(user)
%{ %{
"subject" => "acct:#{user.nickname}@#{Pleroma.Web.Endpoint.host()}", "subject" => "acct:#{user.nickname}@#{Pleroma.Web.Endpoint.host()}",
"aliases" => gather_aliases(user), "aliases" => gather_aliases(user),
@ -73,8 +71,6 @@ def represent_user(user, "JSON") do
end end
def represent_user(user, "XML") do def represent_user(user, "XML") do
{:ok, user} = User.ensure_keys_present(user)
aliases = aliases =
user user
|> gather_aliases() |> gather_aliases()

View File

@ -677,14 +677,14 @@ test "it blocks blacklisted email domains" do
assert changeset.valid? assert changeset.valid?
end end
test "it sets the password_hash and ap_id" do test "it sets the password_hash, ap_id, private key and followers collection address" do
changeset = User.register_changeset(%User{}, @full_user_data) changeset = User.register_changeset(%User{}, @full_user_data)
assert changeset.valid? assert changeset.valid?
assert is_binary(changeset.changes[:password_hash]) assert is_binary(changeset.changes[:password_hash])
assert is_binary(changeset.changes[:keys])
assert changeset.changes[:ap_id] == User.ap_id(%User{nickname: @full_user_data.nickname}) assert changeset.changes[:ap_id] == User.ap_id(%User{nickname: @full_user_data.nickname})
assert changeset.changes.follower_address == "#{changeset.changes.ap_id}/followers" assert changeset.changes.follower_address == "#{changeset.changes.ap_id}/followers"
end end
@ -2131,21 +2131,6 @@ test "Only includes users with no read notifications" do
end end
end end
describe "ensure_keys_present" do
test "it creates keys for a user and stores them in info" do
user = insert(:user)
refute is_binary(user.keys)
{:ok, user} = User.ensure_keys_present(user)
assert is_binary(user.keys)
end
test "it doesn't create keys if there already are some" do
user = insert(:user, keys: "xxx")
{:ok, user} = User.ensure_keys_present(user)
assert user.keys == "xxx"
end
end
describe "get_ap_ids_by_nicknames" do describe "get_ap_ids_by_nicknames" do
test "it returns a list of AP ids for a given set of nicknames" do test "it returns a list of AP ids for a given set of nicknames" do
user = insert(:user) user = insert(:user)

View File

@ -12,7 +12,6 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
test "Renders a user, including the public key" do test "Renders a user, including the public key" do
user = insert(:user) user = insert(:user)
{:ok, user} = User.ensure_keys_present(user)
result = UserView.render("user.json", %{user: user}) result = UserView.render("user.json", %{user: user})
@ -55,7 +54,6 @@ test "Renders with emoji tags" do
test "Does not add an avatar image if the user hasn't set one" do test "Does not add an avatar image if the user hasn't set one" do
user = insert(:user) user = insert(:user)
{:ok, user} = User.ensure_keys_present(user)
result = UserView.render("user.json", %{user: user}) result = UserView.render("user.json", %{user: user})
refute result["icon"] refute result["icon"]
@ -67,8 +65,6 @@ test "Does not add an avatar image if the user hasn't set one" do
banner: %{"url" => [%{"href" => "https://somebanner"}]} banner: %{"url" => [%{"href" => "https://somebanner"}]}
) )
{:ok, user} = User.ensure_keys_present(user)
result = UserView.render("user.json", %{user: user}) result = UserView.render("user.json", %{user: user})
assert result["icon"]["url"] == "https://someurl" assert result["icon"]["url"] == "https://someurl"
assert result["image"]["url"] == "https://somebanner" assert result["image"]["url"] == "https://somebanner"
@ -89,7 +85,6 @@ test "renders AKAs" do
describe "endpoints" do describe "endpoints" do
test "local users have a usable endpoints structure" do test "local users have a usable endpoints structure" do
user = insert(:user) user = insert(:user)
{:ok, user} = User.ensure_keys_present(user)
result = UserView.render("user.json", %{user: user}) result = UserView.render("user.json", %{user: user})
@ -105,7 +100,6 @@ test "local users have a usable endpoints structure" do
test "remote users have an empty endpoints structure" do test "remote users have an empty endpoints structure" do
user = insert(:user, local: false) user = insert(:user, local: false)
{:ok, user} = User.ensure_keys_present(user)
result = UserView.render("user.json", %{user: user}) result = UserView.render("user.json", %{user: user})
@ -115,7 +109,6 @@ test "remote users have an empty endpoints structure" do
test "instance users do not expose oAuth endpoints" do test "instance users do not expose oAuth endpoints" do
user = insert(:user, nickname: nil, local: true) user = insert(:user, nickname: nil, local: true)
{:ok, user} = User.ensure_keys_present(user)
result = UserView.render("user.json", %{user: user}) result = UserView.render("user.json", %{user: user})

View File

@ -7,6 +7,7 @@ defmodule Pleroma.Factory do
require Pleroma.Constants require Pleroma.Constants
alias Pleroma.Keys
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.User alias Pleroma.User
@ -28,6 +29,8 @@ def conversation_factory do
end end
def user_factory(attrs \\ %{}) do def user_factory(attrs \\ %{}) do
{:ok, pem} = Keys.generate_rsa_pem()
user = %User{ user = %User{
name: sequence(:name, &"Test テスト User #{&1}"), name: sequence(:name, &"Test テスト User #{&1}"),
email: sequence(:email, &"user#{&1}@example.com"), email: sequence(:email, &"user#{&1}@example.com"),
@ -39,7 +42,8 @@ def user_factory(attrs \\ %{}) do
last_refreshed_at: NaiveDateTime.utc_now(), last_refreshed_at: NaiveDateTime.utc_now(),
notification_settings: %Pleroma.User.NotificationSetting{}, notification_settings: %Pleroma.User.NotificationSetting{},
multi_factor_authentication_settings: %Pleroma.MFA.Settings{}, multi_factor_authentication_settings: %Pleroma.MFA.Settings{},
ap_enabled: true ap_enabled: true,
keys: pem
} }
urls = urls =