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

Magento EE 1.14.1.0 compatibility issue? #725

Closed
csdougliss opened this issue Jan 12, 2015 · 14 comments
Closed

Magento EE 1.14.1.0 compatibility issue? #725

csdougliss opened this issue Jan 12, 2015 · 14 comments
Assignees

Comments

@csdougliss
Copy link
Contributor

Hi,

I've noticed that after upgrading to Magento EE 1.14.1.0 there is an additional cookie called frontend_cid. Quite often (but not always) I will be re-directed from one page checkout to the basket. My basket is then empty for some reason.

Is there any issues with the new Magento version? Any work arounds? I am just looking into the issue now, was wondering if anyone else had tried the new version.

All error logs are empty. The issue does not occur with varnish disabled

Also when I delete my browser cookies I still have something in my basket. Very strange.

@csdougliss
Copy link
Contributor Author

I have narrowed this down due to a code addition in 1.14.1.0 that checks for man in the middle attacks. This code only runs after turpentine has created it's frontend cookie and is not compatible.

It actually deletes your session!

In app/code/core/Mage/Core/Model/Session/Abstract/Varien.php, line 135

if (Mage::app()->getFrontController()->getRequest()->isSecure() && empty($cookieParams['secure'])) {
            // secure cookie check to prevent MITM attack
            $secureCookieName = $sessionName . '_cid';
            if (isset($_SESSION[self::SECURE_COOKIE_CHECK_KEY])
                && $_SESSION[self::SECURE_COOKIE_CHECK_KEY] !== md5($cookie->get($secureCookieName))
            ) {
                session_regenerate_id(false);
                $sessionHosts = $this->getSessionHosts();
                $currentCookieDomain = $cookie->getDomain();
                foreach (array_keys($sessionHosts) as $host) {
                    // Delete cookies with the same name for parent domains
                    if (strpos($currentCookieDomain, $host) > 0) {
                        $cookie->delete($this->getSessionName(), null, $host);
                    }
                }
                $_SESSION = array();
            }
            if (!isset($_SESSION[self::SECURE_COOKIE_CHECK_KEY])) {
                $checkId = Mage::helper('core')->getRandomString(16);
                $cookie->set($secureCookieName, $checkId, null, null, null, true);
                $_SESSION[self::SECURE_COOKIE_CHECK_KEY] = md5($checkId);
            }
        }

@BradStephens
Copy link

I've noticed the cart clearing for no apparent reason as well. I'd narrowed it down to most likely be a sessions issue, and now that I'm seeing this, I suspect this is the issue. I am not using Turpentine or Varnish in my environment. Have you made any more headway with this, Craig? I've only noticed the issue when adding an item to the cart, and subsequently loading the cart page.

@csdougliss
Copy link
Contributor Author

@BradStephens I've commented out the respective code above and it works fine now

@crasx
Copy link

crasx commented Jun 4, 2015

It took me forever to figure out what was going on, thanks for posting. The above fix worked for me. It's not the most secure thing but it seems like we don't have much of a choice here. I'll turn it into an extension tomorrow. Not sure that they would want to put it in the turpentine extension.

To make this more googleable:
Magento cart lost when switching to ssl, frontend cookie changed

@Petce
Copy link

Petce commented Jun 29, 2015

Hey guys, I have been suffering from a similar if not the same issue and I have some ideas as to what is really going on. Please take a look at Issue #829.

Quick question, are you all running your entire Magento Frontends under SSL? Or do you only switch to SSL when required?

@csdougliss
Copy link
Contributor Author

@Petce Only when required

@crasx
Copy link

crasx commented Jul 8, 2015

my fix is here: https://github.com/crasx/Magento-varnish-ssl-fix
so we don't need to modify core

@wingsryder
Copy link

is that fix also work for community edition 1.9.1 , SSL fix

@crasx
Copy link

crasx commented Jul 27, 2015

I'm not sure. Only tested on EE

@crasx
Copy link

crasx commented Nov 16, 2015

This fix is not compatible with the redissession extension. frustrating as hell.

@pixiemediaweb
Copy link

pixiemediaweb commented Jun 12, 2016

