trinodb / aws-proxy

Proxy for S3
Apache License 2.0
7 stars 3 forks source link

UNSIGNED-PAYLOAD is not handled properly by the proxy #133

Closed vagaerg closed 1 month ago

vagaerg commented 1 month ago

The proxy disables "payload signing" if the content hash equals UNSIGNED-PAYLOAD: https://github.com/trinodb/aws-proxy/blob/a00969b9025a48d679425dd7065c08c9ebc17afa/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/signing/Signer.java#L136-L138

That method's Javadoc states:

Enables payload signing for all situations on clients built via this builder.

Payload signing is optional when chunked encoding is not used and requests are made against an HTTPS endpoint. Under these conditions the client will by default opt to not sign payloads to optimize performance. If this flag is set to true the client will instead always sign payloads.

Note: Payload signing can be expensive, particularly if transferring large payloads in a single chunk. Enabling this option will result in a performance penalty.

However, this appears to be messing with UNSIGNED-PAYLOAD requests as it does not appear to be designed for that use case. From the docs I imagine disabling payload signing may be resulting in UNSIGNED-PAYLOAD not being added to the canonical request, thus making signature verification fail.

In any case, I believe we can safely always enable payload signing, since we never recompute the hash of the payload and instead reuse the value of X-amz-content-sha256: https://github.com/trinodb/aws-proxy/blob/a00969b9025a48d679425dd7065c08c9ebc17afa/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/signing/Signer.java#L140-L146

vagaerg commented 1 month ago

Note: this behaviour where UNSIGNED-PAYLOAD requests fail to validate in the proxy was observed when using a Python boto3-generated unsigned upload request: https://github.com/vagaerg/aws-proxy-testing/blob/9ba255bb047cfd115a936a59ed7d6ebe168d3284/test_s3.py#L21-L27