diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 60 | ||||
| -rw-r--r-- | actionpack/lib/action_controller.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/base.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/content_security_policy.rb | 26 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/http_authentication.rb | 11 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/request_forgery_protection.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/content_security_policy.rb | 233 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/request.rb | 1 | ||||
| -rw-r--r-- | actionpack/test/dispatch/content_security_policy_test.rb | 359 |
10 files changed, 688 insertions, 9 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index e01f88e902..1843f058e0 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,63 @@ +* Add DSL for configuring Content-Security-Policy header + + The DSL allows you to configure a global Content-Security-Policy + header and then override within a controller. For more information + about the Content-Security-Policy header see MDN: + + https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy + + Example global policy: + + # config/initializers/content_security_policy.rb + Rails.application.config.content_security_policy do |p| + p.default_src :self, :https + p.font_src :self, :https, :data + p.img_src :self, :https, :data + p.object_src :none + p.script_src :self, :https + p.style_src :self, :https, :unsafe_inline + end + + Example controller overrides: + + # Override policy inline + class PostsController < ApplicationController + content_security_policy do |p| + p.upgrade_insecure_requests true + end + end + + # Using literal values + class PostsController < ApplicationController + content_security_policy do |p| + p.base_uri "https://www.example.com" + end + end + + # Using mixed static and dynamic values + class PostsController < ApplicationController + content_security_policy do |p| + p.base_uri :self, -> { "https://#{current_user.domain}.example.com" } + end + end + + Allows you to also only report content violations for migrating + legacy content using the `content_security_policy_report_only` + configuration attribute, e.g; + + # config/initializers/content_security_policy.rb + Rails.application.config.content_security_policy_report_only = true + + # controller override + class PostsController < ApplicationController + self.content_security_policy_report_only = true + end + + Note that this feature does not validate the header for performance + reasons since the header is calculated at runtime. + + *Andrew White* + * Make `assert_recognizes` to traverse mounted engines *Yuichiro Kaneko* diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index bd19b8cd5d..f43784f9f2 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -22,6 +22,7 @@ module ActionController autoload_under "metal" do autoload :ConditionalGet + autoload :ContentSecurityPolicy autoload :Cookies autoload :DataStreaming autoload :EtagWithTemplateDigest diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index b73269871b..204a3d400c 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -225,6 +225,7 @@ module ActionController Flash, FormBuilder, RequestForgeryProtection, + ContentSecurityPolicy, ForceSSL, Streaming, DataStreaming, diff --git a/actionpack/lib/action_controller/metal/content_security_policy.rb b/actionpack/lib/action_controller/metal/content_security_policy.rb new file mode 100644 index 0000000000..48a7109bea --- /dev/null +++ b/actionpack/lib/action_controller/metal/content_security_policy.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module ActionController #:nodoc: + module ContentSecurityPolicy + # TODO: Documentation + extend ActiveSupport::Concern + + module ClassMethods + def content_security_policy(**options, &block) + before_action(options) do + if block_given? + policy = request.content_security_policy.clone + yield policy + request.content_security_policy = policy + end + end + end + + def content_security_policy_report_only(report_only = true, **options) + before_action(options) do + request.content_security_policy_report_only = report_only + end + end + end + end +end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 0c8132684a..01676f3237 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -72,10 +72,10 @@ module ActionController before_action(options.except(:name, :password, :realm)) do authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password| # This comparison uses & so that it doesn't short circuit and - # uses `variable_size_secure_compare` so that length information + # uses `secure_compare` so that length information # isn't leaked. - ActiveSupport::SecurityUtils.variable_size_secure_compare(name, options[:name]) & - ActiveSupport::SecurityUtils.variable_size_secure_compare(password, options[:password]) + ActiveSupport::SecurityUtils.secure_compare(name, options[:name]) & + ActiveSupport::SecurityUtils.secure_compare(password, options[:password]) end end end @@ -350,10 +350,7 @@ module ActionController # authenticate_or_request_with_http_token do |token, options| # # Compare the tokens in a time-constant manner, to mitigate # # timing attacks. - # ActiveSupport::SecurityUtils.secure_compare( - # ::Digest::SHA256.hexdigest(token), - # ::Digest::SHA256.hexdigest(TOKEN) - # ) + # ActiveSupport::SecurityUtils.secure_compare(token, TOKEN) # end # end # end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 906494ba16..04fadc90e2 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -369,7 +369,7 @@ module ActionController #:nodoc: end def compare_with_real_token(token, session) # :doc: - ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session)) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, real_csrf_token(session)) end def valid_per_form_csrf_token?(token, session) # :doc: @@ -380,7 +380,7 @@ module ActionController #:nodoc: request.request_method ) - ActiveSupport::SecurityUtils.secure_compare(token, correct_token) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, correct_token) else false end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 34937f3229..6fed911d0a 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -42,6 +42,7 @@ module ActionDispatch eager_autoload do autoload_under "http" do + autoload :ContentSecurityPolicy autoload :Request autoload :Response end diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb new file mode 100644 index 0000000000..d10d4faf3d --- /dev/null +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -0,0 +1,233 @@ +# frozen_string_literal: true + +module ActionDispatch #:nodoc: + class ContentSecurityPolicy + class Middleware + CONTENT_TYPE = "Content-Type".freeze + POLICY = "Content-Security-Policy".freeze + POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only".freeze + + def initialize(app) + @app = app + end + + def call(env) + request = ActionDispatch::Request.new env + _, headers, _ = response = @app.call(env) + + return response unless html_response?(headers) + return response if policy_present?(headers) + + if policy = request.content_security_policy + headers[header_name(request)] = policy.build(request.controller_instance) + end + + response + end + + private + + def html_response?(headers) + if content_type = headers[CONTENT_TYPE] + content_type =~ /html/ + end + end + + def header_name(request) + if request.content_security_policy_report_only + POLICY_REPORT_ONLY + else + POLICY + end + end + + def policy_present?(headers) + headers[POLICY] || headers[POLICY_REPORT_ONLY] + end + end + + module Request + POLICY = "action_dispatch.content_security_policy".freeze + POLICY_REPORT_ONLY = "action_dispatch.content_security_policy_report_only".freeze + + def content_security_policy + get_header(POLICY) + end + + def content_security_policy=(policy) + set_header(POLICY, policy) + end + + def content_security_policy_report_only + get_header(POLICY_REPORT_ONLY) + end + + def content_security_policy_report_only=(value) + set_header(POLICY_REPORT_ONLY, value) + end + end + + MAPPINGS = { + self: "'self'", + unsafe_eval: "'unsafe-eval'", + unsafe_inline: "'unsafe-inline'", + none: "'none'", + http: "http:", + https: "https:", + data: "data:", + mediastream: "mediastream:", + blob: "blob:", + filesystem: "filesystem:", + report_sample: "'report-sample'", + strict_dynamic: "'strict-dynamic'" + }.freeze + + DIRECTIVES = { + base_uri: "base-uri", + child_src: "child-src", + connect_src: "connect-src", + default_src: "default-src", + font_src: "font-src", + form_action: "form-action", + frame_ancestors: "frame-ancestors", + frame_src: "frame-src", + img_src: "img-src", + manifest_src: "manifest-src", + media_src: "media-src", + object_src: "object-src", + script_src: "script-src", + style_src: "style-src", + worker_src: "worker-src" + }.freeze + + private_constant :MAPPINGS, :DIRECTIVES + + attr_reader :directives + + def initialize + @directives = {} + yield self if block_given? + end + + def initialize_copy(other) + @directives = copy_directives(other.directives) + end + + DIRECTIVES.each do |name, directive| + define_method(name) do |*sources| + if sources.first + @directives[directive] = apply_mappings(sources) + else + @directives.delete(directive) + end + end + end + + def block_all_mixed_content(enabled = true) + if enabled + @directives["block-all-mixed-content"] = true + else + @directives.delete("block-all-mixed-content") + end + end + + def plugin_types(*types) + if types.first + @directives["plugin-types"] = types + else + @directives.delete("plugin-types") + end + end + + def report_uri(uri) + @directives["report-uri"] = [uri] + end + + def require_sri_for(*types) + if types.first + @directives["require-sri-for"] = types + else + @directives.delete("require-sri-for") + end + end + + def sandbox(*values) + if values.empty? + @directives["sandbox"] = true + elsif values.first + @directives["sandbox"] = values + else + @directives.delete("sandbox") + end + end + + def upgrade_insecure_requests(enabled = true) + if enabled + @directives["upgrade-insecure-requests"] = true + else + @directives.delete("upgrade-insecure-requests") + end + end + + def build(context = nil) + build_directives(context).compact.join("; ") + ";" + end + + private + def copy_directives(directives) + directives.transform_values { |sources| sources.map(&:dup) } + end + + def apply_mappings(sources) + sources.map do |source| + case source + when Symbol + apply_mapping(source) + when String, Proc + source + else + raise ArgumentError, "Invalid content security policy source: #{source.inspect}" + end + end + end + + def apply_mapping(source) + MAPPINGS.fetch(source) do + raise ArgumentError, "Unknown content security policy source mapping: #{source.inspect}" + end + end + + def build_directives(context) + @directives.map do |directive, sources| + if sources.is_a?(Array) + "#{directive} #{build_directive(sources, context).join(' ')}" + elsif sources + directive + else + nil + end + end + end + + def build_directive(sources, context) + sources.map { |source| resolve_source(source, context) } + end + + def resolve_source(source, context) + case source + when String + source + when Symbol + source.to_s + when Proc + if context.nil? + raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}" + else + context.instance_exec(&source) + end + else + raise RuntimeError, "Unexpected content security policy source: #{source.inspect}" + end + end + end +end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index d631281e4b..3838b84a7a 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -22,6 +22,7 @@ module ActionDispatch include ActionDispatch::Http::Parameters include ActionDispatch::Http::FilterParameters include ActionDispatch::Http::URL + include ActionDispatch::ContentSecurityPolicy::Request include Rack::Request::Env autoload :Session, "action_dispatch/request/session" diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb new file mode 100644 index 0000000000..8a1ac066e8 --- /dev/null +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -0,0 +1,359 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class ContentSecurityPolicyTest < ActiveSupport::TestCase + def setup + @policy = ActionDispatch::ContentSecurityPolicy.new + end + + def test_build + assert_equal ";", @policy.build + + @policy.script_src :self + assert_equal "script-src 'self';", @policy.build + end + + def test_mappings + @policy.script_src :data + assert_equal "script-src data:;", @policy.build + + @policy.script_src :mediastream + assert_equal "script-src mediastream:;", @policy.build + + @policy.script_src :blob + assert_equal "script-src blob:;", @policy.build + + @policy.script_src :filesystem + assert_equal "script-src filesystem:;", @policy.build + + @policy.script_src :self + assert_equal "script-src 'self';", @policy.build + + @policy.script_src :unsafe_inline + assert_equal "script-src 'unsafe-inline';", @policy.build + + @policy.script_src :unsafe_eval + assert_equal "script-src 'unsafe-eval';", @policy.build + + @policy.script_src :none + assert_equal "script-src 'none';", @policy.build + + @policy.script_src :strict_dynamic + assert_equal "script-src 'strict-dynamic';", @policy.build + + @policy.script_src :none, :report_sample + assert_equal "script-src 'none' 'report-sample';", @policy.build + end + + def test_fetch_directives + @policy.child_src :self + assert_match %r{child-src 'self'}, @policy.build + + @policy.child_src false + assert_no_match %r{child-src}, @policy.build + + @policy.connect_src :self + assert_match %r{connect-src 'self'}, @policy.build + + @policy.connect_src false + assert_no_match %r{connect-src}, @policy.build + + @policy.default_src :self + assert_match %r{default-src 'self'}, @policy.build + + @policy.default_src false + assert_no_match %r{default-src}, @policy.build + + @policy.font_src :self + assert_match %r{font-src 'self'}, @policy.build + + @policy.font_src false + assert_no_match %r{font-src}, @policy.build + + @policy.frame_src :self + assert_match %r{frame-src 'self'}, @policy.build + + @policy.frame_src false + assert_no_match %r{frame-src}, @policy.build + + @policy.img_src :self + assert_match %r{img-src 'self'}, @policy.build + + @policy.img_src false + assert_no_match %r{img-src}, @policy.build + + @policy.manifest_src :self + assert_match %r{manifest-src 'self'}, @policy.build + + @policy.manifest_src false + assert_no_match %r{manifest-src}, @policy.build + + @policy.media_src :self + assert_match %r{media-src 'self'}, @policy.build + + @policy.media_src false + assert_no_match %r{media-src}, @policy.build + + @policy.object_src :self + assert_match %r{object-src 'self'}, @policy.build + + @policy.object_src false + assert_no_match %r{object-src}, @policy.build + + @policy.script_src :self + assert_match %r{script-src 'self'}, @policy.build + + @policy.script_src false + assert_no_match %r{script-src}, @policy.build + + @policy.style_src :self + assert_match %r{style-src 'self'}, @policy.build + + @policy.style_src false + assert_no_match %r{style-src}, @policy.build + + @policy.worker_src :self + assert_match %r{worker-src 'self'}, @policy.build + + @policy.worker_src false + assert_no_match %r{worker-src}, @policy.build + end + + def test_document_directives + @policy.base_uri "https://example.com" + assert_match %r{base-uri https://example\.com;}, @policy.build + + @policy.plugin_types "application/x-shockwave-flash" + assert_match %r{plugin-types application/x-shockwave-flash;}, @policy.build + + @policy.sandbox + assert_match %r{sandbox;}, @policy.build + + @policy.sandbox "allow-scripts", "allow-modals" + assert_match %r{sandbox allow-scripts allow-modals;}, @policy.build + + @policy.sandbox false + assert_no_match %r{sandbox}, @policy.build + end + + def test_navigation_directives + @policy.form_action :self + assert_match %r{form-action 'self';}, @policy.build + + @policy.frame_ancestors :self + assert_match %r{frame-ancestors 'self';}, @policy.build + end + + def test_reporting_directives + @policy.report_uri "/violations" + assert_match %r{report-uri /violations;}, @policy.build + end + + def test_other_directives + @policy.block_all_mixed_content + assert_match %r{block-all-mixed-content;}, @policy.build + + @policy.block_all_mixed_content false + assert_no_match %r{block-all-mixed-content}, @policy.build + + @policy.require_sri_for :script, :style + assert_match %r{require-sri-for script style;}, @policy.build + + @policy.require_sri_for "script", "style" + assert_match %r{require-sri-for script style;}, @policy.build + + @policy.require_sri_for + assert_no_match %r{require-sri-for}, @policy.build + + @policy.upgrade_insecure_requests + assert_match %r{upgrade-insecure-requests;}, @policy.build + + @policy.upgrade_insecure_requests false + assert_no_match %r{upgrade-insecure-requests}, @policy.build + end + + def test_multiple_sources + @policy.script_src :self, :https + assert_equal "script-src 'self' https:;", @policy.build + end + + def test_multiple_directives + @policy.script_src :self, :https + @policy.style_src :self, :https + assert_equal "script-src 'self' https:; style-src 'self' https:;", @policy.build + end + + def test_dynamic_directives + request = Struct.new(:host).new("www.example.com") + controller = Struct.new(:request).new(request) + + @policy.script_src -> { request.host } + 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" + assert_equal "script-src 'self' foo.com bar.com;", @policy.build(Object.new) + end + + def test_invalid_directive_source + exception = assert_raises(ArgumentError) do + @policy.script_src [:self] + end + + assert_equal "Invalid content security policy source: [:self]", exception.message + end + + def test_missing_context_for_dynamic_source + @policy.script_src -> { request.host } + + exception = assert_raises(RuntimeError) do + @policy.build + end + + assert_match %r{\AMissing context for the dynamic content security policy source:}, exception.message + end + + def test_raises_runtime_error_when_unexpected_source + @policy.plugin_types [:flash] + + exception = assert_raises(RuntimeError) do + @policy.build + end + + assert_match %r{\AUnexpected content security policy source:}, exception.message + end +end + +class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest + class PolicyController < ActionController::Base + content_security_policy only: :inline do |p| + p.default_src "https://example.com" + end + + content_security_policy only: :conditional, if: :condition? do |p| + p.default_src "https://true.example.com" + end + + content_security_policy only: :conditional, unless: :condition? do |p| + p.default_src "https://false.example.com" + end + + content_security_policy only: :report_only do |p| + p.report_uri "/violations" + end + + content_security_policy_report_only only: :report_only + + def index + head :ok + end + + def inline + head :ok + end + + def conditional + head :ok + end + + def report_only + head :ok + end + + private + def condition? + params[:condition] == "true" + end + end + + ROUTES = ActionDispatch::Routing::RouteSet.new + ROUTES.draw do + scope module: "content_security_policy_integration_test" do + get "/", to: "policy#index" + get "/inline", to: "policy#inline" + get "/conditional", to: "policy#conditional" + get "/report-only", to: "policy#report_only" + end + end + + POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src :self + 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_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_generates_content_security_policy_header + get "/" + assert_policy "default-src 'self';" + end + + def test_generates_inline_content_security_policy + get "/inline" + assert_policy "default-src https://example.com;" + end + + def test_generates_conditional_content_security_policy + get "/conditional", params: { condition: "true" } + assert_policy "default-src https://true.example.com;" + + get "/conditional", params: { condition: "false" } + assert_policy "default-src https://false.example.com;" + end + + def test_generates_report_only_content_security_policy + get "/report-only" + assert_policy "default-src 'self'; report-uri /violations;", report_only: true + end + + private + + def env_config + Rails.application.env_config + end + + def content_security_policy + env_config["action_dispatch.content_security_policy"] + end + + def content_security_policy=(policy) + env_config["action_dispatch.content_security_policy"] = policy + end + + 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 |