Skip to content

Commit

Permalink
Remove secret_token rack env and cookie upgrade code
Browse files Browse the repository at this point in the history
Now that secret_token was removed all this code is now dead.
  • Loading branch information
rafaelfranca committed Jan 17, 2019
1 parent 46ac5fe commit 1a6a3e0
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 261 deletions.
35 changes: 1 addition & 34 deletions actionpack/lib/action_dispatch/middleware/cookies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ def signed_cookie_digest
get_header Cookies::SIGNED_COOKIE_DIGEST
end

def secret_token
get_header Cookies::SECRET_TOKEN
end

def secret_key_base
get_header Cookies::SECRET_KEY_BASE
end
Expand Down Expand Up @@ -181,7 +177,6 @@ class Cookies
USE_AUTHENTICATED_COOKIE_ENCRYPTION = "action_dispatch.use_authenticated_cookie_encryption"
ENCRYPTED_COOKIE_CIPHER = "action_dispatch.encrypted_cookie_cipher"
SIGNED_COOKIE_DIGEST = "action_dispatch.signed_cookie_digest"
SECRET_TOKEN = "action_dispatch.secret_token"
SECRET_KEY_BASE = "action_dispatch.secret_key_base"
COOKIES_SERIALIZER = "action_dispatch.cookies_serializer"
COOKIES_DIGEST = "action_dispatch.cookies_digest"
Expand Down Expand Up @@ -215,9 +210,6 @@ def permanent
# the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed
# cookie was tampered with by the user (or a 3rd party), +nil+ will be returned.
#
# If +secret_key_base+ and +secrets.secret_token+ (deprecated) are both set,
# legacy cookies signed with the old key generator will be transparently upgraded.
#
# This jar requires that you set a suitable secret for the verification on your app's +secret_key_base+.
#
# Example:
Expand All @@ -233,9 +225,6 @@ def signed
# Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read.
# If the cookie was tampered with by the user (or a 3rd party), +nil+ will be returned.
#
# If +secret_key_base+ and +secrets.secret_token+ (deprecated) are both set,
# legacy cookies signed with the old key generator will be transparently upgraded.
#
# If +config.action_dispatch.encrypted_cookie_salt+ and +config.action_dispatch.encrypted_signed_cookie_salt+
# are both set, legacy cookies encrypted with HMAC AES-256-CBC will be transparently upgraded.
#
Expand Down Expand Up @@ -264,10 +253,6 @@ def signed_or_encrypted

private

def upgrade_legacy_signed_cookies?
request.secret_token.present? && request.secret_key_base.present?
end

def upgrade_legacy_hmac_aes_cbc_cookies?
request.secret_key_base.present? &&
request.encrypted_signed_cookie_salt.present? &&
Expand Down Expand Up @@ -592,10 +577,6 @@ def initialize(parent_jar)
request.cookies_rotations.signed.each do |*secrets, **options|
@verifier.rotate(*secrets, serializer: SERIALIZER, **options)
end

if upgrade_legacy_signed_cookies?
@verifier.rotate request.secret_token, serializer: SERIALIZER
end
end

private
Expand Down Expand Up @@ -640,10 +621,6 @@ def initialize(parent_jar)

@encryptor.rotate(secret, sign_secret, cipher: legacy_cipher, digest: digest, serializer: SERIALIZER)
end

if upgrade_legacy_signed_cookies?
@legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, digest: digest, serializer: SERIALIZER)
end
end

private
Expand All @@ -652,24 +629,14 @@ def parse(name, encrypted_message, purpose: nil)
@encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate, purpose: purpose)
end
rescue ActiveSupport::MessageEncryptor::InvalidMessage, ActiveSupport::MessageVerifier::InvalidSignature
parse_legacy_signed_message(name, encrypted_message)
nil
end

def commit(name, options)
options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value]), cookie_metadata(name, options))

raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE
end

def parse_legacy_signed_message(name, legacy_signed_message)
if defined?(@legacy_verifier)
deserialize(name) do |rotate|
rotate.call

@legacy_verifier.verified(legacy_signed_message)
end
end
end
end

def initialize(app)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ module Session
# The cookie jar used for storage is automatically configured to be the
# best possible option given your application's configuration.
#
# If you only have secret_token set, your cookies will be signed, but
# not encrypted. This means a user cannot alter their +user_id+ without
# knowing your app's secret key, but can easily read their +user_id+. This
# was the default for Rails 3 apps.
#
# Your cookies will be encrypted using your apps secret_key_base. This
# goes a step further than signed cookies in that encrypted cookies cannot
# be altered or read by users. This is the default starting in Rails 4.
Expand Down
8 changes: 6 additions & 2 deletions actionpack/test/controller/flash_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,11 @@ def test_does_not_add_flash_type_to_parent_class

class FlashIntegrationTest < ActionDispatch::IntegrationTest
SessionKey = "_myapp_session"
Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33")
Rotations = ActiveSupport::Messages::RotationConfiguration.new
Generator = ActiveSupport::CachingKeyGenerator.new(
ActiveSupport::KeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33", iterations: 1000)
)
Rotations = ActiveSupport::Messages::RotationConfiguration.new
SIGNED_COOKIE_SALT = "signed cookie"

class TestController < ActionController::Base
add_flash_types :bar
Expand Down Expand Up @@ -365,6 +368,7 @@ def get(path, *args)
args[0][:env] ||= {}
args[0][:env]["action_dispatch.key_generator"] ||= Generator
args[0][:env]["action_dispatch.cookies_rotations"] = Rotations
args[0][:env]["action_dispatch.signed_cookie_salt"] = SIGNED_COOKIE_SALT
super(path, *args)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ def authenticate_with_request
setup do
# Used as secret in generating nonce to prevent tampering of timestamp
@secret = "4fb45da9e4ab4ddeb7580d6a35503d99"
@request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new(@secret)
@request.env["action_dispatch.key_generator"] = ActiveSupport::CachingKeyGenerator.new(
ActiveSupport::KeyGenerator.new(@secret)
)
@request.env["action_dispatch.http_auth_salt"] = "http authentication"
end

teardown do
Expand Down
185 changes: 0 additions & 185 deletions actionpack/test/dispatch/cookies_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -525,21 +525,6 @@ def test_signed_cookie_rotating_secret_and_digest
assert_equal 45, verifier.verify(@response.cookies["user_id"])
end

def test_signed_cookie_with_legacy_secret_scheme
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

old_message = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", digest: "SHA1", serializer: Marshal).generate(45)

@request.headers["Cookie"] = "user_id=#{old_message}"
get :get_signed_cookie
assert_equal 45, @controller.send(:cookies).signed[:user_id]

key_generator = @request.env["action_dispatch.key_generator"]
secret = key_generator.generate_key("signed cookie")
verifier = ActiveSupport::MessageVerifier.new(secret, digest: "SHA1", serializer: Marshal)
assert_equal 45, verifier.verify(@response.cookies["user_id"])
end

def test_tampered_with_signed_cookie
key_generator = @request.env["action_dispatch.key_generator"]
secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"])
Expand Down Expand Up @@ -759,175 +744,7 @@ def test_cookie_jar_mutated_by_request_persists_on_future_requests
assert_equal ["user_name", "user_id"], @request.cookie_jar.instance_variable_get(:@cookies).keys
end

def test_raises_argument_error_if_missing_secret
assert_raise(ArgumentError, nil.inspect) {
@request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new(nil)
get :set_signed_cookie
}

assert_raise(ArgumentError, "".inspect) {
@request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("")
get :set_signed_cookie
}
end

def test_raises_argument_error_if_secret_is_probably_insecure
assert_raise(ArgumentError, "password".inspect) {
@request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("password")
get :set_signed_cookie
}

assert_raise(ArgumentError, "secret".inspect) {
@request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("secret")
get :set_signed_cookie
}

assert_raise(ArgumentError, "12345678901234567890123456789".inspect) {
@request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("12345678901234567890123456789")
get :set_signed_cookie
}
end

def test_legacy_signed_cookie_is_read_and_transparently_upgraded_by_signed_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate(45)

@request.headers["Cookie"] = "user_id=#{legacy_value}"
get :get_signed_cookie

assert_equal 45, @controller.send(:cookies).signed[:user_id]

key_generator = @request.env["action_dispatch.key_generator"]
secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"])
verifier = ActiveSupport::MessageVerifier.new(secret)
assert_equal 45, verifier.verify(@response.cookies["user_id"])
end

def test_legacy_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate("bar")

@request.headers["Cookie"] = "foo=#{legacy_value}"
get :get_encrypted_cookie

assert_equal "bar", @controller.send(:cookies).encrypted[:foo]

secret = @request.env["action_dispatch.key_generator"].generate_key(@request.env["action_dispatch.authenticated_encrypted_cookie_salt"], 32)
encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal)
assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"])
end

