search

mgomes / api_auth

HMAC authentication for Rails and HTTP Clients
MIT License
480 stars 147 forks source link

Change canonical string to remove Content-MD5 #176

Closed fwininger closed 2 years ago

fwininger commented 5 years ago

To improve the security of this gem, can we move forward and remove MD5 dependencies and use by default FIPS140-2 algorithms, may be SHA-256 ?

I propose to change default algorithm from SHA1 to SHA-256, any user can change our code with :digest => 'sha1' for backward compatibilities.

I propose to remove Content-MD5 header from the canonical string and replace with HASH(content). HASH is the same hash algorithme than the :digest unless of :content_digest is passed in parameter.

Before I implement a PR, @kjg @mgomes have you any suggestion ?

fwininger commented 5 years ago

Any body here ?

mgomes commented 5 years ago

@fwininger Sorry for the late response.

I think it makes sense to move on from MD5 due to its vulnerabilities. It seems like AWS and others have moved to SHA256 as well.

I would just add that I think we should also stop using the Content-MD5 HTTP header and instead switch to using X-Authorization-Content-SHA256. That seems to be what others have settled on: https://github.com/acquia/http-hmac-spec & AWS as well.

I think this, combined with your proposed change to the canonical string, would allow us to support additional hashing algorithms in the future as well without requiring a major version bump like this will.

Thanks for taking this on. 😄

fwininger commented 5 years ago

Ok, I will do a PR next week.