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

AWS S3 Signature v4 #273

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

AWS S3 Signature v4 #273

wants to merge 3 commits into from

Conversation

shfx
Copy link

@shfx shfx commented Mar 10, 2015

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
Copy link
Contributor

domenic commented Mar 11, 2015

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
Copy link
Author

shfx commented Mar 11, 2015

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
Copy link
Contributor

domenic commented Mar 11, 2015

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
Copy link
Author

shfx commented Mar 11, 2015

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
Copy link
Contributor

domenic commented Apr 7, 2015

@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
Copy link

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

@shfx
Copy link
Author

shfx commented Aug 13, 2015

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
Copy link

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

@slavafomin
Copy link

I would really love to see this problem solved = )

@antony
Copy link

antony commented Apr 12, 2016

Would be great if we could use Knox in Frankfurt and US-Central.

@namse
Copy link

namse commented May 3, 2016

still waiting...

@LukasBombach
Copy link

Wow it took a while to find this. So Knox does not work with Frankfurt. Would be nice to document this, I spent 2 days on this.

@LukasBombach
Copy link

+1 on fixing this

@delijah
Copy link

delijah commented Jun 21, 2016

+1

@harmenjanssen
Copy link

Not sure if it will make a difference, but +1!

@perhallstroem
Copy link

+1!

@subinsebastien
Copy link

Latest guy on the ship.

@PolGuixe
Copy link

+1

@PolGuixe
Copy link

alternative solutions for this?

@karellodewijk
Copy link

+1

@eltonk
Copy link

eltonk commented Feb 13, 2019

+1

@lemaxw
Copy link

lemaxw commented May 2, 2019

At 29/6/2019 amazon will stop use v2 signature:
https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingAWSSDK.html#UsingAWSSDK-sig2-deprecation
So starting this date knox will become unusable :(

@victornikitin
Copy link

Is any solution for this problem?

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

Successfully merging this pull request may close these issues.

Support for signature version 4