aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Draper <matthew@trebex.net>2016-07-01 01:01:45 +0930
committerMatthew Draper <matthew@trebex.net>2016-07-01 01:01:45 +0930
commitea7fee03f78dfeac44b3a80f6fd61bf314b5a369 (patch)
treecdd1ee75794354e4e422d58bc3e4d486df73ecd4
parent21f72a4d3411578e58908ba67a9f20cdb9a103e3 (diff)
downloadrails-ea7fee03f78dfeac44b3a80f6fd61bf314b5a369.tar.gz
rails-ea7fee03f78dfeac44b3a80f6fd61bf314b5a369.tar.bz2
rails-ea7fee03f78dfeac44b3a80f6fd61bf314b5a369.zip
Partially revert #25192
KeyGenerator is used in other contexts, and we cannot change its output... even if it does accidentally default to generating excess key material for our primary internal usage.
-rw-r--r--activesupport/lib/active_support/key_generator.rb12
-rw-r--r--activesupport/test/key_generator_test.rb17
2 files changed, 22 insertions, 7 deletions
diff --git a/activesupport/lib/active_support/key_generator.rb b/activesupport/lib/active_support/key_generator.rb
index 7eafbb571f..0f0e931c06 100644
--- a/activesupport/lib/active_support/key_generator.rb
+++ b/activesupport/lib/active_support/key_generator.rb
@@ -15,8 +15,9 @@ module ActiveSupport
end
# Returns a derived key suitable for use. The default key_size is chosen
- # to be compatible with the acceptable key length of aes-256-cbc, the default cipher.
- def generate_key(salt, key_size=32)
+ # to be compatible with the default settings of ActiveSupport::MessageVerifier.
+ # i.e. OpenSSL::Digest::SHA1#block_length
+ def generate_key(salt, key_size=64)
OpenSSL::PKCS5.pbkdf2_hmac_sha1(@secret, salt, @iterations, key_size)
end
end
@@ -30,10 +31,9 @@ module ActiveSupport
@cache_keys = Concurrent::Map.new
end
- # Returns a derived key suitable for use. The default key_size is chosen
- # to be compatible with the acceptable key length of aes-256-cbc, the default cipher.
- def generate_key(salt, key_size=32)
- @cache_keys["#{salt}#{key_size}"] ||= @key_generator.generate_key(salt, key_size)
+ # Returns a derived key suitable for use.
+ def generate_key(*args)
+ @cache_keys[args.join] ||= @key_generator.generate_key(*args)
end
end
diff --git a/activesupport/test/key_generator_test.rb b/activesupport/test/key_generator_test.rb
index 6cf72f1fec..b60077460e 100644
--- a/activesupport/test/key_generator_test.rb
+++ b/activesupport/test/key_generator_test.rb
@@ -19,7 +19,7 @@ class KeyGeneratorTest < ActiveSupport::TestCase
test "Generating a key of the default length" do
derived_key = @generator.generate_key("some_salt")
assert_kind_of String, derived_key
- assert_equal OpenSSL::Cipher.new('aes-256-cbc').key_len, derived_key.length, "Should have generated a key of the default size"
+ assert_equal 64, derived_key.length, "Should have generated a key of the default size"
end
test "Generating a key of an alternative length" do
@@ -27,6 +27,21 @@ class KeyGeneratorTest < ActiveSupport::TestCase
assert_kind_of String, derived_key
assert_equal 32, derived_key.length, "Should have generated a key of the right size"
end
+
+ test "Expected results" do
+ # For any given set of inputs, this method must continue to return
+ # the same output: if it changes, any existing values relying on a
+ # key would break.
+
+ expected = "b129376f68f1ecae788d7433310249d65ceec090ecacd4c872a3a9e9ec78e055739be5cc6956345d5ae38e7e1daa66f1de587dc8da2bf9e8b965af4b3918a122"
+ assert_equal expected, ActiveSupport::KeyGenerator.new("0" * 64).generate_key("some_salt").unpack('H*').first
+
+ expected = "b129376f68f1ecae788d7433310249d65ceec090ecacd4c872a3a9e9ec78e055"
+ assert_equal expected, ActiveSupport::KeyGenerator.new("0" * 64).generate_key("some_salt", 32).unpack('H*').first
+
+ expected = "cbea7f7f47df705967dc508f4e446fd99e7797b1d70011c6899cd39bbe62907b8508337d678505a7dc8184e037f1003ba3d19fc5d829454668e91d2518692eae"
+ assert_equal expected, ActiveSupport::KeyGenerator.new("0" * 64, iterations: 2).generate_key("some_salt").unpack('H*').first
+ end
end
class CachingKeyGeneratorTest < ActiveSupport::TestCase