Cookie store: use OpenSSL::HMAC instead of basic hash. Introduce :secret block and :digest option.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6296 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 88 insertions, 35 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index e3632d4c86..b01eec5bab 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -21,7 +21,17 @@
 * Routing: better support for escaped values in route segments.  #7544 [Chris
 Roos]
 
-* Introduce a cookie-based session store as the Rails default. Sessions typically contain at most a user_id and flash message; both fit within the 4K cookie size limit. A secure hash is included with the cookie to ensure data integrity (a user cannot alter his user_id without knowing the secret key included in the hash). If you have more than 4K of session data or don't want your data to be visible to the user, pick another session store. Cookie-based sessions are dramatically faster than the alternatives.  [Jeremy Kemper]
+* Introduce a cookie-based session store as the Rails default. Sessions typically contain at most a user_id and flash message; both fit within the 4K cookie size limit. A secure message digest is included with the cookie to ensure data integrity (a user cannot alter his user_id without knowing the secret key included in the digest). If you have more than 4K of session data or don't want your data to be visible to the user, pick another session store.  Cookie-based sessions are dramatically faster than the alternatives.   [Jeremy Kemper]
+
+  Example config/environment.rb:
+    # Use an application-wide secret key and the default SHA1 message digest.
+    config.action_controller.session = { :secret => "can't touch this" }
+
+    # Store a secret key per user and employ a stronger message digest.
+    config.action_controller.session = {
+      :digest => 'SHA512',
+      :secret => Proc.new { User.current.secret_key }
+    }
 
 * Added .erb and .builder as preferred aliases to the now deprecated .rhtml and .rxml extensions [Chad Fowler]. This is done to separate the renderer from the mime type. .erb templates are often used to render emails, atom, csv, whatever. So labeling them .rhtml doesn't make too much sense. The same goes for .rxml, which can be used to build everything from HTML to Atom to whatever. .rhtml and .rxml will continue to work until Rails 3.0, though. So this is a slow phasing out. All generators and examples will start using the new aliases, though.
 
diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb
index e65ff6b262..d232dd448c 100644
--- a/actionpack/lib/action_controller/session/cookie_store.rb
+++ b/actionpack/lib/action_controller/session/cookie_store.rb
@@ -1,24 +1,37 @@
 require 'cgi'
 require 'cgi/session'
-require 'base64'        # to make marshaled data HTTP header friendly
-require 'digest/sha2'   # to generate the data integrity hash
+require 'base64'        # to convert Marshal.dump to ASCII
+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
 # size limit. Cookie-based sessions are dramatically faster than the
 # alternatives.
 #
-# A secure hash is included with the cookie to ensure data integrity:
-# a user cannot alter his user_id without knowing the secret key included in
-# the hash. New apps are generated with a session :secret option in
-# Application Controller. Set your own for old apps you're upgrading.
-# Note that changing the secret invalidates all existing sessions!
-#
 # If you have more than 4K of session data or don't want your data to be
 # visible to the user, pick another session store.
 #
 # CookieOverflow is raised if you attempt to store more than 4K of data.
 # TamperedWithCookie is raised if the data integrity check fails.
+#
+# A message digest is included with the cookie to ensure data integrity:
+# a user cannot alter his user_id without knowing the secret key included in
+# the hash. New apps are generated with a pregenerated secret in
+# config/environment.rb. Set your own for old apps you're upgrading.
+#
+# Session options:
+#   :secret   An application-wide key string or block returning a string
+#             called per generated digest. The block is called with the
+#             CGI::Session instance as an argument.
+#
+#             Example:  :secret => '449fe2e7daee471bffae2fd8dc02313d'
+#                       :secret => Proc.new { User.current_user.secret_key }
+#
+#   :digest   The message digest algorithm used to verify session integrity
+#             defaults to 'SHA1' but may be any digest provided by OpenSSL,
+#             such as 'MD5', 'RIPEMD160', 'SHA256', etc.
+#
+# Note that changing digest or secret invalidates all existing sessions!
 class CGI::Session::CookieStore
   # Cookies can typically store 4096 bytes.
   MAX = 4096
@@ -33,12 +46,15 @@ class CGI::Session::CookieStore
   def initialize(session, options = {})
     # The secret option is required.
     if options['secret'].blank?
