Changeset 501 for trunk/lib/App.inc.php

Timestamp:

Nov 16, 2014 11:07:01 AM (10 years ago)

Author:

anonymous

Message:

Optimizing auth and csrf token.

File:

• trunk/lib/App.inc.php (modified) (6 diffs)

trunk/lib/App.inc.php

@@ -80 +80 @@
         'ssl_enabled' => false,
 
-        // Use CSRF tokens.
+        // Use CSRF tokens. See notes in the getCSRFToken() method.
         'csrf_token_enabled' => true,
+        // Form tokens will expire after this duration, in seconds.
+        'csrf_token_timeout' => 259200, // 259200 seconds = 3 days.
         'csrf_token_name' => 'csrf_token',
-        'csrf_token_timeout' => 86400, // In seconds. This causes form tokens to be unusable after this duration. This might only cause problems when opening forms in multiple tabs left open beyond the timeout duration. But usually their session will timeout first, and they'll receive new tokens when they load the form again..
 
         // HMAC signing method
@@ -1018 +1019 @@
         // This token can be validated upon form submission with $app->verifyCSRFToken() or $app->requireValidCSRFToken()
         if ($this->getParam('csrf_token_enabled') && $include_csrf_token) {
-            printf('<input type="hidden" name="%s" value="%s" />', $this->getParam('csrf_token_name'), $this->recycleCSRFToken());
+            printf('<input type="hidden" name="%s" value="%s" />', $this->getParam('csrf_token_name'), $this->getCSRFToken());
         }
     }
@@ -1041 +1042 @@
 
     /*
-    * Generate a csrf_token, saving it to the session and returning its value.
+    * Generate a csrf_token if it doesn't exist or is expired, save it to the session and return its value.
+    * Otherwise just return the current token.
+    * Details on the synchronizer token pattern:
     * https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#General_Recommendation:_Synchronizer_Token_Pattern
-    * @access   public
-    * @return   string  The new csrf_token.
-    * @author   Quinn Comendant <quinn@strangecode.com>
-    * @version  1.0
-    * @since    15 Nov 2014 17:53:51
-    */
-    public function generateCSRFToken()
-    {
-        return $_SESSION['_app'][$this->_ns]['csrf_tokens'][] = addSignature(time(), null, 64);
-    }
-
-    /*
-    * Update the csrf_token to a new value if it hasn't been set yet or has expired.
-    * Save the previous csrf_token in the session to ensure continuity of currently open sessions.
     *
     * @access   public
-    * @return   string The current csrf_token
+    * @return   string The new or current csrf_token
     * @author   Quinn Comendant <quinn@strangecode.com>
     * @version  1.0
     * @since    15 Nov 2014 17:57:17
     */
