diff --git a/app/controllers/csp_reports_controller.rb b/app/controllers/csp_reports_controller.rb deleted file mode 100644 index 41dfc9502db..00000000000 --- a/app/controllers/csp_reports_controller.rb +++ /dev/null @@ -1,35 +0,0 @@ -# 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 a320d6dc366..640f05f5b71 100644 --- a/config/application.rb +++ b/config/application.rb @@ -190,9 +190,6 @@ 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 deleted file mode 100644 index 676ee367e87..00000000000 --- a/config/initializers/100-mime_types.rb +++ /dev/null @@ -1 +0,0 @@ -Mime::Type.register 'application/csp-report', :json diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e8df322d682..c1ba5847b76 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1267,10 +1267,6 @@ 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." @@ -1872,7 +1868,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 d3cebd11eed..3d072a4691a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -828,8 +828,6 @@ 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 c6b5c718d82..4af22bda68a 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1179,15 +1179,6 @@ 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 @@ -1810,4 +1801,4 @@ tags: remove_muted_tags_from_latest: default: false force_lowercase_tags: - default: true + default: true \ No newline at end of file diff --git a/lib/content_security_policy.rb b/lib/content_security_policy.rb deleted file mode 100644 index 6feb85e0cd1..00000000000 --- a/lib/content_security_policy.rb +++ /dev/null @@ -1,83 +0,0 @@ -# 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 deleted file mode 100644 index dd26fb2d94d..00000000000 --- a/spec/lib/content_security_policy_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -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 13338f06664..03c5488f119 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -196,69 +196,4 @@ 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 deleted file mode 100644 index 1fd6b238760..00000000000 --- a/spec/requests/csp_reports_controller_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -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 5a4c9f2acc1..416de400dcd 100644 --- a/spec/support/fake_logger.rb +++ b/spec/support/fake_logger.rb @@ -1,14 +1,9 @@ class FakeLogger - attr_reader :warnings, :errors, :infos + attr_reader :warnings, :errors def initialize @warnings = [] @errors = [] - @infos = [] - end - - def info(message = nil) - @infos << message end def warn(message)