-      raise ArgumentError, 'A secret is required to generate an integrity hash for cookie session data. Use session :secret => "some secret phrase" in ApplicationController.'
+      raise ArgumentError, 'A secret is required to generate an integrity hash for cookie session data. Use config.action_controller.session = { :secret => "some secret phrase" } in config/environment.rb'
     end
 
     # Keep the session and its secret on hand so we can read and write cookies.
     @session, @secret = session, options['secret']
 
+    # Message digest defaults to SHA1.
+    @digest = options['digest'] || 'SHA1'
+
     # Default cookie options derived from session settings.
     @cookie_options = {
       'name'    => options['session_key'],
@@ -97,11 +113,10 @@ class CGI::Session::CookieStore
       end
     end
 
-    # Generate the inline SHA512 message digest. Larger (128 bytes) than SHA256
-    # (64 bytes) or RMD160 (40 bytes), but small relative to the 4096 byte
-    # max cookie size.
+    # Generate the HMAC keyed message digest. Uses SHA1 by default.
     def generate_digest(data)
-      Digest::SHA512.hexdigest "#{data}#{@secret}"
+      key = @secret.respond_to?(:call) ? @secret.call(@session) : @secret
+      OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new(@digest), key, data)
     end
 
     # Read the session data cookie.
diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb
index 8c1cb7a986..6d98821cfd 100755
--- a/actionpack/test/controller/session/cookie_store_test.rb
+++ b/actionpack/test/controller/session/cookie_store_test.rb
@@ -18,19 +18,19 @@ class CGI
 end
 
 class CookieStoreTest < Test::Unit::TestCase
-  DefaultSessionOptions = {
-    'database_manager' => CGI::Session::CookieStore,
-    'session_key' => '_myapp_session',
-    'secret' => 'Keep it secret; keep it safe.',
-    'no_cookies' => true,
-    'no_hidden' => true
-  }
+  def self.default_session_options
+    { 'database_manager' => CGI::Session::CookieStore,
+      'session_key' => '_myapp_session',
+      'secret' => 'Keep it secret; keep it safe.',
+      'no_cookies' => true,
+      'no_hidden' => true }
+  end
 
-  module Cookies
-    EMPTY = ['BAh7AA%3D%3D--fda6e506d1cc14a1d8e97fd3f5abf77e756ff2d987b069e5f9b0fbadb62ca6fb3cf523e8dfc61464dd98d7bd2d675e0713ce54226f428e521b4c5d21d2389eae', {}]
-    A_ONE = ['BAh7BiIGYWkG--8dfd099b297a60f6742933b1217b81e91c50237eedd8b25f3ce47b86394e14de3b17128225ba984e7d8660f7777e33979b8d98091dc87400be8c54ebbfdbe599', { 'a' => 1 }]
-    TYPICAL = ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--251fa4706464e87bcb90c76a27a1dee2410ff81a1ba9903f9760263ad44e739a42d0a5d5d7229087ddb4b3e1d6b956a6c4f6a2f8dcb5a5b281a342fed12d38c0', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}]
-    FLASHED = ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--a574ffd23d744c363f94a75b449d02dd619fd9409978ea0a2797c98dc638bff9fe0f9cacb2106b1610f0731b386416bcca6e11e031b7885719ba8c956dfd6f2c', { 'user_id' => 123, 'flash' => {} }]
+  def self.cookies
+    { :empty => ['BAgw--0686dcaccc01040f4bd4f35fe160afe9bc04c330', {}],
+      :a_one => ['BAh7BiIGYWkG--5689059497d7f122a7119f171aef81dcfd807fec', { 'a' => 1 }],
+      :typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--9d20154623b9eeea05c62ab819be0e2483238759', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}],
+      :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--bf9785a666d3c4ac09f7fe3353496b437546cfbf', { 'user_id' => 123, 'flash' => {} }] }
   end
 
   def setup
@@ -59,7 +59,7 @@ class CookieStoreTest < Test::Unit::TestCase
   end
 
   def test_restore_unmarshals_good_cookies
-    [Cookies::EMPTY, Cookies::A_ONE, Cookies::TYPICAL].each do |value, expected|
+    cookies(:empty, :a_one, :typical).each do |value, expected|
       set_cookie! value
       new_session do |session|
         assert_nil session['lazy loads the data hash']
@@ -85,7 +85,7 @@ class CookieStoreTest < Test::Unit::TestCase
   end
 
   def test_close_doesnt_write_cookie_if_data_is_unchanged
-    set_cookie! Cookies::TYPICAL.first
+    set_cookie! cookie_value(:typical)
     new_session do |session|
       assert_no_cookies session
       session['user_id'] = session['user_id']
@@ -95,7 +95,7 @@ class CookieStoreTest < Test::Unit::TestCase
   end
 
   def test_close_raises_when_data_overflows
-    set_cookie! Cookies::EMPTY.first
+    set_cookie! cookie_value(:empty)
     new_session do |session|
       session['overflow'] = 'bye!' * 1024
       assert_raise(CGI::Session::CookieStore::CookieOverflow) { session.close }
@@ -104,7 +104,7 @@ class CookieStoreTest < Test::Unit::TestCase
   end
 
   def test_close_marshals_and_writes_cookie
-    set_cookie! Cookies::TYPICAL.first
+    set_cookie! cookie_value(:typical)
     new_session do |session|
       assert_no_cookies session
       session['flash'] = {}
@@ -112,14 +112,12 @@ class CookieStoreTest < Test::Unit::TestCase
       session.close
       assert_equal 1, session.cgi.output_cookies.size
       cookie = session.cgi.output_cookies.first
-      assert_equal ['_myapp_session', [Cookies::FLASHED.first]],
-                   [cookie.name, cookie.value]
-      assert_cookie cookie, Cookies::FLASHED.first
+      assert_cookie cookie, cookie_value(:flashed)
     end
   end
 
   def test_delete_writes_expired_empty_cookie_and_sets_data_to_nil
-    set_cookie! Cookies::TYPICAL.first
+    set_cookie! cookie_value(:typical)
     new_session do |session|
       assert_no_cookies session
       session.delete
@@ -148,6 +146,15 @@ class CookieStoreTest < Test::Unit::TestCase
       assert_equal expires, cookie.expires ? cookie.expires.to_date : cookie.expires, message
     end
 
+
+    def cookies(*which)
+      self.class.cookies.values_at(*which)
+    end
+
+    def cookie_value(which)
+      self.class.cookies[which].first
+    end
+
     def set_cookie!(value)
       ENV['HTTP_COOKIE'] = "_myapp_session=#{value}"
     end
@@ -157,7 +164,7 @@ class CookieStoreTest < Test::Unit::TestCase
         assert_nil cgi.output_hidden, "Output hidden params should be empty: #{cgi.output_hidden.inspect}"
         assert_nil cgi.output_cookies, "Output cookies should be empty: #{cgi.output_cookies.inspect}"
 
-        @options = DefaultSessionOptions.merge(options)
+        @options = self.class.default_session_options.merge(options)
         session = CGI::Session.new(cgi, @options)
 
         assert_nil cgi.output_hidden, "Output hidden params should be empty: #{cgi.output_hidden.inspect}"
@@ -179,3 +186,24 @@ class CookieStoreTest < Test::Unit::TestCase
       $stdin = old_stdin
     end
 end
+
+
+class CookieStoreWithBlockAsSecretTest < CookieStoreTest
+  def self.default_session_options
+    CookieStoreTest.default_session_options.merge 'secret' => Proc.new { 'Keep it secret; keep it safe.' }
+  end
+end
+
+
+class CookieStoreWithMD5DigestTest < CookieStoreTest
+  def self.default_session_options
+    CookieStoreTest.default_session_options.merge 'digest' => 'MD5'
+  end
+
+  def self.cookies
+    { :empty => ['BAgw--0415cc0be9579b14afc22ee2d341aa21', {}],
+      :a_one => ['BAh7BiIGYWkG--5a0ed962089cc6600ff44168a5d59bc8', { 'a' => 1 }],
+      :typical => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7BiILbm90aWNlIgxIZXkgbm93--f426763f6ef435b3738b493600db8d64', { 'user_id' => 123, 'flash' => { 'notice' => 'Hey now' }}],
+      :flashed => ['BAh7ByIMdXNlcl9pZGkBeyIKZmxhc2h7AA%3D%3D--0af9156650dab044a53a91a4ddec2c51', { 'user_id' => 123, 'flash' => {} }] }
+  end
+end