[RFC] partitioned option for setcookie/setrawcookie and sessions

[ndossche]

RFC: https://wiki.php.net/rfc/chips

[iluuu1994]

Partitioned cookies are marked as experimental by Mozilla (https://developer.mozilla.org/en-US/docs/Web/Privacy/Partitioned_cookies). Theoretically this means they are subject to change. It seems they're mostly pushed by Google.

I'm also unsure about the premise of the original issue:

As stated here (link), soon our cookies with SameSite=None; Secured, without the Partitioned parameter, will stop working.

I haven't tested this, but I think what this means is that Partitioned will essentially become the default, unless the domains are part of the same related website set. So I think this is essentially just an opt-in mechanism to ensure the cookies keep working? Related website sets are managed in a GitHub repository (https://github.com/GoogleChrome/related-website-sets). Thank you, Google, for that one. 😒

Maybe this warrants a short e-mail to the ML? I'm not a specialist in regards to cookies. Maybe other people have something to add.

[DustinAPI]

@nielsdos I am attempting to implement CHIPS for my app with an ugly workaround manually setting the cookie header so this PR would be very helpful but I think it should also include changes to the session cookie to truly allow developers to fix the problem.

session_set_cookie_params(['partitioned' => true])
['partitioned' => true]) = session_get_cookie_params()
ini_set('session.cookie_partitioned', 'on');

My use case is the most basic wherin my app (tool) is displayed in an iframe within a learning management system (LMS) and requires a session cookie for further interaction after the inital LTI launch.

[morsssss]

Partitioned cookies are marked as experimental by Mozilla (https://developer.mozilla.org/en-US/docs/Web/Privacy/Partitioned_cookies). Theoretically this means they are subject to change. It seems they're mostly pushed by Google.

AFAIK, Mozilla and Safari have experimented with similar ideas, but right now only Chrome supports the Partitioned attribute. Chrome's goal is to let developers opt in to partitioning, instead of doing it by default.

As stated here (link), soon our cookies with SameSite=None; Secured, without the Partitioned parameter, will stop working.

I haven't tested this, but I think what this means is that Partitioned will essentially become the default, unless the domains are part of the same related website set. So I think this is essentially just an opt-in mechanism to ensure the cookies keep working?

That's it indeed! Chrome just started an experiment, now active for 1% of users, in which it will block third-party cookies that lack the Partitioned attribute and that are not allowed by other means. When a third-party site sets a Partitioned cookie, Chrome allows that site to access the cookie when it's embedded on a page from the domain where it was set. Access from other domains is blocked.

If this experiment results in a launch down the road, I think plenty of developers will want to use Partitioned, even though right now it's only a Chrome thing.

[ndossche]

@nielsdos I am attempting to implement CHIPS for my app with an ugly workaround manually setting the cookie header so this PR would be very helpful but I think it should also include changes to the session cookie to truly allow developers to fix the problem.

session_set_cookie_params(['partitioned' => true]) ['partitioned' => true]) = session_get_cookie_params() ini_set('session.cookie_partitioned', 'on');

My use case is the most basic wherin my app (tool) is displayed in an iframe within a learning management system (LMS) and requires a session cookie for further interaction after the inital LTI launch.

Yeah, I fear that the scope of this must be increased, and ini settings are probably necessary too.
That would mean this needs an RFC, which I can do, but I'm busy with other PHP work right now.

[bukka]

I think we should probably have RFC for this as the agreement is not clear (Derick raised some concerns on internals) so I don't think we can just merge this.

[xepozz]

LGTM

[TimWolla]

It already looks misaligned, so let's have the space after comma:

Suggested change

	STD_PHP_INI_BOOLEAN("session.cookie_partitioned","0",PHP_INI_ALL, OnUpdateSessionBool, cookie_partitioned, php_ps_globals, ps_globals)
	STD_PHP_INI_BOOLEAN("session.cookie_partitioned","0", PHP_INI_ALL, OnUpdateSessionBool, cookie_partitioned, php_ps_globals, ps_globals)

[ndossche]

I'll just add a commit to reformat this table. But a separate commit for history reasons...

[TimWolla]

Using false/true here is inconsistent with the rest and also with the reassignment in line 1809. This should be unified one way or another.

Independent of this, this should probably be fixed once and for all with a tree-wide Coccinelle run. Something like:

 @@
bool b;
@@
- b = 1
+ b = true
@@
bool b;
@@
- b = 0
+ b = false

[ndossche]

I'll fix this for ext/session post-merge as a follow up

[TimWolla]

I'll fix this for ext/session post-merge as a follow up

Would do this tree-wide (possibly with per-extension commits) to not fix this piecemeal. See also: #19418 (comment)

A good opportunity might be right when branching PHP 8.5.

[TimWolla]

Vote seems exceedingly likely to pass and the implementation is good except for minor nits, thus already requesting RM review as per https://externals.io/message/128453.

[ndossche]

Vote was accepted

[TimWolla]

Previous remarks are just nits without an effect on functionality, so this should've been an approval.

[edorian]

No RM objections 👍