Implement more robust nonce handling in Digest authentication.

Shore up some of the implementation gaps we have around nonces. Nonces
should expire and should be generated by the server. I've followed the
Symfony approach to generating nonces which results in short lived (5
minutes by default) nonces that are regenerated each time the client
receives a 401.

By validating and requiring nonce rotation we can avoid replay attacks
with our Digest authentication implementation.

I've targetted 3.next as validating/requiring nonce rotation is
a behavior change that I feel could trip up application developers. By
including it in a minor release we can more effectively communicate the
changes.

Refs #9668

src/Auth/DigestAuthenticate.php

@@ -15,6 +15,8 @@
 namespace Cake\Auth;
 
 use Cake\Controller\ComponentRegistry;
+use Cake\Core\Configure;
+use Cake\Network\Exception\UnauthorizedException;
 use Cake\Network\Request;
 
 /**
@@ -69,11 +71,12 @@ class DigestAuthenticate extends BasicAuthenticate
      * Besides the keys specified in BaseAuthenticate::$_defaultConfig,
      * DigestAuthenticate uses the following extra keys:
      *
+     * - `secret` The secret to use for nonce validation. Defaults to Security.salt.
      * - `realm` The realm authentication is for, Defaults to the servername.
-     * - `nonce` A nonce used for authentication. Defaults to `uniqid()`.
      * - `qop` Defaults to 'auth', no other values are supported at this time.
      * - `opaque` A string that must be returned unchanged by clients.
      *    Defaults to `md5($config['realm'])`
+     * - `nonceExpires` The number of seconds that nonces are valid for. Defaults to 300.
      *
      * @param \Cake\Controller\ComponentRegistry $registry The Component registry
      *   used on this request.
@@ -84,9 +87,10 @@ class DigestAuthenticate extends BasicAuthenticate
         $this->_registry = $registry;
 
         $this->config([
+            'nonceExpires' => 300,
+            'secret' => Configure::read('Security.salt'),
             'realm' => null,
             'qop' => 'auth',
-            'nonce' => uniqid(''),
             'opaque' => null,
         ]);
 
@@ -111,6 +115,10 @@ class DigestAuthenticate extends BasicAuthenticate
             return false;
         }
 
+        if (!$this->validNonce($digest['nonce'])) {
+            return false;
+        }
+
         $field = $this->_config['fields']['password'];
         $password = $user[$field];
         unset($user[$field]);
@@ -215,15 +223,65 @@ class DigestAuthenticate extends BasicAuthenticate
         $options = [
             'realm' => $realm,
             'qop' => $this->_config['qop'],
-            'nonce' => $this->_config['nonce'],
+            'nonce' => $this->generateNonce(),
             'opaque' => $this->_config['opaque'] ?: md5($realm)
         ];
 
+        $digest = $this->_getDigest($request);
+        if ($digest && isset($digest['nonce'])) {
+            if (!$this->validNonce($digest['nonce'])) {
+                $options['stale'] = true;
+            }
+        }
+
         $opts = [];
         foreach ($options as $k => $v) {
-            $opts[] = sprintf('%s="%s"', $k, $v);
+            if (is_bool($v)) {
+                $v = $v ? 'true' : 'false';
+                $opts[] = sprintf('%s=%s', $k, $v);
+            } else {
+                $opts[] = sprintf('%s="%s"', $k, $v);
+            }
         }
 
         return 'WWW-Authenticate: Digest ' . implode(',', $opts);
     }
+
+    /**
+     * Generate a nonce value that is validated in future requests.
+     *
+     * @return string
+     */
+    protected function generateNonce()
+    {
+        $expiryTime = microtime(true) + $this->config('nonceExpires');
+        $signatureValue = md5($expiryTime . ':' . $this->config('secret'));
+        $nonceValue = $expiryTime . ':' . $signatureValue;
+
+        return base64_encode($nonceValue);
+    }
+
+    /**
+     * Check the nonce to ensure it is valid and not expired.
+     *
+     * @param string $nonce The nonce value to check.
+     * @return bool
+     */
+    protected function validNonce($nonce)
+    {
+        $value = base64_decode($nonce);
+        if ($value === false) {
+            return false;
+        }
+        $parts = explode(':', $value);
+        if (count($parts) !== 2) {
+            return false;
+        }
+        list($expires, $checksum) = $parts;
+        if ($expires < microtime(true)) {
+            return false;
+        }
+
+        return md5($expires . ':' . $this->config('secret')) === $checksum;
+    }
 }

tests/TestCase/Auth/DigestAuthenticateTest.php

