diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5f7555fd500..7e3c903e3d5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -64,8 +64,8 @@ module ApplicationHelper google_universal_analytics_json end - def self.google_tag_manager_nonce - @gtm_nonce ||= SecureRandom.hex + def self.google_tag_manager_nonce(env) + env[:discourse_content_security_policy_nonce] ||= SecureRandom.hex end def shared_session_key diff --git a/app/views/common/_google_tag_manager_head.html.erb b/app/views/common/_google_tag_manager_head.html.erb index f026a791902..26790ca3db7 100644 --- a/app/views/common/_google_tag_manager_head.html.erb +++ b/app/views/common/_google_tag_manager_head.html.erb @@ -1,6 +1,6 @@ <%= preload_script 'google-tag-manager' %> diff --git a/lib/content_security_policy.rb b/lib/content_security_policy.rb index 107dc0437df..53cf16d6103 100644 --- a/lib/content_security_policy.rb +++ b/lib/content_security_policy.rb @@ -4,13 +4,13 @@ require "content_security_policy/extension" class ContentSecurityPolicy class << self - def policy(theme_id = nil, base_url: Discourse.base_url, path_info: "/") - new.build(theme_id, base_url: base_url, path_info: path_info) + def policy(theme_id = nil, env: {}, base_url: Discourse.base_url, path_info: "/") + new.build(theme_id, env: env, base_url: base_url, path_info: path_info) end end - def build(theme_id, base_url:, path_info: "/") - builder = Builder.new(base_url: base_url) + def build(theme_id, env: {}, base_url:, path_info: "/") + builder = Builder.new(base_url: base_url, env: env) Extension.theme_extensions(theme_id).each { |extension| builder << extension } Extension.plugin_extensions.each { |extension| builder << extension } diff --git a/lib/content_security_policy/builder.rb b/lib/content_security_policy/builder.rb index e23f55111e2..ddcb9db67eb 100644 --- a/lib/content_security_policy/builder.rb +++ b/lib/content_security_policy/builder.rb @@ -25,8 +25,8 @@ class ContentSecurityPolicy style_src ].freeze - def initialize(base_url:) - @directives = Default.new(base_url: base_url).directives + def initialize(base_url:, env: {}) + @directives = Default.new(base_url: base_url, env: env).directives @base_url = base_url end diff --git a/lib/content_security_policy/default.rb b/lib/content_security_policy/default.rb index 6b147079c70..ff3767c51fe 100644 --- a/lib/content_security_policy/default.rb +++ b/lib/content_security_policy/default.rb @@ -5,8 +5,9 @@ class ContentSecurityPolicy class Default attr_reader :directives - def initialize(base_url:) + def initialize(base_url:, env: {}) @base_url = base_url + @env = env @directives = {}.tap do |directives| directives[:upgrade_insecure_requests] = [] if SiteSetting.force_https @@ -85,7 +86,7 @@ class ContentSecurityPolicy end if SiteSetting.gtm_container_id.present? sources << "https://www.googletagmanager.com/gtm.js" - sources << "'nonce-#{ApplicationHelper.google_tag_manager_nonce}'" + sources << "'nonce-#{ApplicationHelper.google_tag_manager_nonce(@env)}'" end sources << "'#{SplashScreenHelper.fingerprint}'" if SiteSetting.splash_screen diff --git a/lib/content_security_policy/middleware.rb b/lib/content_security_policy/middleware.rb index 79ec0834275..9c729511cab 100644 --- a/lib/content_security_policy/middleware.rb +++ b/lib/content_security_policy/middleware.rb @@ -21,11 +21,13 @@ class ContentSecurityPolicy headers["Content-Security-Policy"] = policy( theme_id, + env: env, base_url: base_url, path_info: env["PATH_INFO"], ) if SiteSetting.content_security_policy headers["Content-Security-Policy-Report-Only"] = policy( theme_id, + env: env, base_url: base_url, path_info: env["PATH_INFO"], ) if SiteSetting.content_security_policy_report_only diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 40056a27bee..a2f461fe868 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -640,12 +640,37 @@ RSpec.describe ApplicationController do SiteSetting.gtm_container_id = "GTM-ABCDEF" get "/latest" - nonce = ApplicationHelper.google_tag_manager_nonce - expect(response.headers).to include("Content-Security-Policy") + expect(response.headers).to include("Content-Security-Policy") script_src = parse(response.headers["Content-Security-Policy"])["script-src"] - expect(script_src.to_s).to include(nonce) - expect(response.body).to include(nonce) + nonce = extract_nonce_from_script_src(script_src) + + gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first + expect(gtm_meta_tag["data-nonce"]).to eq(nonce) + end + + it "doesn't reuse CSP nonces between requests" do + SiteSetting.content_security_policy = true + SiteSetting.gtm_container_id = "GTM-ABCDEF" + + get "/latest" + + expect(response.headers).to include("Content-Security-Policy") + script_src = parse(response.headers["Content-Security-Policy"])["script-src"] + first_nonce = extract_nonce_from_script_src(script_src) + + gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first + expect(gtm_meta_tag["data-nonce"]).to eq(first_nonce) + + get "/latest" + + expect(response.headers).to include("Content-Security-Policy") + script_src = parse(response.headers["Content-Security-Policy"])["script-src"] + second_nonce = extract_nonce_from_script_src(script_src) + + expect(first_nonce).not_to eq(second_nonce) + gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first + expect(gtm_meta_tag["data-nonce"]).to eq(second_nonce) end it "when splash screen is enabled it adds the fingerprint to the policy" do @@ -670,6 +695,12 @@ RSpec.describe ApplicationController do end .to_h end + + def extract_nonce_from_script_src(script_src) + nonce = script_src.find { |src| src.match?(/\A'nonce-\h{32}'\z/) }[-33...-1] + expect(nonce).to be_present + nonce + end end it "can respond to a request with */* accept header" do