Session validation failure in Magento 1 EE v 1.14.3.x (and CE 1.9.3.x)

After some advanced debugging, session tracing and thinking about all that magic I was able to reproduce the problem and understand the reason for it. I prepared a little timing illustration, you can see it below.

problem time

  • red flag is the moment of user login and session creation
  • blue flag is the moment when user opens catalog page, let's assume it is a category page which is opened.
  • green flag is the moment where user submits the order (/sales/order/save/... request)

Here is how to reproduce:

  1. Before you start: Set your PHP session timeout and Magento cookie timeout both to be 1440 which is a default PHP value.
  2. Kill all your cookies or open the incognito tab.
  3. Go to your Magento shop and login (see Flag 1)
  4. Go through catalog and add some products to the cart (Flag 2)
  5. Go through the checkout and submit an order. Note the time when you did it. (Flag 3)
  6. Go through catalog and add some products to the cart (Flag 4)
  7. Keep refreshing your cart page or going through catalog pages so long that the timeout which you have configured for magento cookies expires (Flags 5-6). Note that time between Flag 7 and Flag 3 should be bigger than the cookie timeout.
  8. Go through the checkout and submit an order (Flag 7). The order submission will fail due to the exception described in my question above.

Reason:

There are certain sessions which are only instantiated on given requests, e.g. Mage_Rss_Model_Session is only instantiated during the actual checkout and not while browsing through catalog. At the same time session expiry timestamp is only set when session was instantiated. That means that if there was enough time between two checkouts and the session was not killed meanwhile (because user logged out or cookie expired) the new Magento code will consider that session as not passing validation and will throw an exception, which sounds somehow strange to me.

How to fix:

Well, I have few options:

  1. Wait till Magento reacts on that and reconsiders that code.
  2. Remove this code meanwhile.
  3. Try setting Magento cookie timeout to 0 if that is an option for you.

How did I figure it out:

  1. I started with adding the following to the original code of Mage_Core_Model_Session_Abstract_Varien

    Mage::log(
        sprintf(
            'useValidateSessionExpire fail "%s" "%d" "%d" "%s" "%s" "%s"',
            print_r($sessionData[self::VALIDATOR_SESSION_EXPIRE_TIMESTAMP], 1),
            time(),
            $this->_time,
            get_class($this),
            session_name(),
            session_id()
        ),
        Zend_Log::DEBUG,
        'session-validation.log',
        true
    );
    

    it gave me a good insight about affected classes and their correlation and how much session were expired. But that was not explaining why it happens and which user actions lead to the problem.

  2. Then I started thinking on how can I trace all changes to session data and came across this question https://superuser.com/questions/368231/automatic-versioning-upon-file-change-modify-create-delete I decided to give a try to git and incron combination, but after I implemented it and tested in sandbox, I realized that I will run out of disk space really fast in production.

  3. I decided to build a small PHP script which will decode session data and write logs for each sesssion. This script was called by incron

    <?php
    //log-session-data-change.php
    
    $sessionLogStoragePath = '/var/www/html/logged-session-storage/';
    
    $sessionFilePath = $argv[1];
    $sessionOperationType = $argv[2];
    $sessionFileName = basename($sessionFilePath);
    
    session_start();
    session_decode(file_get_contents($sessionFilePath));
    
    $logString = sprintf(
      '"%s","%s","%s",""' . PHP_EOL,
      date(DateTime::COOKIE),
      $sessionOperationType,
      $sessionFileName
    );
    
    if (file_exists($sessionFilePath)) {
      session_start();
      session_decode(file_get_contents($sessionFilePath));
    
      foreach ($_SESSION as $name => $data) {
        $value = '<empty>';
        if (isset($data['_session_validator_data']) && isset($data['_session_validator_data']['session_expire_timestamp'])) {
          $value = $data['_session_validator_data']['session_expire_timestamp'];
        }
        $logString .= sprintf(
          '"","","","%s","%s"' . PHP_EOL,
          $name,
          $value
        );
      }
    }
    
    file_put_contents($sessionLogStoragePath . $sessionFileName, $logString, FILE_APPEND);
    

    and here is the corresponding incrontab entry

    /var/www/html/magento-doc-root/var/session IN_MODIFY,IN_CREATE,IN_DELETE,IN_MOVE /usr/bin/php /var/www/html/log-session-data-change.php $@/$# $%
    

    sample output

    "Wednesday, 05-Apr-2017 18:09:06 CEST","IN_MODIFY","sess_94rfglnua0phncmp98hbr3k524",""
    "","","","core","1491408665"
    "","","","customer_base","1491408665"
    "","","","catalog","1491408665"
    "","","","checkout","1491408665"
    "","","","reports","1491408494"
    "","","","store_default","1491408665"
    "","","","rss","1491408524"
    "","","","admin","1491408524"
    

P.S.:

Current versions of both

skin/frontend/enterprise/default/js/opcheckout.js 
src/skin/frontend/base/default/js/opcheckout.js

are not able to handle the exception above during the AJAX request. They display literally nothing to the user, while user effectively gets logged out!

P.P.S.:

apparently Magento CE 1.9.3.x versions are also affected, see https://github.com/OpenMage/magento-mirror/blame/magento-1.9/app/code/core/Mage/Core/Model/Session/Abstract/Varien.php

P.P.P.S.:

When I said "Remove this code meanwhile." I meant excluding the following block

