Skip to content

Use constant-time comparison for the HMAC#59

Open
mpdude wants to merge 1 commit intothumbor:masterfrom
mpdude:patch-1
Open

Use constant-time comparison for the HMAC#59
mpdude wants to merge 1 commit intothumbor:masterfrom
mpdude:patch-1

Conversation

@mpdude
Copy link
Copy Markdown

@mpdude mpdude commented May 13, 2023

MACs have to be checked with a constant-time comparison function. Otherwise, they may be forged by timing attacks.

@guilhermef guilhermef closed this Jun 20, 2023
@guilhermef guilhermef reopened this Jun 20, 2023
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 100.0%. remained the same when pulling 2797f12 on mpdude:patch-1 into b41d171 on thumbor:master.

@github-actions
Copy link
Copy Markdown

This PR is stale because it has been open 45 days with no activity. Remove the stale label or add a comment, or this PR will be closed in 10 days. You can always re-open if you feel this is something we should still keep working on. Tag @heynemann for more information.

@github-actions github-actions bot added the Stale label Mar 15, 2026
@mpdude
Copy link
Copy Markdown
Author

mpdude commented Mar 15, 2026

Tho is still valid

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens URL signature (HMAC/MAC) validation by switching from regular equality to a constant-time comparison to mitigate timing attacks.

Changes:

  • Use a constant-time comparison function when validating URL signatures.
  • Add the necessary standard-library import to support the constant-time comparison.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +23 to 24
return secrets.compare_digest(url_signature, actual_signature)

Comment on lines 11 to +12
from six import text_type
import secrets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants