Blind key rotation and stale duration for profiles
See https://blog.dereferenced.org/the-case-for-blind-key-rotation Signed-off-by: Thomas Citharel <tcit@tcit.fr>
This commit is contained in:
parent
3a753312c1
commit
39b7afd1cd
|
@ -21,6 +21,11 @@ Also make sure to remove the `EnvironmentFile=` line from the systemd service an
|
||||||
- Possibility to change email address for the account
|
- Possibility to change email address for the account
|
||||||
- Possibility to delete your account
|
- Possibility to delete your account
|
||||||
|
|
||||||
|
### Changed
|
||||||
|
- Signature validation also now checks if `Date` header has acceptable values
|
||||||
|
- Actor profiles are now stale after two days and have to be refetched
|
||||||
|
- Actor keys are rotated some time after sending a `Delete` activity
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
- Fixed URL search
|
- Fixed URL search
|
||||||
- Fixed content accessed through URL search being public
|
- Fixed content accessed through URL search being public
|
||||||
|
|
|
@ -146,7 +146,11 @@ config :ex_cldr,
|
||||||
config :http_signatures,
|
config :http_signatures,
|
||||||
adapter: Mobilizon.Federation.HTTPSignatures.Signature
|
adapter: Mobilizon.Federation.HTTPSignatures.Signature
|
||||||
|
|
||||||
config :mobilizon, :activitypub, sign_object_fetches: true
|
config :mobilizon, :activitypub,
|
||||||
|
# One day
|
||||||
|
actor_stale_period: 3_600 * 48,
|
||||||
|
actor_key_rotation_delay: 3_600 * 48,
|
||||||
|
sign_object_fetches: true
|
||||||
|
|
||||||
config :mobilizon, Mobilizon.Service.Geospatial, service: Mobilizon.Service.Geospatial.Nominatim
|
config :mobilizon, Mobilizon.Service.Geospatial, service: Mobilizon.Service.Geospatial.Nominatim
|
||||||
|
|
||||||
|
|
|
@ -73,8 +73,8 @@ defmodule Mobilizon.Federation.ActivityPub do
|
||||||
with {:not_http, true} <- {:not_http, String.starts_with?(url, "http")},
|
with {:not_http, true} <- {:not_http, String.starts_with?(url, "http")},
|
||||||
{:existing_event, nil} <- {:existing_event, Events.get_event_by_url(url)},
|
{:existing_event, nil} <- {:existing_event, Events.get_event_by_url(url)},
|
||||||
{:existing_comment, nil} <- {:existing_comment, Events.get_comment_from_url(url)},
|
{:existing_comment, nil} <- {:existing_comment, Events.get_comment_from_url(url)},
|
||||||
{:existing_actor, {:error, :actor_not_found}} <-
|
{:existing_actor, {:error, _err}} <-
|
||||||
{:existing_actor, Actors.get_actor_by_url(url)},
|
{:existing_actor, get_or_fetch_actor_by_url(url)},
|
||||||
{:ok, %{body: body, status_code: code}} when code in 200..299 <-
|
{:ok, %{body: body, status_code: code}} when code in 200..299 <-
|
||||||
HTTPoison.get(
|
HTTPoison.get(
|
||||||
url,
|
url,
|
||||||
|
@ -126,7 +126,7 @@ defmodule Mobilizon.Federation.ActivityPub do
|
||||||
end
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Getting an actor from url, eventually creating it
|
Getting an actor from url, eventually creating it if we don't have it locally or if it needs an update
|
||||||
"""
|
"""
|
||||||
@spec get_or_fetch_actor_by_url(String.t(), boolean) :: {:ok, Actor.t()} | {:error, String.t()}
|
@spec get_or_fetch_actor_by_url(String.t(), boolean) :: {:ok, Actor.t()} | {:error, String.t()}
|
||||||
def get_or_fetch_actor_by_url(url, preload \\ false)
|
def get_or_fetch_actor_by_url(url, preload \\ false)
|
||||||
|
@ -138,12 +138,13 @@ defmodule Mobilizon.Federation.ActivityPub do
|
||||||
end
|
end
|
||||||
|
|
||||||
def get_or_fetch_actor_by_url(url, preload) do
|
def get_or_fetch_actor_by_url(url, preload) do
|
||||||
case Actors.get_actor_by_url(url, preload) do
|
with {:ok, %Actor{} = cached_actor} <- Actors.get_actor_by_url(url, preload),
|
||||||
{:ok, %Actor{} = actor} ->
|
false <- Actors.needs_update?(cached_actor) do
|
||||||
{:ok, actor}
|
{:ok, cached_actor}
|
||||||
|
else
|
||||||
_ ->
|
_ ->
|
||||||
case make_actor_from_url(url, preload) do
|
# For tests, see https://github.com/jjh42/mock#not-supported---mocking-internal-function-calls and Mobilizon.Federation.ActivityPubTest
|
||||||
|
case __MODULE__.make_actor_from_url(url, preload) do
|
||||||
{:ok, %Actor{} = actor} ->
|
{:ok, %Actor{} = actor} ->
|
||||||
{:ok, actor}
|
{:ok, actor}
|
||||||
|
|
||||||
|
@ -351,6 +352,7 @@ defmodule Mobilizon.Federation.ActivityPub do
|
||||||
{:ok, %Tombstone{} = _tombstone} <-
|
{:ok, %Tombstone{} = _tombstone} <-
|
||||||
Tombstone.create_tombstone(%{uri: event.url, actor_id: actor.id}),
|
Tombstone.create_tombstone(%{uri: event.url, actor_id: actor.id}),
|
||||||
Share.delete_all_by_uri(event.url),
|
Share.delete_all_by_uri(event.url),
|
||||||
|
:ok <- check_for_actor_key_rotation(actor),
|
||||||
{:ok, activity} <- create_activity(Map.merge(data, audience), local),
|
{:ok, activity} <- create_activity(Map.merge(data, audience), local),
|
||||||
:ok <- maybe_federate(activity) do
|
:ok <- maybe_federate(activity) do
|
||||||
{:ok, activity, event}
|
{:ok, activity, event}
|
||||||
|
@ -374,6 +376,7 @@ defmodule Mobilizon.Federation.ActivityPub do
|
||||||
{:ok, %Tombstone{} = _tombstone} <-
|
{:ok, %Tombstone{} = _tombstone} <-
|
||||||
Tombstone.create_tombstone(%{uri: comment.url, actor_id: actor.id}),
|
Tombstone.create_tombstone(%{uri: comment.url, actor_id: actor.id}),
|
||||||
Share.delete_all_by_uri(comment.url),
|
Share.delete_all_by_uri(comment.url),
|
||||||
|
:ok <- check_for_actor_key_rotation(actor),
|
||||||
{:ok, activity} <- create_activity(Map.merge(data, audience), local),
|
{:ok, activity} <- create_activity(Map.merge(data, audience), local),
|
||||||
:ok <- maybe_federate(activity) do
|
:ok <- maybe_federate(activity) do
|
||||||
{:ok, activity, comment}
|
{:ok, activity, comment}
|
||||||
|
@ -1012,4 +1015,15 @@ defmodule Mobilizon.Federation.ActivityPub do
|
||||||
})
|
})
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp check_for_actor_key_rotation(%Actor{} = actor) do
|
||||||
|
if Actors.should_rotate_actor_key(actor) do
|
||||||
|
Actors.schedule_key_rotation(
|
||||||
|
actor,
|
||||||
|
Application.get_env(:mobilizon, :activitypub)[:actor_key_rotation_delay]
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
:ok
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -47,6 +47,12 @@ defmodule Mobilizon do
|
||||||
ActivityPub.Federator,
|
ActivityPub.Federator,
|
||||||
cachex_spec(:feed, 2500, 60, 60, &Feed.create_cache/1),
|
cachex_spec(:feed, 2500, 60, 60, &Feed.create_cache/1),
|
||||||
cachex_spec(:ics, 2500, 60, 60, &ICalendar.create_cache/1),
|
cachex_spec(:ics, 2500, 60, 60, &ICalendar.create_cache/1),
|
||||||
|
cachex_spec(
|
||||||
|
:actor_key_rotation,
|
||||||
|
2500,
|
||||||
|
div(Application.get_env(:mobilizon, :activitypub)[:actor_key_rotation_delay], 60),
|
||||||
|
60 * 30
|
||||||
|
),
|
||||||
cachex_spec(:statistics, 10, 60, 60),
|
cachex_spec(:statistics, 10, 60, 60),
|
||||||
cachex_spec(:config, 10, 60, 60),
|
cachex_spec(:config, 10, 60, 60),
|
||||||
cachex_spec(:activity_pub, 2500, 3, 15)
|
cachex_spec(:activity_pub, 2500, 3, 15)
|
||||||
|
|
|
@ -50,7 +50,8 @@ defmodule Mobilizon.Actors.Actor do
|
||||||
mentions: [Mention.t()],
|
mentions: [Mention.t()],
|
||||||
shares: [Share.t()],
|
shares: [Share.t()],
|
||||||
owner_shares: [Share.t()],
|
owner_shares: [Share.t()],
|
||||||
memberships: [t]
|
memberships: [t],
|
||||||
|
last_refreshed_at: DateTime.t()
|
||||||
}
|
}
|
||||||
|
|
||||||
@required_attrs [:preferred_username, :keys, :suspended, :url]
|
@required_attrs [:preferred_username, :keys, :suspended, :url]
|
||||||
|
@ -65,6 +66,7 @@ defmodule Mobilizon.Actors.Actor do
|
||||||
:domain,
|
:domain,
|
||||||
:summary,
|
:summary,
|
||||||
:manually_approves_followers,
|
:manually_approves_followers,
|
||||||
|
:last_refreshed_at,
|
||||||
:user_id
|
:user_id
|
||||||
]
|
]
|
||||||
@attrs @required_attrs ++ @optional_attrs
|
@attrs @required_attrs ++ @optional_attrs
|
||||||
|
@ -118,6 +120,7 @@ defmodule Mobilizon.Actors.Actor do
|
||||||
field(:openness, ActorOpenness, default: :moderated)
|
field(:openness, ActorOpenness, default: :moderated)
|
||||||
field(:visibility, ActorVisibility, default: :private)
|
field(:visibility, ActorVisibility, default: :private)
|
||||||
field(:suspended, :boolean, default: false)
|
field(:suspended, :boolean, default: false)
|
||||||
|
field(:last_refreshed_at, :utc_datetime)
|
||||||
|
|
||||||
embeds_one(:avatar, File, on_replace: :update)
|
embeds_one(:avatar, File, on_replace: :update)
|
||||||
embeds_one(:banner, File, on_replace: :update)
|
embeds_one(:banner, File, on_replace: :update)
|
||||||
|
@ -261,6 +264,7 @@ defmodule Mobilizon.Actors.Actor do
|
||||||
|> unique_constraint(:url, name: :actors_url_index)
|
|> unique_constraint(:url, name: :actors_url_index)
|
||||||
|> unique_constraint(:preferred_username, name: :actors_preferred_username_domain_type_index)
|
|> unique_constraint(:preferred_username, name: :actors_preferred_username_domain_type_index)
|
||||||
|> validate_format(:preferred_username, ~r/[a-z0-9_]+/)
|
|> validate_format(:preferred_username, ~r/[a-z0-9_]+/)
|
||||||
|
|> put_change(:last_refreshed_at, DateTime.utc_now() |> DateTime.truncate(:second))
|
||||||
end
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
|
|
|
@ -260,6 +260,13 @@ defmodule Mobilizon.Actors do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@spec actor_key_rotation(Actor.t()) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
|
||||||
|
def actor_key_rotation(%Actor{} = actor) do
|
||||||
|
actor
|
||||||
|
|> Actor.changeset(%{keys: Crypto.generate_rsa_2048_private_key()})
|
||||||
|
|> Repo.update()
|
||||||
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Returns the list of actors.
|
Returns the list of actors.
|
||||||
"""
|
"""
|
||||||
|
@ -746,6 +753,40 @@ defmodule Mobilizon.Actors do
|
||||||
get_follower_by_followed_and_following(followed_actor, follower_actor)
|
get_follower_by_followed_and_following(followed_actor, follower_actor)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Whether the actor needs to be updated.
|
||||||
|
|
||||||
|
Local actors obviously don't need to be updated
|
||||||
|
"""
|
||||||
|
@spec needs_update?(Actor.t()) :: boolean
|
||||||
|
def needs_update?(%Actor{domain: nil}), do: false
|
||||||
|
|
||||||
|
def needs_update?(%Actor{last_refreshed_at: nil, domain: domain}) when not is_nil(domain),
|
||||||
|
do: true
|
||||||
|
|
||||||
|
def needs_update?(%Actor{domain: domain} = actor) when not is_nil(domain) do
|
||||||
|
DateTime.diff(DateTime.utc_now(), actor.last_refreshed_at) >=
|
||||||
|
Application.get_env(:mobilizon, :activitypub)[:actor_stale_period]
|
||||||
|
end
|
||||||
|
|
||||||
|
def needs_update?(_), do: true
|
||||||
|
|
||||||
|
@spec should_rotate_actor_key(Actor.t()) :: boolean
|
||||||
|
def should_rotate_actor_key(%Actor{id: actor_id}) do
|
||||||
|
with {:ok, value} when is_boolean(value) <- Cachex.exists?(:actor_key_rotation, actor_id) do
|
||||||
|
value
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@spec schedule_key_rotation(Actor.t(), integer()) :: nil
|
||||||
|
def schedule_key_rotation(%Actor{id: actor_id} = actor, delay) do
|
||||||
|
Cachex.put(:actor_key_rotation, actor_id, true)
|
||||||
|
|
||||||
|
Workers.Background.enqueue("actor_key_rotation", %{"actor_id" => actor.id}, schedule_in: delay)
|
||||||
|
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
@spec remove_banner(Actor.t()) :: {:ok, Actor.t()}
|
@spec remove_banner(Actor.t()) :: {:ok, Actor.t()}
|
||||||
defp remove_banner(%Actor{banner: nil} = actor), do: {:ok, actor}
|
defp remove_banner(%Actor{banner: nil} = actor), do: {:ok, actor}
|
||||||
|
|
||||||
|
|
|
@ -14,4 +14,10 @@ defmodule Mobilizon.Service.Workers.Background do
|
||||||
Actors.perform(:delete_actor, actor)
|
Actors.perform(:delete_actor, actor)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def perform(%{"op" => "actor_key_rotation", "actor_id" => actor_id}, _job) do
|
||||||
|
with %Actor{} = actor <- Actors.get_actor(actor_id) do
|
||||||
|
Actors.actor_key_rotation(actor)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,9 @@
|
||||||
|
defmodule Mobilizon.Storage.Repo.Migrations.AddLastRefreshedAtToActors do
|
||||||
|
use Ecto.Migration
|
||||||
|
|
||||||
|
def change do
|
||||||
|
alter table(:actors) do
|
||||||
|
add(:last_refreshed_at, :utc_datetime, null: true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -10,6 +10,7 @@ defmodule Mobilizon.Federation.ActivityPubTest do
|
||||||
import Mock
|
import Mock
|
||||||
import Mobilizon.Factory
|
import Mobilizon.Factory
|
||||||
|
|
||||||
|
alias Mobilizon.Actors
|
||||||
alias Mobilizon.Actors.Actor
|
alias Mobilizon.Actors.Actor
|
||||||
alias Mobilizon.Events
|
alias Mobilizon.Events
|
||||||
|
|
||||||
|
@ -47,10 +48,72 @@ defmodule Mobilizon.Federation.ActivityPubTest do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@actor_url "https://framapiaf.org/users/tcit"
|
||||||
test "returns an actor from url" do
|
test "returns an actor from url" do
|
||||||
|
# Initial fetch
|
||||||
use_cassette "activity_pub/fetch_framapiaf.org_users_tcit" do
|
use_cassette "activity_pub/fetch_framapiaf.org_users_tcit" do
|
||||||
assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
|
assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
|
||||||
ActivityPub.get_or_fetch_actor_by_url("https://framapiaf.org/users/tcit")
|
ActivityPub.get_or_fetch_actor_by_url(@actor_url)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Fetch uses cache if Actors.needs_update? returns false
|
||||||
|
with_mocks([
|
||||||
|
{Actors, [:passthrough],
|
||||||
|
[
|
||||||
|
get_actor_by_url: fn @actor_url, false ->
|
||||||
|
{:ok,
|
||||||
|
%Actor{
|
||||||
|
preferred_username: "tcit",
|
||||||
|
domain: "framapiaf.org"
|
||||||
|
}}
|
||||||
|
end,
|
||||||
|
needs_update?: fn _ -> false end
|
||||||
|
]},
|
||||||
|
{ActivityPub, [:passthrough],
|
||||||
|
make_actor_from_url: fn @actor_url, false ->
|
||||||
|
{:ok,
|
||||||
|
%Actor{
|
||||||
|
preferred_username: "tcit",
|
||||||
|
domain: "framapiaf.org"
|
||||||
|
}}
|
||||||
|
end}
|
||||||
|
]) do
|
||||||
|
assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
|
||||||
|
ActivityPub.get_or_fetch_actor_by_url(@actor_url)
|
||||||
|
|
||||||
|
assert_called(Actors.needs_update?(:_))
|
||||||
|
refute called(ActivityPub.make_actor_from_url(@actor_url, false))
|
||||||
|
end
|
||||||
|
|
||||||
|
# Fetch doesn't use cache if Actors.needs_update? returns true
|
||||||
|
with_mocks([
|
||||||
|
{Actors, [:passthrough],
|
||||||
|
[
|
||||||
|
get_actor_by_url: fn @actor_url, false ->
|
||||||
|
{:ok,
|
||||||
|
%Actor{
|
||||||
|
preferred_username: "tcit",
|
||||||
|
domain: "framapiaf.org"
|
||||||
|
}}
|
||||||
|
end,
|
||||||
|
needs_update?: fn _ -> true end
|
||||||
|
]},
|
||||||
|
{ActivityPub, [:passthrough],
|
||||||
|
make_actor_from_url: fn @actor_url, false ->
|
||||||
|
{:ok,
|
||||||
|
%Actor{
|
||||||
|
preferred_username: "tcit",
|
||||||
|
domain: "framapiaf.org"
|
||||||
|
}}
|
||||||
|
end}
|
||||||
|
]) do
|
||||||
|
assert {:ok, %Actor{preferred_username: "tcit", domain: "framapiaf.org"}} =
|
||||||
|
ActivityPub.get_or_fetch_actor_by_url(@actor_url)
|
||||||
|
|
||||||
|
assert_called(ActivityPub.get_or_fetch_actor_by_url(@actor_url))
|
||||||
|
assert_called(Actors.get_actor_by_url(@actor_url, false))
|
||||||
|
assert_called(Actors.needs_update?(:_))
|
||||||
|
assert_called(ActivityPub.make_actor_from_url(@actor_url, false))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -39,6 +39,7 @@ defmodule Mobilizon.Factory do
|
||||||
following_url: Actor.build_url(preferred_username, :following),
|
following_url: Actor.build_url(preferred_username, :following),
|
||||||
inbox_url: Actor.build_url(preferred_username, :inbox),
|
inbox_url: Actor.build_url(preferred_username, :inbox),
|
||||||
outbox_url: Actor.build_url(preferred_username, :outbox),
|
outbox_url: Actor.build_url(preferred_username, :outbox),
|
||||||
|
last_refreshed_at: DateTime.utc_now(),
|
||||||
user: build(:user)
|
user: build(:user)
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue