From eaa59daa4c229bf47e30ac389563c82b11378e07 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 3 Jul 2020 17:06:20 -0500 Subject: [PATCH 1/6] Add Captcha endpoint to CSP headers when MediaProxy is enabled. Our CSP rules are lax when MediaProxy enabled, but lenient otherwise. This fixes broken captcha on instances not using MediaProxy. --- lib/pleroma/plugs/http_security_plug.ex | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 1420a9611..f7192ebfc 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -125,11 +125,19 @@ defp get_proxy_and_attachment_sources do if Config.get([Pleroma.Upload, :uploader]) == Pleroma.Uploaders.S3, do: URI.parse(Config.get([Pleroma.Uploaders.S3, :public_endpoint])).host + captcha_method = Config.get([Pleroma.Captcha, :method]) + + captcha_endpoint = + if Config.get([Pleroma.Captcha, :enabled]) && + captcha_method != "Pleroma.Captcha.Native", + do: Config.get([captcha_method, :endpoint]) + [] |> add_source(media_proxy_base_url) |> add_source(upload_base_url) |> add_source(s3_endpoint) |> add_source(media_proxy_whitelist) + |> add_source(captcha_endpoint) end defp add_source(iodata, nil), do: iodata From e9a28078ad969204faae600df3ddff8e75ed2f8a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 3 Jul 2020 17:18:22 -0500 Subject: [PATCH 2/6] Rename function and clarify that CSP is only strict with MediaProxy enabled --- lib/pleroma/plugs/http_security_plug.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index f7192ebfc..23a641faf 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -69,10 +69,11 @@ defp csp_string do img_src = "img-src 'self' data: blob:" media_src = "media-src 'self'" + # Strict multimedia CSP enforcement only when MediaProxy is enabled {img_src, media_src} = if Config.get([:media_proxy, :enabled]) && !Config.get([:media_proxy, :proxy_opts, :redirect_on_failure]) do - sources = get_proxy_and_attachment_sources() + sources = build_csp_multimedia_source_list() {[img_src, sources], [media_src, sources]} else {[img_src, " https:"], [media_src, " https:"]} @@ -107,7 +108,7 @@ defp csp_string do |> :erlang.iolist_to_binary() end - defp get_proxy_and_attachment_sources do + defp build_csp_multimedia_source_list do media_proxy_whitelist = Enum.reduce(Config.get([:media_proxy, :whitelist]), [], fn host, acc -> add_source(acc, host) From 991bd78ddad74641f8032c7b373771a5acb10da9 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 3 Jul 2020 17:19:43 -0500 Subject: [PATCH 3/6] Document the Captcha CSP fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85401809a..4b74d064c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Resolving Peertube accounts with Webfinger - `blob:` urls not being allowed by connect-src CSP - Mastodon API: fix `GET /api/v1/notifications` not returning the full result set +- Fix CSP policy generation to include remote Captcha services ## [Unreleased (patch)] From af612bd006a2792e27f9b995c0c86e010cc77e6c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sun, 5 Jul 2020 10:11:43 -0500 Subject: [PATCH 4/6] Ensure all CSP parameters for remote hosts have a scheme --- lib/pleroma/plugs/http_security_plug.ex | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 23a641faf..3bf0b8ce7 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -116,22 +116,22 @@ defp build_csp_multimedia_source_list do media_proxy_base_url = if Config.get([:media_proxy, :base_url]), - do: URI.parse(Config.get([:media_proxy, :base_url])).host + do: build_csp_param(Config.get([:media_proxy, :base_url])) upload_base_url = if Config.get([Pleroma.Upload, :base_url]), - do: URI.parse(Config.get([Pleroma.Upload, :base_url])).host + do: build_csp_param(Config.get([Pleroma.Upload, :base_url])) s3_endpoint = if Config.get([Pleroma.Upload, :uploader]) == Pleroma.Uploaders.S3, - do: URI.parse(Config.get([Pleroma.Uploaders.S3, :public_endpoint])).host + do: build_csp_param(Config.get([Pleroma.Uploaders.S3, :public_endpoint])) captcha_method = Config.get([Pleroma.Captcha, :method]) captcha_endpoint = if Config.get([Pleroma.Captcha, :enabled]) && captcha_method != "Pleroma.Captcha.Native", - do: Config.get([captcha_method, :endpoint]) + do: build_csp_param(Config.get([captcha_method, :endpoint])) [] |> add_source(media_proxy_base_url) @@ -148,6 +148,14 @@ defp add_csp_param(csp_iodata, nil), do: csp_iodata defp add_csp_param(csp_iodata, param), do: [[param, ?;] | csp_iodata] + defp build_csp_param(url) when is_binary(url) do + %{host: host, scheme: scheme} = URI.parse(url) + + if scheme do + scheme <> "://" <> host + end + end + def warn_if_disabled do unless Config.get([:http_security, :enabled]) do Logger.warn(" From 65843d92c4d3999f727d00500dcf8943cfe7bbc0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 6 Jul 2020 10:59:41 -0500 Subject: [PATCH 5/6] Simplify the logic --- lib/pleroma/plugs/http_security_plug.ex | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 3bf0b8ce7..13423ca3f 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -114,24 +114,15 @@ defp build_csp_multimedia_source_list do add_source(acc, host) end) - media_proxy_base_url = - if Config.get([:media_proxy, :base_url]), - do: build_csp_param(Config.get([:media_proxy, :base_url])) + media_proxy_base_url = build_csp_param(Config.get([:media_proxy, :base_url])) - upload_base_url = - if Config.get([Pleroma.Upload, :base_url]), - do: build_csp_param(Config.get([Pleroma.Upload, :base_url])) + upload_base_url = build_csp_param(Config.get([Pleroma.Upload, :base_url])) - s3_endpoint = - if Config.get([Pleroma.Upload, :uploader]) == Pleroma.Uploaders.S3, - do: build_csp_param(Config.get([Pleroma.Uploaders.S3, :public_endpoint])) + s3_endpoint = build_csp_param(Config.get([Pleroma.Uploaders.S3, :public_endpoint])) captcha_method = Config.get([Pleroma.Captcha, :method]) - captcha_endpoint = - if Config.get([Pleroma.Captcha, :enabled]) && - captcha_method != "Pleroma.Captcha.Native", - do: build_csp_param(Config.get([captcha_method, :endpoint])) + captcha_endpoint = build_csp_param(Config.get([captcha_method, :endpoint])) [] |> add_source(media_proxy_base_url) @@ -148,6 +139,8 @@ defp add_csp_param(csp_iodata, nil), do: csp_iodata defp add_csp_param(csp_iodata, param), do: [[param, ?;] | csp_iodata] + defp build_csp_param(nil), do: nil + defp build_csp_param(url) when is_binary(url) do %{host: host, scheme: scheme} = URI.parse(url) From da4029391d217b1e2c151e69479de42e2221e02f Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 6 Jul 2020 11:28:08 -0500 Subject: [PATCH 6/6] IO list, not concatenation --- lib/pleroma/plugs/http_security_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 13423ca3f..472a3ff42 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -145,7 +145,7 @@ defp build_csp_param(url) when is_binary(url) do %{host: host, scheme: scheme} = URI.parse(url) if scheme do - scheme <> "://" <> host + [scheme, "://", host] end end