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

R2 Incompatibility #975

Closed
chreekat opened this issue Jan 15, 2024 · 4 comments · Fixed by #977
Closed

R2 Incompatibility #975

chreekat opened this issue Jan 15, 2024 · 4 comments · Fixed by #977

Comments

@chreekat
Copy link
Contributor

I am unable to send a PutObject to R2, Cloudflare's object storage. I get a SignatureDoesNotMatch error.

I eventually figured out a workaround: remove expect from the list of normalised headers.

While I consider this a bug in R2, it looks like the aws utility doesn't include expect in its signature, either. I don't know the security implications for removing it, so I'm not gonna open a PR yet. Any thoughts?

This is the workaround for 2.0:

commit 5f31f5b53f8daa41d1ea54fdd492e8ff2a2c4a65
Author: Bryan Richter <b@chreekat.net>
Date:   Mon Jan 15 10:48:35 2024 +0200

    Remove expect from normalised headers
    
    It appears this header is incompatible with R2, which is probably a bug
    in R2.

diff --git a/lib/amazonka-core/src/Amazonka/Sign/V4/Base.hs b/lib/amazonka-core/src/Amazonka/Sign/V4/Base.hs
index f1c22d81b8..faa8cc1fcb 100644
--- a/lib/amazonka-core/src/Amazonka/Sign/V4/Base.hs
+++ b/lib/amazonka-core/src/Amazonka/Sign/V4/Base.hs
@@ -308,4 +308,5 @@ normaliseHeaders =
     . Map.toAscList
     . Map.delete "authorization"
     . Map.delete "content-length"
+    . Map.delete "expect"
     . Map.fromListWith const

And similarly for 1.6.1:

commit 62295f17a728ca083ca4ba720559d9a5173bbb4f
Author: Bryan Richter <b@chreekat.net>
Date:   Mon Jan 15 10:48:35 2024 +0200

    Remove expect from normalised headers
    
    It appears this header is incompatible with R2, which is probably a bug
    in R2.

diff --git a/core/src/Network/AWS/Sign/V4/Base.hs b/core/src/Network/AWS/Sign/V4/Base.hs
index 8e0714444b..47d5758252 100644
--- a/core/src/Network/AWS/Sign/V4/Base.hs
+++ b/core/src/Network/AWS/Sign/V4/Base.hs
@@ -266,4 +266,5 @@ normaliseHeaders = Tag
     . map    (first CI.foldedCase)
     . nubBy  ((==)    `on` fst)
     . sortBy (compare `on` fst)
+    . filter ((/= "expect") . fst)
     . filter ((/= "authorization") . fst)
@chreekat
Copy link
Contributor Author

chreekat commented Jan 15, 2024

I have a repro here: https://github.com/chreekat/r2-repro/ . It is based on the package I was originally working on, hence the slightly funky stack.yaml.

I adapted it to work with amazonka-2.0, but now I seem to have hit a stack bug or something, so I need a bit of time to push that...

Oh, and I can provide credentials to a test R2 bucket via PM if that helps.

@chreekat
Copy link
Contributor Author

I updated the repro, but see commercialhaskell/stack#5473 (comment) for why it's not so easy to swap between the repro and the patched test case.

@endgame
Copy link
Collaborator

endgame commented Feb 20, 2024

The AWS SigV4 docs say:

For the purpose of calculating an authorization signature, only the host and any x-amz-* headers are required; however, in order to prevent data tampering, you should consider including all the headers in the signature calculation.

So I don't think there's anything stopping us from filtering out an expect: header. The fact that botocore blacklists it makes me even more confident that it's safe for us to drop it:

https://github.com/boto/botocore/blob/d5b2e4ab4bc4ad84f8e0e568e70ddc8ab7f094a8/botocore/auth.py#L61-L65

@chreekat
Copy link
Contributor Author

In that case I've opened #977 with my little patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants