New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSEC on multipart uploads not passing parameters or encoding #1312

Closed
gabriel403 opened this Issue Jun 19, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@gabriel403
Copy link

gabriel403 commented Jun 19, 2017

When trying to upload large files with SSEC I'm getting an error:

 (Aws\\S3\\Exception\\S3MultipartUploadException(code: 0): An exception occurred while uploading parts to a multipart upload. The following parts had errors:
- Part 1: Error executing \"UploadPart\" on \"https://s3-eu-west-1.amazonaws.com/bucket-name/folder-name/filename.csv?partNumber=1&uploadId=someid\"; AWS HTTP error: Client error: `PUT https://s3-eu-west-1.amazonaws.com/bucket-name/folder-name/filename.csv?partNumber=1&uploadId=someid` resulted in a `400 Bad Request` response:
<?xml version=\"1.0\" encoding=\"UTF-8\"?>
<Error><Code>InvalidRequest</Code><Message>The multipart upload initiate requeste (truncated...)
 InvalidRequest (client): The multipart upload initiate requested encryption. Subsequent part requests must include the appropriate encryption parameters.

This pull request #1291 is purported to fix this issue and when I try it, the message no longer appears. However when checking the files uploaded they definitely aren't encrypted.
Doing a dump of $command at this point https://github.com/aws/aws-sdk-php/blob/master/src/S3/SSECMiddleware.php#L49 shows the expected SSEC keys for the first round, but when the middleware is hit for the subsequent parts they are missing.

If I upload a small file (less than 16mb iirc), it's successfully encrypted

Sample code

try {
    $fileKey = 'very-large-file.csv';
    $stream = fopen('/var/tmp/file/directory/very-large-file.csv', 'rb');

    $container['s3Client']->upload(
        getenv('S3_BUCKET_FILES'),
        $fileKey,
        $stream,
        'private',
        ['params' =>
             [
                'SSECustomerAlgorithm' => "AES256",
                'SSECustomerKey'       => base64_encode(getenv('S3_ENC_PHRASE')),
                'SSECustomerKeyMD5'    => md5(base64_encode(getenv('S3_ENC_PHRASE')), true),
            ]
        ]
    );
} finally {
    fclose($stream);
}
@kstich

This comment has been minimized.

Copy link
Contributor

kstich commented Jun 19, 2017

@gabriel403 Thank you for posting this issue and giving some conditions relevant to #1291. Is this happening for any (16mb+) file with this method after updating from the PR? And confirming I'm reading your details correctly: after updating from the PR, the exception no longer occurs, but the file is not encrypted according to the passed parameters?

@gabriel403

This comment has been minimized.

Copy link

gabriel403 commented Jun 20, 2017

@kstich when using the sdk I get the exception printed above, when using the fork in #1291 I don't get the exception and the file uploads successfully, but the file is not encrypted using SSEC. Small files do encrypt just fine using the sdk and the fork from the other pr.

@kstich kstich added bug and removed guidance labels Jun 21, 2017

@kstich

This comment has been minimized.

Copy link
Contributor

kstich commented Jun 21, 2017

@gabriel403 We've confirmed this issue, thank you again for the details. We are currently working on a fix for the issue and expect it to be available soon. We will be closing #1291 because it breaks the contract that ObjectUploader makes in its constructor with regards to $this->options and filing a new PR.

@gabriel403

This comment has been minimized.

Copy link

gabriel403 commented Jun 22, 2017

@kstich good news, thanks, anything else I can do to help just give me a nudge

@kstich

This comment has been minimized.

Copy link
Contributor

kstich commented Jun 22, 2017

@gabriel403 Please see #1313 for the fix for this bug and additional functionality for the ObjectUploader/MultipartUploader abstractions.

@kstich

This comment has been minimized.

Copy link
Contributor

kstich commented Jun 30, 2017

@gabriel403 These options should now be passing through as #1313 was merged in to 3.31.0. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment