-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Being able to choose the digest algorithm #98
Conversation
Can you help explain the motivation behind wanting to use a different digest algorithm? |
I believe that force the use of SHA-1 is a functional limitation. The last version of AWS api authentication use SHA-256 : According to the Google Chromium Team : |
I'm open to accepting something like this if we can do it in a way that make it obvious what is happening on the server and client side. What I don't want to happen is to have someone set their server up to use sha-256 and then forget to add that bit to their client (or vise versa). Maybe there can be an additional header that specifies what hashing algorithm was used to sign the request. |
If you merge the pull request with Rubocop, I work to add a header like AWS, ie : AWS4-HMAC-SHA256. |
I have add a optional HMAC_METHOD header, to ensure that Client and Server use the same method by default. |
Up ? |
Thanks so much for working on this. We should either follow Amazon's lead there and have the "key" in the Authorization header be something like APIAuth-HMAC-SHA256 or if we're going to use a separate header, it needs to be prfixed with an X- since HMAC-METHOD isn't a standard HTTP header. Another option is to do like OAuth 1a and have key value pairs within the content section of the Authoizaiton header and have a signature_method="SHA256" or something along those lines @mgomes @awendt any opinions on which method above we should use to specify the signature method here? |
@kjg I like following Amazon's lead and avoiding a second header. What's your preference? |
I agree. I like the way amazon does it. |
Hi guys. I'm aggre to follow the new AWS V4 standard with a single Authorisation Header.
I can implement that, as a v2 branch, because I think we will have several breaks with v1 branch. |
We don't necessarily need to follow everything they did. I like just On February 20, 2016 at 3:29:27 AM, florian wininger (
|
Ok, so with your suggestion for the first step, we use : |
Sounds good to me! Thanks! On February 20, 2016 at 7:50:16 AM, florian wininger (
|
@@ -67,7 +69,7 @@ def generate_secret_key | |||
|
|||
private | |||
|
|||
AUTH_HEADER_PATTERN = /APIAuth ([^:]+):(.+)$/ | |||
AUTH_HEADER_PATTERN = /APIAuth(-HMAC-(SHA[0-9]+))? ([^:]+):(.+)$/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's worth it, but you could make the outer (-HMAC) part non-capturing since we're not using it for anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, should we enforce only matching on SHA digests? I'd be okay with that I think but others are supported by OpenSSL::Digest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
f13f5f7
to
00dc7ed
Compare
Thanks so much for all this! I think we're there on the code. Can you please just update the README to reflect that authentic? Will detect the digest used from the Auth header. Do you think we should allow for it to still be overridden in authentic? I'm not sure if that's needed anymore, but I'm indifferent. If you think it should still be overridable in authentic? then just make it clear that it'll override what was detected in the header, and can make debugging harder or something |
52983aa
to
e2abb84
Compare
Hi, I prefer to keep the digest option in authentic to prevent downgrade attacks (like Poodle for SSLv3 vs TLS). In our case we want to support only SHA256 or newer. I have an ApiAuth::InvalidRequestDigest error to raise a exception if the Digest in the client and the server is not the same. |
Ah, that makes sense. Thanks so much for this. It looks great! Merging this now. I'm going to remove the code that allows for the canonical string without the http method, and I'll get a v2.0.0 out. |
Being able to choose the digest algorithm
Thanks. Let me know if you need some code review or some help for others features. |
Hi, could you consider that changes to be able to do HMAC-SHA256 ?
Thanks !