opengovsg / postmangovsg

Templated message sending for public officers
https://postman.gov.sg
60 stars 16 forks source link

Fix/hash collisions #2212

Closed KishenKumarrrrr closed 1 year ago

KishenKumarrrrr commented 1 year ago

Context

The existing hashing implementation for Twilio secrets is problematic because bcrypt only uses the first 72 bytes of a string to generate the resultant hash. This resulted in a bug in the Postman application where different sets of Twilio secrets produced the exact same hash, causing the secrets to be overwritten due to hash collisions.

Approach

After the initial hash is generated from the user-submitted Twilio secrets, the hash is never subsequently used again to verify the authenticity of the credentials. The hash is only used to identify and fetch the particular set of credentials from AWS Secrets Manager.

Since the hash is only used for identification of the secrets, we can circumvent the hash collision problem by generating a uuid for the Twilio secrets instead.

Bug Fixes:

Tests

No new test were written for this change. Manual testing was performed in staging to verify the changes and old tests were updated.

Test updated: