Skip to content

Commit

Permalink
Move the cookie store to use the MessageVerifier class.
Browse files Browse the repository at this point in the history
This removes support for ancient cookie-store generated cookies which were double escaped.
  • Loading branch information
NZKoz committed Nov 23, 2008
1 parent f9b1aa7 commit 04d2d04
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 29 deletions.
2 changes: 2 additions & 0 deletions actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*2.3.0 [Edge]*

* Remove support for old double-encoded cookies from the cookie store. These values haven't been generated since before 2.1.0, and any users who have visited the app in the intervening 6 months will have had their cookie upgraded. [Koz]

* Allow helpers directory to be overridden via ActionController::Base.helpers_dir #1424 [Sam Pohlenz]

* Remove deprecated ActionController::Base#assign_default_content_type_and_charset
Expand Down
32 changes: 14 additions & 18 deletions actionpack/lib/action_controller/session/cookie_store.rb
@@ -1,6 +1,5 @@
require 'cgi'
require 'cgi/session'
require 'openssl' # to generate the HMAC message digest

# This cookie-based session store is the Rails default. Sessions typically
# contain at most a user_id and flash message; both fit within the 4K cookie
Expand Down Expand Up @@ -121,32 +120,20 @@ def delete
write_cookie('value' => nil, 'expires' => 1.year.ago)
end

# Generate the HMAC keyed message digest. Uses SHA1 by default.
def generate_digest(data)
key = @secret.respond_to?(:call) ? @secret.call(@session) : @secret
OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new(@digest), key, data)
end

private
# Marshal a session hash into safe cookie data. Include an integrity hash.
def marshal(session)
data = ActiveSupport::Base64.encode64s(Marshal.dump(session))
"#{data}--#{generate_digest(data)}"
verifier.generate(session)
end

# Unmarshal cookie data to a hash and verify its integrity.
def unmarshal(cookie)
if cookie
data, digest = cookie.split('--')

# Do two checks to transparently support old double-escaped data.
unless digest == generate_digest(data) || digest == generate_digest(data = CGI.unescape(data))
delete
raise TamperedWithCookie
end

Marshal.load(ActiveSupport::Base64.decode64(data))
verifier.verify(cookie)
end
rescue ActiveSupport::MessageVerifier::InvalidSignature
delete
raise TamperedWithCookie
end

# Read the session data cookie.
Expand All @@ -164,4 +151,13 @@ def write_cookie(options)
def clear_old_cookie_value
@session.cgi.cookies[@cookie_options['name']].clear
end

def verifier
if @secret.respond_to?(:call)
key = @secret.call
else
key = @secret
end
ActiveSupport::MessageVerifier.new(key, @digest)
end
end
13 changes: 2 additions & 11 deletions actionpack/test/controller/session/cookie_store_test.rb
Expand Up @@ -45,8 +45,8 @@ def self.cookies
{ :empty => ['BAgw--0686dcaccc01040f4bd4f35fe160afe9bc04c330', {}],
:a_one => ['BAh7BiIGYWkG--5689059497d7f122a7119f171aef81dcfd807fec', { 'a' => 1 }],
:typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--9d20154623b9eeea05c62ab819be0e2483238759', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}],
:flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA==--bf9785a666d3c4ac09f7fe3353496b437546cfbf', { 'user_id' => 123, 'flash' => {} }],
:double_escaped => [CGI.escape('BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--bf9785a666d3c4ac09f7fe3353496b437546cfbf'), { 'user_id' => 123, 'flash' => {} }] }
:flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA==--bf9785a666d3c4ac09f7fe3353496b437546cfbf', { 'user_id' => 123, 'flash' => {} }]
}

end

Expand Down Expand Up @@ -105,15 +105,6 @@ def test_restore_deletes_tampered_cookies
end
end

def test_restores_double_encoded_cookies
set_cookie! cookie_value(:double_escaped)
new_session do |session|
session.dbman.restore
assert_equal session["user_id"], 123
assert_equal session["flash"], {}
end
end

def test_close_doesnt_write_cookie_if_data_is_blank
new_session do |session|
assert_no_cookies session
Expand Down

0 comments on commit 04d2d04

Please sign in to comment.