aws.signature icon indicating copy to clipboard operation
aws.signature copied to clipboard

signature_v4_auth only produces unsigned payload requests

Open s-u opened this issue 4 years ago • 5 comments

Please specify whether your issue is about:

  • [ ] a possible bug
  • [ ] a question about package functionality
  • [X] 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 https://github.com/cloudyr/aws.s3/issues/362 ) and Amazon may, too, at some point so it would be good to add this.

s-u avatar May 27 '20 22:05 s-u

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

jon-mago avatar May 29 '20 12:05 jon-mago

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 avatar May 29 '20 12:05 jon-mago

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.

jon-mago avatar May 29 '20 13:05 jon-mago

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 ...

s-u avatar May 29 '20 22:05 s-u

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.

jon-mago avatar Jun 01 '20 11:06 jon-mago