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

cmd: Add encrypt to enable enc/decrypt objects #2140

Closed
wants to merge 1 commit into from

Conversation

vadmeste
Copy link
Member

@vadmeste vadmeste commented Apr 23, 2017

Fixes #2074

@codecov-io
Copy link

codecov-io commented Apr 23, 2017

Codecov Report

Merging #2140 into master will decrease coverage by 0.08%.
The diff coverage is 4.39%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2140      +/-   ##
=========================================
- Coverage     8.9%   8.82%   -0.09%     
=========================================
  Files          92      92              
  Lines        6870    7005     +135     
=========================================
+ Hits          612     618       +6     
- Misses       6126    6251     +125     
- Partials      132     136       +4
Impacted Files Coverage Δ
cmd/globals.go 0% <ø> (ø) ⬆️
cmd/utils.go 18.98% <0%> (-6.44%) ⬇️
cmd/config-migrate.go 0% <0%> (ø) ⬆️
cmd/config.go 21.95% <0%> (ø) ⬆️
cmd/config-host-add.go 0% <0%> (ø) ⬆️
cmd/config-host-list.go 0% <0%> (ø) ⬆️
cmd/config-old.go 0% <0%> (ø) ⬆️
cmd/config-v9.go 0% <0%> (ø)
cmd/config-host.go 0% <0%> (ø) ⬆️
cmd/config-validate.go 0% <0%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fed9ebe...f8c4aac. Read the comment docs.

@harshavardhana
Copy link
Member

Is this still a WIP @vadmeste ?

@vadmeste vadmeste changed the title [WIP] cmd: Add encrypt to enable enc/decrypt objects cmd: Add encrypt to enable enc/decrypt objects Apr 26, 2017
@vadmeste
Copy link
Member Author

vadmeste commented Apr 26, 2017

I made more simple changes, this PR can be reviewed now.

@harshavardhana
Copy link
Member

Also I need to think if we should add an API in minio-go which returns the real size of an object after being decrypted.

why is this needed?

@harshavardhana
Copy link
Member

Keeping this waiting on @abperiasamy - its blocked to finalize the approach here.

@vadmeste
Copy link
Member Author

The size of an object in encrypted form is different from the original size. mc compares the size of the downloaded data (data are clear because minio-go decrypts the stream on the fly) and finds that the size of the downloaded stream is different to the size of the same object in the server (using client-fs.Stat() primitive)

@bollig
Copy link

bollig commented May 31, 2017

Any updates on this? Encrypt is a feature we're excited to leverage.

@harshavardhana
Copy link
Member

@vadmeste after discussing with @abperiasamy - we decided that we don't need to do any config changes and it was simplified further.

For the time being we will only support AES256 encryption on the client side which will also work with aes256-server when the server side implementation comes in. Only commands which require this change are listed below.

- mc cp --encrypt-key=<key> --encrypt-type=aes256-server /etc/passwd myminio/bucket/
- mc cp -r --encrypt-key=<key> --encrypt-type=aes256 s3/bucket1/encrypted-objects myminio/bucket/encrypted-objects
- mc mirror --encrypt-key|-e=<key> --encrypt-type=aes256-server myminio/ s3/
- mc cat, mc pipe 
`-e` or `--encrypt-key` if has no key then we need to prompt on the command line. 

With this we can also support now FS to FS, FS to S3, S3 to FS decryption and encryption transparently.

@harshavardhana
Copy link
Member

Unblocked this PR since we have finalized the approach.

@vadmeste
Copy link
Member Author

mc mirror with encryption capability is weird because sometimes the size of encrypted object size is different to the size of plain representation. I'll investigate more.

@fwessels
Copy link
Contributor

Have seen stuff like this before, has to do with working on blocks of eg 32 or 64 byte size (depending on algorithm) whereby the final block is likely to be padded with zeros. (And so the encrypted stream may be a little longer).

@vadmeste
Copy link
Member Author

Have seen stuff like this before, has to do with working on blocks of eg 32 or 64 byte size (depending on algorithm) whereby the final block is likely to be padded with zeros. (And so the encrypted stream may be a little longer).

Yes. But you can't know the exact length of the plain data from the length of encrypted data (you can with the reverse way). Though as you said padding will make the size of the encrypted data little longer (the maximum is the original size + block size).

I'll make the first step first then will see how to make this perfect.

@vadmeste
Copy link
Member Author

Closing this in favor of #2197

@vadmeste vadmeste closed this Jun 27, 2017
@vadmeste vadmeste deleted the issue/2074 branch May 29, 2018 20:34
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.

None yet

5 participants