Remove caching and make overrides work as expected

We ran into a bug where overrides weren't working as expected. The user
expectation is that an override should override a config value for whatever
the current configuration for the current request. But, as implemented, they
actually were just forks of a base config..regardless of whether that base
config changed during runtime. So, overrides no work that way and we don't
currently support any way of using an alternate config. This might be added
back later if it is useful.

While making the changes related to overrides it became obvious that caching
was a huge source of complexity. So, this also removes all caching. A quick
local benchmark shows that a non-cached config takes about .18ms to build out
a set of headers, so it shouldn't be a big concern.

Author: ptoomey3

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,35 +148,25 @@ 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
       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)
+      if override = Configuration.overrides(name)
+        config = config_for(request)
+        config.instance_eval(&override)
         override_secure_headers_request_config(request, config)
       else
         raise ArgumentError.new("no override by the name of #{name} has been configured")
@@ -285,35 +255,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

lib/secure_headers/configuration.rb

@@ -4,7 +4,7 @@
 module SecureHeaders
   class Configuration
     DEFAULT_CONFIG = :default
-    NOOP_CONFIGURATION = "secure_headers_noop_config"
+    NOOP_OVERRIDE = "secure_headers_noop_override"
     class NotYetConfiguredError < StandardError; end
     class IllegalPolicyModificationError < StandardError; end
     class << self
@@ -15,8 +15,13 @@ class << self
       # Returns the newly created config.
       def default(&block)
         config = new(&block)
-        add_noop_configuration
         add_configuration(DEFAULT_CONFIG, config)
+        override(NOOP_OVERRIDE) do |config|
+          CONFIG_ATTRIBUTES.each do |attr|
+            config.instance_variable_set("@#{attr}", OPT_OUT)
+          end
+        end
+        config
       end
       alias_method :configure, :default
 
@@ -27,13 +32,14 @@ def default(&block)
       # if no value is supplied.
       #
       # Returns: the newly created config
-      def override(name, base = DEFAULT_CONFIG, &block)
-        unless get(base)
-          raise NotYetConfiguredError, "#{base} policy not yet supplied"
-        end
-        override = @configurations[base].dup
-        override.instance_eval(&block) if block_given?
-        add_configuration(name, override)
+      def override(name, &block)
+        @overrides ||= {}
+        @overrides[name] = block
+      end
+
+      def overrides(name)
+        @overrides ||= {}
+        @overrides[name]
       end
 
       # Public: retrieve a global configuration object
@@ -72,25 +78,10 @@ def named_append(name, target = nil, &block)
       def add_configuration(name, config)
         config.validate_config!
         @configurations ||= {}
-        config.send(:cache_headers!)
-        config.send(:cache_hpkp_report_host)
         config.freeze
         @configurations[name] = config
       end
 
-      # Private: Automatically add an "opt-out of everything" override.
-      #
-      # Returns the noop config
-      def add_noop_configuration
-        noop_config = new do |config|
-          ALL_HEADER_CLASSES.each do |klass|
-            config.send("#{klass::CONFIG_KEY}=", OPT_OUT)
-          end
-        end
-
-        add_configuration(NOOP_CONFIGURATION, noop_config)
-      end
-
       # Public: perform a basic deep dup. The shallow copy provided by dup/clone
       # can lead to modifying parent objects.
       def deep_copy(config)
@@ -115,11 +106,28 @@ def deep_copy_if_hash(value)
       end
     end
 
-    attr_writer :hsts, :x_frame_options, :x_content_type_options,
-      :x_xss_protection, :x_download_options, :x_permitted_cross_domain_policies,
-      :referrer_policy, :clear_site_data, :expect_certificate_transparency
-
-    attr_reader :cached_headers, :csp, :cookies, :csp_report_only, :hpkp, :hpkp_report_host
+    NON_HEADER_ATTRIBUTES = [
+      :cookies, :hpkp_report_host
+    ].freeze
+
+    HEADER_ATTRIBUTES_TO_HEADER_CLASSES = {
+      hsts: StrictTransportSecurity,
+      x_frame_options: XFrameOptions,
+      x_content_type_options: XContentTypeOptions,
+      x_xss_protection: XXssProtection,
+      x_download_options: XDownloadOptions,
+      x_permitted_cross_domain_policies: XPermittedCrossDomainPolicies,
+      referrer_policy: ReferrerPolicy,
+      clear_site_data: ClearSiteData,
+      expect_certificate_transparency: ExpectCertificateTransparency,
+      csp: ContentSecurityPolicy,
+      csp_report_only: ContentSecurityPolicy,
+      hpkp: PublicKeyPins,
+    }.freeze
+
+    CONFIG_ATTRIBUTES = (HEADER_ATTRIBUTES_TO_HEADER_CLASSES.keys + NON_HEADER_ATTRIBUTES).freeze
+
+    attr_accessor(*CONFIG_ATTRIBUTES)
 
     @script_hashes = nil
     @style_hashes = nil
