AWS S3 Signature v4 - Githubissues

shfx commented 9 years ago

This PR holds implementation for AWS S3 Signature v4 for each non-put requests. While main strength of Knox is streaming instead of buffering, I also reimplemented PUT stream to use REST Multipart Upload.

Instead of rewriting lib/auth.js module to match the specification, I added aws4 as a depedency and removed obsolete functions in lib/auth.js and coresponding tests from test/auth.test.js. One thing that I left in lib/auth.js was signQuery() mainly because aws4 library haven't got this implemented yet for some reason.

Despite my effort to keep the API intact I suggest bumping the major version number, because this patch might break some existing code integrations (e.g if somebody was using the auth module that is exported in lib/index.js)

Fixes #254

FYI: This PR does not contain any optimisation, just pure implementation of Signature v4

domenic commented 9 years ago

Thanks for working on this. However,

I also reimplemented PUT stream to use REST Multipart Upload.

please remove this part of the PR. As noted in https://github.com/LearnBoost/knox#multipart-upload we are leaving that for other packages.

shfx commented 9 years ago

I'm not 100% sure but AFAIK there's no way to PUT files on S3 using Signature v4 without knowing their length. All request Must be signed and if there's a body, body hash must be a part of the signature. Removing this part will break put(), putSteram() and putFile() while using Signature v4. Thats why I replaced request call for all put() related methods to multipart stream.

Correct me if im wrong.

domenic commented 9 years ago

Do you have to know just the Content-Length, or the entire body at once?

If you have to know the entire body at once, I wonder if maybe this is better for another library instead of Knox, hmm... will have to think on this more.

shfx commented 9 years ago

Wait. Just found this. So there is a way, we just need to overwrite x-amz-content-sha256 The problem is, that aws4 signing library doesn't support custom value for x-amz-content-sha256 header...

I'll try to fix it. Small patch on aws4 repo to allow overwrite x-amz-content-sha256 is a reasonable solution I guess. What do you think @domenic ?

domenic commented 9 years ago

@shfx I'm so sorry, but I didn't see the above comment until now! Given the many people clamoring for this feature in #254, I would love if we could get that patch accepted and get this working with streaming.

Also, am I misreading, or does that link also give us a way to avoid having to always set the Content-Length header?! That would be huuuuge for knox.

newenegue commented 9 years ago

Any updates on this PR? Would love to get things working for eu-central-1 soon!

shfx commented 9 years ago

I see @domenic and other people are still interested in this PR. I'll do my best to study what I wrote 6 months ago and try to prepare a valid PR. It might be tricky tho.

newenegue commented 8 years ago

Are there any updates for this PR? Lots of people still looking for a solution for the v4 signature.