Use hmac for digest nonces #11103
Use hmac for digest nonces #11103
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11103 +/- ##
============================================
+ Coverage 94.88% 94.88% +<.01%
- Complexity 12841 12865 +24
============================================
Files 437 437
Lines 32742 32792 +50
============================================
+ Hits 31067 31116 +49
- Misses 1675 1676 +1
Continue to review full report at Codecov.
|
| { | ||
| $expiryTime = microtime(true) + $this->getConfig('nonceLifetime'); | ||
| $signatureValue = md5($expiryTime . ':' . $this->getConfig('secret')); | ||
| $signatureValue = hash_hmac('sha256', $expiryTime . ':' . $this->getConfig('secret'), Security::getSalt()); |
There was a problem hiding this comment.
Don't you have to change the validation of the nonce as well?
There was a problem hiding this comment.
yes it should, will take care of that
There was a problem hiding this comment.
There is also a generateNonce method in the test case that is still using md5() as well.
ceeram
force-pushed
the
digest-nonce-hmac
branch
from
a0a3dd1 to
82ac898
Compare
| $time = $time ?: microtime(true); | ||
| $expiryTime = $time + $expires; | ||
| $signatureValue = md5($expiryTime . ':' . $secret); | ||
| $signatureValue = hash_hmac('sha1', $expiryTime . ':' . $secret, $secret); |
There was a problem hiding this comment.
Is this just me, or shouldn't this be sha256? seems to be causing some trouble for me.
There was a problem hiding this comment.
Are you suggesting sha256 for the stronger algorithm?
There was a problem hiding this comment.
no, due to inconsistency with the validNonce function
There was a problem hiding this comment.
I totally missed that 😊. I'll get it fixed.
There was a problem hiding this comment.
great! Thanks. Had me pulling my hair out.
| if ($expires < microtime(true)) { | ||
| return false; | ||
| } | ||
| $check = hash_hmac('sha1', $expires . ':' . $this->getConfig('secret'), $this->getConfig('secret')); |
There was a problem hiding this comment.
Is this just me, or shouldn't this be sha256? seems to be causing some trouble for me.
The generation and validation sides of Digest authentication were using different algorithms which results in broken digest authentication. Refs #11103
Use hmac for digest nonces