From 526467050bd9a6b391dd34efda95d180c0998f3a Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Mon, 27 Jun 2016 15:13:24 -1000 Subject: [PATCH 01/29] add report-only functionality --- lib/secure_headers.rb | 30 +++++-- lib/secure_headers/configuration.rb | 64 +++++++++++--- .../headers/policy_management.rb | 1 + spec/lib/secure_headers/view_helpers_spec.rb | 7 +- spec/lib/secure_headers_spec.rb | 84 ++++++++++++++++++- 5 files changed, 162 insertions(+), 24 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index badfdfd0..940f30dd 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -66,9 +66,19 @@ def override_content_security_policy_directives(request, additions) # # additions - a hash containing directives. e.g. # script_src: %w(another-host.com) - def append_content_security_policy_directives(request, additions) + def append_content_security_policy_directives(request, additions, target=:both) + unless [:both, :enforced, :report_only].include?(target) + raise "Unrecognized target: #{target}. Must be [:both, :enforced, :report_only]" + end + config = config_for(request) - config.dynamic_csp = CSP.combine_policies(config.current_csp, additions) + if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT + config.dynamic_csp = CSP.combine_policies(config.current_csp, additions) + end + if [:both, :report_only].include?(target) && config.current_csp_report_only != OPT_OUT + config.dynamic_csp_report_only = CSP.combine_policies(config.current_csp_report_only, additions) + end + override_secure_headers_request_config(request, config) end @@ -109,7 +119,11 @@ def opt_out_of_all_protection(request) def header_hash_for(request) config = config_for(request) unless ContentSecurityPolicy.idempotent_additions?(config.csp, config.current_csp) - config.rebuild_csp_header_cache!(request.user_agent) + config.rebuild_csp_header_cache!(request.user_agent, CSP::CONFIG_KEY) + end + + unless ContentSecurityPolicy.idempotent_additions?(config.csp_report_only, config.current_csp_report_only) + config.rebuild_csp_header_cache!(request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end use_cached_headers(config.cached_headers, request) @@ -190,7 +204,9 @@ def header_classes_for(request) ALL_HEADER_CLASSES else HTTP_HEADER_CLASSES - end + end.map do |klass| + klass::CONFIG_KEY + end.concat([CSP::REPORT_ONLY_CONFIG_KEY]) end # Private: takes a precomputed hash of headers and returns the Headers @@ -198,9 +214,9 @@ def header_classes_for(request) # # Returns a hash of header names / values valid for a given request. def use_cached_headers(headers, request) - header_classes_for(request).each_with_object({}) do |klass, hash| - if header = headers[klass::CONFIG_KEY] - header_name, value = if klass == CSP + header_classes_for(request).each_with_object({}) do |config_key, hash| + if header = headers[config_key] + header_name, value = if [CSP::CONFIG_KEY, CSP::REPORT_ONLY_CONFIG_KEY].include?(config_key) csp_header_for_ua(header, request) else header diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 59c42c59..6329fc52 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -75,6 +75,7 @@ def add_noop_configuration config.send("#{klass::CONFIG_KEY}=", OPT_OUT) end config.dynamic_csp = OPT_OUT + config.dynamic_csp_report_only = OPT_OUT end add_configuration(NOOP_CONFIGURATION, noop_config) @@ -103,13 +104,13 @@ def deep_copy_if_hash(value) end end - attr_accessor :dynamic_csp + attr_accessor :dynamic_csp, :dynamic_csp_report_only attr_writer :hsts, :x_frame_options, :x_content_type_options, :x_xss_protection, :x_download_options, :x_permitted_cross_domain_policies, :referrer_policy - attr_reader :cached_headers, :csp, :cookies, :hpkp, :hpkp_report_host + attr_reader :cached_headers, :csp, :cookies, :csp_report_only, :hpkp, :hpkp_report_host HASH_CONFIG_FILE = ENV["secure_headers_generated_hashes_file"] || "config/secure_headers_generated_hashes.yml" if File.exists?(HASH_CONFIG_FILE) @@ -122,6 +123,7 @@ def initialize(&block) self.hpkp = OPT_OUT self.referrer_policy = OPT_OUT self.csp = self.class.send(:deep_copy, CSP::DEFAULT_CONFIG) + self.csp_report_only = OPT_OUT instance_eval &block if block_given? end @@ -133,6 +135,8 @@ def dup copy.cookies = @cookies copy.csp = self.class.send(:deep_copy_if_hash, @csp) copy.dynamic_csp = self.class.send(:deep_copy_if_hash, @dynamic_csp) + copy.csp_report_only = self.class.send(:deep_copy_if_hash, @csp_report_only) + copy.dynamic_csp_report_only = self.class.send(:deep_copy_if_hash, @dynamic_csp_report_only) copy.cached_headers = self.class.send(:deep_copy_if_hash, @cached_headers) copy.x_content_type_options = @x_content_type_options copy.hsts = @hsts @@ -149,7 +153,9 @@ def dup def opt_out(header) send("#{header}=", OPT_OUT) if header == CSP::CONFIG_KEY - dynamic_csp = OPT_OUT + self.dynamic_csp = OPT_OUT + elsif header == CSP::REPORT_ONLY_CONFIG_KEY + self.dynamic_csp_report_only = OPT_OUT end self.cached_headers.delete(header) end @@ -160,12 +166,14 @@ def update_x_frame_options(value) end # Public: generated cached headers for a specific user agent. - def rebuild_csp_header_cache!(user_agent) - self.cached_headers[CSP::CONFIG_KEY] = {} - unless current_csp == OPT_OUT + def rebuild_csp_header_cache!(user_agent, header_key) + self.cached_headers[header_key] = {} + + csp = header_key == CSP::CONFIG_KEY ? self.current_csp : self.current_csp_report_only + unless csp == OPT_OUT user_agent = UserAgent.parse(user_agent) variation = CSP.ua_to_variation(user_agent) - self.cached_headers[CSP::CONFIG_KEY][variation] = CSP.make_header(current_csp, user_agent) + self.cached_headers[header_key][variation] = CSP.make_header(csp, user_agent) end end @@ -173,6 +181,10 @@ def current_csp @dynamic_csp || @csp end + def current_csp_report_only + @dynamic_csp_report_only || @csp_report_only + end + # Public: validates all configurations values. # # Raises various configuration errors if any invalid config is detected. @@ -181,6 +193,7 @@ def current_csp def validate_config! StrictTransportSecurity.validate_config!(@hsts) ContentSecurityPolicy.validate_config!(@csp) + ContentSecurityPolicy.validate_config!(@csp_report_only) ReferrerPolicy.validate_config!(@referrer_policy) XFrameOptions.validate_config!(@x_frame_options) XContentTypeOptions.validate_config!(@x_content_type_options) @@ -199,13 +212,36 @@ def secure_cookies=(secure_cookies) protected def csp=(new_csp) + unless new_csp == OPT_OUT + if new_csp[:report_only] + Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" + end + end + if self.dynamic_csp raise IllegalPolicyModificationError, "You are attempting to modify CSP settings directly. Use dynamic_csp= instead." end - @csp = self.class.send(:deep_copy_if_hash, new_csp) end + def csp_report_only=(new_csp) + new_csp = self.class.send(:deep_copy_if_hash, new_csp) + unless new_csp == OPT_OUT + if new_csp[:report_only] == false + Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" + end + + if new_csp[:report_only].nil? + new_csp[:report_only] = true + end + end + + if self.dynamic_csp_report_only + raise IllegalPolicyModificationError, "You are attempting to modify CSP settings directly. Use dynamic_csp_report_only= instead." + end + @csp_report_only = new_csp + end + def cookies=(cookies) @cookies = cookies end @@ -258,12 +294,16 @@ def cache_headers! # # Returns nothing def generate_csp_headers(headers) - unless @csp == OPT_OUT - headers[CSP::CONFIG_KEY] = {} - csp_config = self.current_csp + generate_csp_headers_for_config(headers, CSP::CONFIG_KEY, self.current_csp) + generate_csp_headers_for_config(headers, CSP::REPORT_ONLY_CONFIG_KEY, self.current_csp_report_only) + end + + def generate_csp_headers_for_config(headers, header_key, csp_config) + unless csp_config == OPT_OUT + headers[header_key] = {} CSP::VARIATIONS.each do |name, _| csp = CSP.make_header(csp_config, UserAgent.parse(name)) - headers[CSP::CONFIG_KEY][name] = csp.freeze + headers[header_key][name] = csp.freeze end end end diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index 58e0eb7b..1920e980 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -163,6 +163,7 @@ def self.included(base) }.freeze CONFIG_KEY = :csp + REPORT_ONLY_CONFIG_KEY = :csp_report_only STAR_REGEXP = Regexp.new(Regexp.escape(STAR)) HTTP_SCHEME_REGEX = %r{\Ahttps?://} diff --git a/spec/lib/secure_headers/view_helpers_spec.rb b/spec/lib/secure_headers/view_helpers_spec.rb index 2d8c432a..386129de 100644 --- a/spec/lib/secure_headers/view_helpers_spec.rb +++ b/spec/lib/secure_headers/view_helpers_spec.rb @@ -72,8 +72,11 @@ module SecureHeaders before(:all) do Configuration.default do |config| - config.csp[:script_src] = %w('self') - config.csp[:style_src] = %w('self') + config.csp = { + :default_src => %w('self'), + :script_src => %w('self'), + :style_src => %w('self') + } end end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 4a07c107..dd79093c 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -29,8 +29,11 @@ module SecureHeaders describe "#header_hash_for" do it "allows you to opt out of individual headers via API" do - Configuration.default + Configuration.default do |config| + config.csp_report_only = { default_src: %w('self')} # no default value + end SecureHeaders.opt_out_of_header(request, CSP::CONFIG_KEY) + SecureHeaders.opt_out_of_header(request, CSP::REPORT_ONLY_CONFIG_KEY) SecureHeaders.opt_out_of_header(request, XContentTypeOptions::CONFIG_KEY) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy-Report-Only']).to be_nil @@ -151,14 +154,14 @@ module SecureHeaders expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline' anothercdn.com") end - it "dups global configuration just once when overriding n times and only calls idempotent_additions? once" do + it "dups global configuration just once when overriding n times and only calls idempotent_additions? once per header" do Configuration.default do |config| config.csp = { default_src: %w('self') } end - expect(CSP).to receive(:idempotent_additions?).once + expect(CSP).to receive(:idempotent_additions?).twice # before an override occurs, the env is empty expect(request.env[SECURE_HEADERS_CONFIG]).to be_nil @@ -245,6 +248,81 @@ module SecureHeaders hash = SecureHeaders.header_hash_for(chrome_request) expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src mycdn.com 'nonce-#{nonce}'; style-src 'self'") end + + context "setting two headers" do + it "sets identical values when the configs are the same" do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + config.csp_report_only = { + default_src: %w('self') + } + end + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'") + end + + it "sets different headers when the configs are different" do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + config.csp_report_only = config.csp.merge({script_src: %w('self')}) + end + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self'") + end + + it "allows appending to the enforced policy" do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + config.csp_report_only = config.csp + end + + SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :enforced) + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src 'self' anothercdn.com") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'") + end + + it "allows appending to the report only policy" do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + config.csp_report_only = config.csp + end + + SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :report_only) + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self' anothercdn.com") + end + + it "allows appending to both policies" do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + config.csp_report_only = config.csp + end + + SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :both) + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src 'self' anothercdn.com") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self' anothercdn.com") + end + it "allows overriding the enforced policy" + it "allows overriding the report only policy" + it "allows overriding both policies" + end end end From 2e01a7fdca8c1075d738af595309d9374c49ec68 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Fri, 8 Jul 2016 12:13:48 -1000 Subject: [PATCH 02/29] wip --- lib/secure_headers.rb | 37 ++++++++++++++++++++++++----- lib/secure_headers/configuration.rb | 2 +- spec/lib/secure_headers_spec.rb | 7 +++++- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 940f30dd..002a823c 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -50,13 +50,31 @@ class << self # # additions - a hash containing directives. e.g. # script_src: %w(another-host.com) - def override_content_security_policy_directives(request, additions) + def override_content_security_policy_directives(request, additions, target=:both) + raise_on_unknown_target(target) config = config_for(request) + + puts "before" + puts config.inspect + if config.current_csp == OPT_OUT config.dynamic_csp = {} end + if config.current_csp_report_only == OPT_OUT + config.dynamic_csp_report_only = {} + end + + if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT + binding.pry + config.dynamic_csp = config.current_csp.merge(additions) + end + if [:both, :report_only].include?(target) && config.current_csp_report_only != OPT_OUT + binding.pry + config.dynamic_csp_report_only = config.current_csp_report_only.merge(additions) + end - config.dynamic_csp = config.current_csp.merge(additions) + puts "after" + puts config.inspect override_secure_headers_request_config(request, config) end @@ -67,11 +85,9 @@ def override_content_security_policy_directives(request, additions) # additions - a hash containing directives. e.g. # script_src: %w(another-host.com) def append_content_security_policy_directives(request, additions, target=:both) - unless [:both, :enforced, :report_only].include?(target) - raise "Unrecognized target: #{target}. Must be [:both, :enforced, :report_only]" - end - + raise_on_unknown_target(target) config = config_for(request) + if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT config.dynamic_csp = CSP.combine_policies(config.current_csp, additions) end @@ -118,6 +134,7 @@ def opt_out_of_all_protection(request) # in Rack middleware. def header_hash_for(request) config = config_for(request) + puts "final config#{ config.inspect}" unless ContentSecurityPolicy.idempotent_additions?(config.csp, config.current_csp) config.rebuild_csp_header_cache!(request.user_agent, CSP::CONFIG_KEY) end @@ -126,6 +143,8 @@ def header_hash_for(request) config.rebuild_csp_header_cache!(request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end + puts "final config with cache#{ config.inspect}" + use_cached_headers(config.cached_headers, request) end @@ -176,6 +195,12 @@ def config_for(request) end private + TARGETS = [:both, :enforced, :report_only] + def raise_on_unknown_target(target) + unless TARGETS.include?(target) + raise "Unrecognized target: #{target}. Must be [:both, :enforced, :report_only]" + end + end # Private: gets or creates a nonce for CSP. # diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 6329fc52..cae54af6 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -122,7 +122,7 @@ def deep_copy_if_hash(value) def initialize(&block) self.hpkp = OPT_OUT self.referrer_policy = OPT_OUT - self.csp = self.class.send(:deep_copy, CSP::DEFAULT_CONFIG) + self.csp = OPT_OUT self.csp_report_only = OPT_OUT instance_eval &block if block_given? end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index dd79093c..f6e81936 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -207,9 +207,14 @@ module SecureHeaders end it "overrides non-existant directives" do - Configuration.default + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + end SecureHeaders.override_content_security_policy_directives(request, img_src: [ContentSecurityPolicy::DATA_PROTOCOL]) hash = SecureHeaders.header_hash_for(request) + puts hash expect(hash[CSP::HEADER_NAME]).to eq("default-src https:; img-src data:") end From 7f581bccfdb4460201c6023e2f489b8f4468229e Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Fri, 15 Jul 2016 15:41:06 -1000 Subject: [PATCH 03/29] get csp tests passing --- lib/secure_headers.rb | 38 +++++++++++++++++------------ lib/secure_headers/configuration.rb | 2 +- spec/lib/secure_headers_spec.rb | 9 ++++--- spec/spec_helper.rb | 2 +- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 002a823c..82358fe0 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -50,31 +50,24 @@ class << self # # additions - a hash containing directives. e.g. # script_src: %w(another-host.com) - def override_content_security_policy_directives(request, additions, target=:both) - raise_on_unknown_target(target) - config = config_for(request) - - puts "before" - puts config.inspect + def override_content_security_policy_directives(request, additions, target = nil) + config, target = config_and_target(request, target) if config.current_csp == OPT_OUT config.dynamic_csp = {} end if config.current_csp_report_only == OPT_OUT - config.dynamic_csp_report_only = {} + config.dynamic_csp_report_only = { :report_only => true } end if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT - binding.pry config.dynamic_csp = config.current_csp.merge(additions) end - if [:both, :report_only].include?(target) && config.current_csp_report_only != OPT_OUT - binding.pry + + if [:both, :report_only].include?(target) config.dynamic_csp_report_only = config.current_csp_report_only.merge(additions) end - puts "after" - puts config.inspect override_secure_headers_request_config(request, config) end @@ -84,13 +77,13 @@ def override_content_security_policy_directives(request, additions, target=:both # # additions - a hash containing directives. e.g. # script_src: %w(another-host.com) - def append_content_security_policy_directives(request, additions, target=:both) - raise_on_unknown_target(target) - config = config_for(request) + def append_content_security_policy_directives(request, additions, target = nil) + config, target = config_and_target(request, target) if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT config.dynamic_csp = CSP.combine_policies(config.current_csp, additions) end + if [:both, :report_only].include?(target) && config.current_csp_report_only != OPT_OUT config.dynamic_csp_report_only = CSP.combine_policies(config.current_csp_report_only, additions) end @@ -202,6 +195,21 @@ def raise_on_unknown_target(target) end end + def config_and_target(request, target) + config = config_for(request) + target = guess_target(config) unless target + raise_on_unknown_target(target) + [config, target] + end + + def guess_target(config) + if config.csp + :enforced + elsif config.csp_report_only + :report_only + end + end + # Private: gets or creates a nonce for CSP. # # Returns the nonce diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index cae54af6..2fc99fb7 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -84,6 +84,7 @@ def add_noop_configuration # Public: perform a basic deep dup. The shallow copy provided by dup/clone # can lead to modifying parent objects. def deep_copy(config) + return unless config config.each_with_object({}) do |(key, value), hash| hash[key] = if value.is_a?(Array) value.dup @@ -217,7 +218,6 @@ def csp=(new_csp) Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" end end - if self.dynamic_csp raise IllegalPolicyModificationError, "You are attempting to modify CSP settings directly. Use dynamic_csp= instead." end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index f6e81936..3c99bec3 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -99,9 +99,9 @@ module SecureHeaders firefox_request = Rack::Request.new(request.env.merge("HTTP_USER_AGENT" => USER_AGENTS[:firefox])) # append an unsupported directive - SecureHeaders.override_content_security_policy_directives(firefox_request, plugin_types: %w(flash)) + SecureHeaders.override_content_security_policy_directives(firefox_request, {plugin_types: %w(flash)}, :enforced) # append a supported directive - SecureHeaders.override_content_security_policy_directives(firefox_request, script_src: %w('self')) + SecureHeaders.override_content_security_policy_directives(firefox_request, {script_src: %w('self')}, :enforced) hash = SecureHeaders.header_hash_for(firefox_request) @@ -178,7 +178,9 @@ module SecureHeaders end it "doesn't allow you to muck with csp configs when a dynamic policy is in use" do - default_config = Configuration.default + default_config = Configuration.default do |config| + config.csp = { default_src: %w('none')}.freeze + end expect { default_config.csp = {} }.to raise_error(NoMethodError) # config is frozen @@ -215,6 +217,7 @@ module SecureHeaders SecureHeaders.override_content_security_policy_directives(request, img_src: [ContentSecurityPolicy::DATA_PROTOCOL]) hash = SecureHeaders.header_hash_for(request) puts hash + expect(hash[CSP::REPORT_ONLY]).to be_nil expect(hash[CSP::HEADER_NAME]).to eq("default-src https:; img-src data:") end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9658eb89..637e12bb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,7 +25,6 @@ } def expect_default_values(hash) - expect(hash[SecureHeaders::CSP::HEADER_NAME]).to eq(SecureHeaders::CSP::DEFAULT_VALUE) expect(hash[SecureHeaders::XFrameOptions::HEADER_NAME]).to eq(SecureHeaders::XFrameOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XDownloadOptions::HEADER_NAME]).to eq(SecureHeaders::XDownloadOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::StrictTransportSecurity::HEADER_NAME]).to eq(SecureHeaders::StrictTransportSecurity::DEFAULT_VALUE) @@ -33,6 +32,7 @@ def expect_default_values(hash) expect(hash[SecureHeaders::XContentTypeOptions::HEADER_NAME]).to eq(SecureHeaders::XContentTypeOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XPermittedCrossDomainPolicies::HEADER_NAME]).to eq(SecureHeaders::XPermittedCrossDomainPolicies::DEFAULT_VALUE) expect(hash[SecureHeaders::ReferrerPolicy::HEADER_NAME]).to be_nil + expect(hash[SecureHeaders::CSP::HEADER_NAME]).to be(nil) end module SecureHeaders From 72a74ccd97190bba2eb6b355937d6c9a6072c5d1 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Sun, 17 Jul 2016 22:47:11 -0600 Subject: [PATCH 04/29] wip' --- lib/secure_headers.rb | 7 +++++-- spec/lib/secure_headers/configuration_spec.rb | 4 +++- spec/lib/secure_headers/headers/policy_management_spec.rb | 7 ++++++- spec/lib/secure_headers_spec.rb | 3 +++ spec/spec_helper.rb | 2 +- 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 82358fe0..361d1307 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -127,16 +127,19 @@ def opt_out_of_all_protection(request) # in Rack middleware. def header_hash_for(request) config = config_for(request) - puts "final config#{ config.inspect}" + puts "\nfinal config\n#{ config.inspect}" unless ContentSecurityPolicy.idempotent_additions?(config.csp, config.current_csp) config.rebuild_csp_header_cache!(request.user_agent, CSP::CONFIG_KEY) end + puts "\nafter enforced changes\n#{ config.inspect}" + unless ContentSecurityPolicy.idempotent_additions?(config.csp_report_only, config.current_csp_report_only) config.rebuild_csp_header_cache!(request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end - puts "final config with cache#{ config.inspect}" + puts "\nfinal config with cache\n#{ config.inspect}" + pp config.cached_headers use_cached_headers(config.cached_headers, request) end diff --git a/spec/lib/secure_headers/configuration_spec.rb b/spec/lib/secure_headers/configuration_spec.rb index 0f82c50e..f50a0225 100644 --- a/spec/lib/secure_headers/configuration_spec.rb +++ b/spec/lib/secure_headers/configuration_spec.rb @@ -4,7 +4,9 @@ module SecureHeaders describe Configuration do before(:each) do reset_config - Configuration.default + Configuration.default do |config| + config.csp = { default_src: %w(https:) } + end end it "has a default config" do diff --git a/spec/lib/secure_headers/headers/policy_management_spec.rb b/spec/lib/secure_headers/headers/policy_management_spec.rb index 8ead3137..b91e1fc3 100644 --- a/spec/lib/secure_headers/headers/policy_management_spec.rb +++ b/spec/lib/secure_headers/headers/policy_management_spec.rb @@ -101,7 +101,12 @@ module SecureHeaders describe "#combine_policies" do it "combines the default-src value with the override if the directive was unconfigured" do - combined_config = CSP.combine_policies(Configuration.default.csp, script_src: %w(anothercdn.com)) + Configuration.default do |config| + config.csp = { + default_src: %w(https:) + } + end + combined_config = CSP.combine_policies(Configuration.get.csp, script_src: %w(anothercdn.com)) csp = ContentSecurityPolicy.new(combined_config) expect(csp.name).to eq(CSP::HEADER_NAME) expect(csp.value).to eq("default-src https:; script-src https: anothercdn.com") diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 3c99bec3..ccd5aa3e 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -105,6 +105,8 @@ module SecureHeaders hash = SecureHeaders.header_hash_for(firefox_request) + pp hash + # child-src is translated to frame-src expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; frame-src 'self'; script-src 'self'") end @@ -188,6 +190,7 @@ module SecureHeaders SecureHeaders.append_content_security_policy_directives(request, script_src: %w(anothercdn.com)) new_config = SecureHeaders.config_for(request) + $debug = true expect { new_config.send(:csp=, {}) }.to raise_error(Configuration::IllegalPolicyModificationError) expect do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 637e12bb..d8603f26 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,6 +25,7 @@ } def expect_default_values(hash) + expect(hash[SecureHeaders::CSP::HEADER_NAME]).to be(nil) expect(hash[SecureHeaders::XFrameOptions::HEADER_NAME]).to eq(SecureHeaders::XFrameOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XDownloadOptions::HEADER_NAME]).to eq(SecureHeaders::XDownloadOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::StrictTransportSecurity::HEADER_NAME]).to eq(SecureHeaders::StrictTransportSecurity::DEFAULT_VALUE) @@ -32,7 +33,6 @@ def expect_default_values(hash) expect(hash[SecureHeaders::XContentTypeOptions::HEADER_NAME]).to eq(SecureHeaders::XContentTypeOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XPermittedCrossDomainPolicies::HEADER_NAME]).to eq(SecureHeaders::XPermittedCrossDomainPolicies::DEFAULT_VALUE) expect(hash[SecureHeaders::ReferrerPolicy::HEADER_NAME]).to be_nil - expect(hash[SecureHeaders::CSP::HEADER_NAME]).to be(nil) end module SecureHeaders From 48f92000870478d1156e80a2e24241b8b40a66f2 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Tue, 26 Jul 2016 13:12:33 -1000 Subject: [PATCH 05/29] initial work for making csp config an actual class --- .../headers/content_security_policy.rb | 41 +++++++++++-------- .../headers/policy_management.rb | 5 +++ spec/lib/secure_headers/configuration_spec.rb | 4 +- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index efee3428..c92469d0 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -1,8 +1,8 @@ require_relative 'policy_management' +require_relative 'content_security_policy_config' require 'useragent' module SecureHeaders - class ContentSecurityPolicyConfigError < StandardError; end class ContentSecurityPolicy include PolicyManagement @@ -10,17 +10,22 @@ class ContentSecurityPolicy VERSION_46 = ::UserAgent::Version.new("46") def initialize(config = nil, user_agent = OTHER) - @config = Configuration.send(:deep_copy, config || DEFAULT_CONFIG) + @config = if config.is_a?(Hash) + ContentSecurityPolicyConfig.new(Configuration.send(:deep_copy, config || DEFAULT_CONFIG)) + else + config + end + @parsed_ua = if user_agent.is_a?(UserAgent::Browsers::Base) user_agent else UserAgent.parse(user_agent) end normalize_child_frame_src - @report_only = @config[:report_only] - @preserve_schemes = @config[:preserve_schemes] - @script_nonce = @config[:script_nonce] - @style_nonce = @config[:style_nonce] + @report_only = @config.report_only + @preserve_schemes = @config.preserve_schemes + @script_nonce = @config.script_nonce + @style_nonce = @config.style_nonce end ## @@ -49,16 +54,16 @@ def value # frame-src is deprecated, child-src is being implemented. They are # very similar and in most cases, the same value can be used for both. def normalize_child_frame_src - if @config[:frame_src] && @config[:child_src] && @config[:frame_src] != @config[:child_src] + if @config.frame_src && @config.child_src && @config.frame_src != @config.child_src Kernel.warn("#{Kernel.caller.first}: [DEPRECATION] both :child_src and :frame_src supplied and do not match. This can lead to inconsistent behavior across browsers.") - elsif @config[:frame_src] - Kernel.warn("#{Kernel.caller.first}: [DEPRECATION] :frame_src is deprecated, use :child_src instead. Provided: #{@config[:frame_src]}.") + elsif @config.frame_src + Kernel.warn("#{Kernel.caller.first}: [DEPRECATION] :frame_src is deprecated, use :child_src instead. Provided: #{@config.frame_src}.") end if supported_directives.include?(:child_src) - @config[:child_src] = @config[:child_src] || @config[:frame_src] + @config.child_src = @config.child_src || @config.frame_src else - @config[:frame_src] = @config[:frame_src] || @config[:child_src] + @config.frame_src = @config.frame_src || @config.child_src end end @@ -73,9 +78,9 @@ def build_value directives.map do |directive_name| case DIRECTIVE_VALUE_TYPES[directive_name] when :boolean - symbol_to_hyphen_case(directive_name) if @config[directive_name] + symbol_to_hyphen_case(directive_name) if @config.directive_value(directive_name) when :string - [symbol_to_hyphen_case(directive_name), @config[directive_name]].join(" ") + [symbol_to_hyphen_case(directive_name), @config.directive_value(directive_name)].join(" ") else build_directive(directive_name) end @@ -88,9 +93,9 @@ def build_value # # Returns a string representing a directive. def build_directive(directive) - return if @config[directive].nil? + return if @config.directive_value(directive).nil? - source_list = @config[directive].compact + source_list = @config.directive_value(directive).compact return if source_list.empty? normalized_source_list = minify_source_list(directive, source_list) @@ -170,9 +175,11 @@ def append_nonce(source_list, nonce) # Private: return the list of directives that are supported by the user agent, # starting with default-src and ending with report-uri. def directives - [DEFAULT_SRC, + [ + DEFAULT_SRC, BODY_DIRECTIVES.select { |key| supported_directives.include?(key) }, - REPORT_URI].flatten.select { |directive| @config.key?(directive) } + REPORT_URI + ].flatten.select { |directive| @config.directive_value(directive) } end # Private: Remove scheme from source expressions. diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index 1920e980..9331de48 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -180,6 +180,11 @@ def self.included(base) :preserve_schemes ].freeze + NONCES = [ + :script_nonce, + :style_nonce + ].freeze + module ClassMethods # Public: generate a header name, value array that is user-agent-aware. # diff --git a/spec/lib/secure_headers/configuration_spec.rb b/spec/lib/secure_headers/configuration_spec.rb index f50a0225..0f82c50e 100644 --- a/spec/lib/secure_headers/configuration_spec.rb +++ b/spec/lib/secure_headers/configuration_spec.rb @@ -4,9 +4,7 @@ module SecureHeaders describe Configuration do before(:each) do reset_config - Configuration.default do |config| - config.csp = { default_src: %w(https:) } - end + Configuration.default end it "has a default config" do From e9fa410a7ba14b2607530ec84b5fba8f2b5633ca Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 27 Jul 2016 12:19:03 -1000 Subject: [PATCH 06/29] wip --- lib/secure_headers.rb | 5 +- .../headers/content_security_policy_config.rb | 67 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 lib/secure_headers/headers/content_security_policy_config.rb diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 361d1307..94477af8 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -18,7 +18,10 @@ # All headers (except for hpkp) have a default value. Provide SecureHeaders::OPT_OUT # or ":optout_of_protection" as a config value to disable a given header module SecureHeaders - OPT_OUT = :opt_out_of_protection + class NoOpHeaderConfig + end + + OPT_OUT = NoOpHeaderConfig.new SECURE_HEADERS_CONFIG = "secure_headers_request_config".freeze NONCE_KEY = "secure_headers_content_security_policy_nonce".freeze HTTPS = "https".freeze diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb new file mode 100644 index 00000000..981469d8 --- /dev/null +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -0,0 +1,67 @@ +module SecureHeaders + module DynamicConfig + def self.included(base) + base.send(:attr_reader, *base.attrs) + base.attrs.each do |attr| + base.send(:define_method, "#{attr}=") do |value| + if self.class.attrs.include?(attr) + if PolicyManagement::DIRECTIVE_VALUE_TYPES[k] == :source_list + instance_variable_set("@{k}=", value.dup) + self.send("#{attr}=", value.dup) + else + self.send("#{attr}=", value) + end + else + raise "Unknown config directive: #{attr}" + end + + @modified = true + end + end + end + + def initialize(hash) + hash.keys.map do |k| + next unless hash[k] + if self.class.attrs.include?(k) + if PolicyManagement::DIRECTIVE_VALUE_TYPES[k] == :source_list + self.send("#{k}=", hash[k].dup) + else + self.send("#{k}=", hash[k]) + end + else + binding.pry + raise "Unknown config directive: #{k}" + end + end + end + + def update_directive(directive, value) + if self.class.attrs.include?(directive) + if PolicyManagement::DIRECTIVE_VALUE_TYPES[k] == :source_list + instance_variable_set("@{k}=", hash[k].dup) + else + self.send("#{k}=", hash[k]) + end + end + + + end + + def directive_value(directive) + if self.class.attrs.include?(directive) + self.send(directive) + end + end + end + + class ContentSecurityPolicyConfigError < StandardError; end + class ContentSecurityPolicyConfig + def self.attrs + PolicyManagement::ALL_DIRECTIVES + PolicyManagement::META_CONFIGS + PolicyManagement::NONCES + end + + include DynamicConfig + alias_method :report_only?, :report_only + end +end From 27f27439de8b7e12c6f15111c825bc66395fd0d3 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Mon, 8 Aug 2016 13:21:15 -1000 Subject: [PATCH 07/29] base spec is passing --- lib/secure_headers.rb | 7 ++ lib/secure_headers/configuration.rb | 8 +-- .../headers/content_security_policy.rb | 41 +++++++---- .../headers/content_security_policy_config.rb | 68 +++++++++++-------- .../headers/policy_management.rb | 6 +- spec/lib/secure_headers/configuration_spec.rb | 4 +- spec/spec_helper.rb | 2 +- 7 files changed, 84 insertions(+), 52 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 94477af8..347ae28d 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -19,6 +19,13 @@ # or ":optout_of_protection" as a config value to disable a given header module SecureHeaders class NoOpHeaderConfig + def boom + raise "Illegal State: attempted to modify NoOpHeaderConfig. Create a new config instead." + end + + alias_method :[], :boom + alias_method :[]=, :boom + alias_method :keys, :boom end OPT_OUT = NoOpHeaderConfig.new diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 2fc99fb7..f8e6a36e 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -123,7 +123,7 @@ def deep_copy_if_hash(value) def initialize(&block) self.hpkp = OPT_OUT self.referrer_policy = OPT_OUT - self.csp = OPT_OUT + self.csp = ContentSecurityPolicyConfig::DEFAULT self.csp_report_only = OPT_OUT instance_eval &block if block_given? end @@ -134,10 +134,8 @@ def initialize(&block) def dup copy = self.class.new copy.cookies = @cookies - copy.csp = self.class.send(:deep_copy_if_hash, @csp) - copy.dynamic_csp = self.class.send(:deep_copy_if_hash, @dynamic_csp) - copy.csp_report_only = self.class.send(:deep_copy_if_hash, @csp_report_only) - copy.dynamic_csp_report_only = self.class.send(:deep_copy_if_hash, @dynamic_csp_report_only) + copy.csp = @csp.dup + copy.csp_report_only = @csp_report_only.dup copy.cached_headers = self.class.send(:deep_copy_if_hash, @cached_headers) copy.x_content_type_options = @x_content_type_options copy.hsts = @hsts diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index c92469d0..efb60bea 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -11,7 +11,11 @@ class ContentSecurityPolicy def initialize(config = nil, user_agent = OTHER) @config = if config.is_a?(Hash) - ContentSecurityPolicyConfig.new(Configuration.send(:deep_copy, config || DEFAULT_CONFIG)) + if config[:report_only] + ContentSecurityPolicyReportOnlyConfig.new(config || DEFAULT_CONFIG) + else + ContentSecurityPolicyConfig.new(config || DEFAULT_CONFIG) + end else config end @@ -21,8 +25,7 @@ def initialize(config = nil, user_agent = OTHER) else UserAgent.parse(user_agent) end - normalize_child_frame_src - @report_only = @config.report_only + @frame_src = normalize_child_frame_src @preserve_schemes = @config.preserve_schemes @script_nonce = @config.script_nonce @style_nonce = @config.style_nonce @@ -32,7 +35,7 @@ def initialize(config = nil, user_agent = OTHER) # Returns the name to use for the header. Either "Content-Security-Policy" or # "Content-Security-Policy-Report-Only" def name - if @report_only + if @config.report_only? REPORT_ONLY else HEADER_NAME @@ -61,9 +64,9 @@ def normalize_child_frame_src end if supported_directives.include?(:child_src) - @config.child_src = @config.child_src || @config.frame_src + @config.child_src || @config.frame_src else - @config.frame_src = @config.frame_src || @config.child_src + @config.frame_src || @config.child_src end end @@ -76,8 +79,10 @@ def normalize_child_frame_src # Returns a content security policy header value. def build_value directives.map do |directive_name| + puts directive_name case DIRECTIVE_VALUE_TYPES[directive_name] when :boolean + puts " #{@config.directive_value(directive_name)}" symbol_to_hyphen_case(directive_name) if @config.directive_value(directive_name) when :string [symbol_to_hyphen_case(directive_name), @config.directive_value(directive_name)].join(" ") @@ -93,11 +98,20 @@ def build_value # # Returns a string representing a directive. def build_directive(directive) - return if @config.directive_value(directive).nil? - - source_list = @config.directive_value(directive).compact - return if source_list.empty? - + source_list = case directive + when :child_src + if supported_directives.include?(:child_src) + @frame_src + end + when :frame_src + unless supported_directives.include?(:child_src) + @frame_src + end + else + @config.directive_value(directive) + end + puts " #{directive}=#{source_list}" + return unless source_list && source_list.any? normalized_source_list = minify_source_list(directive, source_list) [symbol_to_hyphen_case(directive), normalized_source_list].join(" ") end @@ -106,6 +120,7 @@ def build_directive(directive) # If a directive contains 'none' but has other values, 'none' is ommitted. # Schemes are stripped (see http://www.w3.org/TR/CSP2/#match-source-expression) def minify_source_list(directive, source_list) + source_list.compact! if source_list.include?(STAR) keep_wildcard_sources(source_list) else @@ -115,7 +130,7 @@ def minify_source_list(directive, source_list) unless directive == REPORT_URI || @preserve_schemes strip_source_schemes!(source_list) end - dedup_source_list(source_list).join(" ") + dedup_source_list(source_list) end end @@ -179,7 +194,7 @@ def directives DEFAULT_SRC, BODY_DIRECTIVES.select { |key| supported_directives.include?(key) }, REPORT_URI - ].flatten.select { |directive| @config.directive_value(directive) } + ].flatten end # Private: Remove scheme from source expressions. diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index 981469d8..4e880986 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -5,47 +5,29 @@ def self.included(base) base.attrs.each do |attr| base.send(:define_method, "#{attr}=") do |value| if self.class.attrs.include?(attr) - if PolicyManagement::DIRECTIVE_VALUE_TYPES[k] == :source_list - instance_variable_set("@{k}=", value.dup) - self.send("#{attr}=", value.dup) - else - self.send("#{attr}=", value) + value = value.dup if PolicyManagement::DIRECTIVE_VALUE_TYPES[attr] == :source_list + prev_value = self.instance_variable_get("@#{attr}") + self.instance_variable_set("@#{attr}", value) + if prev_value != self.instance_variable_get("@#{attr}") + @modified = true end else - raise "Unknown config directive: #{attr}" + raise ContentSecurityPolicyConfigError, "Unknown config directive: #{attr}=#{value}" end - - @modified = true end end end def initialize(hash) - hash.keys.map do |k| - next unless hash[k] - if self.class.attrs.include?(k) - if PolicyManagement::DIRECTIVE_VALUE_TYPES[k] == :source_list - self.send("#{k}=", hash[k].dup) - else - self.send("#{k}=", hash[k]) - end - else - binding.pry - raise "Unknown config directive: #{k}" - end + hash.keys.reject { |k| hash[k].nil? }.map do |k| + self.send("#{k}=", hash[k]) end + + @modified = false end def update_directive(directive, value) - if self.class.attrs.include?(directive) - if PolicyManagement::DIRECTIVE_VALUE_TYPES[k] == :source_list - instance_variable_set("@{k}=", hash[k].dup) - else - self.send("#{k}=", hash[k]) - end - end - - + self.send("#{k}=", value) end def directive_value(directive) @@ -53,6 +35,13 @@ def directive_value(directive) self.send(directive) end end + + def modified? + @modified + end + + alias_method :[], :directive_value + alias_method :[]=, :update_directive end class ContentSecurityPolicyConfigError < StandardError; end @@ -62,6 +51,25 @@ def self.attrs end include DynamicConfig - alias_method :report_only?, :report_only + + # based on what was suggested in https://github.com/rails/rails/pull/24961/files + DEFAULT = { + default_src: %w('self' https:), + font_src: %w('self' https: data:), + img_src: %w('self' https: data:), + object_src: %w('none'), + script_src: %w(https:), + style_src: %w('self' https: 'unsafe-inline') + } + + def report_only? + false + end + end + + class ContentSecurityPolicyReportOnlyConfig < ContentSecurityPolicyConfig + def report_only? + true + end end end diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index 9331de48..e8a53796 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -156,7 +156,7 @@ def self.included(base) PLUGIN_TYPES => :source_list, REFLECTED_XSS => :string, REPORT_URI => :source_list, - SANDBOX => :string, + SANDBOX => :source_list, SCRIPT_SRC => :source_list, STYLE_SRC => :source_list, UPGRADE_INSECURE_REQUESTS => :boolean @@ -202,7 +202,9 @@ def make_header(config, user_agent) def validate_config!(config) return if config.nil? || config == OPT_OUT raise ContentSecurityPolicyConfigError.new(":default_src is required") unless config[:default_src] - config.each do |key, value| + ContentSecurityPolicyConfig.attrs.each do |key| + value = config.directive_value(key) + next unless value if META_CONFIGS.include?(key) raise ContentSecurityPolicyConfigError.new("#{key} must be a boolean value") unless boolean?(value) || value.nil? else diff --git a/spec/lib/secure_headers/configuration_spec.rb b/spec/lib/secure_headers/configuration_spec.rb index 0f82c50e..b7addd4b 100644 --- a/spec/lib/secure_headers/configuration_spec.rb +++ b/spec/lib/secure_headers/configuration_spec.rb @@ -4,7 +4,9 @@ module SecureHeaders describe Configuration do before(:each) do reset_config - Configuration.default + Configuration.default do |config| + config.csp = ContentSecurityPolicyConfig.new(:default_src => %w(https:)) + end end it "has a default config" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d8603f26..b8e31bda 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,7 +25,7 @@ } def expect_default_values(hash) - expect(hash[SecureHeaders::CSP::HEADER_NAME]).to be(nil) + expect(hash[SecureHeaders::CSP::HEADER_NAME]).to be("default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline'") expect(hash[SecureHeaders::XFrameOptions::HEADER_NAME]).to eq(SecureHeaders::XFrameOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XDownloadOptions::HEADER_NAME]).to eq(SecureHeaders::XDownloadOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::StrictTransportSecurity::HEADER_NAME]).to eq(SecureHeaders::StrictTransportSecurity::DEFAULT_VALUE) From dbea8c6bf663de8bf5c91f17fca3899f67efbea8 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Mon, 8 Aug 2016 14:03:14 -1000 Subject: [PATCH 08/29] specs for the configuration class now pass --- lib/secure_headers.rb | 15 ++++- lib/secure_headers/configuration.rb | 67 +++++++------------ .../headers/content_security_policy.rb | 2 + .../headers/content_security_policy_config.rb | 24 ++++++- .../headers/policy_management.rb | 4 +- spec/lib/secure_headers/configuration_spec.rb | 14 ++-- .../headers/policy_management_spec.rb | 32 ++++----- spec/spec_helper.rb | 2 +- 8 files changed, 89 insertions(+), 71 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 347ae28d..de422b4a 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -14,21 +14,32 @@ require "secure_headers/railtie" require "secure_headers/view_helper" require "useragent" +require "singleton" # All headers (except for hpkp) have a default value. Provide SecureHeaders::OPT_OUT # or ":optout_of_protection" as a config value to disable a given header module SecureHeaders class NoOpHeaderConfig - def boom + include Singleton + + def boom(arg = nil) raise "Illegal State: attempted to modify NoOpHeaderConfig. Create a new config instead." end + def to_h + {} + end + + def dup + self.class.instance + end + alias_method :[], :boom alias_method :[]=, :boom alias_method :keys, :boom end - OPT_OUT = NoOpHeaderConfig.new + OPT_OUT = NoOpHeaderConfig.instance SECURE_HEADERS_CONFIG = "secure_headers_request_config".freeze NONCE_KEY = "secure_headers_content_security_policy_nonce".freeze HTTPS = "https".freeze diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index f8e6a36e..c667b90c 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -74,8 +74,6 @@ def add_noop_configuration ALL_HEADER_CLASSES.each do |klass| config.send("#{klass::CONFIG_KEY}=", OPT_OUT) end - config.dynamic_csp = OPT_OUT - config.dynamic_csp_report_only = OPT_OUT end add_configuration(NOOP_CONFIGURATION, noop_config) @@ -105,8 +103,6 @@ def deep_copy_if_hash(value) end end - attr_accessor :dynamic_csp, :dynamic_csp_report_only - attr_writer :hsts, :x_frame_options, :x_content_type_options, :x_xss_protection, :x_download_options, :x_permitted_cross_domain_policies, :referrer_policy @@ -134,8 +130,8 @@ def initialize(&block) def dup copy = self.class.new copy.cookies = @cookies - copy.csp = @csp.dup - copy.csp_report_only = @csp_report_only.dup + copy.csp = @csp.dup if @csp + copy.csp_report_only = @csp_report_only.dup if @csp_report_only copy.cached_headers = self.class.send(:deep_copy_if_hash, @cached_headers) copy.x_content_type_options = @x_content_type_options copy.hsts = @hsts @@ -151,11 +147,6 @@ def dup def opt_out(header) send("#{header}=", OPT_OUT) - if header == CSP::CONFIG_KEY - self.dynamic_csp = OPT_OUT - elsif header == CSP::REPORT_ONLY_CONFIG_KEY - self.dynamic_csp_report_only = OPT_OUT - end self.cached_headers.delete(header) end @@ -168,7 +159,7 @@ def update_x_frame_options(value) def rebuild_csp_header_cache!(user_agent, header_key) self.cached_headers[header_key] = {} - csp = header_key == CSP::CONFIG_KEY ? self.current_csp : self.current_csp_report_only + csp = header_key == CSP::CONFIG_KEY ? self.csp : self.csp_report_only unless csp == OPT_OUT user_agent = UserAgent.parse(user_agent) variation = CSP.ua_to_variation(user_agent) @@ -176,14 +167,6 @@ def rebuild_csp_header_cache!(user_agent, header_key) end end - def current_csp - @dynamic_csp || @csp - end - - def current_csp_report_only - @dynamic_csp_report_only || @csp_report_only - end - # Public: validates all configurations values. # # Raises various configuration errors if any invalid config is detected. @@ -211,33 +194,35 @@ def secure_cookies=(secure_cookies) protected def csp=(new_csp) - unless new_csp == OPT_OUT - if new_csp[:report_only] - Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" + if new_csp == OPT_OUT + @csp = OPT_OUT + else + if new_csp.is_a?(ContentSecurityPolicyConfig) + @csp = new_csp + else + if new_csp[:report_only] + Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" + self.csp_report_only = new_csp + else + @csp = ContentSecurityPolicyConfig.new(new_csp) + end end end - if self.dynamic_csp - raise IllegalPolicyModificationError, "You are attempting to modify CSP settings directly. Use dynamic_csp= instead." - end - @csp = self.class.send(:deep_copy_if_hash, new_csp) end def csp_report_only=(new_csp) - new_csp = self.class.send(:deep_copy_if_hash, new_csp) - unless new_csp == OPT_OUT - if new_csp[:report_only] == false - Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" - end - - if new_csp[:report_only].nil? - new_csp[:report_only] = true + if new_csp == OPT_OUT + @csp_report_only = OPT_OUT + else + if new_csp.is_a?(Hash) + new_csp = ContentSecurityPolicyReportOnlyConfig.new(new_csp) + if new_csp[:report_only] == false + Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" + end end - end - if self.dynamic_csp_report_only - raise IllegalPolicyModificationError, "You are attempting to modify CSP settings directly. Use dynamic_csp_report_only= instead." + @csp_report_only = new_csp end - @csp_report_only = new_csp end def cookies=(cookies) @@ -292,8 +277,8 @@ def cache_headers! # # Returns nothing def generate_csp_headers(headers) - generate_csp_headers_for_config(headers, CSP::CONFIG_KEY, self.current_csp) - generate_csp_headers_for_config(headers, CSP::REPORT_ONLY_CONFIG_KEY, self.current_csp_report_only) + generate_csp_headers_for_config(headers, CSP::CONFIG_KEY, self.csp) + generate_csp_headers_for_config(headers, CSP::REPORT_ONLY_CONFIG_KEY, self.csp_report_only) end def generate_csp_headers_for_config(headers, header_key, csp_config) diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index efb60bea..c172150f 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -16,6 +16,8 @@ def initialize(config = nil, user_agent = OTHER) else ContentSecurityPolicyConfig.new(config || DEFAULT_CONFIG) end + elsif config.nil? + ContentSecurityPolicyConfig.new(DEFAULT_CONFIG) else config end diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index 4e880986..daf6d4e6 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -20,7 +20,11 @@ def self.included(base) def initialize(hash) hash.keys.reject { |k| hash[k].nil? }.map do |k| - self.send("#{k}=", hash[k]) + if self.class.attrs.include?(k) + self.send("#{k}=", hash[k]) + else + raise ContentSecurityPolicyConfigError, "Unknown config directive: #{k}=#{hash[k]}" + end end @modified = false @@ -40,6 +44,24 @@ def modified? @modified end + def merge(new_hash) + self.class.new(CSP.combine_policies(self.to_h, new_hash)) + end + + def to_h + self.class.attrs.each_with_object({}) do |key, hash| + hash[key] = self.send(key) + end.reject { |_, v| v.nil? } + end + + def dup + self.class.new(self.to_h) + end + + def ==(o) + self.class == o.class && self.to_h == o.to_h + end + alias_method :[], :directive_value alias_method :[]=, :update_directive end diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index e8a53796..e9921929 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -201,7 +201,7 @@ def make_header(config, user_agent) # script_src => h*t*t*p: will not raise an exception) def validate_config!(config) return if config.nil? || config == OPT_OUT - raise ContentSecurityPolicyConfigError.new(":default_src is required") unless config[:default_src] + raise ContentSecurityPolicyConfigError.new(":default_src is required") unless config.directive_value(:default_src) ContentSecurityPolicyConfig.attrs.each do |key| value = config.directive_value(key) next unless value @@ -239,7 +239,7 @@ def idempotent_additions?(config, additions) # 3. if a value in additions does exist in the original config, the two # values are joined. def combine_policies(original, additions) - if original == OPT_OUT + if original == {} raise ContentSecurityPolicyConfigError.new("Attempted to override an opt-out CSP config.") end diff --git a/spec/lib/secure_headers/configuration_spec.rb b/spec/lib/secure_headers/configuration_spec.rb index b7addd4b..40b285e5 100644 --- a/spec/lib/secure_headers/configuration_spec.rb +++ b/spec/lib/secure_headers/configuration_spec.rb @@ -4,9 +4,7 @@ module SecureHeaders describe Configuration do before(:each) do reset_config - Configuration.default do |config| - config.csp = ContentSecurityPolicyConfig.new(:default_src => %w(https:)) - end + Configuration.default end it "has a default config" do @@ -38,8 +36,8 @@ module SecureHeaders config = Configuration.get(:test_override) noop = Configuration.get(Configuration::NOOP_CONFIGURATION) - [:csp, :dynamic_csp, :cookies].each do |key| - expect(config.send(key)).to eq(noop.send(key)), "Value not copied: #{key}." + [:csp, :csp_report_only, :cookies].each do |key| + expect(config.send(key)).to eq(noop.send(key)) end end @@ -67,7 +65,7 @@ module SecureHeaders default = Configuration.get override = Configuration.get(:override) - expect(override.csp).not_to eq(default.csp) + expect(override.csp.directive_value(:default_src)).not_to be(default.csp.directive_value(:default_src)) end it "allows you to override an override" do @@ -80,9 +78,9 @@ module SecureHeaders end original_override = Configuration.get(:override) - expect(original_override.csp).to eq(default_src: %w('self')) + expect(original_override.csp.to_h).to eq(default_src: %w('self')) override_config = Configuration.get(:second_override) - expect(override_config.csp).to eq(default_src: %w('self'), script_src: %w(example.org)) + expect(override_config.csp.to_h).to eq(default_src: %w('self'), script_src: %w('self' example.org)) end it "deprecates the secure_cookies configuration" do diff --git a/spec/lib/secure_headers/headers/policy_management_spec.rb b/spec/lib/secure_headers/headers/policy_management_spec.rb index b91e1fc3..e92dbdd5 100644 --- a/spec/lib/secure_headers/headers/policy_management_spec.rb +++ b/spec/lib/secure_headers/headers/policy_management_spec.rb @@ -40,61 +40,61 @@ module SecureHeaders report_uri: %w(https://example.com/uri-directive) } - CSP.validate_config!(config) + CSP.validate_config!(ContentSecurityPolicyConfig.new(config)) end it "requires a :default_src value" do expect do - CSP.validate_config!(script_src: %('self')) + CSP.validate_config!(ContentSecurityPolicyConfig.new(script_src: %('self'))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :report_only to be a truthy value" do expect do - CSP.validate_config!(default_opts.merge(report_only: "steve")) + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(report_only: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :preserve_schemes to be a truthy value" do expect do - CSP.validate_config!(default_opts.merge(preserve_schemes: "steve")) + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(preserve_schemes: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :block_all_mixed_content to be a boolean value" do expect do - CSP.validate_config!(default_opts.merge(block_all_mixed_content: "steve")) + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(block_all_mixed_content: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :upgrade_insecure_requests to be a boolean value" do expect do - CSP.validate_config!(default_opts.merge(upgrade_insecure_requests: "steve")) + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(upgrade_insecure_requests: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires all source lists to be an array of strings" do expect do - CSP.validate_config!(default_src: "steve") + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: "steve")) end.to raise_error(ContentSecurityPolicyConfigError) end it "allows nil values" do expect do - CSP.validate_config!(default_src: %w('self'), script_src: ["https:", nil]) + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w('self'), script_src: ["https:", nil])) end.to_not raise_error end it "rejects unknown directives / config" do expect do - CSP.validate_config!(default_src: %w('self'), default_src_totally_mispelled: "steve") + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w('self'), default_src_totally_mispelled: "steve")) end.to raise_error(ContentSecurityPolicyConfigError) end # this is mostly to ensure people don't use the antiquated shorthands common in other configs it "performs light validation on source lists" do expect do - CSP.validate_config!(default_src: %w(self none inline eval)) + CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w(self none inline eval))) end.to raise_error(ContentSecurityPolicyConfigError) end end @@ -106,7 +106,7 @@ module SecureHeaders default_src: %w(https:) } end - combined_config = CSP.combine_policies(Configuration.get.csp, script_src: %w(anothercdn.com)) + combined_config = CSP.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) csp = ContentSecurityPolicy.new(combined_config) expect(csp.name).to eq(CSP::HEADER_NAME) expect(csp.value).to eq("default-src https:; script-src https: anothercdn.com") @@ -120,7 +120,7 @@ module SecureHeaders }.freeze end report_uri = "https://report-uri.io/asdf" - combined_config = CSP.combine_policies(Configuration.get.csp, report_uri: [report_uri]) + combined_config = CSP.combine_policies(Configuration.get.csp.to_h, report_uri: [report_uri]) csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox]) expect(csp.value).to include("report-uri #{report_uri}") end @@ -135,7 +135,7 @@ module SecureHeaders non_default_source_additions = CSP::NON_FETCH_SOURCES.each_with_object({}) do |directive, hash| hash[directive] = %w("http://example.org) end - combined_config = CSP.combine_policies(Configuration.get.csp, non_default_source_additions) + combined_config = CSP.combine_policies(Configuration.get.csp.to_h, non_default_source_additions) CSP::NON_FETCH_SOURCES.each do |directive| expect(combined_config[directive]).to eq(%w("http://example.org)) @@ -151,7 +151,7 @@ module SecureHeaders report_only: false } end - combined_config = CSP.combine_policies(Configuration.get.csp, report_only: true) + combined_config = CSP.combine_policies(Configuration.get.csp.to_h, report_only: true) csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox]) expect(csp.name).to eq(CSP::REPORT_ONLY) end @@ -163,7 +163,7 @@ module SecureHeaders block_all_mixed_content: false } end - combined_config = CSP.combine_policies(Configuration.get.csp, block_all_mixed_content: true) + combined_config = CSP.combine_policies(Configuration.get.csp.to_h, block_all_mixed_content: true) csp = ContentSecurityPolicy.new(combined_config) expect(csp.value).to eq("default-src https:; block-all-mixed-content") end @@ -173,7 +173,7 @@ module SecureHeaders config.csp = OPT_OUT end expect do - CSP.combine_policies(Configuration.get.csp, script_src: %w(anothercdn.com)) + CSP.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) end.to raise_error(ContentSecurityPolicyConfigError) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b8e31bda..23fc8f62 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,7 +25,7 @@ } def expect_default_values(hash) - expect(hash[SecureHeaders::CSP::HEADER_NAME]).to be("default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline'") + expect(hash[SecureHeaders::CSP::HEADER_NAME]).to eq("default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline'") expect(hash[SecureHeaders::XFrameOptions::HEADER_NAME]).to eq(SecureHeaders::XFrameOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XDownloadOptions::HEADER_NAME]).to eq(SecureHeaders::XDownloadOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::StrictTransportSecurity::HEADER_NAME]).to eq(SecureHeaders::StrictTransportSecurity::DEFAULT_VALUE) From 893d6e9aa9a4177c24c36b87dcfac2d7c4ad8ef7 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Mon, 8 Aug 2016 15:30:08 -1000 Subject: [PATCH 09/29] get append/override working --- lib/secure_headers.rb | 32 +++++++------ lib/secure_headers/configuration.rb | 12 +++-- .../headers/content_security_policy_config.rb | 33 +++++++++---- spec/lib/secure_headers_spec.rb | 46 +------------------ 4 files changed, 48 insertions(+), 75 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index de422b4a..18da587f 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -74,19 +74,19 @@ class << self def override_content_security_policy_directives(request, additions, target = nil) config, target = config_and_target(request, target) - if config.current_csp == OPT_OUT - config.dynamic_csp = {} + if config.csp == OPT_OUT + config.csp = ContentSecurityPolicyConfig.new({}) end - if config.current_csp_report_only == OPT_OUT - config.dynamic_csp_report_only = { :report_only => true } + if config.csp_report_only == OPT_OUT + config.csp_report_only = ContentSecurityPolicyReportOnlyConfig.new({}) end - if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT - config.dynamic_csp = config.current_csp.merge(additions) + if [:both, :enforced].include?(target) + config.csp.merge!(additions) end if [:both, :report_only].include?(target) - config.dynamic_csp_report_only = config.current_csp_report_only.merge(additions) + config.csp_report_only.merge!(additions) end override_secure_headers_request_config(request, config) @@ -101,12 +101,12 @@ def override_content_security_policy_directives(request, additions, target = nil def append_content_security_policy_directives(request, additions, target = nil) config, target = config_and_target(request, target) - if [:both, :enforced].include?(target) && config.current_csp != OPT_OUT - config.dynamic_csp = CSP.combine_policies(config.current_csp, additions) + if [:both, :enforced].include?(target) && config.csp != OPT_OUT + config.csp.append(additions) end - if [:both, :report_only].include?(target) && config.current_csp_report_only != OPT_OUT - config.dynamic_csp_report_only = CSP.combine_policies(config.current_csp_report_only, additions) + if [:both, :report_only].include?(target) && config.csp_report_only != OPT_OUT + config.csp_report_only.append(additions) end override_secure_headers_request_config(request, config) @@ -149,13 +149,13 @@ def opt_out_of_all_protection(request) def header_hash_for(request) config = config_for(request) puts "\nfinal config\n#{ config.inspect}" - unless ContentSecurityPolicy.idempotent_additions?(config.csp, config.current_csp) + if config.csp != OPT_OUT && config.csp.modified? config.rebuild_csp_header_cache!(request.user_agent, CSP::CONFIG_KEY) end puts "\nafter enforced changes\n#{ config.inspect}" - unless ContentSecurityPolicy.idempotent_additions?(config.csp_report_only, config.current_csp_report_only) + if config.csp_report_only != OPT_OUT && config.csp_report_only.modified? config.rebuild_csp_header_cache!(request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end @@ -227,10 +227,12 @@ def config_and_target(request, target) end def guess_target(config) - if config.csp + if config.csp != OPT_OUT :enforced - elsif config.csp_report_only + elsif config.csp_report_only != OPT_OUT :report_only + else + :both end end diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index c667b90c..b85d9e69 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -191,14 +191,12 @@ def secure_cookies=(secure_cookies) @cookies = (@cookies || {}).merge(secure: secure_cookies) end - protected - def csp=(new_csp) if new_csp == OPT_OUT @csp = OPT_OUT else if new_csp.is_a?(ContentSecurityPolicyConfig) - @csp = new_csp + @csp = new_csp.dup else if new_csp[:report_only] Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" @@ -214,17 +212,21 @@ def csp_report_only=(new_csp) if new_csp == OPT_OUT @csp_report_only = OPT_OUT else - if new_csp.is_a?(Hash) - new_csp = ContentSecurityPolicyReportOnlyConfig.new(new_csp) + new_csp = if new_csp.is_a?(ContentSecurityPolicyConfig) + ContentSecurityPolicyReportOnlyConfig.new(new_csp.to_h) + elsif new_csp.is_a?(Hash) if new_csp[:report_only] == false Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" end + ContentSecurityPolicyReportOnlyConfig.new(new_csp) end @csp_report_only = new_csp end end + protected + def cookies=(cookies) @cookies = cookies end diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index daf6d4e6..abf22508 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -1,6 +1,7 @@ module SecureHeaders module DynamicConfig def self.included(base) + base.send(:attr_writer, :modified) base.send(:attr_reader, *base.attrs) base.attrs.each do |attr| base.send(:define_method, "#{attr}=") do |value| @@ -19,19 +20,12 @@ def self.included(base) end def initialize(hash) - hash.keys.reject { |k| hash[k].nil? }.map do |k| - if self.class.attrs.include?(k) - self.send("#{k}=", hash[k]) - else - raise ContentSecurityPolicyConfigError, "Unknown config directive: #{k}=#{hash[k]}" - end - end - + from_hash(hash) @modified = false end def update_directive(directive, value) - self.send("#{k}=", value) + self.send("#{directive}=", value) end def directive_value(directive) @@ -45,7 +39,15 @@ def modified? end def merge(new_hash) - self.class.new(CSP.combine_policies(self.to_h, new_hash)) + CSP.combine_policies(self.to_h, new_hash) + end + + def merge!(new_hash) + from_hash(new_hash) + end + + def append(new_hash) + from_hash(CSP.combine_policies(self.to_h, new_hash)) end def to_h @@ -64,6 +66,17 @@ def ==(o) alias_method :[], :directive_value alias_method :[]=, :update_directive + + private + def from_hash(hash) + hash.keys.reject { |k| hash[k].nil? }.map do |k| + if self.class.attrs.include?(k) + self.send("#{k}=", hash[k]) + else + raise ContentSecurityPolicyConfigError, "Unknown config directive: #{k}=#{hash[k]}" + end + end + end end class ContentSecurityPolicyConfigError < StandardError; end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index ccd5aa3e..3b6da48b 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -156,50 +156,6 @@ module SecureHeaders expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline' anothercdn.com") end - it "dups global configuration just once when overriding n times and only calls idempotent_additions? once per header" do - Configuration.default do |config| - config.csp = { - default_src: %w('self') - } - end - - expect(CSP).to receive(:idempotent_additions?).twice - - # before an override occurs, the env is empty - expect(request.env[SECURE_HEADERS_CONFIG]).to be_nil - - SecureHeaders.append_content_security_policy_directives(request, script_src: %w(anothercdn.com)) - new_config = SecureHeaders.config_for(request) - expect(new_config).to_not be(Configuration.get) - - SecureHeaders.override_content_security_policy_directives(request, script_src: %w(yet.anothercdn.com)) - current_config = SecureHeaders.config_for(request) - expect(current_config).to be(new_config) - - SecureHeaders.header_hash_for(request) - end - - it "doesn't allow you to muck with csp configs when a dynamic policy is in use" do - default_config = Configuration.default do |config| - config.csp = { default_src: %w('none')}.freeze - end - expect { default_config.csp = {} }.to raise_error(NoMethodError) - - # config is frozen - expect { default_config.send(:csp=, {}) }.to raise_error(RuntimeError) - - SecureHeaders.append_content_security_policy_directives(request, script_src: %w(anothercdn.com)) - new_config = SecureHeaders.config_for(request) - $debug = true - expect { new_config.send(:csp=, {}) }.to raise_error(Configuration::IllegalPolicyModificationError) - - expect do - new_config.instance_eval do - new_config.csp = {} - end - end.to raise_error(Configuration::IllegalPolicyModificationError) - end - it "overrides individual directives" do Configuration.default do |config| config.csp = { @@ -214,7 +170,7 @@ module SecureHeaders it "overrides non-existant directives" do Configuration.default do |config| config.csp = { - default_src: %w('self') + default_src: %w(https:) } end SecureHeaders.override_content_security_policy_directives(request, img_src: [ContentSecurityPolicy::DATA_PROTOCOL]) From 40313cef1f77b71e76c57e0bed971bcfe35dd40c Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Mon, 8 Aug 2016 15:43:44 -1000 Subject: [PATCH 10/29] add tests for overriding with two headers --- lib/secure_headers.rb | 6 -- .../headers/content_security_policy.rb | 3 - spec/lib/secure_headers_spec.rb | 55 ++++++++++--------- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 18da587f..b705e170 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -148,20 +148,14 @@ def opt_out_of_all_protection(request) # in Rack middleware. def header_hash_for(request) config = config_for(request) - puts "\nfinal config\n#{ config.inspect}" if config.csp != OPT_OUT && config.csp.modified? config.rebuild_csp_header_cache!(request.user_agent, CSP::CONFIG_KEY) end - puts "\nafter enforced changes\n#{ config.inspect}" - if config.csp_report_only != OPT_OUT && config.csp_report_only.modified? config.rebuild_csp_header_cache!(request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end - puts "\nfinal config with cache\n#{ config.inspect}" - pp config.cached_headers - use_cached_headers(config.cached_headers, request) end diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index c172150f..f55fd620 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -81,10 +81,8 @@ def normalize_child_frame_src # Returns a content security policy header value. def build_value directives.map do |directive_name| - puts directive_name case DIRECTIVE_VALUE_TYPES[directive_name] when :boolean - puts " #{@config.directive_value(directive_name)}" symbol_to_hyphen_case(directive_name) if @config.directive_value(directive_name) when :string [symbol_to_hyphen_case(directive_name), @config.directive_value(directive_name)].join(" ") @@ -112,7 +110,6 @@ def build_directive(directive) else @config.directive_value(directive) end - puts " #{directive}=#{source_list}" return unless source_list && source_list.any? normalized_source_list = minify_source_list(directive, source_list) [symbol_to_hyphen_case(directive), normalized_source_list].join(" ") diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 3b6da48b..8b36ec76 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -175,7 +175,6 @@ module SecureHeaders end SecureHeaders.override_content_security_policy_directives(request, img_src: [ContentSecurityPolicy::DATA_PROTOCOL]) hash = SecureHeaders.header_hash_for(request) - puts hash expect(hash[CSP::REPORT_ONLY]).to be_nil expect(hash[CSP::HEADER_NAME]).to eq("default-src https:; img-src data:") end @@ -217,6 +216,15 @@ module SecureHeaders end context "setting two headers" do + before(:each) do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + config.csp_report_only = config.csp + end + end + it "sets identical values when the configs are the same" do Configuration.default do |config| config.csp = { @@ -246,13 +254,6 @@ module SecureHeaders end it "allows appending to the enforced policy" do - Configuration.default do |config| - config.csp = { - default_src: %w('self') - } - config.csp_report_only = config.csp - end - SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :enforced) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src 'self' anothercdn.com") @@ -260,13 +261,6 @@ module SecureHeaders end it "allows appending to the report only policy" do - Configuration.default do |config| - config.csp = { - default_src: %w('self') - } - config.csp_report_only = config.csp - end - SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :report_only) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy']).to eq("default-src 'self'") @@ -274,21 +268,32 @@ module SecureHeaders end it "allows appending to both policies" do - Configuration.default do |config| - config.csp = { - default_src: %w('self') - } - config.csp_report_only = config.csp - end - SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :both) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src 'self' anothercdn.com") expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self' anothercdn.com") end - it "allows overriding the enforced policy" - it "allows overriding the report only policy" - it "allows overriding both policies" + + it "allows overriding the enforced policy", :focus => true do + SecureHeaders.override_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :enforced) + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src anothercdn.com") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'") + end + + it "allows overriding the report only policy" do + SecureHeaders.override_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :report_only) + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src anothercdn.com") + end + + it "allows overriding both policies" do + SecureHeaders.override_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :both) + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src anothercdn.com") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src anothercdn.com") + end end end end From d19a55134bd9fc488948f5d63b06eb5216b4a2b8 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Mon, 8 Aug 2016 15:45:10 -1000 Subject: [PATCH 11/29] don't specify the target since that's not the point of the test --- spec/lib/secure_headers_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 8b36ec76..9e2d9fab 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -99,14 +99,12 @@ module SecureHeaders firefox_request = Rack::Request.new(request.env.merge("HTTP_USER_AGENT" => USER_AGENTS[:firefox])) # append an unsupported directive - SecureHeaders.override_content_security_policy_directives(firefox_request, {plugin_types: %w(flash)}, :enforced) + SecureHeaders.override_content_security_policy_directives(firefox_request, {plugin_types: %w(flash)}) # append a supported directive - SecureHeaders.override_content_security_policy_directives(firefox_request, {script_src: %w('self')}, :enforced) + SecureHeaders.override_content_security_policy_directives(firefox_request, {script_src: %w('self')}) hash = SecureHeaders.header_hash_for(firefox_request) - pp hash - # child-src is translated to frame-src expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; frame-src 'self'; script-src 'self'") end From c7a57f36951810f10612de217a36bd53f00f6b6c Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Tue, 16 Aug 2016 14:49:46 -1000 Subject: [PATCH 12/29] ensure that config objects are only dup'd when necessary --- lib/secure_headers.rb | 44 +++++++++++++------ lib/secure_headers/configuration.rb | 12 ----- .../headers/content_security_policy.rb | 2 +- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index b705e170..126f74c8 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -74,18 +74,19 @@ class << self def override_content_security_policy_directives(request, additions, target = nil) config, target = config_and_target(request, target) - if config.csp == OPT_OUT - config.csp = ContentSecurityPolicyConfig.new({}) - end - if config.csp_report_only == OPT_OUT - config.csp_report_only = ContentSecurityPolicyReportOnlyConfig.new({}) - end - if [:both, :enforced].include?(target) + if config.csp == OPT_OUT + config.csp = ContentSecurityPolicyConfig.new({}) + end + config.csp.merge!(additions) end if [:both, :report_only].include?(target) + if config.csp_report_only == OPT_OUT + config.csp_report_only = ContentSecurityPolicyReportOnlyConfig.new({}) + end + config.csp_report_only.merge!(additions) end @@ -147,16 +148,18 @@ def opt_out_of_all_protection(request) # returned is meant to be merged into the header value from `@app.call(env)` # in Rack middleware. def header_hash_for(request) - config = config_for(request) + config = config_for(request, prevent_dup = true) + headers = config.cached_headers + if config.csp != OPT_OUT && config.csp.modified? - config.rebuild_csp_header_cache!(request.user_agent, CSP::CONFIG_KEY) + headers = update_cached_csp(config, headers, request.user_agent, CSP::CONFIG_KEY) end if config.csp_report_only != OPT_OUT && config.csp_report_only.modified? - config.rebuild_csp_header_cache!(request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) + headers = update_cached_csp(config, headers, request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end - use_cached_headers(config.cached_headers, request) + use_cached_headers(headers, request) end # Public: specify which named override will be used for this request. @@ -194,11 +197,15 @@ def content_security_policy_style_nonce(request) # Checks to see if there is an override for this request, then # Checks to see if a named override is used for this request, then # Falls back to the global config - def config_for(request) + def config_for(request, prevent_dup = false) config = request.env[SECURE_HEADERS_CONFIG] || Configuration.get(Configuration::DEFAULT_CONFIG) - if config.frozen? + + # Global configs are frozen, per-request configs are not. When we're not + # making modifications to the config, prevent_dup ensures we don't dup + # the object unnecessarily. It's not necessarily frozen to begin with. + if config.frozen? && !prevent_dup config.dup else config @@ -279,6 +286,17 @@ def use_cached_headers(headers, request) end end + def update_cached_csp(config, headers, user_agent, header_key) + headers = Configuration.send(:deep_copy, headers) + headers[header_key] = {} + + csp = header_key == CSP::CONFIG_KEY ? config.csp : config.csp_report_only + user_agent = UserAgent.parse(user_agent) + variation = CSP.ua_to_variation(user_agent) + headers[header_key][variation] = CSP.make_header(csp, user_agent) + headers + end + # Private: chooses the applicable CSP header for the provided user agent. # # headers - a hash of header_config_key => [header_name, header_value] diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index b85d9e69..6a2ac892 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -155,18 +155,6 @@ def update_x_frame_options(value) self.cached_headers[XFrameOptions::CONFIG_KEY] = XFrameOptions.make_header(value) end - # Public: generated cached headers for a specific user agent. - def rebuild_csp_header_cache!(user_agent, header_key) - self.cached_headers[header_key] = {} - - csp = header_key == CSP::CONFIG_KEY ? self.csp : self.csp_report_only - unless csp == OPT_OUT - user_agent = UserAgent.parse(user_agent) - variation = CSP.ua_to_variation(user_agent) - self.cached_headers[header_key][variation] = CSP.make_header(csp, user_agent) - end - end - # Public: validates all configurations values. # # Raises various configuration errors if any invalid config is detected. diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index f55fd620..c1049c16 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -19,7 +19,7 @@ def initialize(config = nil, user_agent = OTHER) elsif config.nil? ContentSecurityPolicyConfig.new(DEFAULT_CONFIG) else - config + config.dup end @parsed_ua = if user_agent.is_a?(UserAgent::Browsers::Base) From 92dd4e58859767f7ec297cbcb9677ff034f8a851 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Tue, 16 Aug 2016 15:07:58 -1000 Subject: [PATCH 13/29] add tests for inferring which policy to modify --- lib/secure_headers.rb | 4 ++- spec/lib/secure_headers_spec.rb | 48 ++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 126f74c8..abb3d0f4 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -228,7 +228,9 @@ def config_and_target(request, target) end def guess_target(config) - if config.csp != OPT_OUT + if config.csp != OPT_OUT && config.csp_report_only != OPT_OUT + :both + elsif config.csp != OPT_OUT :enforced elsif config.csp_report_only != OPT_OUT :report_only diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 9e2d9fab..5c86c28e 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -272,7 +272,7 @@ module SecureHeaders expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self' anothercdn.com") end - it "allows overriding the enforced policy", :focus => true do + it "allows overriding the enforced policy" do SecureHeaders.override_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :enforced) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src anothercdn.com") @@ -292,6 +292,52 @@ module SecureHeaders expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src anothercdn.com") expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src anothercdn.com") end + + context "when inferring which config to modify" do + it "updates the enforced header when configured" do + Configuration.default do |config| + config.csp = { + default_src: %w('self') + } + end + SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}) + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src 'self' anothercdn.com") + expect(hash['Content-Security-Policy-Report-Only']).to be_nil + end + + it "updates the report only header when configured" do + Configuration.default do |config| + config.csp = OPT_OUT + config.csp_report_only = { + default_src: %w('self') + } + end + SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}) + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self' anothercdn.com") + expect(hash['Content-Security-Policy']).to be_nil + end + + it "updates both headers if both are configured" do + Configuration.default do |config| + config.csp = { + default_src: %w(enforced.com) + } + config.csp_report_only = { + default_src: %w(reportonly.com) + } + end + SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}) + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src enforced.com; script-src enforced.com anothercdn.com") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src reportonly.com; script-src reportonly.com anothercdn.com") + end + + end end end end From 60103a11c3491657bbd2b19797fc7be6b34d8984 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 11:15:28 -1000 Subject: [PATCH 14/29] replace some == OPT_OUT with #opt_out? --- lib/secure_headers.rb | 8 ++++++-- lib/secure_headers/configuration.rb | 2 +- .../headers/content_security_policy_config.rb | 4 ++++ lib/secure_headers/headers/policy_management.rb | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index abb3d0f4..8a3833e0 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -34,6 +34,10 @@ def dup self.class.instance end + def opt_out? + true + end + alias_method :[], :boom alias_method :[]=, :boom alias_method :keys, :boom @@ -75,7 +79,7 @@ def override_content_security_policy_directives(request, additions, target = nil config, target = config_and_target(request, target) if [:both, :enforced].include?(target) - if config.csp == OPT_OUT + if config.csp.opt_out? config.csp = ContentSecurityPolicyConfig.new({}) end @@ -83,7 +87,7 @@ def override_content_security_policy_directives(request, additions, target = nil end if [:both, :report_only].include?(target) - if config.csp_report_only == OPT_OUT + if config.csp_report_only.opt_out? config.csp_report_only = ContentSecurityPolicyReportOnlyConfig.new({}) end diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 6a2ac892..438bfba2 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -272,7 +272,7 @@ def generate_csp_headers(headers) end def generate_csp_headers_for_config(headers, header_key, csp_config) - unless csp_config == OPT_OUT + unless csp_config.opt_out? headers[header_key] = {} CSP::VARIATIONS.each do |name, _| csp = CSP.make_header(csp_config, UserAgent.parse(name)) diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index abf22508..20821670 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -60,6 +60,10 @@ def dup self.class.new(self.to_h) end + def opt_out? + self == OPT_OUT + end + def ==(o) self.class == o.class && self.to_h == o.to_h end diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index e9921929..186b6eb9 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -200,7 +200,7 @@ def make_header(config, user_agent) # Does not validate the invididual values of the source expression (e.g. # script_src => h*t*t*p: will not raise an exception) def validate_config!(config) - return if config.nil? || config == OPT_OUT + return if config.nil? || config.opt_out? raise ContentSecurityPolicyConfigError.new(":default_src is required") unless config.directive_value(:default_src) ContentSecurityPolicyConfig.attrs.each do |key| value = config.directive_value(key) From a77ce142ae421b39241ce4d568f4f877d808b1ba Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 11:16:45 -1000 Subject: [PATCH 15/29] remove #idempotent_additions? because it's irrelevent now --- lib/secure_headers/headers/policy_management.rb | 12 ------------ .../headers/policy_management_spec.rb | 14 -------------- 2 files changed, 26 deletions(-) diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index 186b6eb9..51d76cbc 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -213,18 +213,6 @@ def validate_config!(config) end end - # Public: determine if merging +additions+ will cause a change to the - # actual value of the config. - # - # e.g. config = { script_src: %w(example.org google.com)} and - # additions = { script_src: %w(google.com)} then idempotent_additions? would return - # because google.com is already in the config. - def idempotent_additions?(config, additions) - return true if config == OPT_OUT && additions == OPT_OUT - return false if config == OPT_OUT - config == combine_policies(config, additions) - end - # Public: combine the values from two different configs. # # original - the main config diff --git a/spec/lib/secure_headers/headers/policy_management_spec.rb b/spec/lib/secure_headers/headers/policy_management_spec.rb index e92dbdd5..73ac7332 100644 --- a/spec/lib/secure_headers/headers/policy_management_spec.rb +++ b/spec/lib/secure_headers/headers/policy_management_spec.rb @@ -177,19 +177,5 @@ module SecureHeaders end.to raise_error(ContentSecurityPolicyConfigError) end end - - describe "#idempotent_additions?" do - specify { expect(ContentSecurityPolicy.idempotent_additions?(OPT_OUT, script_src: %w(b.com))).to be false } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, script_src: %w(c.com))).to be false } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, style_src: %w(b.com))).to be false } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, script_src: %w(a.com b.com c.com))).to be false } - - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, script_src: %w(b.com))).to be true } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, script_src: %w(b.com a.com))).to be true } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, script_src: %w())).to be true } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, script_src: [nil])).to be true } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, style_src: [nil])).to be true } - specify { expect(ContentSecurityPolicy.idempotent_additions?({script_src: %w(a.com b.com)}, style_src: nil)).to be true } - end end end From df9631015f4ddd225795825b1a31137c42ebbd99 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 11:22:39 -1000 Subject: [PATCH 16/29] very minor cleanup --- lib/secure_headers/configuration.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 438bfba2..096d77c8 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -119,7 +119,7 @@ def deep_copy_if_hash(value) def initialize(&block) self.hpkp = OPT_OUT self.referrer_policy = OPT_OUT - self.csp = ContentSecurityPolicyConfig::DEFAULT + self.csp = ContentSecurityPolicyConfig.new(ContentSecurityPolicyConfig::DEFAULT) self.csp_report_only = OPT_OUT instance_eval &block if block_given? end @@ -202,7 +202,7 @@ def csp_report_only=(new_csp) else new_csp = if new_csp.is_a?(ContentSecurityPolicyConfig) ContentSecurityPolicyReportOnlyConfig.new(new_csp.to_h) - elsif new_csp.is_a?(Hash) + else if new_csp[:report_only] == false Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" end From 2a9021c36e5432f1a23e0018cfa7237e848978ea Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 11:30:26 -1000 Subject: [PATCH 17/29] update docs to show csp/cspro config --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5458c9d9..f4a9b1f2 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ SecureHeaders::Configuration.default do |config| secure: true, # mark all cookies as "Secure" httponly: true, # mark all cookies as "HttpOnly" samesite: { - strict: true # mark all cookies as SameSite=Strict + lax: true # mark all cookies as SameSite=lax } } config.hsts = "max-age=#{20.years.to_i}; includeSubdomains; preload" @@ -48,7 +48,7 @@ SecureHeaders::Configuration.default do |config| config.referrer_policy = "origin-when-cross-origin" config.csp = { # "meta" values. these will shaped the header, but the values are not included in the header. - report_only: true, # default: false + report_only: true, # default: false [DEPRECATED: instead, configure csp_report_only] preserve_schemes: true, # default: false. Schemes are removed from host sources to save bytes and discourage mixed content. # directive values: these values will directly translate into source directives @@ -69,6 +69,10 @@ SecureHeaders::Configuration.default do |config| upgrade_insecure_requests: true, # see https://www.w3.org/TR/upgrade-insecure-requests/ report_uri: %w(https://report-uri.io/example-csp) } + config.csp_report_only = config.csp.merge({ + img_src: %w(somewhereelse.com), + report_uri: %w(https://report-uri.io/example-csp-report-only) + }) config.hpkp = { report_only: false, max_age: 60.days.to_i, From 3afd1ddabbe7442b4083d87658142feb7e74ec0b Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 12:48:01 -1000 Subject: [PATCH 18/29] use #opt_out? helper in another location --- lib/secure_headers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 8a3833e0..d0d4198d 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -106,11 +106,11 @@ def override_content_security_policy_directives(request, additions, target = nil def append_content_security_policy_directives(request, additions, target = nil) config, target = config_and_target(request, target) - if [:both, :enforced].include?(target) && config.csp != OPT_OUT + if [:both, :enforced].include?(target) && !config.csp.opt_out? config.csp.append(additions) end - if [:both, :report_only].include?(target) && config.csp_report_only != OPT_OUT + if [:both, :report_only].include?(target) && !config.csp_report_only.opt_out? config.csp_report_only.append(additions) end From 8c1c019fe6261eba6090f2329cec6d02668390a9 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 12:51:45 -1000 Subject: [PATCH 19/29] use #opt_out? helper in another location again --- lib/secure_headers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index d0d4198d..c796d31f 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -232,7 +232,7 @@ def config_and_target(request, target) end def guess_target(config) - if config.csp != OPT_OUT && config.csp_report_only != OPT_OUT + if !config.csp.opt_out? && !config.csp_report_only.opt_out? :both elsif config.csp != OPT_OUT :enforced From 7536caeb4d9083870fe1f632f703416200a86d1d Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 13:11:34 -1000 Subject: [PATCH 20/29] get rid of even more != OPT_OUT --- lib/secure_headers.rb | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index c796d31f..5b27cd48 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -155,11 +155,11 @@ def header_hash_for(request) config = config_for(request, prevent_dup = true) headers = config.cached_headers - if config.csp != OPT_OUT && config.csp.modified? + if !config.csp.opt_out? && config.csp.modified? headers = update_cached_csp(config, headers, request.user_agent, CSP::CONFIG_KEY) end - if config.csp_report_only != OPT_OUT && config.csp_report_only.modified? + if !config.csp_report_only.opt_out? && config.csp_report_only.modified? headers = update_cached_csp(config, headers, request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) end @@ -234,9 +234,9 @@ def config_and_target(request, target) def guess_target(config) if !config.csp.opt_out? && !config.csp_report_only.opt_out? :both - elsif config.csp != OPT_OUT + elsif !config.csp.opt_out? :enforced - elsif config.csp_report_only != OPT_OUT + elsif !config.csp_report_only.opt_out? :report_only else :both @@ -311,24 +311,6 @@ def update_cached_csp(config, headers, user_agent, header_key) def csp_header_for_ua(headers, request) headers[CSP.ua_to_variation(UserAgent.parse(request.user_agent))] end - - # Private: optionally build a header with a given configure - # - # klass - corresponding Class for a given header - # config - A string, symbol, or hash config for the header - # user_agent - A string representing the UA (only used for CSP feature sniffing) - # - # Returns a 2 element array [header_name, header_value] or nil if config - # is OPT_OUT - def make_header(klass, header_config, user_agent = nil) - unless header_config == OPT_OUT - if klass == CSP - klass.make_header(header_config, user_agent) - else - klass.make_header(header_config) - end - end - end end # These methods are mixed into controllers and delegate to the class method From af0c26c9a0dc771e949bf10aef8b575f352485d5 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 13:20:58 -1000 Subject: [PATCH 21/29] remove last bits of == OPT_OUT --- lib/secure_headers/configuration.rb | 26 +++++++------------ .../headers/content_security_policy_config.rb | 2 +- spec/lib/secure_headers_spec.rb | 15 ++++++++++- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 096d77c8..6e122d94 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -180,36 +180,30 @@ def secure_cookies=(secure_cookies) end def csp=(new_csp) - if new_csp == OPT_OUT - @csp = OPT_OUT + if new_csp.respond_to?(:opt_out?) + @csp = new_csp.dup else - if new_csp.is_a?(ContentSecurityPolicyConfig) - @csp = new_csp.dup + if new_csp[:report_only] + Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" + self.csp_report_only = new_csp else - if new_csp[:report_only] - Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" - self.csp_report_only = new_csp - else - @csp = ContentSecurityPolicyConfig.new(new_csp) - end + @csp = ContentSecurityPolicyConfig.new(new_csp) end end end def csp_report_only=(new_csp) - if new_csp == OPT_OUT - @csp_report_only = OPT_OUT - else - new_csp = if new_csp.is_a?(ContentSecurityPolicyConfig) + @csp_report_only = begin + if new_csp.is_a?(ContentSecurityPolicyConfig) ContentSecurityPolicyReportOnlyConfig.new(new_csp.to_h) + elsif new_csp.respond_to?(:opt_out?) + new_csp.dup else if new_csp[:report_only] == false Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" end ContentSecurityPolicyReportOnlyConfig.new(new_csp) end - - @csp_report_only = new_csp end end diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index 20821670..2fad7dc7 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -61,7 +61,7 @@ def dup end def opt_out? - self == OPT_OUT + false end def ==(o) diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 5c86c28e..b7b964ab 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -59,7 +59,20 @@ module SecureHeaders end it "allows you to opt out entirely" do - Configuration.default + # configure the disabled-by-default headers to ensure they also do not get set + Configuration.default do |config| + config.csp_report_only = { :default_src => ["example.com"] } + config.hpkp = { + report_only: false, + max_age: 10000000, + include_subdomains: true, + report_uri: "https://report-uri.io/example-hpkp", + pins: [ + {sha256: "abc"}, + {sha256: "123"} + ] + } + end SecureHeaders.opt_out_of_all_protection(request) hash = SecureHeaders.header_hash_for(request) ALL_HEADER_CLASSES.each do |klass| From 1e91f7e0a5bce97136ee08d715c2e1b46ac391e9 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 13:38:38 -1000 Subject: [PATCH 22/29] further separate CSP from CSPRO --- lib/secure_headers.rb | 26 +++++++------ lib/secure_headers/configuration.rb | 6 +-- .../headers/content_security_policy.rb | 6 +-- .../headers/content_security_policy_config.rb | 10 ++++- .../headers/policy_management.rb | 6 +-- .../headers/content_security_policy_spec.rb | 4 +- .../headers/policy_management_spec.rb | 38 +++++++++---------- spec/lib/secure_headers_spec.rb | 6 +-- spec/spec_helper.rb | 2 +- 9 files changed, 52 insertions(+), 52 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 5b27cd48..fb728292 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -47,10 +47,12 @@ def opt_out? SECURE_HEADERS_CONFIG = "secure_headers_request_config".freeze NONCE_KEY = "secure_headers_content_security_policy_nonce".freeze HTTPS = "https".freeze - CSP = ContentSecurityPolicy + CSP = ContentSecurityPolicyConfig + CSPRO = ContentSecurityPolicyReportOnlyConfig ALL_HEADER_CLASSES = [ - ContentSecurityPolicy, + ContentSecurityPolicyConfig, + ContentSecurityPolicyReportOnlyConfig, StrictTransportSecurity, PublicKeyPins, ReferrerPolicy, @@ -61,7 +63,7 @@ def opt_out? XXssProtection ].freeze - ALL_HEADERS_BESIDES_CSP = (ALL_HEADER_CLASSES - [CSP]).freeze + ALL_HEADERS_BESIDES_CSP = (ALL_HEADER_CLASSES - [CSP, CSPRO]).freeze # Headers set on http requests (excludes STS and HPKP) HTTP_HEADER_CLASSES = @@ -160,7 +162,7 @@ def header_hash_for(request) end if !config.csp_report_only.opt_out? && config.csp_report_only.modified? - headers = update_cached_csp(config, headers, request.user_agent, CSP::REPORT_ONLY_CONFIG_KEY) + headers = update_cached_csp(config, headers, request.user_agent, CSPRO::CONFIG_KEY) end use_cached_headers(headers, request) @@ -184,7 +186,7 @@ def use_secure_headers_override(request, name) # # Returns the nonce def content_security_policy_script_nonce(request) - content_security_policy_nonce(request, CSP::SCRIPT_SRC) + content_security_policy_nonce(request, ContentSecurityPolicy::SCRIPT_SRC) end # Public: gets or creates a nonce for CSP. @@ -193,7 +195,7 @@ def content_security_policy_script_nonce(request) # # Returns the nonce def content_security_policy_style_nonce(request) - content_security_policy_nonce(request, CSP::STYLE_SRC) + content_security_policy_nonce(request, ContentSecurityPolicy::STYLE_SRC) end # Public: Retreives the config for a given header type: @@ -248,7 +250,7 @@ def guess_target(config) # Returns the nonce def content_security_policy_nonce(request, script_or_style) request.env[NONCE_KEY] ||= SecureRandom.base64(32).chomp - nonce_key = script_or_style == CSP::SCRIPT_SRC ? :script_nonce : :style_nonce + nonce_key = script_or_style == ContentSecurityPolicy::SCRIPT_SRC ? :script_nonce : :style_nonce append_content_security_policy_directives(request, nonce_key => request.env[NONCE_KEY]) request.env[NONCE_KEY] end @@ -272,7 +274,7 @@ def header_classes_for(request) HTTP_HEADER_CLASSES end.map do |klass| klass::CONFIG_KEY - end.concat([CSP::REPORT_ONLY_CONFIG_KEY]) + end end # Private: takes a precomputed hash of headers and returns the Headers @@ -282,7 +284,7 @@ def header_classes_for(request) def use_cached_headers(headers, request) header_classes_for(request).each_with_object({}) do |config_key, hash| if header = headers[config_key] - header_name, value = if [CSP::CONFIG_KEY, CSP::REPORT_ONLY_CONFIG_KEY].include?(config_key) + header_name, value = if [CSP::CONFIG_KEY, CSPRO::CONFIG_KEY].include?(config_key) csp_header_for_ua(header, request) else header @@ -298,8 +300,8 @@ def update_cached_csp(config, headers, user_agent, header_key) csp = header_key == CSP::CONFIG_KEY ? config.csp : config.csp_report_only user_agent = UserAgent.parse(user_agent) - variation = CSP.ua_to_variation(user_agent) - headers[header_key][variation] = CSP.make_header(csp, user_agent) + variation = ContentSecurityPolicy.ua_to_variation(user_agent) + headers[header_key][variation] = ContentSecurityPolicy.make_header(csp, user_agent) headers end @@ -309,7 +311,7 @@ def update_cached_csp(config, headers, user_agent, header_key) # # Returns a CSP [header, value] array def csp_header_for_ua(headers, request) - headers[CSP.ua_to_variation(UserAgent.parse(request.user_agent))] + headers[ContentSecurityPolicy.ua_to_variation(UserAgent.parse(request.user_agent))] end end diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 6e122d94..6d5e1a79 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -262,14 +262,14 @@ def cache_headers! # Returns nothing def generate_csp_headers(headers) generate_csp_headers_for_config(headers, CSP::CONFIG_KEY, self.csp) - generate_csp_headers_for_config(headers, CSP::REPORT_ONLY_CONFIG_KEY, self.csp_report_only) + generate_csp_headers_for_config(headers, CSPRO::CONFIG_KEY, self.csp_report_only) end def generate_csp_headers_for_config(headers, header_key, csp_config) unless csp_config.opt_out? headers[header_key] = {} - CSP::VARIATIONS.each do |name, _| - csp = CSP.make_header(csp_config, UserAgent.parse(name)) + ContentSecurityPolicy::VARIATIONS.each do |name, _| + csp = ContentSecurityPolicy.make_header(csp_config, UserAgent.parse(name)) headers[header_key][name] = csp.freeze end end diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index c1049c16..c67350f1 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -37,11 +37,7 @@ def initialize(config = nil, user_agent = OTHER) # Returns the name to use for the header. Either "Content-Security-Policy" or # "Content-Security-Policy-Report-Only" def name - if @config.report_only? - REPORT_ONLY - else - HEADER_NAME - end + @config.class.const_get(:HEADER_NAME) end ## diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index 2fad7dc7..6bf28aae 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -39,7 +39,7 @@ def modified? end def merge(new_hash) - CSP.combine_policies(self.to_h, new_hash) + ContentSecurityPolicy.combine_policies(self.to_h, new_hash) end def merge!(new_hash) @@ -47,7 +47,7 @@ def merge!(new_hash) end def append(new_hash) - from_hash(CSP.combine_policies(self.to_h, new_hash)) + from_hash(ContentSecurityPolicy.combine_policies(self.to_h, new_hash)) end def to_h @@ -85,6 +85,9 @@ def from_hash(hash) class ContentSecurityPolicyConfigError < StandardError; end class ContentSecurityPolicyConfig + CONFIG_KEY = :csp + HEADER_NAME = "Content-Security-Policy".freeze + def self.attrs PolicyManagement::ALL_DIRECTIVES + PolicyManagement::META_CONFIGS + PolicyManagement::NONCES end @@ -107,6 +110,9 @@ def report_only? end class ContentSecurityPolicyReportOnlyConfig < ContentSecurityPolicyConfig + CONFIG_KEY = :csp_report_only + HEADER_NAME = "Content-Security-Policy-Report-Only".freeze + def report_only? true end diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index 51d76cbc..87bed936 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -7,9 +7,6 @@ def self.included(base) MODERN_BROWSERS = %w(Chrome Opera Firefox) DEFAULT_VALUE = "default-src https:".freeze DEFAULT_CONFIG = { default_src: %w(https:) }.freeze - HEADER_NAME = "Content-Security-Policy".freeze - REPORT_ONLY = "Content-Security-Policy-Report-Only".freeze - HEADER_NAMES = [HEADER_NAME, REPORT_ONLY] DATA_PROTOCOL = "data:".freeze BLOB_PROTOCOL = "blob:".freeze SELF = "'self'".freeze @@ -162,8 +159,7 @@ def self.included(base) UPGRADE_INSECURE_REQUESTS => :boolean }.freeze - CONFIG_KEY = :csp - REPORT_ONLY_CONFIG_KEY = :csp_report_only + STAR_REGEXP = Regexp.new(Regexp.escape(STAR)) HTTP_SCHEME_REGEX = %r{\Ahttps?://} diff --git a/spec/lib/secure_headers/headers/content_security_policy_spec.rb b/spec/lib/secure_headers/headers/content_security_policy_spec.rb index 1e30f62b..ef0fd8ce 100644 --- a/spec/lib/secure_headers/headers/content_security_policy_spec.rb +++ b/spec/lib/secure_headers/headers/content_security_policy_spec.rb @@ -14,11 +14,11 @@ module SecureHeaders describe "#name" do context "when in report-only mode" do - specify { expect(ContentSecurityPolicy.new(default_opts.merge(report_only: true)).name).to eq(ContentSecurityPolicy::HEADER_NAME + "-Report-Only") } + specify { expect(ContentSecurityPolicy.new(default_opts.merge(report_only: true)).name).to eq(ContentSecurityPolicyReportOnlyConfig::HEADER_NAME) } end context "when in enforce mode" do - specify { expect(ContentSecurityPolicy.new(default_opts).name).to eq(ContentSecurityPolicy::HEADER_NAME) } + specify { expect(ContentSecurityPolicy.new(default_opts).name).to eq(ContentSecurityPolicyConfig::HEADER_NAME) } end end diff --git a/spec/lib/secure_headers/headers/policy_management_spec.rb b/spec/lib/secure_headers/headers/policy_management_spec.rb index 73ac7332..a41d7926 100644 --- a/spec/lib/secure_headers/headers/policy_management_spec.rb +++ b/spec/lib/secure_headers/headers/policy_management_spec.rb @@ -40,61 +40,61 @@ module SecureHeaders report_uri: %w(https://example.com/uri-directive) } - CSP.validate_config!(ContentSecurityPolicyConfig.new(config)) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(config)) end it "requires a :default_src value" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(script_src: %('self'))) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(script_src: %('self'))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :report_only to be a truthy value" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(report_only: "steve"))) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(report_only: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :preserve_schemes to be a truthy value" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(preserve_schemes: "steve"))) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(preserve_schemes: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :block_all_mixed_content to be a boolean value" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(block_all_mixed_content: "steve"))) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(block_all_mixed_content: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires :upgrade_insecure_requests to be a boolean value" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(upgrade_insecure_requests: "steve"))) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(upgrade_insecure_requests: "steve"))) end.to raise_error(ContentSecurityPolicyConfigError) end it "requires all source lists to be an array of strings" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: "steve")) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_src: "steve")) end.to raise_error(ContentSecurityPolicyConfigError) end it "allows nil values" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w('self'), script_src: ["https:", nil])) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w('self'), script_src: ["https:", nil])) end.to_not raise_error end it "rejects unknown directives / config" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w('self'), default_src_totally_mispelled: "steve")) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w('self'), default_src_totally_mispelled: "steve")) end.to raise_error(ContentSecurityPolicyConfigError) end # this is mostly to ensure people don't use the antiquated shorthands common in other configs it "performs light validation on source lists" do expect do - CSP.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w(self none inline eval))) + ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_src: %w(self none inline eval))) end.to raise_error(ContentSecurityPolicyConfigError) end end @@ -106,7 +106,7 @@ module SecureHeaders default_src: %w(https:) } end - combined_config = CSP.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) + combined_config = ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) csp = ContentSecurityPolicy.new(combined_config) expect(csp.name).to eq(CSP::HEADER_NAME) expect(csp.value).to eq("default-src https:; script-src https: anothercdn.com") @@ -120,7 +120,7 @@ module SecureHeaders }.freeze end report_uri = "https://report-uri.io/asdf" - combined_config = CSP.combine_policies(Configuration.get.csp.to_h, report_uri: [report_uri]) + combined_config = ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, report_uri: [report_uri]) csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox]) expect(csp.value).to include("report-uri #{report_uri}") end @@ -132,12 +132,12 @@ module SecureHeaders report_only: false }.freeze end - non_default_source_additions = CSP::NON_FETCH_SOURCES.each_with_object({}) do |directive, hash| + non_default_source_additions = ContentSecurityPolicy::NON_FETCH_SOURCES.each_with_object({}) do |directive, hash| hash[directive] = %w("http://example.org) end - combined_config = CSP.combine_policies(Configuration.get.csp.to_h, non_default_source_additions) + combined_config = ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, non_default_source_additions) - CSP::NON_FETCH_SOURCES.each do |directive| + ContentSecurityPolicy::NON_FETCH_SOURCES.each do |directive| expect(combined_config[directive]).to eq(%w("http://example.org)) end @@ -151,9 +151,9 @@ module SecureHeaders report_only: false } end - combined_config = CSP.combine_policies(Configuration.get.csp.to_h, report_only: true) + combined_config = ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, report_only: true) csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox]) - expect(csp.name).to eq(CSP::REPORT_ONLY) + expect(csp.name).to eq(ContentSecurityPolicyReportOnlyConfig::HEADER_NAME) end it "overrides the :block_all_mixed_content flag" do @@ -163,7 +163,7 @@ module SecureHeaders block_all_mixed_content: false } end - combined_config = CSP.combine_policies(Configuration.get.csp.to_h, block_all_mixed_content: true) + combined_config = ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, block_all_mixed_content: true) csp = ContentSecurityPolicy.new(combined_config) expect(csp.value).to eq("default-src https:; block-all-mixed-content") end @@ -173,7 +173,7 @@ module SecureHeaders config.csp = OPT_OUT end expect do - CSP.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) + ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) end.to raise_error(ContentSecurityPolicyConfigError) end end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index b7b964ab..20a79f28 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -33,7 +33,7 @@ module SecureHeaders config.csp_report_only = { default_src: %w('self')} # no default value end SecureHeaders.opt_out_of_header(request, CSP::CONFIG_KEY) - SecureHeaders.opt_out_of_header(request, CSP::REPORT_ONLY_CONFIG_KEY) + SecureHeaders.opt_out_of_header(request, CSPRO::CONFIG_KEY) SecureHeaders.opt_out_of_header(request, XContentTypeOptions::CONFIG_KEY) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy-Report-Only']).to be_nil @@ -186,7 +186,7 @@ module SecureHeaders end SecureHeaders.override_content_security_policy_directives(request, img_src: [ContentSecurityPolicy::DATA_PROTOCOL]) hash = SecureHeaders.header_hash_for(request) - expect(hash[CSP::REPORT_ONLY]).to be_nil + expect(hash[CSPRO::HEADER_NAME]).to be_nil expect(hash[CSP::HEADER_NAME]).to eq("default-src https:; img-src data:") end @@ -367,7 +367,7 @@ module SecureHeaders it "validates your csp config upon configuration" do expect do Configuration.default do |config| - config.csp = { CSP::DEFAULT_SRC => '123456' } + config.csp = { ContentSecurityPolicy::DEFAULT_SRC => '123456' } end end.to raise_error(ContentSecurityPolicyConfigError) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 23fc8f62..20e8eb82 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,7 +25,7 @@ } def expect_default_values(hash) - expect(hash[SecureHeaders::CSP::HEADER_NAME]).to eq("default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline'") + expect(hash[SecureHeaders::ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline'") expect(hash[SecureHeaders::XFrameOptions::HEADER_NAME]).to eq(SecureHeaders::XFrameOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::XDownloadOptions::HEADER_NAME]).to eq(SecureHeaders::XDownloadOptions::DEFAULT_VALUE) expect(hash[SecureHeaders::StrictTransportSecurity::HEADER_NAME]).to eq(SecureHeaders::StrictTransportSecurity::DEFAULT_VALUE) From 9112c625e7220fa78df567ff54f45ef6cd5ca5b8 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 13:40:16 -1000 Subject: [PATCH 23/29] remove CSP/CSPRO shortcut constants --- lib/secure_headers.rb | 15 +++++++------- lib/secure_headers/configuration.rb | 4 ++-- .../headers/policy_management_spec.rb | 2 +- spec/lib/secure_headers/middleware_spec.rb | 2 +- spec/lib/secure_headers/view_helpers_spec.rb | 8 ++++---- spec/lib/secure_headers_spec.rb | 20 +++++++++---------- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index fb728292..96fe790f 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -47,8 +47,6 @@ def opt_out? SECURE_HEADERS_CONFIG = "secure_headers_request_config".freeze NONCE_KEY = "secure_headers_content_security_policy_nonce".freeze HTTPS = "https".freeze - CSP = ContentSecurityPolicyConfig - CSPRO = ContentSecurityPolicyReportOnlyConfig ALL_HEADER_CLASSES = [ ContentSecurityPolicyConfig, @@ -63,7 +61,10 @@ def opt_out? XXssProtection ].freeze - ALL_HEADERS_BESIDES_CSP = (ALL_HEADER_CLASSES - [CSP, CSPRO]).freeze + ALL_HEADERS_BESIDES_CSP = ( + ALL_HEADER_CLASSES - + [ContentSecurityPolicyConfig, ContentSecurityPolicyReportOnlyConfig] + ).freeze # Headers set on http requests (excludes STS and HPKP) HTTP_HEADER_CLASSES = @@ -158,11 +159,11 @@ def header_hash_for(request) headers = config.cached_headers if !config.csp.opt_out? && config.csp.modified? - headers = update_cached_csp(config, headers, request.user_agent, CSP::CONFIG_KEY) + headers = update_cached_csp(config, headers, request.user_agent, ContentSecurityPolicyConfig::CONFIG_KEY) end if !config.csp_report_only.opt_out? && config.csp_report_only.modified? - headers = update_cached_csp(config, headers, request.user_agent, CSPRO::CONFIG_KEY) + headers = update_cached_csp(config, headers, request.user_agent, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY) end use_cached_headers(headers, request) @@ -284,7 +285,7 @@ def header_classes_for(request) def use_cached_headers(headers, request) header_classes_for(request).each_with_object({}) do |config_key, hash| if header = headers[config_key] - header_name, value = if [CSP::CONFIG_KEY, CSPRO::CONFIG_KEY].include?(config_key) + header_name, value = if [ContentSecurityPolicyConfig::CONFIG_KEY, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY].include?(config_key) csp_header_for_ua(header, request) else header @@ -298,7 +299,7 @@ def update_cached_csp(config, headers, user_agent, header_key) headers = Configuration.send(:deep_copy, headers) headers[header_key] = {} - csp = header_key == CSP::CONFIG_KEY ? config.csp : config.csp_report_only + csp = header_key == ContentSecurityPolicyConfig::CONFIG_KEY ? config.csp : config.csp_report_only user_agent = UserAgent.parse(user_agent) variation = ContentSecurityPolicy.ua_to_variation(user_agent) headers[header_key][variation] = ContentSecurityPolicy.make_header(csp, user_agent) diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 6d5e1a79..97ddc790 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -261,8 +261,8 @@ def cache_headers! # # Returns nothing def generate_csp_headers(headers) - generate_csp_headers_for_config(headers, CSP::CONFIG_KEY, self.csp) - generate_csp_headers_for_config(headers, CSPRO::CONFIG_KEY, self.csp_report_only) + generate_csp_headers_for_config(headers, ContentSecurityPolicyConfig::CONFIG_KEY, self.csp) + generate_csp_headers_for_config(headers, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY, self.csp_report_only) end def generate_csp_headers_for_config(headers, header_key, csp_config) diff --git a/spec/lib/secure_headers/headers/policy_management_spec.rb b/spec/lib/secure_headers/headers/policy_management_spec.rb index a41d7926..f711d5f0 100644 --- a/spec/lib/secure_headers/headers/policy_management_spec.rb +++ b/spec/lib/secure_headers/headers/policy_management_spec.rb @@ -108,7 +108,7 @@ module SecureHeaders end combined_config = ContentSecurityPolicy.combine_policies(Configuration.get.csp.to_h, script_src: %w(anothercdn.com)) csp = ContentSecurityPolicy.new(combined_config) - expect(csp.name).to eq(CSP::HEADER_NAME) + expect(csp.name).to eq(ContentSecurityPolicyConfig::HEADER_NAME) expect(csp.value).to eq("default-src https:; script-src https: anothercdn.com") end diff --git a/spec/lib/secure_headers/middleware_spec.rb b/spec/lib/secure_headers/middleware_spec.rb index a84e288e..13fbf8c9 100644 --- a/spec/lib/secure_headers/middleware_spec.rb +++ b/spec/lib/secure_headers/middleware_spec.rb @@ -51,7 +51,7 @@ module SecureHeaders SecureHeaders.use_secure_headers_override(request, "my_custom_config") expect(request.env[SECURE_HEADERS_CONFIG]).to be(Configuration.get("my_custom_config")) _, env = middleware.call request.env - expect(env[CSP::HEADER_NAME]).to match("example.org") + expect(env[ContentSecurityPolicyConfig::HEADER_NAME]).to match("example.org") end context "secure_cookies" do diff --git a/spec/lib/secure_headers/view_helpers_spec.rb b/spec/lib/secure_headers/view_helpers_spec.rb index 386129de..17723853 100644 --- a/spec/lib/secure_headers/view_helpers_spec.rb +++ b/spec/lib/secure_headers/view_helpers_spec.rb @@ -118,10 +118,10 @@ module SecureHeaders Message.new(request).result _, env = middleware.call request.env - expect(env[CSP::HEADER_NAME]).to match(/script-src[^;]*'#{Regexp.escape(expected_hash)}'/) - expect(env[CSP::HEADER_NAME]).to match(/script-src[^;]*'nonce-abc123'/) - expect(env[CSP::HEADER_NAME]).to match(/style-src[^;]*'nonce-abc123'/) - expect(env[CSP::HEADER_NAME]).to match(/style-src[^;]*'#{Regexp.escape(expected_style_hash)}'/) + expect(env[ContentSecurityPolicyConfig::HEADER_NAME]).to match(/script-src[^;]*'#{Regexp.escape(expected_hash)}'/) + expect(env[ContentSecurityPolicyConfig::HEADER_NAME]).to match(/script-src[^;]*'nonce-abc123'/) + expect(env[ContentSecurityPolicyConfig::HEADER_NAME]).to match(/style-src[^;]*'nonce-abc123'/) + expect(env[ContentSecurityPolicyConfig::HEADER_NAME]).to match(/style-src[^;]*'#{Regexp.escape(expected_style_hash)}'/) end end end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 20a79f28..6d3d9a73 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -16,7 +16,7 @@ module SecureHeaders it "raises a NotYetConfiguredError if trying to opt-out of unconfigured headers" do expect do - SecureHeaders.opt_out_of_header(request, CSP::CONFIG_KEY) + SecureHeaders.opt_out_of_header(request, ContentSecurityPolicyConfig::CONFIG_KEY) end.to raise_error(Configuration::NotYetConfiguredError) end @@ -32,8 +32,8 @@ module SecureHeaders Configuration.default do |config| config.csp_report_only = { default_src: %w('self')} # no default value end - SecureHeaders.opt_out_of_header(request, CSP::CONFIG_KEY) - SecureHeaders.opt_out_of_header(request, CSPRO::CONFIG_KEY) + SecureHeaders.opt_out_of_header(request, ContentSecurityPolicyConfig::CONFIG_KEY) + SecureHeaders.opt_out_of_header(request, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY) SecureHeaders.opt_out_of_header(request, XContentTypeOptions::CONFIG_KEY) hash = SecureHeaders.header_hash_for(request) expect(hash['Content-Security-Policy-Report-Only']).to be_nil @@ -98,7 +98,7 @@ module SecureHeaders SecureHeaders.override_content_security_policy_directives(request, default_src: %w(https:), script_src: %w('self')) hash = SecureHeaders.header_hash_for(request) - expect(hash[CSP::HEADER_NAME]).to eq("default-src https:; script-src 'self'") + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src https:; script-src 'self'") expect(hash[XFrameOptions::HEADER_NAME]).to eq(XFrameOptions::SAMEORIGIN) end @@ -119,7 +119,7 @@ module SecureHeaders hash = SecureHeaders.header_hash_for(firefox_request) # child-src is translated to frame-src - expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; frame-src 'self'; script-src 'self'") + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; frame-src 'self'; script-src 'self'") end it "produces a hash of headers with default config" do @@ -164,7 +164,7 @@ module SecureHeaders SecureHeaders.append_content_security_policy_directives(request, script_src: %w(anothercdn.com)) hash = SecureHeaders.header_hash_for(request) - expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline' anothercdn.com") + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline' anothercdn.com") end it "overrides individual directives" do @@ -175,7 +175,7 @@ module SecureHeaders end SecureHeaders.override_content_security_policy_directives(request, default_src: %w('none')) hash = SecureHeaders.header_hash_for(request) - expect(hash[CSP::HEADER_NAME]).to eq("default-src 'none'") + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'none'") end it "overrides non-existant directives" do @@ -186,8 +186,8 @@ module SecureHeaders end SecureHeaders.override_content_security_policy_directives(request, img_src: [ContentSecurityPolicy::DATA_PROTOCOL]) hash = SecureHeaders.header_hash_for(request) - expect(hash[CSPRO::HEADER_NAME]).to be_nil - expect(hash[CSP::HEADER_NAME]).to eq("default-src https:; img-src data:") + expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to be_nil + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src https:; img-src data:") end it "does not append a nonce when the browser does not support it" do @@ -202,7 +202,7 @@ module SecureHeaders safari_request = Rack::Request.new(request.env.merge("HTTP_USER_AGENT" => USER_AGENTS[:safari5])) nonce = SecureHeaders.content_security_policy_script_nonce(safari_request) hash = SecureHeaders.header_hash_for(safari_request) - expect(hash[CSP::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline'; style-src 'self'") + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline'; style-src 'self'") end it "appends a nonce to the script-src when used" do From 6d9d76a7b22ee28374b0da21a56d02b53269a14b Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 13:56:08 -1000 Subject: [PATCH 24/29] remove unnecessary special handling of CSP/CSPRO now that they are separate classes --- lib/secure_headers.rb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 96fe790f..91b298da 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -159,11 +159,11 @@ def header_hash_for(request) headers = config.cached_headers if !config.csp.opt_out? && config.csp.modified? - headers = update_cached_csp(config, headers, request.user_agent, ContentSecurityPolicyConfig::CONFIG_KEY) + headers = update_cached_csp(config.csp, headers, request.user_agent) end if !config.csp_report_only.opt_out? && config.csp_report_only.modified? - headers = update_cached_csp(config, headers, request.user_agent, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY) + headers = update_cached_csp(config.csp_report_only, headers, request.user_agent) end use_cached_headers(headers, request) @@ -273,8 +273,6 @@ def header_classes_for(request) ALL_HEADER_CLASSES else HTTP_HEADER_CLASSES - end.map do |klass| - klass::CONFIG_KEY end end @@ -283,9 +281,9 @@ def header_classes_for(request) # # Returns a hash of header names / values valid for a given request. def use_cached_headers(headers, request) - header_classes_for(request).each_with_object({}) do |config_key, hash| - if header = headers[config_key] - header_name, value = if [ContentSecurityPolicyConfig::CONFIG_KEY, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY].include?(config_key) + header_classes_for(request).each_with_object({}) do |klass, hash| + if header = headers[klass::CONFIG_KEY] + header_name, value = if [ContentSecurityPolicyConfig, ContentSecurityPolicyReportOnlyConfig].include?(klass) csp_header_for_ua(header, request) else header @@ -295,14 +293,13 @@ def use_cached_headers(headers, request) end end - def update_cached_csp(config, headers, user_agent, header_key) + def update_cached_csp(config, headers, user_agent) headers = Configuration.send(:deep_copy, headers) - headers[header_key] = {} + headers[config.class::CONFIG_KEY] = {} - csp = header_key == ContentSecurityPolicyConfig::CONFIG_KEY ? config.csp : config.csp_report_only user_agent = UserAgent.parse(user_agent) variation = ContentSecurityPolicy.ua_to_variation(user_agent) - headers[header_key][variation] = ContentSecurityPolicy.make_header(csp, user_agent) + headers[config.class::CONFIG_KEY][variation] = ContentSecurityPolicy.make_header(config, user_agent) headers end From 1b80f85be36152c3d25f7ff699f51422c3c3ffa4 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 14:02:39 -1000 Subject: [PATCH 25/29] inline method so we're not creating UserAgent objects everywhere --- lib/secure_headers.rb | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 91b298da..3c98007a 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -157,16 +157,26 @@ def opt_out_of_all_protection(request) def header_hash_for(request) config = config_for(request, prevent_dup = true) headers = config.cached_headers + user_agent = UserAgent.parse(request.user_agent) if !config.csp.opt_out? && config.csp.modified? - headers = update_cached_csp(config.csp, headers, request.user_agent) + headers = update_cached_csp(config.csp, headers, user_agent) end if !config.csp_report_only.opt_out? && config.csp_report_only.modified? - headers = update_cached_csp(config.csp_report_only, headers, request.user_agent) + headers = update_cached_csp(config.csp_report_only, headers, user_agent) end - use_cached_headers(headers, request) + header_classes_for(request).each_with_object({}) do |klass, hash| + if header = headers[klass::CONFIG_KEY] + header_name, value = if [ContentSecurityPolicyConfig, ContentSecurityPolicyReportOnlyConfig].include?(klass) + csp_header_for_ua(header, user_agent) + else + header + end + hash[header_name] = value + end + end end # Public: specify which named override will be used for this request. @@ -276,28 +286,9 @@ def header_classes_for(request) end end - # Private: takes a precomputed hash of headers and returns the Headers - # customized for the request. - # - # Returns a hash of header names / values valid for a given request. - def use_cached_headers(headers, request) - header_classes_for(request).each_with_object({}) do |klass, hash| - if header = headers[klass::CONFIG_KEY] - header_name, value = if [ContentSecurityPolicyConfig, ContentSecurityPolicyReportOnlyConfig].include?(klass) - csp_header_for_ua(header, request) - else - header - end - hash[header_name] = value - end - end - end - def update_cached_csp(config, headers, user_agent) headers = Configuration.send(:deep_copy, headers) headers[config.class::CONFIG_KEY] = {} - - user_agent = UserAgent.parse(user_agent) variation = ContentSecurityPolicy.ua_to_variation(user_agent) headers[config.class::CONFIG_KEY][variation] = ContentSecurityPolicy.make_header(config, user_agent) headers @@ -308,8 +299,8 @@ def update_cached_csp(config, headers, user_agent) # headers - a hash of header_config_key => [header_name, header_value] # # Returns a CSP [header, value] array - def csp_header_for_ua(headers, request) - headers[ContentSecurityPolicy.ua_to_variation(UserAgent.parse(request.user_agent))] + def csp_header_for_ua(headers, user_agent) + headers[ContentSecurityPolicy.ua_to_variation(user_agent)] end end From 8e14907adb94048a8b8ca0d3cbc6b51b60642d5b Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 14:07:57 -1000 Subject: [PATCH 26/29] don't modify the original config object, removing the need to dup it --- .../headers/content_security_policy.rb | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index c67350f1..a7242cc5 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -19,7 +19,7 @@ def initialize(config = nil, user_agent = OTHER) elsif config.nil? ContentSecurityPolicyConfig.new(DEFAULT_CONFIG) else - config.dup + config end @parsed_ua = if user_agent.is_a?(UserAgent::Browsers::Base) @@ -115,15 +115,15 @@ def build_directive(directive) # If a directive contains 'none' but has other values, 'none' is ommitted. # Schemes are stripped (see http://www.w3.org/TR/CSP2/#match-source-expression) def minify_source_list(directive, source_list) - source_list.compact! + source_list = source_list.compact if source_list.include?(STAR) keep_wildcard_sources(source_list) else - populate_nonces!(directive, source_list) - reject_all_values_if_none!(source_list) + source_list = populate_nonces(directive, source_list) + source_list = reject_all_values_if_none(source_list) unless directive == REPORT_URI || @preserve_schemes - strip_source_schemes!(source_list) + source_list = strip_source_schemes(source_list) end dedup_source_list(source_list) end @@ -135,8 +135,12 @@ def keep_wildcard_sources(source_list) end # Discard any 'none' values if more directives are supplied since none may override values. - def reject_all_values_if_none!(source_list) - source_list.reject! { |value| value == NONE } if source_list.length > 1 + def reject_all_values_if_none(source_list) + if source_list.length > 1 + source_list.reject { |value| value == NONE } + else + source_list + end end # Removes duplicates and sources that already match an existing wild card. @@ -158,12 +162,14 @@ def dedup_source_list(sources) # Private: append a nonce to the script/style directories if script_nonce # or style_nonce are provided. - def populate_nonces!(directive, source_list) + def populate_nonces(directive, source_list) case directive when SCRIPT_SRC append_nonce(source_list, @script_nonce) when STYLE_SRC append_nonce(source_list, @style_nonce) + else + source_list end end @@ -180,6 +186,8 @@ def append_nonce(source_list, nonce) source_list << UNSAFE_INLINE end end + + source_list end # Private: return the list of directives that are supported by the user agent, @@ -193,8 +201,8 @@ def directives end # Private: Remove scheme from source expressions. - def strip_source_schemes!(source_list) - source_list.map! { |source_expression| source_expression.sub(HTTP_SCHEME_REGEX, "") } + def strip_source_schemes(source_list) + source_list.map { |source_expression| source_expression.sub(HTTP_SCHEME_REGEX, "") } end # Private: determine which directives are supported for the given user agent. From dc1bbac3c878222c92e503890cba2c5a440d67d3 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 14:17:06 -1000 Subject: [PATCH 27/29] ensure that the deprecated way of using the report_only header still works :) --- lib/secure_headers/configuration.rb | 2 ++ spec/lib/secure_headers_spec.rb | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 97ddc790..53b3d965 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -184,7 +184,9 @@ def csp=(new_csp) @csp = new_csp.dup else if new_csp[:report_only] + # Deprecated configuration implies that CSPRO should be set, CSP should not - so opt out Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp=` was supplied a config with report_only: true. Use #csp_report_only=" + @csp = OPT_OUT self.csp_report_only = new_csp else @csp = ContentSecurityPolicyConfig.new(new_csp) diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 6d3d9a73..6407e961 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -226,6 +226,24 @@ module SecureHeaders expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src mycdn.com 'nonce-#{nonce}'; style-src 'self'") end + it "supports the deprecated `report_only: true` format" do + expect(Kernel).to receive(:warn).once + + Configuration.default do |config| + config.csp = { + default_src: %w('self'), + report_only: true + } + end + + expect(Configuration.get.csp).to eq(OPT_OUT) + expect(Configuration.get.csp_report_only).to be_a(ContentSecurityPolicyReportOnlyConfig) + + hash = SecureHeaders.header_hash_for(request) + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to be_nil + expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to eq("default-src 'self'") + end + context "setting two headers" do before(:each) do Configuration.default do |config| From 7600b900c6663883a321719882616652f72e6386 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Wed, 17 Aug 2016 14:25:03 -1000 Subject: [PATCH 28/29] add guard ensuring people aren't setting csp_report_only with report_only: false --- lib/secure_headers/configuration.rb | 7 ++++--- spec/lib/secure_headers_spec.rb | 11 +++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 53b3d965..07a7cf86 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -201,10 +201,11 @@ def csp_report_only=(new_csp) elsif new_csp.respond_to?(:opt_out?) new_csp.dup else - if new_csp[:report_only] == false - Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was supplied a config with report_only: false. Use #csp=" + if new_csp[:report_only] == false # nil is a valid value on which we do not want to raise + raise ContentSecurityPolicyConfigError, "`#csp_report_only=` was supplied a config with report_only: false. Use #csp=" + else + ContentSecurityPolicyReportOnlyConfig.new(new_csp) end - ContentSecurityPolicyReportOnlyConfig.new(new_csp) end end end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 6407e961..99b63c61 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -244,6 +244,17 @@ module SecureHeaders expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to eq("default-src 'self'") end + it "Raises an error if csp_report_only is used with `report_only: false`" do + expect do + Configuration.default do |config| + config.csp_report_only = { + default_src: %w('self'), + report_only: false + } + end + end.to raise_error(ContentSecurityPolicyConfigError) + end + context "setting two headers" do before(:each) do Configuration.default do |config| From 3225ed600306d8c7e90ecf4d6849a9f17e0d9ac0 Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Thu, 18 Aug 2016 12:31:51 -1000 Subject: [PATCH 29/29] infer default policies when incomplete CSP configuration is supplied. --- README.md | 27 ++++++++++- lib/secure_headers/configuration.rb | 15 +++++- .../headers/content_security_policy_config.rb | 8 ++++ spec/lib/secure_headers_spec.rb | 46 ++++++++++++++++++- 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f4a9b1f2..71da2ce7 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,32 @@ use SecureHeaders::Middleware ## Default values -All headers except for PublicKeyPins have a default value. See the [corresponding classes for their defaults](https://github.com/twitter/secureheaders/tree/master/lib/secure_headers/headers). +All headers except for PublicKeyPins have a default value. See the [corresponding classes for their defaults](https://github.com/twitter/secureheaders/tree/master/lib/secure_headers/headers). The default set of headers is: + +``` +Content-Security-Policy: default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline' +Strict-Transport-Security: max-age=631138519 +X-Content-Type-Options: nosniff +X-Download-Options: noopen +X-Frame-Options: sameorigin +X-Permitted-Cross-Domain-Policies: none +X-Xss-Protection: 1; mode=block +``` + +### Default CSP + +By default, the above CSP will be applied to all requests. If you **only** want to set a Report-Only header, opt-out of the default enforced header for clarity. The configuration will assume that if you only supply `csp_report_only` that you intended to opt-out of `csp` but that's for the sake of backwards compatibility and it will be removed in the future. + +```ruby +Configuration.default do |config| + config.csp = SecureHeaders::OPT_OUT # If this line is omitted, we will assume you meant to opt out. + config.csp_report_only = { + default_src: %w('self') + } +end +``` + +If ** ## Named overrides diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index 07a7cf86..0a5d5657 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -194,10 +194,16 @@ def csp=(new_csp) end end + # Configures the Content-Security-Policy-Report-Only header. `new_csp` cannot + # contain `report_only: false` or an error will be raised. + # + # NOTE: if csp has not been configured/has the default value when + # configuring csp_report_only, the code will assume you mean to only use + # report-only mode and you will be opted-out of enforce mode. def csp_report_only=(new_csp) @csp_report_only = begin - if new_csp.is_a?(ContentSecurityPolicyConfig) - ContentSecurityPolicyReportOnlyConfig.new(new_csp.to_h) + if new_csp.is_a?(ContentSecurityPolicyConfig) + new_csp.make_report_only elsif new_csp.respond_to?(:opt_out?) new_csp.dup else @@ -208,6 +214,11 @@ def csp_report_only=(new_csp) end end end + + if !@csp_report_only.opt_out? && @csp.to_h == ContentSecurityPolicyConfig::DEFAULT + Kernel.warn "#{Kernel.caller.first}: [DEPRECATION] `#csp_report_only=` was configured before `#csp=`. It is assumed you intended to opt out of `#csp=` so be sure to add `config.csp = SecureHeaders::OPT_OUT` to your config. Ensure that #csp_report_only is configured after #csp=" + @csp = OPT_OUT + end end protected diff --git a/lib/secure_headers/headers/content_security_policy_config.rb b/lib/secure_headers/headers/content_security_policy_config.rb index 6bf28aae..52d5c673 100644 --- a/lib/secure_headers/headers/content_security_policy_config.rb +++ b/lib/secure_headers/headers/content_security_policy_config.rb @@ -107,6 +107,10 @@ def self.attrs def report_only? false end + + def make_report_only + ContentSecurityPolicyReportOnlyConfig.new(self.to_h) + end end class ContentSecurityPolicyReportOnlyConfig < ContentSecurityPolicyConfig @@ -116,5 +120,9 @@ class ContentSecurityPolicyReportOnlyConfig < ContentSecurityPolicyConfig def report_only? true end + + def make_report_only + self + end end end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 99b63c61..c0e49045 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -30,7 +30,8 @@ module SecureHeaders describe "#header_hash_for" do it "allows you to opt out of individual headers via API" do Configuration.default do |config| - config.csp_report_only = { default_src: %w('self')} # no default value + config.csp = { default_src: %w('self')} + config.csp_report_only = config.csp end SecureHeaders.opt_out_of_header(request, ContentSecurityPolicyConfig::CONFIG_KEY) SecureHeaders.opt_out_of_header(request, ContentSecurityPolicyReportOnlyConfig::CONFIG_KEY) @@ -61,7 +62,8 @@ module SecureHeaders it "allows you to opt out entirely" do # configure the disabled-by-default headers to ensure they also do not get set Configuration.default do |config| - config.csp_report_only = { :default_src => ["example.com"] } + config.csp = { :default_src => ["example.com"] } + config.csp_report_only = config.csp config.hpkp = { report_only: false, max_age: 10000000, @@ -293,6 +295,46 @@ module SecureHeaders expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'; script-src 'self'") end + it "allows you to opt-out of enforced CSP" do + Configuration.default do |config| + config.csp = SecureHeaders::OPT_OUT + config.csp_report_only = { + default_src: %w('self') + } + end + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to be_nil + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'") + end + + it "opts-out of enforced CSP when only csp_report_only is set" do + expect(Kernel).to receive(:warn).once + Configuration.default do |config| + config.csp_report_only = { + default_src: %w('self') + } + end + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to be_nil + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'") + end + + it "allows you to set csp_report_only before csp" do + expect(Kernel).to receive(:warn).once + Configuration.default do |config| + config.csp_report_only = { + default_src: %w('self') + } + config.csp = config.csp_report_only.merge({script_src: %w('unsafe-inline')}) + end + + hash = SecureHeaders.header_hash_for(request) + expect(hash['Content-Security-Policy']).to eq("default-src 'self'; script-src 'self' 'unsafe-inline'") + expect(hash['Content-Security-Policy-Report-Only']).to eq("default-src 'self'") + end + it "allows appending to the enforced policy" do SecureHeaders.append_content_security_policy_directives(request, {script_src: %w(anothercdn.com)}, :enforced) hash = SecureHeaders.header_hash_for(request)