-    public function recycleCSRFToken()
-    {
-        if (!isset($_SESSION['_app'][$this->_ns]['csrf_tokens'])) {
-            // No token yet; generate one and return it.
-            $_SESSION['_app'][$this->_ns]['csrf_tokens'] = array();
-            $return = $this->generateCSRFToken();
-        }
-        if (removeSignature(end($_SESSION['_app'][$this->_ns]['csrf_tokens'])) + $this->getParam('csrf_token_timeout') < time()) {
-            // Newest token is expired; prune array of tokens and generate new token.
-            // We'll save the 10-most-recent tokens. This allows the user to submit up to 5 forms saved in previously opened tabs with expired tokens (loading a form will prune one token, and submitting a form will prune one token, thus 10 = 5).
-            $_SESSION['_app'][$this->_ns]['csrf_tokens'] = array_slice($_SESSION['_app'][$this->_ns]['csrf_tokens'], -10, 10);
-            $return = $this->generateCSRFToken();
+    public function getCSRFToken()
+    {
+        if (!isset($_SESSION['_app'][$this->_ns]['csrf_token']) || (removeSignature($_SESSION['_app'][$this->_ns]['csrf_token']) + $this->getParam('csrf_token_timeout') < time())) {
+            // No token, or token is expired; generate one and return it.
+            return $_SESSION['_app'][$this->_ns]['csrf_token'] = addSignature(time(), null, 64);
         }
         // Current token is not expired; return it.
-        $return = end($_SESSION['_app'][$this->_ns]['csrf_tokens']);
-        $app =& App::getInstance();
-        return $return;
+        return $_SESSION['_app'][$this->_ns]['csrf_token'];
     }
 
@@ -1093 +1073 @@
     * @since    15 Nov 2014 18:06:55
     */
-    public function verifyCSRFToken($csrf_token)
-    {
-        $app =& App::getInstance();
+    public function verifyCSRFToken($user_submitted_csrf_token)
+    {
 
         if (!$this->getParam('csrf_token_enabled')) {
-            $app->logMsg(sprintf('%s method called, but csrf_token_enabled=false', __FUNCTION__), LOG_ERR, __FILE__, __LINE__);
-            return false;
-        }
-        if ('' == trim($csrf_token)) {
-            $app->logMsg(sprintf('Empty string failed CSRF verification.', null), LOG_NOTICE, __FILE__, __LINE__);
-            return false;
-        }
-        if (!verifySignature($csrf_token, null, 64)) {
-            $app->logMsg(sprintf('Input failed CSRF verification (invalid signature in %s).', $csrf_token), LOG_WARNING, __FILE__, __LINE__);
-            return false;
-        }
-        $this->recycleCSRFToken();
-        if (!in_array($csrf_token, $_SESSION['_app'][$this->_ns]['csrf_tokens'])) {
-            $app->logMsg(sprintf('Input failed CSRF verification (%s not in %s).', $csrf_token, getDump($_SESSION['_app'][$this->_ns]['csrf_tokens'])), LOG_WARNING, __FILE__, __LINE__);
-            return false;
-        }
-        // $app->logMsg(sprintf('Verified token %s is in %s', $csrf_token, getDump($_SESSION['_app'][$this->_ns]['csrf_tokens'])), LOG_DEBUG, __FILE__, __LINE__);
+            $this->logMsg(sprintf('%s method called, but csrf_token_enabled=false', __FUNCTION__), LOG_ERR, __FILE__, __LINE__);
+            return true;
+        }
+        if ('' == trim($user_submitted_csrf_token)) {
+            $this->logMsg(sprintf('Empty string failed CSRF verification.', null), LOG_NOTICE, __FILE__, __LINE__);
+            return false;
+        }
+        if (!verifySignature($user_submitted_csrf_token, null, 64)) {
+            $this->logMsg(sprintf('Input failed CSRF verification (invalid signature in %s).', $user_submitted_csrf_token), LOG_WARNING, __FILE__, __LINE__);
+            return false;
+        }
+        $csrf_token = $this->getCSRFToken();
+        if ($user_submitted_csrf_token != $csrf_token) {
+            $this->logMsg(sprintf('Input failed CSRF verification (%s not in %s).', $user_submitted_csrf_token, $csrf_token), LOG_WARNING, __FILE__, __LINE__);
+            return false;
+        }
+        $this->logMsg(sprintf('Verified CSRF token %s is in %s', $user_submitted_csrf_token, $csrf_token), LOG_DEBUG, __FILE__, __LINE__);
         return true;
     }
@@ -1123 +1102 @@
     *
     * @access   public
-    * @param    string  $csrf_token The token to compare with the session token.
+    * @param    string  $user_submitted_csrf_token The user-submitted token to compare with the session token.
     * @param    string  $message    Optional message to display to the user (otherwise default message will display). Set to an empty string to display no message.
     * @param    int    $type    The type of message: MSG_NOTICE,
@@ -1134 +1113 @@
     * @since    15 Nov 2014 18:10:17
     */
-    public function requireValidCSRFToken($csrf_token, $message=null, $type=MSG_NOTICE, $file=null, $line=null)
-    {
-        $app =& App::getInstance();
-
-        if (!$this->verifyCSRFToken($csrf_token)) {
-            $message = isset($message) ? $message : _("Something went wrong; please try again.");
-            $app->raiseMsg($message, $type, $file, $line);
-            $app->dieBoomerangURL();
+    public function requireValidCSRFToken($message=null, $type=MSG_NOTICE, $file=null, $line=null)
+    {
+        if (!$this->verifyCSRFToken(getFormData($this->getParam('csrf_token_name')))) {
+            $message = isset($message) ? $message : _("Sorry, the form token expired. Please try again.");
+            $this->raiseMsg($message, $type, $file, $line);
+            $this->dieBoomerangURL();
         }
     }