FEATURE: allow plugins and themes to extend the default CSP (#6704)

FEATURE: allow plugins and themes to extend the default CSP (#6704)

  • FEATURE: allow plugins and themes to extend the default CSP

For plugins:

extend_content_security_policy(
  script_src: ['https://domain.com/script.js', 'https://your-cdn.com/'],
  style_src: ['https://domain.com/style.css']
)

For themes and components:

extend_content_security_policy:
  type: list
  default: "script_src:https://domain.com/|style_src:https://domain.com"
  • clear CSP base url before each test

we have a test that stubs Rails.env.development? to true

  • Only allow extending directives that core includes, for now
From 488fba3c5f747db7783424b05e1f4190bb494a2b Mon Sep 17 00:00:00 2001
From: Kyle Zhao <kzhao.sw@gmail.com>
Date: Fri, 30 Nov 2018 09:51:45 -0500
Subject: [PATCH] FEATURE: allow plugins and themes to extend the default CSP
 (#6704)

* FEATURE: allow plugins and themes to extend the default CSP

For plugins:

extend_content_security_policy(
script_src: [β€˜https://domain.com/script.js’, β€˜https://your-cdn.com/’],
style_src: [β€˜https://domain.com/style.css’]
)


For themes and components:

extend_content_security_policy:
type: list
default: β€œscript_src:https://domain.com/|style_src:https://domain.com”


* clear CSP base url before each test

we have a test that stubs `Rails.env.development?` to true

* Only allow extending directives that core includes, for now

diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index 01b5c4c..7b235f2 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -249,6 +249,7 @@ COMPILED
     theme.clear_cached_settings!
 
     Stylesheet::Manager.clear_theme_cache! if self.name.include?("scss")
+    CSP::Extension.clear_theme_extensions_cache! if name == 'yaml'
 
     # TODO message for mobile vs desktop
     MessageBus.publish "/header-change/#{theme.id}", self.value if theme && self.name == "header"
diff --git a/app/models/theme_setting.rb b/app/models/theme_setting.rb
index b851e7e..3616cb8 100644
--- a/app/models/theme_setting.rb
+++ b/app/models/theme_setting.rb
@@ -10,6 +10,7 @@ class ThemeSetting < ActiveRecord::Base
     theme.remove_from_cache!
     theme.theme_fields.update_all(value_baked: nil)
     SvgSprite.expire_cache if self.name.to_s.include?("_icon")
+    CSP::Extension.clear_theme_extensions_cache! if name.to_s == CSP::Extension::THEME_SETTING
   end
 
   def self.types
diff --git a/config/application.rb b/config/application.rb
index 692011e..89c161b 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -204,7 +204,7 @@ module Discourse
       config.middleware.insert_after Rack::MethodOverride, Middleware::EnforceHostname
     end
 
-    require 'content_security_policy'
+    require 'content_security_policy/middleware'
     config.middleware.swap ActionDispatch::ContentSecurityPolicy::Middleware, ContentSecurityPolicy::Middleware
 
     require 'middleware/discourse_public_exceptions'
diff --git a/lib/content_security_policy.rb b/lib/content_security_policy.rb
index 8e3ae2e..2f2b4c8 100644
--- a/lib/content_security_policy.rb
+++ b/lib/content_security_policy.rb
@@ -1,107 +1,28 @@
 # frozen_string_literal: true
-require_dependency 'global_path'
+require_dependency 'content_security_policy/builder'
+require_dependency 'content_security_policy/extension'
 
 class ContentSecurityPolicy
-  include GlobalPath
-
-  class Middleware
-    def initialize(app)
-      @app = app
+  class << self
+    def policy
+      new.build
     end
 
-    def call(env)
-      request = Rack::Request.new(env)
-      _, headers, _ = response = @app.call(env)
-
-      return response unless html_response?(headers) && ContentSecurityPolicy.enabled?
-
-      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
-
-      response
-    end
-
-    private
-
-    def html_response?(headers)
-      headers['Content-Type'] && headers['Content-Type'] =~ /html/
+    def base_url
+      @base_url || Discourse.base_url
     end
-  end
-
-  def self.enabled?
-    SiteSetting.content_security_policy || SiteSetting.content_security_policy_report_only
-  end
-
-  def initialize(request = nil)
-    @request = request
-    @directives = {
-      script_src: script_src,
-      worker_src: [:self, :blob],
-    }
-
-    @directives[:report_uri] = path('/csp_reports') if SiteSetting.content_security_policy_collect_reports
+    attr_writer :base_url
   end
 
   def build
-    policy = ActionDispatch::ContentSecurityPolicy.new
+    builder = Builder.new
 
-    @directives.each do |directive, sources|
-      if sources.is_a?(Array)
-        policy.public_send(directive, *sources)
-      else
-        policy.public_send(directive, sources)
-      end
-    end
+    Extension.theme_extensions.each { |extension| builder << extension }
+    Extension.plugin_extensions.each { |extension| builder << extension }
+    builder << Extension.site_setting_extension
 
-    policy.build
-  end
-
-  private
-
-  attr_reader :request
-
-  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],
-    ['/plugins/',            false, true],
-    ['/theme-javascripts/',  false, true],
-    ['/svg-sprite/',         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
+    builder.build
   end
 end
+
+CSP = ContentSecurityPolicy
diff --git a/lib/content_security_policy/builder.rb b/lib/content_security_policy/builder.rb
new file mode 100644
index 0000000..c108988
--- /dev/null
+++ b/lib/content_security_policy/builder.rb
@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+require_dependency 'content_security_policy/default'
+
+class ContentSecurityPolicy
+  class Builder
+    EXTENDABLE_DIRECTIVES = %i[
+      script_src
+      worker_src
+    ].freeze
+
+    # Make extending these directives no-op, until core includes them in default CSP
+    TO_BE_EXTENDABLE = %i[
+      base_uri
+      connect_src
+      default_src
+      font_src
+      form_action
+      frame_ancestors
+      frame_src
+      img_src
+      manifest_src
+      media_src
+      object_src
+      prefetch_src
+      style_src
+    ].freeze
+
+    def initialize
+      @directives = Default.new.directives
+    end
+
+    def <<(extension)
+      return unless valid_extension?(extension)
+
+      extension.each { |directive, sources| extend_directive(normalize(directive), sources) }
+    end
+
+    def build
+      policy = ActionDispatch::ContentSecurityPolicy.new
+
+      @directives.each do |directive, sources|
+        if sources.is_a?(Array)
+          policy.public_send(directive, *sources)
+        else
+          policy.public_send(directive, sources)
+        end
+      end
+
+      policy.build
+    end
+
+    private
+
+    def normalize(directive)
+      directive.to_s.gsub('-', '_').to_sym
+    end
+
+    def extend_directive(directive, sources)
+      return unless extendable?(directive)
+
+      @directives[directive] ||= []
+
+      if sources.is_a?(Array)
+        @directives[directive].concat(sources)
+      else
+        @directives[directive] << sources
+      end
+    end
+
+    def extendable?(directive)
+      EXTENDABLE_DIRECTIVES.include?(directive)
+    end
+
+    def valid_extension?(extension)
+      extension.is_a?(Hash)
+    end
+  end
+end
diff --git a/lib/content_security_policy/default.rb b/lib/cont

GitHub

1 Like