SECURITY: Don't reuse CSP nonce between anonymous requests

This commit is contained in:
OsamaSayegh 2023-07-28 12:53:44 +01:00 committed by David Taylor
parent 672f3e7e41
commit 0976c8fad6
No known key found for this signature in database
GPG Key ID: 46904C18B1D3F434
15 changed files with 105 additions and 22 deletions

View File

@ -64,8 +64,10 @@ module ApplicationHelper
google_universal_analytics_json google_universal_analytics_json
end end
def self.google_tag_manager_nonce(env) def google_tag_manager_nonce_placeholder
env[:discourse_content_security_policy_nonce] ||= SecureRandom.hex placeholder = "[[csp_nonce_placeholder_#{SecureRandom.hex}]]"
response.headers["Discourse-GTM-Nonce-Placeholder"] = placeholder
placeholder
end end
def shared_session_key def shared_session_key

View File

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

View File

@ -167,6 +167,9 @@ module Discourse
config.middleware.swap ActionDispatch::ContentSecurityPolicy::Middleware, config.middleware.swap ActionDispatch::ContentSecurityPolicy::Middleware,
ContentSecurityPolicy::Middleware ContentSecurityPolicy::Middleware
require "middleware/gtm_script_nonce_injector"
config.middleware.insert_after(ActionDispatch::Flash, Middleware::GtmScriptNonceInjector)
require "middleware/discourse_public_exceptions" require "middleware/discourse_public_exceptions"
config.exceptions_app = Middleware::DiscoursePublicExceptions.new(Rails.public_path) config.exceptions_app = Middleware::DiscoursePublicExceptions.new(Rails.public_path)

View File

@ -6,10 +6,11 @@ enabled =
if Rails.configuration.respond_to?(:enable_anon_caching) if Rails.configuration.respond_to?(:enable_anon_caching)
Rails.configuration.enable_anon_caching Rails.configuration.enable_anon_caching
else else
Rails.env.production? Rails.env.production? || Rails.env.test?
end end
if !ENV["DISCOURSE_DISABLE_ANON_CACHE"] && enabled if !ENV["DISCOURSE_DISABLE_ANON_CACHE"] && enabled
# in an ideal world this is position 0, but mobile detection uses ... session and request and params # in an ideal world this is position 0, but mobile detection uses ... session and request and params
Rails.configuration.middleware.insert_after ActionDispatch::Flash, Middleware::AnonymousCache Rails.configuration.middleware.insert_after Middleware::GtmScriptNonceInjector,
Middleware::AnonymousCache
end end

View File

@ -4,13 +4,13 @@ require "content_security_policy/extension"
class ContentSecurityPolicy class ContentSecurityPolicy
class << self class << self
def policy(theme_id = nil, env: {}, base_url: Discourse.base_url, path_info: "/") def policy(theme_id = nil, base_url: Discourse.base_url, path_info: "/")
new.build(theme_id, env: env, base_url: base_url, path_info: path_info) new.build(theme_id, base_url: base_url, path_info: path_info)
end end
end end
def build(theme_id, env: {}, base_url:, path_info: "/") def build(theme_id, base_url:, path_info: "/")
builder = Builder.new(base_url: base_url, env: env) builder = Builder.new(base_url: base_url)
Extension.theme_extensions(theme_id).each { |extension| builder << extension } Extension.theme_extensions(theme_id).each { |extension| builder << extension }
Extension.plugin_extensions.each { |extension| builder << extension } Extension.plugin_extensions.each { |extension| builder << extension }

View File

@ -25,8 +25,8 @@ class ContentSecurityPolicy
style_src style_src
].freeze ].freeze
def initialize(base_url:, env: {}) def initialize(base_url:)
@directives = Default.new(base_url: base_url, env: env).directives @directives = Default.new(base_url: base_url).directives
@base_url = base_url @base_url = base_url
end end

View File

@ -5,9 +5,8 @@ class ContentSecurityPolicy
class Default class Default
attr_reader :directives attr_reader :directives
def initialize(base_url:, env: {}) def initialize(base_url:)
@base_url = base_url @base_url = base_url
@env = env
@directives = @directives =
{}.tap do |directives| {}.tap do |directives|
directives[:upgrade_insecure_requests] = [] if SiteSetting.force_https directives[:upgrade_insecure_requests] = [] if SiteSetting.force_https
@ -86,7 +85,6 @@ class ContentSecurityPolicy
end end
if SiteSetting.gtm_container_id.present? if SiteSetting.gtm_container_id.present?
sources << "https://www.googletagmanager.com/gtm.js" sources << "https://www.googletagmanager.com/gtm.js"
sources << "'nonce-#{ApplicationHelper.google_tag_manager_nonce(@env)}'"
end end
sources << "'#{SplashScreenHelper.fingerprint}'" if SiteSetting.splash_screen sources << "'#{SplashScreenHelper.fingerprint}'" if SiteSetting.splash_screen

