diff --git a/.changes/nextrelease/fix-signature-ordering.json b/.changes/nextrelease/fix-signature-ordering.json new file mode 100644 index 0000000000..e5a4540afc --- /dev/null +++ b/.changes/nextrelease/fix-signature-ordering.json @@ -0,0 +1,7 @@ +[ + { + "type": "bugfix", + "category": "Signature", + "description": "Ensure SignatureV4 sorts query parameters by their URL-encoded names before canonicalization, so array-style keys like param[10] no longer disrupt the canonical order and break signature validation." + } +] diff --git a/src/Signature/SignatureV4.php b/src/Signature/SignatureV4.php index 74c3940fcc..a7b9212215 100644 --- a/src/Signature/SignatureV4.php +++ b/src/Signature/SignatureV4.php @@ -346,7 +346,9 @@ private function getCanonicalizedQuery(array $query) } $qs = ''; - ksort($query); + uksort($query, static function (string $a, string $b): int { + return strcmp(rawurlencode($a), rawurlencode($b)); + }); foreach ($query as $k => $v) { if (!is_array($v)) { $qs .= rawurlencode($k) . '=' . rawurlencode($v !== null ? $v : '') . '&'; diff --git a/tests/Signature/SignatureV4Test.php b/tests/Signature/SignatureV4Test.php index cdadd8b37c..9de7aaaf65 100644 --- a/tests/Signature/SignatureV4Test.php +++ b/tests/Signature/SignatureV4Test.php @@ -107,6 +107,21 @@ private function getFixtures() return array($request, $credentials, $signature); } + private function decodeQueryPairs(string $query): array + { + if ($query === '') { + return []; + } + + $pairs = []; + foreach (explode('&', $query) as $pair) { + $parts = explode('=', $pair, 2); + $pairs[] = array_map('rawurldecode', $parts + ['', '']); + } + + return $pairs; + } + public function getExpiresDateTimeInterfaceInputs() { return [ @@ -312,6 +327,57 @@ public function testPresignBlacklistedHeaders() $this->assertStringNotContainsString('Content-Type', (string)$presigned->getUri()); } + public function testCanonicalQuerySortingOccursAfterEncoding() + { + $_SERVER['aws_time'] = '20110909T233600Z'; + + $signature = new SignatureV4('service', 'region'); + $credentials = new Credentials(self::DEFAULT_KEY, self::DEFAULT_SECRET); + $query = 'param[0]=1111¶m[1]=1111¶m[10]=1111¶m[2]=1111¶m[3]=1111¶m[4]=1111¶m[5]=1111¶m[6]=1111¶m[7]=1111¶m[8]=1111¶m[9]=1111'; + $request = new Request('GET', 'https://example.com/service?' . $query); + + $parseRequest = new \ReflectionMethod($signature, 'parseRequest'); + $parseRequest->setAccessible(true); + $parsedRequest = $parseRequest->invoke($signature, $request); + + $getPayload = new \ReflectionMethod($signature, 'getPayload'); + $getPayload->setAccessible(true); + $payload = $getPayload->invoke($signature, $request); + + $createContext = new \ReflectionMethod($signature, 'createContext'); + $createContext->setAccessible(true); + $context = $createContext->invoke($signature, $parsedRequest, $payload); + + $canonicalQuery = explode("\n", $context['creq'])[2]; + $expectedPairs = [ + ['param[0]', '1111'], + ['param[1]', '1111'], + ['param[10]', '1111'], + ['param[2]', '1111'], + ['param[3]', '1111'], + ['param[4]', '1111'], + ['param[5]', '1111'], + ['param[6]', '1111'], + ['param[7]', '1111'], + ['param[8]', '1111'], + ['param[9]', '1111'], + ]; + + $this->assertSame($expectedPairs, $this->decodeQueryPairs($canonicalQuery)); + + $expectedCanonical = "GET\n/service\n" . $canonicalQuery . "\nhost:example.com\n\nhost\n" . $payload; + $this->assertSame($expectedCanonical, $context['creq']); + + $signed = $signature->signRequest($request, $credentials); + $this->assertSame($expectedPairs, $this->decodeQueryPairs($signed->getUri()->getQuery())); + + $expectedSigned = "GET /service?param%5B0%5D=1111¶m%5B1%5D=1111¶m%5B10%5D=1111¶m%5B2%5D=1111¶m%5B3%5D=1111¶m%5B4%5D=1111¶m%5B5%5D=1111¶m%5B6%5D=1111¶m%5B7%5D=1111¶m%5B8%5D=1111¶m%5B9%5D=1111 HTTP/1.1\r\n" + . "Host: example.com\r\n" + . "X-Amz-Date: 20110909T233600Z\r\n" + . "Authorization: AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20110909/region/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=db59e82a5b69e8d4a0fb1a6ceff6a0ff7beca86ac5531b8c69fa93ea0028634b\r\n\r\n"; + $this->assertSame($expectedSigned, Psr7\Message::toString($signed)); + } + public function testEnsuresContentSha256CanBeCalculated() { $this->expectException(\Aws\Exception\CouldNotCreateChecksumException::class);