diff --git a/CHANGELOG.md b/CHANGELOG.md index 71949e2e4..1bbb382ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Added - Full compatibility with Erlang OTP26 - handling of GET /api/v1/preferences +- Explicit listing of config keys that are allowed to be set by the database + - Previously set config keys will still be loaded, but you will get a warning + that they probably should not be dynamically configured. ## Changed - OTP builds are now built on erlang OTP26 diff --git a/config/config.exs b/config/config.exs index 9a3cdcebf..3399cbdce 100644 --- a/config/config.exs +++ b/config/config.exs @@ -49,7 +49,9 @@ config :pleroma, Pleroma.Repo, telemetry_event: [Pleroma.Repo.Instrumenter], queue_target: 20_000, - migration_lock: nil + migration_lock: nil, + parameters: [gin_fuzzy_search_limit: "500"], + prepare: :unnamed config :pleroma, Pleroma.Captcha, enabled: true, @@ -75,6 +77,7 @@ truncated_namespace: nil, streaming_enabled: true +# This cannot be configured in the database! config :ex_aws, :s3, # host: "s3.wasabisys.com", # required if not Amazon AWS access_key_id: nil, @@ -453,10 +456,6 @@ image_quality: 85, min_content_length: 100 * 1024 -config :pleroma, :shout, - enabled: true, - limit: 5_000 - config :phoenix, :format_encoders, json: Jason, "activity+json": Jason config :phoenix, :json_library, Jason @@ -797,10 +796,6 @@ config :pleroma, configurable_from_database: false -config :pleroma, Pleroma.Repo, - parameters: [gin_fuzzy_search_limit: "500"], - prepare: :unnamed - config :pleroma, :majic_pool, size: 2 private_instance? = :if_instance_is_private diff --git a/config/description.exs b/config/description.exs index e108aaae8..080a2508c 100644 --- a/config/description.exs +++ b/config/description.exs @@ -1,16 +1,5 @@ import Config -websocket_config = [ - path: "/websocket", - serializer: [ - {Phoenix.Socket.V1.JSONSerializer, "~> 1.0.0"}, - {Phoenix.Socket.V2.JSONSerializer, "~> 2.0.0"} - ], - timeout: 60_000, - transport_log: false, - compress: false -] - installed_frontend_options = [ %{ key: "name", @@ -434,6 +423,7 @@ label: "URI Schemes", type: :group, description: "URI schemes related settings", + db_exclusion_reason: "Does not make sense to configure dynamically", children: [ %{ key: :valid_schemes, @@ -464,6 +454,7 @@ key: :features, type: :group, description: "Customizable features", + db_exclusion_reason: "Should be provided at boot-time", children: [ %{ key: :improved_hashtag_timeline, @@ -479,6 +470,7 @@ key: :populate_hashtags_table, type: :group, description: "`populate_hashtags_table` background migration settings", + db_exclusion_reason: "Should be provided at boot-time", children: [ %{ key: :fault_rate_allowance, @@ -1649,6 +1641,7 @@ key: Pleroma.Web.MediaProxy.Invalidation.Script, type: :group, description: "Invalidation script settings", + db_exclusion_reason: "Provides an arbitrary execution path", children: [ %{ key: :script_path, @@ -1762,6 +1755,7 @@ %{ group: :web_push_encryption, key: :vapid_details, + db_exclusion_reason: "Webserver secret keys", label: "Vapid Details", type: :group, description: @@ -1833,19 +1827,13 @@ %{ group: :pleroma, label: "Pleroma Admin Token", - type: :group, description: "Allows setting a token that can be used to authenticate requests with admin privileges without a normal user account token. Append the `admin_token` parameter to requests to utilize it. (Please reconsider using HTTP Basic Auth or OAuth-based authentication if possible)", - children: [ - %{ - key: :admin_token, - type: :string, - description: "Admin token", - suggestions: [ - "Please use a high entropy string or UUID" - ] - } - ] + type: :string, + suggestions: [ + "Please use a high entropy string or UUID" + ], + db_exclusion_reason: "Can provide passwordless admin access" }, %{ group: :pleroma, @@ -2189,6 +2177,7 @@ label: "Pleroma Authenticator", type: :group, description: "Authenticator", + db_exclusion_reason: "Should be provided at boot-time", children: [ %{ key: Pleroma.Web.Auth.Authenticator, @@ -2202,6 +2191,7 @@ key: :ldap, label: "LDAP", type: :group, + db_exclusion_reason: "Provides access to another service", description: "Use LDAP for user authentication. When a user logs in to the Pleroma instance, the name and password" <> " will be verified by trying to authenticate (bind) to a LDAP server." <> @@ -2601,6 +2591,7 @@ label: "Mime Types", type: :group, description: "Mime Types settings", + db_exclusion_reason: "Should be provided at compile-time", children: [ %{ key: :types, @@ -2807,6 +2798,7 @@ group: :cors_plug, label: "CORS plug config", type: :group, + db_exclusion_reason: "Should be provided at compile-time", children: [ %{ key: :max_age, @@ -2964,6 +2956,7 @@ key: :modules, type: :group, description: "Custom Runtime Modules", + db_exclusion_reason: "Allows for custom elixir execution", children: [ %{ key: :runtime_dir, @@ -3100,6 +3093,7 @@ group: :ex_aws, key: :s3, type: :group, + db_exclusion_reason: "Provides access to another service", descriptions: "S3 service related settings", children: [ %{ @@ -3479,6 +3473,7 @@ key: :argos_translate, type: :group, description: "ArgosTranslate Settings.", + db_exclusion_reason: "Excluded for being able to set arbitrary paths to executables", children: [ %{ key: :command_argos_translate, diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index 73fdf9eea..40e251523 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -375,6 +375,11 @@ This section describe PWA manifest instance-specific values. Currently this opti #### Pleroma.Web.MediaProxy.Invalidation.Script +!!! warning + Invalidation script options cannot be set in the database due to the ability to + set the command options to arbitrary paths. The following options **MUST** be + set in your `.exs` file instead. + This strategy allow perform external shell script to purge cache. Urls of attachments are passed to the script as arguments. @@ -1011,21 +1016,6 @@ git clone Boolean, enables/disables in-database configuration. Read [Transfering the config to/from the database](../administration/CLI_tasks/config.md) for more information. -## :database_config_whitelist - -List of valid configuration sections which are allowed to be configured from the -database. Settings stored in the database before the whitelist is configured are -still applied, so it is suggested to only use the whitelist on instances that -have not migrated the config to the database. - -Example: -```elixir -config :pleroma, :database_config_whitelist, [ - {:pleroma, :instance}, - {:pleroma, Pleroma.Web.Metadata}, - {:auto_linker} -] -``` ### Multi-factor authentication - :two_factor_authentication * `totp` - a list containing TOTP configuration @@ -1148,6 +1138,11 @@ Translations are available at `/api/v1/statuses/:id/translations/:language`, whe ### `:argos_translate` +!!! warning + Argos Translate options cannot be set in the database due to the ability to + set the command options to arbitrary paths. The following options **MUST** be + set in your `.exs` file instead. + - `:command_argos_translate` - command for `argos-translate`. Can be the command if it's in your PATH, or the full path to the file (default: `argos-translate`). - `:command_argospm` - command for `argospm`. Can be the command if it's in your PATH, or the full path to the file (default: `argospm`). - `:strip_html` - Strip html from the post before translating it (default: `true`). diff --git a/lib/mix/tasks/pleroma/config.ex b/lib/mix/tasks/pleroma/config.ex index 8661d8d7c..5c163a81f 100644 --- a/lib/mix/tasks/pleroma/config.ex +++ b/lib/mix/tasks/pleroma/config.ex @@ -10,6 +10,7 @@ defmodule Mix.Tasks.Pleroma.Config do alias Pleroma.ConfigDB alias Pleroma.Repo + alias Pleroma.Config.ConfigurableFromDatabase @shortdoc "Manages the location of the config" @moduledoc File.read!("docs/docs/administration/CLI_tasks/config.md") @@ -244,6 +245,38 @@ def run(["delete", group]) do end end + # Primarily a developer tool to check nothing was missed from + # db configwhitelist + def run(["check-allowed"]) do + start_pleroma() + Pleroma.Docs.JSON.compile() + raw = Pleroma.Docs.JSON.compiled_descriptions() + whitelisted = Enum.filter(raw, &ConfigurableFromDatabase.whitelisted_config?/1) + raw_map = MapSet.new(raw) + whitelisted_map = MapSet.new(whitelisted) + + IO.puts( + "Config keys defined in description.exs but not listed as explicitly allowed in the db" + ) + + IO.puts( + " Please check that standard admins should not need to touch the listed settings whilst the server is live." + ) + + IO.puts( + " !! Please remember that admins are not neccesarily sysadmins nor are they immune to oauth/password leakage." + ) + + IO.puts("-------------") + + MapSet.difference(raw_map, whitelisted_map) + |> Enum.each(fn map -> + IO.puts("#{map[:group]}, #{map[:key]} (#{map[:label]})") + IO.puts(map[:db_exclusion_reason] || "No exclusion reason set") + IO.puts("++") + end) + end + @spec migrate_to_db(Path.t() | nil) :: any() def migrate_to_db(file_path \\ nil) do with :ok <- Pleroma.Config.DeprecationWarnings.warn() do @@ -286,6 +319,9 @@ defp do_migrate_to_db(config_file) do defp create(group, settings) do group |> Pleroma.Config.Loader.filter_group(settings) + |> Enum.filter(fn {key, _value} -> + Pleroma.Config.ConfigurableFromDatabase.whitelisted_config?(group, key) + end) |> Enum.each(fn {key, value} -> {:ok, _} = ConfigDB.update_or_create(%{group: group, key: key, value: value}) diff --git a/lib/pleroma/config/configurable_from_database.ex b/lib/pleroma/config/configurable_from_database.ex new file mode 100644 index 000000000..1e97bc640 --- /dev/null +++ b/lib/pleroma/config/configurable_from_database.ex @@ -0,0 +1,112 @@ +defmodule Pleroma.Config.ConfigurableFromDatabase do + alias Pleroma.Config + + # Basically it's silly to let this be configurable + # set a list of things that we can set in the database + # this is mostly our stuff, with some extra in there + @allowed_groups [ + {:logger}, + {:pleroma, Pleroma.Captcha}, + {:pleroma, Pleroma.Captcha.Kocaptcha}, + {:pleroma, Pleroma.Upload}, + {:pleroma, Pleroma.Uploaders.Local}, + {:pleroma, Pleroma.Uploaders.S3}, + {:pleroma, :auth}, + {:pleroma, :emoji}, + {:pleroma, :http}, + {:pleroma, :instance}, + {:pleroma, :welcome}, + {:pleroma, :feed}, + {:pleroma, :markup}, + {:pleroma, :frontend_configurations}, + {:pleroma, :assets}, + {:pleroma, :manifest}, + {:pleroma, :activitypub}, + {:pleroma, :streamer}, + {:pleroma, :user}, + {:pleroma, :mrf_normalize_markup}, + {:pleroma, :mrf_rejectnonpublic}, + {:pleroma, :mrf_hellthread}, + {:pleroma, :mrf_simple}, + {:pleroma, :mrf_keyword}, + {:pleroma, :mrf_hashtag}, + {:pleroma, :mrf_subchain}, + {:pleroma, :mrf_activity_expiration}, + {:pleroma, :mrf_vocabulary}, + {:pleroma, :mrf_inline_quote}, + {:pleroma, :mrf_object_age}, + {:pleroma, :mrf_follow_bot}, + {:pleroma, :mrf_reject_newly_created_account_notes}, + {:pleroma, :rich_media}, + {:pleroma, :media_proxy}, + {:pleroma, Pleroma.Web.MediaProxy.Invalidation.Http}, + {:pleroma, :media_preview_proxy}, + {:pleroma, Pleroma.Web.Metadata}, + {:pleroma, Pleroma.Web.Metadata.Providers.Theme}, + {:pleroma, Pleroma.Web.Preload}, + {:pleroma, :http_security}, + {:pleroma, Pleroma.User}, + {:pleroma, Oban}, + {:pleroma, :workers}, + {:pleroma, Pleroma.Formatter}, + {:pleroma, Pleroma.Emails.Mailer}, + {:pleroma, Pleroma.Emails.UserEmail}, + {:pleroma, Pleroma.Emails.NewUsersDigestEmail}, + {:pleroma, Pleroma.ScheduledActivity}, + {:pleroma, :email_notifications}, + {:pleroma, :oauth2}, + {:pleroma, Pleroma.Web.Plugs.RemoteIp}, + {:pleroma, :static_fe}, + {:pleroma, :frontends}, + {:pleroma, :web_cache_ttl}, + {:pleroma, :majic_pool}, + {:pleroma, :restrict_unauthenticated}, + {:pleroma, Pleroma.Web.ApiSpec.CastAndValidate}, + {:pleroma, :mrf}, + {:pleroma, :instances_favicons}, + {:pleroma, :instances_nodeinfo}, + {:pleroma, Pleroma.User.Backup}, + {:pleroma, ConcurrentLimiter}, + {:pleroma, Pleroma.Web.WebFinger}, + {:pleroma, Pleroma.Search}, + {:pleroma, Pleroma.Search.Meilisearch}, + {:pleroma, Pleroma.Search.Elasticsearch.Cluster}, + {:pleroma, :translator}, + {:pleroma, :deepl}, + {:pleroma, :libre_translate}, + # But not argostranslate, because executables! + {:pleroma, Pleroma.Upload.Filter.AnonymizeFilename}, + {:pleroma, Pleroma.Upload.Filter.Mogrify}, + {:pleroma, Pleroma.Workers.PurgeExpiredActivity}, + {:pleroma, :rate_limit} + ] + + def allowed_groups, do: @allowed_groups + + def enabled, do: Config.get(:configurable_from_database) + + # the whitelist check can be called from either the loader or the + # doc generator, which is spitting out strings + defp maybe_stringified_atom_equal(a, b) do + a == inspect(b) || a == b + end + + def whitelisted_config?(group, key) do + allowed_groups() + |> Enum.any?(fn + {whitelisted_group} -> + maybe_stringified_atom_equal(group, whitelisted_group) + + {whitelisted_group, whitelisted_key} -> + maybe_stringified_atom_equal(group, whitelisted_group) && maybe_stringified_atom_equal(key, whitelisted_key) + end) + end + + def whitelisted_config?(%{group: group, key: key}) do + whitelisted_config?(group, key) + end + + def whitelisted_config?(%{group: group} = config) do + whitelisted_config?(group, config[:key]) + end +end diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index a8a96f393..1c182ebee 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -8,6 +8,7 @@ defmodule Pleroma.Config.TransferTask do alias Pleroma.Config alias Pleroma.ConfigDB alias Pleroma.Repo + alias Pleroma.Config.ConfigurableFromDatabase require Logger @@ -91,6 +92,18 @@ defp invalid_key_or_group(%ConfigDB{group: :invalid_atom}), do: true defp invalid_key_or_group(_), do: false defp merge_with_default(%{group: group, key: key, value: value} = setting) do + if !ConfigurableFromDatabase.whitelisted_config?(setting) do + Logger.warning(~s[ + config #{inspect(group)}, #{inspect(key)} is set in the database, + but it is not explicitly allowed to be there. Consider removing it + with + MIX: mix pleroma.config delete #{group} #{key} + OTP: ./bin/pleroma_ctl config delete #{group} #{key} + and setting it in your .exs file instead + config #{inspect(group)}, #{inspect(key)}, #{inspect(value)} + ]) + end + default = if group == :pleroma do Config.get([key], Config.Holder.default_config(group, key)) diff --git a/lib/pleroma/web/admin_api/controllers/config_controller.ex b/lib/pleroma/web/admin_api/controllers/config_controller.ex index 831ba3b6f..8b2401ef4 100644 --- a/lib/pleroma/web/admin_api/controllers/config_controller.ex +++ b/lib/pleroma/web/admin_api/controllers/config_controller.ex @@ -8,6 +8,7 @@ defmodule Pleroma.Web.AdminAPI.ConfigController do alias Pleroma.Config alias Pleroma.ConfigDB alias Pleroma.Web.Plugs.OAuthScopesPlug + alias Pleroma.Config.ConfigurableFromDatabase plug(Pleroma.Web.ApiSpec.CastAndValidate) plug(OAuthScopesPlug, %{scopes: ["admin:write"]} when action == :update) @@ -71,7 +72,11 @@ defp translate_children(item, _path) do end def descriptions(conn, _params) do - descriptions = Enum.filter(Pleroma.Docs.JSON.compiled_descriptions(), &whitelisted_config?/1) + descriptions = + Enum.filter( + Pleroma.Docs.JSON.compiled_descriptions(), + &ConfigurableFromDatabase.whitelisted_config?/1 + ) json(conn, translate_descriptions(descriptions)) end @@ -132,7 +137,7 @@ def update(%{body_params: %{configs: configs}} = conn, _) do with :ok <- configurable_from_database() do results = configs - |> Enum.filter(&whitelisted_config?/1) + |> Enum.filter(&ConfigurableFromDatabase.whitelisted_config?/1) |> Enum.map(fn %{group: group, key: key, delete: true} = params -> ConfigDB.delete(%{group: group, key: key, subkeys: params[:subkeys]}) @@ -167,32 +172,10 @@ def update(%{body_params: %{configs: configs}} = conn, _) do end defp configurable_from_database do - if Config.get(:configurable_from_database) do + if ConfigurableFromDatabase.enabled() do :ok else {:error, "You must enable configurable_from_database in your config file."} end end - - defp whitelisted_config?(group, key) do - if whitelisted_configs = Config.get(:database_config_whitelist) do - Enum.any?(whitelisted_configs, fn - {whitelisted_group} -> - group == inspect(whitelisted_group) - - {whitelisted_group, whitelisted_key} -> - group == inspect(whitelisted_group) && key == inspect(whitelisted_key) - end) - else - true - end - end - - defp whitelisted_config?(%{group: group, key: key}) do - whitelisted_config?(group, key) - end - - defp whitelisted_config?(%{group: group} = config) do - whitelisted_config?(group, config[:key]) - end end diff --git a/test/pleroma/config/transfer_task_test.exs b/test/pleroma/config/transfer_task_test.exs index 2cb9f2f4b..298e7459e 100644 --- a/test/pleroma/config/transfer_task_test.exs +++ b/test/pleroma/config/transfer_task_test.exs @@ -93,6 +93,31 @@ test "transfer config values with full subkey update" do assert assets_env[:mascots] == [a: 1, b: 2] end + test "transfer config values that are not explicitly whitelisted" do + insert(:config, key: :whoops, value: [not_allowed: true]) + + log = + capture_log(fn -> + TransferTask.start_link([]) + end) + + assert log =~ "config :pleroma, :whoops is set in the database" + assert log =~ "Consider removing it" + assert log =~ "mix pleroma.config delete pleroma whoops" + end + + test "transferring whitelisted values should not warn" do + insert(:config, key: :emoji, value: [allowed: true]) + insert(:config, key: Pleroma.Workers.PurgeExpiredActivity, value: [allowed: true]) + + log = + capture_log(fn -> + TransferTask.start_link([]) + end) + + refute log =~ "Consider removing it" + end + describe "pleroma restart" do setup do on_exit(fn -> diff --git a/test/pleroma/web/admin_api/controllers/config_controller_test.exs b/test/pleroma/web/admin_api/controllers/config_controller_test.exs new file mode 100644 index 000000000..5cb4ed17a --- /dev/null +++ b/test/pleroma/web/admin_api/controllers/config_controller_test.exs @@ -0,0 +1,69 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2021 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do + use Pleroma.Web.ConnCase, async: false + + import Pleroma.Factory + + setup_all do + Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) + + :ok + end + + setup do + clear_config(:configurable_from_database, true) + admin = insert(:user, is_admin: true) + token = insert(:oauth_admin_token, user: admin) + + conn = + build_conn() + |> assign(:user, admin) + |> assign(:token, token) + |> put_req_header("content-type", "application/json") + + {:ok, %{admin: admin, token: token, conn: conn}} + end + + describe "POST /api/v1/pleroma/admin/config" do + test "Refuses to update non-whitelisted config options", %{conn: conn} do + banned_config = %{ + configs: [ + %{ + group: ":mogrify", + key: ":mogrify_command", + value: [ + %{tuple: [":path", "sh"]}, + %{tuple: [":args", ["-c", "echo pwnd > /tmp/a"]]} + ] + }, + %{ + group: ":pleroma", + key: ":http", + value: [ + %{tuple: ["wow", "nice"]} + ] + } + ] + } + + resp = + conn + |> post(~p"/api/v1/pleroma/admin/config", banned_config) + |> json_response_and_validate_schema(200) + + # It should basically just throw out the mogrify option + assert Enum.count(resp["configs"]) == 1 + + assert %{ + "configs" => [ + %{ + "group" => ":pleroma" + } + ] + } = resp + end + end +end