View File

@ -21,13 +21,11 @@ class ContentSecurityPolicy
headers["Content-Security-Policy"] = policy( headers["Content-Security-Policy"] = policy(
theme_id, theme_id,
env: env,
base_url: base_url, base_url: base_url,
path_info: env["PATH_INFO"], path_info: env["PATH_INFO"],
) if SiteSetting.content_security_policy ) if SiteSetting.content_security_policy
headers["Content-Security-Policy-Report-Only"] = policy( headers["Content-Security-Policy-Report-Only"] = policy(
theme_id, theme_id,
env: env,
base_url: base_url, base_url: base_url,
path_info: env["PATH_INFO"], path_info: env["PATH_INFO"],
) if SiteSetting.content_security_policy_report_only ) if SiteSetting.content_security_policy_report_only

View File

@ -43,6 +43,21 @@ module Middleware
env["ANON_CACHE_DURATION"] = duration env["ANON_CACHE_DURATION"] = duration
end end
def self.clear_all_cache!
if Rails.env.production?
raise "for perf reasons, clear_all_cache! cannot be used in production."
end
Discourse.redis.keys("ANON_CACHE_*").each { |k| Discourse.redis.del(k) }
end
def self.disable_anon_cache
@@disabled = true
end
def self.enable_anon_cache
@@disabled = false
end
# This gives us an API to insert anonymous cache segments # This gives us an API to insert anonymous cache segments
class Helper class Helper
RACK_SESSION = "rack.session" RACK_SESSION = "rack.session"
@ -232,7 +247,10 @@ module Middleware
end end
def cacheable? def cacheable?
!!(!has_auth_cookie? && get? && no_cache_bypass) !!(
GlobalSetting.anon_cache_store_threshold > 0 && !has_auth_cookie? && get? &&
no_cache_bypass
)
end end
def compress(val) def compress(val)
@ -326,6 +344,8 @@ module Middleware
PAYLOAD_INVALID_REQUEST_METHODS = %w[GET HEAD] PAYLOAD_INVALID_REQUEST_METHODS = %w[GET HEAD]
def call(env) def call(env)
return @app.call(env) if defined?(@@disabled) && @@disabled
if PAYLOAD_INVALID_REQUEST_METHODS.include?(env[Rack::REQUEST_METHOD]) && if PAYLOAD_INVALID_REQUEST_METHODS.include?(env[Rack::REQUEST_METHOD]) &&
env[Rack::RACK_INPUT].size > 0 env[Rack::RACK_INPUT].size > 0
return 413, { "Cache-Control" => "private, max-age=0, must-revalidate" }, [] return 413, { "Cache-Control" => "private, max-age=0, must-revalidate" }, []

View File

@ -0,0 +1,26 @@
# frozen_string_literal: true
module Middleware
class GtmScriptNonceInjector
def initialize(app, settings = {})
@app = app
end
def call(env)
status, headers, response = @app.call(env)
if nonce_placeholder = headers.delete("Discourse-GTM-Nonce-Placeholder")
nonce = SecureRandom.hex
parts = []
response.each { |part| parts << part.to_s.sub(nonce_placeholder, nonce) }
%w[Content-Security-Policy Content-Security-Policy-Report-Only].each do |name|
next if headers[name].blank?
headers[name] = headers[name].sub("script-src ", "script-src 'nonce-#{nonce}' ")
end
[status, headers, parts]
else
[status, headers, response]
end
end
end
end

View File

@ -106,7 +106,9 @@ RSpec.describe ContentSecurityPolicy do
script_srcs = parse(policy)["script-src"] script_srcs = parse(policy)["script-src"]
expect(script_srcs).to include("https://www.googletagmanager.com/gtm.js") expect(script_srcs).to include("https://www.googletagmanager.com/gtm.js")
expect(script_srcs.to_s).to include("nonce-") # nonce is added by the GtmScriptNonceInjector middleware to prevent the
# nonce from getting cached by AnonymousCache
expect(script_srcs.to_s).not_to include("nonce-")
end end
it "allowlists CDN assets when integrated" do it "allowlists CDN assets when integrated" do

View File

