search

twilio / twilio-go

A Go package for communicating with the Twilio API.
MIT License
278 stars 40 forks source link

The request validator key sorting is broken #139

Closed djeer closed 1 year ago

djeer commented 2 years ago

Issue Summary

According to docs, in order to validate signature POST form, fields have to be sorted by keys (like in Python twilio library code). However, twilio-go uses "keyvalue" concatenated pairs in sorting which leads to incorrect sorting and invalid signature.

Here is an actual line of golang lib code that sorts keyvalue concatenated chunks https://github.com/twilio/twilio-go/blob/main/client/request_validator.go#L38 Python code that sorts by keys https://github.com/twilio/twilio-python/blob/main/twilio/request_validator.py#L76

Steps to Reproduce

  1. Trigger incoming twilio request with fields 
    ReasonConferenceEnded=participant-with-bla
Reason=Participant
  2. Golang code concats keyvalues and sorts prefix "ReasonC" before "ReasonP", says signature is invalid
  3. Python code sorts key "Reason" before key "ReasonConferenceEnded", says signature is valid

Code Snippet

Concatenated string before hashing looks like:

go
<matching string beginning>...ReasonConferenceEndedparticipant-with-end-conference-on-exit-leftReasonParticipant Cxxxxxxxxxxxxxxxxxx1 with endConferenceOnExit left the conferenceSequenceNumber6StatusCallbackEventconference-endTimestampWed, 16 Feb 2022 16:11:53 +0000

py
<matching string beginning>...ReasonParticipant Cxxxxxxxxxxxxxxxxxx1 with endConferenceOnExit left the conferenceReasonConferenceEndedparticipant-with-end-conference-on-exit-leftSequenceNumber6StatusCallbackEventconference-endTimestampWed, 16 Feb 2022 16:11:53 +0000

Exception/Log

signature invalid in (rv *RequestValidator) Validate()

Technical details:

djeer commented 2 years ago

My initial problem was that all conference-end requests had bad signature while all other requests had valid signature. Took quite a time to figure out root cause 😄

childish-sambino commented 2 years ago

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

childish-sambino commented 1 year ago

Fixed by #173