Skip to content
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

TransferManager losing encryption related metadata when writing with AmazonS3Encryption #2311

Closed
tiddman opened this issue May 4, 2020 · 4 comments
Assignees
Labels
closed-for-staleness guidance Question that needs advice or information. response-requested Waiting on additional info or feedback. Will move to "closing-soon" in 5 days.

Comments

@tiddman
Copy link

tiddman commented May 4, 2020

We need to transfer some large files to S3 and encrypt them during transfer with client-side encryption (can't use KMS for this case, which spans accounts, have a specific requirement to use client-side encryption). I'd like to use AWS TransferManager to take advantage of the multi-part uploads and other functionality.

The AmazonS3Encryption client will write two meta-data fields x-amz-iv (initialization vector) and x-amz-key (content key) to the S3 location, and these two fields are needed to decrypt the file.

However, the TransferManager loses these metadata values and does not write them to the location. I am trying to see if there is some way that I can get to them and either cause them to be written or add them to the location after the transfer is complete.

I found this old issue which seems to be very closely related, it is pretty old and I think the API's have changed since then, and the suggested workarounds don't work for me because I cannot find a way to get the metadata fields from the s3 client.

#367

My understanding is that the content key is encrypted for each s3 client and the IV is provided for each file, so I would need to get to the client immediately after it has written each file.

Per the AWS docs I an creating an AmazonS3Encryption like this:

AmazonS3Encryption s3Encryption = AmazonS3EncryptionClientBuilder
    .standard()
    .withRegion(Regions.US_WEST_2)
    .withCryptoConfiguration(new CryptoConfiguration(CryptoMode.EncryptionOnly))
    .withEncryptionMaterials(new StaticEncryptionMaterialsProvider(new EncryptionMaterials(secretKey)))
    .build();

And then building the TransferManager like this:

TransferManager transferManager =
    TransferManagerBuilder.standard().withS3Client(s3Encryption).build();

I have tried transferManager.upload() and also the variants such as uploadFileList() which allows me to provide ObjectMetadataProvider but I cannot find a way to get the metadata to write.

I have found what I think is the smoking gun, the UploadCallable class in AWS:
https://github.com/aws/aws-sdk-java/blob/ccfd63c097874e3a1e9ffda7bf171769b6bd3b21/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/transfer/internal/UploadCallable.java#L131

private UploadResult uploadInOneChunk() {
    PutObjectResult putObjectResult = s3.putObject(origReq);

    UploadResult uploadResult = new UploadResult();
    uploadResult.setBucketName(origReq.getBucketName());
    uploadResult.setKey(origReq.getKey());
    uploadResult.setETag(putObjectResult.getETag());
    uploadResult.setVersionId(putObjectResult.getVersionId());
    return uploadResult;
}

The PutObjectResult returned from s3 contains the metadata I need, but unfortunately it is just ignored and a different result is returned.

I have been digging around through the code to try to find a hook to do this without resorting to awful Java Reflection hacks. So far I have tried:

  • Subclassing AmazonS3EncryptionClient, which can't be done easily because its builder class is public final (could maybe use the deprecated constructors)
  • Providing an alternate UploadCallable to TransferManager, can't find any hook for that
  • Getting to the AmazonS3EncryptionClient instance's internal crypto classes which hold the values that I need, but they are all private and buried several levels deep
    etc.

At this juncture I think my only option is to either roll my own TransferManager or just live with a direct s3 client, but this seems like a pretty significant shortcoming in the AWS client so I am wondering if I am missing something. Thanks for any help.

Your Environment

  • AWS Java SDK version used: 1.11.199
  • JDK version used: 11.0.3
  • Operating System and version: Mac Catalina
@tiddman tiddman added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels May 4, 2020
@aaoswal
Copy link

aaoswal commented Jan 14, 2022

@tiddman I'm sorry for loosing track of this. Are you still experiencing the issue?

@aaoswal aaoswal added response-requested Waiting on additional info or feedback. Will move to "closing-soon" in 5 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 14, 2022
@aaoswal aaoswal self-assigned this Jan 14, 2022
@tiddman
Copy link
Author

tiddman commented Jan 17, 2022 via email

@github-actions github-actions bot removed the response-requested Waiting on additional info or feedback. Will move to "closing-soon" in 5 days. label Jan 17, 2022
@aaoswal
Copy link

aaoswal commented Jan 19, 2022

Hi @tiddman,
On deeper investigation, I apologize but retaining meta-data values by adding putObjectResult.getMetadata() to the UploadResult in uploadInOneChunk() is currently not supported and hence, TransferManager does not support the Encryption Client when not using KMS.

I would advise you to open a feature request for the same so that it could be prioritized in future versions.
Also, if you would like to contribute and implement it yourself, you could also submit a PR that could be reviewed and implemented in the SDK itself.

@aaoswal aaoswal added the response-requested Waiting on additional info or feedback. Will move to "closing-soon" in 5 days. label Jan 19, 2022
@github-actions
Copy link

It looks like this issue has not been active for more than five days. In the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please add a comment to prevent automatic closure, or if the issue is already closed please feel free to reopen it.

@github-actions github-actions bot added closing-soon This issue will close in 2 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will close in 2 days unless further comments are made. labels Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-for-staleness guidance Question that needs advice or information. response-requested Waiting on additional info or feedback. Will move to "closing-soon" in 5 days.
Projects
None yet
Development

No branches or pull requests

2 participants