envoyproxy / envoy

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

AWS Sigv4 signer does not clear previously set headers #35075

Open shulin-sq opened 1 week ago

shulin-sq commented 1 week ago

Title: AWS Sigv4 signer does not clear previously set headers

Description:

It seems that the AWS Sigv4 signer does not set previously set headers that are used for sigv4 signing (eg authorization, X-Amz-Security-Token, etc)

https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/aws_request_signing_filter

I'm unsure what the ideal behavior here is. It seems like either the filter should clear the headers that it needs for sigv4 header, or guidance should be given in the docs on how people can optionally set this behavior.

the issue is that setting any of the sigv4 headers from the client side will invalidate the sigv4 signature. In addition, retries do not work well when the signer is used as an upstream filter.

Repro steps: This is particularly bad when used with retries + sigv4 signer as an upstream filter.

# set some aws creds
export AWS_ACCESS_KEY_ID=... etc

# run envoy
envoy -c envoy-retries.yaml

# run simple go service
go run simple_srv.go

# hit retry
curl -vvv localhost:10000/retry
*   Trying [::1]:10000...
* connect to ::1 port 10000 failed: Connection refused
*   Trying 127.0.0.1:10000...
* Connected to localhost (127.0.0.1) port 10000
> GET /retry HTTP/1.1
> Host: localhost:10000
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/1.1 502 Bad Gateway
< date: Sat, 06 Jul 2024 00:13:30 GMT
< content-type: text/plain; charset=utf-8
< x-envoy-attempt-count: 3
< x-envoy-upstream-service-time: 35
< server: envoy
< transfer-encoding: chunked
<
retry me
X-Amz-Region-Set: *
X-Amz-Region-Set: *
X-Amz-Region-Set: *
Accept: */*
X-Forwarded-Proto: http
X-Envoy-Expected-Rq-Timeout-Ms: 14967
Authorization: AWS4-ECDSA-P256-SHA256 Credential=ASIATVTSGYECX674RMVT/20240706/vpc-lattice-svcs/aws4_request, SignedHeaders=accept;host;user-agent;x-amz-content-sha256;x-amz-date;x-amz-region-set;x-amz-security-token;x-request-id, Signature=redacted,AWS4-ECDSA-P256-SHA256 Credential=ASIATVTSGYECX674RMVT/20240706/vpc-lattice-svcs/aws4_request, SignedHeaders=accept;authorization;host;user-agent;x-amz-content-sha256;x-amz-date;x-amz-region-set;x-amz-security-token;x-request-id, Signature=redacted,AWS4-ECDSA-P256-SHA256 Credential=ASIATVTSGYECX674RMVT/20240706/vpc-lattice-svcs/aws4_request, SignedHeaders=accept;authorization;host;user-agent;x-amz-content-sha256;x-amz-date;x-amz-region-set;x-amz-security-token;x-request-id, Signature=redacted
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
User-Agent: curl/8.4.0
X-Request-Id: 98a420c5-0bdc-4952-9e76-a82f1bfe5f73
X-Amz-Security-Token: some_string_3
X-Amz-Security-Token: some_string_2
X-Amz-Security-Token: some_string_1
X-Amz-Date: 20240706T001300Z
X-Amz-Date: 20240706T001300Z
X-Amz-Date: 20240706T001300Z

You can also do a simpler repro by just sending the authorization header separately. You'll see in this example that my "test" string is now part of the authorization header.

curl -vvv localhost:10000/headers -H "authorization:test"
*   Trying [::1]:10000...
* connect to ::1 port 10000 failed: Connection refused
*   Trying 127.0.0.1:10000...
* Connected to localhost (127.0.0.1) port 10000
> GET /headers HTTP/1.1
> Host: localhost:10000
> User-Agent: curl/8.4.0
> Accept: */*
> authorization:test
>
< HTTP/1.1 200 OK
< date: Sat, 06 Jul 2024 00:19:22 GMT
< content-length: 1398
< content-type: text/plain; charset=utf-8
< x-envoy-attempt-count: 1
< x-envoy-upstream-service-time: 1
< server: envoy
<
X-Request-Id: c53227da-4d9b-42f4-b457-aab90f01f8b8
X-Envoy-Expected-Rq-Timeout-Ms: 15000
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20240706T001900Z
X-Amz-Region-Set: *
User-Agent: curl/8.4.0
Accept: */*
Authorization: test,AWS4-ECDSA-P256-SHA256 Credential=ASIATVTSGYECX674RMVT/20240706/vpc-lattice-svcs/aws4_request, SignedHeaders=accept;authorization;host;user-agent;x-amz-content-sha256;x-amz-date;x-amz-region-set;x-amz-security-token;x-request-id, Signature=redacted
X-Forwarded-Proto: http
X-Amz-Security-Token: redacted
* Connection #0 to host localhost left intact
htuch commented 1 week ago

@nbaws

nbaws commented 1 week ago

Thank you for your bug report! This issue will be addressed via https://github.com/envoyproxy/envoy/pull/35082 , which explicitly replaces (rather than appends) existing headers, and explicitly removes those headers which are going to be used in the signing process. Documentation for this extension has also been updated to describe the behaviour of header modification/removal.

nbaws commented 1 week ago

This PR has been merged, so you are free to test against latest dev build. Please let me know if you see any issues.