diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index 01b5c4c9fc2..7b235f281ee 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 b851e7e4bda..3616cb8a716 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 692011edb9a..89c161bbfe0 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 8e3ae2e31bc..2f2b4c87a1b 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 + def base_url + @base_url || Discourse.base_url end - - private - - def html_response?(headers) - headers['Content-Type'] && headers['Content-Type'] =~ /html/ - 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 00000000000..c1089889901 --- /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/content_security_policy/default.rb new file mode 100644 index 00000000000..46a543f7ed2 --- /dev/null +++ b/lib/content_security_policy/default.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true +require_dependency 'content_security_policy' + +class ContentSecurityPolicy + class Default + attr_reader :directives + + def initialize + @directives = {}.tap do |directives| + directives[:script_src] = script_src + directives[:worker_src] = worker_src + directives[:report_uri] = report_uri if SiteSetting.content_security_policy_collect_reports + end + end + + private + + delegate :base_url, to: :ContentSecurityPolicy + + 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 + [ + :unsafe_eval, + "#{base_url}/logs/", + "#{base_url}/sidekiq/", + "#{base_url}/mini-profiler-resources/", + *script_assets + ].tap do |sources| + sources << 'https://www.google-analytics.com' if SiteSetting.ga_universal_tracking_code.present? + sources << 'https://www.googletagmanager.com' if SiteSetting.gtm_container_id.present? + end + end + + def worker_src + [ + :self, + :blob, # ACE editor registers a service worker with a blob for syntax checking + ] + end + + def report_uri + "#{base_url}/csp_reports" + end + end +end diff --git a/lib/content_security_policy/extension.rb b/lib/content_security_policy/extension.rb new file mode 100644 index 00000000000..cc319861558 --- /dev/null +++ b/lib/content_security_policy/extension.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true +class ContentSecurityPolicy + module Extension + extend self + + def site_setting_extension + { script_src: SiteSetting.content_security_policy_script_src.split('|') } + end + + def plugin_extensions + [].tap do |extensions| + Discourse.plugins.each do |plugin| + extensions.concat(plugin.csp_extensions) if plugin.enabled? + end + end + end + + THEME_SETTING = 'extend_content_security_policy' + + def theme_extensions + cache['theme_extensions'] ||= find_theme_extensions + end + + def clear_theme_extensions_cache! + cache['theme_extensions'] = nil + end + + private + + def cache + @cache ||= DistributedCache.new('csp_extensions') + end + + def find_theme_extensions + extensions = [] + + Theme.find_each do |theme| + theme.cached_settings.each do |setting, value| + extensions << build_theme_extension(value) if setting.to_s == THEME_SETTING + end + end + + extensions + end + + def build_theme_extension(raw) + {}.tap do |extension| + raw.split('|').each do |entry| + directive, source = entry.split(':', 2).map(&:strip) + + extension[directive] ||= [] + extension[directive] << source + end + end + end + end +end diff --git a/lib/content_security_policy/middleware.rb b/lib/content_security_policy/middleware.rb new file mode 100644 index 00000000000..d407ce0146b --- /dev/null +++ b/lib/content_security_policy/middleware.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +require_dependency 'content_security_policy' + +class ContentSecurityPolicy + class Middleware + def initialize(app) + @app = app + end + + def call(env) + request = Rack::Request.new(env) + _, headers, _ = response = @app.call(env) + + return response unless html_response?(headers) + + ContentSecurityPolicy.base_url = request.host_with_port if Rails.env.development? + + 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 + + delegate :policy, to: :ContentSecurityPolicy + + def html_response?(headers) + headers['Content-Type'] && headers['Content-Type'] =~ /html/ + end + end +end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 2d076b3737a..b42377757d9 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -32,7 +32,9 @@ class Plugin::Instance :locales, :service_workers, :styles, - :themes].each do |att| + :themes, + :csp_extensions, + ].each do |att| class_eval %Q{ def #{att} @#{att} ||= [] @@ -361,6 +363,10 @@ class Plugin::Instance DiscoursePluginRegistry.register_svg_icon(icon) end + def extend_content_security_policy(extension) + csp_extensions << extension + end + # @option opts [String] :name # @option opts [String] :nativeName # @option opts [String] :fallbackLocale diff --git a/spec/components/plugin/instance_spec.rb b/spec/components/plugin/instance_spec.rb index 61accafbf48..0153fd8b883 100644 --- a/spec/components/plugin/instance_spec.rb +++ b/spec/components/plugin/instance_spec.rb @@ -10,8 +10,8 @@ describe Plugin::Instance do context "find_all" do it "can find plugins correctly" do plugins = Plugin::Instance.find_all("#{Rails.root}/spec/fixtures/plugins") - expect(plugins.count).to eq(2) - plugin = plugins[1] + expect(plugins.count).to eq(3) + plugin = plugins[2] expect(plugin.name).to eq("plugin-name") expect(plugin.path).to eq("#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb") diff --git a/spec/fixtures/plugins/csp_extension/plugin.rb b/spec/fixtures/plugins/csp_extension/plugin.rb new file mode 100644 index 00000000000..f66915a06b8 --- /dev/null +++ b/spec/fixtures/plugins/csp_extension/plugin.rb @@ -0,0 +1,8 @@ +# name: csp_extension +# about: Fixture plugin that extends default CSP +# version: 1.0 +# authors: xrav3nz + +extend_content_security_policy( + script_src: ['https://from-plugin.com'] +) diff --git a/spec/lib/content_security_policy/builder_spec.rb b/spec/lib/content_security_policy/builder_spec.rb new file mode 100644 index 00000000000..f188dbcc4ce --- /dev/null +++ b/spec/lib/content_security_policy/builder_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true +require 'rails_helper' + +describe ContentSecurityPolicy::Builder do + let(:builder) { described_class.new } + + describe '#<<' do + it 'normalizes directive name' do + builder << { + script_src: ['symbol_underscore'], + 'script-src': ['symbol_dash'], + 'script_src' => ['string_underscore'], + 'script-src' => ['string_dash'], + } + + script_srcs = parse(builder.build)['script-src'] + + expect(script_srcs).to include(*%w[symbol_underscore symbol_dash string_underscore symbol_underscore]) + end + + it 'rejects invalid directives and ones that are not allowed to be extended' do + builder << { + invalid_src: ['invalid'], + } + + expect(builder.build).to_not include('invalid') + end + + it 'no-ops on invalid values' do + previous = builder.build + + builder << nil + builder << 123 + builder << "string" + builder << [] + builder << {} + + expect(builder.build).to eq(previous) + end + end + + def parse(csp_string) + csp_string.split(';').map do |policy| + directive, *sources = policy.split + [directive, sources] + end.to_h + end +end diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb index 41976a31f73..ba625860f1e 100644 --- a/spec/lib/content_security_policy_spec.rb +++ b/spec/lib/content_security_policy_spec.rb @@ -1,21 +1,24 @@ +# frozen_string_literal: true require 'rails_helper' describe ContentSecurityPolicy do + before { ContentSecurityPolicy.base_url = nil } + describe 'report-uri' do it 'is enabled by SiteSetting' do SiteSetting.content_security_policy_collect_reports = true - report_uri = parse(ContentSecurityPolicy.new.build)['report-uri'].first - expect(report_uri).to eq('/csp_reports') + report_uri = parse(policy)['report-uri'].first + expect(report_uri).to eq('http://test.localhost/csp_reports') SiteSetting.content_security_policy_collect_reports = false - report_uri = parse(ContentSecurityPolicy.new.build)['report-uri'] + report_uri = parse(policy)['report-uri'] expect(report_uri).to eq(nil) end end describe 'worker-src' do it 'always has self and blob' do - worker_srcs = parse(ContentSecurityPolicy.new.build)['worker-src'] + worker_srcs = parse(policy)['worker-src'] expect(worker_srcs).to eq(%w[ 'self' blob: @@ -25,8 +28,8 @@ describe ContentSecurityPolicy do describe 'script-src' do it 'always has self, logster, sidekiq, and assets' do - script_srcs = parse(ContentSecurityPolicy.new.build)['script-src'] - expect(script_srcs).to eq(%w[ + script_srcs = parse(policy)['script-src'] + expect(script_srcs).to include(*%w[ 'unsafe-eval' http://test.localhost/logs/ http://test.localhost/sidekiq/ @@ -46,7 +49,7 @@ describe ContentSecurityPolicy do SiteSetting.ga_universal_tracking_code = 'UA-12345678-9' SiteSetting.gtm_container_id = 'GTM-ABCDEF' - script_srcs = parse(ContentSecurityPolicy.new.build)['script-src'] + script_srcs = parse(policy)['script-src'] expect(script_srcs).to include('https://www.google-analytics.com') expect(script_srcs).to include('https://www.googletagmanager.com') end @@ -54,7 +57,7 @@ describe ContentSecurityPolicy do it 'whitelists CDN assets when integrated' do set_cdn_url('https://cdn.com') - script_srcs = parse(ContentSecurityPolicy.new.build)['script-src'] + script_srcs = parse(policy)['script-src'] expect(script_srcs).to include(*%w[ https://cdn.com/assets/ https://cdn.com/brotli_asset/ @@ -67,7 +70,7 @@ describe ContentSecurityPolicy do global_setting(:s3_cdn_url, 'https://s3-cdn.com') - script_srcs = parse(ContentSecurityPolicy.new.build)['script-src'] + script_srcs = parse(policy)['script-src'] expect(script_srcs).to include(*%w[ https://s3-cdn.com/assets/ https://s3-cdn.com/brotli_asset/ @@ -78,14 +81,57 @@ describe ContentSecurityPolicy do http://test.localhost/extra-locales/ ]) end + end - it 'can be extended with more sources' do - SiteSetting.content_security_policy_script_src = 'example.com|another.com' - script_srcs = parse(ContentSecurityPolicy.new.build)['script-src'] - expect(script_srcs).to include('example.com') - expect(script_srcs).to include('another.com') - expect(script_srcs).to include("'unsafe-eval'") - end + it 'can be extended by plugins' do + plugin = Class.new(Plugin::Instance) do + attr_accessor :enabled + def enabled?; @enabled; end + end.new(nil, "#{Rails.root}/spec/fixtures/plugins/csp_extension/plugin.rb") + + plugin.activate! + Discourse.plugins << plugin + + plugin.enabled = true + expect(parse(policy)['script-src']).to include('https://from-plugin.com') + + plugin.enabled = false + expect(parse(policy)['script-src']).to_not include('https://from-plugin.com') + + Discourse.plugins.pop + end + + it 'can be extended by themes' do + policy # call this first to make sure further actions clear the cache + + theme = Fabricate(:theme) + settings = <<~YML + extend_content_security_policy: + type: list + default: 'script-src: from-theme.com' + YML + theme.set_field(target: :settings, name: :yaml, value: settings) + theme.save! + + expect(parse(policy)['script-src']).to include('from-theme.com') + + theme.update_setting(:extend_content_security_policy, "script-src: https://from-theme.net|worker-src: from-theme.com") + theme.save! + + expect(parse(policy)['script-src']).to_not include('from-theme.com') + expect(parse(policy)['script-src']).to include('https://from-theme.net') + expect(parse(policy)['worker-src']).to include('from-theme.com') + + theme.destroy! + + expect(parse(policy)['script-src']).to_not include('https://from-theme.net') + expect(parse(policy)['worker-src']).to_not include('from-theme.com') + end + + it 'can be extended by site setting' do + SiteSetting.content_security_policy_script_src = 'from-site-setting.com|from-site-setting.net' + + expect(parse(policy)['script-src']).to include('from-site-setting.com', 'from-site-setting.net') end def parse(csp_string) @@ -94,4 +140,8 @@ describe ContentSecurityPolicy do [directive, sources] end.to_h end + + def policy + ContentSecurityPolicy.policy + end end