@ -3,6 +3,8 @@
RSpec.describe Middleware::AnonymousCache do RSpec.describe Middleware::AnonymousCache do
let(:middleware) { Middleware::AnonymousCache.new(lambda { |_| [200, {}, []] }) } let(:middleware) { Middleware::AnonymousCache.new(lambda { |_| [200, {}, []] }) }
before { Middleware::AnonymousCache.enable_anon_cache }
def env(opts = {}) def env(opts = {})
create_request_env(path: opts.delete(:path) || "http://test.com/path?bla=1").merge(opts) create_request_env(path: opts.delete(:path) || "http://test.com/path?bla=1").merge(opts)
end end

View File

@ -677,6 +677,8 @@ RSpec.describe Middleware::RequestTracker do
after { Middleware::RequestTracker.unregister_detailed_request_logger(logger) } after { Middleware::RequestTracker.unregister_detailed_request_logger(logger) }
it "can report data from anon cache" do it "can report data from anon cache" do
Middleware::AnonymousCache.enable_anon_cache
cache = Middleware::AnonymousCache.new(app([200, {}, ["i am a thing"]])) cache = Middleware::AnonymousCache.new(app([200, {}, ["i am a thing"]]))
tracker = Middleware::RequestTracker.new(cache) tracker = Middleware::RequestTracker.new(cache)

View File

@ -149,6 +149,8 @@ module TestSetup
BadgeGranter.disable_queue BadgeGranter.disable_queue
OmniAuth.config.test_mode = false OmniAuth.config.test_mode = false
Middleware::AnonymousCache.disable_anon_cache
end end
end end

View File

@ -637,36 +637,63 @@ RSpec.describe ApplicationController do
it "when GTM is enabled it adds the same nonce to the policy and the GTM tag" do it "when GTM is enabled it adds the same nonce to the policy and the GTM tag" do
SiteSetting.content_security_policy = true SiteSetting.content_security_policy = true
SiteSetting.content_security_policy_report_only = true
SiteSetting.gtm_container_id = "GTM-ABCDEF" SiteSetting.gtm_container_id = "GTM-ABCDEF"
get "/latest" get "/latest"
expect(response.headers).to include("Content-Security-Policy")
script_src = parse(response.headers["Content-Security-Policy"])["script-src"] script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
report_only_script_src =
parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]
nonce = extract_nonce_from_script_src(script_src) nonce = extract_nonce_from_script_src(script_src)
report_only_nonce = extract_nonce_from_script_src(report_only_script_src)
expect(nonce).to eq(report_only_nonce)
gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first
expect(gtm_meta_tag["data-nonce"]).to eq(nonce) expect(gtm_meta_tag["data-nonce"]).to eq(nonce)
end end
it "doesn't reuse CSP nonces between requests" do it "doesn't reuse nonces between requests" do
global_setting :anon_cache_store_threshold, 1
Middleware::AnonymousCache.enable_anon_cache
Middleware::AnonymousCache.clear_all_cache!
SiteSetting.content_security_policy = true SiteSetting.content_security_policy = true
SiteSetting.content_security_policy_report_only = true
SiteSetting.gtm_container_id = "GTM-ABCDEF" SiteSetting.gtm_container_id = "GTM-ABCDEF"
get "/latest" get "/latest"
expect(response.headers).to include("Content-Security-Policy") expect(response.headers["X-Discourse-Cached"]).to eq("store")
expect(response.headers).not_to include("Discourse-GTM-Nonce-Placeholder")
script_src = parse(response.headers["Content-Security-Policy"])["script-src"] script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
report_only_script_src =
parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]
first_nonce = extract_nonce_from_script_src(script_src) first_nonce = extract_nonce_from_script_src(script_src)
first_report_only_nonce = extract_nonce_from_script_src(report_only_script_src)
expect(first_nonce).to eq(first_report_only_nonce)
gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first
expect(gtm_meta_tag["data-nonce"]).to eq(first_nonce) expect(gtm_meta_tag["data-nonce"]).to eq(first_nonce)
get "/latest" get "/latest"
expect(response.headers).to include("Content-Security-Policy") expect(response.headers["X-Discourse-Cached"]).to eq("true")
expect(response.headers).not_to include("Discourse-GTM-Nonce-Placeholder")
script_src = parse(response.headers["Content-Security-Policy"])["script-src"] script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
report_only_script_src =
parse(response.headers["Content-Security-Policy-Report-Only"])["script-src"]
second_nonce = extract_nonce_from_script_src(script_src) second_nonce = extract_nonce_from_script_src(script_src)
second_report_only_nonce = extract_nonce_from_script_src(report_only_script_src)
expect(second_nonce).to eq(second_report_only_nonce)
expect(first_nonce).not_to eq(second_nonce) expect(first_nonce).not_to eq(second_nonce)
gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first gtm_meta_tag = Nokogiri::HTML5.fragment(response.body).css("#data-google-tag-manager").first