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

DynamoDB does not work with pre-signed URLs #94

Closed
cbroglie opened this issue Feb 8, 2015 · 11 comments · May be fixed by zencoder/aws-sdk-go#1
Closed

DynamoDB does not work with pre-signed URLs #94

cbroglie opened this issue Feb 8, 2015 · 11 comments · May be fixed by zencoder/aws-sdk-go#1

Comments

@cbroglie
Copy link

cbroglie commented Feb 8, 2015

The develop branch was modified to use pre-signed URLs rather than the Authorization header. With this format, I'm unable to authenticate with DynamoDB (I've actually only tried DynamoDB Local):

Request must contain either a valid (registered) AWS access key ID or X.509 certificate.

I checked AdRoll/goamz and the JS SDK, and both use the Authentication header rather than pre-signed URLs with DynamoDB. In particular, goamz only generates pre-signed URLs when the X-Amz-Expires header is set, and only the S3 signer leverages this.

My understanding of AWS auth is limited, but it seems that the pre-signing logic should only apply to S3, and other services should revert to using the Authentication header.

@lsegal
Copy link
Contributor

lsegal commented Feb 8, 2015

I'm unable to authenticate with DynamoDB (I've actually only tried DynamoDB Local):

Amazon DynamoDB itself works with the signing mechanism provided (we have integration tests for this case). My guess is DynamoDB Local does not (yet) support query string signing in V4, so I would recommend opening a thread on the DynamoDB Forums to point this out.

My understanding of AWS auth is limited, but it seems that the pre-signing logic should only apply to S3, and other services should revert to using the Authentication header.

Signature version 4 supports signing in the query string, also known as "GET request signing". The following is the official documentation, which explains how to build these signatures:

http://docs.aws.amazon.com/general/latest/gr/sigv4_signing.html

Note that the example uses IAM, not S3.

@cbroglie
Copy link
Author

cbroglie commented Feb 8, 2015

Ok, that makes sense. I started https://forums.aws.amazon.com/thread.jspa?threadID=171238.

I don't expect a fast turnaround time for DynamoDB Local supporting query string authentication, and it's critical to my development workflow, so I may add a new function to v4.go called something like SignWithHeader which uses the old Authentication header logic.

@lsegal
Copy link
Contributor

lsegal commented Feb 8, 2015

I wouldn't recommend overriding v4.go, as that would require a fork. You can do this without forking the SDK, simply register your own signer:

db := dynamodb.New(nil)
db.Handlers.Sign.Clear()
db.Handlers.Sign.PushBack(myv4signer.SignWithHeader)

// make request
r, e := db.ListTables(nil)
// ...

Where mysigner.SignWithHeader is your own v4 implementation that either constructs the signature yourself, or uses the existing v4.Sign method and simply rewrites the query string params back down to their respective headers. The latter would be much simpler to implement.

If you end up just rewriting the query string params, you can even drop the db.Handlers.Sign.Clear() and just piggy-back off of the existing v4 implementation (the request would have been built up by the previous handler) instead of calling that code yourself:

db := dynamodb.New(nil)
db.Handlers.Sign.PushBack(myv4signer.RewriteSignedURLToHeader)

@cbroglie
Copy link
Author

cbroglie commented Feb 9, 2015

I planned on doing something like:

db := dynamodb.New(nil)
db.Handlers.Sign.Clear()
db.Handlers.Sign.PushBack(v4.SignWithHeader)

So the functions in v4.go could be shared. But just adding an additional handler which sets the header based on the query string sounds better.

That approach would definitely let me implement it outside of the library code, but I would argue there is still some value in putting in the library as others will likely have the same use case. Maybe in a separate debug package?

cbroglie pushed a commit to cbroglie/aws-sdk-go that referenced this issue Feb 10, 2015
@cbroglie
Copy link
Author

I opened up PR #99 which adds a signer which works as discussed, and I confirmed it works with DynamoDB Local.

This doesn't need to live in the core library, but I think it will be useful to anyone using DynamoDB Local (or the equivalents for other services). Happy to move it elsewhere if there is another package structure which makes more sense.

@lsegal
Copy link
Contributor

lsegal commented Feb 10, 2015

