From 4005a21c5d36fa8e46c37ac8c752757939f43991 Mon Sep 17 00:00:00 2001 From: Geoff Youngs Date: Tue, 6 Sep 2022 17:47:14 +0100 Subject: [PATCH] Add support for SameSite=X & __Secure- / __Host- prefixes Currently if a site specifies SameSite=None in the cookie, it will be lost when HTTP::Cookie.parse parses it. This PR adds support for parsing, preserving and restoring, via HTTP::Cookie#set_cookie_value the SameSite=X attribute. It also respects the __Secure- and __Host- prefixes and rejects cookies that violate the spec. Sources: - https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#samesite_attribute - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#cookie_prefixes - https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cookie-prefixes --- lib/http/cookie.rb | 36 ++++++++++++++++++++++- lib/http/cookie/scanner.rb | 2 ++ test/test_http_cookie.rb | 60 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/lib/http/cookie.rb b/lib/http/cookie.rb index 01e8254..12b1820 100644 --- a/lib/http/cookie.rb +++ b/lib/http/cookie.rb @@ -129,7 +129,7 @@ class HTTP::Cookie # def initialize(*args) @name = @origin = @domain = @path = - @expires = @max_age = nil + @expires = @max_age = @same_site = nil @for_domain = @secure = @httponly = false @session = true @created_at = @accessed_at = Time.now @@ -170,6 +170,8 @@ def initialize(*args) origin = val when :for_domain, :for_domain? for_domain = val + when :same_site + self.same_site = val when :max_age # Let max_age take precedence over expires max_age = val @@ -309,6 +311,8 @@ def parse(set_cookie, origin, options = nil, &block) cookie.secure = avalue when 'httponly' cookie.httponly = avalue + when 'samesite' + cookie.same_site = avalue end rescue => e logger.warn("Couldn't parse #{aname} '#{avalue}': #{e}") if logger @@ -545,6 +549,13 @@ def expire! # The time this cookie was last accessed at. attr_accessor :accessed_at + # Value of SameSite attribute, if set + attr_reader :same_site + + def same_site=(value) + @same_site = value.downcase + end + # Tests if it is OK to accept this cookie if it is sent from a given # URI/URL, `uri`. def acceptable_from_uri?(uri) @@ -572,6 +583,14 @@ def acceptable? raise "domain is missing" when @path.nil? raise "path is missing" + when same_site.eql?('none') + secure? && (URI::HTTPS === @origin) + when secure_prefix? + secure? && (URI::HTTPS === @origin) + when host_prefix? + secure? && (URI::HTTPS === @origin) && + @path == '/' && + !for_domain? when @origin.nil? true else @@ -579,6 +598,18 @@ def acceptable? end end + # Secure prefix check. Returns true if the cookie name begins with + # the magic __Secure- prefix that means a cookie must be secure. + def secure_prefix? + @name.start_with?('__Secure-') + end + + # Host prefix check. Returns true if the cookie name begins with + # the magic __Host- prefix that means a cookie must be domain-locked. + def host_prefix? + @name.start_with?('__Host-') + end + # Tests if it is OK to send this cookie to a given `uri`. A # RuntimeError is raised if the cookie's domain is unknown. def valid_for_uri?(uri) @@ -629,6 +660,9 @@ def set_cookie_value if @secure string << "; Secure" end + if @same_site + string << "; SameSite=#{@same_site}" + end string end diff --git a/lib/http/cookie/scanner.rb b/lib/http/cookie/scanner.rb index 8dbd086..10eb96b 100644 --- a/lib/http/cookie/scanner.rb +++ b/lib/http/cookie/scanner.rb @@ -200,6 +200,8 @@ def scan_set_cookie when 'secure', 'httponly' # RFC 6265 5.2.5, 5.2.6 avalue = true + when 'samesite' + avalue = avalue.downcase end attrs[aname] = avalue end until eos? diff --git a/test/test_http_cookie.rb b/test/test_http_cookie.rb index 2454099..b08022a 100644 --- a/test/test_http_cookie.rb +++ b/test/test_http_cookie.rb @@ -810,6 +810,43 @@ def test_new_tld_domain_from_tld assert_equal true, tld_cookie2.acceptable? end + def test_cookie_prefixes + url = URI 'https://www.example.com/' + + # Must have secure flag set + tld_cookie1 = HTTP::Cookie.new(cookie_values(:name => '__Secure-Foo', :domain => '.example.com', :origin => url, :secure => false)) + assert_equal false, tld_cookie1.acceptable? + + url = URI 'http://www.example.com/' + + # Must have secure flag & domain set + tld_cookie1 = HTTP::Cookie.new(cookie_values(:name => '__Secure-Foo', :secure => true, :domain => '.example.com', :origin => url)) + assert_equal false, tld_cookie1.acceptable? + + url = URI 'https://www.example.com/' + + tld_cookie1 = HTTP::Cookie.new(cookie_values(:name => '__Secure-Foo', :secure => true, :domain => '.example.com', :origin => url)) + assert_equal true, tld_cookie1.acceptable? + + url = URI 'https://www.example.com/' + + # Path must be / + tld_cookie1 = HTTP::Cookie.new(cookie_values(:name => '__Host-Foo', :path => '/admin/', :secure => true, :domain => 'www.example.com', :origin => url)) + assert_equal false, tld_cookie1.acceptable? + + # Domain must not be set + url = URI 'https://www.example.com/' + + tld_cookie1 = HTTP::Cookie.new(cookie_values(:name => '__Host-Foo', :path => '', :secure => true, :domain => 'www.example.com', :origin => url)) + assert_equal false, tld_cookie1.acceptable? + + # Domain must not be set + url = URI 'https://www.example.com/' + + tld_cookie1 = HTTP::Cookie.new(cookie_values(:name => '__Host-Foo', :path => '', :secure => true, :domain => 'www.example.com', :origin => url)) + assert_equal false, tld_cookie1.acceptable? + end + def test_fall_back_rules_for_local_domains url = URI 'http://www.example.local' @@ -869,6 +906,29 @@ def test_path assert '/foo', cookie.path end + def test_same_site + cookie_str = 'a=b; SameSite=lax' + uri = URI.parse('http://example.com') + + cookie = HTTP::Cookie.parse(cookie_str, uri).first + assert_equal 'lax', cookie.same_site + + # SameSite=None requires the secure attribute to be set + cookie_str = 'a=b; SameSite=None' + + cookie = HTTP::Cookie.new({:name => 'a', :value => 'bar', :same_site => 'none', :secure => false, :origin => uri}) + assert_equal 'none', cookie.same_site + assert_equal false, cookie.acceptable? + + + # SameSite=None requires the secure attribute to be set + cookie_str = 'a=b; SameSite=Lax' + + cookie = HTTP::Cookie.parse(cookie_str, uri).first + assert_equal 'lax', cookie.same_site + assert_equal "a=b; SameSite=lax", cookie.set_cookie_value + end + def test_domain_nil cookie = HTTP::Cookie.new('a', 'b') assert_raises(RuntimeError) {