From 347dbefb342e2c11156ad8b93755b1dc847e023d Mon Sep 17 00:00:00 2001 From: Sean O'Brien Date: Thu, 18 Jun 2026 11:32:42 -0400 Subject: [PATCH] bugfix: sign content-type and x-amz-user-agent --- .changes/nextrelease/signed-headers.json | 7 +++ src/Signature/SignatureV4.php | 38 ++++++++++--- tests/Signature/SignatureV4Test.php | 72 ++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 .changes/nextrelease/signed-headers.json diff --git a/.changes/nextrelease/signed-headers.json b/.changes/nextrelease/signed-headers.json new file mode 100644 index 0000000000..69372c4d03 --- /dev/null +++ b/.changes/nextrelease/signed-headers.json @@ -0,0 +1,7 @@ +[ + { + "type": "bugfix", + "category": "Signature", + "description": "Updates signed headers to include `x-amz-user-agent` and `content-type`" + } +] diff --git a/src/Signature/SignatureV4.php b/src/Signature/SignatureV4.php index bb95b0556a..150d1ffcb1 100644 --- a/src/Signature/SignatureV4.php +++ b/src/Signature/SignatureV4.php @@ -4,7 +4,6 @@ use Aws\Credentials\CredentialsInterface; use AWS\CRT\Auth\Signable; use AWS\CRT\Auth\SignatureType; -use AWS\CRT\Auth\SignedBodyHeaderType; use AWS\CRT\Auth\Signing; use AWS\CRT\Auth\SigningAlgorithm; use AWS\CRT\Auth\SigningConfigAWS; @@ -49,7 +48,6 @@ protected function getHeaderBlacklist() { return [ 'cache-control' => true, - 'content-type' => true, 'content-length' => true, 'expect' => true, 'max-forwards' => true, @@ -67,13 +65,31 @@ protected function getHeaderBlacklist() 'from' => true, 'referer' => true, 'user-agent' => true, - 'X-Amz-User-Agent' => true, 'x-amzn-trace-id' => true, 'aws-sdk-invocation-id' => true, 'aws-sdk-retry' => true, ]; } + /** + * Headers that must be excluded from presigned URLs (in addition to + * the regular header blacklist). These headers are excluded because the + * consumer of the presigned URL cannot reliably reproduce them at the + * time the URL is used: + * - content-type: caller-determined at upload time + * - x-amz-user-agent: specific to the SDK that generated the URL + * + * @return array + */ + protected function getPresignHeaderDenyList() + { + return [ + 'content-type' => true, + 'x-amz-user-agent' => true, + ]; + } + + /** * @param string $service Service name to use when signing * @param string $region Region name to use when signing @@ -397,15 +413,16 @@ private function convertExpires($expiresTimestamp, $startTimestamp) private function moveHeadersToQuery(array $parsedRequest) { - //x-amz-user-agent shouldn't be put in a query param - unset($parsedRequest['headers']['X-Amz-User-Agent']); + $presignDenyList = $this->getPresignHeaderDenyList(); + $blacklist = $this->getHeaderBlacklist() + $presignDenyList; foreach ($parsedRequest['headers'] as $name => $header) { $lname = strtolower($name); - if (substr($lname, 0, 5) == 'x-amz') { + // Move x-amz-* headers into the query string, but skip those that + // must not appear in presigned URLs (e.g. x-amz-user-agent). + if (substr($lname, 0, 5) == 'x-amz' && !isset($presignDenyList[$lname])) { $parsedRequest['query'][$name] = $header; } - $blacklist = $this->getHeaderBlacklist(); if (isset($blacklist[$lname]) || $lname === strtolower(self::AMZ_CONTENT_SHA256_HEADER) ) { @@ -416,6 +433,7 @@ private function moveHeadersToQuery(array $parsedRequest) return $parsedRequest; } + private function parseRequest(RequestInterface $request) { // Clean up any previously set headers. @@ -479,7 +497,7 @@ private function removeIllegalV4aHeaders(&$request) 'aws-sdk-invocation-id', 'aws-sdk-retry', 'x-amz-region-set', - 'transfer-encoding' + 'transfer-encoding', ]; $storedHeaders = []; @@ -569,13 +587,15 @@ protected function presignWithV4a( ]); $this->removeIllegalV4aHeaders($request); - foreach ($this->getHeaderBlacklist() as $headerName => $headerValue) { + $denyList = $this->getHeaderBlacklist() + $this->getPresignHeaderDenyList(); + foreach ($denyList as $headerName => $headerValue) { if ($request->hasHeader($headerName)) { $request = $request->withoutHeader($headerName); } } $http_request = $this->CRTRequestFromGuzzleRequest($request); + Signing::signRequestAws( Signable::fromHttpRequest($http_request), $signingConfig, function ($signing_result, $error_code) use (&$http_request) { diff --git a/tests/Signature/SignatureV4Test.php b/tests/Signature/SignatureV4Test.php index d2196fb84d..4393a00fe5 100644 --- a/tests/Signature/SignatureV4Test.php +++ b/tests/Signature/SignatureV4Test.php @@ -319,6 +319,78 @@ public function testPresignBlacklistedHeaders() $this->assertStringNotContainsString('Content-Type', (string)$presigned->getUri()); } + /** + * Regression test for github.com/aws/aws-sdk-php/issues/3283. + * + * `content-type` should be included in SignedHeaders when present on a + * request that is signed (not presigned). Other AWS SDKs (botocore, + * Java, JS) sign `content-type` by default, so excluding it from the + * PHP SDK breaks cross-SDK signature verification. + */ + public function testSignsContentTypeHeader() + { + $sig = new SignatureV4('foo', 'bar'); + $creds = new Credentials('a', 'b'); + $req = new Request('POST', 'http://foo.com', [ + 'host' => 'foo.com', + 'Content-Type' => 'application/json', + ]); + $signed = $sig->signRequest($req, $creds); + $this->assertStringContainsString( + 'content-type', + $signed->getHeaderLine('Authorization') + ); + } + + /** + * Regression test for github.com/aws/aws-sdk-php/issues/3274. + * + * `x-amz-user-agent` should be included in SignedHeaders when present + * on a request that is signed (not presigned). It is an `x-amz-*` + * header and AWS services require all such headers to be signed. + */ + public function testSignsXAmzUserAgentHeader() + { + $sig = new SignatureV4('foo', 'bar'); + $creds = new Credentials('a', 'b'); + $req = new Request('POST', 'http://foo.com', [ + 'host' => 'foo.com', + 'X-Amz-User-Agent' => 'aws-sdk-php/3.x', + ]); + $signed = $sig->signRequest($req, $creds); + $this->assertStringContainsString( + 'x-amz-user-agent', + $signed->getHeaderLine('Authorization') + ); + } + + /** + * `content-type` and `x-amz-user-agent` must NOT be included in + * presigned URL SignedHeaders, since the consumer of the URL cannot + * be expected to reproduce these header values when invoking the URL. + */ + public function testPresignExcludesContentTypeAndUserAgent() + { + $sig = new SignatureV4('foo', 'bar'); + $creds = new Credentials('a', 'b'); + $req = new Request('PUT', 'http://foo.com', [ + 'host' => 'foo.com', + 'Content-Type' => 'application/json', + 'X-Amz-User-Agent' => 'aws-sdk-php/3.x', + 'x-amz-meta-foo' => 'bar', + ]); + $presigned = $sig->presign($req, $creds, '+5 minutes'); + $uri = (string)$presigned->getUri(); + $this->assertStringNotContainsString('content-type', $uri); + $this->assertStringNotContainsString('Content-Type', $uri); + $this->assertStringNotContainsString('user-agent', $uri); + $this->assertStringNotContainsString('User-Agent', $uri); + // x-amz-meta-foo is a normal x-amz-* header that should still be + // moved to query and signed. + $this->assertStringContainsString('x-amz-meta-foo', $uri); + } + + public function testCanonicalQuerySortingOccursAfterEncoding() { $_SERVER['aws_time'] = '20110909T233600Z';