Language Change ? ? Success

User tests: Successful: Unsuccessful:

avatar HLeithner
HLeithner
3 Jul 2019

Use new cookie set function signature as introduced in joomla-framework/input#27 and added the SameSite parameter to the config and is used for the session cookie. Problem is that this is only supported with PHP 7.3 and have no impacted in PHP 7.2.

More information to SameSite can be found at https://www.owasp.org/index.php/SameSite

It should be discussed if 'strict' as default is good or not.

Summary of Changes

  • Added SameSite parameter in global configuration
  • Updated set cookie in Session Storage
  • Updated set cookie in com_banners
  • Updated Authentication cookie (include SameSite)
  • Updated set cookie in languagefilter plugin
  • Updated set cookie in logout plugin
  • Updated user joomla plugin (include SameSite)

Testing Instructions

  • Login/Logout
    also from external Links (if you link from an external Site you should not be logged into Joomla if you are already loggedin)
  • Banner component

Expected result

Not logged in if you come from an external link.

Actual result

Logged in.

Documentation Changes Required

Describe the new option and it's impact

Votes

# of Users Experiencing Issue
2/2
Average Importance Score
5.00

avatar HLeithner HLeithner - open - 3 Jul 2019
avatar HLeithner HLeithner - change - 3 Jul 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 3 Jul 2019
Category Administration com_banners com_config Language & Strings Libraries Front End Plugins
avatar franz-wohlkoenig franz-wohlkoenig - change - 3 Jul 2019
Title
Add SameSite cookie parameter
[4.0] Add SameSite cookie parameter
avatar franz-wohlkoenig franz-wohlkoenig - edited - 3 Jul 2019
avatar brianteeman
brianteeman - comment - 3 Jul 2019

We have always had a policy of not having code that is not supported in all supported server software.

deb6c5c 3 Jul 2019 avatar HLeithner cs
avatar HLeithner HLeithner - change - 3 Jul 2019
Labels Added: ? ?
avatar HLeithner
HLeithner - comment - 3 Jul 2019

Doesn't mean it has to be merged into J4 and a discussion could always be started if this policy is useful for a security feature. Anyway I only created the PR because of the new syntax of the framework package.

avatar brianteeman
brianteeman - comment - 3 Jul 2019

If it's for security then even more reason to either reject immediately or to increase the minimum PHP version. The very idea of having joomla less secure on a supported stack us just plain crazy

avatar mbabker
mbabker - comment - 3 Jul 2019

Not having SameSite on your cookies isn't going to automatically make your site less secure. Otherwise, you can claim that all 3.x releases have an unpatched security vulnerability by having no way to enable SameSite support.

Also, assuming anyone is still interested in data driven decision making, usage numbers don't support a PHP 7.3 minimum requirement.

avatar HLeithner
HLeithner - comment - 3 Jul 2019

What you say is, if I'm a person that has my installation up to date I still can't have the latest security features because others are unable to maintain there infrastructure.

Anyway we have to support J4 3-4 more? years before there will be a new Major version with an increase of the min PHP Version. Not supporting features that could im improve security is sad for people want to do it's best in security.

But i see you are not interested in this. thx for the comments.

avatar HLeithner HLeithner - change - 3 Jul 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-07-03 12:26:43
Closed_By HLeithner
avatar HLeithner HLeithner - close - 3 Jul 2019
avatar dkanchev
dkanchev - comment - 1 Oct 2019

I am not sure if the SameSite cookie support has been discussed in details. But Chrome 80 (scheduled to be released in the beginning of 2020) will force SameSite:Lax by default for cookies that do not have SameSite set. This is possibly a problem for many sites, plugins, themes, etc. I am not sure devs understand the issues which could occur.

Thus, in order to raise awareness I decided to post an update here. I think it is very important for devs to check core and also inform plugins/themes devs about this change. This way we can make sure that as many people as possible will test their code with the new Chrome versions and make sure code works. Third party cookies will stop working in Chrome 80 unless they have SameSite:None.

More info here:

https://www.chromestatus.com/feature/5088147346030592
https://web.dev/samesite-cookies-explained


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.
avatar brianteeman
brianteeman - comment - 1 Oct 2019

Thanks for your comment @dkanchev

I read those posts when they were published (and again now) and I must admit to still not understanding it. Surely all cookies should already be set to samesite anyway?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.

avatar dkanchev
dkanchev - comment - 3 Oct 2019

@brian The problem is that if a site (Joomla! core, plugins, themes) sets cookies which are used by other domains then this will stop working in Chrome 80. I am not sure if the Joomla! core sets such cookies. But this is why I updated this task - so that core devs can check that.

