Skip to content
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

signature_v4_auth only produces unsigned payload requests #59

Open
1 of 3 tasks
s-u opened this issue May 27, 2020 · 5 comments
Open
1 of 3 tasks

signature_v4_auth only produces unsigned payload requests #59

s-u opened this issue May 27, 2020 · 5 comments

Comments

@s-u
Copy link
Member

s-u commented May 27, 2020

Please specify whether your issue is about:

  • a possible bug
  • a question about package functionality
  • a suggested code or documentation change, improvement to the code, or feature request

signature_v4_auth() produces unsigned payload requests, because it doesn't add x-amz-content-sha256 to the canonical headers (see https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html). It does compute the body hash which is needed, so it would be really easy for signature_v4_auth() to add an option to include x-amz-content-sha256 in the computation (e.g. signed_payload=TRUE). It is much harder for the caller to do it, because it would have to either call signature_v4_auth() twice (once to get the body hash and second time to actually sign it) or compute the body hash manually and add it to the explicit canonical headers.

Apparently some back-ends require signed payload (see cloudyr/aws.s3#362 ) and Amazon may, too, at some point so it would be good to add this.

@jon-mago
Copy link
Contributor

I'll look into this. Thanks for raising it!

@jon-mago
Copy link
Contributor

Also, looking at the code, wow. I feel like the canonical_headers needs some work in general. Lets just hope no-one ever wanted to send a filename in a body...

@jon-mago
Copy link
Contributor

Looking more into this, according to the docs the signed body is required (and certainly features in all of their examples). But we've not generated it so far. So I'm unsure whether I should change it to generate that by default.

@s-u
Copy link
Member Author

s-u commented May 29, 2020

It shouldn't hurt unless the caller has changed the body. But good question - it would be nice to have a general aws.* test suite so we could check if changes break other dependent packages ...

@jon-mago
Copy link
Contributor

jon-mago commented Jun 1, 2020

Okay, I've released a version which has the facility to generate that header. So it can at least be used by packages. When I have a better idea of how to test it, it can switch to default TRUE instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants