Skip to content

Conversation

smswz
Copy link
Contributor

@smswz smswz commented Jun 8, 2022

Issue #, if available:

Description of changes:
Add an AES keyring that is compatible with S3 encryption client. Add skeleton S3 client

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@smswz smswz requested a review from texastony June 8, 2022 16:13
Copy link
Contributor

@texastony texastony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have lots of questions from this PR...

I probably am under-informed about what the S3EC specification is.
If it supports multiple EDKs, then things here will not work as is.
If it does not, then this is really close, but we need to stop using lists of EDKs.

I'll see if I can grab time on your calendar tomorrow to sync about this all.

Copy link
Contributor

@texastony texastony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have 4 more blocking comments.
I left suggestions for all 4 of them.
There are 2 still open from the last review.

* Allow AESKeyring to take a `SecureRandom` parameter.
* Remove 'what' comments from AESKeyring.
* Rename algorithm suite name to match spec.
* Rename decrypt materials function to match spec.
Copy link
Contributor

@texastony texastony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smswz smswz merged commit 7f131e2 into aws:main Jun 15, 2022
imabhichow added a commit that referenced this pull request Aug 17, 2022
akareddy04 added a commit that referenced this pull request Jul 14, 2025
Amazon S3 Encryption Client 3.3.5 Release -- 2025-05-20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants