diff --git a/app/controllers/csp_reports_controller.rb b/app/controllers/csp_reports_controller.rb new file mode 100644 index 00000000000..41dfc9502db --- /dev/null +++ b/app/controllers/csp_reports_controller.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true +class CspReportsController < ApplicationController + skip_before_action :check_xhr, :preload_json, :verify_authenticity_token, only: [:create] + + def create + raise Discourse::NotFound unless report_collection_enabled? + + Logster.add_to_env(request.env, 'CSP Report', report) + Rails.logger.warn("CSP Violation: '#{report['blocked-uri']}'") + + head :ok + end + + private + + def report + @report ||= params.require('csp-report').permit( + 'blocked-uri', + 'disposition', + 'document-uri', + 'effective-directive', + 'original-policy', + 'referrer', + 'script-sample', + 'status-code', + 'violated-directive', + 'line-number', + 'source-file' + ).to_h + end + + def report_collection_enabled? + ContentSecurityPolicy.enabled? && SiteSetting.content_security_policy_collect_reports + end +end diff --git a/config/application.rb b/config/application.rb index 640f05f5b71..a320d6dc366 100644 --- a/config/application.rb +++ b/config/application.rb @@ -190,6 +190,9 @@ module Discourse # supports etags (post 1.7) config.middleware.delete Rack::ETag + require 'content_security_policy' + config.middleware.swap ActionDispatch::ContentSecurityPolicy::Middleware, ContentSecurityPolicy::Middleware + require 'middleware/discourse_public_exceptions' config.exceptions_app = Middleware::DiscoursePublicExceptions.new(Rails.public_path) diff --git a/config/initializers/100-mime_types.rb b/config/initializers/100-mime_types.rb new file mode 100644 index 00000000000..676ee367e87 --- /dev/null +++ b/config/initializers/100-mime_types.rb @@ -0,0 +1 @@ +Mime::Type.register 'application/csp-report', :json diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c1ba5847b76..e8df322d682 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1267,6 +1267,10 @@ en: blacklisted_crawler_user_agents: 'Unique case insensitive word in the user agent string identifying web crawlers that should not be allowed to access the site. Does not apply if whitelist is defined.' slow_down_crawler_user_agents: 'User agents of web crawlers that should be rate limited in robots.txt using the Crawl-delay directive' slow_down_crawler_rate: 'If slow_down_crawler_user_agents is specified this rate will apply to all the crawlers (number of seconds delay between requests)' + content_security_policy: EXPERIMENTAL - Turn on Content-Security-Policy + content_security_policy_report_only: EXPERIMENTAL - Turn on Content-Security-Policy-Report-Only + content_security_policy_collect_reports: Enable CSP violation report collection at /csp_reports + content_security_policy_script_src: Additional whitelisted script sources. The current host and CDN are included by default. top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks" post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply" post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on." @@ -1868,7 +1872,7 @@ en: sso_provider_secrets: key: "www.example.com" value: "SSO secret" - + search: within_post: "#%{post_number} by %{username}" types: diff --git a/config/routes.rb b/config/routes.rb index 3d072a4691a..d3cebd11eed 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -828,6 +828,8 @@ Discourse::Application.routes.draw do post "/push_notifications/subscribe" => "push_notification#subscribe" post "/push_notifications/unsubscribe" => "push_notification#unsubscribe" + resources :csp_reports, only: [:create] + get "*url", to: 'permalinks#show', constraints: PermalinkConstraint.new end diff --git a/config/site_settings.yml b/config/site_settings.yml index 4af22bda68a..c6b5c718d82 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1179,6 +1179,15 @@ security: default: 'bingbot' list_type: compact slow_down_crawler_rate: 60 + content_security_policy: + default: false + content_security_policy_report_only: + default: false + content_security_policy_collect_reports: + default: true + content_security_policy_script_src: + type: list + default: '' onebox: enable_flash_video_onebox: false @@ -1801,4 +1810,4 @@ tags: remove_muted_tags_from_latest: default: false force_lowercase_tags: - default: true \ No newline at end of file + default: true diff --git a/lib/content_security_policy.rb b/lib/content_security_policy.rb new file mode 100644 index 00000000000..6feb85e0cd1 --- /dev/null +++ b/lib/content_security_policy.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true +require_dependency 'global_path' + +class ContentSecurityPolicy + include GlobalPath + + class Middleware + WHITELISTED_PATHS = %w( + /logs + ) + + 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.enabled? + return response if whitelisted?(request.path) + + policy = ContentSecurityPolicy.new.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/ + end + + def whitelisted?(path) + if GlobalSetting.relative_url_root + path.slice!(/^#{Regexp.quote(GlobalSetting.relative_url_root)}/) + end + + WHITELISTED_PATHS.any? { |whitelisted| path.start_with?(whitelisted) } + end + end + + def self.enabled? + SiteSetting.content_security_policy || SiteSetting.content_security_policy_report_only + end + + def initialize + @directives = { + script_src: script_src, + } + + @directives[:report_uri] = path('/csp_reports') if SiteSetting.content_security_policy_collect_reports + 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 script_src + sources = [:self, :unsafe_eval] + + 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? + + sources.concat(SiteSetting.content_security_policy_script_src.split('|')) + end +end diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb new file mode 100644 index 00000000000..dd26fb2d94d --- /dev/null +++ b/spec/lib/content_security_policy_spec.rb @@ -0,0 +1,61 @@ +require 'rails_helper' + +describe ContentSecurityPolicy do + 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') + + SiteSetting.content_security_policy_collect_reports = false + report_uri = parse(ContentSecurityPolicy.new.build)['report-uri'] + expect(report_uri).to eq(nil) + end + end + + describe 'script-src defaults' do + it 'always have self and unsafe-eval' 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:') + end + + it 'whitelists Google Analytics and Tag Manager when integrated' do + SiteSetting.ga_universal_tracking_code = 'UA-12345678-9' + 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') + end + + it 'whitelists CDN when integrated' do + set_cdn_url('cdn.com') + + script_srcs = parse(ContentSecurityPolicy.new.build)['script-src'] + expect(script_srcs).to include('cdn.com') + 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'") + expect(script_srcs).to include("'self'") + 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/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 03c5488f119..13338f06664 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -196,4 +196,69 @@ RSpec.describe ApplicationController do expect(controller.theme_ids).to eq([theme.id]) end end + + describe 'Content Security Policy' do + it 'is enabled by SiteSettings' do + SiteSetting.content_security_policy = false + SiteSetting.content_security_policy_report_only = false + + get '/' + + expect(response.headers).to_not include('Content-Security-Policy') + expect(response.headers).to_not include('Content-Security-Policy-Report-Only') + + SiteSetting.content_security_policy = true + SiteSetting.content_security_policy_report_only = true + + get '/' + + expect(response.headers).to include('Content-Security-Policy') + expect(response.headers).to include('Content-Security-Policy-Report-Only') + end + + it 'can be customized with SiteSetting' do + SiteSetting.content_security_policy = true + + get '/' + script_src = parse(response.headers['Content-Security-Policy'])['script-src'] + + expect(script_src).to_not include('example.com') + + SiteSetting.content_security_policy_script_src = 'example.com' + + get '/' + 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 + + it 'does not set CSP when responding to non-HTML' do + SiteSetting.content_security_policy = true + SiteSetting.content_security_policy_report_only = true + + get '/latest.json' + + expect(response.headers).to_not include('Content-Security-Policy') + expect(response.headers).to_not include('Content-Security-Policy-Report-Only') + end + + it 'does not set CSP for /logs' do + sign_in(Fabricate(:admin)) + SiteSetting.content_security_policy = true + + get '/logs' + + expect(response.status).to eq(200) + expect(response.headers).to_not include('Content-Security-Policy') + end + + def parse(csp_string) + csp_string.split(';').map do |policy| + directive, *sources = policy.split + [directive, sources] + end.to_h + end + end end diff --git a/spec/requests/csp_reports_controller_spec.rb b/spec/requests/csp_reports_controller_spec.rb new file mode 100644 index 00000000000..1fd6b238760 --- /dev/null +++ b/spec/requests/csp_reports_controller_spec.rb @@ -0,0 +1,56 @@ +require 'rails_helper' + +describe CspReportsController do + describe '#create' do + before do + SiteSetting.content_security_policy = true + SiteSetting.content_security_policy_collect_reports = true + + @orig_logger = Rails.logger + Rails.logger = @fake_logger = FakeLogger.new + end + + after do + Rails.logger = @orig_logger + end + + def send_report + post '/csp_reports', params: { + "csp-report": { + "document-uri": "http://localhost:3000/", + "referrer": "", + "violated-directive": "script-src", + "effective-directive": "script-src", + "original-policy": "script-src 'unsafe-eval' www.google-analytics.com; report-uri /csp_reports", + "disposition": "report", + "blocked-uri": "http://suspicio.us/assets.js", + "line-number": 25, + "source-file": "http://localhost:3000/", + "status-code": 200, + "script-sample": "" + }, headers: { "Content-Type": "application/csp-report" } + } + end + + it 'is enabled by SiteSetting' do + SiteSetting.content_security_policy = false + SiteSetting.content_security_policy_report_only = false + SiteSetting.content_security_policy_collect_reports = true + send_report + expect(response.status).to eq(404) + + SiteSetting.content_security_policy = true + send_report + expect(response.status).to eq(200) + + SiteSetting.content_security_policy_collect_reports = false + send_report + expect(response.status).to eq(404) + end + + it 'logs the violation report' do + send_report + expect(Rails.logger.warnings).to include("CSP Violation: 'http://suspicio.us/assets.js'") + end + end +end diff --git a/spec/support/fake_logger.rb b/spec/support/fake_logger.rb index 416de400dcd..5a4c9f2acc1 100644 --- a/spec/support/fake_logger.rb +++ b/spec/support/fake_logger.rb @@ -1,9 +1,14 @@ class FakeLogger - attr_reader :warnings, :errors + attr_reader :warnings, :errors, :infos def initialize @warnings = [] @errors = [] + @infos = [] + end + + def info(message = nil) + @infos << message end def warn(message)