Merge pull request #246 from twitter/hpkp-warn

Warn if HPKP report host is the same as the current host

Author: oreoshake

README.md

@@ -332,9 +332,7 @@ config.hpkp = {
     {sha256: '73a2c64f9545172c1195efb6616ca5f7afd1df6f245407cafb90de3998a1c97f'}
   ],
   report_only: true,            # defaults to false (report-only mode)
-  report_uri: 'https://report-uri.io/example-hpkp',
-  app_name: 'example',
-  tag_report_uri: true
+  report_uri: 'https://report-uri.io/example-hpkp'
 }
 ```
 

lib/secure_headers/configuration.rb

@@ -61,6 +61,7 @@ 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
@@ -102,11 +103,13 @@ def deep_copy_if_hash(value)
       end
     end
 
+    attr_accessor :dynamic_csp
+
     attr_writer :hsts, :x_frame_options, :x_content_type_options,
       :x_xss_protection, :x_download_options, :x_permitted_cross_domain_policies,
-      :hpkp, :dynamic_csp, :cookies, :referrer_policy
+      :referrer_policy
 
-    attr_reader :cached_headers, :csp, :dynamic_csp, :cookies
+    attr_reader :cached_headers, :csp, :cookies, :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)
@@ -139,6 +142,7 @@ def dup
       copy.x_permitted_cross_domain_policies = @x_permitted_cross_domain_policies
       copy.referrer_policy = @referrer_policy
       copy.hpkp = @hpkp
+      copy.hpkp_report_host = @hpkp_report_host
       copy
     end
 
@@ -202,12 +206,32 @@ def csp=(new_csp)
       @csp = new_csp
     end
 
+    def cookies=(cookies)
+      @cookies = cookies
+    end
+
     def cached_headers=(headers)
       @cached_headers = headers
     end
 
+    def hpkp=(hpkp)
+      @hpkp = 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 configuraiton.
     # Ensures that headers generated at configure time, not on demand.
     #

lib/secure_headers/middleware.rb

@@ -1,5 +1,7 @@
 module SecureHeaders
   class Middleware
+    HPKP_SAME_HOST_WARNING = "[WARNING] HPKP report host should not be the same as the request host. See https://github.com/twitter/secureheaders/issues/166"
+
     def initialize(app)
       @app = app
     end
@@ -10,6 +12,10 @@ def call(env)
       status, headers, response = @app.call(env)
 
       config = SecureHeaders.config_for(req)
+      if config.hpkp_report_host == req.host
+        Kernel.warn(HPKP_SAME_HOST_WARNING)
+      end
+
       flag_cookies!(headers, override_secure(env, config.cookies)) if config.cookies
       headers.merge!(SecureHeaders.header_hash_for(req))
       [status, headers, response]

spec/lib/secure_headers/middleware_spec.rb

@@ -13,6 +13,24 @@ module SecureHeaders
       Configuration.default
     end
 
+    it "warns if the hpkp report-uri host is the same as the current host" do
+      report_host = "report-uri.io"
+      Configuration.default do |config|
+        config.hpkp = {
+          max_age: 10000000,
+          pins: [
+            {sha256: 'b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c'},
+            {sha256: '73a2c64f9545172c1195efb6616ca5f7afd1df6f245407cafb90de3998a1c97f'}
+          ],
+          report_uri: "https://#{report_host}/example-hpkp"
+        }
+      end
+
+      expect(Kernel).to receive(:warn).with(Middleware::HPKP_SAME_HOST_WARNING)
+
+      middleware.call(Rack::MockRequest.env_for("https://#{report_host}", {}))
+    end
+
     it "sets the headers" do
       _, env = middleware.call(Rack::MockRequest.env_for("https://looocalhost", {}))
       expect_default_values(env)