diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/content_security_policy.rb | 55 | ||||
| -rw-r--r-- | actionpack/test/dispatch/content_security_policy_test.rb | 93 |
3 files changed, 97 insertions, 53 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c85add22a9..93dd532f07 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -2,7 +2,7 @@ Fixes [#35297](https://github.com/rails/rails/issues/32597). - *Andrey Novikov* + *Andrey Novikov*, *Andrew White* * Move default headers configuration into their own module that can be included in controllers. diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 0573f13f94..82d348fa22 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -21,7 +21,8 @@ module ActionDispatch #:nodoc: return response if policy_present?(headers) if policy = request.content_security_policy - headers[header_name(request)] = policy.build(request) + nonce = request.content_security_policy_nonce + headers[header_name(request)] = policy.build(request.controller_instance, nonce) end response @@ -95,14 +96,6 @@ module ActionDispatch #:nodoc: end end - class NonceGenerator - def call(request) - if nonce = request&.content_security_policy_nonce - "'nonce-#{nonce}'" - end - end - end - MAPPINGS = { self: "'self'", unsafe_eval: "'unsafe-eval'", @@ -133,12 +126,14 @@ module ActionDispatch #:nodoc: manifest_src: "manifest-src", media_src: "media-src", object_src: "object-src", - # script_src handled differently + script_src: "script-src", style_src: "style-src", worker_src: "worker-src" }.freeze - private_constant :MAPPINGS, :DIRECTIVES + NONCE_DIRECTIVES = %w[script-src].freeze + + private_constant :MAPPINGS, :DIRECTIVES, :NONCE_DIRECTIVES attr_reader :directives @@ -161,15 +156,6 @@ module ActionDispatch #:nodoc: end end - def script_src(*sources) - if sources.first - @directives["script-src"] = apply_mappings(sources) - @directives["script-src"] << NonceGenerator.new - else - @directives.delete("script-src") - end - end - def block_all_mixed_content(enabled = true) if enabled @directives["block-all-mixed-content"] = true @@ -216,8 +202,8 @@ module ActionDispatch #:nodoc: end end - def build(request = nil) - build_directives(request).compact.join("; ") + def build(context = nil, nonce = nil) + build_directives(context, nonce).compact.join("; ") end private @@ -240,10 +226,15 @@ module ActionDispatch #:nodoc: end end - def build_directives(request) + def build_directives(context, nonce) @directives.map do |directive, sources| if sources.is_a?(Array) - "#{directive} #{build_directive(sources, request).compact.join(' ')}" + "#{directive} #{build_directive(sources, context).join(' ')}" + if nonce && nonce_directive?(directive) + "#{directive} #{build_directive(sources, context).join(' ')} 'nonce-#{nonce}'" + else + "#{directive} #{build_directive(sources, context).join(' ')}" + end elsif sources directive else @@ -252,27 +243,29 @@ module ActionDispatch #:nodoc: end end - def build_directive(sources, request) - sources.map { |source| resolve_source(source, request) } + def build_directive(sources, context) + sources.map { |source| resolve_source(source, context) } end - def resolve_source(source, request) + def resolve_source(source, context) case source when String source when Symbol source.to_s when Proc - if request&.controller_instance.nil? + if context.nil? raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}" else - request.controller_instance.instance_exec(&source) + context.instance_exec(&source) end - when NonceGenerator - source.call(request) else raise RuntimeError, "Unexpected content security policy source: #{source.inspect}" end end + + def nonce_directive?(directive) + NONCE_DIRECTIVES.include?(directive) + end end end diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index eb0b930828..c4c7f53903 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -201,17 +201,17 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase def test_dynamic_directives request = ActionDispatch::Request.new("HTTP_HOST" => "www.example.com") - request.controller_instance = Struct.new(:request).new(request) + controller = Struct.new(:request).new(request) @policy.script_src -> { request.host } - assert_equal "script-src www.example.com", @policy.build(request) + assert_equal "script-src www.example.com", @policy.build(controller) end def test_mixed_static_and_dynamic_directives @policy.script_src :self, -> { "foo.com" }, "bar.com" request = ActionDispatch::Request.new({}) - request.controller_instance = Struct.new(:request).new(request) - assert_equal "script-src 'self' foo.com bar.com", @policy.build(request) + controller = Struct.new(:request).new(request) + assert_equal "script-src 'self' foo.com bar.com", @policy.build(controller) end def test_invalid_directive_source @@ -243,25 +243,88 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase end end +class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest + class PolicyController < ActionController::Base + def index + head :ok + end + end + + ROUTES = ActionDispatch::Routing::RouteSet.new + ROUTES.draw do + scope module: "default_content_security_policy_integration_test" do + get "/", to: "policy#index" + end + end + + POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src :self + p.script_src :https + end + + class PolicyConfigMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.content_security_policy"] = POLICY + env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" } + env["action_dispatch.content_security_policy_report_only"] = false + env["action_dispatch.show_exceptions"] = false + + @app.call(env) + end + end + + APP = build_app(ROUTES) do |middleware| + middleware.use PolicyConfigMiddleware + middleware.use ActionDispatch::ContentSecurityPolicy::Middleware + end + + def app + APP + end + + def test_adds_nonce_to_script_src_content_security_policy_only_once + get "/" + get "/" + 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" + else + expected_header = "Content-Security-Policy" + unexpected_header = "Content-Security-Policy-Report-Only" + end + + assert_nil response.headers[unexpected_header] + assert_equal expected, response.headers[expected_header] + end +end + class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest class PolicyController < ActionController::Base content_security_policy only: :inline do |p| p.default_src "https://example.com" - p.script_src false end content_security_policy only: :conditional, if: :condition? do |p| p.default_src "https://true.example.com" - p.script_src false end content_security_policy only: :conditional, unless: :condition? do |p| p.default_src "https://false.example.com" - p.script_src false end content_security_policy only: :report_only do |p| - p.script_src false p.report_uri "/violations" end @@ -298,10 +361,6 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest head :ok end - def default_script_src - head :ok - end - private def condition? params[:condition] == "true" @@ -317,13 +376,11 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest get "/report-only", to: "policy#report_only" get "/script-src", to: "policy#script_src" get "/no-policy", to: "policy#no_policy" - get "/default-script-src", to: "policy#default_script_src" end end POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| p.default_src :self - p.script_src :https end class PolicyConfigMiddleware @@ -352,7 +409,7 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest def test_generates_content_security_policy_header get "/" - assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" + assert_policy "default-src 'self'" end def test_generates_inline_content_security_policy @@ -378,12 +435,6 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest assert_policy "script-src 'self' 'nonce-iyhD0Yc0W+c='" end - def test_adds_nonce_to_script_src_content_security_policy_only_once - get "/default-script-src" - get "/default-script-src" - assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" - end - def test_generates_no_content_security_policy get "/no-policy" |