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

Add server side encryption support #259

Merged
merged 1 commit into from
Jan 4, 2019
Merged

Conversation

poornas
Copy link
Contributor

@poornas poornas commented Dec 4, 2018

Fixes #251

@poornas poornas force-pushed the encrypt branch 3 times, most recently from 5bda1e1 to d71355b Compare December 7, 2018 19:40
@poornas
Copy link
Contributor Author

poornas commented Dec 13, 2018

@harshavardhana , review comments were fixed.PTAL

Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

Key needs to be converted to byte array of length 32 before being set.

Docs/API.md Outdated Show resolved Hide resolved
Docs/API.md Outdated Show resolved Hide resolved
Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

We should still provide an example with PBKDF for ease of usability - I can't see where its mentioned in this PR.

@poornas
Copy link
Contributor Author

poornas commented Dec 17, 2018

@harshavardhana, updated the example in minio.examples to api.md as well.

@poornas
Copy link
Contributor Author

poornas commented Dec 19, 2018

rebased to master. PTAL @kannappanr && @harshavardhana

@poornas poornas force-pushed the encrypt branch 2 times, most recently from 200dc44 to 3b5e0dd Compare January 3, 2019 16:55
@poornas
Copy link
Contributor Author

poornas commented Jan 3, 2019

@harshavardhana and @kannappanr, PTAL

Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

Some formatting changes need to be made.

@@ -0,0 +1,166 @@
/*
* Minio .NET Library for Amazon S3 Compatible Cloud Storage, (C) 2017 Minio, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Year should be 2019

/// <param name="cancellationToken">Optional cancellation token to cancel the operation</param>
Task GetObjectAsync(string bucketName, string objectName, long offset, long length, Action<Stream> cb, CancellationToken cancellationToken = default(CancellationToken));
Task GetObjectAsync(string bucketName, string objectName, long offset, long length, Action<Stream> cb, ServerSideEncryption sse = null,CancellationToken cancellationToken = default(CancellationToken));
Copy link
Contributor

Choose a reason for hiding this comment

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

need space between ServerSideEncryption sse = null,CancellationToken

/// <param name="cancellationToken">Optional cancellation token to cancel the operation</param>
Task GetObjectAsync(string bucketName, string objectName, Action<Stream> callback, CancellationToken cancellationToken = default(CancellationToken));
Task GetObjectAsync(string bucketName, string objectName, Action<Stream> callback, ServerSideEncryption sse = null,CancellationToken cancellationToken = default(CancellationToken));
Copy link
Contributor

Choose a reason for hiding this comment

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

need space between ServerSideEncryption sse = null,CancellationToken

EncryptedCopyObject_Test1(minioClient).Wait();
EncryptedCopyObject_Test2(minioClient).Wait();

// Uncommment these tests when Mint supports Vault/KMS
Copy link
Contributor

@kannappanr kannappanr Jan 3, 2019

Choose a reason for hiding this comment

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

Can we use an environment variable here which can also be used in Mint, instead of commenting out this piece of code?

/// <param name="cancellationToken">Optional cancellation token to cancel the operation</param>
public async Task GetObjectAsync(string bucketName, string objectName, Action<Stream> cb, CancellationToken cancellationToken = default(CancellationToken))
public async Task GetObjectAsync(string bucketName, string objectName, Action<Stream> cb, ServerSideEncryption sse = null,CancellationToken cancellationToken = default(CancellationToken))
Copy link
Contributor

Choose a reason for hiding this comment

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

need space between ServerSideEncryption sse = null,CancellationToken

Minio/ApiEndpoints/ObjectOperations.cs Show resolved Hide resolved
Minio/ApiEndpoints/ObjectOperations.cs Show resolved Hide resolved
Minio/ApiEndpoints/ObjectOperations.cs Show resolved Hide resolved
/// <param name="cancellationToken">Optional cancellation token to cancel the operation</param>
/// <returns>Facts about the object</returns>
public async Task<ObjectStat> StatObjectAsync(string bucketName, string objectName, CancellationToken cancellationToken = default(CancellationToken))
public async Task<ObjectStat> StatObjectAsync(string bucketName, string objectName, ServerSideEncryption sse = null,CancellationToken cancellationToken = default(CancellationToken))
Copy link
Contributor

Choose a reason for hiding this comment

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

Need space here ServerSideEncryption sse = null,CancellationToken

Minio/ApiEndpoints/ObjectOperations.cs Show resolved Hide resolved
@poornas
Copy link
Contributor Author

poornas commented Jan 3, 2019

@kannappanr, incorporated your feedback.

Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

LGTM

@kannappanr kannappanr merged commit 794aa9a into minio:master Jan 4, 2019
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