Skip to content

Commit

Permalink
Implement more robust nonce handling in Digest authentication.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markstory committed Oct 30, 2016
1 parent 318d7c1 commit c1680b2
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 58 deletions.
66 changes: 62 additions & 4 deletions src/Auth/DigestAuthenticate.php
Expand Up @@ -15,6 +15,8 @@
namespace Cake\Auth;

use Cake\Controller\ComponentRegistry;
use Cake\Core\Configure;
use Cake\Network\Exception\UnauthorizedException;
use Cake\Network\Request;

/**
Expand Down Expand Up @@ -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.
Expand All @@ -84,9 +87,10 @@ public function __construct(ComponentRegistry $registry, array $config = [])
$this->_registry = $registry;

$this->config([
'nonceExpires' => 300,
'secret' => Configure::read('Security.salt'),
'realm' => null,
'qop' => 'auth',
'nonce' => uniqid(''),
'opaque' => null,
]);

Expand All @@ -111,6 +115,10 @@ public function getUser(Request $request)
return false;
}

if (!$this->validNonce($digest['nonce'])) {
return false;
}

$field = $this->_config['fields']['password'];
$password = $user[$field];
unset($user[$field]);
Expand Down Expand Up @@ -215,15 +223,65 @@ public function loginHeaders(Request $request)
$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;
}
}

0 comments on commit c1680b2

Please sign in to comment.