In addition, I think the Joomla! dev community needs to be made aware of this change. Advertising plugins, chat apps, promo plugins can all be affected by this change in Chrome. Unless those cookies are tagged with SameSite:None the will stop working.

avatar brianteeman
brianteeman - comment - 3 Oct 2019

@dkanchev thanks for the explanation - I assumed that cookies would always be set to samesite by default. Looks like they are not.

@HLeithner is there any way that this can be implemented without it requiring php 7.3?

avatar HLeithner
HLeithner - comment - 3 Oct 2019

Yes but I think we don't want to go this way, it would mean we have to set the cookie header by hand (re-implement setcookie function).

avatar HLeithner HLeithner - change - 3 Oct 2019
Status Closed New
Closed_Date 2019-07-03 12:26:43
Closed_By HLeithner
avatar HLeithner HLeithner - change - 3 Oct 2019
Status New Pending
avatar HLeithner HLeithner - reopen - 3 Oct 2019
avatar HLeithner
HLeithner - comment - 3 Oct 2019

And as far as I know Joomla core has no cookies that should be used by 3rd party sites.

avatar dkanchev
dkanchev - comment - 3 Oct 2019

If the core really doesn't set such cookies this is great. Still, it is very very good for at least 2 core team members to confirm that. Magento/WordPress for example allow users to set-up multi-site installations when multiple domains use one and the same app/core installation. In those cases it is imperative to set cookies correct in case they have to be shared between domains using the same core. I am not sure if Joomla! has ever had such features.

And the bigger problem is the community of developers. I think effort needs to be made to inform all devs (or at least the devs of the most popular plugins/themes/modules/etc.) that this change is coming. Because if this doesn't happen many sites could be broken and thinking only about the core is not good. I've never seen a site without any extensions. As a person who has worked in tech support for a major hosting company I can tell you that this is a huge problem. All those clients will contact their hosting companies and they cannot do a thing in this case.

So PLEASE have this communicated in the community and make sure devs are aware as much as possible!

avatar HLeithner
HLeithner - comment - 4 Oct 2019

@dkanchev we will write a blog post about this topic @marcodings is in charge for this.

A simple implementation could be like this (untested code):

function setcookieJF($name, $value = '', $expire = 0, $path = null, $domain = null, $secure = false, $httpOnly = false, $sameSite = null)
{
	$cookie= 'Set-Cookie: ' . rawurlencode($name) . '=' . rawurlencode($value);

	if ($expire)
	{
		$cookie .= ';expires=' . $expire;
	}

	if (!is_null($path))
	{
		$cookie .= ';path=' . rawurlencode($path);
	}

	if (!is_null($domain))
	{
		$cookie .= ';domain=' . rawurlencode($domain);
	}

	if ($secure === true)
	{
		$cookie .= ';secure';
	}

	if ($httpOnly === true)
	{
		$cookie .= ';httponly';
	}
	if (!is_null($sameSite))
	{
		$cookie .= ';samesite=' . rawurlencode($samesite);
	}

	header($cookie, false);
}

But I'm still not sure if it should be implemented in J3, for J4 it would make more sense to add it to the framework...

avatar brianteeman
brianteeman - comment - 4 Oct 2019

For reference there is nothing in core affected by this

avatar mbabker
mbabker - comment - 5 Oct 2019

Please don’t write a setcookie replacement.

avatar brianteeman
brianteeman - comment - 14 Feb 2020

we will write a blog post about this topic @marcodings is in charge for this.

Gentle reminder

avatar peteruoi
peteruoi - comment - 14 Feb 2020

Btw, is there any point to bump to 7.3? We are nine months away from php 7.2 security support and i believe joomla 4 will be out at best after 3-4 months.

avatar mbabker
mbabker - comment - 14 Feb 2020

Btw, is there any point to bump to 7.3?

From a technical perspective, the only reason to bump from 7.2 to 7.3 is if you are in the camp of "a PHP version having security only support means it is unsupported". Aside from SameSite support in cookies, there is no technical benefit to a 7.3 minimum version almost anywhere in the PHP ecosystem. If you're going to advocate for a raised minimum, there is more benefit to the engine changes added in 7.4 than there is 7.3.

i believe joomla 4 will be out at best after 3-4 months.

Wishful thinking.

avatar asiby
asiby - comment - 6 Mar 2020

I have a working code that I will later submit here. I needed only three code changes for my specific use case. It solved our issue now. And at least I can have multiple pairs of eyes analyzing my solution to see if somethings is wrong with it. Stay tuned.

