CSP: drop 'self' in `script-src` (#6611)

lib/content_security_policy.rb

@@ -15,7 +15,7 @@ class ContentSecurityPolicy
 
       return response unless html_response?(headers) && ContentSecurityPolicy.enabled?
 
-      policy = ContentSecurityPolicy.new.build
+      policy = ContentSecurityPolicy.new(request).build
       headers['Content-Security-Policy'] = policy if SiteSetting.content_security_policy
       headers['Content-Security-Policy-Report-Only'] = policy if SiteSetting.content_security_policy_report_only
 
@@ -33,7 +33,8 @@ class ContentSecurityPolicy
     SiteSetting.content_security_policy || SiteSetting.content_security_policy_report_only
   end
 
-  def initialize
+  def initialize(request = nil)
+    @request = request
     @directives = {
       script_src: script_src,
     }
@@ -57,14 +58,47 @@ class ContentSecurityPolicy
 
   private
 
-  def script_src
-    sources = [:self, :unsafe_eval]
+  attr_reader :request
 
-    sources << :https if SiteSetting.force_https
-    sources << Discourse.asset_host if Discourse.asset_host.present?
-    sources << 'www.google-analytics.com' if SiteSetting.ga_universal_tracking_code.present?
-    sources << 'www.googletagmanager.com' if SiteSetting.gtm_container_id.present?
+  SCRIPT_ASSET_DIRECTORIES = [
+    # [dir, can_use_s3_cdn, can_use_cdn]
+    ['/assets/',             true, true],
+    ['/brotli_asset/',       true, true],
+    ['/extra-locales/',      false, false],
+    ['/highlight-js/',       false, true],
+    ['/javascripts/',        false, true],
+    ['/theme-javascripts/',  false, true],
+  ]
+
+  def script_assets(base = base_url, s3_cdn = GlobalSetting.s3_cdn_url, cdn = GlobalSetting.cdn_url)
+    SCRIPT_ASSET_DIRECTORIES.map do |dir, can_use_s3_cdn, can_use_cdn|
+      if can_use_s3_cdn && s3_cdn
+        s3_cdn + dir
+      elsif can_use_cdn && cdn
+        cdn + dir
+      else
+        base + dir
+      end
+    end
+  end
+
+  def script_src
+    sources = [
+      :unsafe_eval,
+      "#{base_url}/logs/",
+      "#{base_url}/sidekiq/",
+      "#{base_url}/mini-profiler-resources/",
+    ]
+
+    sources.concat(script_assets)
+
+    sources << 'https://www.google-analytics.com' if SiteSetting.ga_universal_tracking_code.present?
+    sources << 'https://www.googletagmanager.com' if SiteSetting.gtm_container_id.present?
 
     sources.concat(SiteSetting.content_security_policy_script_src.split('|'))
   end
+
+  def base_url
+    @base_url ||= Rails.env.development? ? request.host_with_port : Discourse.base_url
+  end
 end

spec/lib/content_security_policy_spec.rb

@@ -14,16 +14,20 @@ describe ContentSecurityPolicy do
   end
 
   describe 'script-src defaults' do
-    it 'always have self and unsafe-eval' do
+    it 'always have self, logster, sidekiq, and assets' do
       script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
-      expect(script_srcs).to eq(%w['self' 'unsafe-eval'])
-    end
-
-    it 'enforces https when SiteSetting.force_https' do
-      SiteSetting.force_https = true
-
-      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
-      expect(script_srcs).to include('https:')
+      expect(script_srcs).to eq(%w[
+        'unsafe-eval'
+        http://test.localhost/logs/
+        http://test.localhost/sidekiq/
+        http://test.localhost/mini-profiler-resources/
+        http://test.localhost/assets/
+        http://test.localhost/brotli_asset/
+        http://test.localhost/extra-locales/
+        http://test.localhost/highlight-js/
+        http://test.localhost/javascripts/
+        http://test.localhost/theme-javascripts/
+      ])
     end
 
     it 'whitelists Google Analytics and Tag Manager when integrated' do
@@ -31,15 +35,34 @@ describe ContentSecurityPolicy do
       SiteSetting.gtm_container_id = 'GTM-ABCDEF'
 
       script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
-      expect(script_srcs).to include('www.google-analytics.com')
-      expect(script_srcs).to include('www.googletagmanager.com')
+      expect(script_srcs).to include('https://www.google-analytics.com')
+      expect(script_srcs).to include('https://www.googletagmanager.com')
     end
 
-    it 'whitelists CDN when integrated' do
-      set_cdn_url('cdn.com')
+    it 'whitelists CDN assets when integrated' do
+      set_cdn_url('https://cdn.com')
 
       script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
-      expect(script_srcs).to include('cdn.com')
+      expect(script_srcs).to include(*%w[
+        https://cdn.com/assets/
+        https://cdn.com/brotli_asset/
+        https://cdn.com/highlight-js/
+        https://cdn.com/javascripts/
+        https://cdn.com/theme-javascripts/
+        http://test.localhost/extra-locales/
+      ])
+
+      global_setting(:s3_cdn_url, 'https://s3-cdn.com')
+
+      script_srcs = parse(ContentSecurityPolicy.new.build)['script-src']
+      expect(script_srcs).to include(*%w[
+        https://s3-cdn.com/assets/
+        https://s3-cdn.com/brotli_asset/
+        https://cdn.com/highlight-js/
+        https://cdn.com/javascripts/
+        https://cdn.com/theme-javascripts/
+        http://test.localhost/extra-locales/
+      ])
     end
 
     it 'can be extended with more sources' do
@@ -48,7 +71,6 @@ describe ContentSecurityPolicy do
       expect(script_srcs).to include('example.com')
       expect(script_srcs).to include('another.com')
       expect(script_srcs).to include("'unsafe-eval'")
-      expect(script_srcs).to include("'self'")
     end
   end
 

spec/requests/application_controller_spec.rb

@@ -259,7 +259,6 @@ RSpec.describe ApplicationController do
       script_src = parse(response.headers['Content-Security-Policy'])['script-src']
 
       expect(script_src).to include('example.com')
-      expect(script_src).to include("'self'")
       expect(script_src).to include("'unsafe-eval'")
     end