Merge pull request #383 from ptoomey3/secure-headers-x

Rewrite the entire notion of named overrides (breaking change)

Author: oreoshake

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 6.0
+
+- See the [upgrading to 6.0](docs/upgrading-to-6-0.md) guide for the breaking changes.
+=======
 ## 5.0.5
 
 A release to deprecate `SecureHeaders::Configuration#get` in prep for 6.x

docs/upgrading-to-6-0.md

@@ -0,0 +1,44 @@
+## Named overrides are now dynamically applied
+
+The original implementation of name overrides worked by making a copy of the default policy, applying the overrides, and storing the result for later use. But, this lead to unexpected results if named overrides were combined with a dynamic policy change. If a change was made to the default configuration during a request, followed by a named override, the dynamic changes would be lost. To keep things consistent named overrides have been rewritten to work the same as named appends in that they always operate on the configuration for the current request. As an example:
+
+```ruby
+class ApplicationController < ActionController::Base
+  Configuration.default do |config|
+    config.x_frame_options = OPT_OUT
+  end
+
+  SecureHeaders::Configuration.override(:dynamic_override) do |config|
+    config.x_content_type_options = "nosniff"
+  end
+end
+
+class FooController < ApplicationController
+  def bar
+    # Dynamically update the default config for this request
+    override_x_frame_options("DENY")
+    append_content_security_policy_directives(frame_src: "3rdpartyprovider.com")
+
+    # Override everything, discard modifications above
+    use_secure_headers_override(:dynamic_override)
+  end
+end
+```
+
+Prior to 6.0.0, the response would NOT include a `X-Frame-Options` header since the named override would be a copy of the default configuration, but with `X-Content-Type-Options` set to `nosniff`. As of 6.0.0, the above code results in both `X-Frame-Options` set to `DENY` AND `X-Content-Type-Options` set to `nosniff`.
+
+## `ContentSecurityPolicyConfig#merge` and `ContentSecurityPolicyReportOnlyConfig#merge` work more like `Hash#merge`
+
+These classes are typically not directly instantiated by users of SecureHeaders. But, if you access `config.csp` you end up accessing one of these objects. Prior to 6.0.0, `#merge` worked more like `#append` in that it would combine policies (i.e. if both policies contained the same key the values would be combined rather than overwritten). This was not consistent with `#merge!`, which worked more like ruby's `Hash#merge!` (overwriting duplicate keys). As of 6.0.0, `#merge` works the same as `#merge!`, but returns a new object instead of mutating `self`.
+
+## `Configuration#get` has been removed
+
+This method is not typically directly called by users of SecureHeaders. Given that named overrides are no longer statically stored, fetching them no longer makes sense.
+
+## Configuration headers are no longer cached
+
+Prior to 6.0.0 SecureHeaders pre-built and cached the headers that corresponded to the default configuration. The same was also done for named overrides. However, now that named overrides are applied dynamically, those can no longer be cached. As a result, caching has been removed in the name of simplicity. Some micro-benchmarks indicate this shouldn't be a performance problem and will help to eliminate a class of bugs entirely.
+
+## Configuration the default configuration more than once will result in an Exception
+
+Prior to 6.0.0 you could conceivably, though unlikely, have `Configure#default` called more than once. Because configurations are dynamic, configuring more than once could result in unexpected behavior. So, as of 6.0.0 we raise `AlreadyConfiguredError` if the default configuration is setup more than once.

lib/secure_headers.rb

@@ -1,5 +1,4 @@
 # frozen_string_literal: true
-require "secure_headers/configuration"
 require "secure_headers/hash_helper"
 require "secure_headers/headers/cookie"
 require "secure_headers/headers/public_key_pins"
@@ -18,6 +17,7 @@
 require "secure_headers/view_helper"
 require "useragent"
 require "singleton"
+require "secure_headers/configuration"
 
 # 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
@@ -52,29 +52,9 @@ def opt_out?
   HTTPS = "https".freeze
   CSP = ContentSecurityPolicy
 
