Changeset 8184

Timestamp:

11/21/07 21:31:45 (1 year ago)

Author:

nzkoz

Message:

Make sure that cookie sessions use a secret that is at least 30 chars in length. [Koz]

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/session/cookie_store.rb (modified) (2 diffs)
• trunk/actionpack/test/controller/session_fixation_test.rb (modified) (2 diffs)
• trunk/actionpack/test/controller/session/cookie_store_test.rb (modified) (2 diffs)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Make sure that cookie sessions use a secret that is at least 30 chars in length. [Koz]
-
-* Fixed that partial rendering should look at the type of the first render to determine its own type if no other clues are available (like when using text.plain.erb as the extension in AM) #10130 [java]

trunk/actionpack/lib/action_controller/session/cookie_store.rb

@@ -54 +54 @@
-
-    # The secret option is required.
-
-
-
+
-
-    # Keep the session and its secret on hand so we can read and write cookies.
@@ -77 +75 @@
-    options['no_hidden'] = true
-    options['no_cookies'] = true
+  end
+
+  # To prevent users from using something insecure like "Password" we make sure that the
+  # secret they've provided is at least 30 characters in length.
+  def ensure_secret_secure(secret)
+    # There's no way we can do this check if they've provided a proc for the
+    # secret.
+    return true if secret.is_a?(Proc)
+
+    if secret.blank?
+      raise ArgumentError, 'A secret is required to generate an integrity hash for cookie session data. Use config.action_controller.session = { :session_key => "_myapp_session", :secret => "some secret phrase" } in config/environment.rb'
+    end
+
+    if secret.length < 30
+      raise ArgumentError, "Secret should be something secure, like #{CGI::Session.generate_unique_id}.  The value you provided: [#{secret}]"
+    end
-  end
-

trunk/actionpack/test/controller/session_fixation_test.rb

@@ -1 +1 @@
-require File.dirname(__FILE__) + '/../abstract_unit'
+
-
-class SessionFixationTest < Test::Unit::TestCase
@@ -13 +14 @@
-
-  class TestController < ActionController::Base
-'secret'
+CGI::Session.generate_unique_id
-    session :cookie_only => false, :only => :allow_session_fixation
-

trunk/actionpack/test/controller/session/cookie_store_test.rb

@@ -4 +4 @@
-
-require 'stringio'
+
+
+class CGI::Session::CookieStore
+  def ensure_secret_secure_with_test_hax(secret)
+    if secret == CookieStoreTest.default_session_options['secret']
+      return true
+    else
+      ensure_secret_secure_without_test_hax(secret)
+    end
+  end
+  alias_method_chain :ensure_secret_secure, :test_hax
+end
+
-
-# Expose for tests.
@@ -50 +63 @@
-  end
-
+  def test_raises_argument_error_if_secret_is_probably_insecure
+    ["password", "secret", "12345678901234567890123456789"].each do |blank|
+      assert_raise(ArgumentError, blank.inspect) { new_session 'secret' => blank }
+    end
+  end
+
-  def test_reconfigures_session_to_omit_id_cookie_and_hidden_field
-    new_session do |session|