Make WordPress Core

Opened 9 years ago

Last modified 6 months ago

#37000 new enhancement

Support for the SameSite cookie attribute

Reported by: johnbillion's profile johnbillion Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch dev-feedback needs-dev-note has-unit-tests
Focuses: administration Cc:

Description

IETF's Same-site Cookies draft was shipped in Chrome 51 and Opera 39.

The SameSite cookie attribute instructs a browser not to send that cookie with cross-origin third-party requests (such as iframes, embedded images, and Ajax requests). This effectively mitigates CSRF attacks as, for example, the user will not be authenticated for a given third party URL that's being used in a CSRF attack.

More information on the SameSite attribute can be found here: http://www.sjoerdlangkemper.nl/2016/04/14/preventing-csrf-with-samesite-cookie-attribute/

We should investigate whether setting the SameSite=lax attribute is of benefit to the auth and/or logged_in cookies in WordPress, and if so consider implementing it once the draft becomes an RFC.

PHP uses the setcookie() wrapper for setting cookies, which means that setting the SameSite attribute is not possible using that function, until such point that support for the attribute gets added. If WordPress were to implement the SameSite attribute, we'd need our own cookie handling function which constructs and sets the Set-Cookie header itself, and use it in place of setcookie() (side note: this may also be beneficial to unit testing).

Attachments (2)

samesite.diff (20.7 KB) - added by tomdxw 7 years ago.
pluggable.diff (6.0 KB) - added by mikhailroot 5 years ago.
Introduced filter wp_auth_cookie_same_site to be able to manage SameSite Auth cookies parameter.

Download all attachments as: .zip

Change History (52)

#1 @mwaclawek
8 years ago

Just a quick note: A compatible and testable replacement for PHP's "setcookie" is available here: https://github.com/delight-im/PHP-Cookie It has support for same-site cookies as well.

Do we really need to wait until the spec is finalized? Chrome has shipped an implementation based on the current draft already, so implementing the current spec would have a real impact on user security immediately (though only for Chrome users).

#2 @noam@…
8 years ago

Related: PHP #72230: Add SameSite Cookies to setcookie() https://bugs.php.net/bug.php?id=72230

#4 @demetris
7 years ago

I just came upon a hack that makes it possible to set the SameSite attribute using the default setcookie() function:

<?php
setcookie('a-cookie', '1', 0, '/; samesite=strict');

I am trying it out on WordPress by passing a modified COOKIEPATH to setcookie():

<?php
$path_plus_samesite = COOKIEPATH . '; samesite=strict';

It seems to work OK on Chrome and on Firefox 59 (the first Firefox version to implement SameSite, currently in the beta channel).

I don’t know whether it would be acceptable to use such a hack in WordPress core, but I thought it would be good to know that it exists.

Link: https://stackoverflow.com/questions/39750906/php-setcookie-samesite-strict

Cheers!

#5 @tomdxw
7 years ago

I’ve written a proof of concept plugin which adds SameSite=Lax for all auth cookies: https://gist.github.com/tomdxw/9d7eced5f951680d6eeb52fe6a7a48dc

I tested with a vulnerable plugin, and it works to prevent the CSRF.

The change is tiny - when being merged into core it would require adding one function and modifying 4 lines (the 4 setcookie() lines in wp_set_auth_cookie).

In the very rare situations where a site needs to receive authenticated POST requests, the SameSite cookie can be disabled via filter.

