FEATURE: Add CSP frame-ancestors support (#12404)

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

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 <a href='%{base_path}/admin/customize/embedding'>Embedding</a>"
     content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See <a href='https://meta.discourse.org/t/mitigate-xss-attacks-with-content-security-policy/104243' target='_blank'>Mitigate XSS Attacks with Content Security Policy.</a>"
     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"

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: ""

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

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

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