cloudyr / aws.signature

Amazon Web Services Request Signatures
https://cloud.r-project.org/package=aws.signature
31 stars 33 forks source link

signature_v4_auth only produces unsigned payload requests #59

Open s-u opened 4 years ago

s-u commented 4 years ago

Please specify whether your issue is about:

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.

jon-mago commented 4 years ago

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

jon-mago commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.