From ce3c737fe345b2231198e845b081d8f46e4ac20b Mon Sep 17 00:00:00 2001 From: Julich Mera Date: Tue, 15 Mar 2016 16:43:44 -0400 Subject: [PATCH 1/3] We would like to give users the ability to flag all cookies as secure. https://www.owasp.org/index.php/SecureFlag --- lib/secure_headers/configuration.rb | 2 +- lib/secure_headers/middleware.rb | 26 +++++++++++++++++++ lib/secure_headers/railtie.rb | 2 +- spec/lib/secure_headers/configuration_spec.rb | 2 +- spec/lib/secure_headers/middleware_spec.rb | 24 ++++++++++++++--- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/lib/secure_headers/configuration.rb b/lib/secure_headers/configuration.rb index be2b0c24..d97aff82 100644 --- a/lib/secure_headers/configuration.rb +++ b/lib/secure_headers/configuration.rb @@ -90,7 +90,7 @@ def add_noop_configuration attr_accessor :hsts, :x_frame_options, :x_content_type_options, :x_xss_protection, :csp, :x_download_options, :x_permitted_cross_domain_policies, - :hpkp + :hpkp, :secure_cookies attr_reader :cached_headers def initialize(&block) diff --git a/lib/secure_headers/middleware.rb b/lib/secure_headers/middleware.rb index 02e8472e..06130097 100644 --- a/lib/secure_headers/middleware.rb +++ b/lib/secure_headers/middleware.rb @@ -1,5 +1,7 @@ module SecureHeaders class Middleware + SECURE_COOKIE_REGEXP = /;\s*secure\s*(;|$)/i.freeze + def initialize(app) @app = app end @@ -8,8 +10,32 @@ def initialize(app) def call(env) req = Rack::Request.new(env) status, headers, response = @app.call(env) + + flag_cookies_as_secure!(headers) if config(req).secure_cookies headers.merge!(SecureHeaders.header_hash_for(req)) [status, headers, response] end + + private + + # inspired by https://github.com/tobmatth/rack-ssl-enforcer/blob/6c014/lib/rack/ssl-enforcer.rb#L183-L194 + def flag_cookies_as_secure!(headers) + if cookies = headers['Set-Cookie'] + # Support Rails 2.3 / Rack 1.1 arrays as headers + cookies = cookies.split("\n") unless cookies.is_a?(Array) + + headers['Set-Cookie'] = cookies.map do |cookie| + if cookie !~ SECURE_COOKIE_REGEXP + "#{cookie}; secure" + else + cookie + end + end.join("\n") + end + end + + def config(req) + req.env[SECURE_HEADERS_CONFIG] || Configuration.get(Configuration::DEFAULT_CONFIG) + end end end diff --git a/lib/secure_headers/railtie.rb b/lib/secure_headers/railtie.rb index daeb2c7e..efa80be2 100644 --- a/lib/secure_headers/railtie.rb +++ b/lib/secure_headers/railtie.rb @@ -10,7 +10,7 @@ class Railtie < Rails::Railtie 'Public-Key-Pins', 'Public-Key-Pins-Report-Only'] initializer "secure_headers.middleware" do - Rails.application.config.middleware.use SecureHeaders::Middleware + Rails.application.config.middleware.insert_before 0, SecureHeaders::Middleware end initializer "secure_headers.action_controller" do diff --git a/spec/lib/secure_headers/configuration_spec.rb b/spec/lib/secure_headers/configuration_spec.rb index 59d9d5a2..496acb6b 100644 --- a/spec/lib/secure_headers/configuration_spec.rb +++ b/spec/lib/secure_headers/configuration_spec.rb @@ -37,7 +37,7 @@ module SecureHeaders config = Configuration.get(:test_override) noop = Configuration.get(Configuration::NOOP_CONFIGURATION) [:hsts, :x_frame_options, :x_content_type_options, :x_xss_protection, - :x_download_options, :x_permitted_cross_domain_policies, :hpkp, :csp].each do |key| + :x_download_options, :x_permitted_cross_domain_policies, :hpkp, :csp, :secure_cookies].each do |key| expect(config.send(key)).to eq(noop.send(key)), "Value not copied: #{key}." end diff --git a/spec/lib/secure_headers/middleware_spec.rb b/spec/lib/secure_headers/middleware_spec.rb index 418f88f4..81b4dbba 100644 --- a/spec/lib/secure_headers/middleware_spec.rb +++ b/spec/lib/secure_headers/middleware_spec.rb @@ -3,10 +3,10 @@ module SecureHeaders describe Middleware do let(:app) { ->(env) { [200, env, "app"] } } + let(:cookie_app) { -> (env) { [200, env.merge("Set-Cookie" => "foo=bar"), "app"] } } - let :middleware do - Middleware.new(app) - end + let(:middleware) { Middleware.new(app) } + let(:cookie_middleware) { Middleware.new(cookie_app) } before(:each) do reset_config @@ -36,5 +36,23 @@ module SecureHeaders _, env = middleware.call request.env expect(env[CSP::HEADER_NAME]).to match("example.org") end + + context "cookies should be flagged" do + it "flags cookies as secure" do + Configuration.default { |config| config.secure_cookies = true } + request = Rack::MockRequest.new(cookie_middleware) + response = request.get '/' + expect(response.headers['Set-Cookie']).to match(Middleware::SECURE_COOKIE_REGEXP) + end + end + + context "cookies should not be flagged" do + it "does not flags cookies as secure" do + Configuration.default { |config| config.secure_cookies = false } + request = Rack::MockRequest.new(cookie_middleware) + response = request.get '/' + expect(response.headers['Set-Cookie']).not_to match(Middleware::SECURE_COOKIE_REGEXP) + end + end end end From 0cd223a97978424c57f91bac22fc460ab6203967 Mon Sep 17 00:00:00 2001 From: Julich Mera Date: Tue, 15 Mar 2016 17:00:40 -0400 Subject: [PATCH 2/3] Make CI happy, use longer lambda syntax --- spec/lib/secure_headers/middleware_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/secure_headers/middleware_spec.rb b/spec/lib/secure_headers/middleware_spec.rb index 81b4dbba..2bc9be95 100644 --- a/spec/lib/secure_headers/middleware_spec.rb +++ b/spec/lib/secure_headers/middleware_spec.rb @@ -2,8 +2,8 @@ module SecureHeaders describe Middleware do - let(:app) { ->(env) { [200, env, "app"] } } - let(:cookie_app) { -> (env) { [200, env.merge("Set-Cookie" => "foo=bar"), "app"] } } + let(:app) { lambda { |env| [200, env, "app"] } } + let(:cookie_app) { lambda { |env| [200, env.merge("Set-Cookie" => "foo=bar"), "app"] } } let(:middleware) { Middleware.new(app) } let(:cookie_middleware) { Middleware.new(cookie_app) } From 3301a0d02e1ad28955e414aeb9ab08be1e476ea9 Mon Sep 17 00:00:00 2001 From: Julich Mera Date: Tue, 15 Mar 2016 17:20:09 -0400 Subject: [PATCH 3/3] Make SecureHeaders.config_for public allowing us to access config from different contexts. --- lib/secure_headers.rb | 20 ++++++++++---------- lib/secure_headers/middleware.rb | 7 ++----- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/secure_headers.rb b/lib/secure_headers.rb index 0f48d92f..0a1b5416 100644 --- a/lib/secure_headers.rb +++ b/lib/secure_headers.rb @@ -149,6 +149,16 @@ def content_security_policy_style_nonce(request) content_security_policy_nonce(request, CSP::STYLE_SRC) end + # Public: Retreives the config for a given header type: + # + # Checks to see if there is an override for this request, then + # Checks to see if a named override is used for this request, then + # Falls back to the global config + def config_for(request) + request.env[SECURE_HEADERS_CONFIG] || + Configuration.get(Configuration::DEFAULT_CONFIG) + end + private # Private: gets or creates a nonce for CSP. @@ -217,16 +227,6 @@ def use_cached_headers(default_headers, request) end end - # Private: Retreives the config for a given header type: - # - # Checks to see if there is an override for this request, then - # Checks to see if a named override is used for this request, then - # Falls back to the global config - def config_for(request) - request.env[SECURE_HEADERS_CONFIG] || - Configuration.get(Configuration::DEFAULT_CONFIG) - end - # Private: chooses the applicable CSP header for the provided user agent. # # headers - a hash of header_config_key => [header_name, header_value] diff --git a/lib/secure_headers/middleware.rb b/lib/secure_headers/middleware.rb index 06130097..603359bc 100644 --- a/lib/secure_headers/middleware.rb +++ b/lib/secure_headers/middleware.rb @@ -11,7 +11,8 @@ def call(env) req = Rack::Request.new(env) status, headers, response = @app.call(env) - flag_cookies_as_secure!(headers) if config(req).secure_cookies + config = SecureHeaders.config_for(req) + flag_cookies_as_secure!(headers) if config.secure_cookies headers.merge!(SecureHeaders.header_hash_for(req)) [status, headers, response] end @@ -33,9 +34,5 @@ def flag_cookies_as_secure!(headers) end.join("\n") end end - - def config(req) - req.env[SECURE_HEADERS_CONFIG] || Configuration.get(Configuration::DEFAULT_CONFIG) - end end end