if ($this->useValidateSessionExpire()
    && isset($sessionData[self::VALIDATOR_SESSION_EXPIRE_TIMESTAMP])
    && $sessionData[self::VALIDATOR_SESSION_EXPIRE_TIMESTAMP] < time() ) {
    return false;
} else {
    $this->_data[self::VALIDATOR_KEY][self::VALIDATOR_SESSION_EXPIRE_TIMESTAMP]
        = $validatorData[self::VALIDATOR_SESSION_EXPIRE_TIMESTAMP];
}

you can do that with so many ways, including:

  1. Simply deleting that bit from the file
  2. Commenting it out
  3. Returning before it
  4. Making $this->useValidateSessionExpire() return true
  5. ...
  6. It is programming - be creative ;)

6. It is programming - be creative ;)

Another way to fix this (and improve session validation)

ColinM @ https://github.com/OpenMage/magento-lts

The session code currently stores the session validator data within every namespace and also validates it every time the namespace is inited. This is bad because:

  1. Extremely inefficient of session storage space. The validator data often comprises over 50% of the space used by a namespace and when there are many namespaces this adds up to a ton of waste. Session storage can be cut drastically with this patch and when using an in-memory storage like Redis or Memcached that matters a lot.
  2. Inefficient of compute cycles since multiple namespaces means multiple validations and there is no good reason for these to differ from each other.
  3. Actually creates bugs such as #394 where the validator data is updated on some requests but not others (so it can differ but shouldn't). I haven't tested but I believe this will also fix this issue.
diff --git a/app/code/core/Mage/Core/Model/Session/Abstract/Varien.php b/app/code/core/Mage/Core/Model/Session/Abstract/Varien.php
index 45d736543..ea6b464f1 100644
--- a/app/code/core/Mage/Core/Model/Session/Abstract/Varien.php
+++ b/app/code/core/Mage/Core/Model/Session/Abstract/Varien.php
@@ -35,6 +35,9 @@ class Mage_Core_Model_Session_Abstract_Varien extends Varien_Object
     const VALIDATOR_SESSION_EXPIRE_TIMESTAMP    = 'session_expire_timestamp';
     const SECURE_COOKIE_CHECK_KEY               = '_secure_cookie_check';

+    /** @var bool Flag true if session validator data has already been evaluated */
+    protected static $isValidated = FALSE;
+
     /**
      * Map of session enabled hosts
      * @example array('host.name' => true)
@@ -406,16 +409,21 @@ public function getValidateHttpUserAgentSkip()
     /**
      * Validate session
      *
-     * @param string $namespace
+     * @throws Mage_Core_Model_Session_Exception
      * @return Mage_Core_Model_Session_Abstract_Varien
      */
     public function validate()
     {
-        if (!isset($this->_data[self::VALIDATOR_KEY])) {
-            $this->_data[self::VALIDATOR_KEY] = $this->getValidatorData();
+        // Backwards compatibility with legacy sessions (validator data stored per-namespace)
+        if (isset($this->_data[self::VALIDATOR_KEY])) {
+            $_SESSION[self::VALIDATOR_KEY] = $this->_data[self::VALIDATOR_KEY];
+            unset($this->_data[self::VALIDATOR_KEY]);
+        }
+        if (!isset($_SESSION[self::VALIDATOR_KEY])) {
+            $_SESSION[self::VALIDATOR_KEY] = $this->getValidatorData();
         }
         else {
-            if (!$this->_validate()) {
+            if ( ! self::$isValidated && ! $this->_validate()) {
                 $this->getCookie()->delete(session_name());
                 // throw core session exception
                 throw new Mage_Core_Model_Session_Exception('');
@@ -432,8 +440,9 @@ public function validate()
      */
     protected function _validate()
     {
-        $sessionData = $this->_data[self::VALIDATOR_KEY];
+        $sessionData = $_SESSION[self::VALIDATOR_KEY];
         $validatorData = $this->getValidatorData();
+        self::$isValidated = TRUE; // Only validate once since the validator data is the same for every namespace

         if ($this->useValidateRemoteAddr()
                 && $sessionData[self::VALIDATOR_REMOTE_ADDR_KEY] != $validatorData[self::VALIDATOR_REMOTE_ADDR_KEY]) {
@@ -444,10 +453,8 @@ protected function _validate()
             return false;
         }

-        $sessionValidateHttpXForwardedForKey = $sessionData[self::VALIDATOR_HTTP_X_FORVARDED_FOR_KEY];
-        $validatorValidateHttpXForwardedForKey = $validatorData[self::VALIDATOR_HTTP_X_FORVARDED_FOR_KEY];
         if ($this->useValidateHttpXForwardedFor()
-            && $sessionValidateHttpXForwardedForKey != $validatorValidateHttpXForwardedForKey ) {
+                && $sessionData[self::VALIDATOR_HTTP_X_FORVARDED_FOR_KEY] != $validatorData[self::VALIDATOR_HTTP_X_FORVARDED_FOR_KEY]) {
             return false;
         }
         if ($this->useValidateHttpUserAgent()

Source: https://github.com/OpenMage/magento-lts/commit/de06e671c09b375605a956e100911396822e276a


Update:

Fix for web/session/use_http_x_forwarded_for option disabled option ... https://github.com/OpenMage/magento-lts/pull/457/commits/ec8128b4605e82406679c3cd81244ddf3878c379