Also, I have noticed that this Pull Request is setting the default SameSite attribute to Lax. I can confirm that this will break the current behaviour for several sites that need to be embedded in an IFRAME on a different domain name.

IMHO, the default value should be SameSite: None; Secure . Then, people can purposely dial the setting up based on their specific needs.

avatar brianteeman
brianteeman - comment - 12 Apr 2020

we will write a blog post about this topic @marcodings is in charge for this.

Another reminder

avatar brianteeman
brianteeman - comment - 3 Jul 2020

Anything??


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.

avatar Ddcdom
Ddcdom - comment - 13 Aug 2020

Just confirming @asiby's comment. Joomla pages which use sessions and are embedded via an iframe on external domains have been broken in Chrome since February 2020.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.

avatar asiby
asiby - comment - 13 Aug 2020

(edited the minimum PHP version following @Ddcdom comment)

After manually patching Joomla's core to set the desired SameSite parameter for the Joomla cookies, it got broken during our next Joomla update. I have noticed that some sort of solution was implemented by Joomla regarding the issue, but it is not addressing the issue for the default handling of the sessions and login_state cookies. I had to patch it again.

In our use case, we have extended Joomla by providing an SSO authentication mechanism. And the sites using that method are having our Joomla based pages rendered in a frame. This was working well for years until recently when Google Chrome has issued a new possible regarding the SameSite cookie parameter.

Regardless of what people are saying, the root of the issue (in our case) was found in Joomla's core.

The following codes have fixed it for us. Well, until the next Joomla upgrade.

Note that we are using PHP 7.3.* so we didn't bother checking for the PHP version and selectively using the old or new signature for session_set_cookie_params(). If some folks are still using PHP version prior to PHP 7.3+, then this solution is not for them. Besides, they SHOULD upgrade PHP.

Also, in the following code, the 'secure' parameter is forced to true because that's what we are always using for everything. The proper way to make this patch more portable is to check the value of the $secure argument and act accordingly.

To make things clear, this solution may not work for some of you and you may need to slightly adjust it. However, IMHO, it clearly shows that Joomla's core has something to do the the SameSite related issues that people are having.

Joomla session cookie

File Location: /libraries/joomla/session/handler/joomla.php

  • Step 1 - Original Code: Locate the following code on line 151

    session_set_cookie_params($cookie['lifetime'], $cookie['path'], $cookie['domain'], $cookie['secure'], true);

  • Step 2 - Replace code from step 1 with the following ...

    session_set_cookie_params([
      'lifetime' => $cookie['lifetime'],
      'path' => $cookie['path'],
      'domain' => $cookie['domain'],
      'secure' => true,
      'httponly' => true,
      'SameSite' => 'None',
    ]);
    

Fixing the login state cookie

File location: /libraries/src/Input/Cookie.php

Changeset 1

  • Step 1 - Original Code: Locate the function named public function set(...) at line 89
    Then identify the following code in this function

    setcookie($name . "[$key]", $val, $expire, $path, $domain, $secure, $httpOnly);

  • Step 2 - New code: Replace the code from step 1 with the following code

    setcookie($name . "[$key]", $val, [
     'expires' => $expire,
     'path' => $path,
     'domain' => $domain,
     'secure' => true,
     'httponly' => $httpOnly,
     'SameSite' => 'None',
    ]);
    

Changeset 2

File location: /var/www/html/libraries/src/Input/Cookie.php

  • Step 1 - Original Code: Locate the function named public function set(...) at line 89
    Then identify the following code in this function
    setcookie($name, $value, $expire, $path, $domain, $secure, $httpOnly);

  • Step 2 - New code: Replace the code from step 1 with the following code

    setcookie($name, $value, [
      'expires' => $expire,
      'path' => $path,
      'domain' => $domain,
      'secure' => true,
      'httponly' => $httpOnly,
      'SameSite' => 'None',
    ]);
    
avatar Ddcdom
Ddcdom - comment - 14 Aug 2020

Couple of points for @asiby last comment.

The new options array signature was added to the session_set_cookie_params function in PHP 7.3, so this solution isn't gong to be cut and pasteable for people on 7.0, 7.1, or 7.2.

I've good an ugly work around which doesn't require editing core files. Because neither Joomla's config, nor PHP's session_set_cookie_params parses the path parameter properly, it possible to slip in other cookie attributes using a semicolon. In Joomla's "Global configuration" >> "site" >> "Cookie settings" >> "Path", if you enter /cookie/path; SameSite=None, this will set cookies with a path of 'cookie/path' and a SameSite attribute. If you're cookie path is empty, as most will be, then set at cookie path of just /.

Its not great to rely on this, but until the core is fixed, at least it doesn't require modification to core code.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.

avatar asiby
asiby - comment - 20 Aug 2020

Interesting. Thanks @Ddcdom. I will give this a try and let you know how it works.

avatar HLeithner HLeithner - change - 5 Sep 2020
Labels Added: Conflicting Files
avatar HLeithner
HLeithner - comment - 5 Sep 2020

I added none as cookie option but I'm not sure if it's needed... anyway if none is selected we need the secure parameter set too. This part is out of scope of this pr because if it get done it should be in the library.

@asiby @Ddcdom tests would be great so we can merge this into j4.

@brianteeman based on the progressive enhancement motion this pr should be ok for j4

avatar brianteeman
brianteeman - comment - 5 Sep 2020

That's the undocumented motion?

My reminders were about the blog post you said was being written

avatar HLeithner
HLeithner - comment - 5 Sep 2020

We have a public motion register at https://www.opensourcematters.org/httpswww.opensourcematters.org/organisation/team-membership/registry-of-motions/289-2020.html#production-department-motions-2020 the motion I refer to is #PROD2020/021

But I thought we have a blog post too but didn't searched for it

avatar HLeithner HLeithner - change - 5 Sep 2020
Labels Removed: Conflicting Files
avatar HLeithner
HLeithner - comment - 14 Oct 2020

@asiby @Ddcdom @peteruoi @dkanchev if you still intressted to get this to core please test the pr and mark it as success (if it's a success) at https://issues.joomla.org/tracker/joomla-cms/25414

avatar Bodge-IT
Bodge-IT - comment - 17 Oct 2020

@HLeithner Can you clarify test process please

avatar HLeithner
HLeithner - comment - 17 Oct 2020
  • Install Joomla

  • Check Login frontend/backend

  • Stay logged in

  • Open a new Tab and navigate to your joomla installation, you should be logged in

  • Create a website on another Domain with a link to your joomla installation

  • click on the link of the website, you should NOT been logged in

  • You can change the settings in the global configuration to set samesite cookie to lax (then the link should work and you should be logged in) or none (only works with force https)

avatar zero-24
zero-24 - comment - 25 Oct 2020

@HLeithner can you please take a look into the drone issue? I have already updated the branch but seems something else is still broken?: https://ci.joomla.org/joomla/joomla-cms/37005/1/11

avatar richard67
richard67 - comment - 25 Oct 2020

I've restarted drone, maybe that helps.

avatar Denitz
Denitz - comment - 3 Nov 2020

Sorry, a question: where are the changes for Joomla\CMS\Input\Cookie::set() method?

We can easily use the new method for both PHP < and >= 7.3.

< 7.2 uses the known PHP bug/feature fixed in 7.3 (and actually not used since 7.3 supports samesite attr):
php/php-src@5cb825d

Something similar to :

public function set($name, $value, $expire, $path, $domain, $secure, $httponly, $samesite='Lax')
{
    if (PHP_VERSION_ID < 70300) {
        setcookie($name, $value, $expire, "$path; samesite=$samesite", $domain, $secure, $httponly);
    }
    else {
        setcookie($name, $value, [
            'expires' => $expire,
            'path' => $path,
            'domain' => $domain,
            'secure' => $secure,
            'httponly' => $httponly,
            'samesite' => $samesite,
        ]);
    }
}

`
avatar JamesBaku
JamesBaku - comment - 10 Dec 2020

PHP 7.2 is old not maintained PHP version.

If solutions is good for PHP 8.0, PHP 7.4 it should be implemented.

avatar HLeithner
HLeithner - comment - 10 Dec 2020

Sorry, a question: where are the changes for Joomla\CMS\Input\Cookie::set() method?

We can easily use the new method for both PHP < and >= 7.3.

< 7.2 uses the known PHP bug/feature fixed in 7.3 (and actually not used since 7.3 supports samesite attr):
php/php-src@5cb825d

Something similar to :

public function set($name, $value, $expire, $path, $domain, $secure, $httponly, $samesite='Lax')
{
    if (PHP_VERSION_ID < 70300) {
        setcookie($name, $value, $expire, "$path; samesite=$samesite", $domain, $secure, $httponly);
    }
    else {
        setcookie($name, $value, [
            'expires' => $expire,
            'path' => $path,
            'domain' => $domain,
            'secure' => $secure,
            'httponly' => $httponly,
            'samesite' => $samesite,
        ]);
    }
}

`

The PR has some flaws that's the reason why It's in draft again, I have other priorities at the moment (Release Blockers). The set have to be done in https://github.com/joomla-framework/input/tree/2.0-dev

avatar HLeithner
HLeithner - comment - 10 Dec 2020

PHP 7.2 is old not maintained PHP version.

If solutions is good for PHP 8.0, PHP 7.4 it should be implemented.

If some one want's to take over I have no problem with it, I don't have the time to fix this optional feature atm...

avatar zstergios
zstergios - comment - 14 Dec 2020

@Denitz code works for me. I can confirm path by pass works great!

Actually ,I suggest to add global configuration setting like Cookie Domain & Path. Without samesite will not work any third party service related with our website (payment gateways, captcha etc)

This should be added to 3.9 version too.
There many positives adding this feature. Example "Strict" mode will harden security


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.
avatar zstergios
zstergios - comment - 14 Dec 2020

My suggestion with global configuration setting and with global ini variable.
The goal is to be overridable if no value passed

public function set($name, $value, $expire, $path, $domain, $secure, $httponly, $samesite=null)
{
//Solution 1 with Global Config
$samesite=is_null($samesite) || !in_array(strtolower($samesite),array('none','lax','strict'))?$this->get('samesite','Lax'):$samesite;

//Solution 2 with INI (can be by passed from a custom plugin, so it's flexible too)
$defSameSite=ini_get('session.cookie_samesite')?ini_get('session.cookie_samesite'):'Lax';
$samesite=is_null($samesite) || !in_array(strtolower($samesite),array('none','lax','strict'))?$defSameSite:$samesite;

if (PHP_VERSION_ID < 70300) {
    setcookie($name, $value, $expire, "$path; samesite=$samesite", $domain, $secure, $httponly);
}
else {
    setcookie($name, $value, [
        'expires' => $expire,
        'path' => $path,
        'domain' => $domain,
        'secure' => $secure,
        'httponly' => $httponly,
        'samesite' => $samesite,
    ]);
}

}


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.

avatar richard67
richard67 - comment - 25 Jan 2021

Will this also solve #32147 ? The funny thing is I get that warning in browser console only at installation, but later in backend or frontend I don't get it anymore.

avatar dgrammatiko
dgrammatiko - comment - 25 Jan 2021

The funny thing is I get that warning in browser console only at installation

My localhost config is pointing to http://j4.local and I get the message everywhere. I think localhost is mostly ignoring the sameSite attribute (depends on the browser)

avatar HLeithner
HLeithner - comment - 25 Jan 2021

SameSite=None is only allowed on https connections

avatar richard67
richard67 - comment - 25 Jan 2021

Am always testing with https ;-)

