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.

Sign up for GitHub

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

Jump to bottom

Include content-length in signed headers for signed url #38

Closed
vasco-santos opened this issue Nov 22, 2022 · 2 comments · Fixed by #66
Closed

Include content-length in signed headers for signed url #38

vasco-santos opened this issue Nov 22, 2022 · 2 comments · Fixed by #66
Assignees
@olizilla
Milestone
w3up phase 1

Comments

@vasco-santos
Copy link
Contributor

See storacha/w3up#266
This was referenced Nov 23, 2022
Closed
Closed
@olizilla olizilla self-assigned this Dec 5, 2022
@olizilla
Copy link
Contributor

olizilla commented Dec 5, 2022

I'm refactoring so we don't call the presigned url thing signer as that word is now heavily loaded. Also we have a bucket abstraction for car-store already and creating URLs for uploads to that bucket can live as part of that.

@olizilla
Copy link
Contributor

olizilla commented Dec 5, 2022

Also removing the SESSION_TOKEN as it doesn't seem relevant to our use-case where we are creating the signer in a lambda with the regular aws credentials

@olizilla olizilla mentioned this issue Dec 6, 2022
Merged
2 tasks
@olizilla olizilla added this to the w3up phase 1 milestone Dec 6, 2022
olizilla added a commit that referenced this issue Dec 8, 2022
@olizilla
refactor: move s3 url signing to car-store (#66)
2133ddc 
- Add `createUploadUrl` to `car-store.js` as creating presigned s3 urls
is a job for the bucket abstraction and having a thing called `signer`
that wasn't a keypair for a DID for ucanto was confusing.
- Switch to the `@aws-sdk/s3-request-presigner` for creation of signed
s3 urls to simplify credential management. The existing s3 client is
reused.
- Adds `content-length` to the list of headers that form part of the url
signature.

fixes #38 

### Notes

When presigning a URL for s3, some headers are "hoisted" to the
URLSearchParams; they appear in the query string so you can hand the url
off as a string, and the params needed for auth are baked right in
there. S3 support unpacking those params from either the query _or_ from
http request headers. The query params are part of the signature, so
they can't be tampered with.

You can also "hoist" and sign any other params you want... it is
tempting to move `x-amz-checksum-sha256` to the query so a user wouldn't
need to provide it separately. However it appears that aws does not
support pulling that value out of the query, only from a request header.

It is unknown how s3 reacts to sending a content-length that doesn't
match the payload size, but there are tests for it in
storacha/sigv4#7

License: MIT
Signed-off-by: Oli Evans <oli@protocol.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@olizilla olizilla

Labels
None yet
Projects
None yet
Milestone
w3up phase 1
Development

Successfully merging a pull request may close this issue.

refactor: move s3 url signing to car-store storacha/w3infra
2 participants
@olizilla @vasco-santos