From 64c0acb4e778e9faa35631ef5fc3b94abea8350d Mon Sep 17 00:00:00 2001 From: Neil Matatall Date: Tue, 10 Apr 2018 15:29:27 -1000 Subject: [PATCH] Remove all useragent sniffing --- lib/secure_headers.rb | 4 +- lib/secure_headers/configuration.rb | 4 +- .../headers/content_security_policy.rb | 63 ++---------------- .../headers/policy_management.rb | 50 +------------- secure_headers.gemspec | 1 - .../headers/content_security_policy_spec.rb | 65 +------------------ .../headers/policy_management_spec.rb | 4 +- spec/lib/secure_headers_spec.rb | 37 ----------- 8 files changed, 13 insertions(+), 215 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index f427e988..9926fb2b 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -15,7 +15,6 @@ require "secure_headers/middleware" require "secure_headers/railtie" require "secure_headers/view_helper" -require "useragent" require "singleton" require "secure_headers/configuration" @@ -149,8 +148,7 @@ def header_hash_for(request) prevent_dup = true config = config_for(request, prevent_dup) config.validate_config! - user_agent = UserAgent.parse(request.user_agent) - headers = config.generate_headers(user_agent) + headers = config.generate_headers if request.scheme != HTTPS HTTPS_HEADER_CLASSES.each do |klass| diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index bcdf3268..159e8762 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -194,11 +194,11 @@ def override(name = nil, &block) self end - def generate_headers(user_agent) + def generate_headers headers = {} HEADERABLE_ATTRIBUTES.each do |attr| klass = CONFIG_ATTRIBUTES_TO_HEADER_CLASSES[attr] - header_name, value = klass.make_header(instance_variable_get("@#{attr}"), user_agent) + header_name, value = klass.make_header(instance_variable_get("@#{attr}")) if header_name && value headers[header_name] = value end diff --git a/lib/secure_headers/headers/content_security_policy.rb b/lib/secure_headers/headers/content_security_policy.rb index 3f382a2b..f6af2f79 100644 --- a/lib/secure_headers/headers/content_security_policy.rb +++ b/lib/secure_headers/headers/content_security_policy.rb @@ -1,19 +1,12 @@ # frozen_string_literal: true require_relative "policy_management" require_relative "content_security_policy_config" -require "useragent" module SecureHeaders class ContentSecurityPolicy include PolicyManagement - # constants to be used for version-specific UA sniffing - VERSION_46 = ::UserAgent::Version.new("46") - VERSION_10 = ::UserAgent::Version.new("10") - FALLBACK_VERSION = ::UserAgent::Version.new("0") - - def initialize(config = nil, user_agent = OTHER) - user_agent ||= OTHER + def initialize(config = nil) @config = if config.is_a?(Hash) if config[:report_only] ContentSecurityPolicyReportOnlyConfig.new(config || DEFAULT_CONFIG) @@ -26,12 +19,6 @@ def initialize(config = nil, user_agent = OTHER) config end - @parsed_ua = if user_agent.is_a?(UserAgent::Browsers::Base) - user_agent - else - UserAgent.parse(user_agent) - end - @frame_src = normalize_child_frame_src @preserve_schemes = @config.preserve_schemes @script_nonce = @config.script_nonce @style_nonce = @config.style_nonce @@ -56,20 +43,10 @@ def value private - def normalize_child_frame_src - if @config.frame_src && @config.child_src && @config.frame_src != @config.child_src - raise ArgumentError, "#{Kernel.caller.first}: both :child_src and :frame_src supplied and do not match. This can lead to inconsistent behavior across browsers." - end - - @config.frame_src || @config.child_src - end - # Private: converts the config object into a string representing a policy. # Places default-src at the first directive and report-uri as the last. All # others are presented in alphabetical order. # - # Unsupported directives are filtered based on the user agent. - # # Returns a content security policy header value. def build_value directives.map do |directive_name| @@ -125,18 +102,7 @@ def build_media_type_list_directive(directive) # # Returns a string representing a directive. def build_source_list_directive(directive) - 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 + source_list = @config.directive_value(directive) if source_list != OPT_OUT && source_list && source_list.any? normalized_source_list = minify_source_list(directive, source_list) @@ -219,13 +185,13 @@ def append_nonce(source_list, nonce) source_list end - # Private: return the list of directives that are supported by the user agent, + # Private: return the list of directives, # starting with default-src and ending with report-uri. def directives [ DEFAULT_SRC, - BODY_DIRECTIVES.select { |key| supported_directives.include?(key) }, - REPORT_URI + BODY_DIRECTIVES, + REPORT_URI, ].flatten end @@ -234,25 +200,6 @@ 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. - # - # Add UA-sniffing special casing here. - # - # Returns an array of symbols representing the directives. - def supported_directives - @supported_directives ||= if VARIATIONS[@parsed_ua.browser] - if @parsed_ua.browser == "Firefox" && ((@parsed_ua.version || FALLBACK_VERSION) >= VERSION_46) - VARIATIONS["FirefoxTransitional"] - elsif @parsed_ua.browser == "Safari" && ((@parsed_ua.version || FALLBACK_VERSION) >= VERSION_10) - VARIATIONS["SafariTransitional"] - else - VARIATIONS[@parsed_ua.browser] - end - else - VARIATIONS[OTHER] - end - end - def symbol_to_hyphen_case(sym) sym.to_s.tr("_", "-") end diff --git a/lib/secure_headers/headers/policy_management.rb b/lib/secure_headers/headers/policy_management.rb index 00b9f052..c204f073 100644 --- a/lib/secure_headers/headers/policy_management.rb +++ b/lib/secure_headers/headers/policy_management.rb @@ -81,58 +81,12 @@ def self.included(base) UPGRADE_INSECURE_REQUESTS ].flatten.freeze - EDGE_DIRECTIVES = DIRECTIVES_1_0 - SAFARI_DIRECTIVES = DIRECTIVES_1_0 - SAFARI_10_DIRECTIVES = DIRECTIVES_2_0 - - FIREFOX_UNSUPPORTED_DIRECTIVES = [ - BLOCK_ALL_MIXED_CONTENT, - CHILD_SRC, - WORKER_SRC, - PLUGIN_TYPES - ].freeze - - FIREFOX_46_DEPRECATED_DIRECTIVES = [ - FRAME_SRC - ].freeze - - FIREFOX_46_UNSUPPORTED_DIRECTIVES = [ - BLOCK_ALL_MIXED_CONTENT, - WORKER_SRC, - PLUGIN_TYPES - ].freeze - - FIREFOX_DIRECTIVES = ( - DIRECTIVES_3_0 - FIREFOX_UNSUPPORTED_DIRECTIVES - ).freeze - - FIREFOX_46_DIRECTIVES = ( - DIRECTIVES_3_0 - FIREFOX_46_UNSUPPORTED_DIRECTIVES - FIREFOX_46_DEPRECATED_DIRECTIVES - ).freeze - - CHROME_DIRECTIVES = ( - DIRECTIVES_3_0 - ).freeze - ALL_DIRECTIVES = (DIRECTIVES_1_0 + DIRECTIVES_2_0 + DIRECTIVES_3_0).uniq.sort # Think of default-src and report-uri as the beginning and end respectively, # everything else is in between. BODY_DIRECTIVES = ALL_DIRECTIVES - [DEFAULT_SRC, REPORT_URI] - VARIATIONS = { - "Chrome" => CHROME_DIRECTIVES, - "Opera" => CHROME_DIRECTIVES, - "Firefox" => FIREFOX_DIRECTIVES, - "FirefoxTransitional" => FIREFOX_46_DIRECTIVES, - "Safari" => SAFARI_DIRECTIVES, - "SafariTransitional" => SAFARI_10_DIRECTIVES, - "Edge" => EDGE_DIRECTIVES, - "Other" => CHROME_DIRECTIVES - }.freeze - - OTHER = "Other".freeze - DIRECTIVE_VALUE_TYPES = { BASE_URI => :source_list, BLOCK_ALL_MIXED_CONTENT => :boolean, @@ -199,9 +153,9 @@ module ClassMethods # # Returns a default policy if no configuration is provided, or a # header name and value based on the config. - def make_header(config, user_agent = nil) + def make_header(config) return if config.nil? || config == OPT_OUT - header = new(config, user_agent) + header = new(config) [header.name, header.value] end diff --git a/secure_headers.gemspec b/secure_headers.gemspec index b0d8f7dc..cbf0a3eb 100644 --- a/secure_headers.gemspec +++ b/secure_headers.gemspec @@ -16,5 +16,4 @@ Gem::Specification.new do |gem| gem.test_files = gem.files.grep(%r{^(test|spec|features)/}) gem.require_paths = ["lib"] gem.add_development_dependency "rake" - gem.add_dependency "useragent", ">= 0.15.0" end 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 f2462711..04b98926 100644 --- a/spec/lib/secure_headers/headers/content_security_policy_spec.rb +++ b/spec/lib/secure_headers/headers/content_security_policy_spec.rb @@ -116,73 +116,10 @@ module SecureHeaders ContentSecurityPolicy.new(default_src: %w('self'), frame_src: %w('self')).value end - it "raises an error when child-src and frame-src are supplied but are not equal" do - expect { - ContentSecurityPolicy.new(default_src: %w('self'), child_src: %w(child-src.com), frame_src: %w(frame-src,com)).value - }.to raise_error(ArgumentError) - end - it "supports strict-dynamic" do - csp = ContentSecurityPolicy.new({default_src: %w('self'), script_src: [ContentSecurityPolicy::STRICT_DYNAMIC], script_nonce: 123456}, USER_AGENTS[:chrome]) + csp = ContentSecurityPolicy.new({default_src: %w('self'), script_src: [ContentSecurityPolicy::STRICT_DYNAMIC], script_nonce: 123456}) expect(csp.value).to eq("default-src 'self'; script-src 'strict-dynamic' 'nonce-123456' 'unsafe-inline'") end - - context "browser sniffing" do - let (:complex_opts) do - (ContentSecurityPolicy::ALL_DIRECTIVES - [:frame_src]).each_with_object({}) do |directive, hash| - hash[directive] = ["#{directive.to_s.gsub("_", "-")}.com"] - end.merge({ - block_all_mixed_content: true, - upgrade_insecure_requests: true, - script_src: %w(script-src.com), - script_nonce: 123456, - sandbox: %w(allow-forms), - plugin_types: %w(application/pdf) - }) - end - - it "does not filter any directives for Chrome" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:chrome]) - expect(policy.value).to eq("default-src default-src.com; base-uri base-uri.com; block-all-mixed-content; child-src child-src.com; connect-src connect-src.com; font-src font-src.com; form-action form-action.com; frame-ancestors frame-ancestors.com; img-src img-src.com; manifest-src manifest-src.com; media-src media-src.com; object-src object-src.com; plugin-types application/pdf; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; upgrade-insecure-requests; worker-src worker-src.com; report-uri report-uri.com") - end - - it "does not filter any directives for Opera" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:opera]) - expect(policy.value).to eq("default-src default-src.com; base-uri base-uri.com; block-all-mixed-content; child-src child-src.com; connect-src connect-src.com; font-src font-src.com; form-action form-action.com; frame-ancestors frame-ancestors.com; img-src img-src.com; manifest-src manifest-src.com; media-src media-src.com; object-src object-src.com; plugin-types application/pdf; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; upgrade-insecure-requests; worker-src worker-src.com; report-uri report-uri.com") - end - - it "filters blocked-all-mixed-content, child-src, and plugin-types for firefox" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:firefox]) - expect(policy.value).to eq("default-src default-src.com; base-uri base-uri.com; connect-src connect-src.com; font-src font-src.com; form-action form-action.com; frame-ancestors frame-ancestors.com; frame-src child-src.com; img-src img-src.com; manifest-src manifest-src.com; media-src media-src.com; object-src object-src.com; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; upgrade-insecure-requests; report-uri report-uri.com") - end - - it "filters blocked-all-mixed-content, frame-src, and plugin-types for firefox 46 and higher" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:firefox46]) - expect(policy.value).to eq("default-src default-src.com; base-uri base-uri.com; child-src child-src.com; connect-src connect-src.com; font-src font-src.com; form-action form-action.com; frame-ancestors frame-ancestors.com; img-src img-src.com; manifest-src manifest-src.com; media-src media-src.com; object-src object-src.com; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; upgrade-insecure-requests; report-uri report-uri.com") - end - - it "child-src value is copied to frame-src, adds 'unsafe-inline', filters base-uri, blocked-all-mixed-content, upgrade-insecure-requests, child-src, form-action, frame-ancestors, hash sources, and plugin-types for Edge" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:edge]) - expect(policy.value).to eq("default-src default-src.com; connect-src connect-src.com; font-src font-src.com; frame-src child-src.com; img-src img-src.com; media-src media-src.com; object-src object-src.com; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; report-uri report-uri.com") - end - - it "child-src value is copied to frame-src, adds 'unsafe-inline', filters base-uri, blocked-all-mixed-content, upgrade-insecure-requests, child-src, form-action, frame-ancestors, hash sources, and plugin-types for safari" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:safari6]) - expect(policy.value).to eq("default-src default-src.com; connect-src connect-src.com; font-src font-src.com; frame-src child-src.com; img-src img-src.com; media-src media-src.com; object-src object-src.com; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; report-uri report-uri.com") - end - - it "adds 'unsafe-inline', filters blocked-all-mixed-content, upgrade-insecure-requests, and hash sources for safari 10 and higher" do - policy = ContentSecurityPolicy.new(complex_opts, USER_AGENTS[:safari10]) - expect(policy.value).to eq("default-src default-src.com; base-uri base-uri.com; child-src child-src.com; connect-src connect-src.com; font-src font-src.com; form-action form-action.com; frame-ancestors frame-ancestors.com; img-src img-src.com; media-src media-src.com; object-src object-src.com; plugin-types application/pdf; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; report-uri report-uri.com") - end - - it "falls back to standard Firefox defaults when the useragent version is not present" do - ua = USER_AGENTS[:firefox].dup - allow(ua).to receive(:version).and_return(nil) - policy = ContentSecurityPolicy.new(complex_opts, ua) - expect(policy.value).to eq("default-src default-src.com; base-uri base-uri.com; connect-src connect-src.com; font-src font-src.com; form-action form-action.com; frame-ancestors frame-ancestors.com; frame-src child-src.com; img-src img-src.com; manifest-src manifest-src.com; media-src media-src.com; object-src object-src.com; sandbox allow-forms; script-src script-src.com 'nonce-123456' 'unsafe-inline'; style-src style-src.com; upgrade-insecure-requests; report-uri report-uri.com") - end - end end 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 8c10b074..d0cb0cea 100644 --- a/spec/lib/secure_headers/headers/policy_management_spec.rb +++ b/spec/lib/secure_headers/headers/policy_management_spec.rb @@ -190,7 +190,7 @@ module SecureHeaders report_uri = "https://report-uri.io/asdf" default_policy = Configuration.dup combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, report_uri: [report_uri]) - csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox]) + csp = ContentSecurityPolicy.new(combined_config) expect(csp.value).to include("report-uri #{report_uri}") end @@ -223,7 +223,7 @@ module SecureHeaders end default_policy = Configuration.dup combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, report_only: true) - csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox]) + csp = ContentSecurityPolicy.new(combined_config) expect(csp.name).to eq(ContentSecurityPolicyReportOnlyConfig::HEADER_NAME) end diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index 0fc87481..6ef9d05b 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -127,27 +127,6 @@ module SecureHeaders expect(hash[XFrameOptions::HEADER_NAME]).to eq(XFrameOptions::SAMEORIGIN) end - it "produces a UA-specific CSP when overriding (and busting the cache)" do - Configuration.default do |config| - config.csp = { - default_src: %w('self'), - script_src: %w('self'), - child_src: %w('self') - } - end - 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(application/pdf)}) - # append a supported directive - SecureHeaders.override_content_security_policy_directives(firefox_request, {script_src: %w('self')}) - - hash = SecureHeaders.header_hash_for(firefox_request) - - # child-src is translated to frame-src - 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 Configuration.default hash = SecureHeaders.header_hash_for(request) @@ -197,22 +176,6 @@ module SecureHeaders expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src mycdn.com 'unsafe-inline' anothercdn.com") end - it "child-src and frame-src must match" do - Configuration.default do |config| - config.csp = { - default_src: %w('self'), - frame_src: %w(frame_src.com), - script_src: %w('self') - } - end - - SecureHeaders.append_content_security_policy_directives(chrome_request, child_src: %w(child_src.com)) - - expect { - SecureHeaders.header_hash_for(chrome_request) - }.to raise_error(ArgumentError) - end - it "supports named appends" do Configuration.default do |config| config.csp = {