avatar zstergios
zstergios - comment - 10 Feb 2021

Problem everyday grows in our community.
Cookies for PayPal, 3rd party services (aff,tracking orders) not working anymore. Any chance to fix it on Joomla 3.9?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25414.

avatar chnnst
chnnst - comment - 13 Feb 2021

This pull request is still a work in progress, what still needs to be done there?

avatar chnnst
chnnst - comment - 13 Feb 2021

If you are using PHP 7.4, yes you can use ini_set to workaround this issue.

However, ini_set('cookie_samesite') does not work in PHP Version <= 7.2.
I am sure PHP 7.3 do not support the value 'None'

I am sure there is no option for < 7.4

avatar Denitz
Denitz - comment - 14 Feb 2021

Why not to bump the J4 min version to PHP 7.4?

PHP 7.4 active support ends at 28 Nov 2021 (security at 28 Nov 2022), at the time of Joomla 4 1.0 even 7.4 can be dead :(

avatar chnnst
chnnst - comment - 15 Feb 2021

HLeithner is funny one, maybe it wants to keep php old version, no longer maintained? HLeithner don't have the time to fix this optional feature atm... maybe Denitz or zstergios want's to take over ?

avatar Denitz
Denitz - comment - 15 Feb 2021

Unfortunately, bumping the minimum PHP version for Joomla 4 to 7.4 is not so easy and it doesn't depend on me.

Btw, the samesite param can be quite easily added for all cookies (including session one) originating from Joomla via a system plugin.

avatar PhilETaylor
PhilETaylor - comment - 16 Aug 2021

This is one of only 2 outstanding issues flagged with the Joomla 4.0 Milestone. Please can we move the milestone else Joomla 4 Milestone will never reach 100%. Thanks.

avatar HLeithner HLeithner - change - 9 Jan 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-01-09 22:16:31
Closed_By HLeithner
Labels Added: Language Change ? ?
Removed: ? ?
avatar HLeithner HLeithner - close - 9 Jan 2022

Add a Comment

Login with GitHub to post a comment