mgomes / api_auth

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

Make HTTP Method a part of the canonical string #73

Closed fluxlife closed 8 years ago

fluxlife commented 9 years ago

I thought I have put this issue in before, but it appears that I haven't. I have been using this gem for a while now and appreciate all your work, however there is one part of this HMAC scheme that I feel is insecure and that is the absence of the HTTP Method in the canonical string. I hope I am wrong in this but I believe it possible for someone to do a man-in-the-middle attack and take a GET request, change to a DELETE request and still have the signature be valid on the server. If the signed string included the HTTP Method then that could be avoided.

I believe most of the clients you support allow you to access the HTTP method of the request (with curl being the exception from what I can tell, but again I may be wrong on that).

Having said that, is my assumption incorrect? Is there a specific reason the HTTP Verb is not included in the canonical string?

Thanks.

mgomes commented 9 years ago

I think you're right. The HTTP verb should be included in the canonical string. Amazon includes it in their canonical string: http://docs.aws.amazon.com/AmazonS3/latest/dev/RESTAuthentication.html

I will have to add this to each client and issue a minor release.

kjg commented 9 years ago

I think this change might warrant a major release because I believe this would be backwards incompatible.

kjg commented 9 years ago

It would also be nice if we gave people an "upgrade path" so they don't need to update their server and client at the exact same time to avoid errors.

mgomes commented 9 years ago

Agreed on the Major version.

I'll think about how we can securely achieve an "upgrade path" type of upgrade. Do you have any thoughts?

kjg commented 9 years ago

My thoughts are a minor, or bugfix release that doesn't add the verb to the requests from the client, but will verify requests on the server with or without the verb added to the string. And then a major release which adds the verb to the client requests and removes the check from the server side that allows requests without the verb in the string.

This would mean the upgrade path would be:

  1. Update the server side to the minor version update.
  2. Update the client side to the major version update.
  3. Update the server side to the major version update.
kjg commented 9 years ago

Also for the record, I did just confirm in a test app that you can convert a GET into a DELETE with the same signature and the request is successful.

mgomes commented 9 years ago

Thanks for the feedback and testing, @kjg! :+1:

arrowcircle commented 9 years ago

+1. http method should be included in signing process

fluxlife commented 8 years ago

Thanks guys for getting this implemented! I look forward to getting this update on all our systems :)