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

Fix v2 signature verification #630

Merged
merged 8 commits into from
Jun 7, 2020
Merged

Fix v2 signature verification #630

merged 8 commits into from
Jun 7, 2020

Conversation

kherock
Copy link
Collaborator

@kherock kherock commented Jun 7, 2020

Originally I was under the impression that our test suite ran using v2 signatures for all requests, but they're actually defaulting to v4 signatures (and S3rver makes no attempt to verify those). Turning on v2 signatures makes a whopping 101/184 tests fail!

This PR fixes all v2 verification errors within the testing suite and resolves #513 (closes #526).

@kherock kherock changed the title Fix v2 signatures Fix v2 signature verification Jun 7, 2020
@kherock kherock force-pushed the fix-v2-signatures branch from 004a768 to 4b8a1db Compare June 7, 2020 07:26
@kherock kherock force-pushed the fix-v2-signatures branch from 4b8a1db to 7e38a25 Compare June 7, 2020 07:29
@kherock
Copy link
Collaborator Author

kherock commented Jun 7, 2020

@allan-simon v2 signature verification was actually quite broken and only functioned as expected for pre-signed URLs. I'm pretty confident #567 is fixed by this, but I haven't tested outside of the JavaScript AWS SDK.

If you have time to test against this branch, please let me know if it's still not working for you. Once v3.6.0 is published I'll assume it's fixed otherwise.

@allan-simon
Copy link
Contributor

@kherock thanks for the ping :) looking to the code I think it will fix it yes ! don't wait for me :)

@kherock kherock merged commit 47a7d4b into master Jun 7, 2020
@kherock kherock deleted the fix-v2-signatures branch June 7, 2020 17:06
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.

SignatureDoesNotMatch error when signing a request containing unicode characters
2 participants