Fix wrong signature in SES (#567)

* Fix wrong signature in SES

* Improve comment

* Switch to 1.1

* Fix CS

* Typo

* Remove BC code and conflict with S3<1.1

* Update changelog

* Fix CS

* Replace ref variable by context object

* Reword changelog

* Reword

Author: jderusse

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Fixed
 
+- Fixed invalid chunking of request with large body for most clients but S3. This version removed the invalid code from SignerV4 to make sure requests are not chunked.
 - Use camelCase for all getter methods.
 
 ## 1.0.0

composer.json

@@ -20,6 +20,9 @@
         "symfony/http-client-contracts": "^1.1.8 || ^2.0",
         "symfony/service-contracts": "^1.0 || ^2.0"
     },
+    "conflict": {
+        "async-aws/s3": "<1.1"
+    },
     "extra": {
         "branch-alias": {
             "dev-master": "1.1-dev"

src/Signer/SignerV4.php

@@ -6,10 +6,7 @@
 use AsyncAws\Core\Exception\InvalidArgument;
 use AsyncAws\Core\Request;
 use AsyncAws\Core\RequestContext;
-use AsyncAws\Core\Stream\FixedSizeStream;
-use AsyncAws\Core\Stream\IterableStream;
 use AsyncAws\Core\Stream\ReadOnceResultStream;
-use AsyncAws\Core\Stream\RequestStream;
 use AsyncAws\Core\Stream\RewindableStream;
 use AsyncAws\Core\Stream\StringStream;
 
@@ -21,8 +18,6 @@
 class SignerV4 implements Signer
 {
     private const ALGORITHM_REQUEST = 'AWS4-HMAC-SHA256';
-    private const ALGORITHM_CHUNK = self::ALGORITHM_REQUEST . '-PAYLOAD';
-    private const CHUNK_SIZE = 64 * 1024;
 
     private const BLACKLIST_HEADERS = [
         'cache-control' => true,
@@ -101,6 +96,12 @@ protected function buildBodyDigest(Request $request, bool $isPresign): string
         return $hash;
     }
 
+    protected function convertBodyToStream(SigningContext $context): void
+    {
+        $request = $context->getRequest();
+        $request->setBody(StringStream::create($request->getBody()));
+    }
+
     private function handleSignature(Request $request, Credentials $credentials, \DateTimeImmutable $now, \DateTimeImmutable $expires, bool $isPresign): void
     {
         $this->removePresign($request);
@@ -109,16 +110,17 @@ private function handleSignature(Request $request, Credentials $credentials, \Da
 
         $this->buildTime($request, $now, $expires, $isPresign);
         $credentialScope = $this->buildCredentialString($request, $credentials, $now, $isPresign);
-        $credentialString = \implode('/', $credentialScope);
-        $signingKey = $this->buildSigningKey($credentials, $credentialScope);
-
+        $context = new SigningContext(
+            $request,
+            $now,
+            \implode('/', $credentialScope),
+            $this->buildSigningKey($credentials, $credentialScope)
+        );
         if ($isPresign) {
             // Should be called before `buildBodyDigest` because this method may alter the body
             $this->convertBodyToQuery($request);
         } else {
-            // $signature does not exists but passed by reference then computed buildSignature
-            $signature = '';
-            $this->convertBodyToStream($request, $now, $credentialString, $signingKey, $signature);
+            $this->convertBodyToStream($context);
         }
 
         $bodyDigest = $this->buildBodyDigest($request, $isPresign);
@@ -130,8 +132,8 @@ private function handleSignature(Request $request, Credentials $credentials, \Da
 
         $canonicalHeaders = $this->buildCanonicalHeaders($request, $isPresign);
         $canonicalRequest = $this->buildCanonicalRequest($request, $canonicalHeaders, $bodyDigest);
-        $stringToSign = $this->buildStringToSign($now, $credentialString, $canonicalRequest);
-        $signature = $this->buildSignature($stringToSign, $signingKey);
+        $stringToSign = $this->buildStringToSign($context->getNow(), $context->getCredentialString(), $canonicalRequest);
+        $context->setSignature($signature = $this->buildSignature($stringToSign, $context->getSigningKey()));
 
         if ($isPresign) {
             $request->setQueryAttribute('X-Amz-Signature', $signature);
@@ -254,57 +256,6 @@ private function convertBodyToQuery(Request $request): void
         $request->setBody(StringStream::create(''));
     }
 
-    private function convertBodyToStream(Request $request, \DateTimeImmutable $now, string $credentialString, string $signingKey, string &$signature): void
-    {
-        $body = $request->getBody();
-        if ($request->hasHeader('content-length')) {
-            $contentLength = (int) $request->getHeader('content-length');
-        } else {
-            $contentLength = $body->length();
-        }
-
-        // If content length is unknown, use the rewindable stream to read it once locally in order to get the length
-        if (null === $contentLength) {
-            $request->setBody($body = RewindableStream::create($body));
-            $body->read();
-            $contentLength = $body->length();
-        }
-
-        // no need to stream small body. It's simple to convert it to string directly
-        if ($contentLength < self::CHUNK_SIZE) {
-            $request->setBody($body = StringStream::create($body));
-
-            return;
-        }
-
-        // Convert the body into a chunked stream
-        $request->setHeader('content-encoding', 'aws-chunked');
-        $request->setHeader('x-amz-decoded-content-length', (string) $contentLength);
-        $request->setHeader('x-amz-content-sha256', 'STREAMING-' . self::ALGORITHM_CHUNK);
-
-        // Compute size of content + metadata used sign each Chunk
-        $chunkCount = (int) ceil($contentLength / self::CHUNK_SIZE);
-        $fullChunkCount = $chunkCount * self::CHUNK_SIZE === $contentLength ? $chunkCount : ($chunkCount - 1);
-        $metaLength = \strlen(";chunk-signature=\r\n\r\n") + 64;
-        $request->setHeader('content-length', (string) ($contentLength + $fullChunkCount * ($metaLength + \strlen((string) dechex(self::CHUNK_SIZE))) + ($chunkCount - $fullChunkCount) * ($metaLength + \strlen((string) dechex($contentLength % self::CHUNK_SIZE))) + $metaLength + 1));
-
-        $body = IterableStream::create((function (RequestStream $body) use ($now, $credentialString, $signingKey, &$signature): iterable {
-            foreach (FixedSizeStream::create($body, self::CHUNK_SIZE) as $chunk) {
-                $stringToSign = $this->buildChunkStringToSign($now, $credentialString, $signature, $chunk);
-                $signature = $this->buildSignature($stringToSign, $signingKey);
-
-                yield sprintf("%s;chunk-signature=%s\r\n", dechex(\strlen($chunk)), $signature) . "$chunk\r\n";
-            }
-
-            $stringToSign = $this->buildChunkStringToSign($now, $credentialString, $signature, '');
-            $signature = $this->buildSignature($stringToSign, $signingKey);
-
-            yield sprintf("%s;chunk-signature=%s\r\n\r\n", dechex(0), $signature);
-        })($body));
-
-        $request->setBody($body);
-    }
-
     private function buildCanonicalHeaders(Request $request, bool $isPresign): array
     {
         // Case-insensitively aggregate all of the headers.
@@ -393,21 +344,6 @@ private function buildStringToSign(\DateTimeImmutable $now, string $credentialSt
         ]);
     }
 
-    private function buildChunkStringToSign(\DateTimeImmutable $now, string $credentialString, string $signature, string $chunk): string
-    {
-        static $emptyHash;
-        $emptyHash = $emptyHash ?? hash('sha256', '');
-
-        return implode("\n", [
-            self::ALGORITHM_CHUNK,
-            $now->format('Ymd\THis\Z'),
-            $credentialString,
-            $signature,
-            $emptyHash,
-            hash('sha256', $chunk),
-        ]);
-    }
-
     private function buildSigningKey(Credentials $credentials, array $credentialScope): string
     {
         $signingKey = 'AWS4' . $credentials->getSecretKey();

src/Signer/SigningContext.php

@@ -0,0 +1,65 @@
+<?php
+
+namespace AsyncAws\Core\Signer;
+
+use AsyncAws\Core\Request;
+
+/**
+ * @internal
+ *
+ * @author Jérémy Derussé <jeremy@derusse.com>
+ */
+class SigningContext
+{
+    private $request;
+
+    private $now;
+
+    private $credentialString;
+
+    private $signingKey;
+
+    private $signature = '';
+
+    public function __construct(
+        Request $request,
+        \DateTimeImmutable $now,
+        string $credentialString,
+        string $signingKey
+    ) {
+        $this->request = $request;
+        $this->now = $now;
+        $this->credentialString = $credentialString;
+        $this->signingKey = $signingKey;
+    }
+
+    public function getRequest(): Request
+    {
+        return $this->request;
+    }
+
+    public function getNow(): \DateTimeImmutable
+    {
+        return $this->now;
+    }
+
+    public function getCredentialString(): string
+    {
+        return $this->credentialString;
+    }
+
+    public function getSigningKey(): string
+    {
+        return $this->signingKey;
+    }
+
+    public function getSignature(): string
+    {
+        return $this->signature;
+    }
+
+    public function setSignature(string $signature): void
+    {
+        $this->signature = $signature;
+    }
+}