@@ -154,15 +162,14 @@ def initialize(&block)
       instance_eval(&block) if block_given?
     end
 
-    # Public: copy everything but the cached headers
+    # Public: copy everything
     #
     # Returns a deep-dup'd copy of this configuration.
     def dup
       copy = self.class.new
       copy.cookies = self.class.send(:deep_copy_if_hash, @cookies)
       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
       copy.x_frame_options = @x_frame_options
@@ -173,18 +180,26 @@ def dup
       copy.expect_certificate_transparency = @expect_certificate_transparency
       copy.referrer_policy = @referrer_policy
       copy.hpkp = @hpkp
-      copy.hpkp_report_host = @hpkp_report_host
       copy
     end
 
+    def generate_headers(user_agent)
+      headers = {}
+      HEADER_ATTRIBUTES_TO_HEADER_CLASSES.each do |attr, klass|
+        header_name, value = klass.make_header(instance_variable_get("@#{attr}"), user_agent)
+        if header_name && value
+          headers[header_name] = value
+        end
+      end
+      headers
+    end
+
     def opt_out(header)
       send("#{header}=", OPT_OUT)
-      self.cached_headers.delete(header)
     end
 
     def update_x_frame_options(value)
       @x_frame_options = value
-      self.cached_headers[XFrameOptions::CONFIG_KEY] = XFrameOptions.make_header(value)
     end
 
     # Public: validates all configurations values.
@@ -193,18 +208,9 @@ def update_x_frame_options(value)
     #
     # Returns nothing
     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)
-      XXssProtection.validate_config!(@x_xss_protection)
-      XDownloadOptions.validate_config!(@x_download_options)
-      XPermittedCrossDomainPolicies.validate_config!(@x_permitted_cross_domain_policies)
-      ClearSiteData.validate_config!(@clear_site_data)
-      ExpectCertificateTransparency.validate_config!(@expect_certificate_transparency)
-      PublicKeyPins.validate_config!(@hpkp)
+      HEADER_ATTRIBUTES_TO_HEADER_CLASSES.each do |attr, klass|
+        klass.validate_config!(instance_variable_get("@#{attr}"))
+      end
       Cookie.validate_config!(@cookies)
     end
 
@@ -247,72 +253,19 @@ def csp_report_only=(new_csp)
       end
     end
 
+    def hpkp_report_host
+      return nil unless @hpkp && hpkp != OPT_OUT && @hpkp[:report_uri]
+      URI.parse(@hpkp[:report_uri]).host
+    end
+
     protected
 
     def cookies=(cookies)
       @cookies = cookies
     end
 
-    def cached_headers=(headers)
-      @cached_headers = headers
-    end
-
     def hpkp=(hpkp)
       @hpkp = self.class.send(:deep_copy_if_hash, hpkp)
     end
-
-    def hpkp_report_host=(hpkp_report_host)
-      @hpkp_report_host = hpkp_report_host
-    end
-
-    private
-
-    def cache_hpkp_report_host
-      has_report_uri = @hpkp && @hpkp != OPT_OUT && @hpkp[:report_uri]
-      self.hpkp_report_host = if has_report_uri
-        parsed_report_uri = URI.parse(@hpkp[:report_uri])
-        parsed_report_uri.host
-      end
-    end
-
-    # Public: Precompute the header names and values for this configuration.
-    # Ensures that headers generated at configure time, not on demand.
-    #
-    # Returns the cached headers
-    def cache_headers!
-      # generate defaults for the "easy" headers
-      headers = (ALL_HEADERS_BESIDES_CSP).each_with_object({}) do |klass, hash|
-        config = instance_variable_get("@#{klass::CONFIG_KEY}")
-        unless config == OPT_OUT
-          hash[klass::CONFIG_KEY] = klass.make_header(config).freeze
-        end
-      end
-
-      generate_csp_headers(headers)
-
-      headers.freeze
-      self.cached_headers = headers
-    end
-
-    # Private: adds CSP headers for each variation of CSP support.
-    #
-    # headers - generated headers are added to this hash namespaced by The
-    #   different variations
-    #
-    # Returns nothing
-    def generate_csp_headers(headers)
-      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)
-      unless csp_config.opt_out?
-        headers[header_key] = {}
-        ContentSecurityPolicy::VARIATIONS.each_key do |name|
-          csp = ContentSecurityPolicy.make_header(csp_config, UserAgent.parse(name))
-          headers[header_key][name] = csp.freeze
-        end
-      end
-    end
   end
 end