diff --git a/app/controllers/embed_controller.rb b/app/controllers/embed_controller.rb
index fd4702ba3a7..fc5ae4756e7 100644
--- a/app/controllers/embed_controller.rb
+++ b/app/controllers/embed_controller.rb
@@ -12,7 +12,7 @@ class EmbedController < ApplicationController
layout 'embed'
rescue_from Discourse::InvalidAccess do
- response.headers['X-Frame-Options'] = "ALLOWALL"
+ response.headers.delete('X-Frame-Options')
if current_user.try(:admin?)
@setup_url = "#{Discourse.base_url}/admin/customize/embedding"
@show_reason = true
@@ -24,7 +24,7 @@ class EmbedController < ApplicationController
def topics
discourse_expires_in 1.minute
- response.headers['X-Frame-Options'] = "ALLOWALL"
+ response.headers.delete('X-Frame-Options')
unless SiteSetting.embed_topics_list?
render 'embed_topics_error', status: 400
return
@@ -157,7 +157,7 @@ class EmbedController < ApplicationController
end
end
- response.headers['X-Frame-Options'] = "ALLOWALL"
+ response.headers.delete('X-Frame-Options')
rescue URI::Error
raise Discourse::InvalidAccess.new('invalid referer host')
end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index fbed6eee272..cb3a7ca3e1e 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1602,6 +1602,7 @@ en:
content_security_policy: "Enable Content-Security-Policy"
content_security_policy_report_only: "Enable Content-Security-Policy-Report-Only"
content_security_policy_collect_reports: "Enable CSP violation report collection at /csp_reports"
+ content_security_policy_frame_ancestors: "Restrict who can embed this site in iframes via CSP. Control allowed hosts on Embedding"
content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See Mitigate XSS Attacks with Content Security Policy."
invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable."
top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks"
diff --git a/config/site_settings.yml b/config/site_settings.yml
index c3c2801e1ff..33bd1327e4b 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1593,6 +1593,8 @@ security:
default: false
content_security_policy_collect_reports:
default: false
+ content_security_policy_frame_ancestors:
+ default: false
content_security_policy_script_src:
type: simple_list
default: ""
diff --git a/lib/content_security_policy/default.rb b/lib/content_security_policy/default.rb
index 1d5315709eb..6826de68e82 100644
--- a/lib/content_security_policy/default.rb
+++ b/lib/content_security_policy/default.rb
@@ -13,6 +13,7 @@ class ContentSecurityPolicy
directives[:script_src] = script_src
directives[:worker_src] = worker_src
directives[:report_uri] = report_uri if SiteSetting.content_security_policy_collect_reports
+ directives[:frame_ancestors] = frame_ancestors if restrict_embed?
end
end
@@ -73,5 +74,17 @@ class ContentSecurityPolicy
def report_uri
"#{base_url}/csp_reports"
end
+
+ def frame_ancestors
+ [
+ "'self'",
+ *EmbeddableHost.pluck(:host).map { |host| "https://#{host}" }
+ ]
+ end
+
+ def restrict_embed?
+ SiteSetting.content_security_policy_frame_ancestors &&
+ !SiteSetting.embed_any_origin
+ end
end
end
diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb
index f4a73c6bbc9..3f7b6ac12e3 100644
--- a/spec/lib/content_security_policy_spec.rb
+++ b/spec/lib/content_security_policy_spec.rb
@@ -154,6 +154,39 @@ describe ContentSecurityPolicy do
end
end
+ describe 'frame-ancestors' do
+ context 'with content_security_policy_frame_ancestors enabled' do
+ before do
+ SiteSetting.content_security_policy_frame_ancestors = true
+ Fabricate(:embeddable_host, host: 'https://a.org')
+ Fabricate(:embeddable_host, host: 'https://b.org')
+ end
+
+ it 'always has self' do
+ frame_ancestors = parse(policy)['frame-ancestors']
+ expect(frame_ancestors).to include("'self'")
+ end
+
+ it 'includes all EmbeddableHost' do
+ EmbeddableHost
+ frame_ancestors = parse(policy)['frame-ancestors']
+ expect(frame_ancestors).to include("https://a.org")
+ expect(frame_ancestors).to include("https://b.org")
+ end
+ end
+
+ context 'with content_security_policy_frame_ancestors disabled' do
+ before do
+ SiteSetting.content_security_policy_frame_ancestors = false
+ end
+
+ it 'does not set frame-ancestors' do
+ frame_ancestors = parse(policy)['frame-ancestors']
+ expect(frame_ancestors).to be_nil
+ end
+ end
+ end
+
it 'can be extended by plugins' do
plugin = Class.new(Plugin::Instance) do
attr_accessor :enabled
diff --git a/spec/requests/embed_controller_spec.rb b/spec/requests/embed_controller_spec.rb
index 62e42b1d819..9d476ed3a1c 100644
--- a/spec/requests/embed_controller_spec.rb
+++ b/spec/requests/embed_controller_spec.rb
@@ -94,7 +94,7 @@ describe EmbedController do
'REFERER' => 'https://example.com/evil-trout'
}
expect(response.status).to eq(200)
- expect(response.headers['X-Frame-Options']).to eq("ALLOWALL")
+ expect(response.headers['X-Frame-Options']).to be_nil
expect(response.body).to match("data-embed-id=\"de-1234\"")
expect(response.body).to match("data-topic-id=\"#{topic.id}\"")
expect(response.body).to match("data-referer=\"https://example.com/evil-trout\"")
@@ -157,7 +157,7 @@ describe EmbedController do
context "success" do
after do
expect(response.status).to eq(200)
- expect(response.headers['X-Frame-Options']).to eq("ALLOWALL")
+ expect(response.headers['X-Frame-Options']).to be_nil
end
it "tells the topic retriever to work when no previous embed is found" do
@@ -249,5 +249,21 @@ describe EmbedController do
expect(response.body).to match(I18n.t('embed.error'))
end
end
+
+ context "CSP frame-ancestors enabled" do
+ before do
+ SiteSetting.content_security_policy_frame_ancestors = true
+ end
+
+ it "includes all the hosts" do
+ get '/embed/comments',
+ params: { embed_url: embed_url },
+ headers: { 'REFERER' => "http://eviltrout.com/wat/1-2-3.html" }
+
+ expect(response.headers['Content-Security-Policy']).to match(/frame-ancestors.*https:\/\/discourse\.org/)
+ expect(response.headers['Content-Security-Policy']).to match(/frame-ancestors.*https:\/\/example\.com/)
+ end
+ end
+
end
end