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

fix: X-Twilio-Signature validation when URL query parameters contain @ or : #621

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

adamj9431
Copy link
Contributor

@adamj9431 adamj9431 commented Oct 21, 2020

  • I acknowledge that all my contributions will be made under the project's license

X-Twilio-Signature validation appears to be broken in cases when Twilio sends query parameters containing @ or :. This affects SIP calls since GET calls (e.g. audio URLs) end up containing query parameters that look like ...&Caller=sip%3A1234567890%40209.182.172.181&...

It seems the signature header is calculated on the Twilio side without percent-escaping @ or :, but the actual request is then made by Twilio's infrastructure with those characters escaped.

This PR fixes the issue for me, but the other (potentially more robust) fix is to ensure that Twilio infrastructure calculates the signature on the actual query string that is sent to the server after any escaping has occurred.

Note that, per RFC 3986 @ and : do not need to be escaped in the query portion of the URI. They do need to be escaped in other parts of the URI, though. Go figure.

@thinkingserious
Copy link
Contributor

Hello @adamj9431,

Thank you for taking the time to share your fix! Could you please check the box next to "I acknowledge that all my contributions will be made under the project's license" that I've just added to the description?

With best regards,

Elmer

@thinkingserious thinkingserious added status: cla not signed status: code review request requesting a community code review or review from Twilio status: waiting for feedback waiting for feedback from the submitter labels Oct 23, 2020
@childish-sambino childish-sambino changed the title Fix: X-Twilio-Signature validation when URL query parameters contain @ or : fix: X-Twilio-Signature validation when URL query parameters contain @ or : Oct 23, 2020
@adamj9431
Copy link
Contributor Author

@thinkingserious done

@childish-sambino childish-sambino removed status: cla not signed status: waiting for feedback waiting for feedback from the submitter labels Nov 5, 2020
@childish-sambino childish-sambino merged commit 528fc25 into twilio:main Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants