Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid timing side channels in signature validation #28

Merged
merged 1 commit into from
Jan 18, 2017

Conversation

afk11
Copy link
Contributor

@afk11 afk11 commented Jan 18, 2017

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 for random_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.

…mparison to verify signatures - avoids side-channel through use of ===
@rbone
Copy link

rbone commented Jan 18, 2017

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.

@joho
Copy link

joho commented Jan 18, 2017

Might be worth a backport for a 2.0.5 release too. Don't think 1.x would be worth it.

@afk11
Copy link
Contributor Author

afk11 commented Jan 19, 2017

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants