diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 6426e538..d2be32e7 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -133,6 +133,7 @@ def opt_out_of_all_protection(request) # request. # # StrictTransportSecurity is not applied to http requests. + # upgrade_insecure_requests is not applied to http requests. # See #config_for to determine which config is used for a given request. # # Returns a hash of header names => header values. The value @@ -146,6 +147,11 @@ def header_hash_for(request) if request.scheme != HTTPS headers.delete(StrictTransportSecurity::HEADER_NAME) + + # Remove upgrade_insecure_requests from CSP headers for HTTP requests + # as it doesn't make sense to upgrade requests when the page itself is served over HTTP + remove_upgrade_insecure_requests_from_csp!(headers, config.csp) + remove_upgrade_insecure_requests_from_csp!(headers, config.csp_report_only) end headers end @@ -242,6 +248,23 @@ def content_security_policy_nonce(request, script_or_style) def override_secure_headers_request_config(request, config) request.env[SECURE_HEADERS_CONFIG] = config end + + # Private: removes upgrade_insecure_requests directive from a CSP config + # if it's present, and updates the headers hash with the modified CSP. + # + # headers - the headers hash to update + # csp_config - the CSP config to check and potentially modify + # + # Returns nothing (modifies headers in place) + def remove_upgrade_insecure_requests_from_csp!(headers, csp_config) + return if csp_config.opt_out? + return unless csp_config.directive_value(ContentSecurityPolicy::UPGRADE_INSECURE_REQUESTS) + + modified_config = csp_config.dup + modified_config.update_directive(ContentSecurityPolicy::UPGRADE_INSECURE_REQUESTS, false) + header_name, value = ContentSecurityPolicy.make_header(modified_config) + headers[header_name] = value if header_name && value + end end # These methods are mixed into controllers and delegate to the class method diff --git a/spec/lib/secure_headers_spec.rb b/spec/lib/secure_headers_spec.rb index fd66d487..ae717c88 100644 --- a/spec/lib/secure_headers_spec.rb +++ b/spec/lib/secure_headers_spec.rb @@ -436,6 +436,68 @@ module SecureHeaders end end + + it "does not set upgrade-insecure-requests if request is over HTTP" do + reset_config + Configuration.default do |config| + config.csp = { + default_src: %w('self'), + script_src: %w('self'), + upgrade_insecure_requests: true + } + end + + plaintext_request = Rack::Request.new({}) + hash = SecureHeaders.header_hash_for(plaintext_request) + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'") + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).not_to include("upgrade-insecure-requests") + end + + it "sets upgrade-insecure-requests if request is over HTTPS" do + reset_config + Configuration.default do |config| + config.csp = { + default_src: %w('self'), + script_src: %w('self'), + upgrade_insecure_requests: true + } + end + + https_request = Rack::Request.new("HTTPS" => "on") + hash = SecureHeaders.header_hash_for(https_request) + expect(hash[ContentSecurityPolicyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'; upgrade-insecure-requests") + end + + it "does not set upgrade-insecure-requests in report-only mode if request is over HTTP" do + reset_config + Configuration.default do |config| + config.csp_report_only = { + default_src: %w('self'), + script_src: %w('self'), + upgrade_insecure_requests: true + } + end + + plaintext_request = Rack::Request.new({}) + hash = SecureHeaders.header_hash_for(plaintext_request) + expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'") + expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).not_to include("upgrade-insecure-requests") + end + + it "sets upgrade-insecure-requests in report-only mode if request is over HTTPS" do + reset_config + Configuration.default do |config| + config.csp_report_only = { + default_src: %w('self'), + script_src: %w('self'), + upgrade_insecure_requests: true + } + end + + https_request = Rack::Request.new("HTTPS" => "on") + hash = SecureHeaders.header_hash_for(https_request) + expect(hash[ContentSecurityPolicyReportOnlyConfig::HEADER_NAME]).to eq("default-src 'self'; script-src 'self'; upgrade-insecure-requests") + end end end