Closed afk11 closed 7 years ago
Thanks for the PR @afk11. The approach you've taken looks sound.
I think in the future we should switch to using hash_equals but that'd require a major version bump since we currently specify PHP >= 5.5. Something to keep in mind for our next major version bump.
Will merge this in a moment and push out a new release.
Might be worth a backport for a 2.0.5 release too. Don't think 1.x would be worth it.
@rbone hash_equals is good, it would mean there is no shortcutting early if the contents differ (the side channel) and I had used it first before considering compatibility.
Using it in addition to hashing is probably fine, since hashing seems to have been proposed in case a compiler optimizes out such a check. It isn't strictly necessary since by making the comparison non-deterministic the side-channel information gleaned from the (===) comparison terminating early isn't useful anymore.
It's no harm having it, but with hashing the === operator seems OK and would be a tad quicker (almost guaranteed to fail at the first few bits, just different onces each time)
Use a blinded HMAC comparison to verify signatures, which avoids timing side-channel's through use of ===. See https://paragonie.com/blog/2015/11/preventing-timing-attacks-on-string-comparison-with-double-hmac-strategy for more details
This PR introduces a dependency on
paragonie/random_compat
, which adds a polyfill forrandom_bytes
from PHP7. It doesn't affect PHP7+ installs, but is the recommended way to scramble for random data on older systems.Happy to submit some backports if required.