I think if we were to support it within the library it would be part of v4.go (or the v4 package at least) for performance reasons. The suggestions to override / add a new handler were to get a user-land workaround going for your code, not necessarily the best implementation for the SDK itself.

That said, we are looking into how to best sign requests, so it may be that we have some kind of switch for header/query string signing. I'll leave the PR open for now as we plan the best way forward, but I would recommend using your plugin as a separate package in your own usage for now, rather than trying to maintain a fork.

@lsegal
Copy link
Contributor

lsegal commented Mar 14, 2015

Closing this since we backed out the presigned URL strategy in develop and requests should now be signed in "header mode" unless req.Presign() is used. This should be resolved.

@lsegal lsegal closed this as completed Mar 14, 2015
@cbroglie
Copy link
Author

Thanks

@bracki
Copy link

bracki commented Apr 21, 2015

Is this issue really solved?
I'm still getting the following using d67a47d:

T 127.0.0.1:63833 -> 127.0.0.1:8000 [AP]
POST /?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAAXXXXXXXXXXXX%2F20150421%2Feu-west-1%2Fdynamodb%2Faws4_request&X-Amz-Date=20150421T075501Z&X-Amz-Expires=300&X-Amz-SignedHeaders=host%3Bx-amz-target&X-Amz-Signature=xxxxxxxxxxxxxxxxxxxxxxx HTTP/1.1.
Host: localhost:8000.
User-Agent: aws-sdk-go/0.5.0.
Content-Length: 21.
Content-Type: application/x-amz-json-1.0.
X-Amz-Content-Sha256: ab92c2bbb935dcf9db6b76ea026a523873c16015c2a74250e9ed26b8c4c8b438.
X-Amz-Target: DynamoDB_20120810.DescribeTable.
Accept-Encoding: gzip.
.
{"TableName":"confd"}
##
T 127.0.0.1:8000 -> 127.0.0.1:63833 [AP]
HTTP/1.1 400 Bad Request.
Content-Type: application/x-amz-json-1.0.
x-amzn-RequestId: fd2582c8-9a5f-4b5d-8555-7b4739fa3afb.
Content-Length: 173.
Server: Jetty(8.1.12.v20130726).
.
{"__type":"com.amazonaws.dynamodb.v20120810#MissingAuthenticationToken","Message":"Request must contain either a valid (registered) AWS access key ID or X.509 certificate."}

This runs against a local DynamoDB. I assumed that authentication now should use header based authentication again.

bracki pushed a commit to bracki/confd that referenced this issue Apr 21, 2015
Especially because aws/aws-sdk-go#94 was fixed,
which prevents talking to DynamoDB local.
@lsegal
Copy link
Contributor

lsegal commented Apr 21, 2015

@bracki it should be resolved. Are you sure you've updated your dependency?

@bracki
Copy link

bracki commented Apr 21, 2015

Sorry my mistake. GOPATH/Godeps confusion à la carte.

-- Jan

Am 21.04.2015 um 15:02 schrieb Loren Segal [email protected]:

@bracki it should be resolved. Are you sure you've updated your dependency?


Reply to this email directly or view it on GitHub.

skotambkar pushed a commit to skotambkar/aws-sdk-go that referenced this issue May 20, 2021
Fixes the SDK's handling of a Pagination NextToken's value being an
empty string compared to a nil value. The SDK was expecting NextToken's
to always be unset (nil) and treating any non-nil value as a valid
value. This was not the case in MediaLive's List APIs. As those APIs
return a empty string value instead of null or not setting the field at
all.

This issue exists in both the v1 and v2 SDKs.

Fix aws#84
skotambkar pushed a commit to skotambkar/aws-sdk-go that referenced this issue May 20, 2021
Release v2.0.0-preview.2 (2018-01-15)
===

### Services
* Synced the V2 SDK with latests AWS service API definitions.

### SDK Bugs
* `service/s3/s3manager`: Fix Upload Manger's UploadInput fields ([aws#89](aws/aws-sdk-go-v2#89))
	* Fixes [aws#88](aws/aws-sdk-go-v2#88)
* `aws`: Fix Pagination handling of empty string NextToken ([aws#94](aws/aws-sdk-go-v2#94))
	* Fixes [aws#84](aws/aws-sdk-go-v2#84)
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 a pull request may close this issue.

3 participants