SECURITY: Don't reuse CSP nonce between requests (#22544)

Co-authored-by: OsamaSayegh <asooomaasoooma90@gmail.com>

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

app/views/common/_google_tag_manager_head.html.erb

@@ -1,6 +1,6 @@
 <meta id="data-google-tag-manager"
   data-data-layer="<%= google_tag_manager_json %>"
-  data-nonce="<%= ApplicationHelper.google_tag_manager_nonce %>"
+  data-nonce="<%= ApplicationHelper.google_tag_manager_nonce(request.env) %>"
   data-container-id="<%= SiteSetting.gtm_container_id %>" />
 
 <%= preload_script 'google-tag-manager' %>

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 }

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
 

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

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

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