I agree with @mwaclawek. As it’s already been implemented by three of the major browsers (shipped in Chrome, coming to Firefox in May, implemented in WebKit but not released in Safari yet), there’s little chance of substantial change. But it will do a great deal of good by fixing the majority of CSRF vulnerabilities in WordPress plugins (of which there are a lot: https://wpvulndb.com/search?utf8=%E2%9C%93&text=csrf ).

#6 @johnbillion
7 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 5.0

#7 @tomdxw
7 years ago

  • Keywords has-patch added; needs-patch removed

I’ve written a patch for this. I forked the https://github.com/delight-im/PHP-Cookie library suggested by @mwaclawek, and added support for older versions of PHP. It’s here: https://github.com/dxw/PHP-Cookie-Legacy

Only one change was made to src/Cookie.php, and that was to remove the namespace: https://github.com/dxw/PHP-Cookie-Legacy/commit/a5516d70826a56075eb6d452ae5e2028d61cce7c#diff-6c1c5ff819fd1e7697d48e0098012117

The tests are passing with 5.3, 5.4, 5.5, 5.6, 7.0, 7.1, and 7.2: https://travis-ci.org/dxw/PHP-Cookie-Legacy

Due to the tests using php-fpm, and php-fpm being introduced in PHP 5.3, I wasn’t able to get the tests to work with PHP 5.2 (pull requests welcome).

I copied src/Cookie.php (without modification) into WordPress, and modified four lines in wp-includes/pluggable.php. See attached patch.

@tomdxw
7 years ago

#8 @tomdxw
7 years ago

  • Keywords dev-feedback added

#9 @pento
6 years ago

  • Milestone changed from 5.0 to 5.1

#10 @pento
6 years ago

  • Milestone changed from 5.1 to 5.2

#11 @desrosj
6 years ago

  • Keywords needs-refresh added
  • Milestone changed from 5.2 to 5.3

Patch needs to be refreshed to apply cleanly to trunk. 5.2 beta is in less than 2 days. Punting to 5.3.

#12 @ayeshrajans
6 years ago

I'd love to see this move forward too. Even though we commit to a more recent WordPress version, it wouldn't be PHP 7.3 right away, so for BC, we will need a polyfill.

I worked on a quite small scale polyfill (https://github.com/Ayesh/WordPress-SameSite/blob/50705de58a598c46f1ddd70faf6ccce59877b0cc/samesite.php#L172-L188) at just 16 lines and a single function. This function doesn't try to be a complete poyfill, but is a rather auth-cookie exclusive one.

I'd be happy to roll a patch with this smaller polyfill or the delight-im/PHP-Cookie package.

#13 @ayeshrajans
6 years ago

If anyone is interested, I created a plugin to do just that: https://wordpress.org/plugins/samesite

#14 @johnbillion
6 years ago

  • Milestone changed from 5.3 to Future Release

#15 @johnbillion
5 years ago

#48183 was marked as a duplicate.

#16 @kirasong
5 years ago

Wanted to be sure the information from #48183 made its way to this ticket.

Chrome is going to set SameSite=lax by default, targeted for Chrome 80:
https://www.chromestatus.com/feature/5088147346030592
https://www.chromium.org/updates/same-site

Looks like that will likely be first quarter next year:
https://www.chromestatus.com/features/schedule

Last edited 5 years ago by kirasong (previous) (diff)

#17 @johnbillion
5 years ago

  • Milestone changed from Future Release to 5.4

Moving to 5.4 based on the above.

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


5 years ago

#19 @Toru
5 years ago

Looks like that will likely be first quarter next year:

It is now scheduled for 4 Feb, 2020, which is way before the planned 5.4 date.

https://www.chromestatus.com/features/schedule
https://make.wordpress.org/core/2020/01/14/wordpress-5-4-planning-roundup/

@mikhailroot
5 years ago

Introduced filter wp_auth_cookie_same_site to be able to manage SameSite Auth cookies parameter.

#20 follow-up: @mikhailroot
5 years ago

By default Chrome will treat missing SameSite param as Lax, so most of users will be treated well out of the box. (https://web.dev/samesite-cookies-explained/)

If certain amount of users require to manage this part they will update to PHP 7.3.0 + (e.g. they might require to embed wp-admin or authenticated state of their site into some other one via iframe - they need to set it to None).

There's no big need to have polyfill code to run for everyone to try to support older php versions, which don't support new setcookie syntax which supports SameSite param.

That's why i came up with this simpler solution https://core.trac.wordpress.org/attachment/ticket/37000/pluggable.diff

#21 @davidbaumwald
5 years ago

  • Keywords needs-dev-note added
  • Milestone changed from 5.4 to Future Release

This ticket still needs a decision, and with 5.4 Beta 1 landing tomorrow, this is being moved to Future Release. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

Also marking this as needs-dev-note as the new filter needs at least a small call-out on the Misc Dev Note.

#22 follow-up: @adam320
5 years ago

Will the addition of the wp_auth_cookie_same_site filter solve the issue I am having with multisite domain mapping? Chrome no longer logs you into the front end of a subdomain network because of the SameSite issue. What is odd is that in firefox, the cookies currently are set flagged as SameSite=None; Secure, but in chrome they show as blocked and the flags are not set.

#23 in reply to: ↑ 22 @mikhailroot
5 years ago

The thing is that PHP only since 7.3 has ability to set SameSite, so first check which PHP version you are using. Chrome defaults to SameSite=Lax if it's not set. I've decided to add this filter because I consider it's better to give other developers more control of this setting if it gets set explicitly.
I personally have tested it with WordPress+Shopify sites which are on subdomains like wp.example.com - running WordPress and example.com - hosted by Shopify, and another site is example2.com running WP and shopify.example2.com - running shopify. All works smooth with SameSite=None - I needed to allow access to WordPress authenticated admin side to be used inside Shopify's App Iframe, without SameSite=None it would get blocked in Chrome. I consider if you have php 7.3+ and you've applied patch I've proposed it should work for you.

Replying to adam320:

Will the addition of the wp_auth_cookie_same_site filter solve the issue I am having with multisite domain mapping? Chrome no longer logs you into the front end of a subdomain network because of the SameSite issue. What is odd is that in firefox, the cookies currently are set flagged as SameSite=None; Secure, but in chrome they show as blocked and the flags are not set.

This ticket was mentioned in Slack in #core by steelwagstaff. View the logs.


5 years ago

#25 in reply to: ↑ 20 ; follow-up: @SteelWagstaff
5 years ago

Hi -- I'm the product manager for Pressbooks, an open source publishing tool built on WordPress multisite. We're in that small subset of users who need

to embed wp-admin or authenticated state of their site into some other one via iframe

as described by @mikhailroot. We do this when our tool is securely embedded within a learning management system using the LTI specification. In order for the LTI connection to work as expected in Chrome, we need to be able to set WP authorization cookies to SameSite=None. We're using PHP 7.3, which added an options array supporting the setting of SameSite attributes, but I don't know how we can set this value without forking core WordPress, which we're very reticent to do. Ideas/suggestions welcomed!

#26 in reply to: ↑ 25 ; follow-up: @ayeshrajans
5 years ago

I have released v1.4 of the plugin https://wordpress.org/plugins/samesite/ , that supports configurable SameSite values. You can add the following to wp-config.php file to override the browser-default (what browsers assume when SameSite flag is not set) and php.ini default:

define( 'WP_SAMESITE_COOKIE', 'Lax' );

Change "Lax" to "None" if you want to forcefully disable Chrome 80+ behavior of automatically assuming SameSite=Lax on its A/B test users.

#27 in reply to: ↑ 26 @adam320
5 years ago

Is there a way to set 'Secure' as well. All my research said that you need to set SameSite=None; Secure

#28 follow-up: @ayeshrajans
5 years ago

WordPress should pass the "secure" flag if the page is served with HTTPS. Setting "secure" flag over an HTTP connection will make browsers downright reject the cookie. Is your site is being served over HTTPS?

#29 in reply to: ↑ 28 @adam320
5 years ago

Ah, sweet, thanks for the explanation.

#30 @mikhailroot
5 years ago

I've created this plugin (works with PHP 7.3+ - for older versions it just informs you need to update PHP) which allows to set SameSite parameter in WP Admin https://github.com/MikhailRoot/samesite-cookie-manager
I use it till SameSite param management will be added to Core.
Hope it will help someone.

#31 @MadtownLems
4 years ago

I would like to add my support for hoping we can get a Core filter for the auth/logged-in cookies' samesite attribute. We are one of many networks out there that serve some front-ends from a different domain than the dashboard is served through, and our users are no longer treated as being logged in on the front-end.

This ticket was mentioned in Slack in #core by jonshipman. View the logs.


4 years ago

#33 @jonshipman
4 years ago

I suggest that there be a WordPress function that acts as a wrapper around setcookie that provides an action hook users can remove/add to. Something like a wp_setcookie that performs a 'wp_setcookie' do_action.

This ticket was mentioned in PR #687 on WordPress/wordpress-develop by jonshipman.


4 years ago
#34

  • Keywords has-unit-tests added; needs-refresh removed

Created a function for wp_setcookie acting as a wrapper for an action called wp_setcookie which is hooked to a default action function named... _wp_setcookie. Lays a foundation to implement 37000. Doesn't outright fix the issue, but should retain existing functionality with little fuss while letting devs implement their own solutions in the interim.

https://core.trac.wordpress.org/ticket/37000

#35 @jonshipman
4 years ago

Just noticed that there is in fact a deprecated function called wp_setcookie. We could call it wp_setbiscuit.

This ticket was mentioned in Slack in #core-restapi by jonshipman. View the logs.


4 years ago

#37 follow-up: @jmichaelward
4 years ago

I'm working on an open issue today that relates to this Trac ticket.

In Gravity Forms, users who attempt to authenticate their sites with external services using OAuth usually get redirected back to the settings page in the admin. However, we've found that this fails in Chrome if the admin has been logged into the site for more than 2 minutes. In those situations, when the external service redirects back to WordPress, the users are returned to the login screen.

I suspect there may be other plugins in the ecosystem which will be increasingly affected by this. It looks like there's been a lot of great potential approaches proposed in this ticket over the past few years – I'm happy to collaborate with others who can help prioritize this for the next release.

This ticket was mentioned in Slack in #core by jmichaelward. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by jmichaelward. View the logs.


4 years ago

#40 in reply to: ↑ 37 ; follow-up: @r0wan
4 years ago

Replying to jmichaelward:

In Gravity Forms, users who attempt to authenticate their sites with external services using OAuth usually get redirected back to the settings page in the admin. However, we've found that this fails in Chrome if the admin has been logged into the site for more than 2 minutes. In those situations, when the external service redirects back to WordPress, the users are returned to the login screen.

This sounds like you may be hitting the Lax + POST mitigation which was allowing cookies created within a two minute window to be sent on cross-site POST requests. This was initially intended to provide some breathing room for SSO clients that were setting a nonce or similar security token in a cookie which would be validated on the callback POST from the the sign-in service. https://www.chromium.org/updates/same-site/faq#sites-canvas-main-content:~:text=Lax%20%2B%20POST%20mitigation

It also sounds similar to the issue some sites were hitting with 3-D Secure verification on payments, where the returning call was a cross-site POST request that was resulting in the site's session cookies not being included on that request. I've got a little write up of that behaviour and potential solutions here: https://goo.gle/samesite-3d-secure

#41 in reply to: ↑ 40 @jmichaelward
4 years ago

Replying to r0wan:

Replying to jmichaelward:

In Gravity Forms, users who attempt to authenticate their sites with external services using OAuth usually get redirected back to the settings page in the admin. However, we've found that this fails in Chrome if the admin has been logged into the site for more than 2 minutes. In those situations, when the external service redirects back to WordPress, the users are returned to the login screen.

This sounds like you may be hitting the Lax + POST mitigation which was allowing cookies created within a two minute window to be sent on cross-site POST requests. This was initially intended to provide some breathing room for SSO clients that were setting a nonce or similar security token in a cookie which would be validated on the callback POST from the the sign-in service. https://www.chromium.org/updates/same-site/faq#sites-canvas-main-content:~:text=Lax%20%2B%20POST%20mitigation

It also sounds similar to the issue some sites were hitting with 3-D Secure verification on payments, where the returning call was a cross-site POST request that was resulting in the site's session cookies not being included on that request. I've got a little write up of that behaviour and potential solutions here: https://goo.gle/samesite-3d-secure

Thanks for the response, @r0wan. I've been reviewing your documentation and considering how we might implement this on our side. It appears you've been helpful in other forums, too, as I also discovered your comments about how to temporarily disable the 2-minute window for Lax+POST.

Needless to say, this information should be helpful for me to sort out how to resolve this issue for our plugin and suite of add-ons.

#42 @mikejolley
4 years ago

I am building a headless site on a different domain supporting logins via the Rest API/WPGraphQL using cookies. I require samesite=none.

To workaround this issue I have to use the set_auth_cookie and set_logged_in_cookie actions to set the cookies manually, and bail before WP sets it's cookies using the send_auth_cookies filter.

This is quite a lot of code that could be better achieved if WP had a wrapper for it's setcookie function—one which provided a filter, or method of short circuiting it based on the cookie name.

This ticket was mentioned in Slack in #core by rickcurran. View the logs.


3 years ago

#44 @rickcurran
3 years ago

Hi, I just wanted to raise my hand to say that having SameSite support added here would be an important addition. I had the lack of SameSite attributes on the wordpress_sec_ and wordpress_logged_in_ cookies flagged in a penetration test on a site recently so having this added would be of benefit in regards to security evaluation.

#45 @adam320
3 years ago

Yeah, Chrome just did something that completely breaks multisite networks mapped to domains. You used to be able to disable the samesite cookie in chrome settings, but it doesn't work anymore. So a domain mapped multisite you simply cant be logged into the whole network at the same time. the network sites dont get a wp admin bar because you don't get logged into the mapped domain, just the backend subdomain. Hoping wordpress can do something so multisite works in chrome.

#46 @dbaker
3 years ago

With the rise of headless WordPress, this has become very important to get Previews working across domains. I'd urge the team to raise the priority on this.

@mikhailroot plugin is the best solution for now, good work on that!

#47 @lucastello
2 years ago

Another element to raise priority: Android WebView treats now Cookies without a SameSite attribute as SameSite=Lax.
https://developer.android.com/about/versions/12/behavior-changes-12#samesite

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


6 months ago

#49 @swissspidy
6 months ago

#55440 was marked as a duplicate.

#50 @swissspidy
6 months ago

There was also a similar patch over at #55440

What I don't like about a proposed wp_set_cookie function is that it will become obsolete once we raise our minimum PHP version to 7.3. Right now we're at 7.2+.

Note: See TracTickets for help on using tickets.