Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use authenticated cookie options to disable embedding of the expiry information in the cookies #32141

Merged

Conversation

rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Feb 28, 2018

We were generating the cookie value with the expiration embedded what makes the value impossible to be read by a Rails 5.1 application.

This option is only useful if you need to share your cookies between a Rails 5.1 and a Rails 5.2 application, or if you are still validating the deploy and wants to eventually rollback.

@rails-bot
Copy link

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against 5-2-stable. Please double check that you specified the right target!

# Embeds the experity of a cookie inside its value.
# This makes the cookie value incompatible with Rails < 5.2 so set to false if your
# cookie needs to be read by a Rails < 5.2 application.
# Rails.application.config.action_dispatch.embed_expirity_in_the_cookie_value = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think expirity is a real word 😬

Maybe use_embedded_cookie_expiration?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go with embed_expiry_in_signed_and_encrypted_cookies. Or use_authenticated_encryption_and_embed_expiry_for_cookies if we're unifying with use_authenticated_cookie_encryption.


To improve security, Rails now embeds the expirity information also in encrypted or signed cookies value.

This new embded informaiton make those cookies incompatible with versions of Rails older than 5.2.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

information

This new embded informaiton make those cookies incompatible with versions of Rails older than 5.2.

If you require your cookies to be read by 5.1 and older, set
`Rails.application.config.action_dispatch.embed_expirity_in_the_cookie_value` to `false`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to suggest that it's useful for when you're validating your 5.2 deploy and allows you to rollback.

@@ -14,6 +14,11 @@
# Existing cookies will be converted on read then written with the new scheme.
# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're here we may as well say that this also breaks cookie backwards compatibility with 5.1.

We could also consider unifying the 2 options into one. Or just read use_authenticated_cookie_encryption as embed the expiry.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I'll use use_authenticated_cookie_encryption for both configs.

@@ -14,6 +14,11 @@
# Existing cookies will be converted on read then written with the new scheme.
# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true

# Embeds the experity of a cookie inside its value.
# This makes the cookie value incompatible with Rails < 5.2 so set to false if your
# cookie needs to be read by a Rails < 5.2 application.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say:

# Embed cookie expiry in signed or encrypted cookies for increased security.
# This option is not backwards compatible with earlier Rails versions.
# It's best enabled when your entire app is migrated and stable on 5.2.

# Embeds the experity of a cookie inside its value.
# This makes the cookie value incompatible with Rails < 5.2 so set to false if your
# cookie needs to be read by a Rails < 5.2 application.
# Rails.application.config.action_dispatch.embed_expirity_in_the_cookie_value = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go with embed_expiry_in_signed_and_encrypted_cookies. Or use_authenticated_encryption_and_embed_expiry_for_cookies if we're unifying with use_authenticated_cookie_encryption.

@@ -22,6 +22,7 @@ class Railtie < Rails::Railtie # :nodoc:
config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie"
config.action_dispatch.use_authenticated_cookie_encryption = false
config.action_dispatch.perform_deep_munge = true
config.action_dispatch.embed_expirity_in_the_cookie_value = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be set to true in load_defaults instead? Otherwise upgrading apps will still break because of the commented out value in new_framework_defaults.

@rafaelfranca rafaelfranca force-pushed the rm-configure-embedded-expirity branch from 02f0e76 to aba8fc8 Compare March 6, 2018 18:40
@rafaelfranca rafaelfranca changed the title Add option to disable embedding of the expirity information in the cookies Use authenticated cookie options to disable embedding of the expiration in the cookies Mar 6, 2018
Copy link
Contributor

@kaspth kaspth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation nits. We can always handle those separately if you want.

@@ -502,6 +502,10 @@ Defaults to `'signed cookie'`.
* `config.action_dispatch.cookies_rotations` allows rotating
secrets, ciphers, and digests for encrypted and signed cookies.

* `config.action_dispatch.use_authenticated_cookie_encryption` controls encrypted cookies to use AES-256-GC
authenticated encryption and if signed and excrypted cookies are going to embed the expirition information
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expiry

@@ -76,6 +76,17 @@ Rails 5.2 adds bootsnap gem in the [newly generated app's Gemfile](https://githu
The `app:update` task sets it up in `boot.rb`. If you want to use it, then add it in the Gemfile,
otherwise change the `boot.rb` to not use bootsnap.

### Expirity is now embedded in the cookies values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expiryin signed or encrypted cookies. It's only those cookies that get it embedded.

# Existing cookies will be converted on read then written with the new scheme.
# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true

# Rails.application.config.action_dispatch.embed_expirity_in_the_cookie_value = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment line isn't needed.


To improve security, Rails now embeds the expirity information also in encrypted or signed cookies value.

This new embded information make those cookies incompatible with versions of Rails older than 5.2.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

embed

@@ -76,6 +76,17 @@ Rails 5.2 adds bootsnap gem in the [newly generated app's Gemfile](https://githu
The `app:update` task sets it up in `boot.rb`. If you want to use it, then add it in the Gemfile,
otherwise change the `boot.rb` to not use bootsnap.

### Expirity is now embedded in the cookies values

To improve security, Rails now embeds the expirity information also in encrypted or signed cookies value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expiry


This new embded information make those cookies incompatible with versions of Rails older than 5.2.

If you require your cookies to be read by 5.1 and older, or you are still validating your 5.2 deploy and wants
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want

…on in the cookies

We were generating the cookie value with the expiry information embedded what
makes the value impossible to be read by a Rails 5.1 application.

This option is only useful if you need to share your cookies between
a Rails 5.1 and a Rails 5.2 application, or if you are still validating
the deploy and wants to eventually rollback.
@rafaelfranca rafaelfranca force-pushed the rm-configure-embedded-expirity branch from aba8fc8 to b25fcbc Compare March 6, 2018 18:52
@rafaelfranca rafaelfranca changed the title Use authenticated cookie options to disable embedding of the expiration in the cookies Use authenticated cookie options to disable embedding of the expiry information in the cookies Mar 6, 2018
@rafaelfranca rafaelfranca merged commit 327bac7 into rails:5-2-stable Mar 6, 2018
@rafaelfranca rafaelfranca deleted the rm-configure-embedded-expirity branch March 6, 2018 20:19
@rafaelfranca rafaelfranca restored the rm-configure-embedded-expirity branch September 11, 2020 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants