Skip to content

Commit 8993199

Browse files
authored
bugfix: sign content-type and x-amz-user-agent (#3310)
1 parent cc71d2d commit 8993199

3 files changed

Lines changed: 108 additions & 9 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[
2+
{
3+
"type": "bugfix",
4+
"category": "Signature",
5+
"description": "Updates signed headers to include `x-amz-user-agent` and `content-type`"
6+
}
7+
]

src/Signature/SignatureV4.php

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
use Aws\Credentials\CredentialsInterface;
55
use AWS\CRT\Auth\Signable;
66
use AWS\CRT\Auth\SignatureType;
7-
use AWS\CRT\Auth\SignedBodyHeaderType;
87
use AWS\CRT\Auth\Signing;
98
use AWS\CRT\Auth\SigningAlgorithm;
109
use AWS\CRT\Auth\SigningConfigAWS;
@@ -49,7 +48,6 @@ protected function getHeaderBlacklist()
4948
{
5049
return [
5150
'cache-control' => true,
52-
'content-type' => true,
5351
'content-length' => true,
5452
'expect' => true,
5553
'max-forwards' => true,
@@ -67,13 +65,31 @@ protected function getHeaderBlacklist()
6765
'from' => true,
6866
'referer' => true,
6967
'user-agent' => true,
70-
'X-Amz-User-Agent' => true,
7168
'x-amzn-trace-id' => true,
7269
'aws-sdk-invocation-id' => true,
7370
'aws-sdk-retry' => true,
7471
];
7572
}
7673

74+
/**
75+
* Headers that must be excluded from presigned URLs (in addition to
76+
* the regular header blacklist). These headers are excluded because the
77+
* consumer of the presigned URL cannot reliably reproduce them at the
78+
* time the URL is used:
79+
* - content-type: caller-determined at upload time
80+
* - x-amz-user-agent: specific to the SDK that generated the URL
81+
*
82+
* @return array
83+
*/
84+
protected function getPresignHeaderDenyList()
85+
{
86+
return [
87+
'content-type' => true,
88+
'x-amz-user-agent' => true,
89+
];
90+
}
91+
92+
7793
/**
7894
* @param string $service Service name to use when signing
7995
* @param string $region Region name to use when signing
@@ -397,15 +413,16 @@ private function convertExpires($expiresTimestamp, $startTimestamp)
397413

398414
private function moveHeadersToQuery(array $parsedRequest)
399415
{
400-
//x-amz-user-agent shouldn't be put in a query param
401-
unset($parsedRequest['headers']['X-Amz-User-Agent']);
416+
$presignDenyList = $this->getPresignHeaderDenyList();
417+
$blacklist = $this->getHeaderBlacklist() + $presignDenyList;
402418

403419
foreach ($parsedRequest['headers'] as $name => $header) {
404420
$lname = strtolower($name);
405-
if (substr($lname, 0, 5) == 'x-amz') {
421+
// Move x-amz-* headers into the query string, but skip those that
422+
// must not appear in presigned URLs (e.g. x-amz-user-agent).
423+
if (substr($lname, 0, 5) == 'x-amz' && !isset($presignDenyList[$lname])) {
406424
$parsedRequest['query'][$name] = $header;
407425
}
408-
$blacklist = $this->getHeaderBlacklist();
409426
if (isset($blacklist[$lname])
410427
|| $lname === strtolower(self::AMZ_CONTENT_SHA256_HEADER)
411428
) {
@@ -416,6 +433,7 @@ private function moveHeadersToQuery(array $parsedRequest)
416433
return $parsedRequest;
417434
}
418435

436+
419437
private function parseRequest(RequestInterface $request)
420438
{
421439
// Clean up any previously set headers.
@@ -479,7 +497,7 @@ private function removeIllegalV4aHeaders(&$request)
479497
'aws-sdk-invocation-id',
480498
'aws-sdk-retry',
481499
'x-amz-region-set',
482-
'transfer-encoding'
500+
'transfer-encoding',
483501
];
484502
$storedHeaders = [];
485503

@@ -569,13 +587,15 @@ protected function presignWithV4a(
569587
]);
570588

571589
$this->removeIllegalV4aHeaders($request);
572-
foreach ($this->getHeaderBlacklist() as $headerName => $headerValue) {
590+
$denyList = $this->getHeaderBlacklist() + $this->getPresignHeaderDenyList();
591+
foreach ($denyList as $headerName => $headerValue) {
573592
if ($request->hasHeader($headerName)) {
574593
$request = $request->withoutHeader($headerName);
575594
}
576595
}
577596

578597
$http_request = $this->CRTRequestFromGuzzleRequest($request);
598+
579599
Signing::signRequestAws(
580600
Signable::fromHttpRequest($http_request),
581601
$signingConfig, function ($signing_result, $error_code) use (&$http_request) {

tests/Signature/SignatureV4Test.php

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,78 @@ public function testPresignBlacklistedHeaders()
319319
$this->assertStringNotContainsString('Content-Type', (string)$presigned->getUri());
320320
}
321321

322+
/**
323+
* Regression test for github.com/aws/aws-sdk-php/issues/3283.
324+
*
325+
* `content-type` should be included in SignedHeaders when present on a
326+
* request that is signed (not presigned). Other AWS SDKs (botocore,
327+
* Java, JS) sign `content-type` by default, so excluding it from the
328+
* PHP SDK breaks cross-SDK signature verification.
329+
*/
330+
public function testSignsContentTypeHeader()
331+
{
332+
$sig = new SignatureV4('foo', 'bar');
333+
$creds = new Credentials('a', 'b');
334+
$req = new Request('POST', 'http://foo.com', [
335+
'host' => 'foo.com',
336+
'Content-Type' => 'application/json',
337+
]);
338+
$signed = $sig->signRequest($req, $creds);
339+
$this->assertStringContainsString(
340+
'content-type',
341+
$signed->getHeaderLine('Authorization')
342+
);
343+
}
344+
345+
/**
346+
* Regression test for github.com/aws/aws-sdk-php/issues/3274.
347+
*
348+
* `x-amz-user-agent` should be included in SignedHeaders when present
349+
* on a request that is signed (not presigned). It is an `x-amz-*`
350+
* header and AWS services require all such headers to be signed.
351+
*/
352+
public function testSignsXAmzUserAgentHeader()
353+
{
354+
$sig = new SignatureV4('foo', 'bar');
355+
$creds = new Credentials('a', 'b');
356+
$req = new Request('POST', 'http://foo.com', [
357+
'host' => 'foo.com',
358+
'X-Amz-User-Agent' => 'aws-sdk-php/3.x',
359+
]);
360+
$signed = $sig->signRequest($req, $creds);
361+
$this->assertStringContainsString(
362+
'x-amz-user-agent',
363+
$signed->getHeaderLine('Authorization')
364+
);
365+
}
366+
367+
/**
368+
* `content-type` and `x-amz-user-agent` must NOT be included in
369+
* presigned URL SignedHeaders, since the consumer of the URL cannot
370+
* be expected to reproduce these header values when invoking the URL.
371+
*/
372+
public function testPresignExcludesContentTypeAndUserAgent()
373+
{
374+
$sig = new SignatureV4('foo', 'bar');
375+
$creds = new Credentials('a', 'b');
376+
$req = new Request('PUT', 'http://foo.com', [
377+
'host' => 'foo.com',
378+
'Content-Type' => 'application/json',
379+
'X-Amz-User-Agent' => 'aws-sdk-php/3.x',
380+
'x-amz-meta-foo' => 'bar',
381+
]);
382+
$presigned = $sig->presign($req, $creds, '+5 minutes');
383+
$uri = (string)$presigned->getUri();
384+
$this->assertStringNotContainsString('content-type', $uri);
385+
$this->assertStringNotContainsString('Content-Type', $uri);
386+
$this->assertStringNotContainsString('user-agent', $uri);
387+
$this->assertStringNotContainsString('User-Agent', $uri);
388+
// x-amz-meta-foo is a normal x-amz-* header that should still be
389+
// moved to query and signed.
390+
$this->assertStringContainsString('x-amz-meta-foo', $uri);
391+
}
392+
393+
322394
public function testCanonicalQuerySortingOccursAfterEncoding()
323395
{
324396
$_SERVER['aws_time'] = '20110909T233600Z';

0 commit comments

Comments
 (0)