def test_legacy_json_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.cookies_serializer"] = :json
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate(45)

@request.headers["Cookie"] = "user_id=#{legacy_value}"
get :get_signed_cookie

assert_equal 45, @controller.send(:cookies).signed[:user_id]

key_generator = @request.env["action_dispatch.key_generator"]
secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"])
verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON)
assert_equal 45, verifier.verify(@response.cookies["user_id"])
end

def test_legacy_json_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_json_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.cookies_serializer"] = :json
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate("bar")

@request.headers["Cookie"] = "foo=#{legacy_value}"
get :get_encrypted_cookie

assert_equal "bar", @controller.send(:cookies).encrypted[:foo]

cipher = "aes-256-gcm"
salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"]
secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)]
encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: JSON)
assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"])
end

def test_legacy_json_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_hybrid_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.cookies_serializer"] = :hybrid
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate(45)

@request.headers["Cookie"] = "user_id=#{legacy_value}"
get :get_signed_cookie

assert_equal 45, @controller.send(:cookies).signed[:user_id]

key_generator = @request.env["action_dispatch.key_generator"]
secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"])
verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON)
assert_equal 45, verifier.verify(@response.cookies["user_id"])
end

def test_legacy_json_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_hybrid_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.cookies_serializer"] = :hybrid
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate("bar")

@request.headers["Cookie"] = "foo=#{legacy_value}"
get :get_encrypted_cookie

assert_equal "bar", @controller.send(:cookies).encrypted[:foo]

salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"]
secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")]
encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON)
assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"])
end

def test_legacy_marshal_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_hybrid_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.cookies_serializer"] = :hybrid
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate(45)

@request.headers["Cookie"] = "user_id=#{legacy_value}"
get :get_signed_cookie

assert_equal 45, @controller.send(:cookies).signed[:user_id]

key_generator = @request.env["action_dispatch.key_generator"]
secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"])
verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON)
assert_equal 45, verifier.verify(@response.cookies["user_id"])
end

def test_legacy_marshal_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_hybrid_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
@request.env["action_dispatch.cookies_serializer"] = :hybrid

@request.env["action_dispatch.use_authenticated_cookie_encryption"] = true
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
@request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"

legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate("bar")

@request.headers["Cookie"] = "foo=#{legacy_value}"
get :get_encrypted_cookie

assert_equal "bar", @controller.send(:cookies).encrypted[:foo]

salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"]
secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")]
encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON)
assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"])
end

def test_legacy_signed_cookie_is_treated_as_nil_by_signed_cookie_jar_if_tampered
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

@request.headers["Cookie"] = "user_id=45"
get :get_signed_cookie

Expand All @@ -936,8 +753,6 @@ def test_legacy_signed_cookie_is_treated_as_nil_by_signed_cookie_jar_if_tampered
end

def test_legacy_signed_cookie_is_treated_as_nil_by_encrypted_cookie_jar_if_tampered
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"

@request.headers["Cookie"] = "foo=baz"
get :get_encrypted_cookie

Expand Down
10 changes: 8 additions & 2 deletions actionpack/test/dispatch/routing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4991,8 +4991,12 @@ def test_paths_with_partial_dynamic_segments_are_recognised

class FlashRedirectTest < ActionDispatch::IntegrationTest
SessionKey = "_myapp_session"
Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33")
Rotations = ActiveSupport::Messages::RotationConfiguration.new
Generator = ActiveSupport::CachingKeyGenerator.new(
ActiveSupport::KeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33", iterations: 1000)
)
Rotations = ActiveSupport::Messages::RotationConfiguration.new
SIGNED_COOKIE_SALT = "signed cookie"
ENCRYPTED_SIGNED_COOKIE_SALT = "sigend encrypted cookie"

class KeyGeneratorMiddleware
def initialize(app)
Expand All @@ -5002,6 +5006,8 @@ def initialize(app)
def call(env)
env["action_dispatch.key_generator"] ||= Generator
env["action_dispatch.cookies_rotations"] ||= Rotations
env["action_dispatch.signed_cookie_salt"] = SIGNED_COOKIE_SALT
env["action_dispatch.encrypted_signed_cookie_salt"] = ENCRYPTED_SIGNED_COOKIE_SALT

@app.call(env)
end
Expand Down
Loading

0 comments on commit 1a6a3e0

Please sign in to comment.