Move the cookie store to use the MessageVerifier class.

This removes support for ancient cookie-store generated cookies which were double escaped.

3 files changed, 18 insertions, 29 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 59a4bbea59..9581442208 100644
--- a/actionpack/CHANGELOG
+++ b/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
diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb
index f2fb200950..ea0ea4f841 100644
--- a/actionpack/lib/action_controller/session/cookie_store.rb
+++ b/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
@@ -121,32 +120,20 @@ class CGI::Session::CookieStore
     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.
@@ -164,4 +151,13 @@ class CGI::Session::CookieStore
     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
diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb
index 30422314a1..52c1f7559c 100644
--- a/actionpack/test/controller/session/cookie_store_test.rb
+++ b/actionpack/test/controller/session/cookie_store_test.rb
@@ -45,8 +45,8 @@ class CookieStoreTest < Test::Unit::TestCase
     { :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
 
@@ -105,15 +105,6 @@ class CookieStoreTest < Test::Unit::TestCase
     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