acquia / http-hmac-spec

An HMAC message format for securing RESTful web APIs.
81 stars 14 forks source link

We should not be using MD5 (or SHA1 if possible) in any new code/spec #1

Closed pwolanin closed 9 years ago

pwolanin commented 9 years ago

This spec looks flawed since it's using MD5 as the body hash.

Any new spec should use SHA2 such as SHA256 or SHA512

AWS is probably using the content-md5 since it's the only standard http hash header, but there is not a good reason to mimic this, especially if you are not actually sending that HTTP header. IF you want to send the header, I'd also use a SHA2 hash of the body.

In addition, the algorithm spec is unclear - it suggests hash not HMAC. This should be HMAC-SHA-256 or HMAC-SHA-512

Finally, there is no nonce value specified in the spec.

cpliakas commented 9 years ago

Thanks for the feedback.

Re: the SHA2. I agree, @mimrock and @baliame provided similar feedback.

Re: the spec not being clear, I am all for improving this. It is unclear to me specifically where your feedback could be applied, so any details or a pull request would be appreciated.

Re: The nonce, given that a nonce isn't required o necessary for all implementations of this algorithm, it is expected to be in a custom header such as x-acquia-nonce. I don't see a good reason to enforce a nonce, but I think we should document an opinion and guide people towards understanding whee this might be useful.

This 1.0.0 version of the spec is actively being used both inside and outside of Acquia, so the changes to the hashing algorithm should be reflected in a 2.0.x version. I created a branch at https://github.com/acquia/http-hmac-spec/tree/2.0 where any pull requests or changes can be made against.

pwolanin commented 9 years ago

I would say every implementation must have a nonce even if the back-end doesn't currently track it.

The document as-is doesn't not constitute a spec in my mind since it's way to vague about the specific algorithms to use.

If we want to have spec, it should be very specific so it has value in being able to re-use code.

Also - there is nothing here about a HMAC of the response body. I think that symmetry is important (especially if SSL is not 100% required). acquiasearch module, and the Cloud master API certainly do that. acquia agent used to, not sure where things stand now - nspi weakened the algorithm in some ways.

cpliakas commented 9 years ago

There are a few meaty issues here, so I am going to close this thread in favor or separate ones. The topics I see are below:

Regarding the vagueness of the spec, I think we need to specifics in order to move forward, please feel free to post separate issues as those specifics are identified.

cpliakas commented 9 years ago

Closing as dup of #3, #4, and #5.