From ed91b75c937805cb52b3930f2549b7a179cdc421 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 22 Oct 2018 17:10:01 +0100 Subject: Apply mapping to symbols returned from dynamic CSP sources Previously if a dynamic source returned a symbol such as :self it would be converted to a string implicity, e.g: policy.default_src -> { :self } would generate the header: Content-Security-Policy: default-src self and now it generates: Content-Security-Policy: default-src 'self' --- actionpack/CHANGELOG.md | 17 +++++++++++++++++ .../lib/action_dispatch/http/content_security_policy.rb | 3 ++- .../test/dispatch/content_security_policy_test.rb | 4 ++-- 3 files changed, 21 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3858c211ea..8d0477ead3 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,20 @@ +* Apply mapping to symbols returned from dynamic CSP sources + + Previously if a dynamic source returned a symbol such as :self it + would be converted to a string implicity, e.g: + + policy.default_src -> { :self } + + would generate the header: + + Content-Security-Policy: default-src self + + and now it generates: + + Content-Security-Policy: default-src 'self' + + *Andrew White* + * Add `ActionController::Parameters#each_value`. *Lukáš Zapletal* diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 50953e32b5..15b7bd1233 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -257,7 +257,8 @@ module ActionDispatch #:nodoc: if context.nil? raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}" else - context.instance_exec(&source) + resolved = context.instance_exec(&source) + resolved.is_a?(Symbol) ? apply_mapping(resolved) : resolved end else raise RuntimeError, "Unexpected content security policy source: #{source.inspect}" diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 13ad22b5c5..8dd4b8edb1 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -264,8 +264,8 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT end POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| - p.default_src :self - p.script_src :https + p.default_src -> { :self } + p.script_src -> { :https } end class PolicyConfigMiddleware -- cgit v1.2.3 From a150a026591b7b9dcaba5a2ef5fce02f7d990aba Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 22 Oct 2018 17:15:33 +0100 Subject: Use request object for context if there's no controller There is no controller instance when using a redirect route or a mounted rack application so pass the request object as the context when resolving dynamic CSP sources in this scenario. Fixes #34200. --- actionpack/CHANGELOG.md | 10 ++++++++++ actionpack/lib/action_dispatch/http/content_security_policy.rb | 3 ++- actionpack/test/dispatch/content_security_policy_test.rb | 10 ++++++++-- 3 files changed, 20 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 8d0477ead3..5554d4e6b8 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,13 @@ +* Use request object for context if there's no controller + + There is no controller instance when using a redirect route or a + mounted rack application so pass the request object as the context + when resolving dynamic CSP sources in this scenario. + + Fixes #34200. + + *Andrew White* + * Apply mapping to symbols returned from dynamic CSP sources Previously if a dynamic source returned a symbol such as :self it diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 15b7bd1233..b1e5a28be5 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -22,7 +22,8 @@ module ActionDispatch #:nodoc: if policy = request.content_security_policy nonce = request.content_security_policy_nonce - headers[header_name(request)] = policy.build(request.controller_instance, nonce) + context = request.controller_instance || request + headers[header_name(request)] = policy.build(context, nonce) end response diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 8dd4b8edb1..c8c885f35c 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -260,6 +260,7 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT ROUTES.draw do scope module: "default_content_security_policy_integration_test" do get "/", to: "policy#index" + get "/redirect", to: redirect("/") end end @@ -295,14 +296,19 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT def test_adds_nonce_to_script_src_content_security_policy_only_once get "/" get "/" + assert_response :success + assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" + end + + def test_redirect_works_with_dynamic_sources + get "/redirect" + assert_response :redirect assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" end private def assert_policy(expected, report_only: false) - assert_response :success - if report_only expected_header = "Content-Security-Policy-Report-Only" unexpected_header = "Content-Security-Policy" -- cgit v1.2.3