[project @ Server-generated and one-way nonces patch from python openid]

10 files changed, 88 insertions, 231 deletions

diff --git a/Auth/OpenID/Consumer.php b/Auth/OpenID/Consumer.php
index 7ea75c7..f57a1bd 100644
--- a/Auth/OpenID/Consumer.php
+++ b/Auth/OpenID/Consumer.php
@@ -207,12 +207,6 @@ define('Auth_OpenID_SETUP_NEEDED', 'setup needed');
 define('Auth_OpenID_PARSE_ERROR', 'parse error');
 
 /**
- * This is the characters that the nonces are made from.
- */
-define('Auth_OpenID_DEFAULT_NONCE_CHRS',"abcdefghijklmnopqrstuvwxyz" .
-       "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789");
-
-/**
  * An OpenID consumer implementation that performs discovery and does
  * session management.  See the Consumer.php file documentation for
  * more information.
@@ -487,17 +481,6 @@ class Auth_OpenID_GenericConsumer {
     var $_use_assocs;
 
     /**
-     * This is the number of characters in the generated nonce for
-     * each transaction.
-     */
-    var $nonce_len = 8;
-
-    /**
-     * What characters are allowed in nonces
-     */
-    var $nonce_chrs = Auth_OpenID_DEFAULT_NONCE_CHRS;
-
-    /**
      * This method initializes a new {@link Auth_OpenID_Consumer}
      * instance to access the library.
      *
@@ -527,7 +510,7 @@ class Auth_OpenID_GenericConsumer {
 
     function begin($service_endpoint)
     {
-        $nonce = $this->_createNonce();
+        $nonce = Auth_OpenID_mkNonce();
         $assoc = $this->_getAssociation($service_endpoint->server_url);
         $r = new Auth_OpenID_AuthRequest($assoc, $service_endpoint);
         $r->return_to_args['nonce'] = $nonce;
@@ -546,7 +529,7 @@ class Auth_OpenID_GenericConsumer {
             return new Auth_OpenID_FailureResponse($endpoint, $error);
         } else if ($mode == 'id_res') {
             if ($endpoint->identity_url === null) {
-                return new Auth_OpenID_FailureResponse($identity_url,
+                return new Auth_OpenID_FailureResponse($endpoint,
                                                "No session state found");
             }
 
@@ -557,9 +540,8 @@ class Auth_OpenID_GenericConsumer {
                                                        "HTTP request failed");
             }
             if ($response->status == Auth_OpenID_SUCCESS) {
-                return $this->_checkNonce($response,
-                                          Auth_OpenID::arrayGet($query,
-                                                                'nonce'));
+                return $this->_checkNonce($endpoint->server_url,
+                                          $response);
             } else {
                 return $response;
             }
@@ -753,8 +735,9 @@ class Auth_OpenID_GenericConsumer {
     /**
      * @access private
      */
-    function _checkNonce($response, $nonce)
+    function _checkNonce($server_url, $response)
     {
+        $nonce = $response->getNonce();
         $parsed_url = parse_url($response->getReturnTo());
         $query_str = @$parsed_url['query'];
         $query = array();
@@ -780,7 +763,16 @@ class Auth_OpenID_GenericConsumer {
                                          $response->getReturnTo()));
         }
 
-        if (!$this->store->useNonce($nonce)) {
+        list($timestamp, $salt) = Auth_OpenID_splitNonce($nonce);
+
+        if (!($timestamp &&
+              $salt)) {
+            return new Auth_OpenID_FailureResponse($response,
+                                                   'Malformed nonce');
+        }
+
+        if (!$this->store->useNonce($endpoint->server_url,
+                                    $timestamp, $salt)) {
             return new Auth_OpenID_FailureResponse($response,
                                                    "Nonce missing from store");
         }
@@ -789,17 +781,6 @@ class Auth_OpenID_GenericConsumer {
     }
 
     /**
-     * @access private
-     */
-    function _createNonce()
-    {
-        $nonce = Auth_OpenID_CryptUtil::randomString($this->nonce_len,
-                                                     $this->nonce_chrs);
-        $this->store->storeNonce($nonce);
-        return $nonce;
-    }
-
-    /**
      * @access protected
      */
     function _createDiffieHellman()
@@ -1100,6 +1081,11 @@ class Auth_OpenID_SuccessResponse extends Auth_OpenID_ConsumerResponse {
     {
         return Auth_OpenID::arrayGet($this->signed_args, 'openid.return_to');
     }
+
+    function getNonce()
+    {
+        return Auth_OpenID::arrayGet($this->signed_args, 'openid.nonce');
+    }
 }
 
 /**
diff --git a/Auth/OpenID/DumbStore.php b/Auth/OpenID/DumbStore.php
index d4d8a8b..ce6e67f 100644
--- a/Auth/OpenID/DumbStore.php
+++ b/Auth/OpenID/DumbStore.php
@@ -79,18 +79,11 @@ class Auth_OpenID_DumbStore extends Auth_OpenID_OpenIDStore {
     }
 
     /**
-     * This implementation does nothing.
-     */
-    function storeNonce($nonce)
-    {
-    }
-
-    /**
      * In a system truly limited to dumb mode, nonces must all be
      * accepted. This therefore always returns true, which makes
      * replay attacks feasible.
      */
-    function useNonce($nonce)
+    function useNonce($server_url, $timestamp, $salt)
     {
         return true;
     }
diff --git a/Auth/OpenID/FileStore.php b/Auth/OpenID/FileStore.php
index 6ce8856..ea0165e 100644
--- a/Auth/OpenID/FileStore.php
+++ b/Auth/OpenID/FileStore.php
@@ -427,56 +427,36 @@ class Auth_OpenID_FileStore extends Auth_OpenID_OpenIDStore {
     }
 
     /**
-     * Mark this nonce as present.
-     */
-    function storeNonce($nonce)
-    {
-        if (!$this->active) {
-            trigger_error("FileStore no longer active", E_USER_ERROR);
-            return null;
-        }
-
-        $filename = $this->nonce_dir . DIRECTORY_SEPARATOR . $nonce;
-        $nonce_file = fopen($filename, 'w');
-        if ($nonce_file === false) {
-            return false;
-        }
-        fclose($nonce_file);
-        return true;
-    }
-
-    /**
      * Return whether this nonce is present. As a side effect, mark it
      * as no longer present.
      *
      * @return bool $present
      */
-    function useNonce($nonce)
+    function useNonce($server_url, $timestamp, $salt)
     {
         if (!$this->active) {
             trigger_error("FileStore no longer active", E_USER_ERROR);
             return null;
         }
 
-        $filename = $this->nonce_dir . DIRECTORY_SEPARATOR . $nonce;
-        $st = @stat($filename);
+        list($proto, $rest) = explode('://', $server_url, 2);
+        $parts = explode('/', $rest, 2);
+        $domain = $this->_filenameEscape($parts[0]);
+        $url_hash = $this->_safe64($server_url);
+        $salt_hash = $this->_safe64($salt);
 
-        if ($st === false) {
-            return false;
-        }
+        $filename = sprintf('%08x-%s-%s-%s-%s', $timestamp, $proto,
+                            $domain, $url_hash, $salt_hash);
+        $filename = $this->nonce_dir . DIRECTORY_SEPARATOR . $filename;
+
+        $result = @fopen($filename, 'x');
 
-        // Either it is too old or we are using it. Either way, we
-        // must remove the file.
-        if (!unlink($filename)) {
+        if ($result === false) {
             return false;
+        } else {
+            close($result);
+            return true;
         }
-
-        $now = time();
-        $nonce_age = $now - $st[9];
-
-        // We can us it if the age of the file is less than the
-        // expiration time.
-        return $nonce_age <= $this->max_nonce_age;
     }
 
     /**
@@ -495,15 +475,9 @@ class Auth_OpenID_FileStore extends Auth_OpenID_OpenIDStore {
 
         // Check all nonces for expiry
         foreach ($nonces as $nonce) {
-            $filename = $this->nonce_dir . DIRECTORY_SEPARATOR . $nonce;
-            $st = @stat($filename);
-
-            if ($st !== false) {
-                // Remove the nonce if it has expired
-                $nonce_age = $now - $st[9];
-                if ($nonce_age > $this->max_nonce_age) {
-                    Auth_OpenID_FileStore::_removeIfPresent($filename);
-                }
+            if (!Auth_OpenID_checkTimestamp($nonce, $now)) {
+                $filename = $this->nonce_dir . DIRECTORY_SEPARATOR . $nonce;
+                Auth_OpenID_FileStore::_removeIfPresent($filename);
             }
         }
 
diff --git a/Auth/OpenID/MySQLStore.php b/Auth/OpenID/MySQLStore.php
index f89afc6..44b5d67 100644
--- a/Auth/OpenID/MySQLStore.php
+++ b/Auth/OpenID/MySQLStore.php
@@ -23,8 +23,8 @@ class Auth_OpenID_MySQLStore extends Auth_OpenID_SQLStore {
     function setSQL()
     {
         $this->sql['nonce_table'] =
-            "CREATE TABLE %s (nonce CHAR(8) UNIQUE PRIMARY KEY, ".
-            "expires INTEGER) TYPE=InnoDB";
+            "CREATE TABLE %s (server_url VARCHAR(2047), timestamp INTEGER, ".
+            "salt CHAR(40), UNIQUE (server_url, timestamp, salt)";
 
         $this->sql['assoc_table'] =
             "CREATE TABLE %s (server_url BLOB, handle VARCHAR(255), ".
@@ -61,9 +61,6 @@ class Auth_OpenID_MySQLStore extends Auth_OpenID_SQLStore {
 
         $this->sql['get_nonce'] =
             "SELECT * FROM %s WHERE nonce = ?";
-
-        $this->sql['remove_nonce'] =
-            "DELETE FROM %s WHERE nonce = ?";
     }
 
     /**
diff --git a/Auth/OpenID/PostgreSQLStore.php b/Auth/OpenID/PostgreSQLStore.php
index a415280..a5baef1 100644
--- a/Auth/OpenID/PostgreSQLStore.php
+++ b/Auth/OpenID/PostgreSQLStore.php
@@ -23,8 +23,8 @@ class Auth_OpenID_PostgreSQLStore extends Auth_OpenID_SQLStore {
     function setSQL()
     {
         $this->sql['nonce_table'] =
-            "CREATE TABLE %s (nonce CHAR(8) UNIQUE PRIMARY KEY, ".
-            "expires INTEGER)";
+            "CREATE TABLE %s (server_url VARCHAR(2047), timestamp INTEGER, ".
+            "salt CHAR(40), UNIQUE (server_url, timestamp, salt)";
 
         $this->sql['assoc_table'] =
             "CREATE TABLE %s (server_url VARCHAR(2047), handle VARCHAR(255), ".
@@ -74,9 +74,6 @@ class Auth_OpenID_PostgreSQLStore extends Auth_OpenID_SQLStore {
 
         $this->sql['get_nonce'] =
             "SELECT * FROM %s WHERE nonce = ?";
-
-        $this->sql['remove_nonce'] =
-            "DELETE FROM %s WHERE nonce = ?";
     }
 
     /**
@@ -103,22 +100,6 @@ class Auth_OpenID_PostgreSQLStore extends Auth_OpenID_SQLStore {
     /**
      * @access private
      */
-    function _add_nonce($nonce, $expires)
-    {
-        if ($this->_get_nonce($nonce)) {
-            return $this->resultToBool($this->connection->query(
-                                      $this->sql['add_nonce']['update_nonce'],
-                                      array($expires, $nonce)));
-        } else {
-            return $this->resultToBool($this->connection->query(
-                                      $this->sql['add_nonce']['insert_nonce'],
-                                      array($nonce, $expires)));
-        }
-    }
-
-    /**
-     * @access private
-     */
     function blobEncode($blob)
     {
         return $this->_octify($blob);
diff --git a/Auth/OpenID/SQLStore.php b/Auth/OpenID/SQLStore.php
index c7bd540..4b4fceb 100644
--- a/Auth/OpenID/SQLStore.php
+++ b/Auth/OpenID/SQLStore.php
@@ -236,9 +236,6 @@ class Auth_OpenID_SQLStore extends Auth_OpenID_OpenIDStore {
                                    'get_assoc',
                                    'get_assocs',
                                    'remove_assoc',
-                                   'add_nonce',
-                                   'get_nonce',
-                                   'remove_nonce'
                                    );
 
         foreach ($required_sql_keys as $key) {
@@ -261,9 +258,7 @@ class Auth_OpenID_SQLStore extends Auth_OpenID_OpenIDStore {
                               array(
                                     'value' => $this->nonces_table_name,
                                     'keys' => array('nonce_table',
-                                                    'add_nonce',
-                                                    'get_nonce',
-                                                    'remove_nonce')
+                                                    'add_nonce')
                                     ),
                               array(
                                     'value' => $this->associations_table_name,
@@ -529,72 +524,18 @@ class Auth_OpenID_SQLStore extends Auth_OpenID_OpenIDStore {
     /**
      * @access private
      */
-    function _add_nonce($nonce, $expires)
+    function _add_nonce($server_url, $timestamp, $salt)
     {
         $sql = $this->sql['add_nonce'];
-        $result = $this->connection->query($sql, array($nonce, $expires));
+        $result = $this->connection->query($sql, array($server_url,
+                                                       $timestamp,
+                                                       $salt));
         return $this->resultToBool($result);
     }
 
-    /**
-     * @access private
-     */
-    function storeNonce($nonce)
-    {
-        if ($this->_add_nonce($nonce, time())) {
-            $this->connection->commit();
-        } else {
-            $this->connection->rollback();
-        }
-    }
-
-    /**
-     * @access private
-     */
-    function _get_nonce($nonce)
-    {
-        $result = $this->connection->getRow($this->sql['get_nonce'],
-                                            array($nonce));
-
-        if ($this->isError($result)) {
-            return null;
-        } else {
-            return $result;
-        }
-    }
-
-    /**
-     * @access private
-     */
-    function _remove_nonce($nonce)
+    function useNonce($server_url, $timestamp, $salt)
     {
-        $this->connection->query($this->sql['remove_nonce'],
-                                 array($nonce));
-    }
-
-    function useNonce($nonce)
-    {
-        $row = $this->_get_nonce($nonce);
-
-        if ($row !== null) {
-            $nonce = $row['nonce'];
-            $timestamp = $row['expires'];
-            $nonce_age = time() - $timestamp;
-
-            if ($nonce_age > $this->max_nonce_age) {
-                $present = 0;
-            } else {
-                $present = 1;
-            }
-
-            $this->_remove_nonce($nonce);
-        } else {
-            $present = 0;
-        }
-
-        $this->connection->commit();
-
-        return $present;
+        return $this->_add_nonce($server_url, $timestamp, $salt);
     }
 
     /**
diff --git a/Auth/OpenID/SQLiteStore.php b/Auth/OpenID/SQLiteStore.php
index 6351df3..168f3fa 100644
--- a/Auth/OpenID/SQLiteStore.php
+++ b/Auth/OpenID/SQLiteStore.php
@@ -20,8 +20,8 @@ class Auth_OpenID_SQLiteStore extends Auth_OpenID_SQLStore {
     function setSQL()
     {
         $this->sql['nonce_table'] =
-            "CREATE TABLE %s (nonce CHAR(8) UNIQUE PRIMARY KEY, ".
-            "expires INTEGER)";
+            "CREATE TABLE %s (server_url VARCHAR(2047), timestamp INTEGER, ".
+            "salt CHAR(40), UNIQUE (server_url, timestamp, salt)";
 
         $this->sql['assoc_table'] =
             "CREATE TABLE %s (server_url VARCHAR(2047), handle VARCHAR(255), ".
@@ -57,9 +57,6 @@ class Auth_OpenID_SQLiteStore extends Auth_OpenID_SQLStore {
 
         $this->sql['get_nonce'] =
             "SELECT * FROM %s WHERE nonce = ?";
-
-        $this->sql['remove_nonce'] =
-            "DELETE FROM %s WHERE nonce = ?";
     }
 }
 
diff --git a/Tests/Auth/OpenID/Consumer.php b/Tests/Auth/OpenID/Consumer.php
index f9e2772..9e71713 100644
--- a/Tests/Auth/OpenID/Consumer.php
+++ b/Tests/Auth/OpenID/Consumer.php
@@ -22,6 +22,7 @@ require_once 'Auth/OpenID/FileStore.php';
 require_once 'Auth/OpenID/KVForm.php';
 require_once 'Auth/OpenID/Consumer.php';
 require_once 'Auth/OpenID/Server.php';
+require_once 'Auth/OpenID/Nonce.php';
 require_once 'Tests/Auth/OpenID/MemStore.php';
 require_once 'PHPUnit.php';
 
@@ -349,18 +350,25 @@ class Tests_Auth_OpenID_Consumer_CheckNonceTest extends _TestIdRes {
     function setUp()
     {
         parent::setUp();
-        $this->nonce = "t3stn0nc3";
-        $this->store->storeNonce($this->nonce);
     }
 
-    function test_goodNonce()
+    function test_consumerNonce()
     {
         $this->return_to = sprintf('http://rt.unittest/?nonce=%s',
-                                   $this->nonce);
+                                   Auth_OpenID_mkNonce());
         $this->response = new Auth_OpenID_SuccessResponse($this->endpoint,
                                    array('openid.return_to' => $this->return_to));
 
-        $ret = $this->consumer->_checkNonce($this->response, $this->nonce);
+        $ret = $this->consumer->_checkNonce(null, $this->response);
+        $this->assertEquals($ret->status, Auth_OpenID_SUCCESS);
+        $this->assertEquals($ret->identity_url, $this->consumer_id);
+    }
+
+    function test_serverNonce()
+    {
+        $this->response = new Auth_OpenID_SuccessResponse($this->endpoint,
+                                 array('openid.nonce' => Auth_OpenID_mkNonce()));
+        $ret = $this->consumer->_checkNonce($this->server_url, $this->response);
         $this->assertEquals($ret->status, Auth_OpenID_SUCCESS);
         $this->assertEquals($ret->identity_url, $this->consumer_id);
     }
@@ -368,12 +376,13 @@ class Tests_Auth_OpenID_Consumer_CheckNonceTest extends _TestIdRes {
     function test_badNonce()
     {
         // remove the nonce from the store
-        $this->store->useNonce($this->nonce);
-        $this->return_to = sprintf('http://rt.unittest/?nonce=%s',
-                                   $this->nonce);
+        $nonce = Auth_OpenID_mkNonce();
+        list($timestamp, $salt) = Auth_OpenID_splitNonce($nonce);
+
+        $this->store->useNonce($this->server_url, $timestamp, $salt);
         $this->response = new Auth_OpenID_SuccessResponse($this->endpoint,
-                                 array('openid.return_to' => $this->return_to));
-        $ret = $this->consumer->_checkNonce($this->response, $this->nonce);
+                                 array('openid.nonce' => $nonce));
+        $ret = $this->consumer->_checkNonce($this->server_url, $this->response);
         $this->assertEquals($ret->status, Auth_OpenID_FAILURE);
         $this->assertEquals($ret->identity_url, $this->consumer_id);
         $this->assertTrue(strpos($ret->message, 'Nonce missing from store') === 0);
@@ -381,14 +390,12 @@ class Tests_Auth_OpenID_Consumer_CheckNonceTest extends _TestIdRes {
 
     function test_tamperedNonce()
     {
-        $this->return_to = sprintf('http://rt.unittest/?nonce=HACKED-%s',
-                                   $this->nonce);
         $this->response = new Auth_OpenID_SuccessResponse($this->endpoint,
-                                  array('openid.return_to' => $this->return_to));
-        $ret = $this->consumer->_checkNonce($this->response, $this->nonce);
+                                  array('openid.nonce' => 'malformed'));
+        $ret = $this->consumer->_checkNonce($this->server_url, $this->response);
         $this->assertEquals($ret->status, Auth_OpenID_FAILURE);
         $this->assertEquals($ret->identity_url, $this->consumer_id);
-        $this->assertTrue(strpos($ret->message, 'Nonce mismatch') === 0);
+        $this->assertTrue(strpos($ret->message, 'Malformed nonce') === 0);
     }
 
     function test_missingNonce()
@@ -396,7 +403,7 @@ class Tests_Auth_OpenID_Consumer_CheckNonceTest extends _TestIdRes {
         // no nonce parameter on the return_to
         $this->response = new Auth_OpenID_SuccessResponse($this->endpoint,
                                      array('openid.return_to' => $this->return_to));
-        $ret = $this->consumer->_checkNonce($this->response, $this->nonce);
+        $ret = $this->consumer->_checkNonce($this->server_url, $this->response);
         $this->assertEquals($ret->status, Auth_OpenID_FAILURE);
         $this->assertEquals($ret->identity_url, $this->consumer_id);
         $this->assertTrue(strpos($ret->message,
diff --git a/Tests/Auth/OpenID/MemStore.php b/Tests/Auth/OpenID/MemStore.php
index 660c268..f6c3593 100644
--- a/Tests/Auth/OpenID/MemStore.php
+++ b/Tests/Auth/OpenID/MemStore.php
@@ -80,23 +80,17 @@ class Tests_Auth_OpenID_MemStore extends Auth_OpenID_OpenIDStore {
         return $present;
     }
 
-    function storeNonce($nonce)
+    function useNonce($server_url, $timestamp, $salt)
     {
-        if (!in_array($nonce, $this->nonces)) {
+        $nonce =  sprintf("%s%s%s", $server_url, $timestamp, $salt);
+        if (in_array($nonce, $this->nonces)) {
+            return false;
+        } else {
             $this->nonces[] = $nonce;
+            return true;
         }
     }
 
-    function useNonce($nonce)
-    {
-        $index = array_search($nonce, $this->nonces);
-        $present = $index !== false;
-        if ($present) {
-            unset($this->nonces[$index]);
-        }
-        return $present;
-    }
-
     function reset()
     {
         $this->assocs = array();
diff --git a/Tests/Auth/OpenID/StoreTest.php b/Tests/Auth/OpenID/StoreTest.php
index 4487b17..2d4b91d 100644
--- a/Tests/Auth/OpenID/StoreTest.php
+++ b/Tests/Auth/OpenID/StoreTest.php
@@ -18,6 +18,7 @@
  */
 require_once 'Auth/OpenID/Association.php';
 require_once 'Auth/OpenID/CryptUtil.php';
+require_once 'Auth/OpenID/Nonce.php';
 require_once 'Auth/OpenID.php';
 require_once 'PHPUnit.php';
 
@@ -81,14 +82,6 @@ class Tests_Auth_OpenID_StoreTest extends PHPUnit_TestCase {
     }
 
     /**
-     * Generates a nonce value.
-     */
-    function generateNonce()
-    {
-        return Auth_OpenID_CryptUtil::randomString(8, $this->allowed_nonce);
-    }
-
-    /**
      * Generates an association with the specified parameters.
      */
     function genAssoc($now, $issued = 0, $lifetime = 600)
@@ -298,7 +291,8 @@ explicitly');
 
     function _checkUseNonce(&$store, $nonce, $expected, $msg=null)
     {
-        $actual = $store->useNonce($nonce);
+        list($stamp, $salt) = Auth_OpenID_splitNonce($nonce);
+        $actual = $store->useNonce($server_url, $stamp, $salt);
         $expected = $store->isDumb() || $expected;
         $val = ($actual && $expected) || (!$actual && !$expected);
         $this->assertTrue($val, "_checkUseNonce failed: $msg");
@@ -309,24 +303,17 @@ explicitly');
         // Nonce functions
 
         // Random nonce (not in store)
-        $nonce1 = $this->generateNonce();
+        $nonce1 = Auth_OpenID_mkNonce();
 
-        // A nonce is not present by default
-        $this->_checkUseNonce($store, $nonce1, false, 1);
+        // A nonce is not allowed by default
+        $this->_checkUseNonce($store, $nonce1, true, 1);
 
         // Storing once causes useNonce to return true the first, and
         // only the first, time it is called after the $store->
-        $store->storeNonce($nonce1);
-        $this->_checkUseNonce($store, $nonce1, true, 2);
         $this->_checkUseNonce($store, $nonce1, false, 3);
-        $this->_checkUseNonce($store, $nonce1, false, 4);
 
         // Storing twice has the same effect as storing once.
-        $store->storeNonce($nonce1);
-        $store->storeNonce($nonce1);
-        $this->_checkUseNonce($store, $nonce1, true, 5);
         $this->_checkUseNonce($store, $nonce1, false, 6);
-        $this->_checkUseNonce($store, $nonce1, false, 7);
 
         // Auth key functions