-  ALL_HEADER_CLASSES = [
-    ExpectCertificateTransparency,
-    ClearSiteData,
-    ContentSecurityPolicyConfig,
-    ContentSecurityPolicyReportOnlyConfig,
-    StrictTransportSecurity,
-    PublicKeyPins,
-    ReferrerPolicy,
-    XContentTypeOptions,
-    XDownloadOptions,
-    XFrameOptions,
-    XPermittedCrossDomainPolicies,
-    XXssProtection
-  ].freeze
-
-  ALL_HEADERS_BESIDES_CSP = (
-    ALL_HEADER_CLASSES -
-      [ContentSecurityPolicyConfig, ContentSecurityPolicyReportOnlyConfig]
-  ).freeze
-
   # Headers set on http requests (excludes STS and HPKP)
-  HTTP_HEADER_CLASSES =
-    (ALL_HEADER_CLASSES - [StrictTransportSecurity, PublicKeyPins]).freeze
+  HTTPS_HEADER_CLASSES =
+    [StrictTransportSecurity, PublicKeyPins].freeze
 
   class << self
     # Public: override a given set of directives for the current request. If a
@@ -153,7 +133,7 @@ def opt_out_of_header(request, header_key)
     # Public: opts out of setting all headers by telling secure_headers to use
     # the NOOP configuration.
     def opt_out_of_all_protection(request)
-      use_secure_headers_override(request, Configuration::NOOP_CONFIGURATION)
+      use_secure_headers_override(request, Configuration::NOOP_OVERRIDE)
     end
 
     # Public: Builds the hash of headers that should be applied base on the
@@ -168,39 +148,26 @@ def opt_out_of_all_protection(request)
     def header_hash_for(request)
       prevent_dup = true
       config = config_for(request, prevent_dup)
-      headers = config.cached_headers
+      config.validate_config!
       user_agent = UserAgent.parse(request.user_agent)
+      headers = config.generate_headers(user_agent)
 
-      if !config.csp.opt_out? && config.csp.modified?
-        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, user_agent)
-      end
-
-      header_classes_for(request).each_with_object({}) do |klass, hash|
-        if header = headers[klass::CONFIG_KEY]
-          header_name, value = if klass == ContentSecurityPolicyConfig || klass == ContentSecurityPolicyReportOnlyConfig
-            csp_header_for_ua(header, user_agent)
-          else
-            header
-          end
-          hash[header_name] = value
+      if request.scheme != HTTPS
+        HTTPS_HEADER_CLASSES.each do |klass|
+          headers.delete(klass::HEADER_NAME)
         end
       end
+      headers
     end
 
     # Public: specify which named override will be used for this request.
     # Raises an argument error if no named override exists.
     #
     # name - the name of the previously configured override.
     def use_secure_headers_override(request, name)
-      if config = Configuration.get(name, internal: true)
-        override_secure_headers_request_config(request, config)
-      else
-        raise ArgumentError.new("no override by the name of #{name} has been configured")
-      end
+      config = config_for(request)
+      config.override(name)
+      override_secure_headers_request_config(request, config)
     end
 
     # Public: gets or creates a nonce for CSP.
@@ -228,7 +195,7 @@ def content_security_policy_style_nonce(request)
     # Falls back to the global config
     def config_for(request, prevent_dup = false)
       config = request.env[SECURE_HEADERS_CONFIG] ||
-        Configuration.get(Configuration::DEFAULT_CONFIG, internal: true)
+        Configuration.send(:default_config)
 
 
       # Global configs are frozen, per-request configs are not. When we're not
@@ -285,35 +252,6 @@ 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: determines which headers are applicable to a given request.
-    #
-    # Returns a list of classes whose corresponding header values are valid for
-    # this request.
-    def header_classes_for(request)
-      if request.scheme == HTTPS
-        ALL_HEADER_CLASSES
-      else
-        HTTP_HEADER_CLASSES
-      end
-    end
-
-    def update_cached_csp(config, headers, user_agent)
-      headers = Configuration.send(:deep_copy, headers)
-      headers[config.class::CONFIG_KEY] = {}
-      variation = ContentSecurityPolicy.ua_to_variation(user_agent)
-      headers[config.class::CONFIG_KEY][variation] = ContentSecurityPolicy.make_header(config, 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]
-    #
-    # Returns a CSP [header, value] array
-    def csp_header_for_ua(headers, user_agent)
-      headers[ContentSecurityPolicy.ua_to_variation(user_agent)]
-    end
   end
 
   # These methods are mixed into controllers and delegate to the class method