Implement config encrypt feature

[yanganto]

• set up run time secret by option
• set up run time sceret in s3rs shell
• encrypt and/or save s3rs config
• decrypt and/or save s3rs config
• fix #59

[yanganto]

The feature branch is using strip_prefix, which can only be built in Rust nightly version after June 2020. Relate issue in Rust is #67302, the feature is merged and will be released in July 16th 2020 (version 1.45.0). This feature branch will be merged when the function becomes stable.

[EmanuelKuhn]

Hey @yanganto, I saw your request for a codereview on gitcoin and thought I would have a look.

The main issue I saw is that your xor encryption scheme is not secure. Specifically it is vulnerable to a known plaintext attack. Some of the fields which you encrypt can probably be guessed (hostname, region), then the secret can trivially be obtained by xor'ing the cipher text with guess. Because the secret is reused for all fields, all fields can then be decrypted after the value of one field was guessed correctly.

I think that in general it's not recommended to try to implement your own encryption algorithm as there are many potential ways a crypto algorithm can break.

I would recommend looking for an open source cryptography library and use an existing symmetric encryption algorithm.

[yanganto]

@EmanuelKuhn Thank you for your review. May you send your ethereum address to me? Thanks.

[EmanuelKuhn]

Hey, sorry for the slightly late reply. My eth address is 0x1f1e2F6BC42Ef6b48bcD83E0e471fA741f9D3443.

Generating unique keys seems a lot better than what you had initially, but you should still be aware of the following:

• With this implementation you have to be careful not to accidentally reuse the target string for more than one field
• Because it's not authenticated it doesn't defend against changing the content of an encrypted field without knowing the password. For example if you know/guess what the previous hostname is you change the hostname by flipping the right bits
• It's really is to crack for a short password/seed, so you might want to add a check for a minimum length

[yanganto]

Thanks for keeping reviewing this PR. I will implement the minimum length check, and need to time to think about your other opinions. I appreciated. :)

[yanganto]

There are at least two things that need to be done about this feature.

• [X] secrete length check There may be 255 char in the host field, and 5 fields will be encrypted. 5^2 * 255 ~ 2^12, so the best secret should be greater than 12 (this will implement as a warning, not mandatory)
• [x] fill bytes to the same length and rotate it Filling the byte not used in the char and rotate it, so the ciphertext will not disclose the length of the original text

[kedMertens]

Not related to the issue, I found that your link to patreon is actually leads to home page of patreon.

[kedMertens]

@yanganto, what do you think will it be possible to specify what encryption technique is in use in help, so that a user could have an idea what secret should be used to achieve higher security?

[yanganto]

Many thanks for your review.

Not related to the issue, I found that your link to patreon is actually leads to home page of patreon.

https://github.com/yanganto/s3rs/issues/65

[yanganto]

@yanganto, what do you think will it be possible to specify what encryption technique is in use in help, so that a user could have an idea what secret should be used to achieve higher security?

That's cool. I open an issue for this. :) https://github.com/yanganto/s3rs/issues/66

[yanganto]

@kedMertens Thanks for your review. I fix the problems in this issue, and some parts open other issues to track them. Do you think this PR is good for merge or there is still something we need to fix?

If you are interested in this project, it is welcome to implement things you like or to open an issue on this project? Thanks.

[kedMertens]

@yanganto, I think it can be merged. If something will come up new issues can be opened. Thanks I’ll follow the project👍