envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.28k stars 4.69k forks source link

AwsRequestSigning is broken for S3 Paths that contain special characters #35023

Open j-gottfried opened 5 days ago

j-gottfried commented 5 days ago

AwsRequestSigning is broken for S3 object keys that contain special characters that need URL encoding (most notably whitespace and percent characters that are quite common in filenames). This triggers a signature mismatch and 403 responses from AWS.

Repro steps: Given a S3 object with object key test file (with space): Request against envoy: GET /test%20file

Canonical request as returned in the AWS error response:

GET
/test%20file

host:bucket.s3.us-west-2.amazonaws.com

Canonical request as printed in the debug output from Envoy:

GET
/test%2520file

host:bucket.s3.us-west-2.amazonaws.com

Issue: the S3 object key is already UrlEncoded in the request, but the envoy AwsRequestSigning implementation is quoting it yet another time resulting in a double-quoting and a 403 response from AWS as a result, see https://github.com/envoyproxy/envoy/blob/main/source/extensions/common/aws/utility.cc#L155-L208

htuch commented 5 days ago

@derekargueta @suniltheta @mattklein123 @marcomagdy @nbaws

nbaws commented 5 days ago

will grab this one

nbaws commented 4 days ago

@j-gottfried thank you for your bug report. I've confirmed that in the attached PR we now correctly avoid double encoding and added test cases to match.

for s3 path "test folder/%"

% curl -v localhost:8099/test%20folder/%25

* Host localhost:8099 was resolved.
* IPv4: 127.0.0.1
*   Trying 127.0.0.1:8099...
* Connected to localhost (127.0.0.1) port 8099
> GET /test%20folder/%25 HTTP/1.1
> Host: localhost:8099
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/1.1 200 OK
< x-amz-request-id: xxxxx
< date: Wed, 03 Jul 2024 23:02:13 GMT
< last-modified: Wed, 03 Jul 2024 22:59:57 GMT
< etag: "xxxx"
< x-amz-server-side-encryption: AES256
< accept-ranges: bytes
< content-type: binary/octet-stream
< server: envoy
< content-length: 8
< x-envoy-upstream-service-time: 25
<
testing
* Connection #0 to host localhost left intact
j-gottfried commented 3 days ago

This is great. Thank you so much for the quick turnaround.