EFForg/https-everywhere

onCookieChange overwrites cookies if both secure and non-secure one are sent

jsnajdr opened this issue · 3 comments

jsnajdr commented

Type: code issue/website issue

When logging into the WordPress.com service, the login endpoints sends two variants of the wordpress_logged_in session cookie:

Set-Cookie: wordpress_logged_in=secret
Set-Cookie: wordpress_logged_in=secret; secure=true; SameSite=None

The goal is to have valid session cookie for all the following cases:

  • the site detects a logged-in user when accessed over an unencrypted http:// URL. There are still rare cases that need this.
  • the site can load authenticated iframes or resources (images, videos) from another domain (CDN-like). It needs the SameSite=None attribute to support that usage.

Cookies with SameSite=None also need a secure=true attribute. Otherwise they would be rejected by the browser. That's why the non-secure cookie doesn't and can't have that attribute.

Now the HTTPS Everywhere extension automatically converts all incoming cookies to secure=true. The result is that a secure wordpress_logged_in cookie without the SameSite attribute is stored and used. The second one, with secure=true and SameSite=None, is overwritten/ignored.

That breaks the cross-domain authenticated requests where the SameSite=None attribute is required.

Is there a way around that? Can we specify a wordpress.com ruleset that fixes the issue just for the one site? Or can onCookieChange be patched to cover this case?

zoracon commented

@jsnajdr so we have a ruleset that we can amend here:
https://atlas.eff.org/domains/wordpress.com.html

Where removing the secure cookie mandate should resolve this since there is confirmation that not all insecure cookies can be secured.

The other solution that may help is renaming the insecure cookie. But I do not know the impact that would have in terms of current sessions with WP REST API. I am a former WP and WP VIP developer, so I am familiar with the architecture.

jsnajdr commented

Where removing the secure cookie mandate should resolve this since there is confirmation that not all insecure cookies can be secured.

In #19957 I'm submitting a fix where I exclude the wordpress_logged_in cookie in the securecookie rule.

The other solution that may help is renaming the insecure cookie.

We'll probably do this, too, but we'd need to fix up a lot of places to not break unencrypted HTTP requests. We might even remove the insecure cookie completely, as unencrypted HTTP traffic on WordPress.com is nearly extinct.

  • 👍1
zoracon commented

Ruleset 2021.3.4 released. Please give some time for reflected changes. Thank you.

  • 👍1