Explicitly allow configDB keys #628

Closed
floatingghost wants to merge 9 commits from config-db-keys into develop
10 changed files with 297 additions and 71 deletions

View file

@ -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

View file

@ -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

View file

@ -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"
]
}
]
],
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,

View file

@ -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 <MY MODULE>
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`).

View file

@ -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})

View file

@ -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
Review

The first line of this comment makes sense in the scope of this PR, but not for someone looking at the code as-is. I would remove that line.

I'm not sure what's meant in the last line with "our stuff", so maybe that should also be removed, or clarified?

The first line of this comment makes sense in the scope of this PR, but not for someone looking at the code as-is. I would remove that line. I'm not sure what's meant in the last line with "our stuff", so maybe that should also be removed, or clarified?
@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

View file

@ -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))

View file

@ -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

View file

@ -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 ->

View file

@ -0,0 +1,69 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2021 Pleroma Authors <https://pleroma.social/>
# 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