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

Compare MACs in constant time #143

Merged
merged 1 commit into from Apr 30, 2022

Conversation

MatthiasValvekens
Copy link
Contributor

@MatthiasValvekens MatthiasValvekens commented Apr 30, 2022

This PR replaces MAC comparisons using bytes.__eq__ with calls to hmac.compare_digest in the decryption routines for v1, v3 and v4, since the PASETO spec requires MACs to be checked in constant time. The v2 handler delegates this check to decrypt_and_verify in PyCryptoDome, which uses a randomised MAC comparison strategy instead. As such, v2 didn't require any fixes.

Comments certainly welcome!

The PASETO spec requires MACs to be compared in constant time to combat
timing leaks.
This commit replaces MAC comparisons using `bytes.__eq__` with
calls to `hmac.compare_digest`, which is the standard library function
intended for that purpose.
@dajiaji
Copy link
Owner

dajiaji commented Apr 30, 2022

Thanks for the PR! I should have used hmac.compare_digest for MAC comparisons.

@codecov
Copy link

codecov bot commented Apr 30, 2022

Codecov Report

Merging #143 (5aec592) into main (f7f62a6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #143   +/-   ##
=======================================
  Coverage   98.75%   98.75%           
=======================================
  Files          14       14           
  Lines        1364     1365    +1     
=======================================
+ Hits         1347     1348    +1     
  Misses         17       17           
Impacted Files Coverage Δ
pyseto/versions/v1.py 98.75% <100.00%> (ø)
pyseto/versions/v3.py 97.43% <100.00%> (ø)
pyseto/versions/v4.py 98.03% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7f62a6...5aec592. Read the comment docs.

@dajiaji dajiaji merged commit daf3448 into dajiaji:main Apr 30, 2022
@MatthiasValvekens
Copy link
Contributor Author

Thanks for the quick turn-around time!

@MatthiasValvekens MatthiasValvekens deleted the bugfix/const-time-compare branch April 30, 2022 21:24
@dajiaji
Copy link
Owner

dajiaji commented Apr 30, 2022

@MatthiasValvekens I've released v1.6.8 including your contribution!

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.

None yet

2 participants