brendanhay / amazonka

A comprehensive Amazon Web Services SDK for Haskell.
https://amazonka.brendanhay.nz
Other
599 stars 227 forks source link

R2 Incompatibility #975

Closed chreekat closed 6 months ago

chreekat commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

I updated the repro, but see https://github.com/commercialhaskell/stack/issues/5473#issuecomment-1892110831 for why it's not so easy to swap between the repro and the patched test case.

endgame commented 6 months ago

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 commented 6 months ago

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