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

Send md5 of customer key #217

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robertstrache
Copy link

This PR should send the MD5 checksum of a customer provided encryption key.

It is taken from a comment of another user in vmware-tanzu/velero#7693
I have just turned it into a PR since it seems to solve our issue described in
vmware-tanzu/velero#7693

Signed-off-by: Robert Strache <[email protected]>

Signed-off-by: Robert Strache <[email protected]>
Signed-off-by: Robert Strache <[email protected]>
Signed-off-by: Robert Strache <[email protected]>

Signed-off-by: Robert Strache <[email protected]>
reasonerjt
reasonerjt previously approved these changes Oct 22, 2024
Copy link
Contributor

@reasonerjt reasonerjt left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -267,6 +282,7 @@ func (o *ObjectStore) PutObject(bucket, key string, body io.Reader) error {
case o.sseCustomerKey != "":
input.SSECustomerAlgorithm = aws.String("AES256")
input.SSECustomerKey = &o.sseCustomerKey
input.SSECustomerKeyMD5 = &o.sseCustomerKeyMd5
Copy link
Contributor

@reasonerjt reasonerjt Oct 22, 2024

Choose a reason for hiding this comment

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

Will the func "GetObject" and "ObjectExists" also need this change?

@reasonerjt reasonerjt dismissed their stale review October 22, 2024 06:32

I have some question, so undoing the approval.

@gschei
Copy link

gschei commented Nov 10, 2024

This PR has some code duplication (readCustomerKey is called twice now), also comment was correct, that this is missing for head and get operations. I created a new PR for this: #225

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.

3 participants