@@ -18,6 +18,7 @@ namespace Cake\Test\TestCase\Auth;
 
 use Cake\Auth\DigestAuthenticate;
 use Cake\Controller\ComponentRegistry;
+use Cake\Core\Configure;
 use Cake\I18n\Time;
 use Cake\Network\Exception\UnauthorizedException;
 use Cake\Network\Request;
@@ -106,18 +107,17 @@ class DigestAuthenticateTest extends TestCase
         $request = new Request('posts/index');
         $request->addParams(['pass' => []]);
 
-        $digest = <<<DIGEST
-Digest username="incorrect_user",
-realm="localhost",
-nonce="123456",
-uri="/dir/index.html",
-qop=auth,
-nc=00000001,
-cnonce="0a4f113b",
-response="6629fae49393a05397450978507c4ef1",
-opaque="123abc"
-DIGEST;
-        $request->env('PHP_AUTH_DIGEST', $digest);
+        $data = [
+            'username' => 'incorrect_user',
+            'realm' => 'localhost',
+            'nonce' => $this->generateNonce(),
+            'uri' => '/dir/index.html',
+            'qop' => 'auth',
+            'nc' => 0000001,
+            'cnonce' => '0a4f113b'
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
 
         $this->auth->unauthenticated($request, $this->response);
     }
@@ -142,8 +142,97 @@ DIGEST;
 
         $this->assertNotEmpty($e);
 
-        $expected = ['WWW-Authenticate: Digest realm="localhost",qop="auth",nonce="123",opaque="123abc"'];
-        $this->assertEquals($expected, $e->responseHeader());
+        $header = $e->responseHeader()[0];
+        $this->assertRegexp(
+            '/^WWW\-Authenticate: Digest realm="localhost",qop="auth",nonce="[a-zA-Z0-9=]+",opaque="123abc"$/',
+            $e->responseHeader()[0]
+        );
+    }
+
+    /**
+     * test that challenge headers include stale when the nonce is stale
+     *
+     * @return void
+     */
+    public function testAuthenticateChallengeIncludesStaleAttributeOnStaleNonce()
+    {
+        $request = new Request([
+            'url' => 'posts/index',
+            'environment' => ['REQUEST_METHOD' => 'GET']
+        ]);
+        $request->addParams(['pass' => []]);
+        $data = [
+            'uri' => '/dir/index.html',
+            'nonce' => $this->generateNonce(null, 5, strtotime('-10 minutes')),
+            'nc' => 1,
+            'cnonce' => '123',
+            'qop' => 'auth',
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
+
+        try {
+            $this->auth->unauthenticated($request, $this->response);
+        } catch (UnauthorizedException $e) {
+        }
+        $this->assertNotEmpty($e);
+
+        $header = $e->responseHeader()[0];
+        $this->assertContains('stale=true', $header);
+    }
+
+    /**
+     * Test that authentication fails when a nonce is stale
+     *
+     * @return void
+     */
+    public function testAuthenticateFailsOnStaleNonce()
+    {
+        $request = new Request([
+            'url' => 'posts/index',
+            'environment' => ['REQUEST_METHOD' => 'GET']
+        ]);
+        $request->addParams(['pass' => []]);
+
+        $data = [
+            'uri' => '/dir/index.html',
+            'nonce' => $this->generateNonce(null, 5, strtotime('-10 minutes')),
+            'nc' => 1,
+            'cnonce' => '123',
+            'qop' => 'auth',
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
+        $result = $this->auth->authenticate($request, $this->response);
+        $this->assertFalse($result, 'Stale nonce should fail');
+    }
+
+    /**
+     * Test that nonces are required.
+     *
+     * @return void
+     */
+    public function testAuthenticateValidUsernamePasswordNoNonce()
+    {
+        $request = new Request([
+            'url' => 'posts/index',
+            'environment' => ['REQUEST_METHOD' => 'GET']
+        ]);
+        $request->addParams(['pass' => []]);
+
+        $data = [
+            'username' => 'mariano',
+            'realm' => 'localhos',
+            'uri' => '/dir/index.html',
+            'nonce' => '',
+            'nc' => 1,
+            'cnonce' => '123',
+            'qop' => 'auth',
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
+        $result = $this->auth->authenticate($request, $this->response);
+        $this->assertFalse($result, 'Empty nonce should fail');
     }
 
     /**
@@ -159,18 +248,15 @@ DIGEST;
         ]);
         $request->addParams(['pass' => []]);
 
-        $digest = <<<DIGEST
-Digest username="mariano",
-realm="localhost",
-nonce="123",
-uri="/dir/index.html",
-qop=auth,
-nc=1,
-cnonce="123",
-response="06b257a54befa2ddfb9bfa134224aa29",
-opaque="123abc"
-DIGEST;
-        $request->env('PHP_AUTH_DIGEST', $digest);
+        $data = [
+            'uri' => '/dir/index.html',
+            'nonce' => $this->generateNonce(),
+            'nc' => 1,
+            'cnonce' => '123',
+            'qop' => 'auth',
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
 
         $result = $this->auth->authenticate($request, $this->response);
         $expected = [
@@ -196,18 +282,16 @@ DIGEST;
         ]);
         $request->addParams(['pass' => []]);
 
-        $digest = <<<DIGEST
-Digest username="mariano",
-realm="localhost",
-nonce="123",
-uri="/dir/index.html",
-qop=auth,
-nc=1,
-cnonce="123",
-response="06b257a54befa2ddfb9bfa134224aa29",
-opaque="123abc"
-DIGEST;
-        $request->env('PHP_AUTH_DIGEST', $digest);
+        $data = [
+            'username' => 'mariano',
+            'uri' => '/dir/index.html',
+            'nonce' => $this->generateNonce(),
+            'nc' => 1,
+            'cnonce' => '123',
+            'qop' => 'auth',
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
 
         $result = $this->auth->authenticate($request, $this->response);
         $expected = [
@@ -235,19 +319,16 @@ DIGEST;
         ]);
         $request->addParams(['pass' => []]);
 
-        $digest = <<<DIGEST
-Digest username="mariano",
-realm="localhost",
-nonce="123",
-uri="/dir/index.html",
-qop=auth,
-nc=1,
-cnonce="123",
-response="6629fae49393a05397450978507c4ef1",
-opaque="123abc"
-DIGEST;
-        $request->env('PHP_AUTH_DIGEST', $digest);
-
+        $data = [
+            'username' => 'invalid',
+            'uri' => '/dir/index.html',
+            'nonce' => $this->generateNonce(),
+            'nc' => 1,
+            'cnonce' => '123',
+            'qop' => 'auth',
+        ];
+        $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
+        $request->env('PHP_AUTH_DIGEST', $this->digestHeader($data));
         $this->auth->unauthenticated($request, $this->response);
     }
 
@@ -263,11 +344,13 @@ DIGEST;
         ]);
         $this->auth = new DigestAuthenticate($this->Collection, [
             'realm' => 'localhost',
-            'nonce' => '123'
         ]);
-        $expected = 'WWW-Authenticate: Digest realm="localhost",qop="auth",nonce="123",opaque="421aa90e079fa326b6494f812ad13e79"';
         $result = $this->auth->loginHeaders($request);
-        $this->assertEquals($expected, $result);
+
+        $this->assertRegexp(
+            '/^WWW\-Authenticate: Digest realm="localhost",qop="auth",nonce="[a-zA-Z0-9=]+",opaque="[a-f0-9]+"$/',
+            $result
+        );
     }
 
     /**
@@ -374,4 +457,48 @@ DIGEST;
         $expected = md5('mark:localhost:password');
         $this->assertEquals($expected, $result);
     }
+
+    /**
+     * Generate a nonce for testing.
+     *
+     * @param string $secret The secret to use.
+     * @param int $expires Time to live
+     * @return string
+     */
+    protected function generateNonce($secret = null, $expires = 300, $time = null)
+    {
+        $secret = $secret ?: Configure::read('Security.salt');
+        $time = $time ?: microtime(true);
+        $expiryTime = $time + $expires;
+        $signatureValue = md5($expiryTime . ':' . $secret);
+        $nonceValue = $expiryTime . ':' . $signatureValue;
+        return base64_encode($nonceValue);
+    }
+
+    /**
+     * Create a digest header string from an array of data.
+     *
+     * @param array $data the data to convert into a header.
+     * @return string
+     */
+    protected function digestHeader($data)
+    {
+        $data += [
+            'username' => 'mariano',
+            'realm' => 'localhost',
+            'opaque' => '123abc'
+        ];
+        $digest = <<<DIGEST
+Digest username="mariano",
+realm="{$data['realm']}",
+nonce="{$data['nonce']}",
+uri="{$data['uri']}",
+qop={$data['qop']},
+nc={$data['nc']},
+cnonce="{$data['cnonce']}",
+response="{$data['response']}",
+opaque="{$data['opaque']}"
+DIGEST;
+        return $digest;
+    }
 }