Just wanted to say a massive thanks for craigcarnell for your post about app/code/core/Mage/Core/Model/Session/Abstract/Varien.php. We've built a custom IP based store switcher for EE 1.14 and had an issue when the user has a cart session on one store before switching to another. Once they build a basket on the new store and proceed to checkout the basket would empty. The frontend_cid cookie is then renewed for the new store - and they can start again without issue, but what user would do that! Localised and modified Varien.php and now it's all working properly again.

Thank you

@tdgroot
Copy link

tdgroot commented Jun 22, 2016

@tomdollarmpd Good to hear the "fix" is working, but shouldn't there be a better way to solve this problem? The fix basically reopens a security breach in your shop.

@rafaeljuzwiak
Copy link

I was with the same issue, but I notice in a different location first (when users tried to login for the first time, after the first always worked), however I had the same cart issue as well using Magento 1.9.1 CE. After a while trying to figure out why, I found the problem with the session, which searching on google took me to this issue on Github.

After reading all this thread, I tried a few more things, but at the end I gave up to @crasx solution just rewriting the session class since I was unable to find anything else in the problem, and I could not find the cause.

However, I was also unhappy with this solution, and today I finally fixed the problem (at least for me, I believe it will fix for you guys as well)
So, I am using Varnish 4.0 + Nginx as a proxy for TLS/SSL. As proxy I tested Pound, Apache2, Hitch, and Nginx, all of them presented the same issue, for that reasons I believe that fix would work with any other proxy you are using.

I am not gonna go into details again, but I posted my solution here: #1279

Hope it helps you guys, and you do not have to rewrite Magento Core, just add a new hash into the Tupertine VCL template.

Thanks.

@PromInc
Copy link

PromInc commented Nov 10, 2016

Magento has what seems to be an unpublished patch for this issue: SUPEE-7136.

File:
app/code/core/Mage/Core/Model/Session/Abstract/Varien.php

Replace this:

        if (Mage::app()->getFrontController()->getRequest()->isSecure() && empty($cookieParams['secure'])) {
            // secure cookie check to prevent MITM attack
            $secureCookieName = $sessionName . '_cid';
            if (isset($_SESSION[self::SECURE_COOKIE_CHECK_KEY])
                && $_SESSION[self::SECURE_COOKIE_CHECK_KEY] !== md5($cookie->get($secureCookieName))
            ) {
                session_regenerate_id(false);
                $sessionHosts = $this->getSessionHosts();
                $currentCookieDomain = $cookie->getDomain();
                foreach (array_keys($sessionHosts) as $host) {
                    // Delete cookies with the same name for parent domains
                    if (strpos($currentCookieDomain, $host) > 0) {
                        $cookie->delete($this->getSessionName(), null, $host);
                    }
                }
                $_SESSION = array();
            }
            if (!isset($_SESSION[self::SECURE_COOKIE_CHECK_KEY])) {
                $checkId = Mage::helper('core')->getRandomString(16);
                $cookie->set($secureCookieName, $checkId, null, null, null, true);
                $_SESSION[self::SECURE_COOKIE_CHECK_KEY] = md5($checkId);
            }
        }

with this:

        if (Mage::app()->getFrontController()->getRequest()->isSecure() && empty($cookieParams['secure'])) {
            // secure cookie check to prevent MITM attack
            $secureCookieName = $sessionName . '_cid';
            if (isset($_SESSION[self::SECURE_COOKIE_CHECK_KEY])) {
                if ($_SESSION[self::SECURE_COOKIE_CHECK_KEY] !== md5($cookie->get($secureCookieName))) {
                    session_regenerate_id(false);
                    $sessionHosts = $this->getSessionHosts();
                    $currentCookieDomain = $cookie->getDomain();
                    foreach (array_keys($sessionHosts) as $host) {
                        // Delete cookies with the same name for parent domains
                        if (strpos($currentCookieDomain, $host) > 0) {
                            $cookie->delete($this->getSessionName(), null, $host);
                        }
                    }
                    $_SESSION = array();
                } else {
                    /**
                     * Renew secure cookie expiration time if secure id did not change
                     */
                    $cookie->renew($secureCookieName, null, null, null, true, null);
                }
            }
            if (!isset($_SESSION[self::SECURE_COOKIE_CHECK_KEY])) {
                $checkId = Mage::helper('core')->getRandomString(16);
                $cookie->set($secureCookieName, $checkId, null, null, null, true);
                $_SESSION[self::SECURE_COOKIE_CHECK_KEY] = md5($checkId);
            }
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants